Hi Masahiro,
On Wed, 2014-09-17 at 18:09 +0900, Masahiro Yamada wrote:
Hi Chin,
On Mon, 15 Sep 2014 01:39:07 -0500 Chin Liang See clsee@altera.com wrote:
Hi Masahiro,
On Fri, 2014-09-12 at 17:06 +0900, Masahiro Yamada wrote:
+/* nand_init() - initialize data to make nand usable by SPL */ +void nand_init(void) +{
- /* access to main area */
- writel(0, denali_flash_reg + TRANSFER_SPARE_REG);
- page_size = readl(denali_flash_reg + DEVICE_MAIN_AREA_SIZE);
- oob_size = readl(denali_flash_reg + DEVICE_SPARE_AREA_SIZE);
- pages_per_block = readl(denali_flash_reg + PAGES_PER_BLOCK);
I believe this will work for ONFI NAND devices only. For non-ONFI, the value might not correct.
I don't think so. It depends on the hardware; in my understanding Denali IP is capable of detecting MAIN_AREA_SIZE etc. for non-ONFI devices. At least this is working with non-ONFI devices on some Panasonic boards.
If it does not work for Altera SoCs (and if you are planning to use this driver), these three registers should be set in advance in an earlier board init.
I recall one of my colleague was telling me that it doesn't work for one of non ONFI part where it read incorrect page size. Nevertheless, we can put comments so user which use this driver need to take note.
I have also heard of that. I think it happens on some Hynix devices.
According to my colleague, for the device with maf_id = 0xAD, device_id = 0xDA, the Denali IP seems to set wrong parameters.
Either comments or fixup code like get_hynix_nand_para() in denali.c will do.
BTW I've noticed only a few Hynix devices are supported by denali.c. (only the device id = 0xD5 or 0xD7)
This is, anyway, a upstream limitation and we can send a follow up patch if needed.
Yup, that sound a good plan to enable this patch to move forward. We just need to add some write-up to the commit message.
Currently U-Boot has drivers/mtd/nand/nand_spl_simple.c which handling the SPL NAND image load. Wonder this driver will be integrated into nand_spl_simple.c once drivers/mtd/nand/denali.c is applied?
I am not planning to do so because:
[1] nand_spl_simple.c requires CONFIG_SYS_NAND_BLOCK_SIZE, CONFIG_SYS_NAND_PAGE_SIZE, CONFIG_SYS_NAND_PAGE_COUNT; we need to specify the device attributes at compilation, which the Denali IP is able to detect at run time. It is not acceptable for us because we need (want) the run time configuration.
[2] nand_spl_simple.c is so generic that it cannot use the hardware acceleration of the Denali IP, that is, slower booting.
Yup, you identified the nand_spl_simple.c constrain. This is why I patched this file at http://rocketboards.org/gitweb/?p=u-boot-socfpga.git;a=commit;h=461a61b8f03d.... But I didn't send this patch out as I am waiting the NAND driver patch accepted.
After applying your patch, both [1] and [2] are still the constrain of nand_spl_simple.c
For [1], yup. But the intention is to ensure it works for various devices and NAND controllers. While for [2], the patch will take advantage of the hardware based ECC calculation. But it will not take advantage of the DMA as I believe DMA might not exist for certain NAND controller.
With that, I don't have much concern to remain this file as this would enable better performance as its specific for Denali. Probably Altera should use this too if its accepted into mainline :)
Thanks Chin Liang
Best Regards Masahiro Yamada