From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Wed, 24 Apr 2019 16:43:12 +0200 Subject: [U-Boot] [PATCH v2] arm: socfpga: Move Stratix 10 SDRAM driver to DM In-Reply-To: <1556086869-13501-1-git-send-email-ley.foon.tan@intel.com> References: <1556086869-13501-1-git-send-email-ley.foon.tan@intel.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 4/24/19 8:21 AM, Ley Foon Tan wrote: > Convert Stratix 10 SDRAM driver to device model. > > 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 > --- > v1->v2: > - Change sdr device tree node enabled by default. > - Probe UCLASS_RAM only if CONFIG_ALTERA_SDRAM is enabled. > --- > arch/arm/dts/socfpga_stratix10.dtsi | 9 + > arch/arm/mach-socfpga/spl_s10.c | 11 +- > configs/socfpga_stratix10_defconfig | 1 + > drivers/ddr/altera/Kconfig | 6 +- > drivers/ddr/altera/sdram_s10.c | 246 ++++++++++++------ > .../mach => drivers/ddr/altera}/sdram_s10.h | 4 - > include/configs/socfpga_stratix10_socdk.h | 5 - > 7 files changed, 192 insertions(+), 90 deletions(-) > rename {arch/arm/mach-socfpga/include/mach => drivers/ddr/altera}/sdram_s10.h (97%) > > diff --git a/arch/arm/dts/socfpga_stratix10.dtsi b/arch/arm/dts/socfpga_stratix10.dtsi > index d1ae2fabae..bd68a78a37 100755 > --- a/arch/arm/dts/socfpga_stratix10.dtsi > +++ b/arch/arm/dts/socfpga_stratix10.dtsi > @@ -258,6 +258,15 @@ > 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; > + }; > + > spi0: spi at ffda4000 { > compatible = "snps,dw-apb-ssi"; > #address-cells = <1>; Separate patch please > diff --git a/arch/arm/mach-socfpga/spl_s10.c b/arch/arm/mach-socfpga/spl_s10.c > index a141ffe82a..3d44eabf91 100644 > --- a/arch/arm/mach-socfpga/spl_s10.c > +++ b/arch/arm/mach-socfpga/spl_s10.c > @@ -15,9 +15,9 @@ > #include > #include > #include > -#include > #include > #include > +#include > > DECLARE_GLOBAL_DATA_PTR; > > @@ -119,6 +119,7 @@ void board_init_f(ulong dummy) > { > const struct cm_config *cm_default_cfg = cm_get_default_config(); > int ret; > + struct udevice *dev; > > #ifdef CONFIG_HW_WATCHDOG > /* Ensure watchdog is paused when debugging is happening */ > @@ -175,11 +176,13 @@ void board_init_f(ulong dummy) > clrbits_le32(CCU_REG_ADDR(CCU_IOM_MPRT_ADMASK_MEM_RAM0), > CCU_ADMASK_P_MASK | CCU_ADMASK_NS_MASK); > > - debug("DDR: Initializing Hard Memory Controller\n"); > - if (sdram_mmr_init_full(0)) { > - puts("DDR: Initialization failed.\n"); > +#ifdef CONFIG_ALTERA_SDRAM #if CONFIG_IS_ENABLED(ALTERA_SDRAM) (really, altera sdram ? How will this work with other SDRAM controllers in the FPGA ? I thought that was already discussed too) > + ret = uclass_get_device(UCLASS_RAM, 0, &dev); > + if (ret) { > + debug("DRAM init failed: %d\n", ret); > hang(); > } > +#endif /* CONFIG_ALTERA_SDRAM */ > > mbox_init(); > > diff --git a/configs/socfpga_stratix10_defconfig b/configs/socfpga_stratix10_defconfig > index 4848013b21..bda6ab6637 100644 > --- a/configs/socfpga_stratix10_defconfig > +++ b/configs/socfpga_stratix10_defconfig > @@ -32,6 +32,7 @@ CONFIG_ENV_IS_IN_MMC=y > CONFIG_NET_RANDOM_ETHADDR=y > CONFIG_SPL_DM=y > CONFIG_SPL_DM_SEQ_ALIAS=y > +CONFIG_ALTERA_SDRAM=y > CONFIG_DM_GPIO=y > CONFIG_DWAPB_GPIO=y > CONFIG_DM_I2C=y > diff --git a/drivers/ddr/altera/Kconfig b/drivers/ddr/altera/Kconfig > index 8f60b56eb8..112c4ad7c3 100644 > --- a/drivers/ddr/altera/Kconfig > +++ b/drivers/ddr/altera/Kconfig > @@ -1,7 +1,7 @@ > config ALTERA_SDRAM > bool "SoCFPGA DDR SDRAM driver" > - depends on TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10 > - select RAM if TARGET_SOCFPGA_GEN5 > - select SPL_RAM if TARGET_SOCFPGA_GEN5 > + depends on TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10 || TARGET_SOCFPGA_STRATIX10 > + select RAM if TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_STRATIX10 > + select SPL_RAM if TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_STRATIX10 > help > Enable DDR SDRAM controller for the SoCFPGA devices. > diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c > index e4d4a02ca2..d2f3272609 100644 > --- a/drivers/ddr/altera/sdram_s10.c > +++ b/drivers/ddr/altera/sdram_s10.c > @@ -5,17 +5,32 @@ > */ > > #include > +#include > #include > #include > #include > -#include > +#include > +#include > +#include "sdram_s10.h" > #include > #include > -#include > #include > #include > +#include > #include > > +#ifdef CONFIG_SPL_BUILD Is this ifdef really needed ? > +struct altera_sdram_priv { > + struct ram_info info; > +}; > + > +struct altera_sdram_platdata { > + void __iomem *hmc; > + void __iomem *ddr_sch; > + void __iomem *iomhc; > +}; > + > DECLARE_GLOBAL_DATA_PTR; > > static const struct socfpga_system_manager *sysmgr_regs = > @@ -51,25 +66,26 @@ u32 ddr_config[] = { > DDR_CONFIG(1, 4, 10, 17), > }; > > -static u32 hmc_readl(u32 reg) > +static u32 hmc_readl(struct altera_sdram_platdata *plat, u32 reg) > { > - return readl(((void __iomem *)SOCFPGA_HMC_MMR_IO48_ADDRESS + (reg))); > + return readl(plat->iomhc + (reg)); Superfluous parenthesis around (reg) , drop them, fix globally. [...] > +/** > + * sdram_calculate_size() - Calculate SDRAM size > + * > + * Calculate SDRAM device size based on SDRAM controller parameters. > + * Size is specified in bytes. > + */ > +static phys_size_t sdram_calculate_size(struct altera_sdram_platdata *plat) > +{ > + u32 dramaddrw = hmc_readl(plat, DRAMADDRW); Is this a new piece of code ? > + phys_size_t size = 1 << (DRAMADDRW_CFG_CS_ADDR_WIDTH(dramaddrw) + > + DRAMADDRW_CFG_BANK_GRP_ADDR_WIDTH(dramaddrw) + > + DRAMADDRW_CFG_BANK_ADDR_WIDTH(dramaddrw) + > + DRAMADDRW_CFG_ROW_ADDR_WIDTH(dramaddrw) + > + DRAMADDRW_CFG_COL_ADDR_WIDTH(dramaddrw)); > + > + size *= (2 << (hmc_ecc_readl(plat, DDRIOCTRL) & > + DDR_HMC_DDRIOCTRL_IOSIZE_MSK)); > + > + return size; > +} > + [...] > +static int altera_sdram_probe(struct udevice *dev) > +{ > + int ret; > + struct reset_ctl_bulk resets; > + > + ret = reset_get_bulk(dev, &resets); > + if (ret) { > + dev_err(dev, "Can't get reset: %d\n", ret); > + return -ENODEV; > + } > + reset_deassert_bulk(&resets); > + > + if (sdram_mmr_init_full(dev) != 0) { > + puts("SDRAM init failed.\n"); > + goto failed; > + } > + > + return 0; > + > +failed: > + reset_release_bulk(&resets); > + return -ENODEV; > } Are you missing altera_sdram_remove() , which would assert reset here ? [...] -- Best regards, Marek Vasut