[U-Boot] [PATCH] arm: socfpga: Move Stratix 10 SDRAM driver to DM

[Simon Goldschmidt]

On Thu, Mar 28, 2019 at 3:06 AM Ley Foon Tan <lftan.linux at gmail.com> wrote:
>
> On Wed, Mar 27, 2019 at 7:34 PM Simon Goldschmidt
> <simon.k.r.goldschmidt at gmail.com> wrote:
> >
> > On Wed, Mar 27, 2019 at 10:15 AM Ley Foon Tan <ley.foon.tan at intel.com> wrote:
> > >
> > > Convert Stratix 10 SDRAM driver to device model.
> >
> > Nice to see that :-)
> >
> > >
> > > Get rid of call to socfpga_per_reset() and use reset
> > > framework.
> > >
> > > SPL is changed from calling function in SDRAM driver
> > > directly to just probing UCLASS_RAM.
> > >
> > > Move sdram_s10.h from arch to driver/ddr/altera directory.
> > >
> > > Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
> > > ---
> > >  arch/arm/Kconfig                              |   4 +-
> > >  arch/arm/dts/socfpga_stratix10.dtsi           |  10 +
> > >  arch/arm/dts/socfpga_stratix10_socdk.dts      |   4 +
> > >  arch/arm/mach-socfpga/Kconfig                 |   1 +
> > >  arch/arm/mach-socfpga/spl_s10.c               |   9 +-
> > >  drivers/ddr/altera/Kconfig                    |   2 +-
> > >  drivers/ddr/altera/sdram_s10.c                | 246 ++++++++++++------
> > >  .../mach => drivers/ddr/altera}/sdram_s10.h   |   4 -
> > >  include/configs/socfpga_stratix10_socdk.h     |   5 -
> > >  9 files changed, 195 insertions(+), 90 deletions(-)
> > >  rename {arch/arm/mach-socfpga/include/mach => drivers/ddr/altera}/sdram_s10.h (97%)
> > >
> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > index 77a534f81fd..a2d4f7f4c25 100644
> > > --- a/arch/arm/Kconfig
> > > +++ b/arch/arm/Kconfig
> > > @@ -821,14 +821,14 @@ config ARCH_SOCFPGA
> > >         select DM_SERIAL
> > >         select ENABLE_ARM_SOC_BOOT0_HOOK if TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10
> > >         select OF_CONTROL
> > > -       select RAM if TARGET_SOCFPGA_GEN5
> > > +       select RAM if TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_STRATIX10
> > >         select SPL_DM_RESET if DM_RESET
> > >         select SPL_DM_SERIAL
> > >         select SPL_LIBCOMMON_SUPPORT
> > >         select SPL_LIBGENERIC_SUPPORT
> > >         select SPL_NAND_SUPPORT if SPL_NAND_DENALI
> > >         select SPL_OF_CONTROL
> > > -       select SPL_RAM if TARGET_SOCFPGA_GEN5
> > > +       select SPL_RAM if TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_STRATIX10
> > >         select SPL_SEPARATE_BSS if TARGET_SOCFPGA_STRATIX10
> > >         select SPL_SERIAL_SUPPORT
> > >         select SPL_WATCHDOG_SUPPORT
> > > diff --git a/arch/arm/dts/socfpga_stratix10.dtsi b/arch/arm/dts/socfpga_stratix10.dtsi
> > > index ee93725d648..15d0f88f182 100644
> > > --- a/arch/arm/dts/socfpga_stratix10.dtsi
> > > +++ b/arch/arm/dts/socfpga_stratix10.dtsi
> > > @@ -245,6 +245,16 @@
> > >                         u-boot,dm-pre-reloc;
> > >                 };
> > >
> > > +               sdr: sdr at f8000400 {
> > > +                        compatible = "altr,sdr-ctl-s10";
> > > +                        reg = <0xf8000400 0x80>,
> > > +                              <0xf8010000 0x190>,
> > > +                              <0xf8011000 0x500>;
> > > +                        resets = <&rst DDRSCH_RESET>;
> > > +                        u-boot,dm-pre-reloc;
> > > +                        status = "disabled";
> >
> > Why is this optional? The SPL won't work without that any more by default,
> > or would it? I would have thought it's enabled by default?
> Customer can use either SDRAM controller in HPS or in FPGA.
> We don't need this driver if SDRAM controller is in FPGA. All
> initialization process will handled by HW.
> I think we just need a very simple sdram driver for sdram controller
> in FPGA, just to setup get_info().

Fair enough. But how often will this be used? Would it make sense to use the
internal SDRAM controller by default and let the customers with a controller
in the FPGA override it or is this a common use case?

Regards,
Simon