> -----Original Message----- > From: Igor Grinberg [mailto:grinb...@compulab.co.il] > Sent: Wednesday, December 07, 2011 7:55 PM > To: Rini, Tom > Cc: u-boot@lists.denx.de; Hiremath, Vaibhav > Subject: Re: [U-Boot] [PATCH 3/8] AM3517: Add NOR Flash boot mode Support > > 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. > [Hiremath, Vaibhav] I think, yes this information is board specific, since NOR flash is mounted on board.
Actually we have followed existing NAND/OneNAND implementation here, and that's where we have this here. Also note that, in case of OMAP3/AM37x we have POP package where NAND/OneNAND part is fixed, so it makes sense to bring here. > > + > > #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 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. > [Hiremath, Vaibhav] Agreed, gpmc_init needs some cleanup to make it generic interface. And probably all data should get passed from board file. > > 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? > [Hiremath, Vaibhav] I do not think it breaks anything. > > > > #define SIBNOR_GPMC_CONFIG1 0x1200 > > #define SIBNOR_GPMC_CONFIG2 0x001f1f00 > > diff --git a/board/logicpd/am3517evm/am3517evm.h > b/board/logicpd/am3517evm/am3517evm.h > > index 68d746c..17bb78d 100644 > > --- a/board/logicpd/am3517evm/am3517evm.h > > +++ b/board/logicpd/am3517evm/am3517evm.h > > @@ -327,7 +327,7 @@ const omap3_sysinfo sysinfo = { > > MUX_VAL(CP(SYS_CLKREQ), (IEN | PTD | DIS | M0)) \ > > MUX_VAL(CP(SYS_NIRQ), (IEN | PTU | EN | M0)) \ > > /*SYS_nRESWARM */\ > > - MUX_VAL(CP(SYS_NRESWARM), (IDIS | PTU | DIS | M4)) \ > > + MUX_VAL(CP(SYS_NRESWARM), (IDIS | PTU | EN | M4)) \ > > /* - GPIO30 */\ > > MUX_VAL(CP(SYS_BOOT0), (IEN | PTD | DIS | M4)) /*GPIO_2*/\ > > /* - PEN_IRQ */\ > > -- > Regards, > Igor. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot