[U-Boot] [PATCH v2] arm: socfpga: Move Stratix 10 SDRAM driver to DM
Marek Vasut
marex at denx.de
Wed Apr 24 14:43:12 UTC 2019
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 at 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 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 <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) {
> + 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 ?
> +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
More information about the U-Boot
mailing list