[U-Boot] [PATCH] Add support for NetusG20

[Claudio Mignanti]

Dear Wolfgang Denk,
thanks for your review.

2010/7/5 Wolfgang Denk <wd at denx.de>:
> Dear Claudio Mignanti,
>
> In message <1277651361-26448-1-git-send-email-c.mignanti at gmail.com> you wrote:
>> Add support for the NetusG20 board by Acmesystems srl.
>> This board is based on AT91SAM9G20 SoC.
>>
>> Signed-off-by: Claudio Mignanti <c.mignanti at gmail.com>
>> ---
>>  MAKEALL                                           |    1 +
>>  Makefile                                          |    3 +
>>  arch/arm/cpu/arm926ejs/at91/at91sam9260_devices.c |    5 +
>>  board/acmesystems/netusg20/Makefile               |   56 +++++++
>>  board/acmesystems/netusg20/config.mk              |    1 +
>>  board/acmesystems/netusg20/led.c                  |   40 +++++
>>  board/acmesystems/netusg20/netusg20.c             |  152 +++++++++++++++++
>>  board/acmesystems/netusg20/partition.c            |   39 +++++
>>  include/configs/netusg20.h                        |  181 +++++++++++++++++++++
>>  9 files changed, 478 insertions(+), 0 deletions(-)
>>  create mode 100644 board/acmesystems/netusg20/Makefile
>>  create mode 100644 board/acmesystems/netusg20/config.mk
>>  create mode 100644 board/acmesystems/netusg20/led.c
>>  create mode 100644 board/acmesystems/netusg20/netusg20.c
>>  create mode 100644 board/acmesystems/netusg20/partition.c
>>  create mode 100644 include/configs/netusg20.h
>
> Entry to MAINTAINERS missing.

Ok

>
>> diff --git a/Makefile b/Makefile
>> index 87d5214..b73659f 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -2867,6 +2867,9 @@ at91sam9g45ekes_config  :       unconfig
>>       fi;
>>       @$(MKCONFIG) -a at91sam9m10g45ek arm arm926ejs at91sam9m10g45ek atmel at91
>>
>> +netusg20_config:     unconfig
>> +     @$(MKCONFIG) $(@:_config=) arm arm926ejs netusg20 acmesystems at91
>> +
>>  otc570_config        :       unconfig
>>       @$(MKCONFIG) $(@:_config=) arm arm926ejs otc570 esd at91
>
> NAK. Please rebase your patch against current code. We don't add
> boards to the top level Makefile any more. Add the definition to
> boards.cfg instead.
>
>> diff --git a/arch/arm/cpu/arm926ejs/at91/at91sam9260_devices.c b/arch/arm/cpu/arm926ejs/at91/at91sam9260_devices.c
>> index 77d49ab..87ec531 100644
>> --- a/arch/arm/cpu/arm926ejs/at91/at91sam9260_devices.c
>> +++ b/arch/arm/cpu/arm926ejs/at91/at91sam9260_devices.c
>> @@ -59,7 +59,12 @@ void at91_serial3_hw_init(void)
>>  {
>>       at91_pmc_t      *pmc    = (at91_pmc_t *) AT91_PMC_BASE;
>>
>> +#ifdef CONFIG_NETUSG20
>> +     /* pull-up active on DRXD*/
>> +     at91_set_a_periph(AT91_PIO_PORTB, 14, 1);
>> +#else
>>       at91_set_a_periph(AT91_PIO_PORTB, 14, 0);               /* DRXD */
>> +#endif
>>       at91_set_a_periph(AT91_PIO_PORTB, 15, 1);               /* DTXD */
>>       writel(1 << AT91_ID_SYS, &pmc->pcer);
>>  }
>
> Please do not add board specific defines to common code. If really
> needed, add a feature-specific #define.
>

Something like this is better?

	/* DRXD */
	at91_set_a_periph(AT91_PIO_PORTB, 14, CONFIG_AT91SAM9260_DRXD_PULLUP);
	at91_set_a_periph(AT91_PIO_PORTB, 15, 1);		/* DTXD */


> ...
>> +#ifdef CONFIG_RESET_PHY_R
>> +void reset_phy(void)
>> +{
>> +#ifdef CONFIG_MACB
>> +     /*
>> +      * Initialize ethernet HW addr prior to starting Linux,
>> +      * needed for nfsroot
>> +      */
>> +     eth_init(gd->bd);
>> +#endif
>> +}
>> +#endif
>
> This doesn't look right to me.

This part was copied from board/atmel/at91sam9260ek/at91sam9260ek.c

>> +/* LED */
>> +#define CONFIG_AT91_LED
>> +#define      CONFIG_RED_LED          AT91_PIN_PA9    /* this is the power led */
>> +#define      CONFIG_GREEN_LED        AT91_PIN_PA6    /* this is the user led */
>
> Please use consistent code. Either ALWAYS use a TAB after a #define,
> or (better) always use a SPACE.

ops sorry I will be more consistent

Best Regards