Dear Ilko Iliev,
In message <[EMAIL PROTECTED]> you wrote:
> This patch adds support for the PM9263 board of Ronetix GmbH
> (www.ronetix.at)
>
> Signed-off-by: Ilko Iliev <[EMAIL PROTECTED]>
>
> ---
> MAKEALL | 1 +
> Makefile | 3 +
> board/ronetix/pm9263/Makefile | 60 +++++
> board/ronetix/pm9263/config.mk | 1 +
> board/ronetix/pm9263/led.c | 68 ++++++
> board/ronetix/pm9263/lowlevel_init.S | 375
> ++++++++++++++++++++++++++++++++
Ouch - line wrapped?
> board/ronetix/pm9263/partition.c | 47 ++++
> board/ronetix/pm9263/pm9263.c | 393
> ++++++++++++++++++++++++++++++++++
Ditto?
> --- a/MAKEALL
> +++ b/MAKEALL
> @@ -536,6 +536,7 @@ LIST_at91=" \
> at91sam9260ek \
> at91sam9261ek \
> at91sam9263ek \
> + pm9263 \
> at91sam9rlek \
> cmc_pu2 \
> csb637 \
Please keep lists sorted, and vertical alignment of the backslashes
intact.
> diff --git a/Makefile b/Makefile
> index 9055747..2772dd5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2493,6 +2493,9 @@ at91sam9261ek_config : unconfig
> at91sam9263ek_config : unconfig
> @$(MKCONFIG) $(@:_config=) arm arm926ejs at91sam9263ek atmel at91
>
> +pm9263_config : unconfig
> + @$(MKCONFIG) $(@:_config=) arm arm926ejs pm9263 ronetix at91
> +
Please keep lists sorted.
> at91sam9rlek_config : unconfig
> @$(MKCONFIG) $(@:_config=) arm arm926ejs at91sam9rlek atmel at91
>
...
> --- /dev/null
> +++ b/board/ronetix/pm9263/led.c
> @@ -0,0 +1,68 @@
...
> +void green_LED_off(void)
> +{
> + at91_set_gpio_value(GREEN_LED, 1);
> +}
> +
> +
> +
Too many empty lines. One should be sufficient here.
> diff --git a/board/ronetix/pm9263/lowlevel_init.S
> b/board/ronetix/pm9263/lowlevel_init.S
> new file mode 100644
> index 0000000..de240b7
> --- /dev/null
> +++ b/board/ronetix/pm9263/lowlevel_init.S
> @@ -0,0 +1,375 @@
...
> +#define PIOD_PDR_VAL1 0xFFFF0000 /* define PDC[31:16] as DATA[31:16] */
> +#define PIOD_PPUDR_VAL 0xFFFF0000 /* no pull-up for D[31:16] */
> +#define MATRIX_EBI0CSA_VAL 0x0001010A /* EBI0_CSA, CS1 SDRAM, CS3
> NAND Flash, 3.3V memories */
Maximum line length exceeded. Actually many lines are too long, but
here it is obvious.
> diff --git a/common/lcd.c b/common/lcd.c
> index 25f8664..9f81031 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -824,14 +824,23 @@ static void *lcd_logo (void)
>
> #ifdef CONFIG_ATMEL_LCD
> # ifdef CONFIG_LCD_INFO
> +
> sprintf (info, "%s", U_BOOT_VERSION);
Please do not change common files witout need, and without even
mentioning it!
> lcd_drawchars (LCD_INFO_X, LCD_INFO_Y, (uchar *)info, strlen(info));
>
> +#ifdef CONFIG_LCD_LOGO_TEXT1
> + sprintf (info, CONFIG_LCD_LOGO_TEXT1);
> +#else
> sprintf (info, "(C) 2008 ATMEL Corp");
> +#endif
> lcd_drawchars (LCD_INFO_X, LCD_INFO_Y + VIDEO_FONT_HEIGHT,
> (uchar *)info, strlen(info));
This change is unrelated to your board support. Please split it out
into a separate patch.
> +#ifdef CONFIG_LCD_LOGO_TEXT2
> + sprintf (info, CONFIG_LCD_LOGO_TEXT2);
> +#else
> sprintf (info, "[EMAIL PROTECTED]");
> +#endif
Ditto.
And instead of the #idfef's in the code, please consider something
like this:
#ifndef CONFIG_LCD_LOGO_TEXT2
# define CONFIG_LCD_LOGO_TEXT2 "[EMAIL PROTECTED]"
#endif
...
sprintf (info, CONFIG_LCD_LOGO_TEXT2);
Same for above.
> diff --git a/cpu/arm926ejs/at91/lowlevel_init.S
> b/cpu/arm926ejs/at91/lowlevel_init.S
> index ec6ad5d..7882e89 100644
> --- a/cpu/arm926ejs/at91/lowlevel_init.S
> +++ b/cpu/arm926ejs/at91/lowlevel_init.S
> @@ -27,7 +27,7 @@
> #include <config.h>
> #include <version.h>
>
> -#ifndef CONFIG_SKIP_LOWLEVEL_INIT
> +#if !defined(CONFIG_SKIP_LOWLEVEL_INIT) &&
> !defined(CONFIG_USER_LOWLEVEL_INIT)
This is unrelated to your board support, so please split it out into a
separate patch.
> diff --git a/drivers/mtd/dataflash.c b/drivers/mtd/dataflash.c
> index 049da69..e926b38 100644
> --- a/drivers/mtd/dataflash.c
> +++ b/drivers/mtd/dataflash.c
> @@ -130,8 +130,9 @@ int AT91F_DataflashInit (void)
> dfcode = 0;
> break;
> }
> +
> /* set the last area end to the dataflash size*/
Please don;t change common files without need.
> - area_list[NB_DATAFLASH_AREA -1].end =
> + dataflash_info[i].end_address =
This change is unrelated to you board support and affects all boards.
Modifications by this must explicitely mentioned in the description
and discussed.
> diff --git a/include/configs/pm9263.h b/include/configs/pm9263.h
> new file mode 100644
> index 0000000..1646e9f
> --- /dev/null
> +++ b/include/configs/pm9263.h
> @@ -0,0 +1,285 @@
...
> +/* NOR flash, if populated */
> +#if 0
Please do not add dead code.
> +#define CONFIG_IPADDR 192.168.3.222
> +#define CONFIG_GATEWAYIP 192.168.3.1
> +#define CONFIG_ETHADDR 02:DE:AD:BE:EF:01
> +#define CONFIG_NETMASK 255.255.255.0
> +#define CONFIG_SERVERIP 192.168.3.1
Please never encode network paramters in the default environment.
Delete this.
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: [EMAIL PROTECTED]
How many seconds are there in a year? If I tell you there are 3.155 x
10^7, you won't even try to remember it. On the other hand, who could
forget that, to within half a percent, pi seconds is a nanocentury.
-- Tom Duff, Bell Labs
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
