Hi Pragnesh, On Mon, Jul 20, 2020 at 5:28 PM Pragnesh Patel <[email protected]> wrote: > > Hi Bin, > > >-----Original Message----- > >From: Bin Meng <[email protected]> > >Sent: 20 July 2020 11:37 > >To: Rick Chen <[email protected]>; Pragnesh Patel > ><[email protected]>; Sagar Kadam <[email protected]>; U- > >Boot Mailing List <[email protected]> > >Cc: Bin Meng <[email protected]> > >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 <[email protected]> > > > >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 <[email protected]> > >--- > > > >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

