From: Reinhard Meyer (-VC) <reinhard.meyer@emk-elektronik.de>
Date: Fri, 04 Jun 2010 18:18:19 +0200
Subject: [U-Boot] [PATCH] add support for EMK TOP7000 CPU module
 (modified files)
In-Reply-To: <20100604123922.F2E2216068C@gemini.denx.de>
References: <4C08ED38.8030308@emk-elektronik.de>
	<20100604123922.F2E2216068C@gemini.denx.de>
Message-ID: <4C09274B.2080901@emk-elektronik.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Wolfgang Denk schrieb:
> Dear "Reinhard Meyer (-VC)",
>
> In message <4C08ED38.8030308@emk-elektronik.de> you wrote:
>   
>> This Patch adds support for the EMK TOP7000 CPU Module
>> Part 2: modified files
>>
>> Signed-off-by: Reinhard Meyer reinhard.meyer at emk-elektronik.de
>>
>>
>> ---
>>  MAINTAINERS               |    6 ++++-
>>  Makefile                  |    3 ++
>>  board/emk/common/vpd.c    |   57 
>> ++++++++++++++++++++++++++++++++++++++++----
>>     
>
> Fix your mailer, it is wrapping lines!
>   
I will investigate on that. Using Thunderbird, should have settings for 
that.
>   
>> @@ -928,6 +928,10 @@ Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
>>      ATSTK1006    AT32AP7000
>>      ATNGW100    AT32AP7000
>>  
>> +Reinhard Meyer <reinhard.meyer@emk-elektronik.de>
>>     
>
> Please keep lists sorted - M < S
>   
Well, what about that situation (two names as maintainers for HAMMERHEAD),
where do I sort in?

Mark Jackson <mpfj@mimc.co.uk>

    MIMC200            AT32AP7000

Alex Raimondi <alex.raimondi@miromico.ch>
Julien May <julien.may@miromico.ch>

    HAMMERHEAD        AT32AP7000

Reinhard Meyer <reinhard.meyer@emk-elektronik.de>

    TOP7000        AT32AP7000

Haavard Skinnemoen <haavard.skinnemoen@atmel.com>

>   
>> --- a/board/emk/common/vpd.c
>> +++ b/board/emk/common/vpd.c
>> @@ -1,6 +1,6 @@
>>  /*
>> - * (C) Copyright 2003
>> - * Reinhard Meyer, EMK Elektronik GmbH, r.meyer at emk-elektronik.de
>> + * (C) Copyright 2003-2010
>> + * Reinhard Meyer, EMK Elektronik
>>     
>
> Please add mail address
>   
I can do that. But is it really senseful to add an email address to each 
file?
E-Mail adresses can change. Also "Rechtsform" (GmbH -> GmbH & Co. KG).
Shall that all be reflected in a simple copyright string?
>   
>> @@ -68,12 +69,56 @@ void read_factory_r (void)
>>          addr += p - buf;
>>          /*printf ("%s\n", buf); */
>>          /* search for our specific entry */
>> -        if (!strncmp ((char *) buf, "[RLA/lan/Ethernet] ", 19)) {
>> +        if (!strncmp ((char *) buf, "[RLA/lan/Ethernet] ", 19))
>>              setenv ("ethaddr", (char *)(buf + 19));
>> -        } else if (!strncmp ((char *) buf, "[BOARD/SERIAL] ", 15)) {
>> +#if defined(CONFIG_TOP7000)
>> +        /* boards that have 2 LAN interfaces */
>> +        else if (!strncmp ((char *) buf, "[RLA/lan2/Ethernet] ", 20))
>> +            setenv ("eth1addr", (char *)(buf + 20));
>> +#endif
>>     
>
> Curly braces are needed for multiline statements.
>   
Not sure what is multiline here? its like
 if (cond1)
    statement1;
else if (cond2)
    statement2;
else if (cond3)
    statement3;
> ...
>   
>> +    if (eeprom_write (CONFIG_SYS_I2C_FACT_ADDR, CONFIG_SYS_FACT_OFFSET, 
>> buf, i+2)) {
>> +        printf ("cannot write factory configuration\n");
>>     
>
> All previous comments apply here, too (too long lines, wrapped lines,
> no spaces after function name, ...)
>
>   
willco.

Best Regards
Reinhard