| Message ID | 20241203030430.26089-6-hal.feng@starfivetech.com |
|---|---|
| State | Superseded |
| Delegated to: | Andes |
| Headers | show |
| Series | Support OF_UPSTREAM for StarFive JH7110 | expand |
On Mon, Dec 2, 2024 at 7:06 PM Hal Feng <hal.feng@starfivetech.com> wrote: > > To support JH7110 based boards besides v1.3B, > add a common dtsi and add common code to it. > > Tested-by: Anand Moon <linux.amoon@gmail.com> > Tested-by: E Shattow <lucent@gmail.com> > Reviewed-by: E Shattow <lucent@gmail.com> > Signed-off-by: Hal Feng <hal.feng@starfivetech.com> > --- > arch/riscv/dts/jh7110-common-u-boot.dtsi | 142 ++++++++++++++++++ > ...10-starfive-visionfive-2-v1.3b-u-boot.dtsi | 138 +---------------- > 2 files changed, 143 insertions(+), 137 deletions(-) > create mode 100644 arch/riscv/dts/jh7110-common-u-boot.dtsi > > diff --git a/arch/riscv/dts/jh7110-common-u-boot.dtsi b/arch/riscv/dts/jh7110-common-u-boot.dtsi > new file mode 100644 > index 0000000000..7953459e67 > --- /dev/null > +++ b/arch/riscv/dts/jh7110-common-u-boot.dtsi > @@ -0,0 +1,142 @@ > +// SPDX-License-Identifier: GPL-2.0 OR MIT > +/* > + * Copyright (C) 2023 StarFive Technology Co., Ltd. > + */ > + > +#include "binman.dtsi" > +#include "jh7110-u-boot.dtsi" > +/ { > + aliases { > + spi0 = &qspi; > + }; > + > + chosen { > + bootph-pre-ram; > + }; > + > + firmware { > + spi0 = &qspi; > + bootph-pre-ram; > + }; > + > + config { > + bootph-pre-ram; > + u-boot,spl-payload-offset = <0x100000>; > + }; > + > + memory@40000000 { > + bootph-pre-ram; > + }; > +}; > + > +&uart0 { > + bootph-pre-ram; > + reg-offset = <0>; > + current-speed = <115200>; > + clock-frequency = <24000000>; > +}; > + > +&mmc0 { > + bootph-pre-ram; > +}; > + > +&mmc1 { > + bootph-pre-ram; > +}; > + > +&qspi { > + bootph-pre-ram; > + spi-max-frequency = <250000000>; Drop this line spi-max-frequency 250Mhz. The driver expects spi-max-frequency in any sub-node, and not here? > + > + flash@0 { > + bootph-pre-ram; > + /delete-property/ cdns,read-delay; Drop this line /delete-property/ because it is a problem for the parser if we need to re-define 'cdns,read-delay' later in dts files downstream of jh7110-common-u-boot.dtsi If you want drivers/spi/cadence_qspi.c:spi_calibration() to do this work then it needs to be the same upstream. In my testing on 4GB Star64 the calibration (via /delete-property/ cdns,read-delay) method with spi-max-frequency less than exactly 49.8MHz results in a failure to write SPI-NOR correctly. When spi-max-frequency is approximately 25MHz up to 49.799999MHz and written with a known good tool then the boot may be successful but "Main section boot fail,use backup section" or other errors on boot after using that unstable configuration of U-Boot to write U-Boot image to the SPI-NOR. No problems writing and boot during testing with spi-max-frequency 49.8MHz up to 100MHz (not tested above 100MHz). The upstream dts has spi-max-frequency 12MHz which is not successful with any of the read-delay values I tested: 0 1 2 3 4 5 > + spi-max-frequency = <100000000>; Why 12MHz upstream and 100MHz here? Something is wrong but I do not know is it U-Boot or Linux? Success: + /delete-property/ cdns,read-delay; + spi-max-frequency = <100000000>; Success: + cdns,read-delay = <2>; - /delete-property/ cdns,read-delay; + spi-max-frequency = <100000000>; Failure: + cdns,read-delay = <2>; + spi-max-frequency = <50000000>; The actual fix is going to need testing Linux and U-Boot on all the boards I do not have access to. For the moment we can: - /delete-property/ cdns,read-delay; + cdns,read-delay = <2>; + spi-max-frequency = <100000000>; or do nothing (but yes please drop that 250Mhz line it is non-op I think). What do you think? Or else we discuss here and follow with patches in future to fix the mess. I really do not want any /delete-property/ to be in dtsi but I don't know why it is so different between U-Boot and Linux and cannot personally test all the possible boards to verify a change. > + }; > +}; > + > +&syscrg { > + assigned-clocks = <&syscrg JH7110_SYSCLK_CPU_ROOT>, > + <&syscrg JH7110_SYSCLK_BUS_ROOT>, > + <&syscrg JH7110_SYSCLK_PERH_ROOT>, > + <&syscrg JH7110_SYSCLK_QSPI_REF>; > + assigned-clock-parents = <&pllclk JH7110_PLLCLK_PLL0_OUT>, > + <&pllclk JH7110_PLLCLK_PLL2_OUT>, > + <&pllclk JH7110_PLLCLK_PLL2_OUT>, > + <&syscrg JH7110_SYSCLK_QSPI_REF_SRC>; > + assigned-clock-rates = <0>, <0>, <0>, <0>; > +}; > + > +&sysgpio { > + bootph-pre-ram; > +}; > + > +&mmc0_pins { > + bootph-pre-ram; > + rst-pins { > + bootph-pre-ram; > + }; > +}; > + > +&mmc1_pins { > + bootph-pre-ram; > + clk-pins { > + bootph-pre-ram; > + }; > + > + mmc-pins { > + bootph-pre-ram; > + }; > +}; > + > +&i2c5_pins { > + bootph-pre-ram; > + i2c-pins { > + bootph-pre-ram; > + }; > +}; > + > +&i2c5 { > + bootph-pre-ram; > + eeprom@50 { > + bootph-pre-ram; > + compatible = "atmel,24c04"; > + reg = <0x50>; > + pagesize = <16>; > + }; > +}; > + > +&binman { > + itb { > + fit { > + images { > + fdt-1 { > + description = "NAME"; > + load = <0x40400000>; > + compression = "none"; > + > + uboot_fdt_blob: blob-ext { > + filename = "u-boot.dtb"; > + }; > + }; > + }; > + > + configurations { > + conf-1 { > + fdt = "fdt-1"; > + }; > + }; > + }; > + }; > + > + spl-img { > + filename = "spl/u-boot-spl.bin.normal.out"; > + > + mkimage { > + args = "-T sfspl"; > + > + u-boot-spl { > + }; > + }; > + }; > +}; > diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2-v1.3b-u-boot.dtsi b/arch/riscv/dts/jh7110-starfive-visionfive-2-v1.3b-u-boot.dtsi > index 7953459e67..e6bc6630dc 100644 > --- a/arch/riscv/dts/jh7110-starfive-visionfive-2-v1.3b-u-boot.dtsi > +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2-v1.3b-u-boot.dtsi > @@ -3,140 +3,4 @@ > * Copyright (C) 2023 StarFive Technology Co., Ltd. > */ > > -#include "binman.dtsi" > -#include "jh7110-u-boot.dtsi" > -/ { > - aliases { > - spi0 = &qspi; > - }; > - > - chosen { > - bootph-pre-ram; > - }; > - > - firmware { > - spi0 = &qspi; > - bootph-pre-ram; > - }; > - > - config { > - bootph-pre-ram; > - u-boot,spl-payload-offset = <0x100000>; > - }; > - > - memory@40000000 { > - bootph-pre-ram; > - }; > -}; > - > -&uart0 { > - bootph-pre-ram; > - reg-offset = <0>; > - current-speed = <115200>; > - clock-frequency = <24000000>; > -}; > - > -&mmc0 { > - bootph-pre-ram; > -}; > - > -&mmc1 { > - bootph-pre-ram; > -}; > - > -&qspi { > - bootph-pre-ram; > - spi-max-frequency = <250000000>; > - > - flash@0 { > - bootph-pre-ram; > - /delete-property/ cdns,read-delay; > - spi-max-frequency = <100000000>; > - }; > -}; > - > -&syscrg { > - assigned-clocks = <&syscrg JH7110_SYSCLK_CPU_ROOT>, > - <&syscrg JH7110_SYSCLK_BUS_ROOT>, > - <&syscrg JH7110_SYSCLK_PERH_ROOT>, > - <&syscrg JH7110_SYSCLK_QSPI_REF>; > - assigned-clock-parents = <&pllclk JH7110_PLLCLK_PLL0_OUT>, > - <&pllclk JH7110_PLLCLK_PLL2_OUT>, > - <&pllclk JH7110_PLLCLK_PLL2_OUT>, > - <&syscrg JH7110_SYSCLK_QSPI_REF_SRC>; > - assigned-clock-rates = <0>, <0>, <0>, <0>; > -}; > - > -&sysgpio { > - bootph-pre-ram; > -}; > - > -&mmc0_pins { > - bootph-pre-ram; > - rst-pins { > - bootph-pre-ram; > - }; > -}; > - > -&mmc1_pins { > - bootph-pre-ram; > - clk-pins { > - bootph-pre-ram; > - }; > - > - mmc-pins { > - bootph-pre-ram; > - }; > -}; > - > -&i2c5_pins { > - bootph-pre-ram; > - i2c-pins { > - bootph-pre-ram; > - }; > -}; > - > -&i2c5 { > - bootph-pre-ram; > - eeprom@50 { > - bootph-pre-ram; > - compatible = "atmel,24c04"; > - reg = <0x50>; > - pagesize = <16>; > - }; > -}; > - > -&binman { > - itb { > - fit { > - images { > - fdt-1 { > - description = "NAME"; > - load = <0x40400000>; > - compression = "none"; > - > - uboot_fdt_blob: blob-ext { > - filename = "u-boot.dtb"; > - }; > - }; > - }; > - > - configurations { > - conf-1 { > - fdt = "fdt-1"; > - }; > - }; > - }; > - }; > - > - spl-img { > - filename = "spl/u-boot-spl.bin.normal.out"; > - > - mkimage { > - args = "-T sfspl"; > - > - u-boot-spl { > - }; > - }; > - }; > -}; > +#include "jh7110-common-u-boot.dtsi" > -- > 2.43.2 > -E
> On 04.12.24 03:16, E Shattow wrote: > On Mon, Dec 2, 2024 at 7:06 PM Hal Feng <hal.feng@starfivetech.com> > wrote: > > > > To support JH7110 based boards besides v1.3B, add a common dtsi and > > add common code to it. > > > > Tested-by: Anand Moon <linux.amoon@gmail.com> > > Tested-by: E Shattow <lucent@gmail.com> > > Reviewed-by: E Shattow <lucent@gmail.com> > > Signed-off-by: Hal Feng <hal.feng@starfivetech.com> > > --- > > arch/riscv/dts/jh7110-common-u-boot.dtsi | 142 > ++++++++++++++++++ > > ...10-starfive-visionfive-2-v1.3b-u-boot.dtsi | 138 +---------------- > > 2 files changed, 143 insertions(+), 137 deletions(-) create mode > > 100644 arch/riscv/dts/jh7110-common-u-boot.dtsi > > > > diff --git a/arch/riscv/dts/jh7110-common-u-boot.dtsi > > b/arch/riscv/dts/jh7110-common-u-boot.dtsi > > new file mode 100644 > > index 0000000000..7953459e67 > > --- /dev/null > > +++ b/arch/riscv/dts/jh7110-common-u-boot.dtsi > > @@ -0,0 +1,142 @@ > > +// SPDX-License-Identifier: GPL-2.0 OR MIT > > +/* > > + * Copyright (C) 2023 StarFive Technology Co., Ltd. > > + */ > > + > > +#include "binman.dtsi" > > +#include "jh7110-u-boot.dtsi" > > +/ { > > + aliases { > > + spi0 = &qspi; > > + }; > > + > > + chosen { > > + bootph-pre-ram; > > + }; > > + > > + firmware { > > + spi0 = &qspi; > > + bootph-pre-ram; > > + }; > > + > > + config { > > + bootph-pre-ram; > > + u-boot,spl-payload-offset = <0x100000>; > > + }; > > + > > + memory@40000000 { > > + bootph-pre-ram; > > + }; > > +}; > > + > > +&uart0 { > > + bootph-pre-ram; > > + reg-offset = <0>; > > + current-speed = <115200>; > > + clock-frequency = <24000000>; > > +}; > > + > > +&mmc0 { > > + bootph-pre-ram; > > +}; > > + > > +&mmc1 { > > + bootph-pre-ram; > > +}; > > + > > +&qspi { > > + bootph-pre-ram; > > > + spi-max-frequency = <250000000>; > > Drop this line spi-max-frequency 250Mhz. The driver expects spi-max- > frequency in any sub-node, and not here? Yes, this property is used in sub-node, we can drop this later. > > > + > > + flash@0 { > > + bootph-pre-ram; > > > + /delete-property/ cdns,read-delay; > > Drop this line /delete-property/ because it is a problem for the parser if we > need to re-define 'cdns,read-delay' later in dts files downstream of jh7110- > common-u-boot.dtsi > > If you want drivers/spi/cadence_qspi.c:spi_calibration() to do this work then it > needs to be the same upstream. In my testing on 4GB > Star64 the calibration (via /delete-property/ cdns,read-delay) method with > spi-max-frequency less than exactly 49.8MHz results in a failure to write SPI- > NOR correctly. When spi-max-frequency is approximately 25MHz up to > 49.799999MHz and written with a known good tool then the boot may be > successful but "Main section boot fail,use backup section" > or other errors on boot after using that unstable configuration of U-Boot to > write U-Boot image to the SPI-NOR. No problems writing and boot during > testing with spi-max-frequency 49.8MHz up to 100MHz (not tested above > 100MHz). > > The upstream dts has spi-max-frequency 12MHz which is not successful with > any of the read-delay values I tested: 0 1 2 3 4 5 > > > + spi-max-frequency = <100000000>; > > Why 12MHz upstream and 100MHz here? Something is wrong but I do not > know is it U-Boot or Linux? > > Success: > + /delete-property/ cdns,read-delay; > + spi-max-frequency = <100000000>; > > Success: > + cdns,read-delay = <2>; > - /delete-property/ cdns,read-delay; > + spi-max-frequency = <100000000>; > > Failure: > + cdns,read-delay = <2>; > + spi-max-frequency = <50000000>; > > The actual fix is going to need testing Linux and U-Boot on all the boards I do > not have access to. > > For the moment we can: > - /delete-property/ cdns,read-delay; > + cdns,read-delay = <2>; > + spi-max-frequency = <100000000>; > > or do nothing (but yes please drop that 250Mhz line it is non-op I think). > > What do you think? Or else we discuss here and follow with patches in future > to fix the mess. I really do not want any /delete-property/ to be in dtsi but I > don't know why it is so different between U-Boot and Linux and cannot > personally test all the possible boards to verify a change. I don't know the cause of this problem either. But all properties added in this patch already existed in U-Boot DT before. I tried my best to keep the U-Boot DT the same as the one before OF_UPSTREAM applied. I tested and found that the qspi flash can work successfully in Linux and U-Boot with the settings you suggested above. cdns,read-delay = <2>; spi-max-frequency = <100000000>; If /delete-property/ is really not suitable to exist in this dtsi, we can modify as above in U-Boot and upstream this change to Liunx. After this change is accepted by Linux, I will drop these two properties in U-Boot. Best regards, Hal
On Tue, Dec 3, 2024 at 10:52 PM Hal Feng <hal.feng@starfivetech.com> wrote: > > > On 04.12.24 03:16, E Shattow wrote: > > On Mon, Dec 2, 2024 at 7:06 PM Hal Feng <hal.feng@starfivetech.com> > > wrote: > > > > > > To support JH7110 based boards besides v1.3B, add a common dtsi and > > > add common code to it. > > > > > > Tested-by: Anand Moon <linux.amoon@gmail.com> > > > Tested-by: E Shattow <lucent@gmail.com> > > > Reviewed-by: E Shattow <lucent@gmail.com> > > > Signed-off-by: Hal Feng <hal.feng@starfivetech.com> > > > --- > > > arch/riscv/dts/jh7110-common-u-boot.dtsi | 142 > > ++++++++++++++++++ > > > ...10-starfive-visionfive-2-v1.3b-u-boot.dtsi | 138 +---------------- > > > 2 files changed, 143 insertions(+), 137 deletions(-) create mode > > > 100644 arch/riscv/dts/jh7110-common-u-boot.dtsi > > > > > > diff --git a/arch/riscv/dts/jh7110-common-u-boot.dtsi > > > b/arch/riscv/dts/jh7110-common-u-boot.dtsi > > > new file mode 100644 > > > index 0000000000..7953459e67 > > > --- /dev/null > > > +++ b/arch/riscv/dts/jh7110-common-u-boot.dtsi > > > @@ -0,0 +1,142 @@ > > > +// SPDX-License-Identifier: GPL-2.0 OR MIT > > > +/* > > > + * Copyright (C) 2023 StarFive Technology Co., Ltd. > > > + */ > > > + > > > +#include "binman.dtsi" > > > +#include "jh7110-u-boot.dtsi" > > > +/ { > > > + aliases { > > > + spi0 = &qspi; > > > + }; > > > + > > > + chosen { > > > + bootph-pre-ram; > > > + }; > > > + > > > + firmware { > > > + spi0 = &qspi; > > > + bootph-pre-ram; > > > + }; > > > + > > > + config { > > > + bootph-pre-ram; > > > + u-boot,spl-payload-offset = <0x100000>; > > > + }; > > > + > > > + memory@40000000 { > > > + bootph-pre-ram; > > > + }; > > > +}; > > > + > > > +&uart0 { > > > + bootph-pre-ram; > > > + reg-offset = <0>; > > > + current-speed = <115200>; > > > + clock-frequency = <24000000>; > > > +}; > > > + > > > +&mmc0 { > > > + bootph-pre-ram; > > > +}; > > > + > > > +&mmc1 { > > > + bootph-pre-ram; > > > +}; > > > + > > > +&qspi { > > > + bootph-pre-ram; > > > > > + spi-max-frequency = <250000000>; > > > > Drop this line spi-max-frequency 250Mhz. The driver expects spi-max- > > frequency in any sub-node, and not here? > > Yes, this property is used in sub-node, we can drop this later. > > > > > > + > > > + flash@0 { > > > + bootph-pre-ram; > > > > > + /delete-property/ cdns,read-delay; > > > > Drop this line /delete-property/ because it is a problem for the parser if we > > need to re-define 'cdns,read-delay' later in dts files downstream of jh7110- > > common-u-boot.dtsi > > > > If you want drivers/spi/cadence_qspi.c:spi_calibration() to do this work then it > > needs to be the same upstream. In my testing on 4GB > > Star64 the calibration (via /delete-property/ cdns,read-delay) method with > > spi-max-frequency less than exactly 49.8MHz results in a failure to write SPI- > > NOR correctly. When spi-max-frequency is approximately 25MHz up to > > 49.799999MHz and written with a known good tool then the boot may be > > successful but "Main section boot fail,use backup section" > > or other errors on boot after using that unstable configuration of U-Boot to > > write U-Boot image to the SPI-NOR. No problems writing and boot during > > testing with spi-max-frequency 49.8MHz up to 100MHz (not tested above > > 100MHz). > > > > The upstream dts has spi-max-frequency 12MHz which is not successful with > > any of the read-delay values I tested: 0 1 2 3 4 5 > > > > > + spi-max-frequency = <100000000>; > > > > Why 12MHz upstream and 100MHz here? Something is wrong but I do not > > know is it U-Boot or Linux? > > > > Success: > > + /delete-property/ cdns,read-delay; > > + spi-max-frequency = <100000000>; > > > > Success: > > + cdns,read-delay = <2>; > > - /delete-property/ cdns,read-delay; > > + spi-max-frequency = <100000000>; > > > > Failure: > > + cdns,read-delay = <2>; > > + spi-max-frequency = <50000000>; > > > > The actual fix is going to need testing Linux and U-Boot on all the boards I do > > not have access to. > > > > For the moment we can: > > - /delete-property/ cdns,read-delay; > > + cdns,read-delay = <2>; > > + spi-max-frequency = <100000000>; > > > > or do nothing (but yes please drop that 250Mhz line it is non-op I think). > > > > What do you think? Or else we discuss here and follow with patches in future > > to fix the mess. I really do not want any /delete-property/ to be in dtsi but I > > don't know why it is so different between U-Boot and Linux and cannot > > personally test all the possible boards to verify a change. > > I don't know the cause of this problem either. But all properties added in this > patch already existed in U-Boot DT before. I tried my best to keep the U-Boot > DT the same as the one before OF_UPSTREAM applied. > > I tested and found that the qspi flash can work successfully in Linux and U-Boot > with the settings you suggested above. > > cdns,read-delay = <2>; > spi-max-frequency = <100000000>; Good report on testing Linux and U-Boot, thank you Hal. > > If /delete-property/ is really not suitable to exist in this dtsi, we can modify as > above in U-Boot and upstream this change to Liunx. After this change is accepted > by Linux, I will drop these two properties in U-Boot. > > Best regards, > Hal Yes let's do that. So long as it all works on VF2 1.2a, VF 1.3b, Mars, and Star64. I have tested Star64. You have tested I guess VF2 1.3b? And if we miss some, it's okay even if there is a problem for some board we did not test, the workaround is easy in u-boot specific dtsi per-board because we have avoided '/delete-property/' in the common dtsi. -E
> On 04.12.24 18:56, E Shattow wrote: > On Tue, Dec 3, 2024 at 10:52 PM Hal Feng <hal.feng@starfivetech.com> > wrote: > > > > > On 04.12.24 03:16, E Shattow wrote: > > > On Mon, Dec 2, 2024 at 7:06 PM Hal Feng <hal.feng@starfivetech.com> > > > wrote: > > > > > > > > To support JH7110 based boards besides v1.3B, add a common dtsi > > > > and add common code to it. > > > > > > > > Tested-by: Anand Moon <linux.amoon@gmail.com> > > > > Tested-by: E Shattow <lucent@gmail.com> > > > > Reviewed-by: E Shattow <lucent@gmail.com> > > > > Signed-off-by: Hal Feng <hal.feng@starfivetech.com> > > > > --- > > > > arch/riscv/dts/jh7110-common-u-boot.dtsi | 142 > > > ++++++++++++++++++ > > > > ...10-starfive-visionfive-2-v1.3b-u-boot.dtsi | 138 > > > > +---------------- > > > > 2 files changed, 143 insertions(+), 137 deletions(-) create mode > > > > 100644 arch/riscv/dts/jh7110-common-u-boot.dtsi > > > > > > > > diff --git a/arch/riscv/dts/jh7110-common-u-boot.dtsi > > > > b/arch/riscv/dts/jh7110-common-u-boot.dtsi > > > > new file mode 100644 > > > > index 0000000000..7953459e67 > > > > --- /dev/null > > > > +++ b/arch/riscv/dts/jh7110-common-u-boot.dtsi > > > > @@ -0,0 +1,142 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 OR MIT > > > > +/* > > > > + * Copyright (C) 2023 StarFive Technology Co., Ltd. > > > > + */ > > > > + > > > > +#include "binman.dtsi" > > > > +#include "jh7110-u-boot.dtsi" > > > > +/ { > > > > + aliases { > > > > + spi0 = &qspi; > > > > + }; > > > > + > > > > + chosen { > > > > + bootph-pre-ram; > > > > + }; > > > > + > > > > + firmware { > > > > + spi0 = &qspi; > > > > + bootph-pre-ram; > > > > + }; > > > > + > > > > + config { > > > > + bootph-pre-ram; > > > > + u-boot,spl-payload-offset = <0x100000>; > > > > + }; > > > > + > > > > + memory@40000000 { > > > > + bootph-pre-ram; > > > > + }; > > > > +}; > > > > + > > > > +&uart0 { > > > > + bootph-pre-ram; > > > > + reg-offset = <0>; > > > > + current-speed = <115200>; > > > > + clock-frequency = <24000000>; }; > > > > + > > > > +&mmc0 { > > > > + bootph-pre-ram; > > > > +}; > > > > + > > > > +&mmc1 { > > > > + bootph-pre-ram; > > > > +}; > > > > + > > > > +&qspi { > > > > + bootph-pre-ram; > > > > > > > + spi-max-frequency = <250000000>; > > > > > > Drop this line spi-max-frequency 250Mhz. The driver expects spi-max- > > > frequency in any sub-node, and not here? > > > > Yes, this property is used in sub-node, we can drop this later. > > > > > > > > > + > > > > + flash@0 { > > > > + bootph-pre-ram; > > > > > > > + /delete-property/ cdns,read-delay; > > > > > > Drop this line /delete-property/ because it is a problem for the > > > parser if we need to re-define 'cdns,read-delay' later in dts files > > > downstream of jh7110- common-u-boot.dtsi > > > > > > If you want drivers/spi/cadence_qspi.c:spi_calibration() to do this > > > work then it needs to be the same upstream. In my testing on 4GB > > > Star64 the calibration (via /delete-property/ cdns,read-delay) > > > method with spi-max-frequency less than exactly 49.8MHz results in a > > > failure to write SPI- NOR correctly. When spi-max-frequency is > > > approximately 25MHz up to 49.799999MHz and written with a known > good > > > tool then the boot may be successful but "Main section boot fail,use > backup section" > > > or other errors on boot after using that unstable configuration of > > > U-Boot to write U-Boot image to the SPI-NOR. No problems writing and > > > boot during testing with spi-max-frequency 49.8MHz up to 100MHz (not > > > tested above 100MHz). > > > > > > The upstream dts has spi-max-frequency 12MHz which is not successful > > > with any of the read-delay values I tested: 0 1 2 3 4 5 > > > > > > > + spi-max-frequency = <100000000>; > > > > > > Why 12MHz upstream and 100MHz here? Something is wrong but I do not > > > know is it U-Boot or Linux? > > > > > > Success: > > > + /delete-property/ cdns,read-delay; > > > + spi-max-frequency = <100000000>; > > > > > > Success: > > > + cdns,read-delay = <2>; > > > - /delete-property/ cdns,read-delay; > > > + spi-max-frequency = <100000000>; > > > > > > Failure: > > > + cdns,read-delay = <2>; > > > + spi-max-frequency = <50000000>; > > > > > > The actual fix is going to need testing Linux and U-Boot on all the > > > boards I do not have access to. > > > > > > For the moment we can: > > > - /delete-property/ cdns,read-delay; > > > + cdns,read-delay = <2>; > > > + spi-max-frequency = <100000000>; > > > > > > or do nothing (but yes please drop that 250Mhz line it is non-op I think). > > > > > > What do you think? Or else we discuss here and follow with patches > > > in future to fix the mess. I really do not want any > > > /delete-property/ to be in dtsi but I don't know why it is so > > > different between U-Boot and Linux and cannot personally test all the > possible boards to verify a change. > > > > I don't know the cause of this problem either. But all properties > > added in this patch already existed in U-Boot DT before. I tried my > > best to keep the U-Boot DT the same as the one before OF_UPSTREAM > applied. > > > > I tested and found that the qspi flash can work successfully in Linux > > and U-Boot with the settings you suggested above. > > > > cdns,read-delay = <2>; > > spi-max-frequency = <100000000>; > > Good report on testing Linux and U-Boot, thank you Hal. > > > > > If /delete-property/ is really not suitable to exist in this dtsi, we > > can modify as above in U-Boot and upstream this change to Liunx. After > > this change is accepted by Linux, I will drop these two properties in U-Boot. > > > > Best regards, > > Hal > > Yes let's do that. So long as it all works on VF2 1.2a, VF 1.3b, Mars, and > Star64. I have tested Star64. You have tested I guess VF2 1.3b? > And if we miss some, it's okay even if there is a problem for some board we > did not test, the workaround is easy in u-boot specific dtsi per-board because > we have avoided '/delete-property/' in the common dtsi. OK. Yeah, I tested on VF2 1.3b and VF2 1.2a is very similar to VF2 1.3b. Best regards, Hal
diff --git a/arch/riscv/dts/jh7110-common-u-boot.dtsi b/arch/riscv/dts/jh7110-common-u-boot.dtsi new file mode 100644 index 0000000000..7953459e67 --- /dev/null +++ b/arch/riscv/dts/jh7110-common-u-boot.dtsi @@ -0,0 +1,142 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT +/* + * Copyright (C) 2023 StarFive Technology Co., Ltd. + */ + +#include "binman.dtsi" +#include "jh7110-u-boot.dtsi" +/ { + aliases { + spi0 = &qspi; + }; + + chosen { + bootph-pre-ram; + }; + + firmware { + spi0 = &qspi; + bootph-pre-ram; + }; + + config { + bootph-pre-ram; + u-boot,spl-payload-offset = <0x100000>; + }; + + memory@40000000 { + bootph-pre-ram; + }; +}; + +&uart0 { + bootph-pre-ram; + reg-offset = <0>; + current-speed = <115200>; + clock-frequency = <24000000>; +}; + +&mmc0 { + bootph-pre-ram; +}; + +&mmc1 { + bootph-pre-ram; +}; + +&qspi { + bootph-pre-ram; + spi-max-frequency = <250000000>; + + flash@0 { + bootph-pre-ram; + /delete-property/ cdns,read-delay; + spi-max-frequency = <100000000>; + }; +}; + +&syscrg { + assigned-clocks = <&syscrg JH7110_SYSCLK_CPU_ROOT>, + <&syscrg JH7110_SYSCLK_BUS_ROOT>, + <&syscrg JH7110_SYSCLK_PERH_ROOT>, + <&syscrg JH7110_SYSCLK_QSPI_REF>; + assigned-clock-parents = <&pllclk JH7110_PLLCLK_PLL0_OUT>, + <&pllclk JH7110_PLLCLK_PLL2_OUT>, + <&pllclk JH7110_PLLCLK_PLL2_OUT>, + <&syscrg JH7110_SYSCLK_QSPI_REF_SRC>; + assigned-clock-rates = <0>, <0>, <0>, <0>; +}; + +&sysgpio { + bootph-pre-ram; +}; + +&mmc0_pins { + bootph-pre-ram; + rst-pins { + bootph-pre-ram; + }; +}; + +&mmc1_pins { + bootph-pre-ram; + clk-pins { + bootph-pre-ram; + }; + + mmc-pins { + bootph-pre-ram; + }; +}; + +&i2c5_pins { + bootph-pre-ram; + i2c-pins { + bootph-pre-ram; + }; +}; + +&i2c5 { + bootph-pre-ram; + eeprom@50 { + bootph-pre-ram; + compatible = "atmel,24c04"; + reg = <0x50>; + pagesize = <16>; + }; +}; + +&binman { + itb { + fit { + images { + fdt-1 { + description = "NAME"; + load = <0x40400000>; + compression = "none"; + + uboot_fdt_blob: blob-ext { + filename = "u-boot.dtb"; + }; + }; + }; + + configurations { + conf-1 { + fdt = "fdt-1"; + }; + }; + }; + }; + + spl-img { + filename = "spl/u-boot-spl.bin.normal.out"; + + mkimage { + args = "-T sfspl"; + + u-boot-spl { + }; + }; + }; +}; diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2-v1.3b-u-boot.dtsi b/arch/riscv/dts/jh7110-starfive-visionfive-2-v1.3b-u-boot.dtsi index 7953459e67..e6bc6630dc 100644 --- a/arch/riscv/dts/jh7110-starfive-visionfive-2-v1.3b-u-boot.dtsi +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2-v1.3b-u-boot.dtsi @@ -3,140 +3,4 @@ * Copyright (C) 2023 StarFive Technology Co., Ltd. */ -#include "binman.dtsi" -#include "jh7110-u-boot.dtsi" -/ { - aliases { - spi0 = &qspi; - }; - - chosen { - bootph-pre-ram; - }; - - firmware { - spi0 = &qspi; - bootph-pre-ram; - }; - - config { - bootph-pre-ram; - u-boot,spl-payload-offset = <0x100000>; - }; - - memory@40000000 { - bootph-pre-ram; - }; -}; - -&uart0 { - bootph-pre-ram; - reg-offset = <0>; - current-speed = <115200>; - clock-frequency = <24000000>; -}; - -&mmc0 { - bootph-pre-ram; -}; - -&mmc1 { - bootph-pre-ram; -}; - -&qspi { - bootph-pre-ram; - spi-max-frequency = <250000000>; - - flash@0 { - bootph-pre-ram; - /delete-property/ cdns,read-delay; - spi-max-frequency = <100000000>; - }; -}; - -&syscrg { - assigned-clocks = <&syscrg JH7110_SYSCLK_CPU_ROOT>, - <&syscrg JH7110_SYSCLK_BUS_ROOT>, - <&syscrg JH7110_SYSCLK_PERH_ROOT>, - <&syscrg JH7110_SYSCLK_QSPI_REF>; - assigned-clock-parents = <&pllclk JH7110_PLLCLK_PLL0_OUT>, - <&pllclk JH7110_PLLCLK_PLL2_OUT>, - <&pllclk JH7110_PLLCLK_PLL2_OUT>, - <&syscrg JH7110_SYSCLK_QSPI_REF_SRC>; - assigned-clock-rates = <0>, <0>, <0>, <0>; -}; - -&sysgpio { - bootph-pre-ram; -}; - -&mmc0_pins { - bootph-pre-ram; - rst-pins { - bootph-pre-ram; - }; -}; - -&mmc1_pins { - bootph-pre-ram; - clk-pins { - bootph-pre-ram; - }; - - mmc-pins { - bootph-pre-ram; - }; -}; - -&i2c5_pins { - bootph-pre-ram; - i2c-pins { - bootph-pre-ram; - }; -}; - -&i2c5 { - bootph-pre-ram; - eeprom@50 { - bootph-pre-ram; - compatible = "atmel,24c04"; - reg = <0x50>; - pagesize = <16>; - }; -}; - -&binman { - itb { - fit { - images { - fdt-1 { - description = "NAME"; - load = <0x40400000>; - compression = "none"; - - uboot_fdt_blob: blob-ext { - filename = "u-boot.dtb"; - }; - }; - }; - - configurations { - conf-1 { - fdt = "fdt-1"; - }; - }; - }; - }; - - spl-img { - filename = "spl/u-boot-spl.bin.normal.out"; - - mkimage { - args = "-T sfspl"; - - u-boot-spl { - }; - }; - }; -}; +#include "jh7110-common-u-boot.dtsi"