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

[André Schwarz]

Kim,

I'll fix all your issues mentioned below in the next days.
We're not in a hurry. Getting the board into 1.3.3 is absolutely fine.

Thanks,
André

Kim Phillips wrote:
> 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
>   



MATRIX VISION GmbH, Talstraße 16, DE-71570 Oppenweiler  - Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschäftsführer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.denx.de/pipermail/u-boot/attachments/20080410/8b954151/attachment.htm