"Ilya" == Ilya Yanok ilya.yanok@cogentembedded.com writes:
Hi,
Peter> Why? You already do this in enable_gpmc_cs_config().
Ilya> Hm... Again, I'm not the one who written this code, I just stole Ilya> it ;) but probably the idea was to disable the config even in Ilya> case of CONFIG_CMD_NAND undefined...
Why would you want to do that? I don't see a reason for touching the GPMC if you're not going to use it.
Ilya> Ilya> +#define M_NAND_GPMC_CONFIG1 0x00000800 Ilya> Ilya> +#define M_NAND_GPMC_CONFIG2 0x001e1e00 Ilya> Ilya> +#define M_NAND_GPMC_CONFIG3 0x001e1e00 Ilya> Ilya> +#define M_NAND_GPMC_CONFIG4 0x16051807 Ilya> Ilya> +#define M_NAND_GPMC_CONFIG5 0x00151e1e Ilya> Ilya> +#define M_NAND_GPMC_CONFIG6 0x16000f80 Ilya> Ilya> +#define M_NAND_GPMC_CONFIG7 0x00000008
Peter> For what Micron part is this exactly? How about using the actual part Peter> number in the define like we recently did for the DDR configuration?
Ilya> Ok, I'll try to figure that out from schematics.
Great, thanks. The choice of GPMC configuration values should also be moved to board code, similar to how I did it for DDR.
Peter> Do the non-BCH layouts make sense for am33xx? I've noticed Peter> that the TRM shows the BCH8 format as 13 bytes/256 tightly Peter> packed instead of 14, but that's wrong. I'll report it to the Peter> doc people.
Ilya> Hm. Non-BCH layouts was here because I initially planned to Ilya> support nandecc command to switch between supported ECC Ilya> schemas... But as Tom requested to remove it I guess we don't Ilya> need these layouts any more... Well, my idea was to make Ilya> separate patches (one to just support existing omap_gpmc driver Ilya> on AM33xx and one to add BCH8 support) and this kinda needs Ilya> non-BCH layouts too... but I guess I should just squash the Ilya> patches into one.
I don't think non-BCH makes much sense on am33xx considering the ROM uses/requires BCH8.
Ilya> Thanks for the review.
You're welcome.