2020-03-11 08:40:43

by Yoshio Furuyama

[permalink] [raw]
Subject: [PATCH v3] mtd: rawnand: toshiba: Support actual bitflips for BENAND (Built-in ECC NAND)

Add support vendor specific commands for KIOXIA CORPORATION BENAND.
The actual bitflips number can be get by this command.

Signed-off-by: Yoshio Furuyama <[email protected]>
---
changelog[v3]:Tested version.
original patch:"[RFC,v2] mtd: nand: toshiba: Add support for ->exec_op()".

drivers/mtd/nand/raw/nand_toshiba.c | 55 +++++++++++++++++++++++++++++++++++--
1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/nand_toshiba.c
index 9c03fbb..10fcfbd 100644
--- a/drivers/mtd/nand/raw/nand_toshiba.c
+++ b/drivers/mtd/nand/raw/nand_toshiba.c
@@ -14,14 +14,65 @@
/* Recommended to rewrite for BENAND */
#define TOSHIBA_NAND_STATUS_REWRITE_RECOMMENDED BIT(3)

+/* ECC Status Read Command for BENAND */
+#define TOSHIBA_NAND_CMD_ECC_STATUS_READ 0x7A
+
+/* ECC Status Mask for BENAND */
+#define TOSHIBA_NAND_ECC_STATUS_MASK 0x0F
+
+/* Uncorrectable Error for BENAND */
+#define TOSHIBA_NAND_ECC_STATUS_UNCORR 0x0F
+
+static int toshiba_nand_benand_read_eccstatus_op(struct nand_chip *chip,
+ u8 *buf)
+{
+ u8 *ecc_status = buf;
+
+ if (nand_has_exec_op(chip)) {
+ const struct nand_sdr_timings *sdr =
+ nand_get_sdr_timings(&chip->data_interface);
+ struct nand_op_instr instrs[] = {
+ NAND_OP_CMD(TOSHIBA_NAND_CMD_ECC_STATUS_READ,
+ PSEC_TO_NSEC(sdr->tADL_min)),
+ NAND_OP_8BIT_DATA_IN(chip->ecc.steps, ecc_status, 0),
+ };
+ struct nand_operation op = NAND_OPERATION(chip->cur_cs, instrs);
+
+ return nand_exec_op(chip, &op);
+ }
+
+ return -ENOTSUPP;
+}
+
static int toshiba_nand_benand_eccstatus(struct nand_chip *chip)
{
struct mtd_info *mtd = nand_to_mtd(chip);
int ret;
unsigned int max_bitflips = 0;
- u8 status;
+ u8 status, ecc_status[8];

/* Check Status */
+ ret = toshiba_nand_benand_read_eccstatus_op(chip, ecc_status);
+ if (!ret) {
+ unsigned int i, bitflips = 0;
+
+ for (i = 0; i < chip->ecc.steps; i++) {
+ bitflips = ecc_status[i] & TOSHIBA_NAND_ECC_STATUS_MASK;
+ if (bitflips == TOSHIBA_NAND_ECC_STATUS_UNCORR) {
+ mtd->ecc_stats.failed++;
+ } else {
+ mtd->ecc_stats.corrected += bitflips;
+ max_bitflips = max(max_bitflips, bitflips);
+ }
+ }
+
+ return max_bitflips;
+ }
+
+ /*
+ * Fallback to regular status check if
+ * toshiba_nand_benand_read_eccstatus_op() failed.
+ */
ret = nand_status_op(chip, &status);
if (ret)
return ret;
@@ -108,7 +159,7 @@ static void toshiba_nand_decode_id(struct nand_chip *chip)
*/
if (chip->id.len >= 6 && nand_is_slc(chip) &&
(chip->id.data[5] & 0x7) == 0x6 /* 24nm */ &&
- !(chip->id.data[4] & 0x80) /* !BENAND */) {
+ !(chip->id.data[4] & TOSHIBA_NAND_ID4_IS_BENAND) /* !BENAND */) {
memorg->oobsize = 32 * memorg->pagesize >> 9;
mtd->oobsize = memorg->oobsize;
}
--
1.9.1


2020-03-20 17:22:55

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v3] mtd: rawnand: toshiba: Support actual bitflips for BENAND (Built-in ECC NAND)

Hi Yoshio,

Yoshio Furuyama <[email protected]> wrote on Wed, 11 Mar
2020 15:19:43 +0900:


The title is misleading, would it be better:

"Support reading the number of bitflips"

> Add support vendor specific commands for KIOXIA CORPORATION BENAND.
> The actual bitflips number can be get by this command.

retrieved?

>
> Signed-off-by: Yoshio Furuyama <[email protected]>
> ---
> changelog[v3]:Tested version.
> original patch:"[RFC,v2] mtd: nand: toshiba: Add support for ->exec_op()".
>
> drivers/mtd/nand/raw/nand_toshiba.c | 55 +++++++++++++++++++++++++++++++++++--
> 1 file changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/nand_toshiba.c
> index 9c03fbb..10fcfbd 100644
> --- a/drivers/mtd/nand/raw/nand_toshiba.c
> +++ b/drivers/mtd/nand/raw/nand_toshiba.c
> @@ -14,14 +14,65 @@
> /* Recommended to rewrite for BENAND */
> #define TOSHIBA_NAND_STATUS_REWRITE_RECOMMENDED BIT(3)
>
> +/* ECC Status Read Command for BENAND */
> +#define TOSHIBA_NAND_CMD_ECC_STATUS_READ 0x7A
> +
> +/* ECC Status Mask for BENAND */
> +#define TOSHIBA_NAND_ECC_STATUS_MASK 0x0F
> +
> +/* Uncorrectable Error for BENAND */
> +#define TOSHIBA_NAND_ECC_STATUS_UNCORR 0x0F
> +
> +static int toshiba_nand_benand_read_eccstatus_op(struct nand_chip *chip,
> + u8 *buf)
> +{
> + u8 *ecc_status = buf;
> +
> + if (nand_has_exec_op(chip)) {
> + const struct nand_sdr_timings *sdr =
> + nand_get_sdr_timings(&chip->data_interface);
> + struct nand_op_instr instrs[] = {
> + NAND_OP_CMD(TOSHIBA_NAND_CMD_ECC_STATUS_READ,
> + PSEC_TO_NSEC(sdr->tADL_min)),
> + NAND_OP_8BIT_DATA_IN(chip->ecc.steps, ecc_status, 0),
> + };
> + struct nand_operation op = NAND_OPERATION(chip->cur_cs, instrs);
> +
> + return nand_exec_op(chip, &op);
> + }
> +
> + return -ENOTSUPP;
> +}
> +
> static int toshiba_nand_benand_eccstatus(struct nand_chip *chip)
> {
> struct mtd_info *mtd = nand_to_mtd(chip);
> int ret;
> unsigned int max_bitflips = 0;
> - u8 status;
> + u8 status, ecc_status[8];

Shall we define this ^ ?
It is strange to have the number of steps hardcoded this way.

>
> /* Check Status */
> + ret = toshiba_nand_benand_read_eccstatus_op(chip, ecc_status);
> + if (!ret) {
> + unsigned int i, bitflips = 0;
> +
> + for (i = 0; i < chip->ecc.steps; i++) {
> + bitflips = ecc_status[i] & TOSHIBA_NAND_ECC_STATUS_MASK;
> + if (bitflips == TOSHIBA_NAND_ECC_STATUS_UNCORR) {
> + mtd->ecc_stats.failed++;
> + } else {
> + mtd->ecc_stats.corrected += bitflips;
> + max_bitflips = max(max_bitflips, bitflips);
> + }
> + }
> +
> + return max_bitflips;
> + }
> +
> + /*
> + * Fallback to regular status check if
> + * toshiba_nand_benand_read_eccstatus_op() failed.
> + */
> ret = nand_status_op(chip, &status);
> if (ret)
> return ret;
> @@ -108,7 +159,7 @@ static void toshiba_nand_decode_id(struct nand_chip *chip)
> */
> if (chip->id.len >= 6 && nand_is_slc(chip) &&
> (chip->id.data[5] & 0x7) == 0x6 /* 24nm */ &&
> - !(chip->id.data[4] & 0x80) /* !BENAND */) {
> + !(chip->id.data[4] & TOSHIBA_NAND_ID4_IS_BENAND) /* !BENAND */) {
> memorg->oobsize = 32 * memorg->pagesize >> 9;
> mtd->oobsize = memorg->oobsize;
> }

Looks fine otherwise.

Thanks,
Miquèl