diff mbox series

[v5,06/14] sifive: fu540: add ddr driver

Message ID 20200311070320.21323-7-pragnesh.patel@sifive.com
State New
Headers show
Series RISC-V SiFive FU540 support SPL | expand

Commit Message

Pragnesh Patel March 11, 2020, 7:03 a.m. UTC 
Add driver for fu540 to support ddr initialization in SPL.
This driver is based on FSBL
(https://github.com/sifive/freedom-u540-c000-bootloader.git)

Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com>
---
 drivers/ram/Kconfig              |   7 +
 drivers/ram/Makefile             |   2 +
 drivers/ram/sifive/Kconfig       |   8 +
 drivers/ram/sifive/Makefile      |   6 +
 drivers/ram/sifive/sdram_fu540.c | 295 +++++++++++++++++++++++++++++++
 drivers/ram/sifive/sdram_fu540.h |  94 ++++++++++
 6 files changed, 412 insertions(+)
 create mode 100644 drivers/ram/sifive/Kconfig
 create mode 100644 drivers/ram/sifive/Makefile
 create mode 100644 drivers/ram/sifive/sdram_fu540.c
 create mode 100644 drivers/ram/sifive/sdram_fu540.h

Comments

Bin Meng March 13, 2020, 7:48 a.m. UTC | #1 
On Wed, Mar 11, 2020 at 3:04 PM Pragnesh Patel
<pragnesh.patel at sifive.com> wrote:
>
> Add driver for fu540 to support ddr initialization in SPL.
> This driver is based on FSBL
> (https://github.com/sifive/freedom-u540-c000-bootloader.git)
>
> Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com>
> ---
>  drivers/ram/Kconfig              |   7 +
>  drivers/ram/Makefile             |   2 +
>  drivers/ram/sifive/Kconfig       |   8 +
>  drivers/ram/sifive/Makefile      |   6 +
>  drivers/ram/sifive/sdram_fu540.c | 295 +++++++++++++++++++++++++++++++
>  drivers/ram/sifive/sdram_fu540.h |  94 ++++++++++
>  6 files changed, 412 insertions(+)
>  create mode 100644 drivers/ram/sifive/Kconfig
>  create mode 100644 drivers/ram/sifive/Makefile
>  create mode 100644 drivers/ram/sifive/sdram_fu540.c
>  create mode 100644 drivers/ram/sifive/sdram_fu540.h
>
> diff --git a/drivers/ram/Kconfig b/drivers/ram/Kconfig
> index 56fea7c94c..c60c63204c 100644
> --- a/drivers/ram/Kconfig
> +++ b/drivers/ram/Kconfig
> @@ -73,5 +73,12 @@ config IMXRT_SDRAM
>           to support external memories like sdram, psram & nand.
>           This driver is for the sdram memory interface with the SEMC.
>
> +config SIFIVE_DDR
> +       bool "Enable SiFive DDR support"
> +       depends on RAM
> +       help
> +         Enable support for the internal DDR Memory Controller of SiFive SoCs.
> +
>  source "drivers/ram/rockchip/Kconfig"
>  source "drivers/ram/stm32mp1/Kconfig"
> +source "drivers/ram/sifive/Kconfig"
> diff --git a/drivers/ram/Makefile b/drivers/ram/Makefile
> index 5c897410c6..12bf61510b 100644
> --- a/drivers/ram/Makefile
> +++ b/drivers/ram/Makefile
> @@ -17,3 +17,5 @@ obj-$(CONFIG_ARCH_MEDIATEK) += mediatek/
>  obj-$(CONFIG_K3_J721E_DDRSS) += k3-j721e/
>
>  obj-$(CONFIG_IMXRT_SDRAM) += imxrt_sdram.o
> +
> +obj-$(CONFIG_SIFIVE_DDR) += sifive/
> diff --git a/drivers/ram/sifive/Kconfig b/drivers/ram/sifive/Kconfig
> new file mode 100644
> index 0000000000..b754700db8
> --- /dev/null
> +++ b/drivers/ram/sifive/Kconfig
> @@ -0,0 +1,8 @@
> +config SIFIVE_FU540_DDR
> +       bool "SiFive FU540 DDR driver"
> +       depends on DM && OF_CONTROL
> +       select RAM

Do we need this driver for U-Boot proper?

> +       select SPL_RAM if SPL
> +       select SIFIVE_DDR
> +       help
> +         This enables DDR support for the platforms based on SiFive FU540 SoC.
> diff --git a/drivers/ram/sifive/Makefile b/drivers/ram/sifive/Makefile
> new file mode 100644
> index 0000000000..0187805199
> --- /dev/null
> +++ b/drivers/ram/sifive/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright (c) 2020 SiFive, Inc
> +#
> +
> +obj-$(CONFIG_SIFIVE_FU540_DDR) += sdram_fu540.o
> diff --git a/drivers/ram/sifive/sdram_fu540.c b/drivers/ram/sifive/sdram_fu540.c
> new file mode 100644
> index 0000000000..18926dbe15
> --- /dev/null
> +++ b/drivers/ram/sifive/sdram_fu540.c
> @@ -0,0 +1,295 @@
> +// SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause
> +/*
> + * (C) Copyright 2020 SiFive, Inc.
> + *
> + * Authors:
> + *   Pragnesh Patel <pragnesh.patel at sifive.com>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <init.h>
> +#include <ram.h>
> +#include <regmap.h>
> +#include <syscon.h>
> +#include <asm/io.h>
> +#include "sdram_fu540.h"
> +
> +/* n: Unit bytes */
> +void sdram_copy_to_reg(volatile u32 *dest, volatile u32 *src, u32 n)

This should be static, also use "int n"?

> +{
> +       int i;
> +
> +       for (i = 0; i < n / sizeof(u32); i++) {
> +               writel(*src, dest);
> +               src++;
> +               dest++;
> +       }
> +}
> +
> +static void ddr_setuprangeprotection(volatile u32 *ctl, u64 end_addr)

__maybe_unused ? Otherwise use #ifdef #endif ?


> +{
> +       writel(0x0, DENALI_CTL_209 + ctl);
> +       u32 end_addr_16kblocks = ((end_addr >> 14) & 0x7FFFFF) - 1;

nits: move the variable declaration

> +
> +       writel(end_addr_16kblocks, DENALI_CTL_210 + ctl);
> +       writel(0x0, DENALI_CTL_212 + ctl);
> +       writel(0x0, DENALI_CTL_214 + ctl);
> +       writel(0x0, DENALI_CTL_216 + ctl);
> +       setbits_le32(DENALI_CTL_224 + ctl,
> +                    0x3 << AXI0_RANGE_PROT_BITS_0_OFFSET);
> +       writel(0xFFFFFFFF, DENALI_CTL_225 + ctl);
> +       setbits_le32(DENALI_CTL_208 + ctl, 0x1 << AXI0_ADDRESS_RANGE_ENABLE);
> +       setbits_le32(DENALI_CTL_208 + ctl,
> +                    0x1 << PORT_ADDR_PROTECTION_EN_OFFSET);
> +}
> +
> +static void ddr_start(volatile u32 *ctl, u32 *physical_filter_ctrl, u64 ddr_end)

__maybe_unused ?

> +{
> +       setbits_le32(DENALI_CTL_0 + ctl, 0x1);
> +       while ((readl(DENALI_CTL_132 + ctl) & (1 << MC_INIT_COMPLETE_OFFSET))
> +              == 0) {
> +       }
> +

Use

do {
    val = readl(DENALI_CTL_132 + ctl) & (1 << MC_INIT_COMPLETE_OFFSET);
} while (val == 0)

?


> +       // Disable the BusBlocker in front of the controller AXI slave ports

nits: use /* */

> +       volatile u64 *filterreg = (volatile u64 *)physical_filter_ctrl;

Please move the variable declaration to the beginning of this function

> +
> +       filterreg[0] = 0x0f00000000000000UL | (ddr_end >> 2);
> +}
> +
> +static u64 ddr_phy_fixup(volatile u32 *ddrphyreg)

__maybe_unused ?

> +{
> +       // return bitmask of failed lanes

nits: use /* */

> +
> +       u64 fails     = 0;
> +       u32 slicebase = 0;
> +       u32 dq        = 0;
> +
> +       // check errata condition

nits: use /* */

> +       for (u32 slice = 0; slice < 8; slice++) {
> +               u32 regbase = slicebase + 34;
> +
> +               for (u32 reg = 0; reg < 4; reg++) {
> +                       u32 updownreg = readl(regbase + reg + ddrphyreg);
> +
> +                       for (u32 bit = 0; bit < 2; bit++) {
> +                               u32 phy_rx_cal_dqn_0_offset;
> +
> +                               if (bit == 0) {
> +                                       phy_rx_cal_dqn_0_offset =
> +                                               PHY_RX_CAL_DQ0_0_OFFSET;
> +                               } else {
> +                                       phy_rx_cal_dqn_0_offset =
> +                                               PHY_RX_CAL_DQ1_0_OFFSET;
> +                               }
> +
> +                               u32 down = (updownreg >>
> +                                           phy_rx_cal_dqn_0_offset) & 0x3F;
> +                               u32 up   = (updownreg >>
> +                                           (phy_rx_cal_dqn_0_offset + 6)) &
> +                                           0x3F;
> +
> +                               u8 failc0 = ((down == 0) && (up == 0x3F));
> +                               u8 failc1 = ((up == 0) && (down == 0x3F));
> +
> +                               // print error message on failure

nits: use /* */

> +                               if (failc0 || failc1) {
> +                                       if (fails == 0)
> +                                               printf("DDR error in fixing up\n");
> +
> +                                       fails |= (1 << dq);
> +
> +                                       char slicelsc = '0';
> +                                       char slicemsc = '0';
> +
> +                                       slicelsc += (dq % 10);
> +                                       slicemsc += (dq / 10);
> +                                       printf("S ");
> +                                       printf("%c", slicemsc);
> +                                       printf("%c", slicelsc);
> +
> +                                       if (failc0)
> +                                               printf("U");
> +                                       else
> +                                               printf("D");
> +
> +                                       printf("\n");
> +                               }
> +                               dq++;
> +                       }
> +               }
> +               slicebase += 128;
> +       }
> +       return(0);
> +}
> +
> +static u32 ddr_getdramclass(volatile u32 *ctl)

__maybe_unused ?

> +{
> +       u32 reg = readl(DENALI_CTL_0 + ctl);
> +
> +       return ((reg >> DRAM_CLASS_OFFSET) & 0xF);
> +}
> +
> +static __maybe_unused int fu540_ddr_setup(struct udevice *dev)
> +{
> +       struct ddr_info *priv = dev_get_priv(dev);
> +       struct sifive_dmc_plat *plat = dev_get_platdata(dev);
> +
> +       int ret, i;
> +       u32 physet;
> +
> +       volatile u32 *denali_ctl =  &priv->ctl->denali_ctl[0];
> +       volatile u32 *denali_phy =  &priv->phy->denali_phy[0];
> +
> +       ret = dev_read_u32_array(dev, "sifive,sdram-params",
> +                                (u32 *)&plat->sdram_params,
> +                                sizeof(plat->sdram_params) / sizeof(u32));
> +       if (ret) {
> +               printf("%s: Cannot read sifive,sdram-params %d\n",
> +                      __func__, ret);
> +               return ret;
> +       }
> +
> +       struct fu540_sdram_params *params = &plat->sdram_params;

Please move the variable declaration to the beginning of this function.

> +
> +       sdram_copy_to_reg(&priv->ctl->denali_ctl[0],
> +                         &params->pctl_regs.denali_ctl[0],
> +                         sizeof(struct fu540_ddrctl));
> +
> +       /* phy reset */
> +       for (i = DENALI_PHY_1152; i <= DENALI_PHY_1214; i++) {
> +               physet = params->phy_regs.denali_phy[i];
> +               priv->phy->denali_phy[i] = physet;
> +       }
> +
> +       for (i = 0; i < DENALI_PHY_1152; i++) {
> +               physet = params->phy_regs.denali_phy[i];
> +               priv->phy->denali_phy[i] = physet;
> +       }
> +
> +       /* Disable read interleave DENALI_CTL_120 */
> +       setbits_le32(DENALI_CTL_120 + denali_ctl,
> +                    1 << DISABLE_RD_INTERLEAVE_OFFSET);
> +
> +       /* Disable optimal read/modify/write logic DENALI_CTL_21 */
> +       clrbits_le32(DENALI_CTL_21 + denali_ctl, 1 << OPTIMAL_RMODW_EN_OFFSET);
> +
> +       /* Enable write Leveling DENALI_CTL_170 */
> +       setbits_le32(DENALI_CTL_170 + denali_ctl, (1 << WRLVL_EN_OFFSET)
> +                    | (1 << DFI_PHY_WRLELV_MODE_OFFSET));
> +
> +       /* Enable read leveling DENALI_CTL_181 and DENALI_CTL_260 */
> +       setbits_le32(DENALI_CTL_181 + denali_ctl,
> +                    1 << DFI_PHY_RDLVL_MODE_OFFSET);
> +       setbits_le32(DENALI_CTL_260 + denali_ctl, 1 << RDLVL_EN_OFFSET);
> +
> +       /* Enable read leveling gate DENALI_CTL_260 and DENALI_CTL_182 */
> +       setbits_le32(DENALI_CTL_260 + denali_ctl, 1 << RDLVL_GATE_EN_OFFSET);
> +       setbits_le32(DENALI_CTL_182 + denali_ctl,
> +                    1 << DFI_PHY_RDLVL_GATE_MODE_OFFSET);
> +
> +       if (ddr_getdramclass(denali_ctl) == DRAM_CLASS_DDR4) {
> +               /* Enable vref training DENALI_CTL_184 */
> +               setbits_le32(DENALI_CTL_184 + denali_ctl, 1 << VREF_EN_OFFSET);
> +       }
> +
> +       /* Mask off leveling completion interrupt DENALI_CTL_136 */
> +       setbits_le32(DENALI_CTL_136 + denali_ctl,
> +                    1 << LEVELING_OPERATION_COMPLETED_OFFSET);
> +
> +       /* Mask off MC init complete interrupt DENALI_CTL_136 */
> +       setbits_le32(DENALI_CTL_136 + denali_ctl, 1 << MC_INIT_COMPLETE_OFFSET);
> +
> +       /* Mask off out of range interrupts DENALI_CTL_136 */
> +       setbits_le32(DENALI_CTL_136 + denali_ctl, (1 << OUT_OF_RANGE_OFFSET)
> +                    | (1 << MULTIPLE_OUT_OF_RANGE_OFFSET));
> +
> +       /* set up range protection */
> +       ddr_setuprangeprotection(denali_ctl, DDR_MEM_SIZE);
> +
> +       /* Mask off port command error interrupt DENALI_CTL_136 */
> +       setbits_le32(DENALI_CTL_136 + denali_ctl,
> +                    1 << PORT_COMMAND_CHANNEL_ERROR_OFFSET);
> +
> +       const u64 ddr_size = DDR_MEM_SIZE;
> +       const u64 ddr_end = PAYLOAD_DEST + ddr_size;
> +

Please move the variable declaration to the beginning of this function.

> +       ddr_start(denali_ctl, priv->physical_filter_ctrl, ddr_end);
> +
> +       ddr_phy_fixup(denali_phy);
> +
> +       /* check size */
> +       priv->info.size = get_ram_size((long *)priv->info.base,
> +                                      DDR_MEM_SIZE);
> +
> +       printf("priv->info.size = %lx\n", priv->info.size);
> +       debug("%s : %lx\n", __func__, priv->info.size);
> +
> +       /* check memory access for all memory */
> +
> +       if (priv->info.size != DDR_MEM_SIZE) {
> +               printf("DDR invalid size : 0x%lx, expected 0x%lx\n",
> +                      priv->info.size, DDR_MEM_SIZE);
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +static int fu540_ddr_probe(struct udevice *dev)
> +{
> +       struct ddr_info *priv = dev_get_priv(dev);
> +
> +#if defined(CONFIG_SPL_BUILD)
> +       struct regmap *map;
> +       int ret;
> +
> +       debug("FU540 DDR probe\n");
> +       priv->dev = dev;
> +
> +       ret = regmap_init_mem(dev_ofnode(dev), &map);
> +       if (ret)
> +               return ret;
> +
> +       priv->ctl = regmap_get_range(map, 0);
> +       priv->phy = regmap_get_range(map, 1);
> +       priv->physical_filter_ctrl = regmap_get_range(map, 2);
> +
> +       priv->info.base = CONFIG_SYS_SDRAM_BASE;
> +
> +       priv->info.size = 0;
> +       return fu540_ddr_setup(dev);
> +#else
> +       priv->info.base = CONFIG_SYS_SDRAM_BASE;
> +       priv->info.size = DDR_MEM_SIZE;
> +#endif
> +       return 0;
> +}
> +
> +static int fu540_ddr_get_info(struct udevice *dev, struct ram_info *info)
> +{
> +       struct ddr_info *priv = dev_get_priv(dev);
> +
> +       *info = priv->info;
> +
> +       return 0;
> +}
> +
> +static struct ram_ops fu540_ddr_ops = {
> +       .get_info = fu540_ddr_get_info,
> +};
> +
> +static const struct udevice_id fu540_ddr_ids[] = {
> +       { .compatible = "sifive,fu540-ddr" },

"sifive,fu540-c000-ddr" ?

> +       { }
> +};
> +
> +U_BOOT_DRIVER(fu540_ddr) = {
> +       .name = "fu540_ddr",
> +       .id = UCLASS_RAM,
> +       .of_match = fu540_ddr_ids,
> +       .ops = &fu540_ddr_ops,
> +       .probe = fu540_ddr_probe,
> +       .priv_auto_alloc_size = sizeof(struct ddr_info),
> +       .platdata_auto_alloc_size = sizeof(struct sifive_dmc_plat),
> +};
> diff --git a/drivers/ram/sifive/sdram_fu540.h b/drivers/ram/sifive/sdram_fu540.h
> new file mode 100644
> index 0000000000..09693a038e
> --- /dev/null
> +++ b/drivers/ram/sifive/sdram_fu540.h
> @@ -0,0 +1,94 @@
> +/* SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause */
> +/*
> + * Copyright (C) 2020, SiFive, Inc
> + *
> + * Authors:
> + *   Pragnesh Patel <pragnesh.patel at sifive.com>
> + */
> +
> +#define DENALI_CTL_0   0
> +#define DENALI_CTL_21  21
> +#define DENALI_CTL_120 120
> +#define DENALI_CTL_132 132
> +#define DENALI_CTL_136 136
> +#define DENALI_CTL_170 170
> +#define DENALI_CTL_181 181
> +#define DENALI_CTL_182 182
> +#define DENALI_CTL_184 184
> +#define DENALI_CTL_208 208
> +#define DENALI_CTL_209 209
> +#define DENALI_CTL_210 210
> +#define DENALI_CTL_212 212
> +#define DENALI_CTL_214 214
> +#define DENALI_CTL_216 216
> +#define DENALI_CTL_224 224
> +#define DENALI_CTL_225 225
> +#define DENALI_CTL_260 260
> +
> +#define DENALI_PHY_1152        1152
> +#define DENALI_PHY_1214        1214
> +
> +#define PAYLOAD_DEST   0x80000000
> +#define DDR_MEM_SIZE   (8UL * 1024UL * 1024UL * 1024UL)
> +
> +#define DRAM_CLASS_OFFSET                   8
> +#define DRAM_CLASS_DDR4                     0xA
> +#define OPTIMAL_RMODW_EN_OFFSET             0
> +#define DISABLE_RD_INTERLEAVE_OFFSET        16
> +#define OUT_OF_RANGE_OFFSET                 1
> +#define MULTIPLE_OUT_OF_RANGE_OFFSET        2
> +#define PORT_COMMAND_CHANNEL_ERROR_OFFSET   7
> +#define MC_INIT_COMPLETE_OFFSET             8
> +#define LEVELING_OPERATION_COMPLETED_OFFSET 22
> +#define DFI_PHY_WRLELV_MODE_OFFSET          24
> +#define DFI_PHY_RDLVL_MODE_OFFSET           24
> +#define DFI_PHY_RDLVL_GATE_MODE_OFFSET      0
> +#define VREF_EN_OFFSET                      24
> +#define PORT_ADDR_PROTECTION_EN_OFFSET      0
> +#define AXI0_ADDRESS_RANGE_ENABLE           8
> +#define AXI0_RANGE_PROT_BITS_0_OFFSET       24
> +#define RDLVL_EN_OFFSET                     16
> +#define RDLVL_GATE_EN_OFFSET                24
> +#define WRLVL_EN_OFFSET                     0
> +
> +#define PHY_RX_CAL_DQ0_0_OFFSET             0
> +#define PHY_RX_CAL_DQ1_0_OFFSET             16
> +
> +struct fu540_ddrctl {
> +       volatile u32 denali_ctl[265];
> +};
> +
> +struct fu540_ddrphy {
> +       volatile u32 denali_phy[1215];
> +};
> +
> +struct fu540_sdram_params {
> +       struct fu540_ddrctl pctl_regs;
> +       struct fu540_ddrphy phy_regs;
> +};
> +
> +struct sifive_dmc_plat {
> +#if CONFIG_IS_ENABLED(OF_PLATDATA)
> +       struct dtd_sifive_fu540_dmc dtplat;

Do we use this for SPL?

> +#else
> +       struct fu540_sdram_params sdram_params;
> +#endif
> +};
> +
> +/**
> + * struct ddr_info
> + *
> + * @dev                                : pointer for the device
> + * @info                       : UCLASS RAM information
> + * @ctl                                : DDR controleur base address

typo: controller

> + * @phy                                : DDR PHY base address
> + * @ctrl                       : DDR control base address
> + * @physical_filter_ctrl       : DDR physical filter control base address
> + */
> +struct ddr_info {
> +       struct udevice *dev;
> +       struct ram_info info;
> +       struct fu540_ddrctl *ctl;
> +       struct fu540_ddrphy *phy;
> +       u32 *physical_filter_ctrl;
> +};
> --

Regards,
Bin
Giulio Benetti March 13, 2020, 11:56 a.m. UTC | #2 
Hi Pragnesh,

On 3/11/20 8:03 AM, Pragnesh Patel wrote:
> Add driver for fu540 to support ddr initialization in SPL.
> This driver is based on FSBL
> (https://github.com/sifive/freedom-u540-c000-bootloader.git)
> 
> Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com>
> ---
>   drivers/ram/Kconfig              |   7 +
>   drivers/ram/Makefile             |   2 +
>   drivers/ram/sifive/Kconfig       |   8 +
>   drivers/ram/sifive/Makefile      |   6 +
>   drivers/ram/sifive/sdram_fu540.c | 295 +++++++++++++++++++++++++++++++
>   drivers/ram/sifive/sdram_fu540.h |  94 ++++++++++
>   6 files changed, 412 insertions(+)
>   create mode 100644 drivers/ram/sifive/Kconfig
>   create mode 100644 drivers/ram/sifive/Makefile
>   create mode 100644 drivers/ram/sifive/sdram_fu540.c
>   create mode 100644 drivers/ram/sifive/sdram_fu540.h
> 
> diff --git a/drivers/ram/Kconfig b/drivers/ram/Kconfig
> index 56fea7c94c..c60c63204c 100644
> --- a/drivers/ram/Kconfig
> +++ b/drivers/ram/Kconfig
> @@ -73,5 +73,12 @@ config IMXRT_SDRAM
>   	  to support external memories like sdram, psram & nand.
>   	  This driver is for the sdram memory interface with the SEMC.
>   
> +config SIFIVE_DDR
> +	bool "Enable SiFive DDR support"
> +	depends on RAM
> +	help
> +	  Enable support for the internal DDR Memory Controller of SiFive SoCs.
> +
>   source "drivers/ram/rockchip/Kconfig"
>   source "drivers/ram/stm32mp1/Kconfig"
> +source "drivers/ram/sifive/Kconfig"
> diff --git a/drivers/ram/Makefile b/drivers/ram/Makefile
> index 5c897410c6..12bf61510b 100644
> --- a/drivers/ram/Makefile
> +++ b/drivers/ram/Makefile
> @@ -17,3 +17,5 @@ obj-$(CONFIG_ARCH_MEDIATEK) += mediatek/
>   obj-$(CONFIG_K3_J721E_DDRSS) += k3-j721e/
>   
>   obj-$(CONFIG_IMXRT_SDRAM) += imxrt_sdram.o
> +
> +obj-$(CONFIG_SIFIVE_DDR) += sifive/
> diff --git a/drivers/ram/sifive/Kconfig b/drivers/ram/sifive/Kconfig
> new file mode 100644
> index 0000000000..b754700db8
> --- /dev/null
> +++ b/drivers/ram/sifive/Kconfig
> @@ -0,0 +1,8 @@
> +config SIFIVE_FU540_DDR
> +	bool "SiFive FU540 DDR driver"
> +	depends on DM && OF_CONTROL
> +	select RAM
> +	select SPL_RAM if SPL
> +	select SIFIVE_DDR
> +	help
> +	  This enables DDR support for the platforms based on SiFive FU540 SoC.
> diff --git a/drivers/ram/sifive/Makefile b/drivers/ram/sifive/Makefile
> new file mode 100644
> index 0000000000..0187805199
> --- /dev/null
> +++ b/drivers/ram/sifive/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright (c) 2020 SiFive, Inc
> +#
> +
> +obj-$(CONFIG_SIFIVE_FU540_DDR) += sdram_fu540.o
> diff --git a/drivers/ram/sifive/sdram_fu540.c b/drivers/ram/sifive/sdram_fu540.c
> new file mode 100644
> index 0000000000..18926dbe15
> --- /dev/null
> +++ b/drivers/ram/sifive/sdram_fu540.c
> @@ -0,0 +1,295 @@
> +// SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause
> +/*
> + * (C) Copyright 2020 SiFive, Inc.
> + *
> + * Authors:
> + *   Pragnesh Patel <pragnesh.patel at sifive.com>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <init.h>
> +#include <ram.h>
> +#include <regmap.h>
> +#include <syscon.h>
> +#include <asm/io.h>
> +#include "sdram_fu540.h"
> +
> +/* n: Unit bytes */
> +void sdram_copy_to_reg(volatile u32 *dest, volatile u32 *src, u32 n)
> +{
> +	int i;
> +
> +	for (i = 0; i < n / sizeof(u32); i++) {
> +		writel(*src, dest);
> +		src++;
> +		dest++;
> +	}
> +}
> +
> +static void ddr_setuprangeprotection(volatile u32 *ctl, u64 end_addr)
> +{
> +	writel(0x0, DENALI_CTL_209 + ctl);
> +	u32 end_addr_16kblocks = ((end_addr >> 14) & 0x7FFFFF) - 1;
> +
> +	writel(end_addr_16kblocks, DENALI_CTL_210 + ctl);
> +	writel(0x0, DENALI_CTL_212 + ctl);
> +	writel(0x0, DENALI_CTL_214 + ctl);
> +	writel(0x0, DENALI_CTL_216 + ctl);
> +	setbits_le32(DENALI_CTL_224 + ctl,
> +		     0x3 << AXI0_RANGE_PROT_BITS_0_OFFSET);
> +	writel(0xFFFFFFFF, DENALI_CTL_225 + ctl);
> +	setbits_le32(DENALI_CTL_208 + ctl, 0x1 << AXI0_ADDRESS_RANGE_ENABLE);
> +	setbits_le32(DENALI_CTL_208 + ctl,
> +		     0x1 << PORT_ADDR_PROTECTION_EN_OFFSET);
> +}
> +
> +static void ddr_start(volatile u32 *ctl, u32 *physical_filter_ctrl, u64 ddr_end)
> +{
> +	setbits_le32(DENALI_CTL_0 + ctl, 0x1);
> +	while ((readl(DENALI_CTL_132 + ctl) & (1 << MC_INIT_COMPLETE_OFFSET))
> +	       == 0) {
> +	}
> +
> +	// Disable the BusBlocker in front of the controller AXI slave ports
> +	volatile u64 *filterreg = (volatile u64 *)physical_filter_ctrl;
> +
> +	filterreg[0] = 0x0f00000000000000UL | (ddr_end >> 2);
> +}
> +
> +static u64 ddr_phy_fixup(volatile u32 *ddrphyreg)
> +{
> +	// return bitmask of failed lanes
> +
> +	u64 fails     = 0;
> +	u32 slicebase = 0;
> +	u32 dq        = 0;
> +
> +	// check errata condition
> +	for (u32 slice = 0; slice < 8; slice++) {
> +		u32 regbase = slicebase + 34;
> +
> +		for (u32 reg = 0; reg < 4; reg++) {
> +			u32 updownreg = readl(regbase + reg + ddrphyreg);
> +
> +			for (u32 bit = 0; bit < 2; bit++) {
> +				u32 phy_rx_cal_dqn_0_offset;
> +
> +				if (bit == 0) {
> +					phy_rx_cal_dqn_0_offset =
> +						PHY_RX_CAL_DQ0_0_OFFSET;
> +				} else {
> +					phy_rx_cal_dqn_0_offset =
> +						PHY_RX_CAL_DQ1_0_OFFSET;
> +				}
> +
> +				u32 down = (updownreg >>
> +					    phy_rx_cal_dqn_0_offset) & 0x3F;
> +				u32 up   = (updownreg >>
> +					    (phy_rx_cal_dqn_0_offset + 6)) &
> +					    0x3F;
> +
> +				u8 failc0 = ((down == 0) && (up == 0x3F));
> +				u8 failc1 = ((up == 0) && (down == 0x3F));
> +
> +				// print error message on failure
> +				if (failc0 || failc1) {
> +					if (fails == 0)
> +						printf("DDR error in fixing up\n");
> +
> +					fails |= (1 << dq);
> +
> +					char slicelsc = '0';
> +					char slicemsc = '0';
> +
> +					slicelsc += (dq % 10);
> +					slicemsc += (dq / 10);
> +					printf("S ");
> +					printf("%c", slicemsc);
> +					printf("%c", slicelsc);
> +
> +					if (failc0)
> +						printf("U");
> +					else
> +						printf("D");
> +
> +					printf("\n");
> +				}

Here you've nested 5 times. Can you try to break up all these 
iterations? At least moving some code to external static functions. It 
is difficult to read these 3 nested for cycles plus other 2 nested if 
conditions. And for sure keep declare variables at the top.

> +				dq++;
> +			}
> +		}
> +		slicebase += 128;
> +	}
> +	return(0);
> +}
> +
> +static u32 ddr_getdramclass(volatile u32 *ctl)
> +{
> +	u32 reg = readl(DENALI_CTL_0 + ctl);
> +
> +	return ((reg >> DRAM_CLASS_OFFSET) & 0xF);
> +}
> +
> +static __maybe_unused int fu540_ddr_setup(struct udevice *dev)
> +{
> +	struct ddr_info *priv = dev_get_priv(dev);
> +	struct sifive_dmc_plat *plat = dev_get_platdata(dev);
> +
> +	int ret, i;
> +	u32 physet;
> +
> +	volatile u32 *denali_ctl =  &priv->ctl->denali_ctl[0];
> +	volatile u32 *denali_phy =  &priv->phy->denali_phy[0];
> +
> +	ret = dev_read_u32_array(dev, "sifive,sdram-params",
> +				 (u32 *)&plat->sdram_params,
> +				 sizeof(plat->sdram_params) / sizeof(u32));
> +	if (ret) {
> +		printf("%s: Cannot read sifive,sdram-params %d\n",
> +		       __func__, ret);
> +		return ret;
> +	}
> +
> +	struct fu540_sdram_params *params = &plat->sdram_params;
> +
> +	sdram_copy_to_reg(&priv->ctl->denali_ctl[0],
> +			  &params->pctl_regs.denali_ctl[0],
> +			  sizeof(struct fu540_ddrctl));
> +
> +	/* phy reset */
> +	for (i = DENALI_PHY_1152; i <= DENALI_PHY_1214; i++) {
> +		physet = params->phy_regs.denali_phy[i];
> +		priv->phy->denali_phy[i] = physet;
> +	}
> +
> +	for (i = 0; i < DENALI_PHY_1152; i++) {
> +		physet = params->phy_regs.denali_phy[i];
> +		priv->phy->denali_phy[i] = physet;
> +	}
> +
> +	/* Disable read interleave DENALI_CTL_120 */
> +	setbits_le32(DENALI_CTL_120 + denali_ctl,
> +		     1 << DISABLE_RD_INTERLEAVE_OFFSET);
> +
> +	/* Disable optimal read/modify/write logic DENALI_CTL_21 */
> +	clrbits_le32(DENALI_CTL_21 + denali_ctl, 1 << OPTIMAL_RMODW_EN_OFFSET);
> +
> +	/* Enable write Leveling DENALI_CTL_170 */
> +	setbits_le32(DENALI_CTL_170 + denali_ctl, (1 << WRLVL_EN_OFFSET)
> +		     | (1 << DFI_PHY_WRLELV_MODE_OFFSET));
> +
> +	/* Enable read leveling DENALI_CTL_181 and DENALI_CTL_260 */
> +	setbits_le32(DENALI_CTL_181 + denali_ctl,
> +		     1 << DFI_PHY_RDLVL_MODE_OFFSET);
> +	setbits_le32(DENALI_CTL_260 + denali_ctl, 1 << RDLVL_EN_OFFSET);
> +
> +	/* Enable read leveling gate DENALI_CTL_260 and DENALI_CTL_182 */
> +	setbits_le32(DENALI_CTL_260 + denali_ctl, 1 << RDLVL_GATE_EN_OFFSET);
> +	setbits_le32(DENALI_CTL_182 + denali_ctl,
> +		     1 << DFI_PHY_RDLVL_GATE_MODE_OFFSET);
> +
> +	if (ddr_getdramclass(denali_ctl) == DRAM_CLASS_DDR4) {
> +		/* Enable vref training DENALI_CTL_184 */
> +		setbits_le32(DENALI_CTL_184 + denali_ctl, 1 << VREF_EN_OFFSET);
> +	}
> +
> +	/* Mask off leveling completion interrupt DENALI_CTL_136 */
> +	setbits_le32(DENALI_CTL_136 + denali_ctl,
> +		     1 << LEVELING_OPERATION_COMPLETED_OFFSET);
> +
> +	/* Mask off MC init complete interrupt DENALI_CTL_136 */
> +	setbits_le32(DENALI_CTL_136 + denali_ctl, 1 << MC_INIT_COMPLETE_OFFSET);
> +
> +	/* Mask off out of range interrupts DENALI_CTL_136 */
> +	setbits_le32(DENALI_CTL_136 + denali_ctl, (1 << OUT_OF_RANGE_OFFSET)
> +		     | (1 << MULTIPLE_OUT_OF_RANGE_OFFSET));
> +
> +	/* set up range protection */
> +	ddr_setuprangeprotection(denali_ctl, DDR_MEM_SIZE);
> +
> +	/* Mask off port command error interrupt DENALI_CTL_136 */
> +	setbits_le32(DENALI_CTL_136 + denali_ctl,
> +		     1 << PORT_COMMAND_CHANNEL_ERROR_OFFSET);
> +
> +	const u64 ddr_size = DDR_MEM_SIZE;
> +	const u64 ddr_end = PAYLOAD_DEST + ddr_size;
> +
> +	ddr_start(denali_ctl, priv->physical_filter_ctrl, ddr_end);
> +
> +	ddr_phy_fixup(denali_phy);
> +
> +	/* check size */
> +	priv->info.size = get_ram_size((long *)priv->info.base,
> +				       DDR_MEM_SIZE);
> +
> +	printf("priv->info.size = %lx\n", priv->info.size);
> +	debug("%s : %lx\n", __func__, priv->info.size);
> +
> +	/* check memory access for all memory */
> +
> +	if (priv->info.size != DDR_MEM_SIZE) {
> +		printf("DDR invalid size : 0x%lx, expected 0x%lx\n",
> +		       priv->info.size, DDR_MEM_SIZE);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int fu540_ddr_probe(struct udevice *dev)
> +{
> +	struct ddr_info *priv = dev_get_priv(dev);
> +
> +#if defined(CONFIG_SPL_BUILD)
> +	struct regmap *map;
> +	int ret;
> +
> +	debug("FU540 DDR probe\n");
> +	priv->dev = dev;
> +
> +	ret = regmap_init_mem(dev_ofnode(dev), &map);
> +	if (ret)
> +		return ret;
> +
> +	priv->ctl = regmap_get_range(map, 0);
> +	priv->phy = regmap_get_range(map, 1);
> +	priv->physical_filter_ctrl = regmap_get_range(map, 2);
> +
> +	priv->info.base = CONFIG_SYS_SDRAM_BASE;
> +
> +	priv->info.size = 0;
> +	return fu540_ddr_setup(dev);
> +#else
> +	priv->info.base = CONFIG_SYS_SDRAM_BASE;
> +	priv->info.size = DDR_MEM_SIZE;
> +#endif
> +	return 0;
> +}
> +
> +static int fu540_ddr_get_info(struct udevice *dev, struct ram_info *info)
> +{
> +	struct ddr_info *priv = dev_get_priv(dev);
> +
> +	*info = priv->info;
> +
> +	return 0;
> +}
> +
> +static struct ram_ops fu540_ddr_ops = {
> +	.get_info = fu540_ddr_get_info,
> +};
> +
> +static const struct udevice_id fu540_ddr_ids[] = {
> +	{ .compatible = "sifive,fu540-ddr" },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(fu540_ddr) = {
> +	.name = "fu540_ddr",
> +	.id = UCLASS_RAM,
> +	.of_match = fu540_ddr_ids,
> +	.ops = &fu540_ddr_ops,
> +	.probe = fu540_ddr_probe,
> +	.priv_auto_alloc_size = sizeof(struct ddr_info),
> +	.platdata_auto_alloc_size = sizeof(struct sifive_dmc_plat),
> +};
> diff --git a/drivers/ram/sifive/sdram_fu540.h b/drivers/ram/sifive/sdram_fu540.h
> new file mode 100644
> index 0000000000..09693a038e
> --- /dev/null
> +++ b/drivers/ram/sifive/sdram_fu540.h
> @@ -0,0 +1,94 @@
> +/* SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause */
> +/*
> + * Copyright (C) 2020, SiFive, Inc
> + *
> + * Authors:
> + *   Pragnesh Patel <pragnesh.patel at sifive.com>
> + */
> +
> +#define DENALI_CTL_0	0
> +#define DENALI_CTL_21	21
> +#define DENALI_CTL_120	120
> +#define DENALI_CTL_132	132
> +#define DENALI_CTL_136	136
> +#define DENALI_CTL_170	170
> +#define DENALI_CTL_181	181
> +#define DENALI_CTL_182	182
> +#define DENALI_CTL_184	184
> +#define DENALI_CTL_208	208
> +#define DENALI_CTL_209	209
> +#define DENALI_CTL_210	210
> +#define DENALI_CTL_212	212
> +#define DENALI_CTL_214	214
> +#define DENALI_CTL_216	216
> +#define DENALI_CTL_224	224
> +#define DENALI_CTL_225	225
> +#define DENALI_CTL_260	260
> +
> +#define DENALI_PHY_1152	1152
> +#define DENALI_PHY_1214	1214
> +
> +#define PAYLOAD_DEST	0x80000000
> +#define DDR_MEM_SIZE	(8UL * 1024UL * 1024UL * 1024UL)
> +
> +#define DRAM_CLASS_OFFSET                   8
> +#define DRAM_CLASS_DDR4                     0xA
> +#define OPTIMAL_RMODW_EN_OFFSET             0
> +#define DISABLE_RD_INTERLEAVE_OFFSET        16
> +#define OUT_OF_RANGE_OFFSET                 1
> +#define MULTIPLE_OUT_OF_RANGE_OFFSET        2
> +#define PORT_COMMAND_CHANNEL_ERROR_OFFSET   7
> +#define MC_INIT_COMPLETE_OFFSET             8
> +#define LEVELING_OPERATION_COMPLETED_OFFSET 22
> +#define DFI_PHY_WRLELV_MODE_OFFSET          24
> +#define DFI_PHY_RDLVL_MODE_OFFSET           24
> +#define DFI_PHY_RDLVL_GATE_MODE_OFFSET      0
> +#define VREF_EN_OFFSET                      24
> +#define PORT_ADDR_PROTECTION_EN_OFFSET      0
> +#define AXI0_ADDRESS_RANGE_ENABLE           8
> +#define AXI0_RANGE_PROT_BITS_0_OFFSET       24
> +#define RDLVL_EN_OFFSET                     16
> +#define RDLVL_GATE_EN_OFFSET                24
> +#define WRLVL_EN_OFFSET                     0
> +
> +#define PHY_RX_CAL_DQ0_0_OFFSET             0
> +#define PHY_RX_CAL_DQ1_0_OFFSET             16
> +
> +struct fu540_ddrctl {
> +	volatile u32 denali_ctl[265];
> +};
> +
> +struct fu540_ddrphy {
> +	volatile u32 denali_phy[1215];
> +};
> +
> +struct fu540_sdram_params {
> +	struct fu540_ddrctl pctl_regs;
> +	struct fu540_ddrphy phy_regs;
> +};
> +
> +struct sifive_dmc_plat {
> +#if CONFIG_IS_ENABLED(OF_PLATDATA)
> +	struct dtd_sifive_fu540_dmc dtplat;
> +#else
> +	struct fu540_sdram_params sdram_params;
> +#endif
> +};
> +
> +/**
> + * struct ddr_info
> + *
> + * @dev				: pointer for the device
> + * @info			: UCLASS RAM information
> + * @ctl				: DDR controleur base address
> + * @phy				: DDR PHY base address
> + * @ctrl			: DDR control base address
> + * @physical_filter_ctrl	: DDR physical filter control base address
> + */
> +struct ddr_info {
> +	struct udevice *dev;
> +	struct ram_info info;
> +	struct fu540_ddrctl *ctl;
> +	struct fu540_ddrphy *phy;
> +	u32 *physical_filter_ctrl;
> +};
> 

IMHO I would keep all these structs and define in driver's .c file, I 
mean you should keep them private if not used outside. And then add to 
dt-bindings the define needed by dts files.

Best regards
Pragnesh Patel March 17, 2020, 1 p.m. UTC | #3 
Hi Bin,

>-----Original Message-----
>From: Bin Meng <bmeng.cn at gmail.com>
>Sent: 13 March 2020 13:18
>To: Pragnesh Patel <pragnesh.patel at sifive.com>
>Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Atish Patra
><atish.patra at wdc.com>; Palmer Dabbelt <palmerdabbelt at google.com>; Paul
>Walmsley <paul.walmsley at sifive.com>; Jagan Teki
><jagan at amarulasolutions.com>; Troy Benjegerdes
><troy.benjegerdes at sifive.com>; Anup Patel <anup.patel at wdc.com>; Sagar
>Kadam <sagar.kadam at sifive.com>; Rick Chen <rick at andestech.com>; Kever
>Yang <kever.yang at rock-chips.com>; Giulio Benetti
><giulio.benetti at benettiengineering.com>; Lokesh Vutla
><lokeshvutla at ti.com>; Kevin Scholz <k-scholz at ti.com>; YouMin Chen
><cym at rock-chips.com>
>Subject: Re: [PATCH v5 06/14] sifive: fu540: add ddr driver
>
>On Wed, Mar 11, 2020 at 3:04 PM Pragnesh Patel
><pragnesh.patel at sifive.com> wrote:
>>
>> Add driver for fu540 to support ddr initialization in SPL.
>> This driver is based on FSBL
>> (https://github.com/sifive/freedom-u540-c000-bootloader.git)
>>
>> Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com>
>> ---
>>  drivers/ram/Kconfig              |   7 +
>>  drivers/ram/Makefile             |   2 +
>>  drivers/ram/sifive/Kconfig       |   8 +
>>  drivers/ram/sifive/Makefile      |   6 +
>>  drivers/ram/sifive/sdram_fu540.c | 295
>> +++++++++++++++++++++++++++++++  drivers/ram/sifive/sdram_fu540.h |
>> 94 ++++++++++
>>  6 files changed, 412 insertions(+)
>>  create mode 100644 drivers/ram/sifive/Kconfig  create mode 100644
>> drivers/ram/sifive/Makefile  create mode 100644
>> drivers/ram/sifive/sdram_fu540.c  create mode 100644
>> drivers/ram/sifive/sdram_fu540.h
>>
>> diff --git a/drivers/ram/Kconfig b/drivers/ram/Kconfig index
>> 56fea7c94c..c60c63204c 100644
>> --- a/drivers/ram/Kconfig
>> +++ b/drivers/ram/Kconfig
>> @@ -73,5 +73,12 @@ config IMXRT_SDRAM
>>           to support external memories like sdram, psram & nand.
>>           This driver is for the sdram memory interface with the SEMC.
>>
>> +config SIFIVE_DDR
>> +       bool "Enable SiFive DDR support"
>> +       depends on RAM
>> +       help
>> +         Enable support for the internal DDR Memory Controller of SiFive SoCs.
>> +
>>  source "drivers/ram/rockchip/Kconfig"
>>  source "drivers/ram/stm32mp1/Kconfig"
>> +source "drivers/ram/sifive/Kconfig"
>> diff --git a/drivers/ram/Makefile b/drivers/ram/Makefile index
>> 5c897410c6..12bf61510b 100644
>> --- a/drivers/ram/Makefile
>> +++ b/drivers/ram/Makefile
>> @@ -17,3 +17,5 @@ obj-$(CONFIG_ARCH_MEDIATEK) += mediatek/
>>  obj-$(CONFIG_K3_J721E_DDRSS) += k3-j721e/
>>
>>  obj-$(CONFIG_IMXRT_SDRAM) += imxrt_sdram.o
>> +
>> +obj-$(CONFIG_SIFIVE_DDR) += sifive/
>> diff --git a/drivers/ram/sifive/Kconfig b/drivers/ram/sifive/Kconfig
>> new file mode 100644 index 0000000000..b754700db8
>> --- /dev/null
>> +++ b/drivers/ram/sifive/Kconfig
>> @@ -0,0 +1,8 @@
>> +config SIFIVE_FU540_DDR
>> +       bool "SiFive FU540 DDR driver"
>> +       depends on DM && OF_CONTROL
>> +       select RAM
>
>Do we need this driver for U-Boot proper?

SPL_RAM is depend on RAM that's why select RAM here.

>
>> +       select SPL_RAM if SPL
>> +       select SIFIVE_DDR
>> +       help
>> +         This enables DDR support for the platforms based on SiFive FU540
>SoC.
>> diff --git a/drivers/ram/sifive/Makefile b/drivers/ram/sifive/Makefile
>> new file mode 100644 index 0000000000..0187805199
>> --- /dev/null
>> +++ b/drivers/ram/sifive/Makefile
>> @@ -0,0 +1,6 @@
>> +# SPDX-License-Identifier: GPL-2.0+
>> +#
>> +# Copyright (c) 2020 SiFive, Inc
>> +#
>> +
>> +obj-$(CONFIG_SIFIVE_FU540_DDR) += sdram_fu540.o
>> diff --git a/drivers/ram/sifive/sdram_fu540.c
>> b/drivers/ram/sifive/sdram_fu540.c
>> new file mode 100644
>> index 0000000000..18926dbe15
>> --- /dev/null
>> +++ b/drivers/ram/sifive/sdram_fu540.c
>> @@ -0,0 +1,295 @@
>> +// SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause
>> +/*
>> + * (C) Copyright 2020 SiFive, Inc.
>> + *
>> + * Authors:
>> + *   Pragnesh Patel <pragnesh.patel at sifive.com>
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <init.h>
>> +#include <ram.h>
>> +#include <regmap.h>
>> +#include <syscon.h>
>> +#include <asm/io.h>
>> +#include "sdram_fu540.h"
>> +
>> +/* n: Unit bytes */
>> +void sdram_copy_to_reg(volatile u32 *dest, volatile u32 *src, u32 n)
>
>This should be static, also use "int n"?

I agree with static but "n" should be u32 because it tells how many bytes you want to copy.

>
>> +{
>> +       int i;
>> +
>> +       for (i = 0; i < n / sizeof(u32); i++) {
>> +               writel(*src, dest);
>> +               src++;
>> +               dest++;
>> +       }
>> +}
>> +
>> +static void ddr_setuprangeprotection(volatile u32 *ctl, u64 end_addr)
>
>__maybe_unused ? Otherwise use #ifdef #endif ?

Will use it under " #if defined(CONFIG_SPL_BUILD)" and update in v6. 
 
>
>
>> +{
>> +       writel(0x0, DENALI_CTL_209 + ctl);
>> +       u32 end_addr_16kblocks = ((end_addr >> 14) & 0x7FFFFF) - 1;
>
>nits: move the variable declaration

Will do.

>
>> +
>> +       writel(end_addr_16kblocks, DENALI_CTL_210 + ctl);
>> +       writel(0x0, DENALI_CTL_212 + ctl);
>> +       writel(0x0, DENALI_CTL_214 + ctl);
>> +       writel(0x0, DENALI_CTL_216 + ctl);
>> +       setbits_le32(DENALI_CTL_224 + ctl,
>> +                    0x3 << AXI0_RANGE_PROT_BITS_0_OFFSET);
>> +       writel(0xFFFFFFFF, DENALI_CTL_225 + ctl);
>> +       setbits_le32(DENALI_CTL_208 + ctl, 0x1 <<
>AXI0_ADDRESS_RANGE_ENABLE);
>> +       setbits_le32(DENALI_CTL_208 + ctl,
>> +                    0x1 << PORT_ADDR_PROTECTION_EN_OFFSET); }
>> +
>> +static void ddr_start(volatile u32 *ctl, u32 *physical_filter_ctrl,
>> +u64 ddr_end)
>
>__maybe_unused ?

Will use it under " #if defined(CONFIG_SPL_BUILD)" and update in v6.

>
>> +{
>> +       setbits_le32(DENALI_CTL_0 + ctl, 0x1);
>> +       while ((readl(DENALI_CTL_132 + ctl) & (1 <<
>MC_INIT_COMPLETE_OFFSET))
>> +              == 0) {
>> +       }
>> +
>
>Use
>
>do {
>    val = readl(DENALI_CTL_132 + ctl) & (1 << MC_INIT_COMPLETE_OFFSET); }
>while (val == 0)

Thanks for the suggestion, will update in v6.

>
>?
>
>
>> +       // Disable the BusBlocker in front of the controller AXI slave
>> + ports
>
>nits: use /* */

Will do.

>
>> +       volatile u64 *filterreg = (volatile u64
>> + *)physical_filter_ctrl;
>
>Please move the variable declaration to the beginning of this function

Will do.

>
>> +
>> +       filterreg[0] = 0x0f00000000000000UL | (ddr_end >> 2); }
>> +
>> +static u64 ddr_phy_fixup(volatile u32 *ddrphyreg)
>
>__maybe_unused ?

Will use it under " #if defined(CONFIG_SPL_BUILD)" and update in v6.

>
>> +{
>> +       // return bitmask of failed lanes
>
>nits: use /* */

Will do.

>
>> +
>> +       u64 fails     = 0;
>> +       u32 slicebase = 0;
>> +       u32 dq        = 0;
>> +
>> +       // check errata condition
>
>nits: use /* */

Will do.

>
>> +       for (u32 slice = 0; slice < 8; slice++) {
>> +               u32 regbase = slicebase + 34;
>> +
>> +               for (u32 reg = 0; reg < 4; reg++) {
>> +                       u32 updownreg = readl(regbase + reg +
>> + ddrphyreg);
>> +
>> +                       for (u32 bit = 0; bit < 2; bit++) {
>> +                               u32 phy_rx_cal_dqn_0_offset;
>> +
>> +                               if (bit == 0) {
>> +                                       phy_rx_cal_dqn_0_offset =
>> +                                               PHY_RX_CAL_DQ0_0_OFFSET;
>> +                               } else {
>> +                                       phy_rx_cal_dqn_0_offset =
>> +                                               PHY_RX_CAL_DQ1_0_OFFSET;
>> +                               }
>> +
>> +                               u32 down = (updownreg >>
>> +                                           phy_rx_cal_dqn_0_offset) & 0x3F;
>> +                               u32 up   = (updownreg >>
>> +                                           (phy_rx_cal_dqn_0_offset + 6)) &
>> +                                           0x3F;
>> +
>> +                               u8 failc0 = ((down == 0) && (up == 0x3F));
>> +                               u8 failc1 = ((up == 0) && (down ==
>> + 0x3F));
>> +
>> +                               // print error message on failure
>
>nits: use /* */

Will do.

>
>> +                               if (failc0 || failc1) {
>> +                                       if (fails == 0)
>> +                                               printf("DDR error in
>> + fixing up\n");
>> +
>> +                                       fails |= (1 << dq);
>> +
>> +                                       char slicelsc = '0';
>> +                                       char slicemsc = '0';
>> +
>> +                                       slicelsc += (dq % 10);
>> +                                       slicemsc += (dq / 10);
>> +                                       printf("S ");
>> +                                       printf("%c", slicemsc);
>> +                                       printf("%c", slicelsc);
>> +
>> +                                       if (failc0)
>> +                                               printf("U");
>> +                                       else
>> +                                               printf("D");
>> +
>> +                                       printf("\n");
>> +                               }
>> +                               dq++;
>> +                       }
>> +               }
>> +               slicebase += 128;
>> +       }
>> +       return(0);
>> +}
>> +
>> +static u32 ddr_getdramclass(volatile u32 *ctl)
>
>__maybe_unused ?

Sure, will use __ maybe_unused here.

>
>> +{
>> +       u32 reg = readl(DENALI_CTL_0 + ctl);
>> +
>> +       return ((reg >> DRAM_CLASS_OFFSET) & 0xF); }
>> +
>> +static __maybe_unused int fu540_ddr_setup(struct udevice *dev) {
>> +       struct ddr_info *priv = dev_get_priv(dev);
>> +       struct sifive_dmc_plat *plat = dev_get_platdata(dev);
>> +
>> +       int ret, i;
>> +       u32 physet;
>> +
>> +       volatile u32 *denali_ctl =  &priv->ctl->denali_ctl[0];
>> +       volatile u32 *denali_phy =  &priv->phy->denali_phy[0];
>> +
>> +       ret = dev_read_u32_array(dev, "sifive,sdram-params",
>> +                                (u32 *)&plat->sdram_params,
>> +                                sizeof(plat->sdram_params) / sizeof(u32));
>> +       if (ret) {
>> +               printf("%s: Cannot read sifive,sdram-params %d\n",
>> +                      __func__, ret);
>> +               return ret;
>> +       }
>> +
>> +       struct fu540_sdram_params *params = &plat->sdram_params;
>
>Please move the variable declaration to the beginning of this function.

Will do.

>
>> +
>> +       sdram_copy_to_reg(&priv->ctl->denali_ctl[0],
>> +                         &params->pctl_regs.denali_ctl[0],
>> +                         sizeof(struct fu540_ddrctl));
>> +
>> +       /* phy reset */
>> +       for (i = DENALI_PHY_1152; i <= DENALI_PHY_1214; i++) {
>> +               physet = params->phy_regs.denali_phy[i];
>> +               priv->phy->denali_phy[i] = physet;
>> +       }
>> +
>> +       for (i = 0; i < DENALI_PHY_1152; i++) {
>> +               physet = params->phy_regs.denali_phy[i];
>> +               priv->phy->denali_phy[i] = physet;
>> +       }
>> +
>> +       /* Disable read interleave DENALI_CTL_120 */
>> +       setbits_le32(DENALI_CTL_120 + denali_ctl,
>> +                    1 << DISABLE_RD_INTERLEAVE_OFFSET);
>> +
>> +       /* Disable optimal read/modify/write logic DENALI_CTL_21 */
>> +       clrbits_le32(DENALI_CTL_21 + denali_ctl, 1 <<
>> + OPTIMAL_RMODW_EN_OFFSET);
>> +
>> +       /* Enable write Leveling DENALI_CTL_170 */
>> +       setbits_le32(DENALI_CTL_170 + denali_ctl, (1 << WRLVL_EN_OFFSET)
>> +                    | (1 << DFI_PHY_WRLELV_MODE_OFFSET));
>> +
>> +       /* Enable read leveling DENALI_CTL_181 and DENALI_CTL_260 */
>> +       setbits_le32(DENALI_CTL_181 + denali_ctl,
>> +                    1 << DFI_PHY_RDLVL_MODE_OFFSET);
>> +       setbits_le32(DENALI_CTL_260 + denali_ctl, 1 <<
>> + RDLVL_EN_OFFSET);
>> +
>> +       /* Enable read leveling gate DENALI_CTL_260 and DENALI_CTL_182 */
>> +       setbits_le32(DENALI_CTL_260 + denali_ctl, 1 <<
>RDLVL_GATE_EN_OFFSET);
>> +       setbits_le32(DENALI_CTL_182 + denali_ctl,
>> +                    1 << DFI_PHY_RDLVL_GATE_MODE_OFFSET);
>> +
>> +       if (ddr_getdramclass(denali_ctl) == DRAM_CLASS_DDR4) {
>> +               /* Enable vref training DENALI_CTL_184 */
>> +               setbits_le32(DENALI_CTL_184 + denali_ctl, 1 << VREF_EN_OFFSET);
>> +       }
>> +
>> +       /* Mask off leveling completion interrupt DENALI_CTL_136 */
>> +       setbits_le32(DENALI_CTL_136 + denali_ctl,
>> +                    1 << LEVELING_OPERATION_COMPLETED_OFFSET);
>> +
>> +       /* Mask off MC init complete interrupt DENALI_CTL_136 */
>> +       setbits_le32(DENALI_CTL_136 + denali_ctl, 1 <<
>> + MC_INIT_COMPLETE_OFFSET);
>> +
>> +       /* Mask off out of range interrupts DENALI_CTL_136 */
>> +       setbits_le32(DENALI_CTL_136 + denali_ctl, (1 <<
>OUT_OF_RANGE_OFFSET)
>> +                    | (1 << MULTIPLE_OUT_OF_RANGE_OFFSET));
>> +
>> +       /* set up range protection */
>> +       ddr_setuprangeprotection(denali_ctl, DDR_MEM_SIZE);
>> +
>> +       /* Mask off port command error interrupt DENALI_CTL_136 */
>> +       setbits_le32(DENALI_CTL_136 + denali_ctl,
>> +                    1 << PORT_COMMAND_CHANNEL_ERROR_OFFSET);
>> +
>> +       const u64 ddr_size = DDR_MEM_SIZE;
>> +       const u64 ddr_end = PAYLOAD_DEST + ddr_size;
>> +
>
>Please move the variable declaration to the beginning of this function.

Will do.

>
>> +       ddr_start(denali_ctl, priv->physical_filter_ctrl, ddr_end);
>> +
>> +       ddr_phy_fixup(denali_phy);
>> +
>> +       /* check size */
>> +       priv->info.size = get_ram_size((long *)priv->info.base,
>> +                                      DDR_MEM_SIZE);
>> +
>> +       printf("priv->info.size = %lx\n", priv->info.size);
>> +       debug("%s : %lx\n", __func__, priv->info.size);
>> +
>> +       /* check memory access for all memory */
>> +
>> +       if (priv->info.size != DDR_MEM_SIZE) {
>> +               printf("DDR invalid size : 0x%lx, expected 0x%lx\n",
>> +                      priv->info.size, DDR_MEM_SIZE);
>> +               return -EINVAL;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int fu540_ddr_probe(struct udevice *dev) {
>> +       struct ddr_info *priv = dev_get_priv(dev);
>> +
>> +#if defined(CONFIG_SPL_BUILD)
>> +       struct regmap *map;
>> +       int ret;
>> +
>> +       debug("FU540 DDR probe\n");
>> +       priv->dev = dev;
>> +
>> +       ret = regmap_init_mem(dev_ofnode(dev), &map);
>> +       if (ret)
>> +               return ret;
>> +
>> +       priv->ctl = regmap_get_range(map, 0);
>> +       priv->phy = regmap_get_range(map, 1);
>> +       priv->physical_filter_ctrl = regmap_get_range(map, 2);
>> +
>> +       priv->info.base = CONFIG_SYS_SDRAM_BASE;
>> +
>> +       priv->info.size = 0;
>> +       return fu540_ddr_setup(dev);
>> +#else
>> +       priv->info.base = CONFIG_SYS_SDRAM_BASE;
>> +       priv->info.size = DDR_MEM_SIZE; #endif
>> +       return 0;
>> +}
>> +
>> +static int fu540_ddr_get_info(struct udevice *dev, struct ram_info
>> +*info) {
>> +       struct ddr_info *priv = dev_get_priv(dev);
>> +
>> +       *info = priv->info;
>> +
>> +       return 0;
>> +}
>> +
>> +static struct ram_ops fu540_ddr_ops = {
>> +       .get_info = fu540_ddr_get_info, };
>> +
>> +static const struct udevice_id fu540_ddr_ids[] = {
>> +       { .compatible = "sifive,fu540-ddr" },
>
>"sifive,fu540-c000-ddr" ?

Will update in v6.

>
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(fu540_ddr) = {
>> +       .name = "fu540_ddr",
>> +       .id = UCLASS_RAM,
>> +       .of_match = fu540_ddr_ids,
>> +       .ops = &fu540_ddr_ops,
>> +       .probe = fu540_ddr_probe,
>> +       .priv_auto_alloc_size = sizeof(struct ddr_info),
>> +       .platdata_auto_alloc_size = sizeof(struct sifive_dmc_plat), };
>> diff --git a/drivers/ram/sifive/sdram_fu540.h
>> b/drivers/ram/sifive/sdram_fu540.h
>> new file mode 100644
>> index 0000000000..09693a038e
>> --- /dev/null
>> +++ b/drivers/ram/sifive/sdram_fu540.h
>> @@ -0,0 +1,94 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause */
>> +/*
>> + * Copyright (C) 2020, SiFive, Inc
>> + *
>> + * Authors:
>> + *   Pragnesh Patel <pragnesh.patel at sifive.com>
>> + */
>> +
>> +#define DENALI_CTL_0   0
>> +#define DENALI_CTL_21  21
>> +#define DENALI_CTL_120 120
>> +#define DENALI_CTL_132 132
>> +#define DENALI_CTL_136 136
>> +#define DENALI_CTL_170 170
>> +#define DENALI_CTL_181 181
>> +#define DENALI_CTL_182 182
>> +#define DENALI_CTL_184 184
>> +#define DENALI_CTL_208 208
>> +#define DENALI_CTL_209 209
>> +#define DENALI_CTL_210 210
>> +#define DENALI_CTL_212 212
>> +#define DENALI_CTL_214 214
>> +#define DENALI_CTL_216 216
>> +#define DENALI_CTL_224 224
>> +#define DENALI_CTL_225 225
>> +#define DENALI_CTL_260 260
>> +
>> +#define DENALI_PHY_1152        1152
>> +#define DENALI_PHY_1214        1214
>> +
>> +#define PAYLOAD_DEST   0x80000000
>> +#define DDR_MEM_SIZE   (8UL * 1024UL * 1024UL * 1024UL)
>> +
>> +#define DRAM_CLASS_OFFSET                   8
>> +#define DRAM_CLASS_DDR4                     0xA
>> +#define OPTIMAL_RMODW_EN_OFFSET             0
>> +#define DISABLE_RD_INTERLEAVE_OFFSET        16
>> +#define OUT_OF_RANGE_OFFSET                 1
>> +#define MULTIPLE_OUT_OF_RANGE_OFFSET        2
>> +#define PORT_COMMAND_CHANNEL_ERROR_OFFSET   7
>> +#define MC_INIT_COMPLETE_OFFSET             8
>> +#define LEVELING_OPERATION_COMPLETED_OFFSET 22
>> +#define DFI_PHY_WRLELV_MODE_OFFSET          24
>> +#define DFI_PHY_RDLVL_MODE_OFFSET           24
>> +#define DFI_PHY_RDLVL_GATE_MODE_OFFSET      0
>> +#define VREF_EN_OFFSET                      24
>> +#define PORT_ADDR_PROTECTION_EN_OFFSET      0
>> +#define AXI0_ADDRESS_RANGE_ENABLE           8
>> +#define AXI0_RANGE_PROT_BITS_0_OFFSET       24
>> +#define RDLVL_EN_OFFSET                     16
>> +#define RDLVL_GATE_EN_OFFSET                24
>> +#define WRLVL_EN_OFFSET                     0
>> +
>> +#define PHY_RX_CAL_DQ0_0_OFFSET             0
>> +#define PHY_RX_CAL_DQ1_0_OFFSET             16
>> +
>> +struct fu540_ddrctl {
>> +       volatile u32 denali_ctl[265];
>> +};
>> +
>> +struct fu540_ddrphy {
>> +       volatile u32 denali_phy[1215]; };
>> +
>> +struct fu540_sdram_params {
>> +       struct fu540_ddrctl pctl_regs;
>> +       struct fu540_ddrphy phy_regs;
>> +};
>> +
>> +struct sifive_dmc_plat {
>> +#if CONFIG_IS_ENABLED(OF_PLATDATA)
>> +       struct dtd_sifive_fu540_dmc dtplat;
>
>Do we use this for SPL?

Yes, will update in v6.

>
>> +#else
>> +       struct fu540_sdram_params sdram_params; #endif };
>> +
>> +/**
>> + * struct ddr_info
>> + *
>> + * @dev                                : pointer for the device
>> + * @info                       : UCLASS RAM information
>> + * @ctl                                : DDR controleur base address
>
>typo: controller

Thanks, will update.

>
>> + * @phy                                : DDR PHY base address
>> + * @ctrl                       : DDR control base address
>> + * @physical_filter_ctrl       : DDR physical filter control base address
>> + */
>> +struct ddr_info {
>> +       struct udevice *dev;
>> +       struct ram_info info;
>> +       struct fu540_ddrctl *ctl;
>> +       struct fu540_ddrphy *phy;
>> +       u32 *physical_filter_ctrl;
>> +};
>> --
>
>Regards,
>Bin
Pragnesh Patel March 17, 2020, 1:05 p.m. UTC | #4 
Hi,

>-----Original Message-----
>From: Giulio Benetti <giulio.benetti at benettiengineering.com>
>Sent: 13 March 2020 17:26
>To: Pragnesh Patel <pragnesh.patel at sifive.com>; u-boot at lists.denx.de
>Cc: atish.patra at wdc.com; palmerdabbelt at google.com;
>bmeng.cn at gmail.com; Paul Walmsley <paul.walmsley at sifive.com>;
>jagan at amarulasolutions.com; Troy Benjegerdes
><troy.benjegerdes at sifive.com>; anup.patel at wdc.com; Sagar Kadam
><sagar.kadam at sifive.com>; rick at andestech.com; Kever Yang
><kever.yang at rock-chips.com>; Lokesh Vutla <lokeshvutla at ti.com>; Kevin
>Scholz <k-scholz at ti.com>; YouMin Chen <cym at rock-chips.com>
>Subject: Re: [PATCH v5 06/14] sifive: fu540: add ddr driver
>
>Hi Pragnesh,
>
>On 3/11/20 8:03 AM, Pragnesh Patel wrote:
>> Add driver for fu540 to support ddr initialization in SPL.
>> This driver is based on FSBL
>> (https://github.com/sifive/freedom-u540-c000-bootloader.git)
>>
>> Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com>
>> ---
>>   drivers/ram/Kconfig              |   7 +
>>   drivers/ram/Makefile             |   2 +
>>   drivers/ram/sifive/Kconfig       |   8 +
>>   drivers/ram/sifive/Makefile      |   6 +
>>   drivers/ram/sifive/sdram_fu540.c | 295
>+++++++++++++++++++++++++++++++
>>   drivers/ram/sifive/sdram_fu540.h |  94 ++++++++++
>>   6 files changed, 412 insertions(+)
>>   create mode 100644 drivers/ram/sifive/Kconfig
>>   create mode 100644 drivers/ram/sifive/Makefile
>>   create mode 100644 drivers/ram/sifive/sdram_fu540.c
>>   create mode 100644 drivers/ram/sifive/sdram_fu540.h
>>
>> diff --git a/drivers/ram/Kconfig b/drivers/ram/Kconfig index
>> 56fea7c94c..c60c63204c 100644
>> --- a/drivers/ram/Kconfig
>> +++ b/drivers/ram/Kconfig
>> @@ -73,5 +73,12 @@ config IMXRT_SDRAM
>>   	  to support external memories like sdram, psram & nand.
>>   	  This driver is for the sdram memory interface with the SEMC.
>>
>> +config SIFIVE_DDR
>> +	bool "Enable SiFive DDR support"
>> +	depends on RAM
>> +	help
>> +	  Enable support for the internal DDR Memory Controller of SiFive
>SoCs.
>> +
>>   source "drivers/ram/rockchip/Kconfig"
>>   source "drivers/ram/stm32mp1/Kconfig"
>> +source "drivers/ram/sifive/Kconfig"
>> diff --git a/drivers/ram/Makefile b/drivers/ram/Makefile index
>> 5c897410c6..12bf61510b 100644
>> --- a/drivers/ram/Makefile
>> +++ b/drivers/ram/Makefile
>> @@ -17,3 +17,5 @@ obj-$(CONFIG_ARCH_MEDIATEK) += mediatek/
>>   obj-$(CONFIG_K3_J721E_DDRSS) += k3-j721e/
>>
>>   obj-$(CONFIG_IMXRT_SDRAM) += imxrt_sdram.o
>> +
>> +obj-$(CONFIG_SIFIVE_DDR) += sifive/
>> diff --git a/drivers/ram/sifive/Kconfig b/drivers/ram/sifive/Kconfig
>> new file mode 100644 index 0000000000..b754700db8
>> --- /dev/null
>> +++ b/drivers/ram/sifive/Kconfig
>> @@ -0,0 +1,8 @@
>> +config SIFIVE_FU540_DDR
>> +	bool "SiFive FU540 DDR driver"
>> +	depends on DM && OF_CONTROL
>> +	select RAM
>> +	select SPL_RAM if SPL
>> +	select SIFIVE_DDR
>> +	help
>> +	  This enables DDR support for the platforms based on SiFive FU540
>SoC.
>> diff --git a/drivers/ram/sifive/Makefile b/drivers/ram/sifive/Makefile
>> new file mode 100644 index 0000000000..0187805199
>> --- /dev/null
>> +++ b/drivers/ram/sifive/Makefile
>> @@ -0,0 +1,6 @@
>> +# SPDX-License-Identifier: GPL-2.0+
>> +#
>> +# Copyright (c) 2020 SiFive, Inc
>> +#
>> +
>> +obj-$(CONFIG_SIFIVE_FU540_DDR) += sdram_fu540.o
>> diff --git a/drivers/ram/sifive/sdram_fu540.c
>> b/drivers/ram/sifive/sdram_fu540.c
>> new file mode 100644
>> index 0000000000..18926dbe15
>> --- /dev/null
>> +++ b/drivers/ram/sifive/sdram_fu540.c
>> @@ -0,0 +1,295 @@
>> +// SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause
>> +/*
>> + * (C) Copyright 2020 SiFive, Inc.
>> + *
>> + * Authors:
>> + *   Pragnesh Patel <pragnesh.patel at sifive.com>
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <init.h>
>> +#include <ram.h>
>> +#include <regmap.h>
>> +#include <syscon.h>
>> +#include <asm/io.h>
>> +#include "sdram_fu540.h"
>> +
>> +/* n: Unit bytes */
>> +void sdram_copy_to_reg(volatile u32 *dest, volatile u32 *src, u32 n)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < n / sizeof(u32); i++) {
>> +		writel(*src, dest);
>> +		src++;
>> +		dest++;
>> +	}
>> +}
>> +
>> +static void ddr_setuprangeprotection(volatile u32 *ctl, u64 end_addr)
>> +{
>> +	writel(0x0, DENALI_CTL_209 + ctl);
>> +	u32 end_addr_16kblocks = ((end_addr >> 14) & 0x7FFFFF) - 1;
>> +
>> +	writel(end_addr_16kblocks, DENALI_CTL_210 + ctl);
>> +	writel(0x0, DENALI_CTL_212 + ctl);
>> +	writel(0x0, DENALI_CTL_214 + ctl);
>> +	writel(0x0, DENALI_CTL_216 + ctl);
>> +	setbits_le32(DENALI_CTL_224 + ctl,
>> +		     0x3 << AXI0_RANGE_PROT_BITS_0_OFFSET);
>> +	writel(0xFFFFFFFF, DENALI_CTL_225 + ctl);
>> +	setbits_le32(DENALI_CTL_208 + ctl, 0x1 <<
>AXI0_ADDRESS_RANGE_ENABLE);
>> +	setbits_le32(DENALI_CTL_208 + ctl,
>> +		     0x1 << PORT_ADDR_PROTECTION_EN_OFFSET); }
>> +
>> +static void ddr_start(volatile u32 *ctl, u32 *physical_filter_ctrl,
>> +u64 ddr_end) {
>> +	setbits_le32(DENALI_CTL_0 + ctl, 0x1);
>> +	while ((readl(DENALI_CTL_132 + ctl) & (1 <<
>MC_INIT_COMPLETE_OFFSET))
>> +	       == 0) {
>> +	}
>> +
>> +	// Disable the BusBlocker in front of the controller AXI slave ports
>> +	volatile u64 *filterreg = (volatile u64 *)physical_filter_ctrl;
>> +
>> +	filterreg[0] = 0x0f00000000000000UL | (ddr_end >> 2); }
>> +
>> +static u64 ddr_phy_fixup(volatile u32 *ddrphyreg) {
>> +	// return bitmask of failed lanes
>> +
>> +	u64 fails     = 0;
>> +	u32 slicebase = 0;
>> +	u32 dq        = 0;
>> +
>> +	// check errata condition
>> +	for (u32 slice = 0; slice < 8; slice++) {
>> +		u32 regbase = slicebase + 34;
>> +
>> +		for (u32 reg = 0; reg < 4; reg++) {
>> +			u32 updownreg = readl(regbase + reg + ddrphyreg);
>> +
>> +			for (u32 bit = 0; bit < 2; bit++) {
>> +				u32 phy_rx_cal_dqn_0_offset;
>> +
>> +				if (bit == 0) {
>> +					phy_rx_cal_dqn_0_offset =
>> +						PHY_RX_CAL_DQ0_0_OFFSET;
>> +				} else {
>> +					phy_rx_cal_dqn_0_offset =
>> +						PHY_RX_CAL_DQ1_0_OFFSET;
>> +				}
>> +
>> +				u32 down = (updownreg >>
>> +					    phy_rx_cal_dqn_0_offset) & 0x3F;
>> +				u32 up   = (updownreg >>
>> +					    (phy_rx_cal_dqn_0_offset + 6)) &
>> +					    0x3F;
>> +
>> +				u8 failc0 = ((down == 0) && (up == 0x3F));
>> +				u8 failc1 = ((up == 0) && (down == 0x3F));
>> +
>> +				// print error message on failure
>> +				if (failc0 || failc1) {
>> +					if (fails == 0)
>> +						printf("DDR error in fixing
>up\n");
>> +
>> +					fails |= (1 << dq);
>> +
>> +					char slicelsc = '0';
>> +					char slicemsc = '0';
>> +
>> +					slicelsc += (dq % 10);
>> +					slicemsc += (dq / 10);
>> +					printf("S ");
>> +					printf("%c", slicemsc);
>> +					printf("%c", slicelsc);
>> +
>> +					if (failc0)
>> +						printf("U");
>> +					else
>> +						printf("D");
>> +
>> +					printf("\n");
>> +				}
>
>Here you've nested 5 times. Can you try to break up all these iterations? At
>least moving some code to external static functions. It is difficult to read these
>3 nested for cycles plus other 2 nested if conditions. And for sure keep declare
>variables at the top.

I think you are right, Will check and update.

>
>> +				dq++;
>> +			}
>> +		}
>> +		slicebase += 128;
>> +	}
>> +	return(0);
>> +}
>> +
>> +static u32 ddr_getdramclass(volatile u32 *ctl) {
>> +	u32 reg = readl(DENALI_CTL_0 + ctl);
>> +
>> +	return ((reg >> DRAM_CLASS_OFFSET) & 0xF); }
>> +
>> +static __maybe_unused int fu540_ddr_setup(struct udevice *dev) {
>> +	struct ddr_info *priv = dev_get_priv(dev);
>> +	struct sifive_dmc_plat *plat = dev_get_platdata(dev);
>> +
>> +	int ret, i;
>> +	u32 physet;
>> +
>> +	volatile u32 *denali_ctl =  &priv->ctl->denali_ctl[0];
>> +	volatile u32 *denali_phy =  &priv->phy->denali_phy[0];
>> +
>> +	ret = dev_read_u32_array(dev, "sifive,sdram-params",
>> +				 (u32 *)&plat->sdram_params,
>> +				 sizeof(plat->sdram_params) / sizeof(u32));
>> +	if (ret) {
>> +		printf("%s: Cannot read sifive,sdram-params %d\n",
>> +		       __func__, ret);
>> +		return ret;
>> +	}
>> +
>> +	struct fu540_sdram_params *params = &plat->sdram_params;
>> +
>> +	sdram_copy_to_reg(&priv->ctl->denali_ctl[0],
>> +			  &params->pctl_regs.denali_ctl[0],
>> +			  sizeof(struct fu540_ddrctl));
>> +
>> +	/* phy reset */
>> +	for (i = DENALI_PHY_1152; i <= DENALI_PHY_1214; i++) {
>> +		physet = params->phy_regs.denali_phy[i];
>> +		priv->phy->denali_phy[i] = physet;
>> +	}
>> +
>> +	for (i = 0; i < DENALI_PHY_1152; i++) {
>> +		physet = params->phy_regs.denali_phy[i];
>> +		priv->phy->denali_phy[i] = physet;
>> +	}
>> +
>> +	/* Disable read interleave DENALI_CTL_120 */
>> +	setbits_le32(DENALI_CTL_120 + denali_ctl,
>> +		     1 << DISABLE_RD_INTERLEAVE_OFFSET);
>> +
>> +	/* Disable optimal read/modify/write logic DENALI_CTL_21 */
>> +	clrbits_le32(DENALI_CTL_21 + denali_ctl, 1 <<
>> +OPTIMAL_RMODW_EN_OFFSET);
>> +
>> +	/* Enable write Leveling DENALI_CTL_170 */
>> +	setbits_le32(DENALI_CTL_170 + denali_ctl, (1 << WRLVL_EN_OFFSET)
>> +		     | (1 << DFI_PHY_WRLELV_MODE_OFFSET));
>> +
>> +	/* Enable read leveling DENALI_CTL_181 and DENALI_CTL_260 */
>> +	setbits_le32(DENALI_CTL_181 + denali_ctl,
>> +		     1 << DFI_PHY_RDLVL_MODE_OFFSET);
>> +	setbits_le32(DENALI_CTL_260 + denali_ctl, 1 << RDLVL_EN_OFFSET);
>> +
>> +	/* Enable read leveling gate DENALI_CTL_260 and DENALI_CTL_182 */
>> +	setbits_le32(DENALI_CTL_260 + denali_ctl, 1 <<
>RDLVL_GATE_EN_OFFSET);
>> +	setbits_le32(DENALI_CTL_182 + denali_ctl,
>> +		     1 << DFI_PHY_RDLVL_GATE_MODE_OFFSET);
>> +
>> +	if (ddr_getdramclass(denali_ctl) == DRAM_CLASS_DDR4) {
>> +		/* Enable vref training DENALI_CTL_184 */
>> +		setbits_le32(DENALI_CTL_184 + denali_ctl, 1 <<
>VREF_EN_OFFSET);
>> +	}
>> +
>> +	/* Mask off leveling completion interrupt DENALI_CTL_136 */
>> +	setbits_le32(DENALI_CTL_136 + denali_ctl,
>> +		     1 << LEVELING_OPERATION_COMPLETED_OFFSET);
>> +
>> +	/* Mask off MC init complete interrupt DENALI_CTL_136 */
>> +	setbits_le32(DENALI_CTL_136 + denali_ctl, 1 <<
>> +MC_INIT_COMPLETE_OFFSET);
>> +
>> +	/* Mask off out of range interrupts DENALI_CTL_136 */
>> +	setbits_le32(DENALI_CTL_136 + denali_ctl, (1 <<
>OUT_OF_RANGE_OFFSET)
>> +		     | (1 << MULTIPLE_OUT_OF_RANGE_OFFSET));
>> +
>> +	/* set up range protection */
>> +	ddr_setuprangeprotection(denali_ctl, DDR_MEM_SIZE);
>> +
>> +	/* Mask off port command error interrupt DENALI_CTL_136 */
>> +	setbits_le32(DENALI_CTL_136 + denali_ctl,
>> +		     1 << PORT_COMMAND_CHANNEL_ERROR_OFFSET);
>> +
>> +	const u64 ddr_size = DDR_MEM_SIZE;
>> +	const u64 ddr_end = PAYLOAD_DEST + ddr_size;
>> +
>> +	ddr_start(denali_ctl, priv->physical_filter_ctrl, ddr_end);
>> +
>> +	ddr_phy_fixup(denali_phy);
>> +
>> +	/* check size */
>> +	priv->info.size = get_ram_size((long *)priv->info.base,
>> +				       DDR_MEM_SIZE);
>> +
>> +	printf("priv->info.size = %lx\n", priv->info.size);
>> +	debug("%s : %lx\n", __func__, priv->info.size);
>> +
>> +	/* check memory access for all memory */
>> +
>> +	if (priv->info.size != DDR_MEM_SIZE) {
>> +		printf("DDR invalid size : 0x%lx, expected 0x%lx\n",
>> +		       priv->info.size, DDR_MEM_SIZE);
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int fu540_ddr_probe(struct udevice *dev) {
>> +	struct ddr_info *priv = dev_get_priv(dev);
>> +
>> +#if defined(CONFIG_SPL_BUILD)
>> +	struct regmap *map;
>> +	int ret;
>> +
>> +	debug("FU540 DDR probe\n");
>> +	priv->dev = dev;
>> +
>> +	ret = regmap_init_mem(dev_ofnode(dev), &map);
>> +	if (ret)
>> +		return ret;
>> +
>> +	priv->ctl = regmap_get_range(map, 0);
>> +	priv->phy = regmap_get_range(map, 1);
>> +	priv->physical_filter_ctrl = regmap_get_range(map, 2);
>> +
>> +	priv->info.base = CONFIG_SYS_SDRAM_BASE;
>> +
>> +	priv->info.size = 0;
>> +	return fu540_ddr_setup(dev);
>> +#else
>> +	priv->info.base = CONFIG_SYS_SDRAM_BASE;
>> +	priv->info.size = DDR_MEM_SIZE;
>> +#endif
>> +	return 0;
>> +}
>> +
>> +static int fu540_ddr_get_info(struct udevice *dev, struct ram_info
>> +*info) {
>> +	struct ddr_info *priv = dev_get_priv(dev);
>> +
>> +	*info = priv->info;
>> +
>> +	return 0;
>> +}
>> +
>> +static struct ram_ops fu540_ddr_ops = {
>> +	.get_info = fu540_ddr_get_info,
>> +};
>> +
>> +static const struct udevice_id fu540_ddr_ids[] = {
>> +	{ .compatible = "sifive,fu540-ddr" },
>> +	{ }
>> +};
>> +
>> +U_BOOT_DRIVER(fu540_ddr) = {
>> +	.name = "fu540_ddr",
>> +	.id = UCLASS_RAM,
>> +	.of_match = fu540_ddr_ids,
>> +	.ops = &fu540_ddr_ops,
>> +	.probe = fu540_ddr_probe,
>> +	.priv_auto_alloc_size = sizeof(struct ddr_info),
>> +	.platdata_auto_alloc_size = sizeof(struct sifive_dmc_plat), };
>> diff --git a/drivers/ram/sifive/sdram_fu540.h
>> b/drivers/ram/sifive/sdram_fu540.h
>> new file mode 100644
>> index 0000000000..09693a038e
>> --- /dev/null
>> +++ b/drivers/ram/sifive/sdram_fu540.h
>> @@ -0,0 +1,94 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause */
>> +/*
>> + * Copyright (C) 2020, SiFive, Inc
>> + *
>> + * Authors:
>> + *   Pragnesh Patel <pragnesh.patel at sifive.com>
>> + */
>> +
>> +#define DENALI_CTL_0	0
>> +#define DENALI_CTL_21	21
>> +#define DENALI_CTL_120	120
>> +#define DENALI_CTL_132	132
>> +#define DENALI_CTL_136	136
>> +#define DENALI_CTL_170	170
>> +#define DENALI_CTL_181	181
>> +#define DENALI_CTL_182	182
>> +#define DENALI_CTL_184	184
>> +#define DENALI_CTL_208	208
>> +#define DENALI_CTL_209	209
>> +#define DENALI_CTL_210	210
>> +#define DENALI_CTL_212	212
>> +#define DENALI_CTL_214	214
>> +#define DENALI_CTL_216	216
>> +#define DENALI_CTL_224	224
>> +#define DENALI_CTL_225	225
>> +#define DENALI_CTL_260	260
>> +
>> +#define DENALI_PHY_1152	1152
>> +#define DENALI_PHY_1214	1214
>> +
>> +#define PAYLOAD_DEST	0x80000000
>> +#define DDR_MEM_SIZE	(8UL * 1024UL * 1024UL * 1024UL)
>> +
>> +#define DRAM_CLASS_OFFSET                   8
>> +#define DRAM_CLASS_DDR4                     0xA
>> +#define OPTIMAL_RMODW_EN_OFFSET             0
>> +#define DISABLE_RD_INTERLEAVE_OFFSET        16
>> +#define OUT_OF_RANGE_OFFSET                 1
>> +#define MULTIPLE_OUT_OF_RANGE_OFFSET        2
>> +#define PORT_COMMAND_CHANNEL_ERROR_OFFSET   7
>> +#define MC_INIT_COMPLETE_OFFSET             8
>> +#define LEVELING_OPERATION_COMPLETED_OFFSET 22
>> +#define DFI_PHY_WRLELV_MODE_OFFSET          24
>> +#define DFI_PHY_RDLVL_MODE_OFFSET           24
>> +#define DFI_PHY_RDLVL_GATE_MODE_OFFSET      0
>> +#define VREF_EN_OFFSET                      24
>> +#define PORT_ADDR_PROTECTION_EN_OFFSET      0
>> +#define AXI0_ADDRESS_RANGE_ENABLE           8
>> +#define AXI0_RANGE_PROT_BITS_0_OFFSET       24
>> +#define RDLVL_EN_OFFSET                     16
>> +#define RDLVL_GATE_EN_OFFSET                24
>> +#define WRLVL_EN_OFFSET                     0
>> +
>> +#define PHY_RX_CAL_DQ0_0_OFFSET             0
>> +#define PHY_RX_CAL_DQ1_0_OFFSET             16
>> +
>> +struct fu540_ddrctl {
>> +	volatile u32 denali_ctl[265];
>> +};
>> +
>> +struct fu540_ddrphy {
>> +	volatile u32 denali_phy[1215];
>> +};
>> +
>> +struct fu540_sdram_params {
>> +	struct fu540_ddrctl pctl_regs;
>> +	struct fu540_ddrphy phy_regs;
>> +};
>> +
>> +struct sifive_dmc_plat {
>> +#if CONFIG_IS_ENABLED(OF_PLATDATA)
>> +	struct dtd_sifive_fu540_dmc dtplat;
>> +#else
>> +	struct fu540_sdram_params sdram_params; #endif };
>> +
>> +/**
>> + * struct ddr_info
>> + *
>> + * @dev				: pointer for the device
>> + * @info			: UCLASS RAM information
>> + * @ctl				: DDR controleur base address
>> + * @phy				: DDR PHY base address
>> + * @ctrl			: DDR control base address
>> + * @physical_filter_ctrl	: DDR physical filter control base address
>> + */
>> +struct ddr_info {
>> +	struct udevice *dev;
>> +	struct ram_info info;
>> +	struct fu540_ddrctl *ctl;
>> +	struct fu540_ddrphy *phy;
>> +	u32 *physical_filter_ctrl;
>> +};
>>
>
>IMHO I would keep all these structs and define in driver's .c file, I mean you
>should keep them private if not used outside. And then add to dt-bindings the
>define needed by dts files.

This structures are already private in driver and will move them to driver's .c file.

>
>Best regards
>--
>Giulio Benetti
>Benetti Engineering sas
diff mbox series

Patch

diff --git a/drivers/ram/Kconfig b/drivers/ram/Kconfig
index 56fea7c94c..c60c63204c 100644
--- a/drivers/ram/Kconfig
+++ b/drivers/ram/Kconfig
@@ -73,5 +73,12 @@  config IMXRT_SDRAM
 	  to support external memories like sdram, psram & nand.
 	  This driver is for the sdram memory interface with the SEMC.
 
+config SIFIVE_DDR
+	bool "Enable SiFive DDR support"
+	depends on RAM
+	help
+	  Enable support for the internal DDR Memory Controller of SiFive SoCs.
+
 source "drivers/ram/rockchip/Kconfig"
 source "drivers/ram/stm32mp1/Kconfig"
+source "drivers/ram/sifive/Kconfig"
diff --git a/drivers/ram/Makefile b/drivers/ram/Makefile
index 5c897410c6..12bf61510b 100644
--- a/drivers/ram/Makefile
+++ b/drivers/ram/Makefile
@@ -17,3 +17,5 @@  obj-$(CONFIG_ARCH_MEDIATEK) += mediatek/
 obj-$(CONFIG_K3_J721E_DDRSS) += k3-j721e/
 
 obj-$(CONFIG_IMXRT_SDRAM) += imxrt_sdram.o
+
+obj-$(CONFIG_SIFIVE_DDR) += sifive/
diff --git a/drivers/ram/sifive/Kconfig b/drivers/ram/sifive/Kconfig
new file mode 100644
index 0000000000..b754700db8
--- /dev/null
+++ b/drivers/ram/sifive/Kconfig
@@ -0,0 +1,8 @@ 
+config SIFIVE_FU540_DDR
+	bool "SiFive FU540 DDR driver"
+	depends on DM && OF_CONTROL
+	select RAM
+	select SPL_RAM if SPL
+	select SIFIVE_DDR
+	help
+	  This enables DDR support for the platforms based on SiFive FU540 SoC.
diff --git a/drivers/ram/sifive/Makefile b/drivers/ram/sifive/Makefile
new file mode 100644
index 0000000000..0187805199
--- /dev/null
+++ b/drivers/ram/sifive/Makefile
@@ -0,0 +1,6 @@ 
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (c) 2020 SiFive, Inc
+#
+
+obj-$(CONFIG_SIFIVE_FU540_DDR) += sdram_fu540.o
diff --git a/drivers/ram/sifive/sdram_fu540.c b/drivers/ram/sifive/sdram_fu540.c
new file mode 100644
index 0000000000..18926dbe15
--- /dev/null
+++ b/drivers/ram/sifive/sdram_fu540.c
@@ -0,0 +1,295 @@ 
+// SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause
+/*
+ * (C) Copyright 2020 SiFive, Inc.
+ *
+ * Authors:
+ *   Pragnesh Patel <pragnesh.patel at sifive.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <init.h>
+#include <ram.h>
+#include <regmap.h>
+#include <syscon.h>
+#include <asm/io.h>
+#include "sdram_fu540.h"
+
+/* n: Unit bytes */
+void sdram_copy_to_reg(volatile u32 *dest, volatile u32 *src, u32 n)
+{
+	int i;
+
+	for (i = 0; i < n / sizeof(u32); i++) {
+		writel(*src, dest);
+		src++;
+		dest++;
+	}
+}
+
+static void ddr_setuprangeprotection(volatile u32 *ctl, u64 end_addr)
+{
+	writel(0x0, DENALI_CTL_209 + ctl);
+	u32 end_addr_16kblocks = ((end_addr >> 14) & 0x7FFFFF) - 1;
+
+	writel(end_addr_16kblocks, DENALI_CTL_210 + ctl);
+	writel(0x0, DENALI_CTL_212 + ctl);
+	writel(0x0, DENALI_CTL_214 + ctl);
+	writel(0x0, DENALI_CTL_216 + ctl);
+	setbits_le32(DENALI_CTL_224 + ctl,
+		     0x3 << AXI0_RANGE_PROT_BITS_0_OFFSET);
+	writel(0xFFFFFFFF, DENALI_CTL_225 + ctl);
+	setbits_le32(DENALI_CTL_208 + ctl, 0x1 << AXI0_ADDRESS_RANGE_ENABLE);
+	setbits_le32(DENALI_CTL_208 + ctl,
+		     0x1 << PORT_ADDR_PROTECTION_EN_OFFSET);
+}
+
+static void ddr_start(volatile u32 *ctl, u32 *physical_filter_ctrl, u64 ddr_end)
+{
+	setbits_le32(DENALI_CTL_0 + ctl, 0x1);
+	while ((readl(DENALI_CTL_132 + ctl) & (1 << MC_INIT_COMPLETE_OFFSET))
+	       == 0) {
+	}
+
+	// Disable the BusBlocker in front of the controller AXI slave ports
+	volatile u64 *filterreg = (volatile u64 *)physical_filter_ctrl;
+
+	filterreg[0] = 0x0f00000000000000UL | (ddr_end >> 2);
+}
+
+static u64 ddr_phy_fixup(volatile u32 *ddrphyreg)
+{
+	// return bitmask of failed lanes
+
+	u64 fails     = 0;
+	u32 slicebase = 0;
+	u32 dq        = 0;
+
+	// check errata condition
+	for (u32 slice = 0; slice < 8; slice++) {
+		u32 regbase = slicebase + 34;
+
+		for (u32 reg = 0; reg < 4; reg++) {
+			u32 updownreg = readl(regbase + reg + ddrphyreg);
+
+			for (u32 bit = 0; bit < 2; bit++) {
+				u32 phy_rx_cal_dqn_0_offset;
+
+				if (bit == 0) {
+					phy_rx_cal_dqn_0_offset =
+						PHY_RX_CAL_DQ0_0_OFFSET;
+				} else {
+					phy_rx_cal_dqn_0_offset =
+						PHY_RX_CAL_DQ1_0_OFFSET;
+				}
+
+				u32 down = (updownreg >>
+					    phy_rx_cal_dqn_0_offset) & 0x3F;
+				u32 up   = (updownreg >>
+					    (phy_rx_cal_dqn_0_offset + 6)) &
+					    0x3F;
+
+				u8 failc0 = ((down == 0) && (up == 0x3F));
+				u8 failc1 = ((up == 0) && (down == 0x3F));
+
+				// print error message on failure
+				if (failc0 || failc1) {
+					if (fails == 0)
+						printf("DDR error in fixing up\n");
+
+					fails |= (1 << dq);
+
+					char slicelsc = '0';
+					char slicemsc = '0';
+
+					slicelsc += (dq % 10);
+					slicemsc += (dq / 10);
+					printf("S ");
+					printf("%c", slicemsc);
+					printf("%c", slicelsc);
+
+					if (failc0)
+						printf("U");
+					else
+						printf("D");
+
+					printf("\n");
+				}
+				dq++;
+			}
+		}
+		slicebase += 128;
+	}
+	return(0);
+}
+
+static u32 ddr_getdramclass(volatile u32 *ctl)
+{
+	u32 reg = readl(DENALI_CTL_0 + ctl);
+
+	return ((reg >> DRAM_CLASS_OFFSET) & 0xF);
+}
+
+static __maybe_unused int fu540_ddr_setup(struct udevice *dev)
+{
+	struct ddr_info *priv = dev_get_priv(dev);
+	struct sifive_dmc_plat *plat = dev_get_platdata(dev);
+
+	int ret, i;
+	u32 physet;
+
+	volatile u32 *denali_ctl =  &priv->ctl->denali_ctl[0];
+	volatile u32 *denali_phy =  &priv->phy->denali_phy[0];
+
+	ret = dev_read_u32_array(dev, "sifive,sdram-params",
+				 (u32 *)&plat->sdram_params,
+				 sizeof(plat->sdram_params) / sizeof(u32));
+	if (ret) {
+		printf("%s: Cannot read sifive,sdram-params %d\n",
+		       __func__, ret);
+		return ret;
+	}
+
+	struct fu540_sdram_params *params = &plat->sdram_params;
+
+	sdram_copy_to_reg(&priv->ctl->denali_ctl[0],
+			  &params->pctl_regs.denali_ctl[0],
+			  sizeof(struct fu540_ddrctl));
+
+	/* phy reset */
+	for (i = DENALI_PHY_1152; i <= DENALI_PHY_1214; i++) {
+		physet = params->phy_regs.denali_phy[i];
+		priv->phy->denali_phy[i] = physet;
+	}
+
+	for (i = 0; i < DENALI_PHY_1152; i++) {
+		physet = params->phy_regs.denali_phy[i];
+		priv->phy->denali_phy[i] = physet;
+	}
+
+	/* Disable read interleave DENALI_CTL_120 */
+	setbits_le32(DENALI_CTL_120 + denali_ctl,
+		     1 << DISABLE_RD_INTERLEAVE_OFFSET);
+
+	/* Disable optimal read/modify/write logic DENALI_CTL_21 */
+	clrbits_le32(DENALI_CTL_21 + denali_ctl, 1 << OPTIMAL_RMODW_EN_OFFSET);
+
+	/* Enable write Leveling DENALI_CTL_170 */
+	setbits_le32(DENALI_CTL_170 + denali_ctl, (1 << WRLVL_EN_OFFSET)
+		     | (1 << DFI_PHY_WRLELV_MODE_OFFSET));
+
+	/* Enable read leveling DENALI_CTL_181 and DENALI_CTL_260 */
+	setbits_le32(DENALI_CTL_181 + denali_ctl,
+		     1 << DFI_PHY_RDLVL_MODE_OFFSET);
+	setbits_le32(DENALI_CTL_260 + denali_ctl, 1 << RDLVL_EN_OFFSET);
+
+	/* Enable read leveling gate DENALI_CTL_260 and DENALI_CTL_182 */
+	setbits_le32(DENALI_CTL_260 + denali_ctl, 1 << RDLVL_GATE_EN_OFFSET);
+	setbits_le32(DENALI_CTL_182 + denali_ctl,
+		     1 << DFI_PHY_RDLVL_GATE_MODE_OFFSET);
+
+	if (ddr_getdramclass(denali_ctl) == DRAM_CLASS_DDR4) {
+		/* Enable vref training DENALI_CTL_184 */
+		setbits_le32(DENALI_CTL_184 + denali_ctl, 1 << VREF_EN_OFFSET);
+	}
+
+	/* Mask off leveling completion interrupt DENALI_CTL_136 */
+	setbits_le32(DENALI_CTL_136 + denali_ctl,
+		     1 << LEVELING_OPERATION_COMPLETED_OFFSET);
+
+	/* Mask off MC init complete interrupt DENALI_CTL_136 */
+	setbits_le32(DENALI_CTL_136 + denali_ctl, 1 << MC_INIT_COMPLETE_OFFSET);
+
+	/* Mask off out of range interrupts DENALI_CTL_136 */
+	setbits_le32(DENALI_CTL_136 + denali_ctl, (1 << OUT_OF_RANGE_OFFSET)
+		     | (1 << MULTIPLE_OUT_OF_RANGE_OFFSET));
+
+	/* set up range protection */
+	ddr_setuprangeprotection(denali_ctl, DDR_MEM_SIZE);
+
+	/* Mask off port command error interrupt DENALI_CTL_136 */
+	setbits_le32(DENALI_CTL_136 + denali_ctl,
+		     1 << PORT_COMMAND_CHANNEL_ERROR_OFFSET);
+
+	const u64 ddr_size = DDR_MEM_SIZE;
+	const u64 ddr_end = PAYLOAD_DEST + ddr_size;
+
+	ddr_start(denali_ctl, priv->physical_filter_ctrl, ddr_end);
+
+	ddr_phy_fixup(denali_phy);
+
+	/* check size */
+	priv->info.size = get_ram_size((long *)priv->info.base,
+				       DDR_MEM_SIZE);
+
+	printf("priv->info.size = %lx\n", priv->info.size);
+	debug("%s : %lx\n", __func__, priv->info.size);
+
+	/* check memory access for all memory */
+
+	if (priv->info.size != DDR_MEM_SIZE) {
+		printf("DDR invalid size : 0x%lx, expected 0x%lx\n",
+		       priv->info.size, DDR_MEM_SIZE);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int fu540_ddr_probe(struct udevice *dev)
+{
+	struct ddr_info *priv = dev_get_priv(dev);
+
+#if defined(CONFIG_SPL_BUILD)
+	struct regmap *map;
+	int ret;
+
+	debug("FU540 DDR probe\n");
+	priv->dev = dev;
+
+	ret = regmap_init_mem(dev_ofnode(dev), &map);
+	if (ret)
+		return ret;
+
+	priv->ctl = regmap_get_range(map, 0);
+	priv->phy = regmap_get_range(map, 1);
+	priv->physical_filter_ctrl = regmap_get_range(map, 2);
+
+	priv->info.base = CONFIG_SYS_SDRAM_BASE;
+
+	priv->info.size = 0;
+	return fu540_ddr_setup(dev);
+#else
+	priv->info.base = CONFIG_SYS_SDRAM_BASE;
+	priv->info.size = DDR_MEM_SIZE;
+#endif
+	return 0;
+}
+
+static int fu540_ddr_get_info(struct udevice *dev, struct ram_info *info)
+{
+	struct ddr_info *priv = dev_get_priv(dev);
+
+	*info = priv->info;
+
+	return 0;
+}
+
+static struct ram_ops fu540_ddr_ops = {
+	.get_info = fu540_ddr_get_info,
+};
+
+static const struct udevice_id fu540_ddr_ids[] = {
+	{ .compatible = "sifive,fu540-ddr" },
+	{ }
+};
+
+U_BOOT_DRIVER(fu540_ddr) = {
+	.name = "fu540_ddr",
+	.id = UCLASS_RAM,
+	.of_match = fu540_ddr_ids,
+	.ops = &fu540_ddr_ops,
+	.probe = fu540_ddr_probe,
+	.priv_auto_alloc_size = sizeof(struct ddr_info),
+	.platdata_auto_alloc_size = sizeof(struct sifive_dmc_plat),
+};
diff --git a/drivers/ram/sifive/sdram_fu540.h b/drivers/ram/sifive/sdram_fu540.h
new file mode 100644
index 0000000000..09693a038e
--- /dev/null
+++ b/drivers/ram/sifive/sdram_fu540.h
@@ -0,0 +1,94 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause */
+/*
+ * Copyright (C) 2020, SiFive, Inc
+ *
+ * Authors:
+ *   Pragnesh Patel <pragnesh.patel at sifive.com>
+ */
+
+#define DENALI_CTL_0	0
+#define DENALI_CTL_21	21
+#define DENALI_CTL_120	120
+#define DENALI_CTL_132	132
+#define DENALI_CTL_136	136
+#define DENALI_CTL_170	170
+#define DENALI_CTL_181	181
+#define DENALI_CTL_182	182
+#define DENALI_CTL_184	184
+#define DENALI_CTL_208	208
+#define DENALI_CTL_209	209
+#define DENALI_CTL_210	210
+#define DENALI_CTL_212	212
+#define DENALI_CTL_214	214
+#define DENALI_CTL_216	216
+#define DENALI_CTL_224	224
+#define DENALI_CTL_225	225
+#define DENALI_CTL_260	260
+
+#define DENALI_PHY_1152	1152
+#define DENALI_PHY_1214	1214
+
+#define PAYLOAD_DEST	0x80000000
+#define DDR_MEM_SIZE	(8UL * 1024UL * 1024UL * 1024UL)
+
+#define DRAM_CLASS_OFFSET                   8
+#define DRAM_CLASS_DDR4                     0xA
+#define OPTIMAL_RMODW_EN_OFFSET             0
+#define DISABLE_RD_INTERLEAVE_OFFSET        16
+#define OUT_OF_RANGE_OFFSET                 1
+#define MULTIPLE_OUT_OF_RANGE_OFFSET        2
+#define PORT_COMMAND_CHANNEL_ERROR_OFFSET   7
+#define MC_INIT_COMPLETE_OFFSET             8
+#define LEVELING_OPERATION_COMPLETED_OFFSET 22
+#define DFI_PHY_WRLELV_MODE_OFFSET          24
+#define DFI_PHY_RDLVL_MODE_OFFSET           24
+#define DFI_PHY_RDLVL_GATE_MODE_OFFSET      0
+#define VREF_EN_OFFSET                      24
+#define PORT_ADDR_PROTECTION_EN_OFFSET      0
+#define AXI0_ADDRESS_RANGE_ENABLE           8
+#define AXI0_RANGE_PROT_BITS_0_OFFSET       24
+#define RDLVL_EN_OFFSET                     16
+#define RDLVL_GATE_EN_OFFSET                24
+#define WRLVL_EN_OFFSET                     0
+
+#define PHY_RX_CAL_DQ0_0_OFFSET             0
+#define PHY_RX_CAL_DQ1_0_OFFSET             16
+
+struct fu540_ddrctl {
+	volatile u32 denali_ctl[265];
+};
+
+struct fu540_ddrphy {
+	volatile u32 denali_phy[1215];
+};
+
+struct fu540_sdram_params {
+	struct fu540_ddrctl pctl_regs;
+	struct fu540_ddrphy phy_regs;
+};
+
+struct sifive_dmc_plat {
+#if CONFIG_IS_ENABLED(OF_PLATDATA)
+	struct dtd_sifive_fu540_dmc dtplat;
+#else
+	struct fu540_sdram_params sdram_params;
+#endif
+};
+
+/**
+ * struct ddr_info
+ *
+ * @dev				: pointer for the device
+ * @info			: UCLASS RAM information
+ * @ctl				: DDR controleur base address
+ * @phy				: DDR PHY base address
+ * @ctrl			: DDR control base address
+ * @physical_filter_ctrl	: DDR physical filter control base address
+ */
+struct ddr_info {
+	struct udevice *dev;
+	struct ram_info info;
+	struct fu540_ddrctl *ctl;
+	struct fu540_ddrphy *phy;
+	u32 *physical_filter_ctrl;
+};