On Thu, Apr 16, 2015 at 03:52:16PM +0200, Albert ARIBAUD wrote: > Hello Matt, > > On Tue, 14 Apr 2015 14:07:17 -0400, Matt Porter <mpor...@konsulko.com> > wrote: > > common/image.c currently implicitly depends on CONFIG_NR_DRAM_BANKS > > when CONFIG_ARM is enabled. Make this requirement explicit. > > > > Signed-off-by: Matt Porter <mpor...@konsulko.com> > > --- > > common/image.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/common/image.c b/common/image.c > > index 162b682..73c24f5 100644 > > --- a/common/image.c > > +++ b/common/image.c > > @@ -461,7 +461,7 @@ phys_size_t getenv_bootm_size(void) > > tmp = 0; > > > > > > -#if defined(CONFIG_ARM) > > +#if defined(CONFIG_ARM) && defined(CONFIG_NR_DRAM_BANKS) > > return gd->bd->bi_dram[0].size - tmp; > > #else > > return gd->bd->bi_memsize - tmp; > > I am not entirely fond of a symbol's existence conditioning some code > which does not actually use the symbol. I do understand the dependency > here -- that bi_dram[0] is meaningful only if CONFIG_NR_DRAM_BANKS is 1 > or more -- but then, why does this code not depend on the value of the > symbol? Makes me think the patch is not complete and the code should be > fixed to depend on the value of CONFIG_NR_DRAM_BANKS.
The problem is that CONFIG_NR_DRAM_BANKS means both "I have DRAM" and "I have X number of DRAM banks". In turn include/asm-generic/u-boot.h will only say we have gd->bd->bi_dram if CONFIG_NR_DRAM_BANKS is set so we can only reference that field when it's also set. Otherwise we get a compile error about no such member. There's a further argument (as I read and re-read getenv_bootm_size()) that getenv_bootm_clsize() should be cleaned-up / re-worked as it defaults to too large of a value (which is why stuff gets relocated "high" and then Linux doesn't see it and then people do initrd_high=0xffffffff and fdt_high=0xffffffff) but that's unrelated to this patch I think. -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
