From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752740AbdCWHeC (ORCPT ); Thu, 23 Mar 2017 03:34:02 -0400 Received: from condef-08.nifty.com ([202.248.20.73]:48600 "EHLO condef-08.nifty.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750952AbdCWHeA (ORCPT ); Thu, 23 Mar 2017 03:34:00 -0400 DKIM-Filter: OpenDKIM Filter v2.10.3 conssluserg-04.nifty.com v2N54k0X002867 X-Nifty-SrcIP: [209.85.161.172] MIME-Version: 1.0 In-Reply-To: <20170322215623.531e9690@bbrezillon> References: <1490191680-14481-1-git-send-email-yamada.masahiro@socionext.com> <1490191680-14481-11-git-send-email-yamada.masahiro@socionext.com> <20170322215623.531e9690@bbrezillon> From: Masahiro Yamada Date: Thu, 23 Mar 2017 14:04:44 +0900 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 10/53] mtd: nand: denali: fix erased page checking To: Boris Brezillon Cc: linux-mtd@lists.infradead.org, Laurent Monat , thorsten.christiansson@idquantique.com, Enrico Jorns , Jason Roberts , Artem Bityutskiy , Dinh Nguyen , Marek Vasut , Brian Norris , Graham Moore , David Woodhouse , Masami Hiramatsu , Chuanxiao Dong , Jassi Brar , Linux Kernel Mailing List , Richard Weinberger , Cyrille Pitchen Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Boris, 2017-03-23 5:56 GMT+09:00 Boris Brezillon : > On Wed, 22 Mar 2017 23:07:17 +0900 > Masahiro Yamada wrote: >> dev_err(denali->dev, >> @@ -1148,12 +1136,15 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip, >> if (check_erased_page) { >> read_oob_data(mtd, chip->oob_poi, denali->page); >> >> - /* check ECC failures that may have occurred on erased pages */ >> - if (check_erased_page) { >> - if (!is_erased(buf, mtd->writesize)) >> - mtd->ecc_stats.failed++; >> - if (!is_erased(buf, mtd->oobsize)) >> - mtd->ecc_stats.failed++; >> + stat = nand_check_erased_ecc_chunk( >> + buf, mtd->writesize, >> + chip->oob_poi, mtd->oobsize, >> + NULL, 0, >> + chip->ecc.strength * chip->ecc.steps); > > That's not how it's supposed to be done. Each chunk should be checked > independently. Here is a simple example explaining why this is > important: > > Let's consider the following setup: > - 4k pages > - 16bits/1024bytes ECC > > With your approach, you turn this into: > - 4k pages > - 64bits/4096bytes ECC > > Now suppose you have 32 bitflips in the first 1024 bytes. The real ECC > config is expected to report uncorrectable errors, but your approach > will just report that 32 bits have been fixed, which is wrong. OK. How about adding a helper like follows: static int denali_check_erased_page(struct mtd_info *mtd, struct nand_chip *chip, uint8_t *buf) { uint8_t *ecc_code = chip->buffers->ecccode; int ecc_steps = chip->ecc.steps; int ecc_size = chip->ecc.size; int ecc_bytes = chip->ecc.bytes; int i, ret; ret = mtd_ooblayout_get_eccbytes(mtd, ecc_code, chip->oob_poi, 0, chip->ecc.total); if (ret) return ret; for (i = 0; i < ecc_steps; i++) { ret = nand_check_erased_ecc_chunk(buf, ecc_size, ecc_code, ecc_bytes, NULL, 0, chip->ecc.strength); if (ret < 0) return ret; buf += ecc_size; ecc_code += ecc_bytes; } return 0; } Then, stat = denali_check_erased_page(mtd, chip, buf); if (stat < 0) { mtd->ecc_stats.failed++; /* return 0 for uncorrectable bitflips */ stat = 0; } -- Best Regards Masahiro Yamada