[U-Boot] [PATCH 3/8] AM3517: Add NOR Flash boot mode Support

Hiremath, Vaibhav hvaibhav at ti.com
Thu Dec 8 12:11:14 CET 2011


> -----Original Message-----
> From: Igor Grinberg [mailto:grinberg at compulab.co.il]
> Sent: Wednesday, December 07, 2011 7:55 PM
> To: Rini, Tom
> Cc: u-boot at 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 <hvaibhav at 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 <hvaibhav at ti.com>
> > Signed-off-by: Tom Rini <trini at 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.


More information about the U-Boot mailing list