Hi Bin, >-----Original Message----- >From: Bin Meng <[email protected]> >Sent: 20 July 2020 15:04 >To: Pragnesh Patel <[email protected]> >Cc: Bin Meng <[email protected]>; Sagar Kadam ><[email protected]>; Rick Chen <[email protected]>; U-Boot >Mailing List <[email protected]> >Subject: Re: [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 > >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?
Ohhh yes, my bad. Reviewed-by: Pragnesh Patel <[email protected]> > >Regards, >Bin
