[U-Boot] [PATCH 1/6] image: fix build when CONFIG_NR_DRAM_BANKS is disabled on ARM

[Tom Rini]

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 <mporter at 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 <mporter at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150416/c4648cab/attachment.sig>