This patch series enables to support Toshiba BENAND. The first patch
retreive ECC requirement from extended ID for all Toshiba SLC NAND
devices. The ECC requirement information uses to show a warning
message in second patch.
KOBAYASHI Yoshitake (2):
mtd: nand: toshiba: Retrieve ECC requirements from extended ID
mtd: nand: toshiba: Add support for Toshiba Memory BENAND (Built-in
ECC NAND)
drivers/mtd/nand/nand_toshiba.c | 117 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 117 insertions(+)
--
2.7.4
This patch enables support for Toshiba Memory BENAND. This checks
internal ECC status in read operation when using BENAND and selecting
ECC mode as on-die.
Signed-off-by: KOBAYASHI Yoshitake <[email protected]>
---
drivers/mtd/nand/nand_toshiba.c | 89 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 89 insertions(+)
diff --git a/drivers/mtd/nand/nand_toshiba.c b/drivers/mtd/nand/nand_toshiba.c
index c2c141b..35c0ddf 100644
--- a/drivers/mtd/nand/nand_toshiba.c
+++ b/drivers/mtd/nand/nand_toshiba.c
@@ -17,6 +17,91 @@
#include <linux/mtd/rawnand.h>
+/* ECC Status Read Command for BENAND */
+#define TOSHIBA_NAND_CMD_ECC_STATUS 0x7A
+
+/* Recommended to rewrite for BENAND */
+#define TOSHIBA_NAND_STATUS_REWRITE_RECOMMENDED BIT(3)
+
+static int toshiba_nand_benand_status_chk(struct mtd_info *mtd,
+ struct nand_chip *chip)
+{
+ unsigned int max_bitflips = 0;
+ u8 status;
+
+ /* Check Read Status */
+ chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
+ status = chip->read_byte(mtd);
+
+ /*
+ * TOSHIBA_NAND_CMD_ECC_STATUS is vendor specific command.
+ * Currently we have no way to send arbitrary sequences of
+ * CMD+ADDR+DATA cycles and thus cannot support this custom
+ * TOSHIBA_NAND_CMD_ECC_STATUS operation.
+ * For now, we set max_bitflips mtd->bitflip_threshold.
+ */
+ if (status & NAND_STATUS_FAIL) {
+ /* uncorrectable */
+ mtd->ecc_stats.failed++;
+ } else if (status & TOSHIBA_NAND_STATUS_REWRITE_RECOMMENDED) {
+ /* correctable */
+ max_bitflips = mtd->bitflip_threshold;
+ mtd->ecc_stats.corrected += max_bitflips;
+ }
+
+ return max_bitflips;
+}
+
+static int
+toshiba_nand_read_page_benand(struct mtd_info *mtd,
+ struct nand_chip *chip, uint8_t *buf,
+ int oob_required, int page)
+{
+ nand_read_page_raw(mtd, chip, buf, oob_required, page);
+
+ return toshiba_nand_benand_status_chk(mtd, chip);
+}
+
+static int
+toshiba_nand_read_subpage_benand(struct mtd_info *mtd,
+ struct nand_chip *chip, uint32_t data_offs,
+ uint32_t readlen, uint8_t *bufpoi, int page)
+{
+ uint8_t *p;
+
+ if (data_offs != 0)
+ chip->cmdfunc(mtd, NAND_CMD_RNDOUT, data_offs, -1);
+
+ p = bufpoi + data_offs;
+ chip->read_buf(mtd, p, readlen);
+
+ return toshiba_nand_benand_status_chk(mtd, chip);
+}
+
+static void toshiba_nand_benand_init(struct nand_chip *chip)
+{
+ struct mtd_info *mtd = nand_to_mtd(chip);
+
+ /*
+ * On BENAND, the entire OOB region can be used by the MTD user.
+ * The calculated ECC bytes are stored into other isolated
+ * area which is not accessible to users.
+ * This is why chip->ecc.bytes = 0.
+ */
+ chip->ecc.bytes = 0;
+ chip->ecc.size = 512;
+ chip->ecc.strength = 8;
+ chip->ecc.read_page = toshiba_nand_read_page_benand;
+ chip->ecc.read_subpage = toshiba_nand_read_subpage_benand;
+ chip->ecc.write_page = nand_write_page_raw;
+ chip->ecc.read_page_raw = nand_read_page_raw;
+ chip->ecc.write_page_raw = nand_write_page_raw;
+
+ chip->options |= NAND_SUBPAGE_READ;
+
+ mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
+}
+
static void toshiba_nand_decode_id(struct nand_chip *chip)
{
struct mtd_info *mtd = nand_to_mtd(chip);
@@ -70,6 +155,10 @@ static int toshiba_nand_init(struct nand_chip *chip)
if (nand_is_slc(chip))
chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
+ if (nand_is_slc(chip) && (chip->id.data[4] & 0x80) /* BENAND */ &&
+ (chip->ecc.mode == NAND_ECC_ON_DIE))
+ toshiba_nand_benand_init(chip);
+
return 0;
}
--
2.7.4
This patch enables support to read the ECC strength and size from the
NAND flash using Toshiba Memory SLC NAND extended-ID. This patch is
based on the information of the 6th ID byte of the Toshiba Memory SLC
NAND.
Signed-off-by: KOBAYASHI Yoshitake <[email protected]>
---
drivers/mtd/nand/nand_toshiba.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/mtd/nand/nand_toshiba.c b/drivers/mtd/nand/nand_toshiba.c
index 57df857..c2c141b 100644
--- a/drivers/mtd/nand/nand_toshiba.c
+++ b/drivers/mtd/nand/nand_toshiba.c
@@ -35,6 +35,34 @@ static void toshiba_nand_decode_id(struct nand_chip *chip)
(chip->id.data[5] & 0x7) == 0x6 /* 24nm */ &&
!(chip->id.data[4] & 0x80) /* !BENAND */)
mtd->oobsize = 32 * mtd->writesize >> 9;
+
+ /*
+ * Extract ECC requirements from 6th id byte.
+ * For Toshiba SLC, ecc requrements are as follows:
+ * - 43nm: 1 bit ECC for each 512Byte is required.
+ * - 32nm: 4 bit ECC for each 512Byte is required.
+ * - 24nm: 8 bit ECC for each 512Byte is required.
+ */
+ if (chip->id.len >= 6 && nand_is_slc(chip)) {
+ chip->ecc_step_ds = 512;
+ switch (chip->id.data[5] & 0x7) {
+ case 0x4:
+ chip->ecc_strength_ds = 1;
+ break;
+ case 0x5:
+ chip->ecc_strength_ds = 4;
+ break;
+ case 0x6:
+ chip->ecc_strength_ds = 8;
+ break;
+ default:
+ WARN(1, "Could not get ECC info");
+ chip->ecc_step_ds = 0;
+ break;
+ }
+ } else if (chip->id.len < 6 && nand_is_slc(chip)) {
+ WARN(1, "Could not get ECC info, 6th nand id byte does not exist.");
+ }
}
static int toshiba_nand_init(struct nand_chip *chip)
--
2.7.4
On Wed, 6 Dec 2017 23:04:57 +0900
KOBAYASHI Yoshitake <[email protected]> wrote:
> This patch enables support to read the ECC strength and size from the
> NAND flash using Toshiba Memory SLC NAND extended-ID. This patch is
> based on the information of the 6th ID byte of the Toshiba Memory SLC
> NAND.
>
> Signed-off-by: KOBAYASHI Yoshitake <[email protected]>
> ---
> drivers/mtd/nand/nand_toshiba.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/mtd/nand/nand_toshiba.c b/drivers/mtd/nand/nand_toshiba.c
> index 57df857..c2c141b 100644
> --- a/drivers/mtd/nand/nand_toshiba.c
> +++ b/drivers/mtd/nand/nand_toshiba.c
> @@ -35,6 +35,34 @@ static void toshiba_nand_decode_id(struct nand_chip *chip)
> (chip->id.data[5] & 0x7) == 0x6 /* 24nm */ &&
> !(chip->id.data[4] & 0x80) /* !BENAND */)
> mtd->oobsize = 32 * mtd->writesize >> 9;
> +
> + /*
> + * Extract ECC requirements from 6th id byte.
> + * For Toshiba SLC, ecc requrements are as follows:
> + * - 43nm: 1 bit ECC for each 512Byte is required.
> + * - 32nm: 4 bit ECC for each 512Byte is required.
> + * - 24nm: 8 bit ECC for each 512Byte is required.
> + */
> + if (chip->id.len >= 6 && nand_is_slc(chip)) {
> + chip->ecc_step_ds = 512;
> + switch (chip->id.data[5] & 0x7) {
> + case 0x4:
> + chip->ecc_strength_ds = 1;
> + break;
> + case 0x5:
> + chip->ecc_strength_ds = 4;
> + break;
> + case 0x6:
> + chip->ecc_strength_ds = 8;
> + break;
> + default:
> + WARN(1, "Could not get ECC info");
> + chip->ecc_step_ds = 0;
> + break;
> + }
> + } else if (chip->id.len < 6 && nand_is_slc(chip)) {
> + WARN(1, "Could not get ECC info, 6th nand id byte does not exist.");
I'm pretty sure you have old NAND chips that do not have 6bytes ids
(see the table here [1]), and printing a huge backtrace in this case is
probably not what you want.
If you're okay with dropping this else block, I'll do the change when
applying, no need to send a new version.
> + }
> }
>
> static int toshiba_nand_init(struct nand_chip *chip)
On Wed, 6 Dec 2017 23:04:58 +0900
KOBAYASHI Yoshitake <[email protected]> wrote:
> This patch enables support for Toshiba Memory BENAND. This checks
> internal ECC status in read operation when using BENAND and selecting
> ECC mode as on-die.
>
> Signed-off-by: KOBAYASHI Yoshitake <[email protected]>
> ---
> drivers/mtd/nand/nand_toshiba.c | 89 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 89 insertions(+)
>
> diff --git a/drivers/mtd/nand/nand_toshiba.c b/drivers/mtd/nand/nand_toshiba.c
> index c2c141b..35c0ddf 100644
> --- a/drivers/mtd/nand/nand_toshiba.c
> +++ b/drivers/mtd/nand/nand_toshiba.c
> @@ -17,6 +17,91 @@
>
> #include <linux/mtd/rawnand.h>
>
> +/* ECC Status Read Command for BENAND */
> +#define TOSHIBA_NAND_CMD_ECC_STATUS 0x7A
> +
> +/* Recommended to rewrite for BENAND */
> +#define TOSHIBA_NAND_STATUS_REWRITE_RECOMMENDED BIT(3)
> +
> +static int toshiba_nand_benand_status_chk(struct mtd_info *mtd,
> + struct nand_chip *chip)
> +{
> + unsigned int max_bitflips = 0;
> + u8 status;
> +
> + /* Check Read Status */
> + chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
> + status = chip->read_byte(mtd);
Please use the recently introduced nand_status_op() helper.
> +
> + /*
> + * TOSHIBA_NAND_CMD_ECC_STATUS is vendor specific command.
> + * Currently we have no way to send arbitrary sequences of
> + * CMD+ADDR+DATA cycles and thus cannot support this custom
> + * TOSHIBA_NAND_CMD_ECC_STATUS operation.
That's about to change :-), if everything goes well, nand_exec_op()
should be available in 4.16.
> + * For now, we set max_bitflips mtd->bitflip_threshold.
> + */
> + if (status & NAND_STATUS_FAIL) {
> + /* uncorrectable */
> + mtd->ecc_stats.failed++;
> + } else if (status & TOSHIBA_NAND_STATUS_REWRITE_RECOMMENDED) {
> + /* correctable */
> + max_bitflips = mtd->bitflip_threshold;
> + mtd->ecc_stats.corrected += max_bitflips;
> + }
> +
> + return max_bitflips;
> +}
> +
> +static int
> +toshiba_nand_read_page_benand(struct mtd_info *mtd,
> + struct nand_chip *chip, uint8_t *buf,
> + int oob_required, int page)
> +{
> + nand_read_page_raw(mtd, chip, buf, oob_required, page);
Please check the return code of nand_read_page_raw().
> +
> + return toshiba_nand_benand_status_chk(mtd, chip);
> +}
> +
> +static int
> +toshiba_nand_read_subpage_benand(struct mtd_info *mtd,
> + struct nand_chip *chip, uint32_t data_offs,
> + uint32_t readlen, uint8_t *bufpoi, int page)
> +{
> + uint8_t *p;
> +
> + if (data_offs != 0)
> + chip->cmdfunc(mtd, NAND_CMD_RNDOUT, data_offs, -1);
> +
> + p = bufpoi + data_offs;
> + chip->read_buf(mtd, p, readlen);
The core is no longer sending the initial READ0 command, so this should
be turned into:
ret = nand_read_page_op(chip, data_offs, bufpoi + data_offs,
readlen);
if (ret)
return ret;
> +
> + return toshiba_nand_benand_status_chk(mtd, chip);
> +}
> +
> +static void toshiba_nand_benand_init(struct nand_chip *chip)
> +{
> + struct mtd_info *mtd = nand_to_mtd(chip);
> +
> + /*
> + * On BENAND, the entire OOB region can be used by the MTD user.
> + * The calculated ECC bytes are stored into other isolated
> + * area which is not accessible to users.
> + * This is why chip->ecc.bytes = 0.
> + */
> + chip->ecc.bytes = 0;
> + chip->ecc.size = 512;
> + chip->ecc.strength = 8;
> + chip->ecc.read_page = toshiba_nand_read_page_benand;
> + chip->ecc.read_subpage = toshiba_nand_read_subpage_benand;
> + chip->ecc.write_page = nand_write_page_raw;
> + chip->ecc.read_page_raw = nand_read_page_raw;
> + chip->ecc.write_page_raw = nand_write_page_raw;
> +
> + chip->options |= NAND_SUBPAGE_READ;
> +
> + mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
> +}
> +
> static void toshiba_nand_decode_id(struct nand_chip *chip)
> {
> struct mtd_info *mtd = nand_to_mtd(chip);
> @@ -70,6 +155,10 @@ static int toshiba_nand_init(struct nand_chip *chip)
> if (nand_is_slc(chip))
> chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
>
> + if (nand_is_slc(chip) && (chip->id.data[4] & 0x80) /* BENAND */ &&
> + (chip->ecc.mode == NAND_ECC_ON_DIE))
> + toshiba_nand_benand_init(chip);
> +
Please move this block to toshiba_nand_init().
> return 0;
> }
>
On 2017/12/07 0:08, Boris Brezillon wrote:
> On Wed, 6 Dec 2017 23:04:57 +0900
> KOBAYASHI Yoshitake <[email protected]> wrote:
>
>> This patch enables support to read the ECC strength and size from the
>> NAND flash using Toshiba Memory SLC NAND extended-ID. This patch is
>> based on the information of the 6th ID byte of the Toshiba Memory SLC
>> NAND.
>>
>> Signed-off-by: KOBAYASHI Yoshitake <[email protected]>
>> ---
>> drivers/mtd/nand/nand_toshiba.c | 28 ++++++++++++++++++++++++++++
>> 1 file changed, 28 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/nand_toshiba.c b/drivers/mtd/nand/nand_toshiba.c
>> index 57df857..c2c141b 100644
>> --- a/drivers/mtd/nand/nand_toshiba.c
>> +++ b/drivers/mtd/nand/nand_toshiba.c
>> @@ -35,6 +35,34 @@ static void toshiba_nand_decode_id(struct nand_chip *chip)
>> (chip->id.data[5] & 0x7) == 0x6 /* 24nm */ &&
>> !(chip->id.data[4] & 0x80) /* !BENAND */)
>> mtd->oobsize = 32 * mtd->writesize >> 9;
>> +
>> + /*
>> + * Extract ECC requirements from 6th id byte.
>> + * For Toshiba SLC, ecc requrements are as follows:
>> + * - 43nm: 1 bit ECC for each 512Byte is required.
>> + * - 32nm: 4 bit ECC for each 512Byte is required.
>> + * - 24nm: 8 bit ECC for each 512Byte is required.
>> + */
>> + if (chip->id.len >= 6 && nand_is_slc(chip)) {
>> + chip->ecc_step_ds = 512;
>> + switch (chip->id.data[5] & 0x7) {
>> + case 0x4:
>> + chip->ecc_strength_ds = 1;
>> + break;
>> + case 0x5:
>> + chip->ecc_strength_ds = 4;
>> + break;
>> + case 0x6:
>> + chip->ecc_strength_ds = 8;
>> + break;
>> + default:
>> + WARN(1, "Could not get ECC info");
>> + chip->ecc_step_ds = 0;
>> + break;
>> + }
>> + } else if (chip->id.len < 6 && nand_is_slc(chip)) {
>> + WARN(1, "Could not get ECC info, 6th nand id byte does not exist.");
>
> I'm pretty sure you have old NAND chips that do not have 6bytes ids
> (see the table here [1]), and printing a huge backtrace in this case is
> probably not what you want.
>
> If you're okay with dropping this else block, I'll do the change when
> applying, no need to send a new version.
Some controllers may have limitation in reading ids beyond 5 bytes,
considering such scenario we think it is better to keep this warning.
However if you feel huge backtrace is an issue, how about we using pr_warn() instead?
On Tue, 19 Dec 2017 20:42:36 +0900
KOBAYASHI Yoshitake <[email protected]> wrote:
> On 2017/12/07 0:08, Boris Brezillon wrote:
> > On Wed, 6 Dec 2017 23:04:57 +0900
> > KOBAYASHI Yoshitake <[email protected]> wrote:
> >
> >> This patch enables support to read the ECC strength and size from the
> >> NAND flash using Toshiba Memory SLC NAND extended-ID. This patch is
> >> based on the information of the 6th ID byte of the Toshiba Memory SLC
> >> NAND.
> >>
> >> Signed-off-by: KOBAYASHI Yoshitake <[email protected]>
> >> ---
> >> drivers/mtd/nand/nand_toshiba.c | 28 ++++++++++++++++++++++++++++
> >> 1 file changed, 28 insertions(+)
> >>
> >> diff --git a/drivers/mtd/nand/nand_toshiba.c b/drivers/mtd/nand/nand_toshiba.c
> >> index 57df857..c2c141b 100644
> >> --- a/drivers/mtd/nand/nand_toshiba.c
> >> +++ b/drivers/mtd/nand/nand_toshiba.c
> >> @@ -35,6 +35,34 @@ static void toshiba_nand_decode_id(struct nand_chip *chip)
> >> (chip->id.data[5] & 0x7) == 0x6 /* 24nm */ &&
> >> !(chip->id.data[4] & 0x80) /* !BENAND */)
> >> mtd->oobsize = 32 * mtd->writesize >> 9;
> >> +
> >> + /*
> >> + * Extract ECC requirements from 6th id byte.
> >> + * For Toshiba SLC, ecc requrements are as follows:
> >> + * - 43nm: 1 bit ECC for each 512Byte is required.
> >> + * - 32nm: 4 bit ECC for each 512Byte is required.
> >> + * - 24nm: 8 bit ECC for each 512Byte is required.
> >> + */
> >> + if (chip->id.len >= 6 && nand_is_slc(chip)) {
> >> + chip->ecc_step_ds = 512;
> >> + switch (chip->id.data[5] & 0x7) {
> >> + case 0x4:
> >> + chip->ecc_strength_ds = 1;
> >> + break;
> >> + case 0x5:
> >> + chip->ecc_strength_ds = 4;
> >> + break;
> >> + case 0x6:
> >> + chip->ecc_strength_ds = 8;
> >> + break;
> >> + default:
> >> + WARN(1, "Could not get ECC info");
> >> + chip->ecc_step_ds = 0;
> >> + break;
> >> + }
> >> + } else if (chip->id.len < 6 && nand_is_slc(chip)) {
> >> + WARN(1, "Could not get ECC info, 6th nand id byte does not exist.");
> >
> > I'm pretty sure you have old NAND chips that do not have 6bytes ids
> > (see the table here [1]), and printing a huge backtrace in this case is
> > probably not what you want.
> >
> > If you're okay with dropping this else block, I'll do the change when
> > applying, no need to send a new version.
>
> Some controllers may have limitation in reading ids beyond 5 bytes,
> considering such scenario we think it is better to keep this warning.
> However if you feel huge backtrace is an issue, how about we using pr_warn() instead?
>
Toshiba NANDs with an id smaller than 6 bytes exist, so no, we should
not complain at all. If the controller is broken and can't read the 8 id
bytes the core is asking for, then it should be detected at the core
level not in the NAND manufacturer driver.
Thanks for reviewing the patches and taht is a good news for me about nand_exec_op().
I'll change this patch as you suggested.
I would like to make sure the following one.
>> @@ -70,6 +155,10 @@ static int toshiba_nand_init(struct nand_chip *chip)
>> if (nand_is_slc(chip))
>> chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
>>
>> + if (nand_is_slc(chip) && (chip->id.data[4] & 0x80) /* BENAND */ &&
>> + (chip->ecc.mode == NAND_ECC_ON_DIE))
>> + toshiba_nand_benand_init(chip);
>> +
>
> Please move this block to toshiba_nand_init().
I think it's already in toshiba_nand_init().
-- YOSHI
On 2017/12/07 0:24, Boris Brezillon wrote:
> On Wed, 6 Dec 2017 23:04:58 +0900
> KOBAYASHI Yoshitake <[email protected]> wrote:
>
>> This patch enables support for Toshiba Memory BENAND. This checks
>> internal ECC status in read operation when using BENAND and selecting
>> ECC mode as on-die.
>>
>> Signed-off-by: KOBAYASHI Yoshitake <[email protected]>
>> ---
>> drivers/mtd/nand/nand_toshiba.c | 89 +++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 89 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/nand_toshiba.c b/drivers/mtd/nand/nand_toshiba.c
>> index c2c141b..35c0ddf 100644
>> --- a/drivers/mtd/nand/nand_toshiba.c
>> +++ b/drivers/mtd/nand/nand_toshiba.c
>> @@ -17,6 +17,91 @@
>>
>> #include <linux/mtd/rawnand.h>
>>
>> +/* ECC Status Read Command for BENAND */
>> +#define TOSHIBA_NAND_CMD_ECC_STATUS 0x7A
>> +
>> +/* Recommended to rewrite for BENAND */
>> +#define TOSHIBA_NAND_STATUS_REWRITE_RECOMMENDED BIT(3)
>> +
>> +static int toshiba_nand_benand_status_chk(struct mtd_info *mtd,
>> + struct nand_chip *chip)
>> +{
>> + unsigned int max_bitflips = 0;
>> + u8 status;
>> +
>> + /* Check Read Status */
>> + chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
>> + status = chip->read_byte(mtd);
>
> Please use the recently introduced nand_status_op() helper.
>
>> +
>> + /*
>> + * TOSHIBA_NAND_CMD_ECC_STATUS is vendor specific command.
>> + * Currently we have no way to send arbitrary sequences of
>> + * CMD+ADDR+DATA cycles and thus cannot support this custom
>> + * TOSHIBA_NAND_CMD_ECC_STATUS operation.
>
> That's about to change :-), if everything goes well, nand_exec_op()
> should be available in 4.16.
>
>> + * For now, we set max_bitflips mtd->bitflip_threshold.
>> + */
>> + if (status & NAND_STATUS_FAIL) {
>> + /* uncorrectable */
>> + mtd->ecc_stats.failed++;
>> + } else if (status & TOSHIBA_NAND_STATUS_REWRITE_RECOMMENDED) {
>> + /* correctable */
>> + max_bitflips = mtd->bitflip_threshold;
>> + mtd->ecc_stats.corrected += max_bitflips;
>> + }
>> +
>> + return max_bitflips;
>> +}
>> +
>> +static int
>> +toshiba_nand_read_page_benand(struct mtd_info *mtd,
>> + struct nand_chip *chip, uint8_t *buf,
>> + int oob_required, int page)
>> +{
>> + nand_read_page_raw(mtd, chip, buf, oob_required, page);
>
> Please check the return code of nand_read_page_raw().
>
>> +
>> + return toshiba_nand_benand_status_chk(mtd, chip);
>> +}
>> +
>> +static int
>> +toshiba_nand_read_subpage_benand(struct mtd_info *mtd,
>> + struct nand_chip *chip, uint32_t data_offs,
>> + uint32_t readlen, uint8_t *bufpoi, int page)
>> +{
>> + uint8_t *p;
>> +
>> + if (data_offs != 0)
>> + chip->cmdfunc(mtd, NAND_CMD_RNDOUT, data_offs, -1);
>> +
>> + p = bufpoi + data_offs;
>> + chip->read_buf(mtd, p, readlen);
>
> The core is no longer sending the initial READ0 command, so this should
> be turned into:
>
> ret = nand_read_page_op(chip, data_offs, bufpoi + data_offs,
> readlen);
> if (ret)
> return ret;
>
>
>> +
>> + return toshiba_nand_benand_status_chk(mtd, chip);
>> +}
>> +
>> +static void toshiba_nand_benand_init(struct nand_chip *chip)
>> +{
>> + struct mtd_info *mtd = nand_to_mtd(chip);
>> +
>> + /*
>> + * On BENAND, the entire OOB region can be used by the MTD user.
>> + * The calculated ECC bytes are stored into other isolated
>> + * area which is not accessible to users.
>> + * This is why chip->ecc.bytes = 0.
>> + */
>> + chip->ecc.bytes = 0;
>> + chip->ecc.size = 512;
>> + chip->ecc.strength = 8;
>> + chip->ecc.read_page = toshiba_nand_read_page_benand;
>> + chip->ecc.read_subpage = toshiba_nand_read_subpage_benand;
>> + chip->ecc.write_page = nand_write_page_raw;
>> + chip->ecc.read_page_raw = nand_read_page_raw;
>> + chip->ecc.write_page_raw = nand_write_page_raw;
>> +
>> + chip->options |= NAND_SUBPAGE_READ;
>> +
>> + mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
>> +}
>> +
>> static void toshiba_nand_decode_id(struct nand_chip *chip)
>> {
>> struct mtd_info *mtd = nand_to_mtd(chip);
>> @@ -70,6 +155,10 @@ static int toshiba_nand_init(struct nand_chip *chip)
>> if (nand_is_slc(chip))
>> chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
>>
>> + if (nand_is_slc(chip) && (chip->id.data[4] & 0x80) /* BENAND */ &&
>> + (chip->ecc.mode == NAND_ECC_ON_DIE))
>> + toshiba_nand_benand_init(chip);
>> +
>
> Please move this block to toshiba_nand_init().
>
>> return 0;
>> }
>>
>
>
>
On Tue, 19 Dec 2017 21:01:47 +0900
KOBAYASHI Yoshitake <[email protected]> wrote:
> Thanks for reviewing the patches and taht is a good news for me about nand_exec_op().
> I'll change this patch as you suggested.
> I would like to make sure the following one.
>
> >> @@ -70,6 +155,10 @@ static int toshiba_nand_init(struct nand_chip *chip)
> >> if (nand_is_slc(chip))
> >> chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
> >>
> >> + if (nand_is_slc(chip) && (chip->id.data[4] & 0x80) /* BENAND */ &&
> >> + (chip->ecc.mode == NAND_ECC_ON_DIE))
> >> + toshiba_nand_benand_init(chip);
> >> +
> >
> > Please move this block to toshiba_nand_init().
>
> I think it's already in toshiba_nand_init().
My bad, I thought it was in the decode_id() function.
>
> -- YOSHI
>
>
> On 2017/12/07 0:24, Boris Brezillon wrote:
> > On Wed, 6 Dec 2017 23:04:58 +0900
> > KOBAYASHI Yoshitake <[email protected]> wrote:
> >
> >> This patch enables support for Toshiba Memory BENAND. This checks
> >> internal ECC status in read operation when using BENAND and selecting
> >> ECC mode as on-die.
> >>
> >> Signed-off-by: KOBAYASHI Yoshitake <[email protected]>
> >> ---
> >> drivers/mtd/nand/nand_toshiba.c | 89 +++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 89 insertions(+)
> >>
> >> diff --git a/drivers/mtd/nand/nand_toshiba.c b/drivers/mtd/nand/nand_toshiba.c
> >> index c2c141b..35c0ddf 100644
> >> --- a/drivers/mtd/nand/nand_toshiba.c
> >> +++ b/drivers/mtd/nand/nand_toshiba.c
> >> @@ -17,6 +17,91 @@
> >>
> >> #include <linux/mtd/rawnand.h>
> >>
> >> +/* ECC Status Read Command for BENAND */
> >> +#define TOSHIBA_NAND_CMD_ECC_STATUS 0x7A
> >> +
> >> +/* Recommended to rewrite for BENAND */
> >> +#define TOSHIBA_NAND_STATUS_REWRITE_RECOMMENDED BIT(3)
> >> +
> >> +static int toshiba_nand_benand_status_chk(struct mtd_info *mtd,
> >> + struct nand_chip *chip)
> >> +{
> >> + unsigned int max_bitflips = 0;
> >> + u8 status;
> >> +
> >> + /* Check Read Status */
> >> + chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
> >> + status = chip->read_byte(mtd);
> >
> > Please use the recently introduced nand_status_op() helper.
> >
> >> +
> >> + /*
> >> + * TOSHIBA_NAND_CMD_ECC_STATUS is vendor specific command.
> >> + * Currently we have no way to send arbitrary sequences of
> >> + * CMD+ADDR+DATA cycles and thus cannot support this custom
> >> + * TOSHIBA_NAND_CMD_ECC_STATUS operation.
> >
> > That's about to change :-), if everything goes well, nand_exec_op()
> > should be available in 4.16.
> >
> >> + * For now, we set max_bitflips mtd->bitflip_threshold.
> >> + */
> >> + if (status & NAND_STATUS_FAIL) {
> >> + /* uncorrectable */
> >> + mtd->ecc_stats.failed++;
> >> + } else if (status & TOSHIBA_NAND_STATUS_REWRITE_RECOMMENDED) {
> >> + /* correctable */
> >> + max_bitflips = mtd->bitflip_threshold;
> >> + mtd->ecc_stats.corrected += max_bitflips;
> >> + }
> >> +
> >> + return max_bitflips;
> >> +}
> >> +
> >> +static int
> >> +toshiba_nand_read_page_benand(struct mtd_info *mtd,
> >> + struct nand_chip *chip, uint8_t *buf,
> >> + int oob_required, int page)
> >> +{
> >> + nand_read_page_raw(mtd, chip, buf, oob_required, page);
> >
> > Please check the return code of nand_read_page_raw().
> >
> >> +
> >> + return toshiba_nand_benand_status_chk(mtd, chip);
> >> +}
> >> +
> >> +static int
> >> +toshiba_nand_read_subpage_benand(struct mtd_info *mtd,
> >> + struct nand_chip *chip, uint32_t data_offs,
> >> + uint32_t readlen, uint8_t *bufpoi, int page)
> >> +{
> >> + uint8_t *p;
> >> +
> >> + if (data_offs != 0)
> >> + chip->cmdfunc(mtd, NAND_CMD_RNDOUT, data_offs, -1);
> >> +
> >> + p = bufpoi + data_offs;
> >> + chip->read_buf(mtd, p, readlen);
> >
> > The core is no longer sending the initial READ0 command, so this should
> > be turned into:
> >
> > ret = nand_read_page_op(chip, data_offs, bufpoi + data_offs,
> > readlen);
> > if (ret)
> > return ret;
> >
> >
> >> +
> >> + return toshiba_nand_benand_status_chk(mtd, chip);
> >> +}
> >> +
> >> +static void toshiba_nand_benand_init(struct nand_chip *chip)
> >> +{
> >> + struct mtd_info *mtd = nand_to_mtd(chip);
> >> +
> >> + /*
> >> + * On BENAND, the entire OOB region can be used by the MTD user.
> >> + * The calculated ECC bytes are stored into other isolated
> >> + * area which is not accessible to users.
> >> + * This is why chip->ecc.bytes = 0.
> >> + */
> >> + chip->ecc.bytes = 0;
> >> + chip->ecc.size = 512;
> >> + chip->ecc.strength = 8;
> >> + chip->ecc.read_page = toshiba_nand_read_page_benand;
> >> + chip->ecc.read_subpage = toshiba_nand_read_subpage_benand;
> >> + chip->ecc.write_page = nand_write_page_raw;
> >> + chip->ecc.read_page_raw = nand_read_page_raw;
> >> + chip->ecc.write_page_raw = nand_write_page_raw;
> >> +
> >> + chip->options |= NAND_SUBPAGE_READ;
> >> +
> >> + mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
> >> +}
> >> +
> >> static void toshiba_nand_decode_id(struct nand_chip *chip)
> >> {
> >> struct mtd_info *mtd = nand_to_mtd(chip);
> >> @@ -70,6 +155,10 @@ static int toshiba_nand_init(struct nand_chip *chip)
> >> if (nand_is_slc(chip))
> >> chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
> >>
> >> + if (nand_is_slc(chip) && (chip->id.data[4] & 0x80) /* BENAND */ &&
> >> + (chip->ecc.mode == NAND_ECC_ON_DIE))
> >> + toshiba_nand_benand_init(chip);
> >> +
> >
> > Please move this block to toshiba_nand_init().
> >
> >> return 0;
> >> }
> >>
> >
> >
> >
>
On Tue, 19 Dec 2017 12:56:24 +0100
Boris Brezillon <[email protected]> wrote:
> On Tue, 19 Dec 2017 20:42:36 +0900
> KOBAYASHI Yoshitake <[email protected]> wrote:
>
> > On 2017/12/07 0:08, Boris Brezillon wrote:
> > > On Wed, 6 Dec 2017 23:04:57 +0900
> > > KOBAYASHI Yoshitake <[email protected]> wrote:
> > >
> > >> This patch enables support to read the ECC strength and size from the
> > >> NAND flash using Toshiba Memory SLC NAND extended-ID. This patch is
> > >> based on the information of the 6th ID byte of the Toshiba Memory SLC
> > >> NAND.
> > >>
> > >> Signed-off-by: KOBAYASHI Yoshitake <[email protected]>
> > >> ---
> > >> drivers/mtd/nand/nand_toshiba.c | 28 ++++++++++++++++++++++++++++
> > >> 1 file changed, 28 insertions(+)
> > >>
> > >> diff --git a/drivers/mtd/nand/nand_toshiba.c b/drivers/mtd/nand/nand_toshiba.c
> > >> index 57df857..c2c141b 100644
> > >> --- a/drivers/mtd/nand/nand_toshiba.c
> > >> +++ b/drivers/mtd/nand/nand_toshiba.c
> > >> @@ -35,6 +35,34 @@ static void toshiba_nand_decode_id(struct nand_chip *chip)
> > >> (chip->id.data[5] & 0x7) == 0x6 /* 24nm */ &&
> > >> !(chip->id.data[4] & 0x80) /* !BENAND */)
> > >> mtd->oobsize = 32 * mtd->writesize >> 9;
> > >> +
> > >> + /*
> > >> + * Extract ECC requirements from 6th id byte.
> > >> + * For Toshiba SLC, ecc requrements are as follows:
> > >> + * - 43nm: 1 bit ECC for each 512Byte is required.
> > >> + * - 32nm: 4 bit ECC for each 512Byte is required.
> > >> + * - 24nm: 8 bit ECC for each 512Byte is required.
> > >> + */
> > >> + if (chip->id.len >= 6 && nand_is_slc(chip)) {
> > >> + chip->ecc_step_ds = 512;
> > >> + switch (chip->id.data[5] & 0x7) {
> > >> + case 0x4:
> > >> + chip->ecc_strength_ds = 1;
> > >> + break;
> > >> + case 0x5:
> > >> + chip->ecc_strength_ds = 4;
> > >> + break;
> > >> + case 0x6:
> > >> + chip->ecc_strength_ds = 8;
> > >> + break;
> > >> + default:
> > >> + WARN(1, "Could not get ECC info");
> > >> + chip->ecc_step_ds = 0;
> > >> + break;
> > >> + }
> > >> + } else if (chip->id.len < 6 && nand_is_slc(chip)) {
> > >> + WARN(1, "Could not get ECC info, 6th nand id byte does not exist.");
> > >
> > > I'm pretty sure you have old NAND chips that do not have 6bytes ids
> > > (see the table here [1]), and printing a huge backtrace in this case is
> > > probably not what you want.
> > >
> > > If you're okay with dropping this else block, I'll do the change when
> > > applying, no need to send a new version.
> >
> > Some controllers may have limitation in reading ids beyond 5 bytes,
> > considering such scenario we think it is better to keep this warning.
> > However if you feel huge backtrace is an issue, how about we using pr_warn() instead?
> >
>
> Toshiba NANDs with an id smaller than 6 bytes exist, so no, we should
> not complain at all. If the controller is broken and can't read the 8 id
> bytes the core is asking for, then it should be detected at the core
> level not in the NAND manufacturer driver.
It seems I forgot the link to the NAND table, so here it is [1], and as
you can see, some chips have only 2 id bytes (TC58DVG02A5 is one of
them).
[1]http://www.linux-mtd.infradead.org/nand-data/nanddata.html
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
On 2017/12/19 20:56, Boris Brezillon wrote:
> On Tue, 19 Dec 2017 20:42:36 +0900
> KOBAYASHI Yoshitake <[email protected]> wrote:
>
>> On 2017/12/07 0:08, Boris Brezillon wrote:
>>> On Wed, 6 Dec 2017 23:04:57 +0900
>>> KOBAYASHI Yoshitake <[email protected]> wrote:
>>>
>>>> This patch enables support to read the ECC strength and size from the
>>>> NAND flash using Toshiba Memory SLC NAND extended-ID. This patch is
>>>> based on the information of the 6th ID byte of the Toshiba Memory SLC
>>>> NAND.
>>>>
>>>> Signed-off-by: KOBAYASHI Yoshitake <[email protected]>
>>>> ---
>>>> drivers/mtd/nand/nand_toshiba.c | 28 ++++++++++++++++++++++++++++
>>>> 1 file changed, 28 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/nand/nand_toshiba.c b/drivers/mtd/nand/nand_toshiba.c
>>>> index 57df857..c2c141b 100644
>>>> --- a/drivers/mtd/nand/nand_toshiba.c
>>>> +++ b/drivers/mtd/nand/nand_toshiba.c
>>>> @@ -35,6 +35,34 @@ static void toshiba_nand_decode_id(struct nand_chip *chip)
>>>> (chip->id.data[5] & 0x7) == 0x6 /* 24nm */ &&
>>>> !(chip->id.data[4] & 0x80) /* !BENAND */)
>>>> mtd->oobsize = 32 * mtd->writesize >> 9;
>>>> +
>>>> + /*
>>>> + * Extract ECC requirements from 6th id byte.
>>>> + * For Toshiba SLC, ecc requrements are as follows:
>>>> + * - 43nm: 1 bit ECC for each 512Byte is required.
>>>> + * - 32nm: 4 bit ECC for each 512Byte is required.
>>>> + * - 24nm: 8 bit ECC for each 512Byte is required.
>>>> + */
>>>> + if (chip->id.len >= 6 && nand_is_slc(chip)) {
>>>> + chip->ecc_step_ds = 512;
>>>> + switch (chip->id.data[5] & 0x7) {
>>>> + case 0x4:
>>>> + chip->ecc_strength_ds = 1;
>>>> + break;
>>>> + case 0x5:
>>>> + chip->ecc_strength_ds = 4;
>>>> + break;
>>>> + case 0x6:
>>>> + chip->ecc_strength_ds = 8;
>>>> + break;
>>>> + default:
>>>> + WARN(1, "Could not get ECC info");
>>>> + chip->ecc_step_ds = 0;
>>>> + break;
>>>> + }
>>>> + } else if (chip->id.len < 6 && nand_is_slc(chip)) {
>>>> + WARN(1, "Could not get ECC info, 6th nand id byte does not exist.");
>>>
>>> I'm pretty sure you have old NAND chips that do not have 6bytes ids
>>> (see the table here [1]), and printing a huge backtrace in this case is
>>> probably not what you want.
>>>
>>> If you're okay with dropping this else block, I'll do the change when
>>> applying, no need to send a new version.
>>
>> Some controllers may have limitation in reading ids beyond 5 bytes,
>> considering such scenario we think it is better to keep this warning.
>> However if you feel huge backtrace is an issue, how about we using pr_warn() instead?
>>
>
> Toshiba NANDs with an id smaller than 6 bytes exist, so no, we should
> not complain at all. If the controller is broken and can't read the 8 id
> bytes the core is asking for, then it should be detected at the core
> level not in the NAND manufacturer driver.
I understood your opinion. Please apply this patch with dropping the else block.
-- YOSHI
On 2017/12/27 15:06, KOBAYASHI Yoshitake wrote:
> On 2017/12/19 20:56, Boris Brezillon wrote:
>> On Tue, 19 Dec 2017 20:42:36 +0900
>> KOBAYASHI Yoshitake <[email protected]> wrote:
>>
>>> On 2017/12/07 0:08, Boris Brezillon wrote:
>>>> On Wed, 6 Dec 2017 23:04:57 +0900
>>>> KOBAYASHI Yoshitake <[email protected]> wrote:
>>>>
>>>>> This patch enables support to read the ECC strength and size from the
>>>>> NAND flash using Toshiba Memory SLC NAND extended-ID. This patch is
>>>>> based on the information of the 6th ID byte of the Toshiba Memory SLC
>>>>> NAND.
>>>>>
>>>>> Signed-off-by: KOBAYASHI Yoshitake <[email protected]>
>>>>> ---
>>>>> drivers/mtd/nand/nand_toshiba.c | 28 ++++++++++++++++++++++++++++
>>>>> 1 file changed, 28 insertions(+)
>>>>>
>>>>> diff --git a/drivers/mtd/nand/nand_toshiba.c b/drivers/mtd/nand/nand_toshiba.c
>>>>> index 57df857..c2c141b 100644
>>>>> --- a/drivers/mtd/nand/nand_toshiba.c
>>>>> +++ b/drivers/mtd/nand/nand_toshiba.c
>>>>> @@ -35,6 +35,34 @@ static void toshiba_nand_decode_id(struct nand_chip *chip)
>>>>> (chip->id.data[5] & 0x7) == 0x6 /* 24nm */ &&
>>>>> !(chip->id.data[4] & 0x80) /* !BENAND */)
>>>>> mtd->oobsize = 32 * mtd->writesize >> 9;
>>>>> +
>>>>> + /*
>>>>> + * Extract ECC requirements from 6th id byte.
>>>>> + * For Toshiba SLC, ecc requrements are as follows:
>>>>> + * - 43nm: 1 bit ECC for each 512Byte is required.
>>>>> + * - 32nm: 4 bit ECC for each 512Byte is required.
>>>>> + * - 24nm: 8 bit ECC for each 512Byte is required.
>>>>> + */
>>>>> + if (chip->id.len >= 6 && nand_is_slc(chip)) {
>>>>> + chip->ecc_step_ds = 512;
>>>>> + switch (chip->id.data[5] & 0x7) {
>>>>> + case 0x4:
>>>>> + chip->ecc_strength_ds = 1;
>>>>> + break;
>>>>> + case 0x5:
>>>>> + chip->ecc_strength_ds = 4;
>>>>> + break;
>>>>> + case 0x6:
>>>>> + chip->ecc_strength_ds = 8;
>>>>> + break;
>>>>> + default:
>>>>> + WARN(1, "Could not get ECC info");
>>>>> + chip->ecc_step_ds = 0;
>>>>> + break;
>>>>> + }
>>>>> + } else if (chip->id.len < 6 && nand_is_slc(chip)) {
>>>>> + WARN(1, "Could not get ECC info, 6th nand id byte does not exist.");
>>>>
>>>> I'm pretty sure you have old NAND chips that do not have 6bytes ids
>>>> (see the table here [1]), and printing a huge backtrace in this case is
>>>> probably not what you want.
>>>>
>>>> If you're okay with dropping this else block, I'll do the change when
>>>> applying, no need to send a new version.
>>>
>>> Some controllers may have limitation in reading ids beyond 5 bytes,
>>> considering such scenario we think it is better to keep this warning.
>>> However if you feel huge backtrace is an issue, how about we using pr_warn() instead?
>>>
>>
>> Toshiba NANDs with an id smaller than 6 bytes exist, so no, we should
>> not complain at all. If the controller is broken and can't read the 8 id
>> bytes the core is asking for, then it should be detected at the core
>> level not in the NAND manufacturer driver.
>
> I understood your opinion. Please apply this patch with dropping the else block.
Should I repost patch with else block dropped? Please let me know if that is necessary.
-- Yoshi
On Tue, 30 Jan 2018 08:44:30 +0900
KOBAYASHI Yoshitake <[email protected]> wrote:
> On 2017/12/27 15:06, KOBAYASHI Yoshitake wrote:
> > On 2017/12/19 20:56, Boris Brezillon wrote:
> >> On Tue, 19 Dec 2017 20:42:36 +0900
> >> KOBAYASHI Yoshitake <[email protected]> wrote:
> >>
> >>> On 2017/12/07 0:08, Boris Brezillon wrote:
> >>>> On Wed, 6 Dec 2017 23:04:57 +0900
> >>>> KOBAYASHI Yoshitake <[email protected]> wrote:
> >>>>
> >>>>> This patch enables support to read the ECC strength and size from the
> >>>>> NAND flash using Toshiba Memory SLC NAND extended-ID. This patch is
> >>>>> based on the information of the 6th ID byte of the Toshiba Memory SLC
> >>>>> NAND.
> >>>>>
> >>>>> Signed-off-by: KOBAYASHI Yoshitake <[email protected]>
> >>>>> ---
> >>>>> drivers/mtd/nand/nand_toshiba.c | 28 ++++++++++++++++++++++++++++
> >>>>> 1 file changed, 28 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/mtd/nand/nand_toshiba.c b/drivers/mtd/nand/nand_toshiba.c
> >>>>> index 57df857..c2c141b 100644
> >>>>> --- a/drivers/mtd/nand/nand_toshiba.c
> >>>>> +++ b/drivers/mtd/nand/nand_toshiba.c
> >>>>> @@ -35,6 +35,34 @@ static void toshiba_nand_decode_id(struct nand_chip *chip)
> >>>>> (chip->id.data[5] & 0x7) == 0x6 /* 24nm */ &&
> >>>>> !(chip->id.data[4] & 0x80) /* !BENAND */)
> >>>>> mtd->oobsize = 32 * mtd->writesize >> 9;
> >>>>> +
> >>>>> + /*
> >>>>> + * Extract ECC requirements from 6th id byte.
> >>>>> + * For Toshiba SLC, ecc requrements are as follows:
> >>>>> + * - 43nm: 1 bit ECC for each 512Byte is required.
> >>>>> + * - 32nm: 4 bit ECC for each 512Byte is required.
> >>>>> + * - 24nm: 8 bit ECC for each 512Byte is required.
> >>>>> + */
> >>>>> + if (chip->id.len >= 6 && nand_is_slc(chip)) {
> >>>>> + chip->ecc_step_ds = 512;
> >>>>> + switch (chip->id.data[5] & 0x7) {
> >>>>> + case 0x4:
> >>>>> + chip->ecc_strength_ds = 1;
> >>>>> + break;
> >>>>> + case 0x5:
> >>>>> + chip->ecc_strength_ds = 4;
> >>>>> + break;
> >>>>> + case 0x6:
> >>>>> + chip->ecc_strength_ds = 8;
> >>>>> + break;
> >>>>> + default:
> >>>>> + WARN(1, "Could not get ECC info");
> >>>>> + chip->ecc_step_ds = 0;
> >>>>> + break;
> >>>>> + }
> >>>>> + } else if (chip->id.len < 6 && nand_is_slc(chip)) {
> >>>>> + WARN(1, "Could not get ECC info, 6th nand id byte does not exist.");
> >>>>
> >>>> I'm pretty sure you have old NAND chips that do not have 6bytes ids
> >>>> (see the table here [1]), and printing a huge backtrace in this case is
> >>>> probably not what you want.
> >>>>
> >>>> If you're okay with dropping this else block, I'll do the change when
> >>>> applying, no need to send a new version.
> >>>
> >>> Some controllers may have limitation in reading ids beyond 5 bytes,
> >>> considering such scenario we think it is better to keep this warning.
> >>> However if you feel huge backtrace is an issue, how about we using pr_warn() instead?
> >>>
> >>
> >> Toshiba NANDs with an id smaller than 6 bytes exist, so no, we should
> >> not complain at all. If the controller is broken and can't read the 8 id
> >> bytes the core is asking for, then it should be detected at the core
> >> level not in the NAND manufacturer driver.
> >
> > I understood your opinion. Please apply this patch with dropping the else block.
>
> Should I repost patch with else block dropped? Please let me know if that is necessary.
I forgot to apply it :-/. Please, send a new version so I can track
it in patchwork.
Thanks,
Boris