On Wed, Dec 7, 2011 at 7:25 AM, Igor Grinberg <grinb...@compulab.co.il> wrote: > Hi Tom, > > On 12/06/11 17:49, Tom Rini wrote: >> From: Vaibhav Hiremath <hvaib...@ti.com> >> >> Please note that NOR Flash is located on Application board and requires >> hardware modification to get NOR boot mode working. >> >> NOR Flash boot mode configuration - >> >> - From baseboard remove R235 resistor. >> - Set switch S11.3 position to "ON" >> - Set S7 switch position to >> 1 2 3 4 5 >> ----------------- >> on off off off off >> >> Please note that, once you remove R235 resistor from the baseboard, you >> will not be able to boot from NAND without Application board. >> The GPMC_nCS0 is now routed through Application board. >> >> Please note that, <Rev4 revision of Application board doesn't >> work with NOR Flash due to HW issue. >> >> Signed-off-by: Vaibhav Hiremath <hvaib...@ti.com> >> Signed-off-by: Tom Rini <tr...@ti.com> >> --- >> arch/arm/cpu/armv7/omap3/mem.c | 61 >> +++++++++++++++++++++++++++----- >> arch/arm/include/asm/arch-omap3/mem.h | 15 +++++--- >> board/logicpd/am3517evm/am3517evm.h | 2 +- >> 3 files changed, 61 insertions(+), 17 deletions(-) >> >> diff --git a/arch/arm/cpu/armv7/omap3/mem.c b/arch/arm/cpu/armv7/omap3/mem.c >> index 4a8c025..4ad3d12 100644 >> --- a/arch/arm/cpu/armv7/omap3/mem.c >> +++ b/arch/arm/cpu/armv7/omap3/mem.c >> @@ -51,6 +51,17 @@ static const u32 gpmc_m_nand[GPMC_MAX_REG] = { >> >> #endif >> >> +#if defined (CONFIG_CMD_FLASH) >> +static const u32 gpmc_nor[GPMC_MAX_REG] = { >> + STNOR_GPMC_CONFIG1, >> + STNOR_GPMC_CONFIG2, >> + STNOR_GPMC_CONFIG3, >> + STNOR_GPMC_CONFIG4, >> + STNOR_GPMC_CONFIG5, >> + STNOR_GPMC_CONFIG6, 0 >> +}; >> +#endif > > This does not seem to be the right place for these settings... > I think those must be board specific.
I'm not sure, it's hard to tell with just one board having NOR and using this code (and I don't have access to custom boards with NOR on them, but I know they exist). In NAND/OneNAND land, this apparently isn't board-specific settings we're shoving in here. Another argument in favor of not just shoving magic values behind a define. >> #if defined(CONFIG_CMD_ONENAND) >> static const u32 gpmc_onenand[GPMC_MAX_REG] = { >> ONENAND_GPMC_CONFIG1, >> @@ -118,21 +129,34 @@ void enable_gpmc_cs_config(const u32 *gpmc_config, >> struct gpmc_cs *cs, u32 base, >> sdelay(2000); >> } >> >> -/***************************************************** >> +/* >> * gpmc_init(): init gpmc bus >> - * Init GPMC for x16, MuxMode (SDRAM in x32). >> - * This code can only be executed from SRAM or SDRAM. >> - *****************************************************/ >> + * Init GPMC for x16, MuxMode (SDRAM in x32). This code can only be >> + * executed from SRAM or SDRAM. In the common case, we will disable >> + * and then configure chip select 0 for our needs (NAND or OneNAND). >> + * However, on the AM3517 EVM the way that NOR can be exposed requires us >> + * to rethink this. When NOR is enabled, it will be chip select 0 if >> + * we are booting from NOR or chip select 2 otherwise. This requires us >> + * to check the value we get from the SYSBOOT pins. > > All the above looks like board specific... > >> + */ >> void gpmc_init(void) >> { >> /* putting a blanket check on GPMC based on ZeBu for now */ >> gpmc_cfg = (struct gpmc *)GPMC_BASE; >> -#if defined(CONFIG_CMD_NAND) || defined(CONFIG_CMD_ONENAND) >> +#if defined(CONFIG_CMD_NAND) || defined(CONFIG_CMD_ONENAND) || \ >> + (defined(CONFIG_OMAP3_AM3517EVM) && defined(CONFIG_SYS_HAS_NORFLASH)) >> const u32 *gpmc_config = NULL; >> u32 base = 0; >> - u32 size = 0; >> + u32 sz = 0; >> #endif >> u32 config = 0; >> + u32 nor_boot = 0; >> + >> +#if defined(CONFIG_OMAP3_AM3517EVM) && defined(CONFIG_SYS_HAS_NORFLASH) >> + /* 0xD means NOR boot on AM35x */ >> + if (get_boot_type() == 0xD) >> + nor_boot = 1; >> +#endif >> >> /* global settings */ >> writel(0, &gpmc_cfg->irqenable); /* isr's sources masked */ >> @@ -146,14 +170,31 @@ void gpmc_init(void) >> gpmc_config = gpmc_m_nand; >> >> base = PISMO1_NAND_BASE; >> - size = PISMO1_NAND_SIZE; >> - enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base, size); >> + sz = PISMO1_NAND_SIZE; >> + if (!nor_boot) >> + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base, sz); >> + else >> + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[2], base, sz); >> + >> #endif >> >> #if defined(CONFIG_CMD_ONENAND) >> gpmc_config = gpmc_onenand; >> base = PISMO1_ONEN_BASE; >> - size = PISMO1_ONEN_SIZE; >> - enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base, size); >> + sz = PISMO1_ONEN_SIZE; >> + if (!nor_boot) >> + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base, sz); >> + else >> + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[2], base, sz); >> + >> +#endif >> + >> +#if defined(CONFIG_OMAP3_AM3517EVM) && defined(CONFIG_SYS_HAS_NORFLASH) >> + /* NOR - CS2 */ >> + gpmc_config = gpmc_nor; >> + base = DEBUG_BASE; >> + sz = GPMC_SIZE_64M; >> + if (!nor_boot) >> + enable_gpmc_cs_config(gpmc_nor, &gpmc_cfg->cs[2], base, sz); >> #endif >> } > > All the above look very hackish... > You just bring board specific code into a common location. I'm not sure it is board-specific, but I don't have another NOR using example around... > I don't think we should be doing it this way. > Instead, change the gpmc_init() function signature to get a parameter(s), > so it will be a generic function, that will initialize the right stuff > according to the parameters and will not have this board specific > ifdeffery crap. This I think is right, gpmc_init needs a re-think. >> diff --git a/arch/arm/include/asm/arch-omap3/mem.h >> b/arch/arm/include/asm/arch-omap3/mem.h >> index 5fd02d4..2f52481 100644 >> --- a/arch/arm/include/asm/arch-omap3/mem.h >> +++ b/arch/arm/include/asm/arch-omap3/mem.h >> @@ -329,12 +329,15 @@ enum { >> #define M_NAND_GPMC_CONFIG6 0x1f0f0A80 >> #define M_NAND_GPMC_CONFIG7 0x00000C44 >> >> -#define STNOR_GPMC_CONFIG1 0x3 >> -#define STNOR_GPMC_CONFIG2 0x00151501 >> -#define STNOR_GPMC_CONFIG3 0x00060602 >> -#define STNOR_GPMC_CONFIG4 0x11091109 >> -#define STNOR_GPMC_CONFIG5 0x01141F1F >> -#define STNOR_GPMC_CONFIG6 0x000004c4 >> +/* >> + * Configuration required for AM3517EVM PC28F640P30B85 Flash >> + */ >> +#define STNOR_GPMC_CONFIG1 0x00001210 >> +#define STNOR_GPMC_CONFIG2 0x00101001 >> +#define STNOR_GPMC_CONFIG3 0x00020201 >> +#define STNOR_GPMC_CONFIG4 0x0f031003 >> +#define STNOR_GPMC_CONFIG5 0x000f1111 >> +#define STNOR_GPMC_CONFIG6 0x0f030080 > > I see also: > arch/arm/cpu/armv7/omap3/lowlevel_init.S:184: .word STNOR_GPMC_CONFIG3 > arch/arm/cpu/armv7/omap3/lowlevel_init.S:188: .word STNOR_GPMC_CONFIG4 > arch/arm/cpu/armv7/omap3/lowlevel_init.S:190: .word STNOR_GPMC_CONFIG5 > > I haven't looked into this, but will your change break anything else? I think the lowlevel_init.S bits need stepping around with a JTAG on a few boards, but I believe no since no other boards have NOR. -- Tom _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot