From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Fri, 26 Apr 2013 18:41:30 -0500 Subject: [U-Boot] [PATCH v3 07/11] mtd: nand: add Faraday FTNANDC021 NAND controller support In-Reply-To: <1366963360-2987-8-git-send-email-dantesu@gmail.com> (from dantesu@gmail.com on Fri Apr 26 03:02:36 2013) References: <1366277139-29728-2-git-send-email-dantesu@gmail.com> <1366963360-2987-8-git-send-email-dantesu@gmail.com> Message-ID: <1367019690.26749.12@snotra> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 04/26/2013 03:02:36 AM, Kuo-Jung Su wrote: > From: Kuo-Jung Su > > Faraday FTNANDC021 is a integrated NAND flash controller. > It use a build-in command table to abstract the underlying > NAND flash control logic. > > For example: > > Issuing a command 0x10 to FTNANDC021 would result in > a page write + a read status operation. > > Signed-off-by: Kuo-Jung Su > CC: Scott Wood > --- > README | 7 + > drivers/mtd/nand/Makefile | 1 + > drivers/mtd/nand/ftnandc021.c | 724 > +++++++++++++++++++++++++++++++++++++++++ > drivers/mtd/nand/ftnandc021.h | 137 ++++++++ > include/faraday/nand.h | 34 ++ > 5 files changed, 903 insertions(+) > create mode 100644 drivers/mtd/nand/ftnandc021.c > create mode 100644 drivers/mtd/nand/ftnandc021.h > create mode 100644 include/faraday/nand.h > > diff --git a/README b/README > index 862bb3e..adc198f 100644 > --- a/README > +++ b/README > @@ -3872,6 +3872,13 @@ Low Level (hardware related) configuration > options: > - drivers/mtd/nand/ndfc.c > - drivers/mtd/nand/mxc_nand.c > > +- CONFIG_SYS_NAND_TIMING > + Defined to tell the NAND controller that the NAND chip > is using > + a customized timing parameters. > + Not all NAND drivers use this symbol. > + Example of drivers that use it: > + - drivers/mtd/nand/ftnandc021.c This doesn't seem to have any standardized meaning (even if that meaning is applicable to only a subset of controllers), so please call it CONFIG_SYS_FTNANDC021_TIMING and document the ftnandc021-specific semantics. > diff --git a/drivers/mtd/nand/ftnandc021.c > b/drivers/mtd/nand/ftnandc021.c > new file mode 100644 > index 0000000..39c181f > --- /dev/null > +++ b/drivers/mtd/nand/ftnandc021.c > @@ -0,0 +1,724 @@ > +/* > + * Faraday NAND Flash Controller > + * > + * (C) Copyright 2010 Faraday Technology > + * Dante Su > + * > + * This file is released under the terms of GPL v2 and any later > version. > + * See the file COPYING in the root directory of the source tree for > details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "ftnandc021.h" > + > +#define CFG_HWECC /* Enable hardware ECC */ If this is really to be optional, call it CONFIG_FTNANDC021_ECC (HWECC suggests the alternative is SW ECC, not no ECC) and define it in the board config file (or better, invert it to CONFIG_FTNANDC021_NO_ECC so ECC is the default). If it's not meant to be optional, just remove the non-ECC code. If the ECC doesn't cause major problems in a reasonable use case, I'd rather see the non-ECC code just removed. > +static struct nand_ecclayout ftnandc021_ecclayout[] = { > + { /* page size = 512 (oob size = 16) */ > + .eccbytes = 6, > + .eccpos = { 0, 1, 2, 3, 6, 7 }, > + .oobfree = { > +#ifdef CFG_HWECC > + { 9, 3 }, > +#else > + { 8, 4 }, > +#endif > + } > + }, > + { /* page size = 2048 (oob size = 64) */ > + .eccbytes = 24, > + .eccpos = { > + 40, 41, 42, 43, 44, 45, 46, 47, > + 48, 49, 50, 51, 52, 53, 54, 55, > + 56, 57, 58, 59, 60, 61, 62, 63 > + }, > + .oobfree = { > +#ifdef CFG_HWECC > + { 9, 3 }, > +#else > + { 8, 4 }, > +#endif > + }, > + }, > + { /* page size = 4096 (oob size = 128) */ > + .eccbytes = 48, > + .eccpos = { > + 80, 81, 82, 83, 84, 85, 86, 87, > + 88, 89, 90, 91, 92, 93, 94, 95, > + 96, 97, 98, 99, 100, 101, 102, 103, > + 104, 105, 106, 107, 108, 109, 110, 111, > + 112, 113, 114, 115, 116, 117, 118, 119, > + 120, 121, 122, 123, 124, 125, 126, 127 > + }, > + .oobfree = { > +#ifdef CFG_HWECC > + { 9, 7 }, > +#else > + { 8, 8 }, > +#endif > + }, > + }, > +}; Shouldn't .eccpos depend on HWECC? > +static int ftnandc021_ecc_correct(struct mtd_info *mtd, uint8_t *dat, > + uint8_t *read_ecc, uint8_t *calc_ecc) > +{ > + struct nand_chip *chip = mtd->priv; > + struct faraday_nand_chip *info = chip->priv; > + struct ftnandc021_chip *priv = info->priv; > + struct ftnandc021_regs __iomem *regs = priv->regs; > + uint32_t st = readl(®s->ecc_sr); > + int ret = 0; > + > + if (st & ECC_SR_CERR) { > + printf("ftnandc021: ecc corection error\n"); > + ret = -EIO; > + } else if (st & ECC_SR_ERR) { > + printf("ftnandc021: ecc error\n"); > + ret = -EIO; > + } > + > + return ret; Can you detect correctable errors? > +#ifdef CFG_HWECC > + chip->ecc.bytes = chip->ecc.layout->eccbytes; > + chip->ecc.size = info->pgsz; > + chip->ecc.steps = 1; Is it really all in one step, regardless of page size? -Scott