From patchwork Tue Apr 26 05:53:55 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Brian Norris X-Patchwork-Id: 614785 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2001:1868:205::9]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3qvC51658Yz9t4h for ; Tue, 26 Apr 2016 15:56:05 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=0eiLz3Bv; dkim-atps=neutral Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1auvxA-0000Il-S5; Tue, 26 Apr 2016 05:54:32 +0000 Received: from mail-pf0-x22b.google.com ([2607:f8b0:400e:c00::22b]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1auvx9-0000Fx-Am for linux-mtd@lists.infradead.org; Tue, 26 Apr 2016 05:54:31 +0000 Received: by mail-pf0-x22b.google.com with SMTP id n1so2867994pfn.2 for ; Mon, 25 Apr 2016 22:54:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=ZB1ZeA92lXUuSRGIfVetOMcX/q4yruRkA0pqUxLRd4c=; b=0eiLz3Bvd1IN0SykCqOyxjyKCa/m2E3FxLCyUj7Qa2tF/5/Z0NSR1ncOz2Pwm/0MuA 1Zi0P3tbCSkaiWV05NnATu+KfU3bxpSxl0rdVYfUrGxTAEaTBQngBBRP/PU5S7PUNUjR 8PFT+8OGiiN00acO1aP7jcLsNjZ1OhxStY4M8RFwtBImUQ0bk8dGjIu8B3EvfuwoyT6w SmGEKx+Jq9RfOibLwoeScqnunVabeihQHROLNEaRmKN1I39pfmHrorhgmN1fLVtfW/4j +Nl5FpUzBwDpD6q5PGf+n8tUpewuHBICE9qTEWdk9mfcvBRk7aPTLPXAH8m5T2ZAKCQa dkPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=ZB1ZeA92lXUuSRGIfVetOMcX/q4yruRkA0pqUxLRd4c=; b=KkuMMDkA7B3uil1/Z8dqdu4/ZMrGSwwkd4Mzz52tZ4NfRjQqa5IUCKzkTmRABsc4+2 J2BzcXrFa48kpa1WZqYvi4sfd8zt3Ts/5oFryo4nd5T6f9NOCT6nIZUcgpHOcmnNb0fU 5x3CMZIPy2/TwtcDHVWStXxZn2FImeNemintDZV+2hnenWtXuahjbJpPv8bKXn0l9ID+ tIwHVmnTN2Z2xf1BGqSm8Jsz3ddaBYgU2xfm3CZi+w567RmDp1/ARQRAr7MJZe+oQpkg G2A40d16f8oQgAwhsOjvJ1tcC8bLiMsVXOrdQp+7m4UE/lzl4ggcOgSFNUnLUGINS8Bf Aj0w== X-Gm-Message-State: AOPr4FUHY2oCCkXJlp3/XlEcC5lBCK4m2/tQjrpYNSslMVBf3Z6U3TIkDTFeonrWwzCcdA== X-Received: by 10.98.20.197 with SMTP id 188mr1015969pfu.144.1461650050221; Mon, 25 Apr 2016 22:54:10 -0700 (PDT) Received: from localhost ([66.232.90.194]) by smtp.gmail.com with ESMTPSA id d6sm1442906pfj.75.2016.04.25.22.54.00 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 25 Apr 2016 22:54:09 -0700 (PDT) Date: Mon, 25 Apr 2016 22:53:55 -0700 From: Brian Norris To: =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= Subject: Re: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND subsystem Message-ID: <20160426055355.GA25981@localhost> References: <1461324197-1333-1-git-send-email-zajec5@gmail.com> <1461324197-1333-3-git-send-email-zajec5@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1461324197-1333-3-git-send-email-zajec5@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160425_225431_454210_F320B52E X-CRM114-Status: GOOD ( 20.18 ) X-Spam-Score: -2.7 (--) X-Spam-Report: SpamAssassin version 3.4.0 on bombadil.infradead.org summary: Content analysis details: (-2.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (computersforpeace[at]gmail.com) -0.0 SPF_PASS SPF: sender matches SPF record -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [2607:f8b0:400e:c00:0:0:0:22b listed in] [list.dnswl.org] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Boris Brezillon , Kamal Dasu , Richard Weinberger , open list , "open list:BROADCOM STB NAND FLASH DRIVER" , linux-mtd@lists.infradead.org, David Woodhouse Sender: "linux-mtd" Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org On Fri, Apr 22, 2016 at 01:23:15PM +0200, Rafał Miłecki wrote: > It's more reliable than guessing based on ECC strength. It allows using > NAND on devices with BCH-1 (e.g. D-Link DIR-885L). > > Signed-off-by: Rafał Miłecki > --- > drivers/mtd/nand/brcmnand/brcmnand.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c > index c3331ff..dcb22dc 100644 > --- a/drivers/mtd/nand/brcmnand/brcmnand.c > +++ b/drivers/mtd/nand/brcmnand/brcmnand.c > @@ -1927,7 +1927,7 @@ static int brcmnand_setup_dev(struct brcmnand_host *host) > > switch (chip->ecc.size) { > case 512: > - if (chip->ecc.strength == 1) /* Hamming */ > + if (chip->ecc.algo == NAND_ECC_HAMMING) This doesn't handle most of the problems I noted on the early version of this series. (But thank you for following through on the algorithm selection refactoring!) Particularly, this change will (a) break any existing DTs which used to have 'nand-ecc-size = <1>', and would assume this gets Hamming ECC; and (b) allows DTs to specify Hamming ECC for unsupported modes, like 1024B sectors, or ecc_level != 1. None of these are supported in HW. > cfg->ecc_level = 15; > else > cfg->ecc_level = chip->ecc.strength; Something like the following probably works better (not tested): ---8<--- From: Brian Norris Date: Mon, 25 Apr 2016 20:48:02 -0700 Subject: [PATCH] mtd: brcmnand: respect ECC algorithm set by the NAND subsystem This is more obvious than guessing based on ECC strength. It allows using NAND on devices with BCH-1 (e.g. D-Link DIR-885L). This maintains DT backward compatibility by defaulting to Hamming if a 1-bit ECC algorithm is specified without a corresponding algorithm selection. i.e., to use BCH-1, you must specify: nand-ecc-strength = <1>; nand-ecc-step-size = <512>; nand-ecc-algo = "bch"; Also adds a check to ensure we haven't allowed someone to get by with SW ECC. If we want to support SW ECC, we need to refactor some other pieces of this driver. Signed-off-by: Brian Norris Tested-by: Rafał Miłecki --- drivers/mtd/nand/brcmnand/brcmnand.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c index c3331ffcaffd..b76ad7c0144f 100644 --- a/drivers/mtd/nand/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/brcmnand/brcmnand.c @@ -1925,9 +1925,31 @@ static int brcmnand_setup_dev(struct brcmnand_host *host) cfg->col_adr_bytes = 2; cfg->blk_adr_bytes = get_blk_adr_bytes(mtd->size, mtd->writesize); + if (chip->ecc.mode != NAND_ECC_HW) { + dev_err(ctrl->dev, "only HW ECC supported; selected: %d\n", + chip->ecc.mode); + return -EINVAL; + } + + if (chip->ecc.algo == NAND_ECC_UNKNOWN) { + if (chip->ecc.strength == 1 && chip->ecc.size == 512) + /* Default to Hamming for 1-bit ECC, if unspecified */ + chip->ecc.algo = NAND_ECC_HAMMING; + else + /* Otherwise, BCH */ + chip->ecc.algo = NAND_ECC_BCH; + } + + if (chip->ecc.algo == NAND_ECC_HAMMING && (chip->ecc.strength != 1 || + chip->ecc.size != 512)) { + dev_err(ctrl->dev, "invalid Hamming params: %d bits per %d bytes\n", + chip->ecc.strength, chip->ecc.size); + return -EINVAL; + } + switch (chip->ecc.size) { case 512: - if (chip->ecc.strength == 1) /* Hamming */ + if (chip->ecc.algo == NAND_ECC_HAMMING) cfg->ecc_level = 15; else cfg->ecc_level = chip->ecc.strength;