[U-Boot] [PATCH V3 2/3] arm/davinci: spl - boot device selection

[Sughosh Ganu <urwithsughosh@gmail.com>]

On Tue Jul 10, 2012 at 11:20:53PM +0400, Mikhail Kshevetskiy wrote:
> On Wed, 11 Jul 2012 00:09:06 +0530
> Sughosh Ganu <urwithsughosh@gmail.com> wrote:
> > > diff --git a/arch/arm/cpu/arm926ejs/davinci/spl.c
> > > b/arch/arm/cpu/arm926ejs/davinci/spl.c index 74632e5..50b4bbc 100644
> > > --- a/arch/arm/cpu/arm926ejs/davinci/spl.c
> > > +++ b/arch/arm/cpu/arm926ejs/davinci/spl.c
> > 
> > <snip>
> > 
> > >  void board_init_r(gd_t *id, ulong dummy)
> > >  {
> > > -#ifdef CONFIG_SPL_NAND_LOAD
> > > -	nand_init();
> > > -	puts("Nand boot...\n");
> > > -	nand_boot();
> > > -#endif
> > > -#ifdef CONFIG_SPL_SPI_LOAD
> > > -	mem_malloc_init(CONFIG_SYS_TEXT_BASE - CONFIG_SYS_MALLOC_LEN,
> > > -			CONFIG_SYS_MALLOC_LEN);
> > > +	u32 boot_device;
> > > +	void (*uboot)(void) __noreturn;
> > > +
> > > +	mem_malloc_init(CONFIG_SYS_SPL_MALLOC_START,
> > > +			CONFIG_SYS_SPL_MALLOC_SIZE);
> > 
> > We do not need any heap for the spl on the hawkboard, so can you
> > please explain why do we need the heap allocation for all spl
> > images. Can't this be made configurable.
> 
> this is needed at least for:
>   * MMC support
>   * SPI support
>   * gunzip support (see next patch)
> 
> it can be configurable, but is it reasonable?

I would think so -- i guess it should be fine to include this only for
boards/configurations that actually need the heap.

> 
> > >  
> > >  	gd = &gdata;
> > >  	gd->bd = &bdata;
> > >  	gd->flags |= GD_FLG_RELOC;
> > > +#ifdef CONFIG_SPL_SERIAL_SUPPORT
> > >  	gd->baudrate = CONFIG_BAUDRATE;
> > > -	serial_init();          /* serial communications setup */
> > > +	serial_init();
> > >  	gd->have_console = 1;
> > > +#endif
> > 
> > This call to serial_init is not needed on the hawkboard -- we
> > initialise the console needed for spl through NS16550_init for all
> > da8xx based spl's in arch_cpu_init, which suffices for spl's debug
> > message requirement. I know this is being used for spi based spl
> > images, but now that this is being made common, just wanted to
> > know why we need this. Also, do these changes have any impact on the
> > size of the spl images.
> 
> hm, i'll look on it more carefully. I've just look on spi support in spl.
> 
> What about ymodem support? If I am not mistake it require a normal console.

If it's needed for ymodem support, include it for that use case -- i
see that your patch already has a separate cases for ymodem
support, spi flash support and so on. Can't it be called for cases
where it is really needed.

Btw, forgot to mention in my earlier mail that you need to add the
license header for all the new files, like spl_mmc.c etc.

-sughosh