Re: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large page NAND chips

[David Brownell <david-b@pacbell.net>]

I'm glad to see this patch is so small ... basically, just
adding a special case for 2K pages, and keeping the core of
this NAND driver the same.  Not needing to change the 4-bit
ECC support from the patch I sent earlier seems a good sign.

Two comments though:  (a) the board-dm355-evm.c file isn't
yet in mainline, so the MTD folk can't take this patch as-is;
(b) as noted elsewhere, there are still issues with 4K pages
and the NAND core infrastructure.

This patch is sufficient to support development boards for
the dm355, dm357, and dm365 ... right?  They all have 2 GByte
NAND chips, ISTR with 2K pages, and haven't yet had to switch
to more current parts with 4K pages.


On Wednesday 06 May 2009, nsnehaprabha@ti.com wrote:
> --- a/drivers/mtd/nand/davinci_nand.c
> +++ b/drivers/mtd/nand/davinci_nand.c
> @@ -500,6 +500,21 @@ static struct nand_ecclayout hwecc4_small __initconst
> = { },
>  };
>  
> +/* An ECC layout for using 4-bit ECC with large-page (2048bytes) flash,
> + * storing ten ECC bytes plus the manufacturer's bad block marker byte,
> + * and not overlapping the default BBT markers.
> + */
> +static struct nand_ecclayout hwecc4_2048 __initconst = {
> +       .eccbytes = 10,

Not ".eccbytes = 40"?  This is 4 chunks, 10 ecc bytes each...


> +       .eccpos = { 0, 1, 2, 3, 4, 6, 7, 8, 9, 10,
> +               11, 12, 13, 14, 15, 24, 25, 26, 27, 28,
> +               29, 30, 31, 32, 33, 34, 35, 36, 37, 38,
> +               39, 40, 41, 42, 43, 44, 45, 46, 47, 48, },
> +       .oobfree = {
> +               {.offset = 16, .length = 8, },
> +               {.offset = 49, },

Comments would be good, highlighting (a) byte 5 is reserved,
it's the manufacturer bad block marker, (b) 8 bytes @16 are
expected by JFFS2.  Not everyone will "just know" those.


> +       },
> +};
>  
>  static int __init nand_davinci_probe(struct platform_device *pdev)
>  {

> @@ -690,6 +705,13 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
>                                 info->mtd.oobsize - 16;
>                         goto syndrome_done;
>                 }
> +               if (chunks == 4) {
> +                       info->ecclayout = hwecc4_2048;
> +                       info->ecclayout.oobfree[1].length =
> +                               info->mtd.oobsize - 49;
> +                       info->chip.ecc.mode = NAND_ECC_HW_OOB_FIRST;
> +                       goto syndrome_done;
> +               }
>  
>                 /* For large page chips we'll be wanting to use a
>                  * not-yet-implemented mode that reads OOB data

You should update that comment ... that not-yet-implemented mode
is now called "NAND_ECC_HW_OOB_FIRST", from patch 1/2 ... ;)