From patchwork Wed Sep 18 21:42:25 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Norris X-Patchwork-Id: 2909561 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id CEF629F289 for ; Wed, 18 Sep 2013 21:43:11 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id CD8D1203ED for ; Wed, 18 Sep 2013 21:43:10 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 67D00203DB for ; Wed, 18 Sep 2013 21:43:09 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VMPWc-0002W7-0D; Wed, 18 Sep 2013 21:43:06 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VMPWa-0007vi-0k; Wed, 18 Sep 2013 21:43:04 +0000 Received: from mail-vb0-f45.google.com ([209.85.212.45]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VMPWS-0007un-BA; Wed, 18 Sep 2013 21:42:57 +0000 Received: by mail-vb0-f45.google.com with SMTP id e15so5886625vbg.32 for ; Wed, 18 Sep 2013 14:42:29 -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-type:content-disposition:in-reply-to:user-agent; bh=HLnXRC0/nbEXVF0wGdzxPzAgE0hq1gcVmMHFSE4xmp8=; b=sdbivJbLLmMot+t+mblZr5L+vKGH+83h7wuTDc51iaWjZd0x7NKiLCJBF3Sntsy4gT FJoolOcAAajtbzsXS/mgvFn1nfEjhaDahYsV6iuEJPFzZOulNuqI6ECIVnvPNilR0nN9 Ik4dvrLpSizRqcl0gN0Lvw9m4B9YCeEgQc7u5zmZFHs9laP7nAsI/FSeFRScqiCD+Sy/ B+5RAoKrO1Pca45m4lL+LiwXz+z/CADOYFqXxRxHY90XXwKyEkS/VnR4ppGI0nbwrLRn hkrM9BQJFiy5eV5c0WzUB1wPBoRUEYA+ADLBgKA1EdoGoNGhcvd7lIAiVKn0QF1jwKlL S2tg== X-Received: by 10.220.237.208 with SMTP id kp16mr39489162vcb.4.1379540549843; Wed, 18 Sep 2013 14:42:29 -0700 (PDT) Received: from ld-irv-0074.broadcom.com (5520-maca-inet1-outside.broadcom.com. [216.31.211.11]) by mx.google.com with ESMTPSA id ww6sm3821163vec.5.1969.12.31.16.00.00 (version=TLSv1 cipher=RC4-SHA bits=128/128); Wed, 18 Sep 2013 14:42:29 -0700 (PDT) Date: Wed, 18 Sep 2013 14:42:25 -0700 From: Brian Norris To: Josh Wu Subject: Re: [PATCH v3] mtd: atmel_nand: use minimum ecc requirements of nand: ecc_{strength,step}_ds Message-ID: <20130918214225.GP4550@ld-irv-0074.broadcom.com> References: <1379483928-18253-1-git-send-email-josh.wu@atmel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1379483928-18253-1-git-send-email-josh.wu@atmel.com> User-Agent: Mutt/1.5.20 (2009-06-14) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130918_174256_532161_CB218247 X-CRM114-Status: GOOD ( 26.24 ) X-Spam-Score: -2.7 (--) Cc: nicolas.ferre@atmel.com, plagnioj@jcrosoft.com, linux-mtd@lists.infradead.org, linux-arm-kernel@lists.infradead.org, dedekind1@gmail.com X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Sep 18, 2013 at 01:58:48PM +0800, Josh Wu wrote: > Since ecc_{strength,step}_ds is introduced in nand_chip structure for > minimum ecc requirements. So we can use them directly and remove our > own get_onfi_ecc_param function. > > Signed-off-by: Josh Wu > --- > v2 --> v3: > 1. since ecc_{strength,step}_ds is non ONFI specific remove the check code: > 'if (host->nand_chip.onfi_version)' > use 'if (host->nand_chip.ecc_strength_ds)' instead. > 2. refined the commit message and change the comment since now we are not > ONFI specific. > > v1 --> v2: > 1. updated the refered commit id as it is changed when merged in mainline. > 2. refined the commit message > > drivers/mtd/nand/atmel_nand.c | 46 ++++++++--------------------------------- > 1 file changed, 9 insertions(+), 37 deletions(-) > > diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c > index 5a5d457..46e8794 100644 > --- a/drivers/mtd/nand/atmel_nand.c > +++ b/drivers/mtd/nand/atmel_nand.c > @@ -1062,56 +1062,28 @@ static void atmel_pmecc_core_init(struct mtd_info *mtd) > } > > /* > - * Get ECC requirement in ONFI parameters, returns -1 if ONFI > - * parameters is not supported. > - * return 0 if success to get the ECC requirement. > - */ > -static int get_onfi_ecc_param(struct nand_chip *chip, > - int *ecc_bits, int *sector_size) > -{ > - *ecc_bits = *sector_size = 0; > - > - if (chip->onfi_params.ecc_bits == 0xff) > - /* TODO: the sector_size and ecc_bits need to be find in > - * extended ecc parameter, currently we don't support it. > - */ > - return -1; > - > - *ecc_bits = chip->onfi_params.ecc_bits; > - > - /* The default sector size (ecc codeword size) is 512 */ > - *sector_size = 512; > - > - return 0; > -} > - > -/* > - * Get ecc requirement from ONFI parameters ecc requirement. > + * Get minimum ecc requirements from NAND. > * If pmecc-cap, pmecc-sector-size in DTS are not specified, this function > - * will set them according to ONFI ecc requirement. Otherwise, use the > + * will set them according to minimum ecc requirement. Otherwise, use the > * value in DTS file. > * return 0 if success. otherwise return error code. > */ > static int pmecc_choose_ecc(struct atmel_nand_host *host, > int *cap, int *sector_size) > { > - /* Get ECC requirement from ONFI parameters */ > - *cap = *sector_size = 0; > - if (host->nand_chip.onfi_version) { > - if (!get_onfi_ecc_param(&host->nand_chip, cap, sector_size)) > - dev_info(host->dev, "ONFI params, minimum required ECC: %d bits in %d bytes\n", > + /* Get minimum ECC requirements */ > + if (host->nand_chip.ecc_strength_ds) { > + *cap = host->nand_chip.ecc_strength_ds; > + *sector_size = host->nand_chip.ecc_step_ds; > + dev_info(host->dev, "minimum ECC requirements: %d bits in %d bytes\n", > *cap, *sector_size); > - else > - dev_info(host->dev, "NAND chip ECC reqirement is in Extended ONFI parameter, we don't support yet.\n"); > } else { > - dev_info(host->dev, "NAND chip is not ONFI compliant, assume ecc_bits is 2 in 512 bytes"); > - } > - if (*cap == 0 && *sector_size == 0) { > *cap = 2; > *sector_size = 512; > + dev_info(host->dev, "cannot detect minimum ECC requirements, assume 2-bit correction per 512 bytes"); Missing '\n' at the end of the string. > } > > - /* If dts file doesn't specify then use the one in ONFI parameters */ > + /* If dts file doesn't specify then use the NAND's minimum ecc parameters */ This code is not interested in a "dts file", it is interested in the device tree. > if (host->pmecc_corr_cap == 0) { > /* use the most fitable ecc bits (the near bigger one ) */ > if (*cap <= 2) I'll just apply the appended diff, to address these small comments and to shorten the messages a little (they are a bit verbose and are over 80 characters). It's still not under 80, but it's more reasonable. Let me know if you have any objections, or else I'll push this to l2-mtd.git. Thanks, Brian diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c index 46e8794..ef9c9f5 100644 --- a/drivers/mtd/nand/atmel_nand.c +++ b/drivers/mtd/nand/atmel_nand.c @@ -1075,15 +1075,15 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host, if (host->nand_chip.ecc_strength_ds) { *cap = host->nand_chip.ecc_strength_ds; *sector_size = host->nand_chip.ecc_step_ds; - dev_info(host->dev, "minimum ECC requirements: %d bits in %d bytes\n", + dev_info(host->dev, "minimum ECC: %d bits in %d bytes\n", *cap, *sector_size); } else { *cap = 2; *sector_size = 512; - dev_info(host->dev, "cannot detect minimum ECC requirements, assume 2-bit correction per 512 bytes"); + dev_info(host->dev, "can't detect min. ECC, assume 2 bits in 512 bytes\n"); } - /* If dts file doesn't specify then use the NAND's minimum ecc parameters */ + /* If device tree doesn't specify, use NAND's minimum ECC parameters */ if (host->pmecc_corr_cap == 0) { /* use the most fitable ecc bits (the near bigger one ) */ if (*cap <= 2)