On 10/27/2011 05:46 AM, Premi, Sanjeev wrote: >> -----Original Message----- >> From: [email protected] >> [mailto:[email protected]] On Behalf Of Rini, Tom >> Sent: Thursday, October 27, 2011 2:44 AM >> To: [email protected] >> 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 <[email protected]> >> Beagleboard xM rev C: >> Tested-by: Matt Ranostay <[email protected]> >> Beagleboard rev B7, C2, xM rev B: >> Tested-by: Matt Porter <[email protected]> >> Signed-off-by: Tom Rini <[email protected]> >> --- > > [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.
Yeah, this is all a drop-in of the x-loader code which needs some clean up. But the overall logic (poke this to determine that) is what I want to make sure we're going to be OK with. -- Tom _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
