From mboxrd@z Thu Jan 1 00:00:00 1970 From: Leo Liang Date: Fri, 17 Jul 2020 13:58:02 +0800 Subject: [PATCH 4/4] ram: sifive: Avoid using hardcoded ram base and size In-Reply-To: <1594869783-20189-4-git-send-email-bmeng.cn@gmail.com> References: <1594869783-20189-1-git-send-email-bmeng.cn@gmail.com> <1594869783-20189-4-git-send-email-bmeng.cn@gmail.com> Message-ID: <20200717055802.GA12072@andestech.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Bin, This whole patch set looks pretty good to me. Just out of curiosity and as being rather new to the u-boot community, would the following fix be more direct and avoid modifying general code? On Wed, Jul 15, 2020 at 08:23:03PM -0700, Bin Meng wrote: > From: Bin Meng > > 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 > --- > > drivers/ram/sifive/fu540_ddr.c | 28 +++++++++++++--------------- > 1 file changed, 13 insertions(+), 15 deletions(-) > > diff --git a/drivers/ram/sifive/fu540_ddr.c b/drivers/ram/sifive/fu540_ddr.c > index f8f8ca9..2f38023 100644 > --- a/drivers/ram/sifive/fu540_ddr.c > +++ b/drivers/ram/sifive/fu540_ddr.c > @@ -8,6 +8,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -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,9 @@ static int fu540_ddr_probe(struct udevice *dev) > { > struct fu540_ddr_info *priv = dev_get_priv(dev); > > + fdtdec_get_mem_size_base(gd->fdt_blob, (phys_size_t *)&priv->info.size, > + (unsigned long *)&priv->info.base); > + Instead of introducing new API, could we do something as such with the existing API? fdtdec_setup_mem_size_base(gd->blob); priv->info.base = gd->ram_base; priv->info.size = gd->ram_size; > #if defined(CONFIG_SPL_BUILD) > struct regmap *map; > int ret; > @@ -368,14 +371,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; > #endif > + > return 0; > } > > -- > 2.7.4 > Best regards, Leo