[U-Boot] [PATCH v2] powerpc, 8xx: Add support for MCR3000 board from CSSI

[Christophe LEROY]

Dear Wolfgang,

Le 06/07/2017 à 13:12, Wolfgang Denk a écrit :
> Dear Christophe,
> 
> In message <20170706083955.0D92F69745 at pc13941vm.idsi0.si.c-s.fr> you wrote:
>>
>> This patch adds support for the first board.
> ...
> 
>> diff --git a/board/cssi/MCR3000/u-boot.lds b/board/cssi/MCR3000/u-boot.lds
>> new file mode 100644
>> index 0000000000..2234bd8d1d
>> --- /dev/null
>> +++ b/board/cssi/MCR3000/u-boot.lds
> ...
>> +OUTPUT_ARCH(powerpc)
>> +SECTIONS
>> +{
>> +  /* Read-only sections, merged into text segment: */
>> +  . = + SIZEOF_HEADERS;
>> +  .text          :
>> +  {
> ...
> 
> Like with C code, TABs should be used for indentation.

Ok

> 
> 
> ...
>> diff --git a/include/configs/MCR3000.h b/include/configs/MCR3000.h
>> new file mode 100644
>> index 0000000000..53d4e9468a
>> --- /dev/null
>> +++ b/include/configs/MCR3000.h
> ...
>> +#define CONFIG_IPADDR			192.168.0.3
>> +#define CONFIG_SERVERIP			192.168.0.1
>> +#define CONFIG_NETMASK			255.0.0.0
> 
> Static network configuration in board config files is strongly
> discouraged.  Please remove.

But with no IP CONFIG, the board won't boot properly.
I propose to leave it for the time being (there is plenty in other board 
configs) and add that in my TODO list.

> 
>> +#define CONFIG_SYS_BAUDRATE_TABLE	{9600, 19200, 38400, 57600, 115200}
> 
> Can you not use the standard baudrate table?

Yes indeed, it is the same. I fix it.

> 
> 
>> +/*-----------------------------------------------------------------------
>> + * Physical memory map of the MCR3000 board
>> + */
> 
> Nitpick: illegal multiline comment style.  Please fix globally.
> 
> 
> Reviewed-by: Wolfgang Denk <wd at denx.de>
> 
> Best regards,
> 
> Wolfgang Denk
> 

Thanks for your review
Regards
Christophe