linux-kernel - Re: [PATCH v2 2/5] riscv: dts: starfive: jh7110-common: qspi flash setting read-delay 2 cycles max 100MHz

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <33b971dc-1266-4d13-b9b7-620c58e5a804@freeshell.de>
Date: Wed, 5 Feb 2025 05:21:11 -0800
From: E Shattow <e@...eshell.de>
To: Emil Renner Berthing <emil.renner.berthing@...onical.com>,
 Emil Renner Berthing <kernel@...il.dk>, Conor Dooley <conor@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>
Cc: linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 linux-riscv@...ts.infradead.org, Hal Feng <hal.feng@...rfivetech.com>
Subject: Re: [PATCH v2 2/5] riscv: dts: starfive: jh7110-common: qspi flash
 setting read-delay 2 cycles max 100MHz

Hi Emil,

On 2/5/25 02:18, Emil Renner Berthing wrote:
> E Shattow wrote:
>> Sync qspi flash setting to read-delay=2 and spi-max-frequency 100MHz for
>> better compatibility with operating system and downstream boot loader SPL
>> secondary program loader.
> 
> Here you should be explaining why these values better describe the hardware. To
> me this just reads as "u-boot does this, so let's do the same" whith doesn't
> really explain anything.
> 
> /Emil
> 
>>
>> Signed-off-by: E Shattow <e@...eshell.de>
>> Reviewed-by: Hal Feng <hal.feng@...rfivetech.com>
>> ---
>>  arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> index a5661b677687..8a59c3001339 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> @@ -317,8 +317,8 @@ &qspi {
>>  	nor_flash: flash@0 {
>>  		compatible = "jedec,spi-nor";
>>  		reg = <0>;
>> -		cdns,read-delay = <5>;
>> -		spi-max-frequency = <12000000>;
>> +		cdns,read-delay = <2>;
>> +		spi-max-frequency = <100000000>;
>>  		cdns,tshsl-ns = <1>;
>>  		cdns,tsd2d-ns = <1>;
>>  		cdns,tchsh-ns = <1>;
>> --
>> 2.47.2
>>

That is true, there's not much to explain. It works at delay 2 and
100MHz max. I tried many delay values with the pre-existing 12MHz max,
they did not work. I don't have more information than that except what I
wrote about on the U-Boot mailing list [1]

[1]
https://lore.kernel.org/u-boot/ZQ2PR01MB13072E932737DD9D65E3A250E637A@ZQ2PR01MB1307.CHNPR01.prod.partner.outlook.cn/

The parameter training code in U-Boot when given max 100MHz has this at
somewhere between delay 2 and delay 3, where delay 3 almost appears to
work but there's data corruption, and delay 1 is totally non-functional.
With all testing so far the delay 2 and max frequency 100MHz is reliable
and also is a common occurrence in the Linux code base for similar qspi
access.

My testing methodology was pretty extensive and I wrote about the result
on the U-Boot mailing list, however here we're taking the success of
that in U-Boot and literally just applying it to Linux where it also
happens to align with some common similar assignments for other boards
in the devicetree code base. It works and there's no deeper insight here.

Welcome to hear from anyone that might actually understand this hardware?

-E