[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Premi, Sanjeev]

> -----Original Message-----
> From: u-boot-bounces at lists.denx.de 
> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Rini, Tom
> Sent: Thursday, October 27, 2011 2:44 AM
> To: u-boot at lists.denx.de
> Subject: [U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard
> 
> This introduces 200MHz Micron parts timing information based 
> on x-loader
> and re-organizes the file slightly for grouping.  The memory 
> init logic
> is also based on what x-loader does in these cases.  Note that while
> previously u-boot would be flashed in with SW ECC in this case it now
> must be flashed with HW ECC.
> 
> Beagleboard rev C5, xM rev A:
> Tested-by: Tom Rini <trini at ti.com>
> Beagleboard xM rev C:
> Tested-by: Matt Ranostay <mranostay at gmail.com>
> Beagleboard rev B7, C2, xM rev B:
> Tested-by: Matt Porter <mporter at ti.com>
> Signed-off-by: Tom Rini <trini at ti.com>
> ---

[snip]...[snip]

>  
> +#ifdef CONFIG_SPL_BUILD
> +
> +#define MICRON_DDR	0
> +#define NUMONYX_MCP	1
> +#define MICRON_MCP	2
> +
> +#define NAND_CMD_STATUS		0x70
> +#define NAND_CMD_READID		0x90
> +#define NAND_CMD_RESET		0xff
> +
> +#define GPMC_NAND_COMMAND_0      (OMAP34XX_GPMC_BASE+0x7C)
> +#define GPMC_NAND_ADDRESS_0      (OMAP34XX_GPMC_BASE+0x80)
> +#define GPMC_NAND_DATA_0	 (OMAP34XX_GPMC_BASE+0x84)
> +
> +#define WRITE_NAND_COMMAND(d, adr) \
> +	do {*(volatile u16 *)GPMC_NAND_COMMAND_0 = d;} while(0)
> +#define WRITE_NAND_ADDRESS(d, adr) \
> +	do {*(volatile u16 *)GPMC_NAND_ADDRESS_0 = d;} while(0)
> +#define READ_NAND(adr)          (*(volatile u16 *)GPMC_NAND_DATA_0)
> +

I am not yet familiar with SPL code, but I would suggest using
"struct gpmc" instead of hardcoded offsets above.

[snip]...[snip]

> +
> +#define GPMC_CONFIG_CS0_CONFIG1		0x6E000060
> +#define GPMC_CONFIG_CS0_CONFIG2		0x6E000064
> +#define GPMC_CONFIG_CS0_CONFIG3		0x6E000068
> +#define GPMC_CONFIG_CS0_CONFIG4		0x6E00006C
> +#define GPMC_CONFIG_CS0_CONFIG5		0x6E000070
> +#define GPMC_CONFIG_CS0_CONFIG6		0x6E000074
> +#define GPMC_CONFIG_CS0_CONFIG7		0x6E000078
> +#define OMAP34XX_GPMC_CS0_SIZE		0x8

Suggest using "struct gpmc_cs" instead of defining offsets.

[snip]...[snip]

> +
> +static int identify_xm_ddr(void)
> +{
> +	int mfr, id;
> +
> +	/* Make sure that we have setup GPMC for NAND correctly. */
> +	writel(M_NAND_GPMC_CONFIG1, GPMC_CONFIG_CS0_CONFIG1);
> +	writel(M_NAND_GPMC_CONFIG2, GPMC_CONFIG_CS0_CONFIG2);
> +	writel(M_NAND_GPMC_CONFIG3, GPMC_CONFIG_CS0_CONFIG3);
> +	writel(M_NAND_GPMC_CONFIG4, GPMC_CONFIG_CS0_CONFIG4);
> +	writel(M_NAND_GPMC_CONFIG5, GPMC_CONFIG_CS0_CONFIG5);
> +	writel(M_NAND_GPMC_CONFIG6, GPMC_CONFIG_CS0_CONFIG6);
> +
> +	/* Enable the GPMC Mapping */
> +	writel((((OMAP34XX_GPMC_CS0_SIZE & 0xF) << 8) |
> +			     ((NAND_BASE >> 24) & 0x3F) |
> +			     (1 << 6)),  (GPMC_CONFIG_CS0_CONFIG7));

Same comment as before. Looks like there may be few more places
where hardcoded addresses/ offsets are being used.

~sanjeev

[snip]...[snip]