* [PATCH 1/2] mtd: rawnand: micron: Define the proper layout for 8bit/512bytes on-die ECC
2018-07-09 21:09 [PATCH 0/2] mtd: rawnand: micron: Fix on-die ECC Boris Brezillon
@ 2018-07-09 21:09 ` Boris Brezillon
2018-07-16 8:37 ` Chris Packham
2018-07-09 21:09 ` [PATCH 2/2] mtd: rawnand: micron: Fix on-die ECC detection logic Boris Brezillon
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Boris Brezillon @ 2018-07-09 21:09 UTC (permalink / raw)
To: Boris Brezillon, Richard Weinberger, Miquel Raynal, linux-mtd,
Bean Huo (beanhuo),
Chris Packham
Cc: David Woodhouse, Brian Norris, Marek Vasut
The MT29F1G08ABAFAWP datasheet says that all ECC bytes are placed at the
end of the OOB area.
Fixes: 386fc2609d15 ("mtd: rawnand: micron: support 8/512 on-die ECC")
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
drivers/mtd/nand/raw/nand_micron.c | 59 ++++++++++++++++++++++++++++++++------
1 file changed, 50 insertions(+), 9 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index 754e9cdd44ac..00a965ad7ab2 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -103,8 +103,9 @@ static int micron_nand_onfi_init(struct nand_chip *chip)
return 0;
}
-static int micron_nand_on_die_ooblayout_ecc(struct mtd_info *mtd, int section,
- struct mtd_oob_region *oobregion)
+static int micron_nand_on_die_4_ooblayout_ecc(struct mtd_info *mtd,
+ int section,
+ struct mtd_oob_region *oobregion)
{
if (section >= 4)
return -ERANGE;
@@ -115,8 +116,9 @@ static int micron_nand_on_die_ooblayout_ecc(struct mtd_info *mtd, int section,
return 0;
}
-static int micron_nand_on_die_ooblayout_free(struct mtd_info *mtd, int section,
- struct mtd_oob_region *oobregion)
+static int micron_nand_on_die_4_ooblayout_free(struct mtd_info *mtd,
+ int section,
+ struct mtd_oob_region *oobregion)
{
if (section >= 4)
return -ERANGE;
@@ -127,9 +129,44 @@ static int micron_nand_on_die_ooblayout_free(struct mtd_info *mtd, int section,
return 0;
}
-static const struct mtd_ooblayout_ops micron_nand_on_die_ooblayout_ops = {
- .ecc = micron_nand_on_die_ooblayout_ecc,
- .free = micron_nand_on_die_ooblayout_free,
+static const struct mtd_ooblayout_ops micron_nand_on_die_4_ooblayout_ops = {
+ .ecc = micron_nand_on_die_4_ooblayout_ecc,
+ .free = micron_nand_on_die_4_ooblayout_free,
+};
+
+static int micron_nand_on_die_8_ooblayout_ecc(struct mtd_info *mtd,
+ int section,
+ struct mtd_oob_region *oobregion)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+
+ if (section)
+ return -ERANGE;
+
+ oobregion->offset = mtd->oobsize - chip->ecc.total;
+ oobregion->length = chip->ecc.total;
+
+ return 0;
+}
+
+static int micron_nand_on_die_8_ooblayout_free(struct mtd_info *mtd,
+ int section,
+ struct mtd_oob_region *oobregion)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+
+ if (section)
+ return -ERANGE;
+
+ oobregion->offset = 2;
+ oobregion->length = mtd->oobsize - chip->ecc.total - 2;
+
+ return 0;
+}
+
+static const struct mtd_ooblayout_ops micron_nand_on_die_8_ooblayout_ops = {
+ .ecc = micron_nand_on_die_8_ooblayout_ecc,
+ .free = micron_nand_on_die_8_ooblayout_free,
};
static int micron_nand_on_die_ecc_setup(struct nand_chip *chip, bool enable)
@@ -425,6 +462,12 @@ static int micron_nand_init(struct nand_chip *chip)
ret = -ENOMEM;
goto err_free_manuf_data;
}
+
+ mtd_set_ooblayout(mtd,
+ µn_nand_on_die_4_ooblayout_ops);
+ } else {
+ mtd_set_ooblayout(mtd,
+ µn_nand_on_die_8_ooblayout_ops);
}
chip->ecc.bytes = chip->ecc_strength_ds * 2;
@@ -435,8 +478,6 @@ static int micron_nand_init(struct nand_chip *chip)
chip->ecc.write_page = micron_nand_write_page_on_die_ecc;
chip->ecc.read_page_raw = nand_read_page_raw;
chip->ecc.write_page_raw = nand_write_page_raw;
-
- mtd_set_ooblayout(mtd, µn_nand_on_die_ooblayout_ops);
}
return 0;
--
2.14.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] mtd: rawnand: micron: Define the proper layout for 8bit/512bytes on-die ECC
2018-07-09 21:09 ` [PATCH 1/2] mtd: rawnand: micron: Define the proper layout for 8bit/512bytes " Boris Brezillon
@ 2018-07-16 8:37 ` Chris Packham
0 siblings, 0 replies; 9+ messages in thread
From: Chris Packham @ 2018-07-16 8:37 UTC (permalink / raw)
To: Boris Brezillon, Richard Weinberger, Miquel Raynal, linux-mtd,
Bean Huo (beanhuo)
Cc: David Woodhouse, Brian Norris, Marek Vasut
On 10/07/18 09:09, Boris Brezillon wrote:
> The MT29F1G08ABAFAWP datasheet says that all ECC bytes are placed at the
> end of the OOB area.
>
> Fixes: 386fc2609d15 ("mtd: rawnand: micron: support 8/512 on-die ECC")
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
With d73e5d29 reverted,
Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> drivers/mtd/nand/raw/nand_micron.c | 59 ++++++++++++++++++++++++++++++++------
> 1 file changed, 50 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
> index 754e9cdd44ac..00a965ad7ab2 100644
> --- a/drivers/mtd/nand/raw/nand_micron.c
> +++ b/drivers/mtd/nand/raw/nand_micron.c
> @@ -103,8 +103,9 @@ static int micron_nand_onfi_init(struct nand_chip *chip)
> return 0;
> }
>
> -static int micron_nand_on_die_ooblayout_ecc(struct mtd_info *mtd, int section,
> - struct mtd_oob_region *oobregion)
> +static int micron_nand_on_die_4_ooblayout_ecc(struct mtd_info *mtd,
> + int section,
> + struct mtd_oob_region *oobregion)
> {
> if (section >= 4)
> return -ERANGE;
> @@ -115,8 +116,9 @@ static int micron_nand_on_die_ooblayout_ecc(struct mtd_info *mtd, int section,
> return 0;
> }
>
> -static int micron_nand_on_die_ooblayout_free(struct mtd_info *mtd, int section,
> - struct mtd_oob_region *oobregion)
> +static int micron_nand_on_die_4_ooblayout_free(struct mtd_info *mtd,
> + int section,
> + struct mtd_oob_region *oobregion)
> {
> if (section >= 4)
> return -ERANGE;
> @@ -127,9 +129,44 @@ static int micron_nand_on_die_ooblayout_free(struct mtd_info *mtd, int section,
> return 0;
> }
>
> -static const struct mtd_ooblayout_ops micron_nand_on_die_ooblayout_ops = {
> - .ecc = micron_nand_on_die_ooblayout_ecc,
> - .free = micron_nand_on_die_ooblayout_free,
> +static const struct mtd_ooblayout_ops micron_nand_on_die_4_ooblayout_ops = {
> + .ecc = micron_nand_on_die_4_ooblayout_ecc,
> + .free = micron_nand_on_die_4_ooblayout_free,
> +};
> +
> +static int micron_nand_on_die_8_ooblayout_ecc(struct mtd_info *mtd,
> + int section,
> + struct mtd_oob_region *oobregion)
> +{
> + struct nand_chip *chip = mtd_to_nand(mtd);
> +
> + if (section)
> + return -ERANGE;
> +
> + oobregion->offset = mtd->oobsize - chip->ecc.total;
> + oobregion->length = chip->ecc.total;
> +
> + return 0;
> +}
> +
> +static int micron_nand_on_die_8_ooblayout_free(struct mtd_info *mtd,
> + int section,
> + struct mtd_oob_region *oobregion)
> +{
> + struct nand_chip *chip = mtd_to_nand(mtd);
> +
> + if (section)
> + return -ERANGE;
> +
> + oobregion->offset = 2;
> + oobregion->length = mtd->oobsize - chip->ecc.total - 2;
> +
> + return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops micron_nand_on_die_8_ooblayout_ops = {
> + .ecc = micron_nand_on_die_8_ooblayout_ecc,
> + .free = micron_nand_on_die_8_ooblayout_free,
> };
>
> static int micron_nand_on_die_ecc_setup(struct nand_chip *chip, bool enable)
> @@ -425,6 +462,12 @@ static int micron_nand_init(struct nand_chip *chip)
> ret = -ENOMEM;
> goto err_free_manuf_data;
> }
> +
> + mtd_set_ooblayout(mtd,
> + µn_nand_on_die_4_ooblayout_ops);
> + } else {
> + mtd_set_ooblayout(mtd,
> + µn_nand_on_die_8_ooblayout_ops);
> }
>
> chip->ecc.bytes = chip->ecc_strength_ds * 2;
> @@ -435,8 +478,6 @@ static int micron_nand_init(struct nand_chip *chip)
> chip->ecc.write_page = micron_nand_write_page_on_die_ecc;
> chip->ecc.read_page_raw = nand_read_page_raw;
> chip->ecc.write_page_raw = nand_write_page_raw;
> -
> - mtd_set_ooblayout(mtd, µn_nand_on_die_ooblayout_ops);
> }
>
> return 0;
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] mtd: rawnand: micron: Fix on-die ECC detection logic
2018-07-09 21:09 [PATCH 0/2] mtd: rawnand: micron: Fix on-die ECC Boris Brezillon
2018-07-09 21:09 ` [PATCH 1/2] mtd: rawnand: micron: Define the proper layout for 8bit/512bytes " Boris Brezillon
@ 2018-07-09 21:09 ` Boris Brezillon
2018-07-16 8:36 ` Chris Packham
2018-07-10 8:10 ` [PATCH 0/2] mtd: rawnand: micron: Fix on-die ECC Boris Brezillon
2018-07-16 8:07 ` Chris Packham
3 siblings, 1 reply; 9+ messages in thread
From: Boris Brezillon @ 2018-07-09 21:09 UTC (permalink / raw)
To: Boris Brezillon, Richard Weinberger, Miquel Raynal, linux-mtd,
Bean Huo (beanhuo),
Chris Packham
Cc: David Woodhouse, Brian Norris, Marek Vasut
Basing the "mandatory on-die" detection on ID byte 2 does not work,
because Micron has plenty of NANDs using the same device ID code, and
not all of them have forcibly enabled on-die ECC.
Since the "Array Operation" feature does not provide the "ECC
enabled/disabled" bit when the ECC can't be disabled, let's try to use
the "ECC enabled/disabled" bit in the READ_ID bytes.
It seems that this bit is dynamically updated on NANDs where on-die ECC
can freely be enabled/disabled, so let's hope it stays at one when we
have a NAND with on-die ECC forcibly enabled.
Fixes: 51f3b3970a8c ("mtd: rawnand: micron: detect forced on-die ECC")
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
drivers/mtd/nand/raw/nand_micron.c | 51 +++++++++++++++++++-------------------
1 file changed, 25 insertions(+), 26 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index 00a965ad7ab2..6ae708fbbd3a 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -345,13 +345,8 @@ enum {
MICRON_ON_DIE_MANDATORY,
};
-/*
- * These parts are known to have on-die ECC forceably enabled
- */
-static u8 micron_on_die_ecc[] = {
- 0xd1, /* MT29F1G08ABAFA */
- 0xa1, /* MT29F1G08ABBFA */
-};
+#define MICRON_ID_INTERNAL_ECC_MASK GENMASK(1, 0)
+#define MICRON_ID_ECC_ENABLED BIT(7)
/*
* Try to detect if the NAND support on-die ECC. To do this, we enable
@@ -366,12 +361,8 @@ static u8 micron_on_die_ecc[] = {
static int micron_supports_on_die_ecc(struct nand_chip *chip)
{
u8 feature[ONFI_SUBFEATURE_PARAM_LEN] = { 0, };
+ u8 id[5];
int ret;
- int i;
-
- for (i = 0; i < ARRAY_SIZE(micron_on_die_ecc); i++)
- if (chip->id.data[1] == micron_on_die_ecc[i])
- return MICRON_ON_DIE_MANDATORY;
if (!chip->parameters.onfi.version)
return MICRON_ON_DIE_UNSUPPORTED;
@@ -379,34 +370,42 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
if (chip->bits_per_cell != 1)
return MICRON_ON_DIE_UNSUPPORTED;
+ /*
+ * We only support on-die ECC of 4/512 or 8/512
+ */
+ if (chip->ecc_strength_ds != 4 && chip->ecc_strength_ds != 8)
+ return MICRON_ON_DIE_UNSUPPORTED;
+
+ /*
+ * We don't know what INTERNAL_ECC means, but it seems that 0x2 matches
+ * all NANDs with on-die ECC.
+ */
+ if (chip->id.len != 5 ||
+ (chip->id.data[4] & MICRON_ID_INTERNAL_ECC_MASK) != 0x2)
+ return MICRON_ON_DIE_UNSUPPORTED;
+
ret = micron_nand_on_die_ecc_setup(chip, true);
if (ret)
return MICRON_ON_DIE_UNSUPPORTED;
- ret = nand_get_features(chip, ONFI_FEATURE_ON_DIE_ECC, feature);
- if (ret < 0)
- return ret;
+ ret = nand_readid_op(chip, 0, id, sizeof(id));
+ if (ret)
+ return MICRON_ON_DIE_UNSUPPORTED;
- if ((feature[0] & ONFI_FEATURE_ON_DIE_ECC_EN) == 0)
+ if (!(id[4] & MICRON_ID_ECC_ENABLED))
return MICRON_ON_DIE_UNSUPPORTED;
ret = micron_nand_on_die_ecc_setup(chip, false);
if (ret)
return MICRON_ON_DIE_UNSUPPORTED;
- ret = nand_get_features(chip, ONFI_FEATURE_ON_DIE_ECC, feature);
- if (ret < 0)
- return ret;
+ ret = nand_readid_op(chip, 0, id, sizeof(id));
+ if (ret)
+ return MICRON_ON_DIE_UNSUPPORTED;
- if (feature[0] & ONFI_FEATURE_ON_DIE_ECC_EN)
+ if (id[4] & MICRON_ID_ECC_ENABLED)
return MICRON_ON_DIE_MANDATORY;
- /*
- * We only support on-die ECC of 4/512 or 8/512
- */
- if (chip->ecc_strength_ds != 4 && chip->ecc_strength_ds != 8)
- return MICRON_ON_DIE_UNSUPPORTED;
-
return MICRON_ON_DIE_SUPPORTED;
}
--
2.14.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] mtd: rawnand: micron: Fix on-die ECC detection logic
2018-07-09 21:09 ` [PATCH 2/2] mtd: rawnand: micron: Fix on-die ECC detection logic Boris Brezillon
@ 2018-07-16 8:36 ` Chris Packham
0 siblings, 0 replies; 9+ messages in thread
From: Chris Packham @ 2018-07-16 8:36 UTC (permalink / raw)
To: Boris Brezillon, Richard Weinberger, Miquel Raynal, linux-mtd,
Bean Huo (beanhuo)
Cc: David Woodhouse, Brian Norris, Marek Vasut
Hi Boris,
On 10/07/18 09:09, Boris Brezillon wrote:
> Basing the "mandatory on-die" detection on ID byte 2 does not work,
> because Micron has plenty of NANDs using the same device ID code, and
> not all of them have forcibly enabled on-die ECC.
>
> Since the "Array Operation" feature does not provide the "ECC
> enabled/disabled" bit when the ECC can't be disabled, let's try to use
> the "ECC enabled/disabled" bit in the READ_ID bytes.
>
> It seems that this bit is dynamically updated on NANDs where on-die ECC
> can freely be enabled/disabled, so let's hope it stays at one when we
> have a NAND with on-die ECC forcibly enabled.
>
> Fixes: 51f3b3970a8c ("mtd: rawnand: micron: detect forced on-die ECC")
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
With d73e5d29 reverted,
Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
One compiler warning below.
> ---
> drivers/mtd/nand/raw/nand_micron.c | 51 +++++++++++++++++++-------------------
> 1 file changed, 25 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
> index 00a965ad7ab2..6ae708fbbd3a 100644
> --- a/drivers/mtd/nand/raw/nand_micron.c
> +++ b/drivers/mtd/nand/raw/nand_micron.c
> @@ -345,13 +345,8 @@ enum {
> MICRON_ON_DIE_MANDATORY,
> };
>
> -/*
> - * These parts are known to have on-die ECC forceably enabled
> - */
> -static u8 micron_on_die_ecc[] = {
> - 0xd1, /* MT29F1G08ABAFA */
> - 0xa1, /* MT29F1G08ABBFA */
> -};
> +#define MICRON_ID_INTERNAL_ECC_MASK GENMASK(1, 0)
> +#define MICRON_ID_ECC_ENABLED BIT(7)
>
> /*
> * Try to detect if the NAND support on-die ECC. To do this, we enable
> @@ -366,12 +361,8 @@ static u8 micron_on_die_ecc[] = {
> static int micron_supports_on_die_ecc(struct nand_chip *chip)
> {
> u8 feature[ONFI_SUBFEATURE_PARAM_LEN] = { 0, };
feature is now unused so this causes a compiler warning.
> + u8 id[5];
> int ret;
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(micron_on_die_ecc); i++)
> - if (chip->id.data[1] == micron_on_die_ecc[i])
> - return MICRON_ON_DIE_MANDATORY;
>
> if (!chip->parameters.onfi.version)
> return MICRON_ON_DIE_UNSUPPORTED;
> @@ -379,34 +370,42 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
> if (chip->bits_per_cell != 1)
> return MICRON_ON_DIE_UNSUPPORTED;
>
> + /*
> + * We only support on-die ECC of 4/512 or 8/512
> + */
> + if (chip->ecc_strength_ds != 4 && chip->ecc_strength_ds != 8)
> + return MICRON_ON_DIE_UNSUPPORTED;
> +
> + /*
> + * We don't know what INTERNAL_ECC means, but it seems that 0x2 matches
> + * all NANDs with on-die ECC.
> + */
> + if (chip->id.len != 5 ||
> + (chip->id.data[4] & MICRON_ID_INTERNAL_ECC_MASK) != 0x2)
> + return MICRON_ON_DIE_UNSUPPORTED;
> +
> ret = micron_nand_on_die_ecc_setup(chip, true);
> if (ret)
> return MICRON_ON_DIE_UNSUPPORTED;
>
> - ret = nand_get_features(chip, ONFI_FEATURE_ON_DIE_ECC, feature);
> - if (ret < 0)
> - return ret;
> + ret = nand_readid_op(chip, 0, id, sizeof(id));
> + if (ret)
> + return MICRON_ON_DIE_UNSUPPORTED;
>
> - if ((feature[0] & ONFI_FEATURE_ON_DIE_ECC_EN) == 0)
> + if (!(id[4] & MICRON_ID_ECC_ENABLED))
> return MICRON_ON_DIE_UNSUPPORTED;
>
> ret = micron_nand_on_die_ecc_setup(chip, false);
> if (ret)
> return MICRON_ON_DIE_UNSUPPORTED;
>
> - ret = nand_get_features(chip, ONFI_FEATURE_ON_DIE_ECC, feature);
> - if (ret < 0)
> - return ret;
> + ret = nand_readid_op(chip, 0, id, sizeof(id));
> + if (ret)
> + return MICRON_ON_DIE_UNSUPPORTED;
>
> - if (feature[0] & ONFI_FEATURE_ON_DIE_ECC_EN)
> + if (id[4] & MICRON_ID_ECC_ENABLED)
> return MICRON_ON_DIE_MANDATORY;
>
> - /*
> - * We only support on-die ECC of 4/512 or 8/512
> - */
> - if (chip->ecc_strength_ds != 4 && chip->ecc_strength_ds != 8)
> - return MICRON_ON_DIE_UNSUPPORTED;
> -
> return MICRON_ON_DIE_SUPPORTED;
> }
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] mtd: rawnand: micron: Fix on-die ECC
2018-07-09 21:09 [PATCH 0/2] mtd: rawnand: micron: Fix on-die ECC Boris Brezillon
2018-07-09 21:09 ` [PATCH 1/2] mtd: rawnand: micron: Define the proper layout for 8bit/512bytes " Boris Brezillon
2018-07-09 21:09 ` [PATCH 2/2] mtd: rawnand: micron: Fix on-die ECC detection logic Boris Brezillon
@ 2018-07-10 8:10 ` Boris Brezillon
2018-07-10 21:40 ` Chris Packham
2018-07-16 8:07 ` Chris Packham
3 siblings, 1 reply; 9+ messages in thread
From: Boris Brezillon @ 2018-07-10 8:10 UTC (permalink / raw)
To: Boris Brezillon, Richard Weinberger, Miquel Raynal, linux-mtd,
Bean Huo (beanhuo),
Chris Packham
Cc: Marek Vasut, Brian Norris, David Woodhouse
On Mon, 9 Jul 2018 23:09:35 +0200
Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> Chris, Bean,
>
> Here are 2 patches for you to review/test. The first one is fixing
> the layout definition, and unless I missed something it should be
> correct.
>
> The second one is just my understanding of how byte 5 of READ_ID works
> based on our experience with the 4bit/512 on-die ECC chip we have
> worked on and the other datasheet I had a look at.
> I'm not 100% sure this will work for all chips, but might work for the
> 2 chips we support right now.
I tested it on a MT29F2G08ABAEAH4 (4bit/512bytes on-die ECC) and it
works as expected. When I do the SET_FEATURES(ECC_EN) the ECC enabled
bit in READID byte 5 is set, and when I do SET_FEATURES(ECC_DIS), the
bit is cleared.
Now we need to make sure it works correctly on MT29F1G08ABAFAWP.
> So please test and/or review it and let me know if this approach works.
>
> Regards,
>
> Boris
>
> Boris Brezillon (2):
> mtd: rawnand: micron: Define the proper layout for 8bit/512bytes
> on-die ECC
> mtd: rawnand: micron: Fix on-die ECC detection logic
>
> drivers/mtd/nand/raw/nand_micron.c | 110 +++++++++++++++++++++++++------------
> 1 file changed, 75 insertions(+), 35 deletions(-)
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] mtd: rawnand: micron: Fix on-die ECC
2018-07-10 8:10 ` [PATCH 0/2] mtd: rawnand: micron: Fix on-die ECC Boris Brezillon
@ 2018-07-10 21:40 ` Chris Packham
0 siblings, 0 replies; 9+ messages in thread
From: Chris Packham @ 2018-07-10 21:40 UTC (permalink / raw)
To: Boris Brezillon, Richard Weinberger, Miquel Raynal, linux-mtd,
Bean Huo (beanhuo)
Cc: Marek Vasut, Brian Norris, David Woodhouse
Hi Boris,
On 10/07/18 20:11, Boris Brezillon wrote:
> On Mon, 9 Jul 2018 23:09:35 +0200
> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
>
>> Chris, Bean,
>>
>> Here are 2 patches for you to review/test. The first one is fixing
>> the layout definition, and unless I missed something it should be
>> correct.
>>
>> The second one is just my understanding of how byte 5 of READ_ID works
>> based on our experience with the 4bit/512 on-die ECC chip we have
>> worked on and the other datasheet I had a look at.
>> I'm not 100% sure this will work for all chips, but might work for the
>> 2 chips we support right now.
>
> I tested it on a MT29F2G08ABAEAH4 (4bit/512bytes on-die ECC) and it
> works as expected. When I do the SET_FEATURES(ECC_EN) the ECC enabled
> bit in READID byte 5 is set, and when I do SET_FEATURES(ECC_DIS), the
> bit is cleared.
>
> Now we need to make sure it works correctly on MT29F1G08ABAFAWP.
I'll test that as soon as I can get access to the hardware (probably
Monday next week).
>
>> So please test and/or review it and let me know if this approach works.
>>
>> Regards,
>>
>> Boris
>>
>> Boris Brezillon (2):
>> mtd: rawnand: micron: Define the proper layout for 8bit/512bytes
>> on-die ECC
>> mtd: rawnand: micron: Fix on-die ECC detection logic
>>
>> drivers/mtd/nand/raw/nand_micron.c | 110 +++++++++++++++++++++++++------------
>> 1 file changed, 75 insertions(+), 35 deletions(-)
>>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] mtd: rawnand: micron: Fix on-die ECC
2018-07-09 21:09 [PATCH 0/2] mtd: rawnand: micron: Fix on-die ECC Boris Brezillon
` (2 preceding siblings ...)
2018-07-10 8:10 ` [PATCH 0/2] mtd: rawnand: micron: Fix on-die ECC Boris Brezillon
@ 2018-07-16 8:07 ` Chris Packham
3 siblings, 0 replies; 9+ messages in thread
From: Chris Packham @ 2018-07-16 8:07 UTC (permalink / raw)
To: Boris Brezillon, Richard Weinberger, Miquel Raynal, linux-mtd,
Bean Huo (beanhuo)
Cc: David Woodhouse, Brian Norris, Marek Vasut
On 10/07/18 09:09, Boris Brezillon wrote:
> Chris, Bean,
>
> Here are 2 patches for you to review/test. The first one is fixing
> the layout definition, and unless I missed something it should be
> correct.
>
> The second one is just my understanding of how byte 5 of READ_ID works
> based on our experience with the 4bit/512 on-die ECC chip we have
> worked on and the other datasheet I had a look at.
> I'm not 100% sure this will work for all chips, but might work for the
> 2 chips we support right now.
> So please test and/or review it and let me know if this approach works.
>
Hi Boris,
I went to test this today and found that I could no-longer mount the
Micron MT29F1G08ABAFAWP-ITE chip. However it doesn't appear to these
changes but d73e5d29 ("mtd: rawnand: micron: Disable ECC earlier in the
read path").
[root@linuxbox ~]# ubiformat -q -y /dev/mtd0
[root@linuxbox ~]# ubiattach -p /dev/mtd0
ubi0: attaching mtd0
ubi0: scanning is finished
ubi0 error: ubi_read_volume_table: the layout volume was not found
ubi0 error: ubi_attach_mtd_dev: failed to attach mtd0, error -22
ubiattach: error!: cannot attach "/dev/mtd0"
error 22 (Invalid argument)
[root@linuxbox ~]# ubimkvol /dev/ubi0 -N user -m
libubi: error!: cannot get information about "/dev/ubi0"
error 2 (No such file or directory)
ubimkvol: error!: error while probing "/dev/ubi0"
error 2 (No such file or directory)
I'll try juggling some of the nand/next patches around to see if I can
test these two without d73e5d29.
^ permalink raw reply [flat|nested] 9+ messages in thread