Re: [U-Boot] [PATCH 01/20] keymile: rework headerfiles for keymile boards

2011-03-14 Thread Heiko Schocher
Hello Wolfgang,

Wolfgang Denk wrote:
> In message <1299591018-8944-2-git-send-email...@denx.de> you wrote:
>> - This patch reworks all headerfiles for keymile boards (coge, supx4,
>>   eter1, suen3).
>>   Furthermore, a refactoring on the whole environment variables has been
>>   acomplished.
> 
> Two independent changes => Please submit as two separate patches.

Ok, I try to split this in two patches.

> ...
>> +#define CONFIG_HUSH_INIT_VAR1
> ...
>> +#define CONFIG_I2C_MULTI_BUS1
> ...
>> +#define CONFIG_I2C_MUX  1
> 
> Defines like these that select functions only should not be assigned
> any numeric (or other value).  Please omit thse.  Please fix globally.

Ok.

>> +#define CONFIG_KM_DEF_ENV_CONSTANTS \
> ...
>> +"default="  \
>> +"setenv default \'run newenv; reset\' &&  " \
> 
> These backslashes are redundant at best.  Why not drop them?

Ok, drop it.

> ...
>> +"u-boot=" xstr(CONFIG_HOSTNAME) "/u-boot.bin \0"\
> 
> Are you sure you really want that trailing blank? [Please check
> globally.]

Good cathc, check this.

> ...
>> +#define CONFIG_SYS_MONITOR_LEN  (768 << 10) /* Reserve 
>> 768KB for Monitor */
> 
> Line too long.  Please fix globally.

Ok.

bye,
Heiko
-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 01/20] keymile: rework headerfiles for keymile boards

2011-03-13 Thread Wolfgang Denk
Dear Heiko Schocher,

In message <1299591018-8944-2-git-send-email...@denx.de> you wrote:
> - This patch reworks all headerfiles for keymile boards (coge, supx4,
>   eter1, suen3).
>   Furthermore, a refactoring on the whole environment variables has been
>   acomplished.

Two independent changes => Please submit as two separate patches.

...
> +#define CONFIG_HUSH_INIT_VAR 1
...
> +#define CONFIG_I2C_MULTI_BUS 1
...
> +#define CONFIG_I2C_MUX   1

Defines like these that select functions only should not be assigned
any numeric (or other value).  Please omit thse.  Please fix globally.

> +#define CONFIG_KM_DEF_ENV_CONSTANTS  \
...
> + "default="  \
> + "setenv default \'run newenv; reset\' &&  " \

These backslashes are redundant at best.  Why not drop them?

...
> + "u-boot=" xstr(CONFIG_HOSTNAME) "/u-boot.bin \0"\

Are you sure you really want that trailing blank? [Please check
globally.]

...
> +#define CONFIG_SYS_MONITOR_LEN   (768 << 10) /* Reserve 
> 768KB for Monitor */

Line too long.  Please fix globally.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
I object to intellect without discipline;  I object to power without
constructive purpose.
-- Spock, "The Squire of Gothos", stardate 2124.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot