[U-Boot] [U-Boot, 2/2] NAND: add support for reading ONFI page table

[Scott Wood]

On Fri, Dec 10, 2010 at 12:16:42PM -0000, Florian Fainelli wrote:
> From: Florian Fainelli <florian at openwrt.org>
> 
> This patch adds support for reading an ONFI page parameter from a NAND device
> supporting it. If this is the case, struct nand_chip onfi_version member
> contains the supported ONFI version, 0 otherwise.
> 
> This allows NAND drivers past nand_scan_ident to set the best timings for the
> NAND chip.

Wrap changelogs around 72 characters, as git log indents them a bit.

> 
> Signed-off-by: Florian Fainelli <florian at openwrt.org>
> 
> ---
> drivers/mtd/nand/nand_base.c |  129 ++++++++++++++++++++++++++++++++++-------
>  include/linux/mtd/nand.h     |   68 ++++++++++++++++++++++
>  2 files changed, 175 insertions(+), 22 deletions(-)

Tested on what I assume is a non-ONFI chip, no breakage.

Please wrap this in a CONFIG option, so that we don't increase the
image size of boards that don't need to support this.

> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 21cc5a3..a3a0507 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2406,15 +2406,94 @@ static void nand_set_defaults(struct nand_chip *chip, int busw)
>  		chip->controller = &chip->hwcontrol;
>  }
>  
> +static u16 onfi_crc16(u16 crc, u8 const *p, size_t len)
> +{
> +	int i;
> +
> +	while (len--) {
> +		crc ^= *p++ << 8;
> +		for (i = 0; i < 8; i++)
> +			crc = (crc << 1) ^ ((crc & 0x8000) ? 0x8005 : 0);
> +	}
> +
> +	return crc;
> +}
> +
> +/*
> + * Check if the NAND chip is ONFI compliant, returns 1 if it is, 0 otherwise
> + */
> +static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
> +								int busw)
> +{

Keep lines under 80 columns.

> +
> +	/* try ONFI for unknow chip or LP */

"unknown".

> +	chip->cmdfunc(mtd, NAND_CMD_READID, 0x20, -1);
> +	if (chip->read_byte(mtd) != 'O' || chip->read_byte(mtd) != 'N' ||
> +		chip->read_byte(mtd) != 'F' || chip->read_byte(mtd) != 'I')
> +		return 0;

I'm not sure how this comment matches the code -- it looks like it's
checking for an explicit ONFI ID.  I don't see anything about unknown
chips or large pages.

> +	/* check version */
> +	val = le16_to_cpu(p->revision);
> +	if (val == 1 || val > (1 << 4)) {
> +		printk(KERN_INFO "%s: unsupported ONFI version: %d\n",
> +								__func__, val);
> +		return 0;
> +	}

Ideally I'd like to see continuation lines lined up nicely with
the start of arguments on the previous line, but at least
don't tab it all the way over to the right edge of the screen.

>  	chip->chipsize = (uint64_t)type->chipsize << 20;
> +	chip->onfi_version = 0;
> +
> +	ret = nand_flash_detect_onfi(mtd, chip, busw);
> +	if (ret)
> +		goto ident_done;

Move the non-ONFI code out into its own function, instead of using
goto.

> +	__le32 blocks_per_lun;
> +	u8 lun_count;
> +	u8 addr_cycles;
> +	u8 bits_per_cell;
> +	__le16 bb_per_lun;
> +	__le16 block_endurance;
> +	u8 guaranteed_good_blocks;
> +	__le16 guaranteed_block_endurance;
[snip]
> +} __attribute__((packed));

Sigh.  Someone on the standards body needs to learn about alignment.

-Scott