[U-Boot] [PATCH] p2020ds: add alternate boot bank support using the ngPIXIS FPGA

[Wolfgang Denk]

Dear Timur Tabi,

In message <1269296309-23976-1-git-send-email-timur at freescale.com> you wrote:
> The Freescale P2020DS board uses a new type of PIXIS FPGA, called the ngPIXIS.
> The ngPIXIS has one distinct new feature: the values of the on-board switches
> can be selectively overridden with shadow registers.  This feature is used to
> boot from a different NOR flash bank, instead of having a register dedicated
> for this purpose.  Because the ngPIXIS is so different from the previous PIXIS,
> a new file is introduced: ngpixis.c.
...
> --- /dev/null
> +++ b/board/freescale/common/ngpixis.c
> @@ -0,0 +1,136 @@
> +/**
> + * Copyright 2010 Freescale Semiconductor
> + * Author: Timur Tabi <timur at freescale.com>
> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2.  This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.

Please make this "either version 2 of the License, or (at your
option) any later version'.

> + * PIXIS_BASE - The virtual address of the base of the PIXIS register map

Please use a C struct to desctibe the register map.

> +void pixis_reset(void)
> +{
> +	out8(PIXIS_BASE + PIXIS_RST, 0);
> +}
> +
> +/*
> + * Reset the board.  Like pixis_reset(), but it honors the ENx registers.
> + */
> +void pixis_bank_reset(void)
> +{
> +	out8(PIXIS_BASE + PIXIS_VCTL, 0);
> +	out8(PIXIS_BASE + PIXIS_VCTL, 1);

NAK. We don't accept base + offset. Please use a C struct. Please fix
globally.


> +	/* Tell the ngPIXIS to use this the bits in the physical switch for the
> +	 * boot bank value, instead of the SWx register.  We need to be careful
> +	 * only to set the bits in SWx that correspond to the boot bank.
> +	 */

Incorrect multiline comment style. Please fix globally.


> +	/*
> +	 * No args is a simple reset request.
> +	 */
> +	if (argc <= 1)
> +		pixis_reset();
> +		/* not reached */

Braces needed here.

> +U_BOOT_CMD(
> +	pixis_reset, CONFIG_SYS_MAXARGS, 1, pixis_reset_cmd,
> +	"Reset the board using the FPGA sequencer",
> +	"    pixis_reset\n"
> +	"    pixis_reset [altbank]\n"

Please check the message format. I think there is one too many newline
here.

> -#define CONFIG_FSL_PIXIS	1	/* use common PIXIS code */
> +#define CONFIG_FSL_NGPIXIS             /* use common ngPIXIS code */

Please use TABs for vertical alignment. 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: wd at denx.de
"Though a program be but three lines long,
someday it will have to be maintained."
- The Tao of Programming