[PATCH 00/10] About the SLC/MLC

[PATCH 00/10] 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 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).

Huang Shijie (10):
  mtd: set the cell information for ONFI nand
  mtd: add a helper to check the SLC/MLC nand chip
  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 more information for the MTD_NANDFLASH case
  mtd: add a helper to detect the nand type

 drivers/mtd/inftlcore.c                |    2 +-
 drivers/mtd/mtdcore.c                  |    5 ++++-
 drivers/mtd/nand/denali.c              |    2 +-
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   31 ++++++++++++++++++++++---------
 drivers/mtd/nand/nand_base.c           |   24 +++++++++++++-----------
 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               |   10 ++++++++++
 include/uapi/mtd/mtd-abi.h             |    4 ++--
 15 files changed, 65 insertions(+), 32 deletions(-)

[PATCH 01/10] 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 chip->cellinfo when the onfi nand is a MLC(or TLC) nand
(p->bits_per_cell > 1).

The macro NAND_CI_CELLTYPE_SHIFT is added to avoid the hardcode.

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

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ff605c8..ee1aa52 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2988,6 +2988,9 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
 	chip->chipsize = le32_to_cpu(p->blocks_per_lun);
 	chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
 
+	/* @bits_per_cell equals 1 means this is a SLC nand. */
+	chip->cellinfo = (p->bits_per_cell - 1) << NAND_CI_CELLTYPE_SHIFT;
+
 	if (onfi_feature(chip) & ONFI_FEATURE_16_BIT_BUS)
 		*busw = NAND_BUSWIDTH_16;
 	else
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index ac8e89d..bf743ba 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;
-- 
1.7.1

RE: [PATCH 01/10] mtd: set the cell information for ONFI nand

[Gupta, Pekon]

Hi,
> 
> 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 chip->cellinfo when the onfi nand is a MLC(or TLC) nand
> (p->bits_per_cell > 1).
> 
> The macro NAND_CI_CELLTYPE_SHIFT is added to avoid the hardcode.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/nand/nand_base.c |    3 +++
>  include/linux/mtd/nand.h     |    1 +
>  2 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index ff605c8..ee1aa52 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2988,6 +2988,9 @@ static int nand_flash_detect_onfi(struct mtd_info
> *mtd, struct nand_chip *chip,
>  	chip->chipsize = le32_to_cpu(p->blocks_per_lun);
>  	chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
> 
> +	/* @bits_per_cell equals 1 means this is a SLC nand. */
> +	chip->cellinfo = (p->bits_per_cell - 1) << NAND_CI_CELLTYPE_SHIFT;
> +
[Pekon]: For future scalability, good to update only MLC related bit-fields
So ORing instead of assigning..
chip->cellinfo |= (p->bits_per_cell - 1) << NAND_CI_CELLTYPE_SHIFT;

>  	if (onfi_feature(chip) & ONFI_FEATURE_16_BIT_BUS)
>  		*busw = NAND_BUSWIDTH_16;
>  	else
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index ac8e89d..bf743ba 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;
> --
> 1.7.1
> 

With regards, pekon

Re: [PATCH 01/10] mtd: set the cell information for ONFI nand

[Huang Shijie]

于 2013年08月12日 15:22, Gupta, Pekon 写道:
> [Pekon]: For future scalability, good to update only MLC related bit-fields
> So ORing instead of assigning..
> chip->cellinfo |= (p->bits_per_cell - 1)<<  NAND_CI_CELLTYPE_SHIFT;
>
thanks for review.

yes, use the ORing is better. I will fix it in the next version.

thanks
Huang Shijie

Re: [PATCH 01/10] mtd: set the cell information for ONFI nand

[Brian Norris]

On Mon, Aug 12, 2013 at 07:22:38AM +0000, Gupta, Pekon wrote:
> > 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 chip->cellinfo when the onfi nand is a MLC(or TLC) nand
> > (p->bits_per_cell > 1).
> > 
> > The macro NAND_CI_CELLTYPE_SHIFT is added to avoid the hardcode.
> > 
> > Signed-off-by: Huang Shijie <b32955@freescale.com>
> > ---
> >  drivers/mtd/nand/nand_base.c |    3 +++
> >  include/linux/mtd/nand.h     |    1 +
> >  2 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > index ff605c8..ee1aa52 100644
> > --- a/drivers/mtd/nand/nand_base.c
> > +++ b/drivers/mtd/nand/nand_base.c
> > @@ -2988,6 +2988,9 @@ static int nand_flash_detect_onfi(struct mtd_info
> > *mtd, struct nand_chip *chip,
> >  	chip->chipsize = le32_to_cpu(p->blocks_per_lun);
> >  	chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
> > 
> > +	/* @bits_per_cell equals 1 means this is a SLC nand. */
> > +	chip->cellinfo = (p->bits_per_cell - 1) << NAND_CI_CELLTYPE_SHIFT;
> > +
> [Pekon]: For future scalability, good to update only MLC related bit-fields
> So ORing instead of assigning..
> chip->cellinfo |= (p->bits_per_cell - 1) << NAND_CI_CELLTYPE_SHIFT;

I was thinking of an alternate approach: since nand_chip.cellinfo is
only used for checking SLC vs. MLC (and it is admittedly bad at that,
currently), we should modify it so that is a reliable source of *only* 1
piece of information -- the number of bits per cell. Currently, it
contains unused (and potentially unmaintainable) information for some
chips about number of simultaneously-programmed pages, write caching,
internal chip numbering, etc.

So personally, I would rename cellinfo to bits_per_cell and make sure
it is set properly. That is, for the legacy chips, make sure we
initialize it 1 (SLC); for extended-ID chips, make sure the third ID
byte has the correct info (you can refer to [1]) and set it with
something like this:

  chip->bits_per_cell = id_data[2] & NAND_CI_CELLTYPE_MSK
  chip->bits_per_cell >>= NAND_CI_CELLTYPE_SHIFT;
  chip->bits_per_cell += 1;

for chips listed by full-ID, add an appropriate flag/field; and for ONFI
chips, just use p->bits_per_cell.

If you really need the other cellinfo fields in the future, we can add
more fields to nand_chip.

> 
> >  	if (onfi_feature(chip) & ONFI_FEATURE_16_BIT_BUS)
> >  		*busw = NAND_BUSWIDTH_16;
> >  	else

Thanks,
Brian

[1] An incomplete NAND ID table:

  http://www.linux-mtd.infradead.org/nand-data/nanddata.html

Re: [PATCH 01/10] mtd: set the cell information for ONFI nand

[Huang Shijie]

于 2013年08月13日 08:49, Brian Norris 写道:
> On Mon, Aug 12, 2013 at 07:22:38AM +0000, Gupta, Pekon wrote:
>>> 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 chip->cellinfo when the onfi nand is a MLC(or TLC) nand
>>> (p->bits_per_cell>  1).
>>>
>>> The macro NAND_CI_CELLTYPE_SHIFT is added to avoid the hardcode.
>>>
>>> Signed-off-by: Huang Shijie<b32955@freescale.com>
>>> ---
>>>   drivers/mtd/nand/nand_base.c |    3 +++
>>>   include/linux/mtd/nand.h     |    1 +
>>>   2 files changed, 4 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>>> index ff605c8..ee1aa52 100644
>>> --- a/drivers/mtd/nand/nand_base.c
>>> +++ b/drivers/mtd/nand/nand_base.c
>>> @@ -2988,6 +2988,9 @@ static int nand_flash_detect_onfi(struct mtd_info
>>> *mtd, struct nand_chip *chip,
>>>   	chip->chipsize = le32_to_cpu(p->blocks_per_lun);
>>>   	chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
>>>
>>> +	/* @bits_per_cell equals 1 means this is a SLC nand. */
>>> +	chip->cellinfo = (p->bits_per_cell - 1)<<  NAND_CI_CELLTYPE_SHIFT;
>>> +
>> [Pekon]: For future scalability, good to update only MLC related bit-fields
>> So ORing instead of assigning..
>> chip->cellinfo |= (p->bits_per_cell - 1)<<  NAND_CI_CELLTYPE_SHIFT;
> I was thinking of an alternate approach: since nand_chip.cellinfo is
> only used for checking SLC vs. MLC (and it is admittedly bad at that,
> currently), we should modify it so that is a reliable source of *only* 1
> piece of information -- the number of bits per cell. Currently, it
> contains unused (and potentially unmaintainable) information for some
> chips about number of simultaneously-programmed pages, write caching,
> internal chip numbering, etc.
I do not object to rename the cellinfo to bits_per_cell. :)
It's okay to me.

Do Artem & David (or some other people) have any opinion about this?



> byte has the correct info (you can refer to [1]) and set it with
> something like this:
>
>    chip->bits_per_cell = id_data[2]&  NAND_CI_CELLTYPE_MSK
>    chip->bits_per_cell>>= NAND_CI_CELLTYPE_SHIFT;
>    chip->bits_per_cell += 1;
>
> for chips listed by full-ID, add an appropriate flag/field; and for ONFI
> chips, just use p->bits_per_cell.
we do not need to worry about the full-id case, we can get the correct 
cell info from the id[2] for all the 4 toshiba nand.

> If you really need the other cellinfo fields in the future, we can add
> more fields to nand_chip.
>
I only need the SLC/MLC info now.

thanks
Huang Shijie
>

Re: [PATCH 01/10] mtd: set the cell information for ONFI nand

[Brian Norris]

On Mon, Aug 12, 2013 at 7:32 PM, Huang Shijie <b32955@freescale.com> wrote:
> 于 2013年08月13日 08:49, Brian Norris 写道:
>> On Mon, Aug 12, 2013 at 07:22:38AM +0000, Gupta, Pekon wrote:
>> for chips listed by full-ID, add an appropriate flag/field; and for ONFI
>> chips, just use p->bits_per_cell.
>
> we do not need to worry about the full-id case, we can get the correct cell
> info from the id[2] for all the 4 toshiba nand.

I see. I forgot that you assign nand_chip.cellinfo in
find_full_id_nand(). It still seems like a bad design decision to base
the implementation of the full-ID table on the fact that it currently
only holds your 4 Toshiba entries, but I suppose it can be improved
if/when additions are needed that don't support the same CELLINFO
layout.

Brian

Re: [PATCH 01/10] mtd: set the cell information for ONFI nand

[Brian Norris]

(You used HTML again, so this might not have gotten through the list)

On Tue, Aug 13, 2013 at 11:03:14AM +0800, Huang Shijie wrote:
> 于 2013年08月13日 08:49, Brian Norris 写道:
>      currently), we should modify it so that is a reliable source of
>      *only* 1
>      piece of information -- the number of bits per cell. Currently, it
> do you need to rename the cellinfo to bits_per_cell, or add a new field :
> bits_per_cell?

Well, my whole point was that 'cellinfo' is really not very useful for
us. We just mask it off all the time, and it makes life more
complicated.

So I'd just rename cellinfo to bits_per_cell and change its
assignment/usage appropriately.

But we can also wait a few days, considering that you asked for others'
thoughts, although I doubt many others will speak up (I'd be glad to be
proven wrong). Then you can send v2 at your leisure.

Brian

Re: [PATCH 01/10] mtd: set the cell information for ONFI nand

[Huang Shijie]

于 2013年08月13日 11:17, Brian Norris 写道:
> But we can also wait a few days, considering that you asked for others'
> thoughts, although I doubt many others will speak up (I'd be glad to be
yes. i will wait for several days.

  I hope others give us some other opinions.

thanks
Huang Shijie

RE: [PATCH 01/10] mtd: set the cell information for ONFI nand

[Gupta, Pekon]

> 
> On Tue, Aug 13, 2013 at 11:03:14AM +0800, Huang Shijie wrote:
> > 于 2013年08月13日 08:49, Brian Norris 写道:
> >      currently), we should modify it so that is a reliable source of
> >      *only* 1
> >      piece of information -- the number of bits per cell. Currently, it
> > do you need to rename the cellinfo to bits_per_cell, or add a new field :
> > bits_per_cell?
> 
> Well, my whole point was that 'cellinfo' is really not very useful for
> us. We just mask it off all the time, and it makes life more
> complicated.
> 
> So I'd just rename cellinfo to bits_per_cell and change its
> assignment/usage appropriately.
> 
[Pekon]: How about moving 'bit_per_cell' info to chip->options ?
Considering 'bit_per_cell' info is similar to NAND_BUSWIDTH_16..
- It can be determined by reading ONFI parameters. OR
- It can be taken from 'nand_flash_id' table. OR
- And it can be provided along with board-data (which now is obsolete).

with regards, pekon

Re: [PATCH 01/10] mtd: set the cell information for ONFI nand

[Huang Shijie]

于 2013年08月13日 12:10, Gupta, Pekon 写道:
>> On Tue, Aug 13, 2013 at 11:03:14AM +0800, Huang Shijie wrote:
>>> 于 2013年08月13日 08:49, Brian Norris 写道:
>>>       currently), we should modify it so that is a reliable source of
>>>       *only* 1
>>>       piece of information -- the number of bits per cell. Currently, it
>>> do you need to rename the cellinfo to bits_per_cell, or add a new field :
>>> bits_per_cell?
>> Well, my whole point was that 'cellinfo' is really not very useful for
>> us. We just mask it off all the time, and it makes life more
>> complicated.
>>
>> So I'd just rename cellinfo to bits_per_cell and change its
>> assignment/usage appropriately.
>>
> [Pekon]: How about moving 'bit_per_cell' info to chip->options ?
sorry, i do not think this is a good idea.

the drivers may changes the chip->options.

I prefer to keep it as new field.

thanks
Huang Shijie

[PATCH 02/10] mtd: add a helper to check the SLC/MLC nand chip

[Huang Shijie]

Add a helper to check if a nand chip is SLC or MLC.
This helper makes the code more readable.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/denali.c    |    2 +-
 drivers/mtd/nand/nand_base.c |   14 ++++++--------
 include/linux/mtd/nand.h     |    9 +++++++++
 3 files changed, 16 insertions(+), 9 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 ee1aa52..fd5117d 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3105,8 +3105,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;
@@ -3138,7 +3137,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 */
@@ -3201,7 +3200,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;
@@ -3262,11 +3261,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 ||
@@ -3742,8 +3741,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 bf743ba..3c5aae6 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -798,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->cellinfo & NAND_CI_CELLTYPE_MSK);
+}
 #endif /* __LINUX_MTD_NAND_H */
-- 
1.7.1

Re: [PATCH 02/10] mtd: add a helper to check the SLC/MLC nand chip

[Brian Norris]

On Mon, Aug 12, 2013 at 01:42:45PM +0800, Huang Shijie wrote:
> Add a helper to check if a nand chip is SLC or MLC.
> This helper makes the code more readable.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/nand/denali.c    |    2 +-
>  drivers/mtd/nand/nand_base.c |   14 ++++++--------
>  include/linux/mtd/nand.h     |    9 +++++++++
>  3 files changed, 16 insertions(+), 9 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) &&

According to my recommendations in patch 1, this would only need to be:

  if (denali->nand.bits_per_cell > 1 && ...)

IMO, that is plenty readable, then we don't need the helper.

>  			(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 ee1aa52..fd5117d 100644

(ditto elsewhere)

Thanks,
Brian

Re: [PATCH 02/10] mtd: add a helper to check the SLC/MLC nand chip

[Huang Shijie]

于 2013年08月13日 08:52, Brian Norris 写道:
>> -	if (denali->nand.cellinfo&  NAND_CI_CELLTYPE_MSK&&
>> >  +	if (!nand_is_slc(&denali->nand)&&
> According to my recommendations in patch 1, this would only need to be:
>
>    if (denali->nand.bits_per_cell>  1&&  ...)
>
> IMO, that is plenty readable, then we don't need the helper.
>

The drivers may also check the SLC/MLC.

IMO, using a helper is more readable :)

Huang Shijie

Re: [PATCH 02/10] mtd: add a helper to check the SLC/MLC nand chip

[Brian Norris]

On Mon, Aug 12, 2013 at 7:35 PM, Huang Shijie <b32955@freescale.com> wrote:
> 于 2013年08月13日 08:52, Brian Norris 写道:
>>>
>>> -       if (denali->nand.cellinfo&  NAND_CI_CELLTYPE_MSK&&
>>> >  +    if (!nand_is_slc(&denali->nand)&&
>>
>> According to my recommendations in patch 1, this would only need to be:
>>
>>    if (denali->nand.bits_per_cell>  1&&  ...)
>>
>>
>> IMO, that is plenty readable, then we don't need the helper.
>
> The drivers may also check the SLC/MLC.
>
> IMO, using a helper is more readable :)

OK, the helper is still fine. I just thought that bits_per_cell has
direct meaning in itself (whereas "cellinfo & NAND_CI_CELLTYPE_MSK"
doesn't).

Brian

[PATCH 03/10] 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 fd5117d..9bd7f04 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3445,10 +3445,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 04/10] 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 05/10] 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 06/10] 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 9bd7f04..4751814 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3767,7 +3767,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 07/10] 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 08/10] 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.

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

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 6aa952b..c7cee29 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

Re: [PATCH 08/10] mtd: add MTD_MLCNANDFLASH case for mtd_type_show()

[Brian Norris]

On Mon, Aug 12, 2013 at 01:42:51PM +0800, Huang Shijie wrote:
> The current mtd_type_show() misses the MTD_MLCNANDFLASH case.
> This patch adds the case for it.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/mtdcore.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 6aa952b..c7cee29 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";

The current convention uses lower-case, single-word names. And your
selection isn't very consistent with the SLC NAND case (but I see that
you're trying to change the SLC NAND case). I'd go with "mlc-nand" or
just "mlc".

But more importantly, this is probably a break in ABI. At a minimum,
this needs documentation here in Documentation/ABI. I know tools which
currently check only for "nand", and if the name suddely becomes
different for MLC, that is a breakage.

But really, does user-space need to know SLC vs. MLC? If so, this needs
a clear argument for why in the patch description. Perhaps to
differentiate whether or not JFFS2 support is even possible? I'm not
opposed to adding a new name, especially since the MTD_MLCNANDFLASH
macro has existed in mtd/mtd-abi.h for a long time (but it was rotten).
Just do it sensibly (i.e., better name string, proper ABI documentation
inlcuded in this patch, explain the reason for the ABI addition in this
patch, etc.).

Brian

Re: [PATCH 08/10] mtd: add MTD_MLCNANDFLASH case for mtd_type_show()

[Huang Shijie]

于 2013年08月13日 09:05, Brian Norris 写道:
> The current convention uses lower-case, single-word names. And your
> selection isn't very consistent with the SLC NAND case (but I see that
> you're trying to change the SLC NAND case). I'd go with "mlc-nand" or
> just "mlc".
>
> But more importantly, this is probably a break in ABI. At a minimum,
> this needs documentation here in Documentation/ABI. I know tools which
> currently check only for "nand", and if the name suddely becomes
> different for MLC, that is a breakage.
thanks for pointing this. If the tool may check the "nand", I prefer to 
do not change the ABI.

> But really, does user-space need to know SLC vs. MLC? If so, this needs
I expose it to user-space only for mtd-info.
But if these two patches will break current tools (not the mtd-info), i
will abandon these two patches.

thanks
Huang Shijie


> a clear argument for why in the patch description. Perhaps to
> differentiate whether or not JFFS2 support is even possible? I'm not
> opposed to adding a new name, especially since the MTD_MLCNANDFLASH
> macro has existed in mtd/mtd-abi.h for a long time (but it was rotten).
> Just do it sensibly (i.e., better name string, proper ABI documentation
> inlcuded in this patch, explain the reason for the ABI addition in this
> patch, etc.).

Re: [PATCH 08/10] mtd: add MTD_MLCNANDFLASH case for mtd_type_show()

[Brian Norris]

On Mon, Aug 12, 2013 at 7:20 PM, Huang Shijie <b32955@freescale.com> wrote:
> 于 2013年08月13日 09:05, Brian Norris 写道:
>
>> The current convention uses lower-case, single-word names. And your
>> selection isn't very consistent with the SLC NAND case (but I see that
>> you're trying to change the SLC NAND case). I'd go with "mlc-nand" or
>> just "mlc".
>>
>> But more importantly, this is probably a break in ABI. At a minimum,
>> this needs documentation here in Documentation/ABI. I know tools which
>> currently check only for "nand", and if the name suddely becomes
>> different for MLC, that is a breakage.
>
> thanks for pointing this. If the tool may check the "nand", I prefer to do
> not change the ABI.

OK. And I guess for a tool that wants to check the type for SLC vs.
MLC from userspace, they can still use ioctl(MEMGETINFO) which has
always documented the MTD_MLCNANDFLASH type (we just didn't use it
right).

>> But really, does user-space need to know SLC vs. MLC? If so, this needs
>
> I expose it to user-space only for mtd-info.
> But if these two patches will break current tools (not the mtd-info), i
> will abandon these two patches.

I think the "nand" to "SLC nand" patch should be dropped, but this
patch is a different story. Without this patch, MLC NAND will be
exported as "unknown". So you need a new patch which will combine the
'switch (mtd->type)' cases for MTD_NANDFLASH and MTD_MLCNANDFLASH.
Otherwise, your patch set leaves MLC exported as "unknown" (a
definite, undesirable breakage).

[Side note: onenand already uses MTD_MLCNANDLFASH, so this is already
exported as "unknkown" in the current upstream. I assume no one cares
about this entry for MLC onenand, if such a thing exists.]

>> a clear argument for why in the patch description. Perhaps to
>> differentiate whether or not JFFS2 support is even possible? I'm not
>> opposed to adding a new name, especially since the MTD_MLCNANDFLASH
>> macro has existed in mtd/mtd-abi.h for a long time (but it was rotten).
>> Just do it sensibly (i.e., better name string, proper ABI documentation
>> inlcuded in this patch, explain the reason for the ABI addition in this
>> patch, etc.).

Regards,
Brian

Re: [PATCH 08/10] mtd: add MTD_MLCNANDFLASH case for mtd_type_show()

[Huang Shijie]

于 2013年08月13日 11:10, Brian Norris 写道:
> I think the "nand" to "SLC nand" patch should be dropped, but this
> patch is a different story. Without this patch, MLC NAND will be
> exported as "unknown". So you need a new patch which will combine the
> 'switch (mtd->type)' cases for MTD_NANDFLASH and MTD_MLCNANDFLASH.
> Otherwise, your patch set leaves MLC exported as "unknown" (a
ok, I will keep this patch, and update the document.

thanks
Huang Shijie

[PATCH 09/10] mtd: add more information for the MTD_NANDFLASH case

[Huang Shijie]

In mtd_type_show(), the MTD_NANDFLASH case shows "nand".
In actually, the printed information is not accurate enough.
The MTD_NANDFLASH stands for SLC nand now.

This patch adds the SLC information for the MTD_NANDFLASH case.

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

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index c7cee29..78b8f2e 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -149,7 +149,7 @@ static ssize_t mtd_type_show(struct device *dev,
 		type = "nor";
 		break;
 	case MTD_NANDFLASH:
-		type = "nand";
+		type = "SLC nand";
 		break;
 	case MTD_DATAFLASH:
 		type = "dataflash";
-- 
1.7.1

Re: [PATCH 09/10] mtd: add more information for the MTD_NANDFLASH case

[Brian Norris]

On Mon, Aug 12, 2013 at 01:42:52PM +0800, Huang Shijie wrote:
> In mtd_type_show(), the MTD_NANDFLASH case shows "nand".
> In actually, the printed information is not accurate enough.
> The MTD_NANDFLASH stands for SLC nand now.
> 
> This patch adds the SLC information for the MTD_NANDFLASH case.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/mtdcore.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index c7cee29..78b8f2e 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -149,7 +149,7 @@ static ssize_t mtd_type_show(struct device *dev,
>  		type = "nor";
>  		break;
>  	case MTD_NANDFLASH:
> -		type = "nand";
> +		type = "SLC nand";

Nak. This is clearly an ABI change, doesn't follow the existing naming
convention, and isn't even necessary (just differentiating the
"mlc-nand" case in the prior patch is sufficient).

>  		break;
>  	case MTD_DATAFLASH:
>  		type = "dataflash";

Brian

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

[Huang Shijie]

From: Huang Shijie <shijie8@gmail.com>

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 5a8c858..1911a5d 100644
--- a/drivers/mtd/tests/nandbiterrs.c
+++ b/drivers/mtd/tests/nandbiterrs.c
@@ -355,7 +355,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 22cc38c..3af8d25 100644
--- a/drivers/mtd/tests/pagetest.c
+++ b/drivers/mtd/tests/pagetest.c
@@ -405,7 +405,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 effdd41..2a91a23 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