| Message ID | 1433462329-10680-7-git-send-email-osp@andrep.de |
|---|---|
| State | Rejected |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Andre, On 4 June 2015 at 17:58, Andre Przywara <osp@andrep.de> wrote: > From: Rob Herring <robh@kernel.org> > > Currently, u-boot will always fixup the DT memory node on ARM. If the dtb > has correct memory information, then we don't want or need u-boot to touch > the memory node. Allow platforms to skip this by not filling in dram bank > information. > > Signed-off-by: Rob Herring <robh@kernel.org> > Signed-off-by: Andre Przywara <osp@andrep.de> > --- > arch/arm/lib/board.c | 2 ++ > arch/arm/lib/bootm-fdt.c | 5 ++++- > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c > index 37ea6e9..a3c7315 100644 > --- a/arch/arm/lib/board.c > +++ b/arch/arm/lib/board.c I don't think we should be patching this file now - everything should be on generic board. > @@ -191,8 +191,10 @@ typedef int (init_fnc_t) (void); > > __weak void dram_init_banksize(void) > { > +#if CONFIG_NR_DRAM_BANKS > gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE; > gd->bd->bi_dram[0].size = gd->ram_size; > +#endif > } > > __weak int arch_cpu_init(void) [snip] Regards, Simon
On Fri, 5 Jun 2015 08:12:25 -0600 Simon Glass <sjg@chromium.org> wrote: Hi Simon, thanks for taking a look. > On 4 June 2015 at 17:58, Andre Przywara <osp@andrep.de> wrote: > > From: Rob Herring <robh@kernel.org> > > > > Currently, u-boot will always fixup the DT memory node on ARM. If > > the dtb has correct memory information, then we don't want or need > > u-boot to touch the memory node. Allow platforms to skip this by > > not filling in dram bank information. > > > > Signed-off-by: Rob Herring <robh@kernel.org> > > Signed-off-by: Andre Przywara <osp@andrep.de> > > --- > > arch/arm/lib/board.c | 2 ++ > > arch/arm/lib/bootm-fdt.c | 5 ++++- > > 2 files changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c > > index 37ea6e9..a3c7315 100644 > > --- a/arch/arm/lib/board.c > > +++ b/arch/arm/lib/board.c > > I don't think we should be patching this file now - everything should > be on generic board. Right, looking closer at this patch much of it is actually not necessary. Having CONFIG_NR_DRAM_BANKS set to 0 automatically skips a lot of the code already, so we don't need any extra gating. Hope that there are no compilers which complain about zero-sized arrays. The only thing that harms us is the final fdt_setprop(), which will _clear_ the reg property in the memory node in this case. So I replaced this patch by just skipping the reg property setting if the number of banks is 0 in common/fdt_support.c. From looking at all callers of fdt_fixup_memory_banks() this looks like a safe approach. Any objections? I will send the replaced patch with the next revision. Cheers, Andre. > > > @@ -191,8 +191,10 @@ typedef int (init_fnc_t) (void); > > > > __weak void dram_init_banksize(void) > > { > > +#if > > gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE; > > gd->bd->bi_dram[0].size = gd->ram_size; > > +#endif > > } > > > > __weak int arch_cpu_init(void) > > [snip] > > Regards, > Simon
Hi Andre, On 5 June 2015 at 18:14, Andre Przywara <osp@andrep.de> wrote: > On Fri, 5 Jun 2015 08:12:25 -0600 > Simon Glass <sjg@chromium.org> wrote: > > Hi Simon, > > thanks for taking a look. > >> On 4 June 2015 at 17:58, Andre Przywara <osp@andrep.de> wrote: >> > From: Rob Herring <robh@kernel.org> >> > >> > Currently, u-boot will always fixup the DT memory node on ARM. If >> > the dtb has correct memory information, then we don't want or need >> > u-boot to touch the memory node. Allow platforms to skip this by >> > not filling in dram bank information. >> > >> > Signed-off-by: Rob Herring <robh@kernel.org> >> > Signed-off-by: Andre Przywara <osp@andrep.de> >> > --- >> > arch/arm/lib/board.c | 2 ++ >> > arch/arm/lib/bootm-fdt.c | 5 ++++- >> > 2 files changed, 6 insertions(+), 1 deletion(-) >> > >> > diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c >> > index 37ea6e9..a3c7315 100644 >> > --- a/arch/arm/lib/board.c >> > +++ b/arch/arm/lib/board.c >> >> I don't think we should be patching this file now - everything should >> be on generic board. > > Right, looking closer at this patch much of it is actually not > necessary. Having CONFIG_NR_DRAM_BANKS set to 0 automatically skips a > lot of the code already, so we don't need any extra gating. > Hope that there are no compilers which complain about zero-sized arrays. > The only thing that harms us is the final fdt_setprop(), which will > _clear_ the reg property in the memory node in this case. > So I replaced this patch by just skipping the reg property setting if > the number of banks is 0 in common/fdt_support.c. > From looking at all callers of fdt_fixup_memory_banks() this looks like > a safe approach. Any objections? > I will send the replaced patch with the next revision. Should be OK - can you please add docs for that function to the header file? See fdt_fixup_display() for an example. > > Cheers, > Andre. > >> >> > @@ -191,8 +191,10 @@ typedef int (init_fnc_t) (void); >> > >> > __weak void dram_init_banksize(void) >> > { >> > +#if >> > gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE; >> > gd->bd->bi_dram[0].size = gd->ram_size; >> > +#endif >> > } >> > >> > __weak int arch_cpu_init(void) Regards, Simon
diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c index 37ea6e9..a3c7315 100644 --- a/arch/arm/lib/board.c +++ b/arch/arm/lib/board.c @@ -191,8 +191,10 @@ typedef int (init_fnc_t) (void); __weak void dram_init_banksize(void) { +#if CONFIG_NR_DRAM_BANKS gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE; gd->bd->bi_dram[0].size = gd->ram_size; +#endif } __weak int arch_cpu_init(void) diff --git a/arch/arm/lib/bootm-fdt.c b/arch/arm/lib/bootm-fdt.c index 7677358..dbf3d56 100644 --- a/arch/arm/lib/bootm-fdt.c +++ b/arch/arm/lib/bootm-fdt.c @@ -26,8 +26,10 @@ DECLARE_GLOBAL_DATA_PTR; int arch_fixup_fdt(void *blob) { + int ret = 0; +#if CONFIG_NR_DRAM_BANKS bd_t *bd = gd->bd; - int bank, ret; + int bank; u64 start[CONFIG_NR_DRAM_BANKS]; u64 size[CONFIG_NR_DRAM_BANKS]; @@ -42,6 +44,7 @@ int arch_fixup_fdt(void *blob) } ret = fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS); +#endif #ifdef CONFIG_ARMV7_NONSEC if (ret) return ret;