On 10/27/2011 02:18 PM, Igor Grinberg wrote: > On 10/26/2011 11:13 PM, Tom Rini wrote: >> 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. > > You have two spaces between the sentences, why is that?
Old habit. >> 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]> >> --- >> arch/arm/include/asm/arch-omap3/mem.h | 24 +++++ >> board/ti/beagle/beagle.c | 160 >> ++++++++++++++++++++++++++++++++- >> board/ti/beagle/config.mk | 33 ------- >> include/configs/omap3_beagle.h | 60 ++++++++++++- >> 4 files changed, 242 insertions(+), 35 deletions(-) >> delete mode 100644 board/ti/beagle/config.mk > > config.mk removal does not belong to that patch... > It should be a separate one, say cleanup patch. I'll see if I can restructure things and kill that off first then. > >> diff --git a/arch/arm/include/asm/arch-omap3/mem.h >> b/arch/arm/include/asm/arch-omap3/mem.h >> index af3504c..a784813 100644 >> --- a/arch/arm/include/asm/arch-omap3/mem.h >> +++ b/arch/arm/include/asm/arch-omap3/mem.h >> @@ -171,6 +171,30 @@ enum { >> #define MICRON_V_MR ((MICRON_WBST << 9) | (MICRON_CASL << 4) | \ >> (MICRON_SIL << 3) | (MICRON_BL)) >> >> + >> +/* Micron part (200MHz optimized) 5 ns >> + */ >> +#define MICRON_TDAL_200 6 >> +#define MICRON_TDPL_200 3 >> +#define MICRON_TRRD_200 2 >> +#define MICRON_TRCD_200 3 >> +#define MICRON_TRP_200 3 >> +#define MICRON_TRAS_200 8 >> +#define MICRON_TRC_200 11 >> +#define MICRON_TRFC_200 15 >> +#define MICRON_V_ACTIMA_200 ((MICRON_TRFC_200 << 27) | (MICRON_TRC_200 << >> 22) | (MICRON_TRAS_200 << 18) \ >> + | (MICRON_TRP_200 << 15) | (MICRON_TRCD_200 << 12) >> |(MICRON_TRRD_200 << 9) | \ >> + (MICRON_TDPL_200 << 6) | (MICRON_TDAL_200)) > > MICRON_TDAL_200 does not need parenthesis. As I said to Sanjeev this is all yanked right from X-loader so yes, there's spacing and so on problems (and I swore git send-email --subject modified subject for all patches, but I guess not). [snip] >> +#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) > > This is definitely needs a cleanup... > Consider Sanjeev's proposal. If it will not work for some reason, > you need at least to use writel() readl() io accessors. At first pass you can't call gpmc_init() as-is this early, but I'll switch over to readl/writel as a start. > >> + >> +/* nand_command: Send a flash command to the flash chip */ >> +static void nand_command(unsigned char command) >> +{ >> + WRITE_NAND_COMMAND(command, NAND_ADDR); >> + >> + if (command == NAND_CMD_RESET) { >> + unsigned char ret_val; >> + nand_command(NAND_CMD_STATUS); >> + do { >> + ret_val = READ_NAND(NAND_ADDR);/* wait till ready */ >> + } while ((ret_val & 0x40) != 0x40); > > You should be using some kind of timeout, so you will not stuck in here > without being noticed. OK. I've been wondering if we shouldn't somehow make a not-tied-to-full-mtd nand_command more available since I suspect a few other boards will be in a similar situation, for probing early on. [snip] >> + *mr = MICRON_V_MR; >> + switch (get_board_revision()) { >> + case REVISION_C4: >> + if (identify_xm_ddr() == NUMONYX_MCP) { >> + *cs_cfg = 0x4; >> + *mcfg = 0x04590099; >> + *ctrla = NUMONYX_V_ACTIMA_165; >> + *ctrlb = NUMONYX_V_ACTIMB_165; >> + *rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz; > > aligning the assignments above (and below) will make it much more readable I'm really not a fan of spacing out assignments but I'll see what CodingSytle says. [snip] >> -#ifdef CONFIG_GENERIC_MMC >> +#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD) > > This should be another patch. Really? I'm adding SPL support here and that's what introduces the need to not build this on just CONFIG_GENERIC_MMC. [snip] >> +#define CONFIG_SPL_BSS_START_ADDR 0x80000000 >> +#define CONFIG_SPL_BSS_MAX_SIZE 0x80000 /* 512 KB */ > > alignment I'll double check but all of them but this one is fine once applied. Thanks. -- Tom _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
