linux-kernel - Re: [PATCH v2] mtd: nand: support for Toshiba BENAND (Built-in ECC NAND)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <55842B51.6000507@toshiba.co.jp>
Date:	Fri, 19 Jun 2015 23:46:41 +0900
From:	KOBAYASHI Yoshitake <yoshitake.kobayashi@...hiba.co.jp>
To:	Richard Weinberger <richard@....at>
Cc:	David Woodhouse <dwmw2@...radead.org>,
	Brian Norris <computersforpeace@...il.com>,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] mtd: nand: support for Toshiba BENAND (Built-in ECC
 NAND)

> Please see my patches how to deal with that. Maybe you can find a better solution.

Thank you for your suggestion. I considered to use your submitted patch on BENAND
driver, but I believe reading twice the same page approach will affect read performance.
Additionally, BENAND does not support Disable ECC. So, I cannot use same approach.

I consider other solutions.

BENAND Read Status CMD (70h) can report Rewrite Recommend. BENAND also has extended
ECC Status CMD (7Ah). This can report number of bit error / sector data.
If I can use an extended ECC Status CMD (7Ah) in BENAND driver (nand_benand.c),
I suggest to implement it as the follows:

+++ b/drivers/mtd/nand/nand_benand.c
     .....

+/* ECC Status Read Command */
+#define NAND_CMD_ECC_STATUS 0x7A
     .....
+	chip->ecc.strength = 8;
     .....
+            /* correctable */
+            else if (status & NAND_STATUS_RECOM_REWRT) {
+                          if (chip->cmd_ctrl &&
+                                        IS_ENABLED(CONFIG_MTD_NAND_BENAND_ECC_STATUS)) {
+
+                                        /* Check Read ECC Status */
+                                        chip->cmd_ctrl(mtd, NAND_CMD_ECC_STATUS,
+                                                      NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
+                                        ecc_status = chip->read_byte(mtd);
+                                        bitflips = ecc_status & 0xff;
+                                        mtd->ecc_stats.corrected += bitflips;


If above implementation cannot be acceptable, I would like to use "mtd->bitflip_threshold"
to inform number of bitflip, as the follows:

+                          } else {
+                                        /*
+                                        * If can't use chip->cmd_ctrl,
+                                        * we can't get real number of bitflips.
+                                        * So, we set bitflips mtd->bitflip_threshold.
+                                        */
+                                        bitflips = mtd->bitflip_threshold;
+                                        mtd->ecc_stats.corrected += bitflips;
+                          }
+            }
+
+            return bitflips;
+}

-- Yoshi

On 2015/06/16 22:53, Richard Weinberger wrote:
> Am 16.06.2015 um 15:33 schrieb KOBAYASHI Yoshitake:
>>>> +       /* correctable */
>>>> +       else if (status & NAND_STATUS_RECOM_REWRT) {
>>>> +               pr_info("BENAND : Recommended to rewrite!\n");
>>>> +               bitflips = chip->ecc.strength;
>>>
>>> In your case this might be okay, as you set strength to 1.
>>> Otherweise you'd have to report the real number of bitflips.
>>
>> I also thought it is okay in this case.
>> BENAND return corrected data to Host NAND Controller till uncorrectable status.
>> The current patch uses this Read Status command 70h to abstract BENAND Multi
>> bit ECC and Need to Rewrite judgement so BENAND would look like 1bit ECC device.
>
> The layers above MTD need to know how many bits got repaired.
> It seems like BENAND suffers from the same shortcomings than On-Die-ECC. ;-\
> Please see my patches how to deal with that. Maybe you can find a better solution.
>
> Thanks,
> //richard
>
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/