[U-Boot] [PATCH v4 1/6] nand: denali: add Denali NAND driver for SPL

[Masahiro Yamada]

Hi.



> > Changes in v4:
> >   - Add a workaround to not depend on the Denali driver
> >     posted by Chin Liang See.
> >     This driver has been taking too long:
> >     http://patchwork.ozlabs.org/patch/381305/
> > 
> 
> Yup, hopefully v10 would be the final patch.


I hope so.


> > +static int nand_block_isbad(int block)
> > +{
> > +	int ret;
> > +
> > +	ret = nand_read_oob(page_buffer, block * pages_per_block);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return page_buffer[CONFIG_SYS_NAND_BAD_BLOCK_POS] != 0xff;
> > +}
> > +
> > +/* 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.




> > +}
> > +
> > +int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
> > +{
> > +	int block, page, column, readlen;
> > +	int ret;
> > +	int force_bad_block_check = 1;
> > +
> > +	page = offs / page_size;
> > +	column = offs % page_size;
> > +
> > +	block = page / pages_per_block;
> > +	page = page % pages_per_block;
> > +
> > +	while (size) {
> 
> I believe we need to error out when reading beyond last block.


How do you know the number of blocks of non-ONFI devices?
Scan nand_ids.c for SPL?

> 

> > +		if (force_bad_block_check || page == 0) {
> > +			ret = nand_block_isbad(block);
> > +			if (ret < 0)
> > +				return ret;
> > +
> > +			if (ret) {
> > +				block++;
> > +				continue;
> > +			}
> > +		}
> > +
> > +		force_bad_block_check = 0;
> 
> I believe we still need to check the subsequent block whether is bad or
> not too. This can be enable when cross the block boundary.


I am afraid you are misunderstanding my code.

This function does bad block checking for every block.

Here,

		if (force_bad_block_check || page == 0) {
			ret = nand_block_isbad(block);


"page == 0" means the beginning of each block.





> 
> 
> 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.


Best Regards
Masahiro Yamada