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

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 ley.foon.tan@intel.com --- 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@f8000400 { + compatible = "altr,sdr-ctl-s10"; + reg = <0xf8000400 0x80>, + <0xf8010000 0x190>, + <0xf8011000 0x500>; + resets = <&rst DDRSCH_RESET>; + u-boot,dm-pre-reloc; + }; + spi0: spi@ffda4000 { compatible = "snps,dw-apb-ssi"; #address-cells = <1>; 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 <asm/arch/firewall_s10.h> #include <asm/arch/mailbox_s10.h> #include <asm/arch/reset_manager.h> -#include <asm/arch/sdram_s10.h> #include <asm/arch/system_manager.h> #include <watchdog.h> +#include <dm/uclass.h>
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 + 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 <common.h> +#include <dm.h> #include <errno.h> #include <div64.h> #include <fdtdec.h> -#include <asm/io.h> +#include <ram.h> +#include <reset.h> +#include "sdram_s10.h" #include <wait_bit.h> #include <asm/arch/firewall_s10.h> -#include <asm/arch/sdram_s10.h> #include <asm/arch/system_manager.h> #include <asm/arch/reset_manager.h> +#include <asm/io.h> #include <linux/sizes.h>
+#ifdef CONFIG_SPL_BUILD + +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)); }
-static u32 hmc_ecc_readl(u32 reg) +static u32 hmc_ecc_readl(struct altera_sdram_platdata *plat, u32 reg) { - return readl((void __iomem *)SOCFPGA_SDR_ADDRESS + (reg)); + return readl(plat->hmc + (reg)); }
-static u32 hmc_ecc_writel(u32 data, u32 reg) +static u32 hmc_ecc_writel(struct altera_sdram_platdata *plat, + u32 data, u32 reg) { - return writel(data, (void __iomem *)SOCFPGA_SDR_ADDRESS + (reg)); + return writel(data, plat->hmc + (reg)); }
-static u32 ddr_sch_writel(u32 data, u32 reg) +static u32 ddr_sch_writel(struct altera_sdram_platdata *plat, u32 data, + u32 reg) { - return writel(data, - (void __iomem *)SOCFPGA_SDR_SCHEDULER_ADDRESS + (reg)); + return writel(data, plat->ddr_sch + (reg)); }
int match_ddr_conf(u32 ddr_conf) @@ -83,37 +99,38 @@ int match_ddr_conf(u32 ddr_conf) return 0; }
-static int emif_clear(void) +static int emif_clear(struct altera_sdram_platdata *plat) { - hmc_ecc_writel(0, RSTHANDSHAKECTRL); + hmc_ecc_writel(plat, 0, RSTHANDSHAKECTRL);
- return wait_for_bit_le32((const void *)(SOCFPGA_SDR_ADDRESS + + return wait_for_bit_le32((const void *)(plat->hmc + RSTHANDSHAKESTAT), DDR_HMC_RSTHANDSHAKE_MASK, false, 1000, false); }
-static int emif_reset(void) +static int emif_reset(struct altera_sdram_platdata *plat) { u32 c2s, s2c, ret;
- c2s = hmc_ecc_readl(RSTHANDSHAKECTRL) & DDR_HMC_RSTHANDSHAKE_MASK; - s2c = hmc_ecc_readl(RSTHANDSHAKESTAT) & DDR_HMC_RSTHANDSHAKE_MASK; + c2s = hmc_ecc_readl(plat, RSTHANDSHAKECTRL) & DDR_HMC_RSTHANDSHAKE_MASK; + s2c = hmc_ecc_readl(plat, RSTHANDSHAKESTAT) & DDR_HMC_RSTHANDSHAKE_MASK;
debug("DDR: c2s=%08x s2c=%08x nr0=%08x nr1=%08x nr2=%08x dst=%08x\n", - c2s, s2c, hmc_readl(NIOSRESERVED0), hmc_readl(NIOSRESERVED1), - hmc_readl(NIOSRESERVED2), hmc_readl(DRAMSTS)); + c2s, s2c, hmc_readl(plat, NIOSRESERVED0), + hmc_readl(plat, NIOSRESERVED1), hmc_readl(plat, NIOSRESERVED2), + hmc_readl(plat, DRAMSTS));
- if (s2c && emif_clear()) { + if (s2c && emif_clear(plat)) { printf("DDR: emif_clear() failed\n"); return -1; }
debug("DDR: Triggerring emif reset\n"); - hmc_ecc_writel(DDR_HMC_CORE2SEQ_INT_REQ, RSTHANDSHAKECTRL); + hmc_ecc_writel(plat, DDR_HMC_CORE2SEQ_INT_REQ, RSTHANDSHAKECTRL);
/* if seq2core[3] = 0, we are good */ - ret = wait_for_bit_le32((const void *)(SOCFPGA_SDR_ADDRESS + + ret = wait_for_bit_le32((const void *)(plat->hmc + RSTHANDSHAKESTAT), DDR_HMC_SEQ2CORE_INT_RESP_MASK, false, 1000, false); @@ -122,7 +139,7 @@ static int emif_reset(void) return ret; }
- ret = emif_clear(); + ret = emif_clear(plat); if (ret) { printf("DDR: emif_clear() failed\n"); return ret; @@ -240,13 +257,37 @@ static void sdram_size_check(bd_t *bd) debug("DDR: SDRAM size check passed!\n"); }
+/** + * 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); + + 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; +} + /** * sdram_mmr_init_full() - Function to initialize SDRAM MMR * * Initialize the SDRAM MMR. */ -int sdram_mmr_init_full(unsigned int unused) +static int sdram_mmr_init_full(struct udevice *dev) { + struct altera_sdram_platdata *plat = dev->platdata; + struct altera_sdram_priv *priv = dev_get_priv(dev); u32 update_value, io48_value, ddrioctl; u32 i; int ret; @@ -303,19 +344,16 @@ int sdram_mmr_init_full(unsigned int unused) return -1; }
- /* release DDR scheduler from reset */ - socfpga_per_reset(SOCFPGA_RESET(SDR), 0); - /* Try 3 times to do a calibration */ for (i = 0; i < 3; i++) { - ret = wait_for_bit_le32((const void *)(SOCFPGA_SDR_ADDRESS + + ret = wait_for_bit_le32((const void *)(plat->hmc + DDRCALSTAT), DDR_HMC_DDRCALSTAT_CAL_MSK, true, 1000, false); if (!ret) break;
- emif_reset(); + emif_reset(plat); }
if (ret) { @@ -324,16 +362,16 @@ int sdram_mmr_init_full(unsigned int unused) } debug("DDR: Calibration success\n");
- u32 ctrlcfg0 = hmc_readl(CTRLCFG0); - u32 ctrlcfg1 = hmc_readl(CTRLCFG1); - u32 dramaddrw = hmc_readl(DRAMADDRW); - u32 dramtim0 = hmc_readl(DRAMTIMING0); - u32 caltim0 = hmc_readl(CALTIMING0); - u32 caltim1 = hmc_readl(CALTIMING1); - u32 caltim2 = hmc_readl(CALTIMING2); - u32 caltim3 = hmc_readl(CALTIMING3); - u32 caltim4 = hmc_readl(CALTIMING4); - u32 caltim9 = hmc_readl(CALTIMING9); + u32 ctrlcfg0 = hmc_readl(plat, CTRLCFG0); + u32 ctrlcfg1 = hmc_readl(plat, CTRLCFG1); + u32 dramaddrw = hmc_readl(plat, DRAMADDRW); + u32 dramtim0 = hmc_readl(plat, DRAMTIMING0); + u32 caltim0 = hmc_readl(plat, CALTIMING0); + u32 caltim1 = hmc_readl(plat, CALTIMING1); + u32 caltim2 = hmc_readl(plat, CALTIMING2); + u32 caltim3 = hmc_readl(plat, CALTIMING3); + u32 caltim4 = hmc_readl(plat, CALTIMING4); + u32 caltim9 = hmc_readl(plat, CALTIMING9);
/* * Configure the DDR IO size [0xFFCFB008] @@ -349,12 +387,12 @@ int sdram_mmr_init_full(unsigned int unused) * bit[9:6] = Minor Release # * bit[14:10] = Major Release # */ - update_value = hmc_readl(NIOSRESERVED0); - hmc_ecc_writel(((update_value & 0xFF) >> 5), DDRIOCTRL); - ddrioctl = hmc_ecc_readl(DDRIOCTRL); + update_value = hmc_readl(plat, NIOSRESERVED0); + hmc_ecc_writel(plat, ((update_value & 0xFF) >> 5), DDRIOCTRL); + ddrioctl = hmc_ecc_readl(plat, DDRIOCTRL);
/* enable HPS interface to HMC */ - hmc_ecc_writel(DDR_HMC_HPSINTFCSEL_ENABLE_MASK, HPSINTFCSEL); + hmc_ecc_writel(plat, DDR_HMC_HPSINTFCSEL_ENABLE_MASK, HPSINTFCSEL);
/* Set the DDR Configuration */ io48_value = DDR_CONFIG(CTRLCFG1_CFG_ADDR_ORDER(ctrlcfg1), @@ -365,10 +403,10 @@ int sdram_mmr_init_full(unsigned int unused)
update_value = match_ddr_conf(io48_value); if (update_value) - ddr_sch_writel(update_value, DDR_SCH_DDRCONF); + ddr_sch_writel(plat, update_value, DDR_SCH_DDRCONF);
/* Configure HMC dramaddrw */ - hmc_ecc_writel(hmc_readl(DRAMADDRW), DRAMADDRWIDTH); + hmc_ecc_writel(plat, hmc_readl(plat, DRAMADDRW), DRAMADDRWIDTH);
/* * Configure DDR timing @@ -392,7 +430,7 @@ int sdram_mmr_init_full(unsigned int unused) CALTIMING0_CFG_ACT_TO_RDWR(caltim0) + CALTIMING4_CFG_PCH_TO_VALID(caltim4));
- ddr_sch_writel(((CALTIMING0_CFG_ACT_TO_ACT(caltim0) << + ddr_sch_writel(plat, ((CALTIMING0_CFG_ACT_TO_ACT(caltim0) << DDR_SCH_DDRTIMING_ACTTOACT_OFF) | (update_value << DDR_SCH_DDRTIMING_RDTOMISS_OFF) | (io48_value << DDR_SCH_DDRTIMING_WRTOMISS_OFF) | @@ -406,12 +444,12 @@ int sdram_mmr_init_full(unsigned int unused) DDR_SCH_DDRTIMING);
/* Configure DDR mode [precharge = 0] */ - ddr_sch_writel(((ddrioctl ? 0 : 1) << + ddr_sch_writel(plat, ((ddrioctl ? 0 : 1) << DDR_SCH_DDRMOD_BWRATIOEXTENDED_OFF), DDR_SCH_DDRMODE);
/* Configure the read latency */ - ddr_sch_writel((DRAMTIMING0_CFG_TCL(dramtim0) >> 1) + + ddr_sch_writel(plat, (DRAMTIMING0_CFG_TCL(dramtim0) >> 1) + DDR_READ_LATENCY_DELAY, DDR_SCH_READ_LATENCY);
@@ -419,7 +457,7 @@ int sdram_mmr_init_full(unsigned int unused) * Configuring timing values concerning activate commands * [FAWBANK alway 1 because always 4 bank DDR] */ - ddr_sch_writel(((CALTIMING0_CFG_ACT_TO_ACT_DB(caltim0) << + ddr_sch_writel(plat, ((CALTIMING0_CFG_ACT_TO_ACT_DB(caltim0) << DDR_SCH_ACTIVATE_RRD_OFF) | (CALTIMING9_CFG_4_ACT_TO_ACT(caltim9) << DDR_SCH_ACTIVATE_FAW_OFF) | @@ -431,7 +469,7 @@ int sdram_mmr_init_full(unsigned int unused) * Configuring timing values concerning device to device data bus * ownership change */ - ddr_sch_writel(((CALTIMING1_CFG_RD_TO_RD_DC(caltim1) << + ddr_sch_writel(plat, ((CALTIMING1_CFG_RD_TO_RD_DC(caltim1) << DDR_SCH_DEVTODEV_BUSRDTORD_OFF) | (CALTIMING1_CFG_RD_TO_WR_DC(caltim1) << DDR_SCH_DEVTODEV_BUSRDTOWR_OFF) | @@ -440,7 +478,7 @@ int sdram_mmr_init_full(unsigned int unused) DDR_SCH_DEVTODEV);
/* assigning the SDRAM size */ - unsigned long long size = sdram_calculate_size(); + unsigned long long size = sdram_calculate_size(plat); /* If the size is invalid, use default Config size */ if (size <= 0) hw_size = PHYS_SDRAM_1_SIZE; @@ -462,18 +500,17 @@ int sdram_mmr_init_full(unsigned int unused)
/* Enable or disable the SDRAM ECC */ if (CTRLCFG1_CFG_CTRL_EN_ECC(ctrlcfg1)) { - setbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL1, + setbits_le32(plat->hmc + ECCCTRL1, (DDR_HMC_ECCCTL_AWB_CNT_RST_SET_MSK | DDR_HMC_ECCCTL_CNT_RST_SET_MSK | DDR_HMC_ECCCTL_ECC_EN_SET_MSK)); - clrbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL1, + clrbits_le32(plat->hmc + ECCCTRL1, (DDR_HMC_ECCCTL_AWB_CNT_RST_SET_MSK | DDR_HMC_ECCCTL_CNT_RST_SET_MSK)); - setbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL2, + setbits_le32(plat->hmc + ECCCTRL2, (DDR_HMC_ECCCTL2_RMW_EN_SET_MSK | DDR_HMC_ECCCTL2_AWB_EN_SET_MSK)); - writel(DDR_HMC_ERRINTEN_INTMASK, - SOCFPGA_SDR_ADDRESS + ERRINTENS); + hmc_ecc_writel(plat, DDR_HMC_ERRINTEN_INTMASK, ERRINTENS);
/* Enable non-secure writes to HMC Adapter for SDRAM ECC */ writel(FW_HMC_ADAPTOR_MPU_MASK, FW_HMC_ADAPTOR_REG_ADDR); @@ -482,39 +519,100 @@ int sdram_mmr_init_full(unsigned int unused) if (!cpu_has_been_warmreset()) sdram_init_ecc_bits(&bd); } else { - clrbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL1, + clrbits_le32(plat->hmc + ECCCTRL1, (DDR_HMC_ECCCTL_AWB_CNT_RST_SET_MSK | DDR_HMC_ECCCTL_CNT_RST_SET_MSK | DDR_HMC_ECCCTL_ECC_EN_SET_MSK)); - clrbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL2, + clrbits_le32(plat->hmc + ECCCTRL2, (DDR_HMC_ECCCTL2_RMW_EN_SET_MSK | DDR_HMC_ECCCTL2_AWB_EN_SET_MSK)); }
sdram_size_check(&bd);
+ priv->info.base = bd.bi_dram[0].start; + priv->info.size = gd->ram_size; + debug("DDR: HMC init success\n"); return 0; }
-/** - * sdram_calculate_size() - Calculate SDRAM size - * - * Calculate SDRAM device size based on SDRAM controller parameters. - * Size is specified in bytes. - */ -phys_size_t sdram_calculate_size(void) +static int altera_sdram_ofdata_to_platdata(struct udevice *dev) { - u32 dramaddrw = hmc_readl(DRAMADDRW); + struct altera_sdram_platdata *plat = dev->platdata; + fdt_addr_t addr;
- 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)); + addr = dev_read_addr_index(dev, 0); + if (addr == FDT_ADDR_T_NONE) + return -EINVAL; + plat->ddr_sch = (void __iomem *)addr;
- size *= (2 << (hmc_ecc_readl(DDRIOCTRL) & - DDR_HMC_DDRIOCTRL_IOSIZE_MSK)); + addr = dev_read_addr_index(dev, 1); + if (addr == FDT_ADDR_T_NONE) + return -EINVAL; + plat->iomhc = (void __iomem *)addr;
- return size; + addr = dev_read_addr_index(dev, 2); + if (addr == FDT_ADDR_T_NONE) + return -EINVAL; + plat->hmc = (void __iomem *)addr; + + return 0; +} + +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; } + +static int altera_sdram_get_info(struct udevice *dev, + struct ram_info *info) +{ + struct altera_sdram_priv *priv = dev_get_priv(dev); + + info->base = priv->info.base; + info->size = priv->info.size; + + return 0; +} + +static struct ram_ops altera_sdram_ops = { + .get_info = altera_sdram_get_info, +}; + +static const struct udevice_id altera_sdram_ids[] = { + { .compatible = "altr,sdr-ctl-s10" }, + { /* sentinel */ } +}; + +U_BOOT_DRIVER(altera_sdram) = { + .name = "altr_sdr_ctl", + .id = UCLASS_RAM, + .of_match = altera_sdram_ids, + .ops = &altera_sdram_ops, + .ofdata_to_platdata = altera_sdram_ofdata_to_platdata, + .platdata_auto_alloc_size = sizeof(struct altera_sdram_platdata), + .probe = altera_sdram_probe, + .priv_auto_alloc_size = sizeof(struct altera_sdram_priv), +}; + +#endif /* CONFIG_SPL_BUILD */ diff --git a/arch/arm/mach-socfpga/include/mach/sdram_s10.h b/drivers/ddr/altera/sdram_s10.h similarity index 97% rename from arch/arm/mach-socfpga/include/mach/sdram_s10.h rename to drivers/ddr/altera/sdram_s10.h index f39206ca1e..096c06cba2 100644 --- a/arch/arm/mach-socfpga/include/mach/sdram_s10.h +++ b/drivers/ddr/altera/sdram_s10.h @@ -7,10 +7,6 @@ #ifndef _SDRAM_S10_H_ #define _SDRAM_S10_H_
-phys_size_t sdram_calculate_size(void); -int sdram_mmr_init_full(unsigned int sdr_phy_reg); -int sdram_calibration_full(void); - #define DDR_TWR 15 #define DDR_READ_LATENCY_DELAY 40 #define DDR_ACTIVATE_FAWBANK 0x1 diff --git a/include/configs/socfpga_stratix10_socdk.h b/include/configs/socfpga_stratix10_socdk.h index 31c267f55d..2af808cadf 100644 --- a/include/configs/socfpga_stratix10_socdk.h +++ b/include/configs/socfpga_stratix10_socdk.h @@ -129,11 +129,6 @@ unsigned int cm_get_qspi_controller_clk_hz(void); #define CONFIG_SYS_MEMTEST_START 0 #define CONFIG_SYS_MEMTEST_END PHYS_SDRAM_1_SIZE - 0x200000
-/* - * SDRAM controller - */ -#define CONFIG_ALTERA_SDRAM - /* * Serial / UART configurations */

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 ley.foon.tan@intel.com
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@f8000400 {
compatible = "altr,sdr-ctl-s10";
reg = <0xf8000400 0x80>,
<0xf8010000 0x190>,
<0xf8011000 0x500>;
resets = <&rst DDRSCH_RESET>;
u-boot,dm-pre-reloc;
};
- spi0: spi@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 <asm/arch/firewall_s10.h> #include <asm/arch/mailbox_s10.h> #include <asm/arch/reset_manager.h> -#include <asm/arch/sdram_s10.h> #include <asm/arch/system_manager.h> #include <watchdog.h> +#include <dm/uclass.h>
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) {
hang(); }debug("DRAM init failed: %d\n", ret);
+#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 <common.h> +#include <dm.h> #include <errno.h> #include <div64.h> #include <fdtdec.h> -#include <asm/io.h> +#include <ram.h> +#include <reset.h> +#include "sdram_s10.h" #include <wait_bit.h> #include <asm/arch/firewall_s10.h> -#include <asm/arch/sdram_s10.h> #include <asm/arch/system_manager.h> #include <asm/arch/reset_manager.h> +#include <asm/io.h> #include <linux/sizes.h>
+#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 ?
[...]

On Wed, Apr 24, 2019 at 11:01 PM Marek Vasut marex@denx.de wrote:
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 ley.foon.tan@intel.com
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@f8000400 {
compatible = "altr,sdr-ctl-s10";
reg = <0xf8000400 0x80>,
<0xf8010000 0x190>,
<0xf8011000 0x500>;
resets = <&rst DDRSCH_RESET>;
u-boot,dm-pre-reloc;
};
spi0: spi@ffda4000 { compatible = "snps,dw-apb-ssi"; #address-cells = <1>;
Separate patch please
Okay.
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 <asm/arch/firewall_s10.h> #include <asm/arch/mailbox_s10.h> #include <asm/arch/reset_manager.h> -#include <asm/arch/sdram_s10.h> #include <asm/arch/system_manager.h> #include <watchdog.h> +#include <dm/uclass.h>
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)
Tried change to #if CONFIG_IS_ENABLED(ALTERA_SDRAM), but it is not get compiled in SPL. Found that it is only working if CONFIG consist of _SPL_ keyword, i.e: CONFIG_SPL_ALTERA_SDRAM.
FPGA SDRAM doesn't really need a driver. So, we don't need to probe UCLASS_RAM here.
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 <common.h> +#include <dm.h> #include <errno.h> #include <div64.h> #include <fdtdec.h> -#include <asm/io.h> +#include <ram.h> +#include <reset.h> +#include "sdram_s10.h" #include <wait_bit.h> #include <asm/arch/firewall_s10.h> -#include <asm/arch/sdram_s10.h> #include <asm/arch/system_manager.h> #include <asm/arch/reset_manager.h> +#include <asm/io.h> #include <linux/sizes.h>
+#ifdef CONFIG_SPL_BUILD
Is this ifdef really needed ?
Yes, these code will compile into Uboot image as well.
+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.
Okay.
[...]
+/**
- 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 ?
Not new. Just move this function to earlier of the file, before call to this function.
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 ?
Will add it.
[...]
--

On 4/26/19 10:23 AM, Ley Foon Tan wrote:
On Wed, Apr 24, 2019 at 11:01 PM Marek Vasut marex@denx.de wrote:
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 ley.foon.tan@intel.com
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@f8000400 {
compatible = "altr,sdr-ctl-s10";
reg = <0xf8000400 0x80>,
<0xf8010000 0x190>,
<0xf8011000 0x500>;
resets = <&rst DDRSCH_RESET>;
u-boot,dm-pre-reloc;
};
spi0: spi@ffda4000 { compatible = "snps,dw-apb-ssi"; #address-cells = <1>;
Separate patch please
Okay.
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 <asm/arch/firewall_s10.h> #include <asm/arch/mailbox_s10.h> #include <asm/arch/reset_manager.h> -#include <asm/arch/sdram_s10.h> #include <asm/arch/system_manager.h> #include <watchdog.h> +#include <dm/uclass.h>
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)
Tried change to #if CONFIG_IS_ENABLED(ALTERA_SDRAM), but it is not get compiled in SPL. Found that it is only working if CONFIG consist of _SPL_ keyword, i.e: CONFIG_SPL_ALTERA_SDRAM.
Isn't that exactly what you want -- compile the DRAM driver only into SPL ?
FPGA SDRAM doesn't really need a driver. So, we don't need to probe UCLASS_RAM here.
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 <common.h> +#include <dm.h> #include <errno.h> #include <div64.h> #include <fdtdec.h> -#include <asm/io.h> +#include <ram.h> +#include <reset.h> +#include "sdram_s10.h" #include <wait_bit.h> #include <asm/arch/firewall_s10.h> -#include <asm/arch/sdram_s10.h> #include <asm/arch/system_manager.h> #include <asm/arch/reset_manager.h> +#include <asm/io.h> #include <linux/sizes.h>
+#ifdef CONFIG_SPL_BUILD
Is this ifdef really needed ?
Yes, these code will compile into Uboot image as well.
No, it won't, the code will only be compiled into the SPL with this ifdef. But that's something you should solve on the Kconfig/Kbuild level.
The way forward here is to create a new Kconfig symbol, some CONFIG_SPL_ALTERA_STRATIX10_DRAM or somesuch and then add a new entry into drivers/ddr/altera/Makefile to only compile this entire code when that symbol is enabled.
+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.
Okay.
[...]
+/**
- 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 ?
Not new. Just move this function to earlier of the file, before call to this function.
OK
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 ?
Will add it.
But won't that prevent the DRAM controller from working ?

On Mon, Apr 29, 2019 at 5:55 PM Marek Vasut marex@denx.de wrote:
On 4/26/19 10:23 AM, Ley Foon Tan wrote:
On Wed, Apr 24, 2019 at 11:01 PM Marek Vasut marex@denx.de wrote:
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 ley.foon.tan@intel.com
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@f8000400 {
compatible = "altr,sdr-ctl-s10";
reg = <0xf8000400 0x80>,
<0xf8010000 0x190>,
<0xf8011000 0x500>;
resets = <&rst DDRSCH_RESET>;
u-boot,dm-pre-reloc;
};
spi0: spi@ffda4000 { compatible = "snps,dw-apb-ssi"; #address-cells = <1>;
Separate patch please
Okay.
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 <asm/arch/firewall_s10.h> #include <asm/arch/mailbox_s10.h> #include <asm/arch/reset_manager.h> -#include <asm/arch/sdram_s10.h> #include <asm/arch/system_manager.h> #include <watchdog.h> +#include <dm/uclass.h>
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)
Tried change to #if CONFIG_IS_ENABLED(ALTERA_SDRAM), but it is not get compiled in SPL. Found that it is only working if CONFIG consist of _SPL_ keyword, i.e: CONFIG_SPL_ALTERA_SDRAM.
Isn't that exactly what you want -- compile the DRAM driver only into SPL ?
Yes. Will change to CONFIG_SPL_ALTERA_SDRAM.
FPGA SDRAM doesn't really need a driver. So, we don't need to probe UCLASS_RAM here.
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 <common.h> +#include <dm.h> #include <errno.h> #include <div64.h> #include <fdtdec.h> -#include <asm/io.h> +#include <ram.h> +#include <reset.h> +#include "sdram_s10.h" #include <wait_bit.h> #include <asm/arch/firewall_s10.h> -#include <asm/arch/sdram_s10.h> #include <asm/arch/system_manager.h> #include <asm/arch/reset_manager.h> +#include <asm/io.h> #include <linux/sizes.h>
+#ifdef CONFIG_SPL_BUILD
Is this ifdef really needed ?
Yes, these code will compile into Uboot image as well.
No, it won't, the code will only be compiled into the SPL with this ifdef. But that's something you should solve on the Kconfig/Kbuild level.
The way forward here is to create a new Kconfig symbol, some CONFIG_SPL_ALTERA_STRATIX10_DRAM or somesuch and then add a new entry into drivers/ddr/altera/Makefile to only compile this entire code when that symbol is enabled.
Will change ALTERA_SDRAM to SPL_ALTERA_SDRAM and compile in SPL only.
+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.
Okay.
[...]
+/**
- 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 ?
Not new. Just move this function to earlier of the file, before call to this function.
OK
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 ?
Will add it.
But won't that prevent the DRAM controller from working ?
Added assert reset in _remove, SDRAM controller still can work after boot to Uboot. Seem _remove is not called when boot to Uboot.
Regards Ley Foon

On 4/30/19 9:40 AM, Ley Foon Tan wrote: [...]
+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 ?
Will add it.
But won't that prevent the DRAM controller from working ?
Added assert reset in _remove, SDRAM controller still can work after boot to Uboot. Seem _remove is not called when boot to Uboot.
Look at DM_FLAG_OS_PREPARE

On Tue, Apr 30, 2019 at 5:56 PM Marek Vasut marex@denx.de wrote:
On 4/30/19 9:40 AM, Ley Foon Tan wrote: [...]
+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 ?
Will add it.
But won't that prevent the DRAM controller from working ?
Added assert reset in _remove, SDRAM controller still can work after boot to Uboot. Seem _remove is not called when boot to Uboot.
Look at DM_FLAG_OS_PREPARE
Tested add DM_FLAG_OS_PREPARE flag, but still didn't see it call to _remove().
BTW, I think we shouldn't assert SDRAM controller. Otherwise, SDRAM is not working in next boot stage.
Regards Ley Foon

On 5/2/19 5:46 AM, Ley Foon Tan wrote:
On Tue, Apr 30, 2019 at 5:56 PM Marek Vasut marex@denx.de wrote:
On 4/30/19 9:40 AM, Ley Foon Tan wrote: [...]
> +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 ?
Will add it.
But won't that prevent the DRAM controller from working ?
Added assert reset in _remove, SDRAM controller still can work after boot to Uboot. Seem _remove is not called when boot to Uboot.
Look at DM_FLAG_OS_PREPARE
Tested add DM_FLAG_OS_PREPARE flag, but still didn't see it call to _remove().
BTW, I think we shouldn't assert SDRAM controller. Otherwise, SDRAM is not working in next boot stage.
Hence my question above -- "But won't that prevent the DRAM controller from working ?"
I think you're right.
participants (3)
-
Ley Foon Tan
-
Ley Foon Tan
-
Marek Vasut