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

Tom Rini tom.rini at gmail.com
Thu Dec 8 02:07:19 CET 2011


On Wed, Dec 7, 2011 at 7:25 AM, Igor Grinberg <grinberg at compulab.co.il> wrote:
> 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.

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


More information about the U-Boot mailing list