[RFC PATCH v2 0/6] mtd: rawnand: support MT29F1G08ABAFAWP-ITE:F

[RFC PATCH v2 0/6] mtd: rawnand: support MT29F1G08ABAFAWP-ITE:F

[Chris Packham]

Hi,

I'm looking at adding support for the Micron MT29F1G08ABAFAWP-ITE:F chip
to one of our boards which uses the Marvell NFCv2 controller.

This particular chip is a bit odd in that the datasheet states support
for ONFI 1.0 but the revision number field is 00 00. It also is marked
ABAFA but reports internally as ABAGA. Finally it has internal 8-bit ECC
which cannot be disabled.

The existing test in micron_supports_on_die_ecc() determines that on-die
ECC is supported but not mandatory but I know for this chip it is
mandatory despite what set_features returns.

In order for this to work I need to set nand-ecc-mode = "on-die" in my
dts. Ideally I'd like it to be automatic based on what the hardware can
support but that may be asking too much at the moment.

I think I need to do some re-testing on other Micron parts so this
series is still RFC. Although the first 3 patches are probably good to
go.

Here's a dump of the parameter page from the chip I have

00000000: 4f 4e 46 49 00 00 18 00 3f 00 00 00 00 00 00 00 ONFI....?.......
00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00000020: 4d 49 43 52 4f 4e 20 20 20 20 20 20 4d 54 32 39  MICRON MT29
00000030: 46 31 47 30 38 41 42 41 47 41 57 50 20 20 20 20  F1G08ABAGAWP    
00000040: 2c 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ,...............
00000050: 00 08 00 00 80 00 00 02 00 00 20 00 40 00 00 00  ..........  .@...
00000060: 00 04 00 00 01 22 01 14 00 01 05 08 00 00 04 00 ....."..........
00000070: 08 01 0e 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00000080: 08 3f 00 3f 00 58 02 10 27 46 00 64 00 00 00 00 .?.?.X..'F.d....
00000090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
000000a0: 00 00 00 00 01 00 00 00 00 02 04 80 01 81 04 03 ................
000000b0: 02 01 1e 90 00 00 00 00 00 00 00 00 00 00 00 00 ................
000000c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
000000d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
000000e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
000000f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 85 a6 ................

Chris Packham (6):
  mtd: rawnand: marvell: Handle on-die ECC
  mtd: rawnand: add manufacturer fixup for ONFI parameter page
  mtd: rawnand: micron: add fixup for ONFI revision
  mtd: rawnand: marvell: Support page size of 2048 with 8-bit ECC
  mtd: rawnand: micron: add ONFI_FEATURE_ON_DIE_ECC to supported
    features
  mtd: rawnand: micron: support 8/512 on-die ECC

 drivers/mtd/nand/raw/marvell_nand.c |  2 ++
 drivers/mtd/nand/raw/nand_base.c    |  4 ++++
 drivers/mtd/nand/raw/nand_micron.c  | 18 ++++++++++++++----
 include/linux/mtd/rawnand.h         |  1 +
 4 files changed, 21 insertions(+), 4 deletions(-)

-- 
2.17.1

[RFC PATCH v2 1/6] mtd: rawnand: marvell: Handle on-die ECC

[Chris Packham]

From the controllers point of view this is the same as no or
software only ECC.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Changes in v2:
- New

 drivers/mtd/nand/raw/marvell_nand.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index ebb1d141b900..ba6889bbe802 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -2157,6 +2157,7 @@ static int marvell_nand_ecc_init(struct mtd_info *mtd,
 		break;
 	case NAND_ECC_NONE:
 	case NAND_ECC_SOFT:
+	case NAND_ECC_ON_DIE:
 		if (!nfc->caps->is_nfcv2 && mtd->writesize != SZ_512 &&
 		    mtd->writesize != SZ_2K) {
 			dev_err(nfc->dev, "NFCv1 cannot write %d bytes pages\n",
-- 
2.17.1

[RFC PATCH v2 1/6] mtd: rawnand: marvell: Handle on-die ECC

[Chris Packham]

>From the controllers point of view this is the same as no or
software only ECC.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Changes in v2:
- New

 drivers/mtd/nand/raw/marvell_nand.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index ebb1d141b900..ba6889bbe802 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -2157,6 +2157,7 @@ static int marvell_nand_ecc_init(struct mtd_info *mtd,
 		break;
 	case NAND_ECC_NONE:
 	case NAND_ECC_SOFT:
+	case NAND_ECC_ON_DIE:
 		if (!nfc->caps->is_nfcv2 && mtd->writesize != SZ_512 &&
 		    mtd->writesize != SZ_2K) {
 			dev_err(nfc->dev, "NFCv1 cannot write %d bytes pages\n",
-- 
2.17.1

Re: [RFC PATCH v2 1/6] mtd: rawnand: marvell: Handle on-die ECC

[Boris Brezillon]

On Tue, 19 Jun 2018 17:31:20 +1200
Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:

> From the controllers point of view this is the same as no or
> software only ECC.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
> Changes in v2:
> - New
> 
>  drivers/mtd/nand/raw/marvell_nand.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> index ebb1d141b900..ba6889bbe802 100644
> --- a/drivers/mtd/nand/raw/marvell_nand.c
> +++ b/drivers/mtd/nand/raw/marvell_nand.c
> @@ -2157,6 +2157,7 @@ static int marvell_nand_ecc_init(struct mtd_info *mtd,
>  		break;
>  	case NAND_ECC_NONE:
>  	case NAND_ECC_SOFT:
> +	case NAND_ECC_ON_DIE:
>  		if (!nfc->caps->is_nfcv2 && mtd->writesize != SZ_512 &&
>  		    mtd->writesize != SZ_2K) {
>  			dev_err(nfc->dev, "NFCv1 cannot write %d bytes pages\n",

[RFC PATCH v2 2/6] mtd: rawnand: add manufacturer fixup for ONFI parameter page

[Chris Packham]

This is called after the ONFI parameter page checksum is verified
and allows us to override the contents of the parameter page.

Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com>
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Changes in v2:
- New

 drivers/mtd/nand/raw/nand_base.c | 4 ++++
 include/linux/mtd/rawnand.h      | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 0cd3e216b95c..65250308c82d 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5172,6 +5172,10 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 		}
 	}
 
+	if (chip->manufacturer.desc && chip->manufacturer.desc->ops &&
+	    chip->manufacturer.desc->ops->fixup_onfi_param_page)
+		chip->manufacturer.desc->ops->fixup_onfi_param_page(chip, p);
+
 	/* Check version */
 	val = le16_to_cpu(p->revision);
 	if (val & (1 << 5))
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 3e8ec3b8a39c..6db42091ee5e 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -783,6 +783,7 @@ struct nand_manufacturer_ops {
 	void (*detect)(struct nand_chip *chip);
 	int (*init)(struct nand_chip *chip);
 	void (*cleanup)(struct nand_chip *chip);
+	void (*fixup_onfi_param_page)(struct nand_chip *chip, struct nand_onfi_params *p);
 };
 
 /**
-- 
2.17.1

Re: [RFC PATCH v2 2/6] mtd: rawnand: add manufacturer fixup for ONFI parameter page

[Boris Brezillon]

On Tue, 19 Jun 2018 17:31:21 +1200
Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:

> This is called after the ONFI parameter page checksum is verified
> and allows us to override the contents of the parameter page.
> 
> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
> Changes in v2:
> - New
> 
>  drivers/mtd/nand/raw/nand_base.c | 4 ++++
>  include/linux/mtd/rawnand.h      | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 0cd3e216b95c..65250308c82d 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5172,6 +5172,10 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
>  		}
>  	}
>  
> +	if (chip->manufacturer.desc && chip->manufacturer.desc->ops &&
> +	    chip->manufacturer.desc->ops->fixup_onfi_param_page)
> +		chip->manufacturer.desc->ops->fixup_onfi_param_page(chip, p);
> +
>  	/* Check version */
>  	val = le16_to_cpu(p->revision);
>  	if (val & (1 << 5))
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 3e8ec3b8a39c..6db42091ee5e 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -783,6 +783,7 @@ struct nand_manufacturer_ops {
>  	void (*detect)(struct nand_chip *chip);
>  	int (*init)(struct nand_chip *chip);
>  	void (*cleanup)(struct nand_chip *chip);
> +	void (*fixup_onfi_param_page)(struct nand_chip *chip, struct nand_onfi_params *p);
>  };
>  
>  /**

[RFC PATCH v2 3/6] mtd: rawnand: micron: add fixup for ONFI revision

[Chris Packham]

Some Micron NAND chips (MT29F1G08ABAFAWP-ITE:F) report 00 00 for the
revision number field of the ONFI parameter page. Rather than rejecting
these outright assume ONFI version 1.0 if the revision number is 00 00.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
This is now qualified on vendor == MICRON. I haven't qualified this
based on specific chips the ABAFA (id=d1) and ABBFA (id=a1) variants are
documented to have this behaviour.

Changes in v2:
- use fixup_onfi_param_page

 drivers/mtd/nand/raw/nand_micron.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index 0af45b134c0c..e582c9e61715 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -287,6 +287,14 @@ static int micron_nand_init(struct nand_chip *chip)
 	return 0;
 }
 
+static void micron_fixup_onfi_param_page(struct nand_chip *chip,
+					 struct nand_onfi_params *p)
+{
+	if (le16_to_cpu(p->revision) == 0)
+		p->revision = cpu_to_le16(1 << 1);
+}
+
 const struct nand_manufacturer_ops micron_nand_manuf_ops = {
 	.init = micron_nand_init,
+	.fixup_onfi_param_page = micron_fixup_onfi_param_page,
 };
-- 
2.17.1

Re: [RFC PATCH v2 3/6] mtd: rawnand: micron: add fixup for ONFI revision

[Boris Brezillon]

On Tue, 19 Jun 2018 17:31:22 +1200
Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:

> Some Micron NAND chips (MT29F1G08ABAFAWP-ITE:F) report 00 00 for the
> revision number field of the ONFI parameter page. Rather than rejecting
> these outright assume ONFI version 1.0 if the revision number is 00 00.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> This is now qualified on vendor == MICRON. I haven't qualified this
> based on specific chips the ABAFA (id=d1) and ABBFA (id=a1) variants are
> documented to have this behaviour.
> 
> Changes in v2:
> - use fixup_onfi_param_page
> 
>  drivers/mtd/nand/raw/nand_micron.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
> index 0af45b134c0c..e582c9e61715 100644
> --- a/drivers/mtd/nand/raw/nand_micron.c
> +++ b/drivers/mtd/nand/raw/nand_micron.c
> @@ -287,6 +287,14 @@ static int micron_nand_init(struct nand_chip *chip)
>  	return 0;
>  }
>  
> +static void micron_fixup_onfi_param_page(struct nand_chip *chip,
> +					 struct nand_onfi_params *p)
> +{

Could you add a comment repeating what you have in your commit message
here?

With this addressed

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> +	if (le16_to_cpu(p->revision) == 0)
> +		p->revision = cpu_to_le16(1 << 1);
> +}
> +
>  const struct nand_manufacturer_ops micron_nand_manuf_ops = {
>  	.init = micron_nand_init,
> +	.fixup_onfi_param_page = micron_fixup_onfi_param_page,
>  };

[RFC PATCH v2 4/6] mtd: rawnand: marvell: Support page size of 2048 with 8-bit ECC

[Chris Packham]

The MT29F1G08ABAFAWP-ITE:F chip has 2048 byte pages and requires a
minimum ECC strength of 8-bits. Allow for this combination of
requirements using the marvell_nand controller.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
I've tried to follow the recommended AN-379 from Marvell. They do seem
to have information that covers this particular set of chip
requirements.

As discussed I don't think my particular configuration will be supported
with this change due to the conflict with the on-die ECC. But this may
be useful for others so I've left it in.

Changes in v2:
- update as suggested by miquel

 drivers/mtd/nand/raw/marvell_nand.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index ba6889bbe802..96aee8ffd408 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -217,6 +217,7 @@ static const struct marvell_hw_ecc_layout marvell_nfc_layouts[] = {
 	MARVELL_LAYOUT(  512,   512,  1,  1,  1,  512,  8,  8,  0,  0,  0),
 	MARVELL_LAYOUT( 2048,   512,  1,  1,  1, 2048, 40, 24,  0,  0,  0),
 	MARVELL_LAYOUT( 2048,   512,  4,  1,  1, 2048, 32, 30,  0,  0,  0),
+	MARVELL_LAYOUT( 2048,   512,  8,  2,  1, 1024, 0, 30,  1024,  32,  30),
 	MARVELL_LAYOUT( 4096,   512,  4,  2,  2, 2048, 32, 30,  0,  0,  0),
 	MARVELL_LAYOUT( 4096,   512,  8,  5,  4, 1024,  0, 30,  0, 64, 30),
 };
-- 
2.17.1

[RFC PATCH v2 5/6] mtd: rawnand: micron: add ONFI_FEATURE_ON_DIE_ECC to supported features

[Chris Packham]

Add ONFI_FEATURE_ON_DIE_ECC to the set/get features list for Micron
NAND flash.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Changes in v2:
- New

 drivers/mtd/nand/raw/nand_micron.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index e582c9e61715..d1e8f57544a0 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -66,7 +66,9 @@ static int micron_nand_onfi_init(struct nand_chip *chip)
 
 	if (p->supports_set_get_features) {
 		set_bit(ONFI_FEATURE_ADDR_READ_RETRY, p->set_feature_list);
+		set_bit(ONFI_FEATURE_ON_DIE_ECC, p->set_feature_list);
 		set_bit(ONFI_FEATURE_ADDR_READ_RETRY, p->get_feature_list);
+		set_bit(ONFI_FEATURE_ON_DIE_ECC, p->get_feature_list);
 	}
 
 	return 0;
-- 
2.17.1

Re: [RFC PATCH v2 5/6] mtd: rawnand: micron: add ONFI_FEATURE_ON_DIE_ECC to supported features

[Boris Brezillon]

On Tue, 19 Jun 2018 17:31:24 +1200
Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:

> Add ONFI_FEATURE_ON_DIE_ECC to the set/get features list for Micron
> NAND flash.
>

Fixes: 789157e41a06 ("mtd: rawnand: allow vendors to declare (un)supported features")
Cc: <stable@vger.kernel.org>

No need to send a new version, I'll add that when queuing the patch.

Miquel, if you're okay, I'm gonna take this patch in the mtd/fixes branch
and let you deal with other patches in this series.

> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> Changes in v2:
> - New
> 
>  drivers/mtd/nand/raw/nand_micron.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
> index e582c9e61715..d1e8f57544a0 100644
> --- a/drivers/mtd/nand/raw/nand_micron.c
> +++ b/drivers/mtd/nand/raw/nand_micron.c
> @@ -66,7 +66,9 @@ static int micron_nand_onfi_init(struct nand_chip *chip)
>  
>  	if (p->supports_set_get_features) {
>  		set_bit(ONFI_FEATURE_ADDR_READ_RETRY, p->set_feature_list);
> +		set_bit(ONFI_FEATURE_ON_DIE_ECC, p->set_feature_list);
>  		set_bit(ONFI_FEATURE_ADDR_READ_RETRY, p->get_feature_list);
> +		set_bit(ONFI_FEATURE_ON_DIE_ECC, p->get_feature_list);
>  	}
>  
>  	return 0;

Re: [RFC PATCH v2 5/6] mtd: rawnand: micron: add ONFI_FEATURE_ON_DIE_ECC to supported features

[Miquel Raynal]

Hi Boris, Chris,

On Tue, 19 Jun 2018 07:40:56 +0200, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:

> On Tue, 19 Jun 2018 17:31:24 +1200
> Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
> 
> > Add ONFI_FEATURE_ON_DIE_ECC to the set/get features list for Micron
> > NAND flash.
> >  
> 
> Fixes: 789157e41a06 ("mtd: rawnand: allow vendors to declare (un)supported features")

Sorry for breaking it in my previous series.

> Cc: <stable@vger.kernel.org>
> 
> No need to send a new version, I'll add that when queuing the patch.
> 
> Miquel, if you're okay, I'm gonna take this patch in the mtd/fixes branch
> and let you deal with other patches in this series.

Yes please, take this one.

* Chris, I'll take patches 1, 2, 3 with Boris comments.
* Patch 4 I think I'll just drop it as nobody can test it (and it's a
  oneliner). I prefer people requesting a feature than complaining
  about a bug :)
* Boris will take patch 5 in the mtd/fixes branch.
* I'll let you write a v3 for patch 6 only.

Thanks,
Miquèl

Re: [RFC PATCH v2 5/6] mtd: rawnand: micron: add ONFI_FEATURE_ON_DIE_ECC to supported features

[Chris Packham]

On 19/06/18 18:53, Miquel Raynal wrote:
> Hi Boris, Chris,
> 
> On Tue, 19 Jun 2018 07:40:56 +0200, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> 
>> On Tue, 19 Jun 2018 17:31:24 +1200
>> Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
>>
>>> Add ONFI_FEATURE_ON_DIE_ECC to the set/get features list for Micron
>>> NAND flash.
>>>   
>>
>> Fixes: 789157e41a06 ("mtd: rawnand: allow vendors to declare (un)supported features")
> 
> Sorry for breaking it in my previous series.
> 
>> Cc: <stable@vger.kernel.org>
>>
>> No need to send a new version, I'll add that when queuing the patch.
>>
>> Miquel, if you're okay, I'm gonna take this patch in the mtd/fixes branch
>> and let you deal with other patches in this series.
> 
> Yes please, take this one.
> 
> * Chris, I'll take patches 1, 2, 3 with Boris comments.

OK.

I also got a kbuild report on

include/linux/mtd/rawnand.h:788: warning: Function parameter or member 
'fixup_onfi_param_page' not described in 'nand_manufacturer_ops'

Do you want me to send a v3 for that? (given our timezones I'll just 
send it anyway and you can choose to take it or just stick with v2).

> * Patch 4 I think I'll just drop it as nobody can test it (and it's a
>    oneliner). I prefer people requesting a feature than complaining
>    about a bug :)

Agreed. It's in the mailing list archive now if someone needs it we can 
resurrect it from there

> * Boris will take patch 5 in the mtd/fixes branch.

Thanks all. Sorry our emails crossed so I couldn't just send #5 as a fix 
on it's own.

> * I'll let you write a v3 for patch 6 only.

I'll have a read through the old thread and see if I can make sense of it.

Re: [RFC PATCH v2 5/6] mtd: rawnand: micron: add ONFI_FEATURE_ON_DIE_ECC to supported features

[Miquel Raynal]

Hi Chris,

On Tue, 19 Jun 2018 21:22:00 +0000, Chris Packham
<Chris.Packham@alliedtelesis.co.nz> wrote:

> On 19/06/18 18:53, Miquel Raynal wrote:
> > Hi Boris, Chris,
> > 
> > On Tue, 19 Jun 2018 07:40:56 +0200, Boris Brezillon
> > <boris.brezillon@bootlin.com> wrote:
> >   
> >> On Tue, 19 Jun 2018 17:31:24 +1200
> >> Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
> >>  
> >>> Add ONFI_FEATURE_ON_DIE_ECC to the set/get features list for Micron
> >>> NAND flash.
> >>>     
> >>
> >> Fixes: 789157e41a06 ("mtd: rawnand: allow vendors to declare (un)supported features")  
> > 
> > Sorry for breaking it in my previous series.
> >   
> >> Cc: <stable@vger.kernel.org>
> >>
> >> No need to send a new version, I'll add that when queuing the patch.
> >>
> >> Miquel, if you're okay, I'm gonna take this patch in the mtd/fixes branch
> >> and let you deal with other patches in this series.  
> > 
> > Yes please, take this one.
> > 
> > * Chris, I'll take patches 1, 2, 3 with Boris comments.  
> 
> OK.
> 
> I also got a kbuild report on
> 
> include/linux/mtd/rawnand.h:788: warning: Function parameter or member 
> 'fixup_onfi_param_page' not described in 'nand_manufacturer_ops'
> 
> Do you want me to send a v3 for that? (given our timezones I'll just 
> send it anyway and you can choose to take it or just stick with v2).

I'll take v3 for sure, thanks for already sending it.

> 
> > * Patch 4 I think I'll just drop it as nobody can test it (and it's a
> >    oneliner). I prefer people requesting a feature than complaining
> >    about a bug :)  
> 
> Agreed. It's in the mailing list archive now if someone needs it we can 
> resurrect it from there
> 
> > * Boris will take patch 5 in the mtd/fixes branch.  
> 
> Thanks all. Sorry our emails crossed so I couldn't just send #5 as a fix 
> on it's own.
> 
> > * I'll let you write a v3 for patch 6 only.  
> 
> I'll have a read through the old thread and see if I can make sense of it.

Thanks!

Miquèl

Re: [RFC PATCH v2 5/6] mtd: rawnand: micron: add ONFI_FEATURE_ON_DIE_ECC to supported features

[Miquel Raynal]

> On Tue, 19 Jun 2018 17:31:24 +1200
> Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
> 
> > Add ONFI_FEATURE_ON_DIE_ECC to the set/get features list for Micron
> > NAND flash.
> >  
> 
> Fixes: 789157e41a06 ("mtd: rawnand: allow vendors to declare (un)supported features")
> Cc: <stable@vger.kernel.org>
> 
> No need to send a new version, I'll add that when queuing the patch.
> 
> Miquel, if you're okay, I'm gonna take this patch in the mtd/fixes branch
> and let you deal with other patches in this series.
> 
> > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

Re: [RFC PATCH v2 5/6] mtd: rawnand: micron: add ONFI_FEATURE_ON_DIE_ECC to supported features

[Boris Brezillon]

On Tue, 19 Jun 2018 17:31:24 +1200
Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:

> Add ONFI_FEATURE_ON_DIE_ECC to the set/get features list for Micron
> NAND flash.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Queued to master (AKA mtd/fixes).

Thanks,

Boris

> ---
> Changes in v2:
> - New
> 
>  drivers/mtd/nand/raw/nand_micron.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
> index e582c9e61715..d1e8f57544a0 100644
> --- a/drivers/mtd/nand/raw/nand_micron.c
> +++ b/drivers/mtd/nand/raw/nand_micron.c
> @@ -66,7 +66,9 @@ static int micron_nand_onfi_init(struct nand_chip *chip)
>  
>  	if (p->supports_set_get_features) {
>  		set_bit(ONFI_FEATURE_ADDR_READ_RETRY, p->set_feature_list);
> +		set_bit(ONFI_FEATURE_ON_DIE_ECC, p->set_feature_list);
>  		set_bit(ONFI_FEATURE_ADDR_READ_RETRY, p->get_feature_list);
> +		set_bit(ONFI_FEATURE_ON_DIE_ECC, p->get_feature_list);
>  	}
>  
>  	return 0;

[RFC PATCH v2 6/6] mtd: rawnand: micron: support 8/512 on-die ECC

[Chris Packham]

Micron MT29F1G08ABAFAWP-ITE:F supports an on-die ECC with 8 bits
per 512 bytes. Add support for this combination.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
This seems deceptively easy so I've probably missed something. I have
tested with running some of the ubifs stress tests from mtd-utils and
things seem OK.

Changes in v2:
- New

 drivers/mtd/nand/raw/nand_micron.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index d1e8f57544a0..2164dd112f5c 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -240,9 +240,9 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
 
 	/*
 	 * Some Micron NANDs have an on-die ECC of 4/512, some other
-	 * 8/512. We only support the former.
+	 * 8/512.
 	 */
-	if (chip->ecc_strength_ds != 4)
+	if (chip->ecc_strength_ds != 4 && chip->ecc_strength_ds != 8)
 		return MICRON_ON_DIE_UNSUPPORTED;
 
 	return MICRON_ON_DIE_SUPPORTED;
@@ -274,9 +274,9 @@ static int micron_nand_init(struct nand_chip *chip)
 			return -EINVAL;
 		}
 
-		chip->ecc.bytes = 8;
+		chip->ecc.bytes = chip->ecc_strength_ds * 2;
 		chip->ecc.size = 512;
-		chip->ecc.strength = 4;
+		chip->ecc.strength = chip->ecc_strength_ds;
 		chip->ecc.algo = NAND_ECC_BCH;
 		chip->ecc.read_page = micron_nand_read_page_on_die_ecc;
 		chip->ecc.write_page = micron_nand_write_page_on_die_ecc;
-- 
2.17.1

Re: [RFC PATCH v2 6/6] mtd: rawnand: micron: support 8/512 on-die ECC

[Boris Brezillon]

On Tue, 19 Jun 2018 17:31:25 +1200
Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:

> Micron MT29F1G08ABAFAWP-ITE:F supports an on-die ECC with 8 bits
> per 512 bytes. Add support for this combination.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> This seems deceptively easy so I've probably missed something. I have
> tested with running some of the ubifs stress tests from mtd-utils and
> things seem OK.
> 
> Changes in v2:
> - New
> 
>  drivers/mtd/nand/raw/nand_micron.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
> index d1e8f57544a0..2164dd112f5c 100644
> --- a/drivers/mtd/nand/raw/nand_micron.c
> +++ b/drivers/mtd/nand/raw/nand_micron.c
> @@ -240,9 +240,9 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
>  
>  	/*
>  	 * Some Micron NANDs have an on-die ECC of 4/512, some other
> -	 * 8/512. We only support the former.
> +	 * 8/512.
>  	 */
> -	if (chip->ecc_strength_ds != 4)
> +	if (chip->ecc_strength_ds != 4 && chip->ecc_strength_ds != 8)
>  		return MICRON_ON_DIE_UNSUPPORTED;

I remember that chips with 8bits/512bytes on-die ECC had an extra
command to query a more precise number of bitflips (see this
discussion [1]). It's probably worth implementing that, since those
chips are more likely to have bitflips than the 4bit/512 versions, and
you don't want to move the data around as soon as you have one bitflip. 

>  
>  	return MICRON_ON_DIE_SUPPORTED;
> @@ -274,9 +274,9 @@ static int micron_nand_init(struct nand_chip *chip)
>  			return -EINVAL;
>  		}
>  
> -		chip->ecc.bytes = 8;
> +		chip->ecc.bytes = chip->ecc_strength_ds * 2;
>  		chip->ecc.size = 512;
> -		chip->ecc.strength = 4;
> +		chip->ecc.strength = chip->ecc_strength_ds;
>  		chip->ecc.algo = NAND_ECC_BCH;
>  		chip->ecc.read_page = micron_nand_read_page_on_die_ecc;
>  		chip->ecc.write_page = micron_nand_write_page_on_die_ecc;

[1]http://lists.infradead.org/pipermail/linux-mtd/2017-March/072974.html