[U-Boot] [PATCH 5/5] imx: Add support for zmx25 board

[Matthias Weißer]

Am 30.06.2011 12:44, schrieb Wolfgang Denk:
> Dear Matthias Weisser,
>
> In message<1309427865-17531-6-git-send-email-weisserm at arcor.de>  you wrote:
>> zmx25 is a board based on imx25 SoC, 64 Megs of LPDDR, 32 Megs of NOR flash, an
>> optional NAND flash.
>>
>> Signed-off-by: Matthias Weisser<weisserm at arcor.de>
>> ---
>>   MAINTAINERS                        |    1 +
>>   board/syteco/zmx25/Makefile        |   51 +++++++++
>>   board/syteco/zmx25/lowlevel_init.S |  136 +++++++++++++++++++++++
>>   board/syteco/zmx25/zmx25.c         |  209 ++++++++++++++++++++++++++++++++++++
>>   boards.cfg                         |    1 +
>>   include/configs/zmx25.h            |  182 +++++++++++++++++++++++++++++++
>>   6 files changed, 580 insertions(+), 0 deletions(-)
>>   create mode 100644 board/syteco/zmx25/Makefile
>>   create mode 100644 board/syteco/zmx25/lowlevel_init.S
>>   create mode 100644 board/syteco/zmx25/zmx25.c
>>   create mode 100644 include/configs/zmx25.h
>
> checkpatch says:
>
> please, no space before tabs

I see. The checkpatch.pl I was using (0.28) didn't catch this. That one 
from latest kernel (0.31) does. So this will be fixed in v2.

> ...
>> diff --git a/board/syteco/zmx25/zmx25.c b/board/syteco/zmx25/zmx25.c
>> new file mode 100644
>> index 0000000..c27ad20
>> --- /dev/null
>> +++ b/board/syteco/zmx25/zmx25.c
>> @@ -0,0 +1,209 @@
>> +/*
>> + * (c) 2011 Graf-Syteco, Matthias Weisser
>> + *<weisserm at arcor.de>
>
> Are you absolutely sure that this code was wriiten by you from
> scratch, without re-using any existing code?

Well, actually not. But as the changes where that big (there are only 
two handful of identical lines without comments and common function 
names) that I thought that it will be the right way to do it so. That 
may be wrong. I (re)add the copyright from tx25.c in v2.

>> +void tx25_fec_init(void)
> ...
>> +#define tx25_fec_init()
>
> etc.
>
> This looks suspiciously as if the code was copied from the TX25 board,
> and then adapted.  If so then what gives you the right to claim
> exclusive ownership of this code and remove/omit oall existing
> copyright entries???

See above. Will (re)add. It was not my intention to offend someone.

> ...
>> +#define CONFIG_CMDLINE_TAG	1	/* enable passing of ATAGs	*/
>> +#define CONFIG_SETUP_MEMORY_TAGS 1
>> +#define CONFIG_INITRD_TAG	1
>> +#define BOARD_LATE_INIT		1
>
> Please omit values from #defines that enable features only.
>
>> +#define CONFIG_BOOTP_BOOTFILESIZE	1
>> +#define CONFIG_BOOTP_BOOTPATH		1
>> +#define CONFIG_BOOTP_GATEWAY		1
>> +#define CONFIG_BOOTP_HOSTNAME		1
>
> Ditto. Please fix globally.

OK

>> +#include<config_cmd_default.h>
>> +#define CONFIG_CMD_NET
>> +#define CONFIG_CMD_CACHE
>> +#undef CONFIG_CMD_FPGA
>> +#undef CONFIG_CMD_IMLS
>> +#undef CONFIG_CMD_LOADS
>> +#undef CONFIG_CMD_SOURCE
>> +#undef CONFIG_CMD_NFS
>> +#undef CONFIG_CMD_XIMG
>
> Is there any specific reason for sisabling these commands?
> You don't appear to be very short on resources, so this makes no sense
> to me.

Well, mainly copy+paste from our other board "jadecpu". I will remove 
these undefs in v2 as it only saves about 10k of image size, some of 
them may be useful and, as you have noticed, we are not short on flash.

Thanks for your review.

Matthias