Hi Peter,
On Thu, Nov 8, 2012 at 10:33 AM, Peter Korsgaard jacmet@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.