[U-Boot] [PATCH v4] arm: dts: Stratix10: Add QSPI node

[Ley Foon Tan]

On Fri, Mar 29, 2019 at 2:57 PM Simon Goldschmidt
<simon.k.r.goldschmidt at gmail.com> wrote:
>
> On Fri, Mar 29, 2019 at 7:39 AM Ley Foon Tan <lftan.linux at gmail.com> wrote:
> >
> > On Wed, Mar 27, 2019 at 7:31 PM Simon Goldschmidt
> > <simon.k.r.goldschmidt at gmail.com> wrote:
> > >
> > > On Wed, Mar 27, 2019 at 9:44 AM Ley Foon Tan <ley.foon.tan at intel.com> wrote:
> > > >
> > > > Add QSPI device tree to Stratix 10.
> > > > Sync from Linux Stratix 10 dts.
> > >
> > > Which tree? Which commit?
> > It is based on kernel v5.0 in mainline.
>
> In my understanding, 'sync from Linux' means this is a 1:1 copy of the file
> as it is in Linux. You should add the commit id or something like that to help
> reviewers identify the source and compare it.
>
> However, I don't see all these U-Boot specific properties in the Linux version,
> so the term 'sync from Linux' does not seem correct. It seems like
> this difference
> has been there before this patch, so could you clean it up first? You'd do that
> by copying the files (e.g. from Linux 5.0) and after that restore the U-Boot
> contents by adding "-u-boot.dtsi" files. See my commit for gen5:
> commit c402e8170245 ("dts: arm: socfpga: merge gen5 devicetrees from linux")
> with the downside of a bad example: I forgot to add a Linux commit id :-(
>
Okay, I will take a look this.
Thanks.

Regards
Ley Foon
> >
> > >
> > > >
> > > > Tested on Stratix 10 SoC devkit.
> > > > SOCFPGA_STRATIX10 # sf probe 0:0
> > > > SF: Detected mt25qu02g with page size 256 Bytes, erase size 64 KiB, total 256 MiB
> > > >
> > > > Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
> > > >
> > > > ---
> > > > v3->v4:
> > > > - Add qspi node to dtsi based on alphabetical order
> > > > - Add spi-tx-bus-width and spi-rx-bus-width
> > > >
> > > > v2->v3:
> > > > - Change flash compatible to "jedec,spi-nor"
> > > > - Change spi-max-frequency to 100MHz
> > > > ---
> > > >  arch/arm/dts/socfpga_stratix10.dtsi      | 15 +++++++++++++++
> > > >  arch/arm/dts/socfpga_stratix10_socdk.dts | 23 +++++++++++++++++++++++
> > > >  2 files changed, 38 insertions(+)
> > > >
> > > > diff --git a/arch/arm/dts/socfpga_stratix10.dtsi b/arch/arm/dts/socfpga_stratix10.dtsi
> > > > index ee93725d648..fde76774047 100644
> > > > --- a/arch/arm/dts/socfpga_stratix10.dtsi
> > > > +++ b/arch/arm/dts/socfpga_stratix10.dtsi
> > > > @@ -237,6 +237,21 @@
> > > >                         reg = <0xffe00000 0x100000>;
> > > >                 };
> > > >
> > > > +               qspi: spi at ff8d2000 {
> > > > +                       compatible = "cdns,qspi-nor";
> > > > +                       #address-cells = <1>;
> > > > +                       #size-cells = <0>;
> > > > +                       reg = <0xff8d2000 0x100>,
> > > > +                             <0xff900000 0x100000>;
> > > > +                       interrupts = <0 3 4>;
> > > > +                       cdns,fifo-depth = <128>;
> > > > +                       cdns,fifo-width = <4>;
> > > > +                       cdns,trigger-address = <0x00000000>;
> > > > +                       bus-num = <0>;
> > > > +                       status = "disabled";
> > > > +                       u-boot,dm-pre-reloc;
> > >
> > > This file should be in sync with the Linux one, no? Does the Linux on
> > > havethis U-Boot specific property here? I could check myself if you would
> > > have given a tree or commit id of the sync source in the commit message...
> > >
> > > If the Linux dts does not have this property, it would have to be moved
> > > to a U-Boot specific "-u-boot.dtsi" addon file.
> > Okay, will add this in new revision.
> >
> > Thanks.
> >
> > Regards
> > Ley Foon