From patchwork Wed Mar 22 14:07:17 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Masahiro Yamada X-Patchwork-Id: 95764 Delivered-To: patch@linaro.org Received: by 10.182.3.34 with SMTP id 2csp233630obz; Wed, 22 Mar 2017 07:11:34 -0700 (PDT) X-Received: by 10.98.91.130 with SMTP id p124mr43948754pfb.165.1490191894388; Wed, 22 Mar 2017 07:11:34 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 33si1978287plk.81.2017.03.22.07.11.34; Wed, 22 Mar 2017 07:11:34 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@nifty.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934426AbdCVOLU (ORCPT + 11 others); Wed, 22 Mar 2017 10:11:20 -0400 Received: from conuserg-07.nifty.com ([210.131.2.74]:46837 "EHLO conuserg-07.nifty.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934333AbdCVOK5 (ORCPT ); Wed, 22 Mar 2017 10:10:57 -0400 Received: from pug.jp.socionext.com (p14092-ipngnfx01kyoto.kyoto.ocn.ne.jp [153.142.97.92]) (authenticated) by conuserg-07.nifty.com with ESMTP id v2ME8LDT010154; Wed, 22 Mar 2017 23:08:37 +0900 DKIM-Filter: OpenDKIM Filter v2.10.3 conuserg-07.nifty.com v2ME8LDT010154 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nifty.com; s=dec2015msa; t=1490191718; bh=4fY1lO0n4sY5X3gRy4Dunpb62ZCGHW++MXJt5PFaI3g=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=mVPTKaO0XH/a92AJtvJbQsCdZ9EleLvwwrhvIY3Ay7CXX+HGSE25QPwmF9Y8q4OHn CPQzyLSEbyGeFRRlUeGUZYQf2bW/PeT4VZiq4BJ+NsbSSYHc5ozGd78k7qJM+sLQ8f DQ46Eh0R14MvYd1iW2cWuZ2A/OIb9zX28IVnu0BBjlv+mCZxHgUFXXz+0MJACjfV12 xkRTSP6qVJu/onoQ9NXLSDMiOlTIokj6/wjay660mF0nAmMklNpM8ECtDmjGNOiiH2 J6b3LlWWHoOItf3TzoTRWCw/PnNiyQXUUPc0yADloZQkD7h3x0uUMp8in5RkmpvPPP HeW6Te102MKAw== X-Nifty-SrcIP: [153.142.97.92] From: Masahiro Yamada To: linux-mtd@lists.infradead.org Cc: laurent.monat@idquantique.com, thorsten.christiansson@idquantique.com, Enrico Jorns , Jason Roberts , Artem Bityutskiy , Dinh Nguyen , Boris Brezillon , Marek Vasut , Brian Norris , Graham Moore , David Woodhouse , Masami Hiramatsu , Chuanxiao Dong , Jassi Brar , Masahiro Yamada , linux-kernel@vger.kernel.org, Richard Weinberger , Cyrille Pitchen Subject: [PATCH v2 10/53] mtd: nand: denali: fix erased page checking Date: Wed, 22 Mar 2017 23:07:17 +0900 Message-Id: <1490191680-14481-11-git-send-email-yamada.masahiro@socionext.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1490191680-14481-1-git-send-email-yamada.masahiro@socionext.com> References: <1490191680-14481-1-git-send-email-yamada.masahiro@socionext.com> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This part is wrong in multiple ways: [1] is_erased() is called against "buf" twice, so the second one is meaningless. The second call should check chip->oob_poi. [2] This code block is nested by double "if (check_erase_page)". The inner one is redundant. [3] Erased page checking without threshold is false-positive. Basically, there are two ways for erased page checking: - read the whole of page + oob in raw transfer, then check if all the data are 0xFF. - read the ECC-corrected page + oob, then check if *almost* all the data are 0xFF (bit-flips less than ecc.strength are allowed) While here, it checks if all data in ECC-corrected page are 0xFF. This is too strong because not all of the data are 0xFF after they are manipulated by the ECC engine. Proper threshold must be taken into account to avoid false-positive ecc_stats.failed increments. [4] positive return value for uncorrectable bitflips The comment of ecc->read_page() says it should return "0 if bitflips uncorrectable", but the current code could return a positive value in the case. This commit solves the problems above. The nand framework provides a helper nand_check_erased_ecc_chunk() for erased page check with threshold. The driver's own helper is unneeded. Signed-off-by: Masahiro Yamada --- Changes in v2: - Squash some patches into one. - Use nand_check_erased_ecc_chunk() with threshold drivers/mtd/nand/denali.c | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) -- 2.7.4 diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c index 2c59eb3..86381ac 100644 --- a/drivers/mtd/nand/denali.c +++ b/drivers/mtd/nand/denali.c @@ -883,19 +883,6 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page) } } -/* - * this function examines buffers to see if they contain data that - * indicate that the buffer is part of an erased region of flash. - */ -static bool is_erased(uint8_t *buf, int len) -{ - int i; - - for (i = 0; i < len; i++) - if (buf[i] != 0xFF) - return false; - return true; -} #define ECC_SECTOR_SIZE 512 #define ECC_SECTOR(x) (((x) & ECC_ERROR_ADDRESS__SECTOR_NR) >> 12) @@ -1119,6 +1106,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip, uint32_t irq_status; uint32_t irq_mask = INTR__ECC_TRANSACTION_DONE | INTR__ECC_ERR; bool check_erased_page = false; + int stat; if (page != denali->page) { 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); + if (stat < 0) { + mtd->ecc_stats.failed++; + /* return 0 for uncorrectable bitflips */ + max_bitflips = 0; } } return max_bitflips;