diff mbox

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

Message ID 1292019402-25433-2-git-send-email-florian@openwrt.org
State Superseded
Delegated to: Scott Wood
Headers show

Commit Message

Florian Fainelli Dec. 10, 2010, 10:16 p.m. UTC 
From: Florian Fainelli <florian@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.

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

Comments

Scott Wood Dec. 11, 2010, 12:15 a.m. UTC | #1 
On Fri, Dec 10, 2010 at 12:16:42PM -0000, Florian Fainelli wrote:
> From: Florian Fainelli <florian@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@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
Florian Fainelli Dec. 11, 2010, 5:08 p.m. UTC | #2 
On Saturday 11 December 2010 01:15:13 Scott Wood wrote:
> On Fri, Dec 10, 2010 at 12:16:42PM -0000, Florian Fainelli wrote:
> > From: Florian Fainelli <florian@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@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.

Makes sense, I will do 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.

It does not actually, I did not think about checking it when porting this from Linux.

> 
> > +	/* 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
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
diff mbox

Patch

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)
+{
+	struct nand_onfi_params *p = &chip->onfi_params;
+	int i;
+	int val;
+
+	/* try ONFI for unknow chip or LP */
+	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;
+
+	printk(KERN_INFO "ONFI flash detected\n");
+	chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
+	for (i = 0; i < 3; i++) {
+		chip->read_buf(mtd, (uint8_t *)p, sizeof(*p));
+		if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) ==
+						le16_to_cpu(p->crc)) {
+			printk(KERN_INFO "ONFI param page %d valid\n", i);
+			break;
+		}
+	}
+
+	if (i == 3)
+		return 0;
+
+	/* 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;
+	}
+
+	if (val & (1 << 4))
+		chip->onfi_version = 22;
+	else if (val & (1 << 3))
+		chip->onfi_version = 21;
+	else if (val & (1 << 2))
+		chip->onfi_version = 20;
+	else
+		chip->onfi_version = 10;
+
+	if (!mtd->name)
+		mtd->name = p->model;
+
+	mtd->writesize = le32_to_cpu(p->byte_per_page);
+	mtd->erasesize = le32_to_cpu(p->pages_per_block) * mtd->writesize;
+	mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
+	chip->chipsize = le32_to_cpu(p->blocks_per_lun) * mtd->erasesize;
+	busw = 0;
+	if (le16_to_cpu(p->features) & 1)
+		busw = NAND_BUSWIDTH_16;
+
+	chip->options &= ~NAND_CHIPOPTIONS_MSK;
+	chip->options |= (NAND_NO_READRDY |
+			NAND_NO_AUTOINCR) & NAND_CHIPOPTIONS_MSK;
+
+	return 1;
+}
+
 /*
  * Get the flash and manufacturer id and lookup if the type is supported
  */
 static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 						  struct nand_chip *chip,
-						  int busw, int *maf_id)
+						  int busw,
+						  int *maf_id, int *dev_id)
 {
 	struct nand_flash_dev *type = NULL;
-	int i, dev_id, maf_idx;
+	int i, ret, maf_idx;
 	int tmp_id, tmp_manf;
 
 	/* Select the device */
@@ -2431,7 +2510,7 @@  static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 
 	/* Read manufacturer and device IDs */
 	*maf_id = chip->read_byte(mtd);
-	dev_id = chip->read_byte(mtd);
+	*dev_id = chip->read_byte(mtd);
 
 	/* Try again to make sure, as some systems the bus-hold or other
 	 * interface concerns can cause random data which looks like a
@@ -2446,16 +2525,16 @@  static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 	tmp_manf = chip->read_byte(mtd);
 	tmp_id = chip->read_byte(mtd);
 
-	if (tmp_manf != *maf_id || tmp_id != dev_id) {
+	if (tmp_manf != *maf_id || tmp_id != *dev_id) {
 		printk(KERN_INFO "%s: second ID read did not match "
 		       "%02x,%02x against %02x,%02x\n", __func__,
-		       *maf_id, dev_id, tmp_manf, tmp_id);
+		       *maf_id, *dev_id, tmp_manf, tmp_id);
 		return ERR_PTR(-ENODEV);
 	}
 
 	/* Lookup the flash id */
 	for (i = 0; nand_flash_ids[i].name != NULL; i++) {
-		if (dev_id == nand_flash_ids[i].id) {
+		if (*dev_id == nand_flash_ids[i].id) {
 			type =  &nand_flash_ids[i];
 			break;
 		}
@@ -2464,10 +2543,10 @@  static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 	if (!type) {
 		/* supress warning if there is no nand */
 		if (*maf_id != 0x00 && *maf_id != 0xff &&
-		    dev_id  != 0x00 && dev_id  != 0xff)
+		    *dev_id  != 0x00 && *dev_id  != 0xff)
 			printk(KERN_INFO "%s: unknown NAND device: "
 				"Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
-				__func__, *maf_id, dev_id);
+				__func__, *maf_id, *dev_id);
 		return ERR_PTR(-ENODEV);
 	}
 
@@ -2475,6 +2554,11 @@  static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 		mtd->name = type->name;
 
 	chip->chipsize = (uint64_t)type->chipsize << 20;
+	chip->onfi_version = 0;
+
+	ret = nand_flash_detect_onfi(mtd, chip, busw);
+	if (ret)
+		goto ident_done;
 
 	/* Newer devices have all the information in additional id bytes */
 	if (!type->pagesize) {
@@ -2505,6 +2589,16 @@  static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 		busw = type->options & NAND_BUSWIDTH_16;
 	}
 
+	/* Get chip options, preserve non chip based options */
+	chip->options &= ~NAND_CHIPOPTIONS_MSK;
+	chip->options |= type->options & NAND_CHIPOPTIONS_MSK;
+
+	/*
+	 * Set chip as a default. Board drivers can override it, if necessary
+	 */
+ident_done:
+	chip->options |= NAND_NO_AUTOINCR;
+
 	/* Try to identify manufacturer */
 	for (maf_idx = 0; nand_manuf_ids[maf_idx].id != 0x0; maf_idx++) {
 		if (nand_manuf_ids[maf_idx].id == *maf_id)
@@ -2518,7 +2612,7 @@  static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 	if (busw != (chip->options & NAND_BUSWIDTH_16)) {
 		printk(KERN_INFO "NAND device: Manufacturer ID:"
 		       " 0x%02x, Chip ID: 0x%02x (%s %s)\n", *maf_id,
-		       dev_id, nand_manuf_ids[maf_idx].name, mtd->name);
+		       *dev_id, nand_manuf_ids[maf_idx].name, mtd->name);
 		printk(KERN_WARNING "NAND bus width %d instead %d bit\n",
 		       (chip->options & NAND_BUSWIDTH_16) ? 16 : 8,
 		       busw ? 16 : 8);
@@ -2541,15 +2635,6 @@  static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 	chip->badblockpos = mtd->writesize > 512 ?
 		NAND_LARGE_BADBLOCK_POS : NAND_SMALL_BADBLOCK_POS;
 
-	/* Get chip options, preserve non chip based options */
-	chip->options &= ~NAND_CHIPOPTIONS_MSK;
-	chip->options |= type->options & NAND_CHIPOPTIONS_MSK;
-
-	/*
-	 * Set chip as a default. Board drivers can override it, if necessary
-	 */
-	chip->options |= NAND_NO_AUTOINCR;
-
 	/* Check if chip is a not a samsung device. Do not clear the
 	 * options for chips which are not having an extended id.
 	 */
@@ -2567,7 +2652,7 @@  static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 		chip->cmdfunc = nand_command_lp;
 
 	MTDDEBUG (MTD_DEBUG_LEVEL0, "NAND device: Manufacturer ID:"
-	          " 0x%02x, Chip ID: 0x%02x (%s %s)\n", *maf_id, dev_id,
+	          " 0x%02x, Chip ID: 0x%02x (%s %s)\n", *maf_id, *dev_id,
 	          nand_manuf_ids[maf_idx].name, type->name);
 
 	return type;
@@ -2585,7 +2670,7 @@  static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
  */
 int nand_scan_ident(struct mtd_info *mtd, int maxchips)
 {
-	int i, busw, nand_maf_id;
+	int i, busw, nand_maf_id, nand_dev_id;
 	struct nand_chip *chip = mtd->priv;
 	struct nand_flash_dev *type;
 
@@ -2595,7 +2680,7 @@  int nand_scan_ident(struct mtd_info *mtd, int maxchips)
 	nand_set_defaults(chip, busw);
 
 	/* Read the flash type */
-	type = nand_get_flash_type(mtd, chip, busw, &nand_maf_id);
+	type = nand_get_flash_type(mtd, chip, busw, &nand_maf_id, &nand_dev_id);
 
 	if (IS_ERR(type)) {
 #ifndef CONFIG_SYS_NAND_QUIET_TEST
@@ -2614,7 +2699,7 @@  int nand_scan_ident(struct mtd_info *mtd, int maxchips)
 		chip->cmdfunc(mtd, NAND_CMD_READID, 0x00, -1);
 		/* Read manufacturer and device IDs */
 		if (nand_maf_id != chip->read_byte(mtd) ||
-		    type->id != chip->read_byte(mtd))
+		    nand_dev_id != chip->read_byte(mtd))
 			break;
 	}
 #ifdef DEBUG
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 7db87e1..beb35c6 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -215,6 +215,71 @@  typedef enum {
 /* Keep gcc happy */
 struct nand_chip;
 
+struct nand_onfi_params {
+	/* rev info and features block */
+	/* 'O' 'N' 'F' 'I'  */
+	u8 sig[4];
+	__le16 revision;
+	__le16 features;
+	__le16 opt_cmd;
+	u8 reserved[22];
+
+	/* manufacturer information block */
+	char manufacturer[12];
+	char model[20];
+	u8 jedec_id;
+	__le16 date_code;
+	u8 reserved2[13];
+
+	/* memory organization block */
+	__le32 byte_per_page;
+	__le16 spare_bytes_per_page;
+	__le32 data_bytes_per_ppage;
+	__le16 spare_bytes_per_ppage;
+	__le32 pages_per_block;
+	__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;
+	u8 programs_per_page;
+	u8 ppage_attr;
+	u8 ecc_bits;
+	u8 interleaved_bits;
+	u8 interleaved_ops;
+	u8 reserved3[13];
+
+	/* electrical parameter block */
+	u8 io_pin_capacitance_max;
+	__le16 async_timing_mode;
+	__le16 program_cache_timing_mode;
+	__le16 t_prog;
+	__le16 t_bers;
+	__le16 t_r;
+	__le16 t_ccs;
+	__le16 src_sync_timing_mode;
+	__le16 src_ssync_features;
+	__le16 clk_pin_capacitance_typ;
+	__le16 io_pin_capacitance_typ;
+	__le16 input_pin_capacitance_typ;
+	u8 input_pin_capacitance_max;
+	u8 driver_strenght_support;
+	__le16 t_int_r;
+	__le16 t_ald;
+	u8 reserved4[7];
+
+	/* vendor */
+	u8 reserved5[90];
+
+	 __le16 crc;
+} __attribute__((packed));
+
+#define ONFI_CRC_BASE	0x4F4E
+
+
 /**
  * struct nand_hw_control - Control structure for hardware controller (e.g ECC generator) shared among independent devices
  * @lock:               protection lock
@@ -405,6 +470,9 @@  struct nand_chip {
 	uint8_t		cellinfo;
 	int		badblockpos;
 
+	int		onfi_version;
+	struct nand_onfi_params onfi_params;
+
 	int 		state;
 
 	uint8_t		*oob_poi;