[PATCH 2/7] ddr: altera: arria10: Move SDRAM driver to DM

[Tan, Ley Foon]

> -----Original Message-----
> From: Marek Vasut <marex at denx.de>
> Sent: Wednesday, April 15, 2020 8:42 PM
> To: Tan, Ley Foon <ley.foon.tan at intel.com>; u-boot at lists.denx.de
> Cc: Ley Foon Tan <lftan.linux at gmail.com>; See, Chin Liang
> <chin.liang.see at intel.com>; Simon Goldschmidt
> <simon.k.r.goldschmidt at gmail.com>; Chee, Tien Fong
> <tien.fong.chee at intel.com>
> Subject: Re: [PATCH 2/7] ddr: altera: arria10: Move SDRAM driver to DM
> 
> On 4/15/20 11:00 AM, Ley Foon Tan wrote:
> [...]
> 
> > +++ b/arch/arm/mach-socfpga/include/mach/sdram_arria10.h
> > @@ -6,76 +6,65 @@
> 
> [...]
> 
> > +/* FW DDR MPU F2S SCR */
> > +#define FW_DDR_MPU_F2S_SCR_EN		0x00
> > +#define FW_DDR_MPU_F2S_SCR_MPUR0	0x10
> > +#define FW_DDR_MPU_F2S_SCR_MPUR1	0x14
> > +#define FW_DDR_MPU_F2S_SCR_MPUR2	0x18
> > +#define FW_DDR_MPU_F2S_SCR_MPUR3	0x1c
> 
> These can be:
> #define FW_DDR_MPU_F2S_SCR_MPUR(n)	(0x10 + (n) * 4)
> 
> and the other macros can be reduced in a similar manner.
Okay.
> 
> 
> [...]
> 
> > +#if CONFIG_IS_ENABLED(ALTERA_SDRAM)
> >  	/* If the IOSSM/full FPGA is already loaded, start DDR */
> > -	if (is_fpgamgr_early_user_mode() || is_fpgamgr_user_mode())
> > -		ddr_calibration_sequence();
> > +	if (is_fpgamgr_early_user_mode() || is_fpgamgr_user_mode()) {
> > +		ret = uclass_get_device(UCLASS_RAM, 0, &dev);
> > +		if (ret) {
> > +			debug("DRAM init failed: %d\n", ret);
> 
> This should be printf(), since it's an error, no ?
Yes, should use printf().
> 
> > +			hang();
> > +		}
> > +	}
> > +#endif
> >
> >  	if (!is_fpgamgr_user_mode())
> >  		fpgamgr_program(buf, FPGA_BUFSIZ, 0); diff --git
> > a/drivers/ddr/altera/Kconfig b/drivers/ddr/altera/Kconfig index
> > 8f590dc5f611..30ee884dcf47 100644
> > --- a/drivers/ddr/altera/Kconfig
> > +++ b/drivers/ddr/altera/Kconfig
> > @@ -2,7 +2,7 @@ config SPL_ALTERA_SDRAM
> >  	bool "SoCFPGA DDR SDRAM driver in SPL"
> >  	depends on SPL
> >  	depends on TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10
> || TARGET_SOCFPGA_STRATIX10 || TARGET_SOCFPGA_AGILEX
> > -	select RAM if TARGET_SOCFPGA_GEN5 ||
> TARGET_SOCFPGA_STRATIX10 || TARGET_SOCFPGA_AGILEX
> > -	select SPL_RAM if TARGET_SOCFPGA_GEN5 ||
> TARGET_SOCFPGA_STRATIX10 || TARGET_SOCFPGA_AGILEX
> > +	select RAM
> > +	select SPL_RAM
> 
> Shouldn't this be selected for Arria 10 only ?
Previously, we select SPL_RAM and RAM for Gen5, S10 and Agilex only. They support device model.
Now, we convert A10 SDRAM driver support device model, so we just need to select SPL_RAM and RAM always, regardless of TARGET.

Regards
Ley Foon