[PATCH v7 05/12] riscv: dts: jh7110: Move common code to the new jh7110-common-u-boot.dtsi

[Hal Feng]

> On 04.12.24 03:16, E Shattow wrote:
> On Mon, Dec 2, 2024 at 7:06 PM Hal Feng <hal.feng at 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 at gmail.com>
> > Tested-by: E Shattow <lucent at gmail.com>
> > Reviewed-by: E Shattow <lucent at gmail.com>
> > Signed-off-by: Hal Feng <hal.feng at 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 at 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 at 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