From mboxrd@z Thu Jan 1 00:00:00 1970 From: Igor Grinberg Date: Fri, 28 Oct 2011 01:10:25 +0200 Subject: [U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard In-Reply-To: <4EA9CD2F.1060306@ti.com> References: <1319663618-10005-1-git-send-email-trini@ti.com> <1319663618-10005-3-git-send-email-trini@ti.com> <4EA9CA9D.6010209@compulab.co.il> <4EA9CD2F.1060306@ti.com> Message-ID: <4EA9E4E1.6040603@compulab.co.il> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 10/27/2011 11:29 PM, Tom Rini wrote: > 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 >>> Beagleboard xM rev C: >>> Tested-by: Matt Ranostay >>> Beagleboard rev B7, C2, xM rev B: >>> Tested-by: Matt Porter >>> Signed-off-by: Tom Rini >>> --- >>> 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. That would be much better solution. > [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. Yeah, me too, but it looks like these, can benefit from it... It is not a blocker - up to you ;) > [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. No, not really ;) I've been disrupted and did a mistake - sorry for that. > [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. > Regards, Igor