[U-Boot] [PATCH v4 07/13] davinci: Use correct #ifdef around gdata/bdata

[Sughosh Ganu]

hi Christian,

On Mon Feb 27, 2012 at 12:37:16PM +0100, Christian Riesch wrote:
> Hi Sughosh,
> 
> On Mon, Feb 27, 2012 at 11:56 AM, Sughosh Ganu <urwithsughosh at gmail.com> wrote:
> > hi Christian,
> >
> > On Mon Feb 27, 2012 at 11:39:42AM +0100, Christian Riesch wrote:
> >> Hi,
> >>
> >> On Mon, Feb 27, 2012 at 11:16 AM, Sughosh Ganu <urwithsughosh at gmail.com> wrote:
> >
> > <snip>
> >
> >> >> >>  arch/arm/cpu/arm926ejs/davinci/spl.c |    2 ++
> >> >> >>  1 files changed, 2 insertions(+), 0 deletions(-)
> >> >> >>
> >> >> >> diff --git a/arch/arm/cpu/arm926ejs/davinci/spl.c b/arch/arm/cpu/arm926ejs/davinci/spl.c
> >> >> >> index b1eff26..2861907 100644
> >> >> >> --- a/arch/arm/cpu/arm926ejs/davinci/spl.c
> >> >> >> +++ b/arch/arm/cpu/arm926ejs/davinci/spl.c
> >> >> >> @@ -32,10 +32,12 @@
> >> >> >>
> >> >> >>  #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> >> >> >>
> >> >> >> +#ifdef CONFIG_SPL_SPI_LOAD
> >> >> >>  DECLARE_GLOBAL_DATA_PTR;
> >> >> >>  /* Define global data structure pointer to it*/
> >> >> >>  static gd_t gdata __attribute__ ((section(".data")));
> >> >> >>  static bd_t bdata __attribute__ ((section(".data")));
> >> >> >> +#endif
> >
> > <snip>
> >
> >> >> >  Can you specify which boards you get this warning for. With your
> >> >> >  patch to add libcommon to hawkboard's spl image, this is now also
> >> >> >  needed for hawkboard which uses CONFIG_SPL_NAND_LOAD.
> >>
> >> Simon's patch is for the hawkboard, since due to another patch in his
> >> patchset LIBCOMMON is enabled in hawkboard's SPL. Now we have a board
> >> that boots from NAND with SPL and has LIBCOMMON enabled (Simon, I did
> >> not check the rest of your patchset, why do you need LIBCOMMON on the
> >> hawkboard at all?)
> >
> >  LIBCOMMON is now needed as the generic relocation based functions
> >  are part of the libcommon.o, which are being enabled in the same
> >  patchset for all arm boards. So if i understand correct, all arm
> >  board based spl's now need libcommon and libgeneric.
> >
> >  The only thing i see is that libcommon and libgeneric are not
> >  defined for cam_enc_4xx board which uses spl, and this patchset does
> >  not add it either. Not sure whether it got missed.
> 
> When I asked Heiko Schocher a few month ago why he defined putc and
> puts in arch/arm/cpu/arm926ejs/davinci/spl.c he replied that he could
> not use LIBCOMMON due to size limitations for the SPL. So I guess that
> this board will not be able to use the generic relocation functions,
> unless the SPL is smaller than 16kB, right? Simon's patchset will
> break this board then, right?

  That is exactly what i reported in one of the threads in response to
  addition of libcommon and libgeneric to the hawkboard's spl. In
  fact, this might cause problems on quite a few boards with spl size
  restrictions. I am not sure, whether the generic relocation feature
  should be turned on by default on all boards or should be a config
  option -- at least for the spl builds. Another option would be to
  move it to a place where it is not needed to compile in the entire
  libcommon/libgeneric support that is not needed for the generic
  relocation code. I think that would help us keep the generic
  relocation without the size bloat that we see right now.

  http://lists.denx.de/pipermail/u-boot/2012-February/118567.html

<snip>

> >> void board_init_r(gd_t *id, ulong dummy)
> >> {
> >> #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> >>         mem_malloc_init(CONFIG_SYS_TEXT_BASE - CONFIG_SYS_MALLOC_LEN,
> >>                         CONFIG_SYS_MALLOC_LEN);
> >
> >  Can you please explain why we need the mem_malloc_init. I did not
> >  include this, and spl boots up just fine on my board.
> >
> 
> malloc is required for the SPI code only, so you could also put it
> within #ifdef CONFIG_SPL_SPI_LOAD

  Ok, i will move the common changes to a static function, and call
  it from both the nand and spi load cases. Or, should i wait till a
  consensus is drawn on whether to enable this feature for spl images
  also. In case this is not needed for spl, then we don't need this
  change.

-sughosh