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.

> +
>  #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.

> 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?

>  
>  #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

Reply via email to