[U-Boot] [PATCH 01/11] snowball: Add support for ux500 based snowball board

Mathieu Poirier mathieu.poirier at linaro.org
Fri Aug 3 21:32:26 CEST 2012


Thanks for the review - see my comments below.

Mathieu.

On 12-08-03 08:46 AM, Tom Rini wrote:
> On Tue, Jul 31, 2012 at 12:59:23PM -0600, mathieu.poirier at linaro.org wrote:
> 
>> From: "Mathieu J. Poirier" <mathieu.poirier at linaro.org>
>>
>> Signed-off-by: Mathieu Poirier <mathieu.poirier at linaro.org>
>> Signed-off-by: John Rigby <john.rigby at linaro.org>
> [snip]
>> +++ b/board/st-ericsson/snowball/Makefile
> [snip]
>> +#prcmu.o
> 
> Drop please.
> 
> [snip]
>> +++ b/board/st-ericsson/snowball/snowball.c
>> +int dram_init(void)
>> +{
>> +	uint32_t unused_cols_rows;
>> +	unsigned int nrows;
>> +	unsigned int ncols;
>> +
>> +	gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
>> +	gd->ram_size = PHYS_SDRAM_SIZE_1;
>> +
>> +	/*
>> +	 * Assumption: 2 CS active, both CS have same layout.
>> +	 *             15 rows max, 11 cols max (controller spec).
>> +	 *             memory chip has 8 banks, I/O width 32 bit.
>> +	 * The correct way would be to read MR#8: I/O width and density,
>> +	 * but this requires locking against the PRCMU firmware.
>> +	 * Simplified approach:
>> +	 * Read number of unused rows and columns from mem controller.
>> +	 * size = nCS x 2^(rows+cols) x nbanks x buswidth_bytes
>> +	 */
>> +	unused_cols_rows = readl(DMC_CTL_97);
>> +	nrows = 15 - (unused_cols_rows & 0x07);
>> +	ncols = 11 - ((unused_cols_rows & 0x0700) >> 8);
>> +	gd->ram_size = gd->bd->bi_dram[0].size =
>> +					2 * (1 << (nrows + ncols)) * 8 * 4;
> 
> Please use get_mem_size(CONFIG_SYS_SDRAM_BASE, possible-max-size) (and
> defining CONFIG_SYS_MAX_RAM_SIZE is common convention) instead of all
> the above.
> 

Is 'get_mem_size' supposed to be an existing API ?  I grep'ed for it in
the code to no avail.

> [snip]
>> +++ b/drivers/gpio/db8500_gpio.c
> [snip]
>> +/*
>> + * Macros to work with IO space
>> + * Not actually used?
>> + */
>> +#define __iomem
> 
> <linux/compiler.h> provides this for us, please use that instead.
> 



More information about the U-Boot mailing list