[U-Boot] [PATCH 1/8 V2] add at91 SoC access with c structures

[Tom]

Jens Scharsig wrote:
> * prepare board config files for Soc access update
> 

This review was cut short.
The review of this and patch #2 are fairly complete.

run kernel checkpatch.pl against the patches you submit.
In general errors must be fixed.
Warnings should be fixed, if appropriate.

There should also be no warnings/errors when git -am is used
on the patches.

The names of the patches should match what the patch is doing
Every patch is named the same but there is much more going
on in this patch set than just that.  It may be appropriate
to split this patchset up.

This patch is not bisectable.
There are config setting changes that do not happen until
later in the patch set.  It would seem like the first patch
would only set the CONFIG_LEGACY_AT91.  Mixed in are config
setting for gpio, nand, led and eth.

For large structural changes, the changes must be described
in detail in the commit log.

<snip>

> 
> diff --git a/include/configs/afeb9260.h b/include/configs/afeb9260.h
> index 58b8c8c..ddd5f52 100644
> --- a/include/configs/afeb9260.h
> +++ b/include/configs/afeb9260.h
> @@ -26,6 +26,8 @@
>  #ifndef __CONFIG_H
>  #define __CONFIG_H
>  
> +#define CONFIG_AT91_LEGACY
> +

The meaning of this should be described in detail in the
commit log. so other at91 developers can understand
how to migrate or what happened to their board

>  /* ARM asynchronous clock */
>  #define AT91_MAIN_CLOCK		18429952	/* from 18.432 MHz crystal */
>  #define CONFIG_SYS_HZ		1000
> @@ -45,6 +47,7 @@
>  /*
>   * Hardware drivers
>   */
> +#define CONFIG_AT91_GPIO	1

This is not used until patch 5/8 where new gpio functions are defined
Move this to that patch or later

>  #define CONFIG_ATMEL_USART	1
>  #undef CONFIG_USART0
>  #undef CONFIG_USART1
> @@ -107,6 +110,7 @@
>  #define CONFIG_SYS_NAND_ENABLE_PIN		AT91_PIN_PC14
>  #define CONFIG_SYS_NAND_READY_PIN		AT91_PIN_PC13
>  
> +#define CONFIG_SYS_64BIT_VSPRINTF		/* needed for nand_util.c */

This is a fix for a nand warning.
It an ok fix but belongs in its own patch

>  #endif
>  

<snip>
>  #define PHYS_SDRAM 0x20000000
>  #define PHYS_SDRAM_SIZE 0x2000000  /* 32 megs */
> @@ -122,7 +122,14 @@
>  #define CONFIG_SYS_MEMTEST_START		PHYS_SDRAM
>  #define CONFIG_SYS_MEMTEST_END			CONFIG_SYS_MEMTEST_START + PHYS_SDRAM_SIZE - 262144
>  
> -#define CONFIG_DRIVER_ETHER

This is an ethernet change that doesn't
happen until patch 7

> +#define CONFIG_NET_MULTI		1
> +#ifdef CONFIG_NET_MULTI
> +#define CONFIG_DRIVER_AT91EMAC		1

<snip>
> +
>  #define CONFIG_ATMEL_USART	1
>  #undef CONFIG_USART0
>  #undef CONFIG_USART1
> @@ -69,9 +71,9 @@
>  
>  /* LED */
>  #define CONFIG_AT91_LED
> -#define	CONFIG_RED_LED		AT91_PIN_PB7	/* this is the power led */
> -#define	CONFIG_GREEN_LED	AT91_PIN_PB8	/* this is the user1 led */
> -#define	CONFIG_YELLOW_LED	AT91_PIN_PC29	/* this is the user2 led */
> +#define	CONFIG_RED_LED		AT91_PORTPIN(B, 7)	/* the power led */
> +#define	CONFIG_GREEN_LED	AT91_PORTPIN(B, 8)	/* the user1 led */
> +#define	CONFIG_YELLOW_LED	AT91_PORTPIN(C, 29)	/* the user2 led */
>  

AT91_PORTPIN is not defined until patch 5/8 - part 2/3

>  #define CONFIG_BOOTDELAY	3
>  
> @@ -147,39 +149,36 @@
>  #ifndef CONFIG_SKIP_LOWLEVEL_INIT
>  #define MASTER_PLL_MUL		171
>  #define MASTER_PLL_DIV		14
> +#define MASTER_PLL_OUT		3

Why is this needed ?


Tom