[U-Boot] [PATCH v3 07/11] mtd: nand: add Faraday FTNANDC021 NAND controller support

Kuo-Jung Su
2013-04-29T03:28:14Z

2013/4/27 Scott Wood <scottwood@freescale.com>:
> On 04/26/2013 03:02:36 AM, Kuo-Jung Su wrote:
>>
>> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>>
>> 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 <dantesu@faraday-tech.com>
>> CC: Scott Wood <scottwood@freescale.com>
>> ---
>> 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.
>
>

Got it, thanks

>> 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 <dantesu@faraday-tech.com>
>> + *
>> + * 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 <common.h>
>> +#include <asm/errno.h>
>> +#include <asm/io.h>
>> +#include <asm/unaligned.h>
>> +#include <nand.h>
>> +#include <malloc.h>
>> +#include <faraday/nand.h>
>> +
>> +#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.
>
>

Got it, thanks
I'll just remove the non-ECC codes later

>> +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?
>
>

Actually it means nothing here, the ECC function is designed to be some kind of
a blackbox to users (i.e softwares), the actual position of ECC codes
are not documented
in the datasheet, I have to dig it out from RTL which I do not have
permission to access.

And there are only 5 bytes are free to users in 2K page flash, users
actually could only
control the 'value' of the 5 bytes, the actual position of these data
are hard-coded in RTL level.

P.S: The 5-bytes are: 1-byte Bad Block Info + 4-bytes data in OOB.

>> +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(&regs->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?
>
>

No, the FTNANDC021 does not have this function.
Like I said, it was designed for SSD applications, which usually uses
some sort of
bare-metal software, and cares nothing about if it has encounter
correctable ECC error.

>> +#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?
>

Theoretical, NO.
It uses R-S or BCH for Forwarding Error Correction, which is selected
at RTL level.
Which means it actually 522 bytes per step at RS and 528 bytes per step at BCH.

However it's all blackbox to software, the FTNANDC021 simply supports only
page read, so looks like everything is done at 1 step.

> -Scott

--
Best wishes,
Kuo-Jung Su