[U-Boot] [PATCH v2 3/8] am33xx: NAND support

[Ilya Yanok]

Hi Peter,

On Thu, Nov 8, 2012 at 10:33 AM, Peter Korsgaard <jacmet at sunsite.dk> wrote:

>  Ilya> +void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs
> *cs, u32 base,
>  Ilya> +                        u32 size)
>  Ilya> +{
>  Ilya> +        writel(0, &cs->config7);
>  Ilya> +        sdelay(1000);
>  Ilya> +        /* Delay for settling */
>
> That comment should go above the delay.
>

Ok, will fix.


>   Ilya> +        writel(gpmc_config[0], &cs->config1);
>  Ilya> +        writel(gpmc_config[1], &cs->config2);
>  Ilya> +        writel(gpmc_config[2], &cs->config3);
>  Ilya> +        writel(gpmc_config[3], &cs->config4);
>  Ilya> +        writel(gpmc_config[4], &cs->config5);
>  Ilya> +        writel(gpmc_config[5], &cs->config6);
>  Ilya> +        /* Enable the config */
>  Ilya> +        writel((((size & 0xF) << 8) | ((base >> 24) & 0x3F) |
>  Ilya> +                (1 << 6)), &cs->config7);
>  Ilya> +        sdelay(2000);
>
> Any reason you now wait double as long?
>

No idea ;) To tell the truth I've taken all this code from omap3 as is.


>  Ilya> +}
>  Ilya> +
>  Ilya> +/*****************************************************
>  Ilya> + * gpmc_init(): init gpmc bus
>  Ilya> + * Init GPMC for x16, MuxMode (SDRAM in x32).
>  Ilya> + * This code can only be executed from SRAM or SDRAM.
>  Ilya> + *****************************************************/
>  Ilya> +void gpmc_init(void)
>  Ilya> +{
>  Ilya> +        /* putting a blanket check on GPMC based on ZeBu for now */
>  Ilya> +        gpmc_cfg = (struct gpmc *)GPMC_BASE;
>  Ilya> +
>  Ilya> +#ifdef CONFIG_CMD_NAND
>  Ilya> +        const u32 *gpmc_config = NULL;
>  Ilya> +        u32 base = 0;
>  Ilya> +        u32 size = 0;
>  Ilya> +#endif
>  Ilya> +        /* global settings */
>  Ilya> +        writel(0x00000008, &gpmc_cfg->sysconfig);
>  Ilya> +        writel(0x00000100, &gpmc_cfg->irqstatus);
>  Ilya> +        writel(0x00000200, &gpmc_cfg->irqenable);
>  Ilya> +        writel(0x00000012, &gpmc_cfg->config);
>  Ilya> +        /*
>  Ilya> +         * Disable the GPMC0 config set by ROM code
>  Ilya> +         */
>  Ilya> +        writel(0, &gpmc_cfg->cs[0].config7);
>  Ilya> +        sdelay(1000);
>
> Why? You already do this in enable_gpmc_cs_config().
>

Hm... Again, I'm not the one who written this code, I just stole it ;)
but probably the idea was to disable the config even in case of
CONFIG_CMD_NAND undefined...


>  Ilya> +
>  Ilya> +#ifdef CONFIG_CMD_NAND
>  Ilya> +        gpmc_config = gpmc_m_nand;
>  Ilya> +
>  Ilya> +        base = PISMO1_NAND_BASE;
>  Ilya> +        size = PISMO1_NAND_SIZE;
>  Ilya> +        enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base,
> size);
>  Ilya> +#endif
>  Ilya> +}
>
>  Ilya> +#define M_NAND_GPMC_CONFIG1     0x00000800
>  Ilya> +#define M_NAND_GPMC_CONFIG2     0x001e1e00
>  Ilya> +#define M_NAND_GPMC_CONFIG3     0x001e1e00
>  Ilya> +#define M_NAND_GPMC_CONFIG4     0x16051807
>  Ilya> +#define M_NAND_GPMC_CONFIG5     0x00151e1e
>  Ilya> +#define M_NAND_GPMC_CONFIG6     0x16000f80
>  Ilya> +#define M_NAND_GPMC_CONFIG7     0x00000008
>
> For what Micron part is this exactly? How about using the actual part
> number in the define like we recently did for the DDR configuration?
>

Ok, I'll try to figure that out from schematics.

Do the non-BCH layouts make sense for am33xx? I've noticed that the TRM
> shows the BCH8 format as 13 bytes/256 tightly packed instead of 14, but
> that's wrong. I'll report it to the doc people.
>

Hm. Non-BCH layouts was here because I initially planned to support nandecc
command to switch between supported ECC schemas... But as Tom requested to
remove it I guess we don't need these layouts any more...
Well, my idea was to make separate patches (one to just support existing
omap_gpmc driver on AM33xx and one to add BCH8 support) and this kinda
needs non-BCH layouts too... but I guess I should just squash the patches
into one.

Thanks for the review.

Regards, Ilya.