| Message ID | 20200415090030.128489-6-ley.foon.tan@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ddr: altera: arria10: Convert SDRAM driver to DM | expand |
On 4/15/20 11:00 AM, Ley Foon Tan wrote: > Add call to get_ram_size() function to check memory range > for valid RAM. > > Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com> > --- > drivers/ddr/altera/sdram_arria10.c | 25 ++++++++++++++++++++++++- > 1 file changed, 24 insertions(+), 1 deletion(-) > > diff --git a/drivers/ddr/altera/sdram_arria10.c b/drivers/ddr/altera/sdram_arria10.c > index 6b74423ea789..e3f11984a978 100644 > --- a/drivers/ddr/altera/sdram_arria10.c > +++ b/drivers/ddr/altera/sdram_arria10.c > @@ -8,6 +8,7 @@ > #include <dm.h> > #include <errno.h> > #include <fdtdec.h> > +#include <hang.h> > #include <malloc.h> > #include <ram.h> > #include <reset.h> > @@ -17,7 +18,7 @@ > #include <asm/arch/fpga_manager.h> > #include <asm/arch/misc.h> > #include <dm/device_compat.h> > -#include <linux/kernel.h> > +#include <linux/sizes.h> > > #include "sdram_arria10.h" > > @@ -671,6 +672,27 @@ static int ddr_calibration_sequence(struct altera_sdram_platdata *plat) > return 0; > } > > +static void sdram_size_check(struct ram_info *ram) > +{ > + phys_size_t ram_check = 0; > + phys_size_t size = ram->size; > + phys_addr_t base = ram->base; > + > + debug("DDR: Running SDRAM size sanity check\n"); > + > + while (ram_check < size) { > + ram_check += get_ram_size((void *)(base + ram_check), > + (phys_size_t)SZ_1G); Why is it running in 1 GiB steps ?
> -----Original Message----- > From: Marek Vasut <marex at denx.de> > Sent: Wednesday, April 15, 2020 8:44 PM > To: Tan, Ley Foon <ley.foon.tan at intel.com>; u-boot at lists.denx.de > Cc: Ley Foon Tan <lftan.linux at gmail.com>; See, Chin Liang > <chin.liang.see at intel.com>; Simon Goldschmidt > <simon.k.r.goldschmidt at gmail.com>; Chee, Tien Fong > <tien.fong.chee at intel.com> > Subject: Re: [PATCH 5/7] ddr: altera: arria10: Add RAM size check > > On 4/15/20 11:00 AM, Ley Foon Tan wrote: > > Add call to get_ram_size() function to check memory range for valid > > RAM. > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com> > > --- > > drivers/ddr/altera/sdram_arria10.c | 25 ++++++++++++++++++++++++- > > 1 file changed, 24 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/ddr/altera/sdram_arria10.c > > b/drivers/ddr/altera/sdram_arria10.c > > index 6b74423ea789..e3f11984a978 100644 > > --- a/drivers/ddr/altera/sdram_arria10.c > > +++ b/drivers/ddr/altera/sdram_arria10.c > > @@ -8,6 +8,7 @@ > > #include <dm.h> > > #include <errno.h> > > #include <fdtdec.h> > > +#include <hang.h> > > #include <malloc.h> > > #include <ram.h> > > #include <reset.h> > > @@ -17,7 +18,7 @@ > > #include <asm/arch/fpga_manager.h> > > #include <asm/arch/misc.h> > > #include <dm/device_compat.h> > > -#include <linux/kernel.h> > > +#include <linux/sizes.h> > > > > #include "sdram_arria10.h" > > > > @@ -671,6 +672,27 @@ static int ddr_calibration_sequence(struct > altera_sdram_platdata *plat) > > return 0; > > } > > > > +static void sdram_size_check(struct ram_info *ram) { > > + phys_size_t ram_check = 0; > > + phys_size_t size = ram->size; > > + phys_addr_t base = ram->base; > > + > > + debug("DDR: Running SDRAM size sanity check\n"); > > + > > + while (ram_check < size) { > > + ram_check += get_ram_size((void *)(base + ram_check), > > + (phys_size_t)SZ_1G); > > Why is it running in 1 GiB steps ? Don't have any special reason. I'm following S10/Agilex's implementation. Do you prefer use smaller step size? Regards Ley Foon
On 4/16/20 3:34 AM, Tan, Ley Foon wrote: [...] >>> +static void sdram_size_check(struct ram_info *ram) { >>> + phys_size_t ram_check = 0; >>> + phys_size_t size = ram->size; >>> + phys_addr_t base = ram->base; >>> + >>> + debug("DDR: Running SDRAM size sanity check\n"); >>> + >>> + while (ram_check < size) { >>> + ram_check += get_ram_size((void *)(base + ram_check), >>> + (phys_size_t)SZ_1G); >> >> Why is it running in 1 GiB steps ? > Don't have any special reason. I'm following S10/Agilex's implementation. > Do you prefer use smaller step size? Rather, why doesn't it use the entire DRAM size, without the while loop?
> -----Original Message----- > From: Marek Vasut <marex at denx.de> > Sent: Thursday, April 16, 2020 4:52 PM > To: Tan, Ley Foon <ley.foon.tan at intel.com>; u-boot at lists.denx.de > Cc: Ley Foon Tan <lftan.linux at gmail.com>; See, Chin Liang > <chin.liang.see at intel.com>; Simon Goldschmidt > <simon.k.r.goldschmidt at gmail.com>; Chee, Tien Fong > <tien.fong.chee at intel.com> > Subject: Re: [PATCH 5/7] ddr: altera: arria10: Add RAM size check > > On 4/16/20 3:34 AM, Tan, Ley Foon wrote: > [...] > >>> +static void sdram_size_check(struct ram_info *ram) { > >>> + phys_size_t ram_check = 0; > >>> + phys_size_t size = ram->size; > >>> + phys_addr_t base = ram->base; > >>> + > >>> + debug("DDR: Running SDRAM size sanity check\n"); > >>> + > >>> + while (ram_check < size) { > >>> + ram_check += get_ram_size((void *)(base + ram_check), > >>> + (phys_size_t)SZ_1G); > >> > >> Why is it running in 1 GiB steps ? > > Don't have any special reason. I'm following S10/Agilex's implementation. > > Do you prefer use smaller step size? > > Rather, why doesn't it use the entire DRAM size, without the while loop? Yes, can change to entire DRAM size. Will change it. S10/Agilex have 2 banks of memory, that's why we have the while loop for 2 banks checking. But, A10 only have 1 bank. Regards Ley Foon
diff --git a/drivers/ddr/altera/sdram_arria10.c b/drivers/ddr/altera/sdram_arria10.c index 6b74423ea789..e3f11984a978 100644 --- a/drivers/ddr/altera/sdram_arria10.c +++ b/drivers/ddr/altera/sdram_arria10.c @@ -8,6 +8,7 @@ #include <dm.h> #include <errno.h> #include <fdtdec.h> +#include <hang.h> #include <malloc.h> #include <ram.h> #include <reset.h> @@ -17,7 +18,7 @@ #include <asm/arch/fpga_manager.h> #include <asm/arch/misc.h> #include <dm/device_compat.h> -#include <linux/kernel.h> +#include <linux/sizes.h> #include "sdram_arria10.h" @@ -671,6 +672,27 @@ static int ddr_calibration_sequence(struct altera_sdram_platdata *plat) return 0; } +static void sdram_size_check(struct ram_info *ram) +{ + phys_size_t ram_check = 0; + phys_size_t size = ram->size; + phys_addr_t base = ram->base; + + debug("DDR: Running SDRAM size sanity check\n"); + + while (ram_check < size) { + ram_check += get_ram_size((void *)(base + ram_check), + (phys_size_t)SZ_1G); + } + + if (ram_check != size) { + puts("DDR: SDRAM size check failed!\n"); + hang(); + } + + debug("DDR: SDRAM size check passed!\n"); +} + static int altera_sdram_ofdata_to_platdata(struct udevice *dev) { struct altera_sdram_platdata *plat = dev->platdata; @@ -715,6 +737,7 @@ static int altera_sdram_probe(struct udevice *dev) priv->info.base = gd->bd->bi_dram[0].start; priv->info.size = gd->ram_size; + sdram_size_check(&priv->info); return 0; failed:
Add call to get_ram_size() function to check memory range for valid RAM. Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com> --- drivers/ddr/altera/sdram_arria10.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)