From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754576AbbFSOrp (ORCPT ); Fri, 19 Jun 2015 10:47:45 -0400 Received: from imx2.toshiba.co.jp ([106.186.93.51]:52637 "EHLO imx2.toshiba.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752522AbbFSOrf (ORCPT ); Fri, 19 Jun 2015 10:47:35 -0400 Message-ID: <55842B51.6000507@toshiba.co.jp> Date: Fri, 19 Jun 2015 23:46:41 +0900 From: KOBAYASHI Yoshitake User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Richard Weinberger Cc: David Woodhouse , Brian Norris , "linux-mtd@lists.infradead.org" , LKML Subject: Re: [PATCH v2] mtd: nand: support for Toshiba BENAND (Built-in ECC NAND) References: <1434038402-26365-1-git-send-email-yoshitake.kobayashi@toshiba.co.jp> <558025B7.3030103@toshiba.co.jp> <55802A44.6040708@nod.at> In-Reply-To: <55802A44.6040708@nod.at> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > 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/ From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <55842B51.6000507@toshiba.co.jp> Date: Fri, 19 Jun 2015 23:46:41 +0900 From: KOBAYASHI Yoshitake MIME-Version: 1.0 To: Richard Weinberger Subject: Re: [PATCH v2] mtd: nand: support for Toshiba BENAND (Built-in ECC NAND) References: <1434038402-26365-1-git-send-email-yoshitake.kobayashi@toshiba.co.jp> <558025B7.3030103@toshiba.co.jp> <55802A44.6040708@nod.at> In-Reply-To: <55802A44.6040708@nod.at> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: "linux-mtd@lists.infradead.org" , Brian Norris , David Woodhouse , LKML List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , > 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 > > >