[PATCH v2 0/9] About the SLC/MLC

[PATCH v2 0/9] About the SLC/MLC

[Huang Shijie]

In current mtd code, the MTD_NANDFLASH is used to represent both the
SLC nand MLC(including the TLC). But we already have the MTD_MLCNANDFLASH
to stand for the MLC. What is worse is that the JFFS2 may run on the MLC
nand with current code. For the reason of READ/WRITE disturbance, the JFFS2
should runs on the SLC only,
       
This patch set tries to make clear what is the SLC/MLC by renaming,
adding the macros, adding helpers, adding the comments. ..

After this patch set, the gpmi can support the JFFS2 for some SLC NAND now 
(only when the left oob area is big enough).

Tested this patch set with ONFI SLC: M529F8G08ABACAWP

v1 --> v2:
	[0] rename the @cellinfo to @bits_per_cell
	[1] abandon the patch:
       		"mtd: add more information for the MTD_NANDFLASH case"
	[2] update the ABI for patch 8
	[3] misc

Huang Shijie (9):
  mtd: nand: rename the cellinfo to bits_per_cell
  mtd: set the cell information for ONFI nand
  mtd: print out the cell information for nand chip
  mtd: gpmi: rewrite the gpmi_ecc_write_oob() to support the jffs2
  mtd: add more comment for MTD_NANDFLASH/MTD_MLCNANDFLASH
  mtd: fix the wrong mtd->type for nand chip
  jffs2: init the ret with -EINVAL
  mtd: add MTD_MLCNANDFLASH case for mtd_type_show()
  mtd: add a helper to detect the nand type

 Documentation/ABI/testing/sysfs-class-mtd |    2 +-
 drivers/mtd/inftlcore.c                   |    2 +-
 drivers/mtd/mtdcore.c                     |    3 ++
 drivers/mtd/nand/denali.c                 |    2 +-
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c    |   31 +++++++++++++++++-------
 drivers/mtd/nand/nand_base.c              |   36 ++++++++++++++++++----------
 drivers/mtd/nftlcore.c                    |    2 +-
 drivers/mtd/ssfdc.c                       |    2 +-
 drivers/mtd/tests/nandbiterrs.c           |    2 +-
 drivers/mtd/tests/oobtest.c               |    2 +-
 drivers/mtd/tests/pagetest.c              |    2 +-
 drivers/mtd/tests/subpagetest.c           |    2 +-
 fs/jffs2/fs.c                             |    2 +-
 include/linux/mtd/mtd.h                   |    5 ++++
 include/linux/mtd/nand.h                  |   14 +++++++++-
 include/uapi/mtd/mtd-abi.h                |    4 +-
 16 files changed, 77 insertions(+), 36 deletions(-)

[PATCH v2 1/9] mtd: nand: rename the cellinfo to bits_per_cell

[Huang Shijie]

From: Huang Shijie <shijie8@gmail.com>

The @cellinfo fields contains unused information, such as write caching,
internal chip numbering, etc. But we only use it to check the SLC or MLC.

This patch tries to make it more clear and simple, renames the @cellinfo
to @bits_per_cell.

In order to avoiding the bisect issue, this patch also does the following
changes:
  (0) add a macro NAND_CI_CELLTYPE_SHIFT to avoid the hardcode.

  (1) add a helper to check the SLC : nand_is_slc()

  (2) add a helper to parse out the cell type : nand_get_bits_per_cell()

  (3) parse out the cell type for legacy nand chips and extended-ID
      chips, and the full-id nand chips.

Signed-off-by: Huang Shijie <shijie8@gmail.com>
Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/denali.c    |    2 +-
 drivers/mtd/nand/nand_base.c |   28 ++++++++++++++++++----------
 include/linux/mtd/nand.h     |   14 ++++++++++++--
 3 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 2ed2bb3..645721e 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1520,7 +1520,7 @@ int denali_init(struct denali_nand_info *denali)
 	 * so just let controller do 15bit ECC for MLC and 8bit ECC for
 	 * SLC if possible.
 	 * */
-	if (denali->nand.cellinfo & NAND_CI_CELLTYPE_MSK &&
+	if (!nand_is_slc(&denali->nand) &&
 			(denali->mtd.oobsize > (denali->bbtskipbytes +
 			ECC_15BITS * (denali->mtd.writesize /
 			ECC_SECTOR_SIZE)))) {
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 7ed4841..567cbcd 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3077,6 +3077,15 @@ static int nand_id_len(u8 *id_data, int arrlen)
 	return arrlen;
 }
 
+static int nand_get_bits_per_cell(u8 data)
+{
+	int bits;
+
+	bits = data & NAND_CI_CELLTYPE_MSK;
+	bits >>= NAND_CI_CELLTYPE_SHIFT;
+	return bits + 1;
+}
+
 /*
  * Many new NAND share similar device ID codes, which represent the size of the
  * chip. The rest of the parameters must be decoded according to generic or
@@ -3087,7 +3096,7 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 {
 	int extid, id_len;
 	/* The 3rd id byte holds MLC / multichip data */
-	chip->cellinfo = id_data[2];
+	chip->bits_per_cell = nand_get_bits_per_cell(id_data[2]);
 	/* The 4th id byte is the important one */
 	extid = id_data[3];
 
@@ -3103,8 +3112,7 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 	 * ID to decide what to do.
 	 */
 	if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG &&
-			(chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
-			id_data[5] != 0x00) {
+			!nand_is_slc(chip) && id_data[5] != 0x00) {
 		/* Calc pagesize */
 		mtd->writesize = 2048 << (extid & 0x03);
 		extid >>= 2;
@@ -3136,7 +3144,7 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 			(((extid >> 1) & 0x04) | (extid & 0x03));
 		*busw = 0;
 	} else if (id_len == 6 && id_data[0] == NAND_MFR_HYNIX &&
-			(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
+			!nand_is_slc(chip)) {
 		unsigned int tmp;
 
 		/* Calc pagesize */
@@ -3199,7 +3207,7 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 		 * - ID byte 5, bit[7]:    1 -> BENAND, 0 -> raw SLC
 		 */
 		if (id_len >= 6 && id_data[0] == NAND_MFR_TOSHIBA &&
-				!(chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
+				nand_is_slc(chip) &&
 				(id_data[5] & 0x7) == 0x6 /* 24nm */ &&
 				!(id_data[4] & 0x80) /* !BENAND */) {
 			mtd->oobsize = 32 * mtd->writesize >> 9;
@@ -3224,6 +3232,7 @@ static void nand_decode_id(struct mtd_info *mtd, struct nand_chip *chip,
 	mtd->oobsize = mtd->writesize / 32;
 	*busw = type->options & NAND_BUSWIDTH_16;
 
+	chip->bits_per_cell = nand_get_bits_per_cell(id_data[2]);
 	/*
 	 * Check for Spansion/AMD ID + repeating 5th, 6th byte since
 	 * some Spansion chips have erasesize that conflicts with size
@@ -3260,11 +3269,11 @@ static void nand_decode_bbm_options(struct mtd_info *mtd,
 	 * Micron devices with 2KiB pages and on SLC Samsung, Hynix, Toshiba,
 	 * AMD/Spansion, and Macronix.  All others scan only the first page.
 	 */
-	if ((chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
+	if (!nand_is_slc(chip) &&
 			(maf_id == NAND_MFR_SAMSUNG ||
 			 maf_id == NAND_MFR_HYNIX))
 		chip->bbt_options |= NAND_BBT_SCANLASTPAGE;
-	else if ((!(chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
+	else if ((nand_is_slc(chip) &&
 				(maf_id == NAND_MFR_SAMSUNG ||
 				 maf_id == NAND_MFR_HYNIX ||
 				 maf_id == NAND_MFR_TOSHIBA ||
@@ -3288,7 +3297,7 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
 		mtd->erasesize = type->erasesize;
 		mtd->oobsize = type->oobsize;
 
-		chip->cellinfo = id_data[2];
+		chip->bits_per_cell = nand_get_bits_per_cell(id_data[2]);
 		chip->chipsize = (uint64_t)type->chipsize << 20;
 		chip->options |= type->options;
 		chip->ecc_strength_ds = NAND_ECC_STRENGTH(type);
@@ -3740,8 +3749,7 @@ int nand_scan_tail(struct mtd_info *mtd)
 	chip->ecc.total = chip->ecc.steps * chip->ecc.bytes;
 
 	/* Allow subpage writes up to ecc.steps. Not possible for MLC flash */
-	if (!(chip->options & NAND_NO_SUBPAGE_WRITE) &&
-	    !(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
+	if (!(chip->options & NAND_NO_SUBPAGE_WRITE) && nand_is_slc(chip)) {
 		switch (chip->ecc.steps) {
 		case 2:
 			mtd->subpage_sft = 1;
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index ac8e89d..6e9106b 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -198,6 +198,7 @@ typedef enum {
 /* Cell info constants */
 #define NAND_CI_CHIPNR_MSK	0x03
 #define NAND_CI_CELLTYPE_MSK	0x0C
+#define NAND_CI_CELLTYPE_SHIFT	2
 
 /* Keep gcc happy */
 struct nand_chip;
@@ -477,7 +478,7 @@ struct nand_buffers {
  * @badblockbits:	[INTERN] minimum number of set bits in a good block's
  *			bad block marker position; i.e., BBM == 11110111b is
  *			not bad when badblockbits == 7
- * @cellinfo:		[INTERN] MLC/multichip data from chip ident
+ * @bits_per_cell:	[INTERN] the bits of per cell. i.e., 1 means SLC.
  * @ecc_strength_ds:	[INTERN] ECC correctability from the datasheet.
  *			Minimum amount of bit errors per @ecc_step_ds guaranteed
  *			to be correctable. If unknown, set to zero.
@@ -559,7 +560,7 @@ struct nand_chip {
 	int pagebuf;
 	unsigned int pagebuf_bitflips;
 	int subpagesize;
-	uint8_t cellinfo;
+	uint8_t bits_per_cell;
 	uint16_t ecc_strength_ds;
 	uint16_t ecc_step_ds;
 	int badblockpos;
@@ -797,4 +798,13 @@ static inline int onfi_get_sync_timing_mode(struct nand_chip *chip)
 	return le16_to_cpu(chip->onfi_params.src_sync_timing_mode);
 }
 
+/*
+ * Check if it is a SLC nand.
+ * The !nand_is_slc() can be used to check the MLC/TLC nand chips.
+ * We do not distinguish the MLC and TLC now.
+ */
+static inline bool nand_is_slc(struct nand_chip *chip)
+{
+	return chip->bits_per_cell == 1;
+}
 #endif /* __LINUX_MTD_NAND_H */
-- 
1.7.1

[PATCH v2 2/9] mtd: set the cell information for ONFI nand

[Huang Shijie]

The current code does not set the SLC/MLC information for onfi nand.
(This makes that the kernel treats all the onfi nand as SLC nand.)

This patch fills the cell type information for ONFI nands.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/nand_base.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 567cbcd..69c4b25 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2988,6 +2988,7 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
 	mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
 	chip->chipsize = le32_to_cpu(p->blocks_per_lun);
 	chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
+	chip->bits_per_cell = p->bits_per_cell;
 
 	if (onfi_feature(chip) & ONFI_FEATURE_16_BIT_BUS)
 		*busw = NAND_BUSWIDTH_16;
-- 
1.7.1

[PATCH v2 3/9] mtd: print out the cell information for nand chip

[Huang Shijie]

Print out the cell information for nand chip.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/nand_base.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 69c4b25..8b487d5 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3454,10 +3454,11 @@ ident_done:
 		chip->cmdfunc = nand_command_lp;
 
 	pr_info("NAND device: Manufacturer ID: 0x%02x, Chip ID: 0x%02x (%s %s),"
-		" %dMiB, page size: %d, OOB size: %d\n",
+		" %dMiB, %s, page size: %d, OOB size: %d\n",
 		*maf_id, *dev_id, nand_manuf_ids[maf_idx].name,
 		chip->onfi_version ? chip->onfi_params.model : type->name,
-		(int)(chip->chipsize >> 20), mtd->writesize, mtd->oobsize);
+		(int)(chip->chipsize >> 20), nand_is_slc(chip) ? "SLC" : "MLC",
+		mtd->writesize, mtd->oobsize);
 
 	return type;
 }
-- 
1.7.1

[PATCH v2 4/9] mtd: gpmi: rewrite the gpmi_ecc_write_oob() to support the jffs2

[Huang Shijie]

When we use the ECC info which is get from the nand chip's datasheet,
we may have some freed oob area now.

This patch rewrites the gpmi_ecc_write_oob() to implement the ecc.write_oob().
We also update the comment for gpmi_hw_ecclayout.

Yes! We can support the JFFS2 for the SLC nand now.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   31 ++++++++++++++++++++++---------
 1 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 9c89e80..cc0306b 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -45,7 +45,10 @@ static struct nand_bbt_descr gpmi_bbt_descr = {
 	.pattern	= scan_ff_pattern
 };
 
-/*  We will use all the (page + OOB). */
+/*
+ * We may change the layout if we can get the ECC info from the datasheet,
+ * else we will use all the (page + OOB).
+ */
 static struct nand_ecclayout gpmi_hw_ecclayout = {
 	.eccbytes = 0,
 	.eccpos = { 0, },
@@ -1263,14 +1266,24 @@ static int gpmi_ecc_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 static int
 gpmi_ecc_write_oob(struct mtd_info *mtd, struct nand_chip *chip, int page)
 {
-	/*
-	 * The BCH will use all the (page + oob).
-	 * Our gpmi_hw_ecclayout can only prohibit the JFFS2 to write the oob.
-	 * But it can not stop some ioctls such MEMWRITEOOB which uses
-	 * MTD_OPS_PLACE_OOB. So We have to implement this function to prohibit
-	 * these ioctls too.
-	 */
-	return -EPERM;
+	struct nand_oobfree *of = gpmi_hw_ecclayout.oobfree;
+	int status = 0;
+
+	/* Do we have available oob area? */
+	if (!of->length)
+		return -EPERM;
+
+	if (!nand_is_slc(chip))
+		return -EPERM;
+
+	pr_debug("page number is %d\n", page);
+
+	chip->cmdfunc(mtd, NAND_CMD_SEQIN, mtd->writesize + of->offset, page);
+	chip->write_buf(mtd, chip->oob_poi + of->offset, of->length);
+	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+
+	status = chip->waitfunc(mtd, chip);
+	return status & NAND_STATUS_FAIL ? -EIO : 0;
 }
 
 static int gpmi_block_markbad(struct mtd_info *mtd, loff_t ofs)
-- 
1.7.1

[PATCH v2 5/9] mtd: add more comment for MTD_NANDFLASH/MTD_MLCNANDFLASH

[Huang Shijie]

In current code, the MTD_NANDFLASH is used to represent both the SLC and
MLC. It is confusing to us.

By adding an explict comment about these two macros, this patch makes it
clear that:
	MTD_NANDFLASH    : stands for SLC nand,
	MTD_MLCNANDFLASH : stands for MLC nand(including TLC).

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 include/uapi/mtd/mtd-abi.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/mtd/mtd-abi.h b/include/uapi/mtd/mtd-abi.h
index 36eace0..16a9406 100644
--- a/include/uapi/mtd/mtd-abi.h
+++ b/include/uapi/mtd/mtd-abi.h
@@ -94,10 +94,10 @@ struct mtd_write_req {
 #define MTD_RAM			1
 #define MTD_ROM			2
 #define MTD_NORFLASH		3
-#define MTD_NANDFLASH		4
+#define MTD_NANDFLASH		4	/* stand for SLC nand */
 #define MTD_DATAFLASH		6
 #define MTD_UBIVOLUME		7
-#define MTD_MLCNANDFLASH	8
+#define MTD_MLCNANDFLASH	8	/* stand for MLC nand(including TLC) */
 
 #define MTD_WRITEABLE		0x400	/* Device is writeable */
 #define MTD_BIT_WRITEABLE	0x800	/* Single bits can be flipped */
-- 
1.7.1

[PATCH v2 6/9] mtd: fix the wrong mtd->type for nand chip

[Huang Shijie]

Current code sets the mtd->type with MTD_NANDFLASH for both
SLC and MLC. So the jffs2 may supports the MLC nand, but in actually,
the jffs2 should not support the MLC.

This patch uses the nand_is_slc() to check the nand cell type,
and set the mtd->type with the right nand type.

After this patch, the jffs2 only can support the SLC nand.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/nand_base.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 8b487d5..f2bb925 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3776,7 +3776,7 @@ int nand_scan_tail(struct mtd_info *mtd)
 		chip->options |= NAND_SUBPAGE_READ;
 
 	/* Fill in remaining MTD driver data */
-	mtd->type = MTD_NANDFLASH;
+	mtd->type = nand_is_slc(chip) ? MTD_NANDFLASH : MTD_MLCNANDFLASH;
 	mtd->flags = (chip->options & NAND_ROM) ? MTD_CAP_ROM :
 						MTD_CAP_NANDFLASH;
 	mtd->_erase = nand_erase;
-- 
1.7.1

[PATCH v2 7/9] jffs2: init the ret with -EINVAL

[Huang Shijie]

If the media is not SLC nand, dataflash, Sibley flash or a
ubi volume, we should return -EINVAL to the caller.
The caller should exit in this case.

Current code returns 0 in this case which is not proper.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 fs/jffs2/fs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c
index fe3c052..0452445 100644
--- a/fs/jffs2/fs.c
+++ b/fs/jffs2/fs.c
@@ -702,7 +702,7 @@ void jffs2_gc_release_page(struct jffs2_sb_info *c,
 }
 
 static int jffs2_flash_setup(struct jffs2_sb_info *c) {
-	int ret = 0;
+	int ret = -EINVAL;
 
 	if (jffs2_cleanmarker_oob(c)) {
 		/* NAND flash... do setup accordingly */
-- 
1.7.1

[PATCH v2 8/9] mtd: add MTD_MLCNANDFLASH case for mtd_type_show()

[Huang Shijie]

The current mtd_type_show() misses the MTD_MLCNANDFLASH case.
This patch adds the case for it, and also updates the ABI.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 Documentation/ABI/testing/sysfs-class-mtd |    2 +-
 drivers/mtd/mtdcore.c                     |    3 +++
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-mtd b/Documentation/ABI/testing/sysfs-class-mtd
index a795582..8104410 100644
--- a/Documentation/ABI/testing/sysfs-class-mtd
+++ b/Documentation/ABI/testing/sysfs-class-mtd
@@ -104,7 +104,7 @@ Description:
 		One of the following ASCII strings, representing the device
 		type:
 
-		absent, ram, rom, nor, nand, dataflash, ubi, unknown
+		absent, ram, rom, nor, nand, mlc-nand, dataflash, ubi, unknown
 
 What:		/sys/class/mtd/mtdX/writesize
 Date:		April 2009
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 5e14d54..92311a5 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -157,6 +157,9 @@ static ssize_t mtd_type_show(struct device *dev,
 	case MTD_UBIVOLUME:
 		type = "ubi";
 		break;
+	case MTD_MLCNANDFLASH:
+		type = "mlc-nand";
+		break;
 	default:
 		type = "unknown";
 	}
-- 
1.7.1

[PATCH v2 9/9] mtd: add a helper to detect the nand type

[Huang Shijie]

This helper detects that whether the mtd's type is nand type.

Now, it's clear that the MTD_NANDFLASH stands for SLC nand only.
So use the mtd_type_is_nand() to replace the old check method
to do the nand type (include the SLC and MLC) check.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/inftlcore.c         |    2 +-
 drivers/mtd/nftlcore.c          |    2 +-
 drivers/mtd/ssfdc.c             |    2 +-
 drivers/mtd/tests/nandbiterrs.c |    2 +-
 drivers/mtd/tests/oobtest.c     |    2 +-
 drivers/mtd/tests/pagetest.c    |    2 +-
 drivers/mtd/tests/subpagetest.c |    2 +-
 include/linux/mtd/mtd.h         |    5 +++++
 8 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/inftlcore.c b/drivers/mtd/inftlcore.c
index 3af3514..b66b541 100644
--- a/drivers/mtd/inftlcore.c
+++ b/drivers/mtd/inftlcore.c
@@ -50,7 +50,7 @@ static void inftl_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
 	struct INFTLrecord *inftl;
 	unsigned long temp;
 
-	if (mtd->type != MTD_NANDFLASH || mtd->size > UINT_MAX)
+	if (!mtd_type_is_nand(mtd) || mtd->size > UINT_MAX)
 		return;
 	/* OK, this is moderately ugly.  But probably safe.  Alternatives? */
 	if (memcmp(mtd->name, "DiskOnChip", 10))
diff --git a/drivers/mtd/nftlcore.c b/drivers/mtd/nftlcore.c
index c5f4ebf..46f27de 100644
--- a/drivers/mtd/nftlcore.c
+++ b/drivers/mtd/nftlcore.c
@@ -50,7 +50,7 @@ static void nftl_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
 	struct NFTLrecord *nftl;
 	unsigned long temp;
 
-	if (mtd->type != MTD_NANDFLASH || mtd->size > UINT_MAX)
+	if (!mtd_type_is_nand(mtd) || mtd->size > UINT_MAX)
 		return;
 	/* OK, this is moderately ugly.  But probably safe.  Alternatives? */
 	if (memcmp(mtd->name, "DiskOnChip", 10))
diff --git a/drivers/mtd/ssfdc.c b/drivers/mtd/ssfdc.c
index ab2a52a..daf82ba 100644
--- a/drivers/mtd/ssfdc.c
+++ b/drivers/mtd/ssfdc.c
@@ -290,7 +290,7 @@ static void ssfdcr_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
 	int cis_sector;
 
 	/* Check for small page NAND flash */
-	if (mtd->type != MTD_NANDFLASH || mtd->oobsize != OOB_SIZE ||
+	if (!mtd_type_is_nand(mtd) || mtd->oobsize != OOB_SIZE ||
 	    mtd->size > UINT_MAX)
 		return;
 
diff --git a/drivers/mtd/tests/nandbiterrs.c b/drivers/mtd/tests/nandbiterrs.c
index 3cd3aab..6f97615 100644
--- a/drivers/mtd/tests/nandbiterrs.c
+++ b/drivers/mtd/tests/nandbiterrs.c
@@ -349,7 +349,7 @@ static int __init mtd_nandbiterrs_init(void)
 		goto exit_mtddev;
 	}
 
-	if (mtd->type != MTD_NANDFLASH) {
+	if (!mtd_type_is_nand(mtd)) {
 		pr_info("this test requires NAND flash\n");
 		err = -ENODEV;
 		goto exit_nand;
diff --git a/drivers/mtd/tests/oobtest.c b/drivers/mtd/tests/oobtest.c
index ff35c46..2e9e2d1 100644
--- a/drivers/mtd/tests/oobtest.c
+++ b/drivers/mtd/tests/oobtest.c
@@ -289,7 +289,7 @@ static int __init mtd_oobtest_init(void)
 		return err;
 	}
 
-	if (mtd->type != MTD_NANDFLASH) {
+	if (!mtd_type_is_nand(mtd)) {
 		pr_info("this test requires NAND flash\n");
 		goto out;
 	}
diff --git a/drivers/mtd/tests/pagetest.c b/drivers/mtd/tests/pagetest.c
index 44b96e9..ed2d3f6 100644
--- a/drivers/mtd/tests/pagetest.c
+++ b/drivers/mtd/tests/pagetest.c
@@ -353,7 +353,7 @@ static int __init mtd_pagetest_init(void)
 		return err;
 	}
 
-	if (mtd->type != MTD_NANDFLASH) {
+	if (!mtd_type_is_nand(mtd)) {
 		pr_info("this test requires NAND flash\n");
 		goto out;
 	}
diff --git a/drivers/mtd/tests/subpagetest.c b/drivers/mtd/tests/subpagetest.c
index e2c0adf..a876371 100644
--- a/drivers/mtd/tests/subpagetest.c
+++ b/drivers/mtd/tests/subpagetest.c
@@ -299,7 +299,7 @@ static int __init mtd_subpagetest_init(void)
 		return err;
 	}
 
-	if (mtd->type != MTD_NANDFLASH) {
+	if (!mtd_type_is_nand(mtd)) {
 		pr_info("this test requires NAND flash\n");
 		goto out;
 	}
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index f9bfe52..88409b8 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -354,6 +354,11 @@ static inline int mtd_has_oob(const struct mtd_info *mtd)
 	return mtd->_read_oob && mtd->_write_oob;
 }
 
+static inline int mtd_type_is_nand(const struct mtd_info *mtd)
+{
+	return mtd->type == MTD_NANDFLASH || mtd->type == MTD_MLCNANDFLASH;
+}
+
 static inline int mtd_can_have_bb(const struct mtd_info *mtd)
 {
 	return !!mtd->_block_isbad;
-- 
1.7.1

[PATCH v2 append] mtd: mtd-abi: add a helper to detect the nand type

[Huang Shijie]

The helper is for user applications, and it is just a copy of
the kernel helper: mtd_type_is_nand();

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 include/uapi/mtd/mtd-abi.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/include/uapi/mtd/mtd-abi.h b/include/uapi/mtd/mtd-abi.h
index 16a9406..ad8e867 100644
--- a/include/uapi/mtd/mtd-abi.h
+++ b/include/uapi/mtd/mtd-abi.h
@@ -275,4 +275,9 @@ enum mtd_file_modes {
 	MTD_FILE_MODE_RAW,
 };
 
+static inline bool mtd_type_is_nand_user(const struct mtd_info_user *mtd)
+{
+	return mtd->type == MTD_NANDFLASH || mtd->type == MTD_MLCNANDFLASH;
+}
+
 #endif /* __MTD_ABI_H__ */
-- 
1.7.1

[PATCH append fix] mtd: mtd-abi: add a helper to detect the nand type

[Huang Shijie]

The helper is for user applications, and it is just a copy of
the kernel helper: mtd_type_is_nand();

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
If use the `bool`, we may meet the compiler error in the mtd-utils.

So change the return type from bool to int.

---
 include/uapi/mtd/mtd-abi.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/include/uapi/mtd/mtd-abi.h b/include/uapi/mtd/mtd-abi.h
index 16a9406..66c2b0c 100644
--- a/include/uapi/mtd/mtd-abi.h
+++ b/include/uapi/mtd/mtd-abi.h
@@ -275,4 +275,9 @@ enum mtd_file_modes {
 	MTD_FILE_MODE_RAW,
 };
 
+static inline int mtd_type_is_nand_user(const struct mtd_info_user *mtd)
+{
+	return mtd->type == MTD_NANDFLASH || mtd->type == MTD_MLCNANDFLASH;
+}
+
 #endif /* __MTD_ABI_H__ */
-- 
1.7.1

Re: [PATCH v2 1/9] mtd: nand: rename the cellinfo to bits_per_cell

[Brian Norris]

On Mon, Aug 19, 2013 at 10:31:10AM +0800, Huang Shijie wrote:
> From: Huang Shijie <shijie8@gmail.com>
> 
> The @cellinfo fields contains unused information, such as write caching,
> internal chip numbering, etc. But we only use it to check the SLC or MLC.
> 
> This patch tries to make it more clear and simple, renames the @cellinfo
> to @bits_per_cell.
> 
> In order to avoiding the bisect issue, this patch also does the following
> changes:
>   (0) add a macro NAND_CI_CELLTYPE_SHIFT to avoid the hardcode.
> 
>   (1) add a helper to check the SLC : nand_is_slc()
> 
>   (2) add a helper to parse out the cell type : nand_get_bits_per_cell()
> 
>   (3) parse out the cell type for legacy nand chips and extended-ID
>       chips, and the full-id nand chips.
> 
> Signed-off-by: Huang Shijie <shijie8@gmail.com>
> Signed-off-by: Huang Shijie <b32955@freescale.com>

Did you really want two signed-off-by lines? :)

> ---
>  drivers/mtd/nand/denali.c    |    2 +-
>  drivers/mtd/nand/nand_base.c |   28 ++++++++++++++++++----------
>  include/linux/mtd/nand.h     |   14 ++++++++++++--
>  3 files changed, 31 insertions(+), 13 deletions(-)

[...]

> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 7ed4841..567cbcd 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -3077,6 +3077,15 @@ static int nand_id_len(u8 *id_data, int arrlen)
>  	return arrlen;
>  }
>  
> +static int nand_get_bits_per_cell(u8 data)

Maybe make the parameter name 'cellinfo' to make it clearer? And maybe
a short comment to say that it extracts this information from the 3rd
byte of the extended ID de-facto standard?

> +{
> +	int bits;
> +
> +	bits = data & NAND_CI_CELLTYPE_MSK;
> +	bits >>= NAND_CI_CELLTYPE_SHIFT;
> +	return bits + 1;
> +}
> +
>  /*
>   * Many new NAND share similar device ID codes, which represent the size of the
>   * chip. The rest of the parameters must be decoded according to generic or

[...]

> @@ -3224,6 +3232,7 @@ static void nand_decode_id(struct mtd_info *mtd, struct nand_chip *chip,
>  	mtd->oobsize = mtd->writesize / 32;
>  	*busw = type->options & NAND_BUSWIDTH_16;
>  
> +	chip->bits_per_cell = nand_get_bits_per_cell(id_data[2]);

This is wrong. The NAND covered by nand_decode_id() do not have an
extended ID, so the third ID byte is not meaningful. But all of these
should be SLC, so just:

	/* All legacy ID NAND are small-page, SLC */
	chip->bits_per_cell = 1;

This also highlights the problem I was alluding to if we were to try to
maintain the whole cellinfo field for all NAND; we never guaranteed that
the other bitfields of cellinfo were consistent for extended ID vs.
legacy ID NAND. For legacy ID NAND, cellinfo was always 0, and I don't
know off the top of my head whether 0 makes sense for all the other
bitfields within cellinfo.

>  	/*
>  	 * Check for Spansion/AMD ID + repeating 5th, 6th byte since
>  	 * some Spansion chips have erasesize that conflicts with size

[...]

The rest of the patch looks good. Thanks for doing this!

Brian

Re: [PATCH v2 3/9] mtd: print out the cell information for nand chip

[Brian Norris]

On Mon, Aug 19, 2013 at 10:31:12AM +0800, Huang Shijie wrote:
> Print out the cell information for nand chip.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/nand/nand_base.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 69c4b25..8b487d5 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -3454,10 +3454,11 @@ ident_done:
>  		chip->cmdfunc = nand_command_lp;
>  
>  	pr_info("NAND device: Manufacturer ID: 0x%02x, Chip ID: 0x%02x (%s %s),"
> -		" %dMiB, page size: %d, OOB size: %d\n",
> +		" %dMiB, %s, page size: %d, OOB size: %d\n",
>  		*maf_id, *dev_id, nand_manuf_ids[maf_idx].name,
>  		chip->onfi_version ? chip->onfi_params.model : type->name,
> -		(int)(chip->chipsize >> 20), mtd->writesize, mtd->oobsize);
> +		(int)(chip->chipsize >> 20), nand_is_slc(chip) ? "SLC" : "MLC",
> +		mtd->writesize, mtd->oobsize);

This message is getting mighty long (approx. 120 characters when
printed). Are you sure we need all this? Maybe split into two separate
pr_info's sometime. But I'm OK with merging this for now.

>  
>  	return type;
>  }

Brian

Re: [PATCH v2 4/9] mtd: gpmi: rewrite the gpmi_ecc_write_oob() to support the jffs2

[Brian Norris]

On Mon, Aug 19, 2013 at 10:31:13AM +0800, Huang Shijie wrote:
> When we use the ECC info which is get from the nand chip's datasheet,
> we may have some freed oob area now.
> 
> This patch rewrites the gpmi_ecc_write_oob() to implement the ecc.write_oob().
> We also update the comment for gpmi_hw_ecclayout.
> 
> Yes! We can support the JFFS2 for the SLC nand now.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   31 ++++++++++++++++++++++---------
>  1 files changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 9c89e80..cc0306b 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -45,7 +45,10 @@ static struct nand_bbt_descr gpmi_bbt_descr = {
>  	.pattern	= scan_ff_pattern
>  };
>  
> -/*  We will use all the (page + OOB). */
> +/*
> + * We may change the layout if we can get the ECC info from the datasheet,
> + * else we will use all the (page + OOB).
> + */
>  static struct nand_ecclayout gpmi_hw_ecclayout = {
>  	.eccbytes = 0,
>  	.eccpos = { 0, },
> @@ -1263,14 +1266,24 @@ static int gpmi_ecc_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
>  static int
>  gpmi_ecc_write_oob(struct mtd_info *mtd, struct nand_chip *chip, int page)
>  {
> -	/*
> -	 * The BCH will use all the (page + oob).
> -	 * Our gpmi_hw_ecclayout can only prohibit the JFFS2 to write the oob.
> -	 * But it can not stop some ioctls such MEMWRITEOOB which uses
> -	 * MTD_OPS_PLACE_OOB. So We have to implement this function to prohibit
> -	 * these ioctls too.
> -	 */
> -	return -EPERM;
> +	struct nand_oobfree *of = gpmi_hw_ecclayout.oobfree;

Why do you directly access your static layout? I think you should use
the proper indirection, through chip->ecc.layout. (There's a similar
issue with set_geometry_by_ecc_info() currently.)

[BTW, I was about to recommend nand_chip.ecclayout, but it looks like
that is just a dead field. It's not initialized anywhere, AFAICT, and if
it's used anywhere, it's more likely a bug than anything else...]

> +	int status = 0;
> +
> +	/* Do we have available oob area? */
> +	if (!of->length)
> +		return -EPERM;
> +
> +	if (!nand_is_slc(chip))
> +		return -EPERM;
> +
> +	pr_debug("page number is %d\n", page);
> +
> +	chip->cmdfunc(mtd, NAND_CMD_SEQIN, mtd->writesize + of->offset, page);
> +	chip->write_buf(mtd, chip->oob_poi + of->offset, of->length);
> +	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> +
> +	status = chip->waitfunc(mtd, chip);
> +	return status & NAND_STATUS_FAIL ? -EIO : 0;
>  }
>  
>  static int gpmi_block_markbad(struct mtd_info *mtd, loff_t ofs)

Brian

Re: [PATCH v2 7/9] jffs2: init the ret with -EINVAL

[Brian Norris]

On Mon, Aug 19, 2013 at 10:31:16AM +0800, Huang Shijie wrote:
> If the media is not SLC nand, dataflash, Sibley flash or a
> ubi volume, we should return -EINVAL to the caller.
> The caller should exit in this case.
> 
> Current code returns 0 in this case which is not proper.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  fs/jffs2/fs.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c
> index fe3c052..0452445 100644
> --- a/fs/jffs2/fs.c
> +++ b/fs/jffs2/fs.c
> @@ -702,7 +702,7 @@ void jffs2_gc_release_page(struct jffs2_sb_info *c,
>  }
>  
>  static int jffs2_flash_setup(struct jffs2_sb_info *c) {
> -	int ret = 0;
> +	int ret = -EINVAL;

This doesn't look correct to me. This function isn't meant to screen out
unsupported flash; it's only for doing special things for certain flash.
In fact, I think this will break many NOR flash which currently pass
through this function without doing anything.

If you really want to make MLC NAND fail, you should do it with an
explicit check for MLC, and you probably don't want it in this function.
Maybe in jffs2_do_fill_super(), near the other 'if (c->mtd->type == ...)'
checks (but outside of the #ifndef, of course).

>  
>  	if (jffs2_cleanmarker_oob(c)) {
>  		/* NAND flash... do setup accordingly */

Brian

Re: [PATCH v2 4/9] mtd: gpmi: rewrite the gpmi_ecc_write_oob() to support the jffs2

[Brian Norris]

On Sat, Aug 24, 2013 at 03:18:13PM -0400, Huang Shijie wrote:
> On Fri, Aug 23, 2013 at 11:19:16PM -0700, Brian Norris wrote:
> > On Mon, Aug 19, 2013 at 10:31:13AM +0800, Huang Shijie wrote:
> > > -	 */
> > > -	return -EPERM;
> > > +	struct nand_oobfree *of = gpmi_hw_ecclayout.oobfree;
> > 
> > Why do you directly access your static layout? I think you should use
> > the proper indirection, through chip->ecc.layout. (There's a similar
> 		
> yes, it is better to use the chip->ecc.layout.
> 
> > issue with set_geometry_by_ecc_info() currently.)
> 
> Not a same issue.  The set_geometry_by_ecc_info() is called before we
> call the nand_scan_tail(). so it should not changed.

OK.

> > 
> > [BTW, I was about to recommend nand_chip.ecclayout, but it looks like
> > that is just a dead field. It's not initialized anywhere, AFAICT, and if
> The jffs2 write may also uses the ecclayout. 

It uses mtd_info.ecclayout. I'm pretty sure nand_chip.ecclayout is never
used (except for a debug print).

Brian

Re: [PATCH v2 4/9] mtd: gpmi: rewrite the gpmi_ecc_write_oob() to support the jffs2

[Huang Shijie]

On Fri, Aug 23, 2013 at 11:19:16PM -0700, Brian Norris wrote:
> On Mon, Aug 19, 2013 at 10:31:13AM +0800, Huang Shijie wrote:
> > -	 */
> > -	return -EPERM;
> > +	struct nand_oobfree *of = gpmi_hw_ecclayout.oobfree;
> 
> Why do you directly access your static layout? I think you should use
> the proper indirection, through chip->ecc.layout. (There's a similar
		
yes, it is better to use the chip->ecc.layout.

> issue with set_geometry_by_ecc_info() currently.)

Not a same issue.  The set_geometry_by_ecc_info() is called before we
call the nand_scan_tail(). so it should not changed.

> 
> [BTW, I was about to recommend nand_chip.ecclayout, but it looks like
> that is just a dead field. It's not initialized anywhere, AFAICT, and if
The jffs2 write may also uses the ecclayout. 

thanks
Huang Shijie

Re: [PATCH v2 3/9] mtd: print out the cell information for nand chip

[Ezequiel Garcia]

On Fri, Aug 23, 2013 at 10:58:21PM -0700, Brian Norris wrote:
> On Mon, Aug 19, 2013 at 10:31:12AM +0800, Huang Shijie wrote:
> > Print out the cell information for nand chip.
> > 
> > Signed-off-by: Huang Shijie <b32955@freescale.com>
> > ---
> >  drivers/mtd/nand/nand_base.c |    5 +++--
> >  1 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > index 69c4b25..8b487d5 100644
> > --- a/drivers/mtd/nand/nand_base.c
> > +++ b/drivers/mtd/nand/nand_base.c
> > @@ -3454,10 +3454,11 @@ ident_done:
> >  		chip->cmdfunc = nand_command_lp;
> >  
> >  	pr_info("NAND device: Manufacturer ID: 0x%02x, Chip ID: 0x%02x (%s %s),"
> > -		" %dMiB, page size: %d, OOB size: %d\n",
> > +		" %dMiB, %s, page size: %d, OOB size: %d\n",
> >  		*maf_id, *dev_id, nand_manuf_ids[maf_idx].name,
> >  		chip->onfi_version ? chip->onfi_params.model : type->name,
> > -		(int)(chip->chipsize >> 20), mtd->writesize, mtd->oobsize);
> > +		(int)(chip->chipsize >> 20), nand_is_slc(chip) ? "SLC" : "MLC",
> > +		mtd->writesize, mtd->oobsize);
> 
> This message is getting mighty long (approx. 120 characters when
> printed). Are you sure we need all this? Maybe split into two separate
> pr_info's sometime. But I'm OK with merging this for now.
> 

Hm.. well, although it's still a two-line message, I also don't like
to see this growing.

I think we should try to answer: why would anyone want to see any of
this when the kernel boots?

So maybe we should pr_info only the most important information,
and then either: 1) pr_debug the rest, or 2) show it in some debugfs files.

PS: Can't avoid thinking of Eric Raymond's rule of *silence* :-)
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v2 1/9] mtd: nand: rename the cellinfo to bits_per_cell

[Huang Shijie]

On Fri, Aug 23, 2013 at 10:49:13PM -0700, Brian Norris wrote:
> On Mon, Aug 19, 2013 at 10:31:10AM +0800, Huang Shijie wrote:
> > +static int nand_get_bits_per_cell(u8 data)
> 
> Maybe make the parameter name 'cellinfo' to make it clearer? And maybe
> a short comment to say that it extracts this information from the 3rd
> byte of the extended ID de-facto standard?
yes, the "cellinfo" is better.

> 
> > +{
> > +	int bits;
> > +
> > +	bits = data & NAND_CI_CELLTYPE_MSK;
> > +	bits >>= NAND_CI_CELLTYPE_SHIFT;
> > +	return bits + 1;
> > +}
> > +
> >  /*
> >   * Many new NAND share similar device ID codes, which represent the size of the
> >   * chip. The rest of the parameters must be decoded according to generic or
> 
> [...]
> 
> > @@ -3224,6 +3232,7 @@ static void nand_decode_id(struct mtd_info *mtd, struct nand_chip *chip,
> >  	mtd->oobsize = mtd->writesize / 32;
> >  	*busw = type->options & NAND_BUSWIDTH_16;
> >  
> > +	chip->bits_per_cell = nand_get_bits_per_cell(id_data[2]);
> 
> This is wrong. The NAND covered by nand_decode_id() do not have an
> extended ID, so the third ID byte is not meaningful. But all of these
> should be SLC, so just:
> 
> 	/* All legacy ID NAND are small-page, SLC */
> 	chip->bits_per_cell = 1;

it is okay to me. Use this in the next version.

thanks
Huang Shijie

Re: [PATCH v2 7/9] jffs2: init the ret with -EINVAL

[Huang Shijie]

On Fri, Aug 23, 2013 at 11:37:39PM -0700, Brian Norris wrote:
> On Mon, Aug 19, 2013 at 10:31:16AM +0800, Huang Shijie wrote:
> > If the media is not SLC nand, dataflash, Sibley flash or a
> > ubi volume, we should return -EINVAL to the caller.
> > The caller should exit in this case.
> > 
> > Current code returns 0 in this case which is not proper.
> > 
> > Signed-off-by: Huang Shijie <b32955@freescale.com>
> > ---
> >  fs/jffs2/fs.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c
> > index fe3c052..0452445 100644
> > --- a/fs/jffs2/fs.c
> > +++ b/fs/jffs2/fs.c
> > @@ -702,7 +702,7 @@ void jffs2_gc_release_page(struct jffs2_sb_info *c,
> >  }
> >  
> >  static int jffs2_flash_setup(struct jffs2_sb_info *c) {
> > -	int ret = 0;
> > +	int ret = -EINVAL;
> 
> This doesn't look correct to me. This function isn't meant to screen out
> unsupported flash; it's only for doing special things for certain flash.
> In fact, I think this will break many NOR flash which currently pass
> through this function without doing anything.

thanks for pointing this.
This patch was created when i was not familiar with the NOR.


> 
> If you really want to make MLC NAND fail, you should do it with an
> explicit check for MLC, and you probably don't want it in this function.
> Maybe in jffs2_do_fill_super(), near the other 'if (c->mtd->type == ...)'
> checks (but outside of the #ifndef, of course).

I will create a new patch to fix the MLC issue for the jffs2.

Huang Shijie

Re: [PATCH v2 3/9] mtd: print out the cell information for nand chip

[Huang Shijie]

On Sat, Aug 24, 2013 at 06:02:16PM -0300, Ezequiel Garcia wrote:
> On Fri, Aug 23, 2013 at 10:58:21PM -0700, Brian Norris wrote:
> > On Mon, Aug 19, 2013 at 10:31:12AM +0800, Huang Shijie wrote:
> > > Print out the cell information for nand chip.
> > > 
> > > Signed-off-by: Huang Shijie <b32955@freescale.com>
> > > ---
> > >  drivers/mtd/nand/nand_base.c |    5 +++--
> > >  1 files changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > > index 69c4b25..8b487d5 100644
> > > --- a/drivers/mtd/nand/nand_base.c
> > > +++ b/drivers/mtd/nand/nand_base.c
> > > @@ -3454,10 +3454,11 @@ ident_done:
> > >  		chip->cmdfunc = nand_command_lp;
> > >  
> > >  	pr_info("NAND device: Manufacturer ID: 0x%02x, Chip ID: 0x%02x (%s %s),"
> > > -		" %dMiB, page size: %d, OOB size: %d\n",
> > > +		" %dMiB, %s, page size: %d, OOB size: %d\n",
> > >  		*maf_id, *dev_id, nand_manuf_ids[maf_idx].name,
> > >  		chip->onfi_version ? chip->onfi_params.model : type->name,
> > > -		(int)(chip->chipsize >> 20), mtd->writesize, mtd->oobsize);
> > > +		(int)(chip->chipsize >> 20), nand_is_slc(chip) ? "SLC" : "MLC",
> > > +		mtd->writesize, mtd->oobsize);
> > 
> > This message is getting mighty long (approx. 120 characters when
> > printed). Are you sure we need all this? Maybe split into two separate
> > pr_info's sometime. But I'm OK with merging this for now.
> > 
> 
> Hm.. well, although it's still a two-line message, I also don't like
> to see this growing.
> 
> I think we should try to answer: why would anyone want to see any of
> this when the kernel boots?
> 
> So maybe we should pr_info only the most important information,

But what information can be regarded as the most important?

if you think the cell type information is not so important, i can
abandon this patch.

thanks
Huang Shijie