| Message ID | 20250203013730.269558-3-e@freeshell.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | riscv: dts: starfive: jh7110-common: Sync downstream U-Boot changes | expand |
| Context | Check | Description |
|---|---|---|
| bjorn/pre-ci_am | success | Success |
| bjorn/build-rv32-defconfig | success | build-rv32-defconfig |
| bjorn/build-rv64-clang-allmodconfig | success | build-rv64-clang-allmodconfig |
| bjorn/build-rv64-gcc-allmodconfig | success | build-rv64-gcc-allmodconfig |
| bjorn/build-rv64-nommu-k210-defconfig | success | build-rv64-nommu-k210-defconfig |
| bjorn/build-rv64-nommu-k210-virt | success | build-rv64-nommu-k210-virt |
| bjorn/checkpatch | success | checkpatch |
| bjorn/dtb-warn-rv64 | success | dtb-warn-rv64 |
| bjorn/header-inline | success | header-inline |
| bjorn/kdoc | success | kdoc |
| bjorn/module-param | success | module-param |
| bjorn/verify-fixes | success | verify-fixes |
| bjorn/verify-signedoff | success | verify-signedoff |
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@freeshell.de> > Reviewed-by: Hal Feng <hal.feng@starfivetech.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 >
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@freeshell.de> >> Reviewed-by: Hal Feng <hal.feng@starfivetech.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
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>;