[U-Boot] [PATCH 4/4] MergerBox: add board specific files in vendor dir.

[Wolfgang Denk]

Dear Andre Schwarz,

In message <1302786707-15359-5-git-send-email-andre.schwarz at matrix-vision.de> you wrote:
> 
> Signed-off-by: Andre Schwarz <andre.schwarz at matrix-vision.de>

There is a ton ot checkpatch warnings and errors. Please ix these.

...
> +int fpga_config_fn(int assert, int flush, int cookie)
> +{
> +	volatile immap_t *im = (immap_t *)CONFIG_SYS_IMMR;
> +	volatile gpio83xx_t *gpio = (gpio83xx_t *)&im->gpio[0];
> +	u32 dvo = gpio->dat;
> +
> +	dvo &= ~FPGA_CONFIG;
> +	gpio->dat = dvo;
> +	udelay(5);
> +	dvo |= FPGA_CONFIG;
> +	gpio->dat = dvo;

NAK.  Please get rid of the "volatile" and use I/O accessors.
Please fix globally.

...
> +#if defined(CONFIG_SYS_DRAM_TEST)
> +int testdram(void)
> +{
> +	uint *pstart = (uint *) CONFIG_SYS_MEMTEST_START;
> +	uint *pend = (uint *) CONFIG_SYS_MEMTEST_END;
> +	uint *p;
> +
> +	printf("Testing DRAM from 0x%08x to 0x%08x\n",
> +		CONFIG_SYS_MEMTEST_START, CONFIG_SYS_MEMTEST_END);
> +
> +	printf("DRAM test phase 1:\n");
> +	for (p = pstart; p < pend; p++)
> +		*p = 0xaaaaaaaa;
> +
> +	for (p = pstart; p < pend; p++) {
> +		if (*p != 0xaaaaaaaa) {
> +			printf("DRAM test fails at: %08x\n", (uint) p);
> +			return 1;
> +		}
> +	}
> +
> +	printf("DRAM test phase 2:\n");
> +	for (p = pstart; p < pend; p++)
> +		*p = 0x55555555;
> +
> +	for (p = pstart; p < pend; p++) {
> +		if (*p != 0x55555555) {
> +			printf("DRAM test fails at: %08x\n", (uint) p);
> +			return 1;
> +		}
> +	}
> +
> +	printf("DRAM test passed.\n");
> +	return 0;
> +}
> +#endif

Do you _really_ have to implement yet another version of a memory test
(and not even a good one).  Why cannot you use one of the existing
ones instead?

> +phys_size_t initdram(int board_type)
> +{
> +	u32 msize;
> +
> +	volatile immap_t *immr = (immap_t *)CONFIG_SYS_IMMR;
> +	volatile clk83xx_t *clk = (clk83xx_t *)&immr->clk;
> +
> +	/* Enable PCI_CLK[0:1] */
> +	clk->occr |= 0xc0000000;
> +	udelay(2000);
> +
> +#if defined(CONFIG_SPD_EEPROM)
> +	msize = spd_sdram();
> +#else
> +	immap_t *im = (immap_t *) CONFIG_SYS_IMMR;
> +	u32 msize_log2;

NAK.  We don't allow variable declarations in the middle of the code.

> +	return msize << 20;

Is there any specific reason for not using get_ram_size() ?


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
"Everybody is talking about the  weather  but  nobody  does  anything
about it."                                               - Mark Twain