[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87ldhxubt5.fsf@bootlin.com>
Date: Fri, 16 Jan 2026 10:49:42 +0100
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
Cc: Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Geert Uytterhoeven <geert+renesas@...der.be>,
Magnus Damm <magnus.damm@...il.com>, Vaishnav Achath
<vaishnav.a@...com>, Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
Hervé Codina <herve.codina@...tlin.com>, Wolfram Sang
<wsa+renesas@...g-engineering.com>, Vignesh Raghavendra
<vigneshr@...com>, Santhosh Kumar K <s-k6@...com>, Pratyush Yadav
<pratyush@...nel.org>, Pascal Eberhard <pascal.eberhard@...com>,
linux-spi@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH v2 13/13] ARM: dts: r9a06g032: Describe the QSPI controller
Hi Geert,
>> + qspi0: spi@...05000 {
>> + compatible = "renesas,r9a06g032-qspi", "renesas,rzn1-qspi", "cdns,qspi-nor";
>> + reg = <0x40005000 0x1000>, <0x10000000 0x10000000>;
>> + interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&sysctrl R9A06G032_CLK_QSPI0>, <&sysctrl R9A06G032_HCLK_QSPI0>,
>> + <&sysctrl R9A06G032_HCLK_QSPI0>;
>> + clock-names = "ref", "ahb", "apb";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + cdns,fifo-width = <4>;
>
> <4> is the default, right?
It is the default in the bindings indeed, however the driver does not
imply that default and errors out if the property is missing. The
property is also marked required in the bindings, which is kind of
incorrect I guess. Also, all DTS explicitly set this value to 4.
However looking into the RM I found "Transmit and receive FIFOs are 16
bytes". I haven't tested that, I will.
>> + cdns,trigger-address = <0>;
>
> Where in the RZ/N1 docs can I find if these two properties are
> correct?
This property is mandatory. Maybe I could just discard it for my
compatible, because it is only relevant for indirect modes, which are
unsupported.
>> + status = "disabled";
>> + };
>> +
>> rtc0: rtc@...06000 {
>> compatible = "renesas,r9a06g032-rtc", "renesas,rzn1-rtc";
>> reg = <0x40006000 0x1000>;
>
> The rest LGTM, ignoring my comments on the bindings:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@...der.be>
Thanks for the review, but I guess if I end up changing the DTS snippet
I might drop it. Or would you like me to keep it anyway?
Thanks,
Miquèl
Powered by blists - more mailing lists