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

[Peter Korsgaard]

>>>>> "Ilya" == Ilya Yanok <ilya.yanok at 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.

-- 
Bye, Peter Korsgaard