[PATCH v2 2/2] ram: sifive: Avoid using hardcoded ram base and size

[Bin Meng]

Hi Pragnesh,

On Mon, Jul 20, 2020 at 5:28 PM Pragnesh Patel
<pragnesh.patel at sifive.com> wrote:
>
> Hi Bin,
>
> >-----Original Message-----
> >From: Bin Meng <bmeng.cn at gmail.com>
> >Sent: 20 July 2020 11:37
> >To: Rick Chen <rick at andestech.com>; Pragnesh Patel
> ><pragnesh.patel at sifive.com>; Sagar Kadam <sagar.kadam at sifive.com>; U-
> >Boot Mailing List <u-boot at lists.denx.de>
> >Cc: Bin Meng <bin.meng at windriver.com>
> >Subject: [PATCH v2 2/2] ram: sifive: Avoid using hardcoded ram base and size
> >
> >[External Email] Do not click links or attachments unless you recognize the
> >sender and know the content is safe
> >
> >From: Bin Meng <bin.meng at windriver.com>
> >
> >At present the SiFive FU540 RAM driver uses hard-coded memory base
> >address and size to initialize the DDR controller. This may not be true when
> >this driver is used on another board based on FU540.
> >
> >Update the driver to read the memory information from DT and use that
> >during the initialization.
> >
> >Signed-off-by: Bin Meng <bin.meng at windriver.com>
> >---
> >
> >Changes in v2:
> >- Change to use fdtdec_setup_mem_size_base() API
> >- Drop the 2 patches that added a new API in fdtdec
> >
> > drivers/ram/sifive/fu540_ddr.c | 30 +++++++++++++++---------------
> > 1 file changed, 15 insertions(+), 15 deletions(-)
> >
> >diff --git a/drivers/ram/sifive/fu540_ddr.c b/drivers/ram/sifive/fu540_ddr.c
> >index f8f8ca9..2eef1e7 100644
> >--- a/drivers/ram/sifive/fu540_ddr.c
> >+++ b/drivers/ram/sifive/fu540_ddr.c
> >@@ -8,6 +8,7 @@
> >
> > #include <common.h>
> > #include <dm.h>
> >+#include <fdtdec.h>
> > #include <init.h>
> > #include <ram.h>
> > #include <regmap.h>
> >@@ -39,9 +40,6 @@
> > #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
> >@@ -65,6 +63,8 @@
> > #define PHY_RX_CAL_DQ0_0_OFFSET                        0
> > #define PHY_RX_CAL_DQ1_0_OFFSET                        16
> >
> >+DECLARE_GLOBAL_DATA_PTR;
> >+
> > struct fu540_ddrctl {
> >        volatile u32 denali_ctl[265];
> > };
> >@@ -235,8 +235,8 @@ static int fu540_ddr_setup(struct udevice *dev)
> >        struct fu540_ddr_params *params = &plat->ddr_params;
> >        volatile u32 *denali_ctl =  priv->ctl->denali_ctl;
> >        volatile u32 *denali_phy =  priv->phy->denali_phy;
> >-       const u64 ddr_size = DDR_MEM_SIZE;
> >-       const u64 ddr_end = PAYLOAD_DEST + ddr_size;
> >+       const u64 ddr_size = priv->info.size;
> >+       const u64 ddr_end = priv->info.base + ddr_size;
> >        int ret, i;
> >        u32 physet;
> >
> >@@ -302,7 +302,7 @@ static int fu540_ddr_setup(struct udevice *dev)
> >                     | (1 << MULTIPLE_OUT_OF_RANGE_OFFSET));
> >
> >        /* set up range protection */
> >-       fu540_ddr_setup_range_protection(denali_ctl, DDR_MEM_SIZE);
> >+       fu540_ddr_setup_range_protection(denali_ctl, priv->info.size);
> >
> >        /* Mask off port command error interrupt DENALI_CTL_136 */
> >        setbits_le32(DENALI_CTL_136 + denali_ctl, @@ -314,14 +314,14 @@ static
> >int fu540_ddr_setup(struct udevice *dev)
> >
> >        /* check size */
> >        priv->info.size = get_ram_size((long *)priv->info.base,
> >-                                      DDR_MEM_SIZE);
> >+                                      ddr_size);
> >
> >        debug("%s : %lx\n", __func__, priv->info.size);
> >
> >        /* check memory access for all memory */
> >-       if (priv->info.size != DDR_MEM_SIZE) {
> >+       if (priv->info.size != ddr_size) {
> >                printf("DDR invalid size : 0x%lx, expected 0x%lx\n",
> >-                      priv->info.size, DDR_MEM_SIZE);
> >+                      priv->info.size, (uintptr_t)ddr_size);
> >                return -EINVAL;
> >        }
> >
> >@@ -333,6 +333,11 @@ static int fu540_ddr_probe(struct udevice *dev)  {
> >        struct fu540_ddr_info *priv = dev_get_priv(dev);
> >
> >+       /* Read memory base and size from DT */
> >+       fdtdec_setup_mem_size_base();
> >+       priv->info.base = gd->ram_base;
> >+       priv->info.size = gd->ram_size;
> >+
> > #if defined(CONFIG_SPL_BUILD)
> >        struct regmap *map;
> >        int ret;
> >@@ -368,14 +373,9 @@ static int fu540_ddr_probe(struct udevice *dev)
> >        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;
>
> This "else" part also need to be updated rather than removing it because it used by
> fu540_ddr_get_info() function in case of U-Boot.
>

It's already assigned at the beginning of this function. Could you
please recheck?

Regards,
Bin