On 08/25/2012 05:05 AM, Pavel Machek wrote:
> Hi!
> 
>> [snip]
>>>> It's the omap-common/spl.c code with the bits for IH_OS_LINUX taken out
>>>> :)  Can you please re-do your series on top of the SPL series I just
>>>> posted that provides a common SPL framework?  Thanks!
>>>
>>> I'll take a look. OTOH, ammount of code duplication here is quite low,
>>> and redoing it on top of SPL series will mean delaying merge, right?
>>
>> My hope is that since I got the SPL patches posted in time we can get
>> them in for v2012.10 so no, this will only help your chances :)
> 
> Ok, this is promissed patch for comments. socfpga spl.c now got
> reasonably smaller. It is on top of 

Looks good overall.  I don't see asm/arch/spl.h but that's where you
could put the extern for __stack_start and then define CONFIG_SPL_STACK
to &__stack_start.

> http://github.com/trini/u-boot WIP/spl-improvements
> 
> Now... can I leave spl_ram_load_image() where it is? Probably add some
> #ifdef?

Yeah, I think that's OK to start with, we can move things more later if
needed.

> 
> Dinh, does this look acceptable to you? Could we switch to absolute
> addresses for stack/heap in the SPL?
[snip]
> +/*
> + * Console Initialization
> + */
> +static void spl_console_init(void)
> +{
> +     gd->flags |= GD_FLG_RELOC;
> +     gd->baudrate = CONFIG_BAUDRATE;
> +
> +     /* serial communications setup */
> +     serial_init();
> +     gd->have_console = 1;
> +}

Looks like preloader_console_init :)

> +
> +u32 spl_boot_device(void)
> +{
> +     return BOOT_DEVICE_RAM;
> +}
> +
> +/*
> + * Board initialization after bss clearance
> + */
> +void spl_board_init(void)
> +{
> +     /* init timer for enabling delay function */
> +     timer_init();
> +
> +     /* de-assert reset for peripherals and bridges based on handoff */
> +     reset_deassert_peripherals_handoff();
> +
> +     /* enable console uart printing */
> +     spl_console_init();
> +
> +     /* setup MALLOC after clocks going faster */
> +     mem_malloc_init((ulong) &__malloc_start,
> +                     (&__malloc_end - &__malloc_start));

Shouldn't need this, should already be called before you get to
spl_board_init.

[snip]
> diff --git a/arch/arm/cpu/armv7/socfpga/u-boot-spl.lds 
> b/arch/arm/cpu/armv7/socfpga/u-boot-spl.lds

You should setup MEMORY declarations like the other u-boot-spl linker
scripts do so we get build-time confirmation that we haven't exceeded
our size limitations.

> +     . = . + 4;
> +     . = ALIGN(8);
> +     __malloc_start = .;
> +     . = . + CONFIG_SPL_MALLOC_SIZE;
> +     __malloc_end = .;
> +
> +     . = . + 4;
> +     . = . + CONFIG_SPL_STACK_SIZE;
> +     . = ALIGN(8);
> +     __stack_start = .;

Do you really need to do . = . + 4 and then align statements?

[snip]
> diff --git a/spl/Makefile b/spl/Makefile
> index f96c08e4..d757448 100644
> --- a/spl/Makefile
> +++ b/spl/Makefile
> @@ -58,6 +58,7 @@ LIBS-$(CONFIG_SPL_NAND_SUPPORT) += 
> drivers/mtd/nand/libnand.o
>  LIBS-$(CONFIG_SPL_ONENAND_SUPPORT) += drivers/mtd/onenand/libonenand.o
>  LIBS-$(CONFIG_SPL_DMA_SUPPORT) += drivers/dma/libdma.o
>  LIBS-$(CONFIG_SPL_POST_MEM_SUPPORT) += post/drivers/memory.o
> +LIBS-$(CONFIG_SPL_LIBGENERIC_SUPPORT) += lib/libgeneric.o
>  
>  ifneq 
> ($(CONFIG_AM33XX)$(CONFIG_OMAP34XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX),)
>  LIBS-y += $(CPUDIR)/omap-common/libomap-common.o
> 

Er?  There's already a line for lib/libgeneric.o

> 

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to