[U-Boot] [PATCHv1] ARM: Add Altera SOCFPGA Cyclone5

[Tom Rini <trini@ti.com>]

On 08/29/2012 04:21 PM, Pavel Machek wrote:
> Hi!
> 
>>>>> 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.
>>>
>>> Hmm, I tried, but I don't know socfpga memory layout by heart.
>>>
>>> Dinh, can you help here?
>>
>> I think once you get the answers you should be able to re-post the
>> series cleanly and depend on my v5 (or v6) branch.  Thanks!
> 
> Ok, figuring the .lds might be tricky. But... that's mostly additional
> (safety) feature. Would it be possible to merge it without that and
> add it later?
> 
> Port to v5 of the patch should not be a problem.
> 
> For the record and review, current version of patch is here.

A few small comments.  No need to re-post intermediate patches, just
address in the next full go-round.  Thanks!

> diff --git a/arch/arm/cpu/armv7/socfpga/spl.c b/arch/arm/cpu/armv7/socfpga/spl.c
[snip]
> +/*
> + * Board initialization after bss clearance
> + */
> +void spl_board_init(void)
[snip]
> +	puts("SPL Boot\n");

Sure you want to keep that line?  We already have a general "U-Boot SPL
..." line being printed.

[snip]
> diff --git a/arch/arm/include/asm/arch-socfpga/spl.h b/arch/arm/include/asm/arch-socfpga/spl.h
[snip]
> +#define CONFIG_SPL_STACK (&__stack_start)
> +#define CONFIG_SYS_SPL_MALLOC_START (&__malloc_start)
> +#define CONFIG_SYS_SPL_MALLOC_SIZE (&__malloc_end - &__malloc_start)

These should be in the config file.

> diff --git a/board/altera/socfpga_cyclone5/socfpga_cyclone5.c b/board/altera/socfpga_cyclone5/socfpga_cyclone5.c
[snip]
> +/*
> + * miscellaneous platform dependent initialisations
> + */
> +int misc_init_r(void)
> +{
> +	/* Set to "n" for not verifying the uImage */
> +	setenv("verify", "n");
> +	return 0;
> +}

Not good to enforce policy like this.  Just don't set it in the config
file by default and let users do as they need.

[snip]
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index eaea1c8..5adbf0e 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -78,6 +78,7 @@ void spl_parse_image_header(const struct image_header *header)
>  	u32 header_size = sizeof(struct image_header);
>  
>  	if (__be32_to_cpu(header->ih_magic) == IH_MAGIC) {
> +		/* Valid image. Extract information out of header */
>  		spl_image.size = __be32_to_cpu(header->ih_size) + header_size;
>  		spl_image.entry_point = __be32_to_cpu(header->ih_load);
>  		/* Load including the header */

Just an extra comment, drop please.  Or split out if you feel it's
really helpful.  No strong opinion here other than not in the same patch
as the rest.

[snip]
> diff --git a/include/common.h b/include/common.h
> index 55025c0..469a3af 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -704,7 +704,9 @@ void	external_interrupt (struct pt_regs *);
>  void	irq_install_handler(int, interrupt_handler_t *, void *);
>  void	irq_free_handler   (int);
>  void	reset_timer	   (void);
> -ulong	get_timer	   (ulong base);
> +ulong	get_timer(ulong base);
> +void	reset_timer_count(void);
> +ulong	get_timer_count(ulong base);
>  void	enable_interrupts  (void);
>  int	disable_interrupts (void);

Make the new declarations match the old, spacing wise please.

>  
> diff --git a/include/configs/socfpga_cyclone5.h b/include/configs/socfpga_cyclone5.h
> new file mode 100644
> index 0000000..d001cbb
> --- /dev/null
> +++ b/include/configs/socfpga_cyclone5.h
[snip]
> +#undef CONFIG_USE_IRQ
[snip]
> +
> +/*
> + * Stack sizes
> + * The stack sizes are set up in start.S using the settings below
> + */
> +/* regular stack */
> +#define CONFIG_STACKSIZE		(128 << 10)
> +#ifdef CONFIG_USE_IRQ
> +/* IRQ stack */
> +#define CONFIG_STACKSIZE_IRQ		(4 << 10)
> +/* FIQ stack */
> +#define CONFIG_STACKSIZE_FIQ		(4 << 10)
> +#endif

Unused, please drop.

[snip]
> +#define CONFIG_SYS_PROMPT_HUSH_PS2	"> "

This is the default now, can be dropped.

-- 
Tom