[U-Boot-Users] [PATCH] add MPC8343 based board mvBlueLYNX-M7 aka mvblm7

[Kim Phillips]

On Wed, 09 Apr 2008 16:16:28 +0200
Andre Schwarz <andre.schwarz at matrix-vision.de> wrote:

> MPC8343 based stereo camera system with Cyclone-II FPGA and miniPCI Slot.
> CPU utilizes dual 10/100/1000 Ethernet using Vitesse VSC8601 RGMII Phys 
> and USB over ULPI.
> 512MB Micron DDR-II memory, 8MB Flash on LocalBus, SD over SPU and 32MB 
> NAND @ FPGA.
> 
> Signed-off-by: Andre Schwarz <andre.schwarz at matrix-vision.de>
> --

Wolfgang, I believe the window for new board support is closed; would
you rather I maintain this on a "next" branch on mpc83xx, or are you
willing to let this in for 1.3.3?

Andre, thanks for the new board contribution - please include diffstats
in your patches.

> diff --git a/CREDITS b/CREDITS
> index e84ef38..713f58a 100644
> --- a/CREDITS
> +++ b/CREDITS
> @@ -424,6 +424,11 @@ N: Paolo Scaffardi
>  E: arsenio at tin.it
>  D: FADS823 configuration, MPC823 video support, I2C, wireless keyboard, lots more
>  
> +N: Andre Schwarz
> +E: andre.schwarz at matrix-vision.de
> +D: Support for BlueLYNX and BlueCOUGAR series
> +W: www.matrix-vision.com
> +
>  N: Robert Schwebel
>  E: r.schwebel at pengutronix.de
>  D: Support for csb226, logodl and innokom boards (PXA2xx)
> diff --git a/MAKEALL b/MAKEALL
> index 2a872ac..f21c34e 100755
> --- a/MAKEALL
> +++ b/MAKEALL
> @@ -327,6 +327,7 @@ LIST_83xx="		\
>  	MPC8360ERDK_66	\
>  	MPC837XEMDS	\
>  	MPC837XERDB	\
> +	MVBLM7		\
>  	sbc8349		\
>  	TQM834x		\
>  "
> diff --git a/Makefile b/Makefile
> index a7f886b..3f217fe 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2084,6 +2084,9 @@ sbc8349_config:		unconfig
>  TQM834x_config:	unconfig
>  	@$(MKCONFIG) $(@:_config=) ppc mpc83xx tqm834x
>  
> +MVBLM7_config: unconfig
> +	@$(MKCONFIG) $(@:_config=) ppc mpc83xx mvblm7
> +

I believe intra-family targets are kept in alpha order in the Makefile
(as you did in MAKEALL).

please add a MAINTAINERS entry. A doc/README.mvblm7 would be nice.

<snip>
> diff --git a/board/mvblm7/config.mk b/board/mvblm7/config.mk
> new file mode 100644
> index 0000000..a659203
> --- /dev/null
> +++ b/board/mvblm7/config.mk
<snip>
> +#
> +# MPC8349E-mITX and MPC8349E-mITX-GP
> +#

I'd leave this out :)

> +
> +sinclude $(OBJTREE)/board/$(BOARDDIR)/config.tmp
> +
> +TEXT_BASE  = 0xFFF00000
> diff --git a/board/mvblm7/fpga.c b/board/mvblm7/fpga.c
> new file mode 100644
> index 0000000..d286ef1
> --- /dev/null
> +++ b/board/mvblm7/fpga.c
<snip>
> +#if (CONFIG_FPGA)

#if defined?

<snip>
> +int mvblm7_init_fpga (void)

we don't put spaces between function name and open parenthesis, do we?

All your functions are like this.

> +{
> +	DECLARE_GLOBAL_DATA_PTR;

this needs to be put somewhere global.

> +
> +	fpga_debug("%s:%d: Initialize FPGA interface (relocation offset = 0x%.8lx)\n",
> +		__FUNCTION__, __LINE__, gd->reloc_off);
> +	fpga_init (gd->reloc_off);
> +
> +	fpga_debug("%s:%d: Adding fpga 0\n", __FUNCTION__, __LINE__);
> +	fpga_add (fpga_altera, &cyclone2);
> +	return 1;
> +}
> +
> +int fpga_null_fn (int cookie)
> +{
> +        return 0;
> +}
> +
> +int fpga_config_fn (int assert, int flush, int cookie)
> +{
> +        volatile immap_t        *im = (volatile immap_t *)CFG_IMMR;
> +        volatile gpio83xx_t	*gpio = (volatile gpio83xx_t *)&im->gpio[0];

fix indentation; use tabs, not spaces.

> +
> +	u32 dvo = gpio->dat;

keep declarations together, leave blank space between declarations and
code - i.e, mv above blank line to before this line:

> +	fpga_debug("SET config : %s\n", assert ? "low" : "high");
> +	if ( assert ) 

no spaces around assert

> +		dvo |= FPGA_CONFIG;
> +	else 
> +		dvo &= ~FPGA_CONFIG;
> +	
> +	if ( flush )
> +		gpio->dat = dvo;
> +	return assert;

can you also get in the habit of leaving blank lines before return
statements?  thanks.

<snipped more code that gets above comments also>

> +	for ( i=0; i<8; i++ ) {

please read CodingStyle!  this should look like:

for (i = 0; i < 8; i++) {

<snip similar cases>

> diff --git a/board/mvblm7/mvblm7.c b/board/mvblm7/mvblm7.c
<snip>
> +        im->ddr.sdram_interval = CFG_DDR_INTERVAL;
> +        im->ddr.sdram_clk_cntl = CFG_DDR_CLK_CNTL;

use tabs, not spaces.

<snip>
> +u8 *dhcp_vendorex_prep (u8 * e)

u8 *dhcp_vendorex_prep(u8 *e)

> +{
> +    char *ptr;
> +
> +/* DHCP vendor-class-identifier = 60 */
> +    if ((ptr = getenv ("dhcp_vendor-class-identifier"))) {

fix indentation.  Use tabs, not spaces.

> +        *e++ = 60;
> +        *e++ = strlen (ptr);

strlen(ptr);

> +        while (*ptr)
> +            *e++ = *ptr++;
> +    }
> +/* DHCP_CLIENT_IDENTIFIER = 61 */
> +    if ((ptr = getenv ("dhcp_client_id"))) {

..

<snip>
> diff --git a/board/mvblm7/pci.c b/board/mvblm7/pci.c
<sniiiiip>
> +			tmp[0] = cpu_to_be32(gd->pci_clk);
> +			do_fixup_by_path(blob, path, "clock-frequency",
> +				&tmp, sizeof(tmp[0]), 1);
> +		}
> +	}
> +}
> +#endif
 
please use generic 83xx pci code (cpu/mpc83xx/pci.c).  See the
mpc8313erdb board implementation for an example use-case.

Again, thanks for your contribution!

Kim