From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bin Meng Date: Fri, 17 Jul 2020 14:09:10 +0800 Subject: [PATCH 4/4] ram: sifive: Avoid using hardcoded ram base and size In-Reply-To: <20200717055802.GA12072@andestech.com> References: <1594869783-20189-1-git-send-email-bmeng.cn@gmail.com> <1594869783-20189-4-git-send-email-bmeng.cn@gmail.com> <20200717055802.GA12072@andestech.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Leo, On Fri, Jul 17, 2020 at 1:58 PM Leo Liang wrote: > > 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; Yes, I think that works too. Maybe it's not worth introducing a new API in fdtdec. > > > #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; > > } > > Regards, Bin