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

[Simon Goldschmidt]

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 :-(

Regards,
Simon

>
> >
> > >
> > > 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