[PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large page NAND chips

[PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large page NAND chips

[nsnehaprabha]

From: Sneha Narnakaje <nsnehaprabha@ti.com>

This patch adds 4-bit ECC support for large page NAND chips using the new ECC
mode NAND_ECC_HW_OOB_FIRST. The platform data from board-dm355-evm has been
adjusted to use this mode.

The patches have been verified on DM355 device with 2K Micron devices using
mtd-tests and JFFS2. Error correction upto 4-bits has also been verified using
nandwrite/nanddump utilities.

Reviewed-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Sneha Narnakaje <nsnehaprabha@ti.com>
---
 drivers/mtd/nand/davinci_nand.c |   37 +++++++++++++++++++++++++++++++------
 1 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index ba6940d..4557b8d 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -500,6 +500,24 @@ 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 = 40,
+	.eccpos = { 0, 1, 2, 3, 4,
+		/* offset 5 holds the badblock marker */
+		6, 7, 8, 9, 10,	11, 12, 13, 14, 15,
+		/* 8 bytes at offset 16 hold JFFS2 clean markers */
+		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, },
+	},
+};
 
 static int __init nand_davinci_probe(struct platform_device *pdev)
 {
@@ -689,15 +707,22 @@ 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
-		 * before reading the body of the page, to avoid
-		 * the "infix OOB" model of NAND_ECC_HW_SYNDROME
-		 * (and preserve manufacturer badblock markings).
+		/* 4K page chips are not yet supported. The eccpos from
+		 * nand_ecclayout cannot hold 80bytes and change to eccpos[]
+		 * breaks userspace ioctl interface with mtd-utils. Once we
+		 * resolve this issue, NAND_ECC_HW_OOB_FIRST mode can be used
+		 * for the 4K page chips.
 		 */
 		dev_warn(&pdev->dev, "no 4-bit ECC support yet "
-				"for large page NAND\n");
+				"for 4K page NAND\n");
 		ret = -EIO;
 		goto err_scan;
 
-- 
1.6.0.4

Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large page NAND chips

[Troy Kisky]

nsnehaprabha@ti.com wrote:
> From: Sneha Narnakaje <nsnehaprabha@ti.com>
> 
> This patch adds 4-bit ECC support for large page NAND chips using the new ECC
> mode NAND_ECC_HW_OOB_FIRST. The platform data from board-dm355-evm has been
> adjusted to use this mode.
> 
> The patches have been verified on DM355 device with 2K Micron devices using
> mtd-tests and JFFS2. Error correction upto 4-bits has also been verified using
> nandwrite/nanddump utilities.
> 
> Reviewed-by: David Brownell <dbrownell@users.sourceforge.net>
> Signed-off-by: Sneha Narnakaje <nsnehaprabha@ti.com>
> ---
>  drivers/mtd/nand/davinci_nand.c |   37 +++++++++++++++++++++++++++++++------
>  1 files changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
> index ba6940d..4557b8d 100644
> --- a/drivers/mtd/nand/davinci_nand.c
> +++ b/drivers/mtd/nand/davinci_nand.c
> @@ -500,6 +500,24 @@ 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 = 40,
> +	.eccpos = { 0, 1, 2, 3, 4,
> +		/* offset 5 holds the badblock marker */

I don't see any bad block overrides to move it from bytes 0,1
to byte 5 in this patch. What am I missing?


> +		6, 7, 8, 9, 10,	11, 12, 13, 14, 15,
> +		/* 8 bytes at offset 16 hold JFFS2 clean markers */
> +		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, },
> +	},
> +};
>  
>  static int __init nand_davinci_probe(struct platform_device *pdev)
>  {
> @@ -689,15 +707,22 @@ 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->ecclayout.oobfree[1].offset instead of 49 would look better.
or move the .offset = 49 here too.

> +			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
> -		 * before reading the body of the page, to avoid
> -		 * the "infix OOB" model of NAND_ECC_HW_SYNDROME
> -		 * (and preserve manufacturer badblock markings).
> +		/* 4K page chips are not yet supported. The eccpos from
> +		 * nand_ecclayout cannot hold 80bytes and change to eccpos[]
> +		 * breaks userspace ioctl interface with mtd-utils. Once we
> +		 * resolve this issue, NAND_ECC_HW_OOB_FIRST mode can be used
> +		 * for the 4K page chips.
>  		 */
>  		dev_warn(&pdev->dev, "no 4-bit ECC support yet "
> -				"for large page NAND\n");
> +				"for 4K page NAND\n");
>  		ret = -EIO;
>  		goto err_scan;
>  

Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large page NAND chips

[Troy Kisky]

Troy Kisky wrote:
> nsnehaprabha@ti.com wrote:
>> From: Sneha Narnakaje <nsnehaprabha@ti.com>
>>
>> This patch adds 4-bit ECC support for large page NAND chips using the new ECC
>> mode NAND_ECC_HW_OOB_FIRST. The platform data from board-dm355-evm has been
>> adjusted to use this mode.
>>
>> The patches have been verified on DM355 device with 2K Micron devices using
>> mtd-tests and JFFS2. Error correction upto 4-bits has also been verified using
>> nandwrite/nanddump utilities.
>>
>> Reviewed-by: David Brownell <dbrownell@users.sourceforge.net>
>> Signed-off-by: Sneha Narnakaje <nsnehaprabha@ti.com>
>> ---
>>  drivers/mtd/nand/davinci_nand.c |   37 +++++++++++++++++++++++++++++++------
>>  1 files changed, 31 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
>> index ba6940d..4557b8d 100644
>> --- a/drivers/mtd/nand/davinci_nand.c
>> +++ b/drivers/mtd/nand/davinci_nand.c
>> @@ -689,15 +707,22 @@ 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;
> 
Most drivers set chip->ecc.layout = ... Is it ok to use ecclayout instead?????

RE: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large page NAND chips

[Narnakaje, Snehaprabha]

Troy,

> -----Original Message-----
> From: Troy Kisky [mailto:troy.kisky@boundarydevices.com]
> Sent: Monday, May 18, 2009 8:17 PM
> To: Narnakaje, Snehaprabha
> Cc: dwmw2@infradead.org; davinci-linux-open-source@linux.davincidsp.com;
> linux-mtd@lists.infradead.org; tglx@linutronix.de; akpm@linux-
> foundation.org
> Subject: Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large
> page NAND chips
> 
> Troy Kisky wrote:
> > nsnehaprabha@ti.com wrote:
> >> From: Sneha Narnakaje <nsnehaprabha@ti.com>
> >>
> >> This patch adds 4-bit ECC support for large page NAND chips using the
> new ECC
> >> mode NAND_ECC_HW_OOB_FIRST. The platform data from board-dm355-evm has
> been
> >> adjusted to use this mode.
> >>
> >> The patches have been verified on DM355 device with 2K Micron devices
> using
> >> mtd-tests and JFFS2. Error correction upto 4-bits has also been
> verified using
> >> nandwrite/nanddump utilities.
> >>
> >> Reviewed-by: David Brownell <dbrownell@users.sourceforge.net>
> >> Signed-off-by: Sneha Narnakaje <nsnehaprabha@ti.com>
> >> ---
> >>  drivers/mtd/nand/davinci_nand.c |   37
> +++++++++++++++++++++++++++++++------
> >>  1 files changed, 31 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/mtd/nand/davinci_nand.c
> b/drivers/mtd/nand/davinci_nand.c
> >> index ba6940d..4557b8d 100644
> >> --- a/drivers/mtd/nand/davinci_nand.c
> >> +++ b/drivers/mtd/nand/davinci_nand.c
> >> @@ -689,15 +707,22 @@ 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;
> >
> Most drivers set chip->ecc.layout = ... Is it ok to use ecclayout
> instead?????

info->ecclayout is internal to davinci nand driver, since info is of type 'struct davinci_nand_info'.

In the davinci_nand.c source, there is already code to assign info->ecclayout to chip->ecc.layout.

syndrome_done:
                info->chip.ecc.layout = &info->ecclayout;

We have this internal structure to handle different ecc modes (ecc bits vs. page sizes).

Thanks
Sneha

> 
> 
> 

Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large page NAND chips

[Troy Kisky]

Narnakaje, Snehaprabha wrote:
> Troy,
> 
>> -----Original Message-----
>> From: Troy Kisky [mailto:troy.kisky@boundarydevices.com]
>> Sent: Monday, May 18, 2009 8:17 PM
>> To: Narnakaje, Snehaprabha
>> Cc: dwmw2@infradead.org; davinci-linux-open-source@linux.davincidsp.com;
>> linux-mtd@lists.infradead.org; tglx@linutronix.de; akpm@linux-
>> foundation.org
>> Subject: Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large
>> page NAND chips
>>
>> Troy Kisky wrote:
>>> nsnehaprabha@ti.com wrote:
>>>> From: Sneha Narnakaje <nsnehaprabha@ti.com>
>>>>
>>>> This patch adds 4-bit ECC support for large page NAND chips using the
>> new ECC
>>>> mode NAND_ECC_HW_OOB_FIRST. The platform data from board-dm355-evm has
>> been
>>>> adjusted to use this mode.
>>>>
>>>> The patches have been verified on DM355 device with 2K Micron devices
>> using
>>>> mtd-tests and JFFS2. Error correction upto 4-bits has also been
>> verified using
>>>> nandwrite/nanddump utilities.
>>>>
>>>> Reviewed-by: David Brownell <dbrownell@users.sourceforge.net>
>>>> Signed-off-by: Sneha Narnakaje <nsnehaprabha@ti.com>
>>>> ---
>>>>  drivers/mtd/nand/davinci_nand.c |   37
>> +++++++++++++++++++++++++++++++------
>>>>  1 files changed, 31 insertions(+), 6 deletions(-)
>>>>

>>>> diff --git a/drivers/mtd/nand/davinci_nand.c
>> b/drivers/mtd/nand/davinci_nand.c
>>>> index ba6940d..4557b8d 100644
>>>> --- a/drivers/mtd/nand/davinci_nand.c
>>>> +++ b/drivers/mtd/nand/davinci_nand.c
>>>> @@ -500,6 +500,24 @@ 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 = 40,
>>>> +	.eccpos = { 0, 1, 2, 3, 4,
>>>> +		/* offset 5 holds the badblock marker */
>> 
>> I don't see any bad block overrides to move it from bytes 0,1
>> to byte 5 in this patch. What am I missing?

> We are making sure we are not overwriting offset 5, that holds the badblock marker from nand manufacturer. Thus offset 5 is skipped in the eccpos.

Yes, I agree, you are. But how does Linux know where to look for the bad block marker?


>>>> @@ -689,15 +707,22 @@ 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;
>> Most drivers set chip->ecc.layout = ... Is it ok to use ecclayout
>> instead?????
> 
> info->ecclayout is internal to davinci nand driver, since info is of type 'struct davinci_nand_info'.
> 
> In the davinci_nand.c source, there is already code to assign info->ecclayout to chip->ecc.layout.
> 
> syndrome_done:
>                 info->chip.ecc.layout = &info->ecclayout;
> 
> We have this internal structure to handle different ecc modes (ecc bits vs. page sizes).
> 
> Thanks
> Sneha

Ok, this is on top of previous patches not yet accepted. I missed that.



Troy

RE: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large page NAND chips

[Narnakaje, Snehaprabha]

> -----Original Message-----
> From: Troy Kisky [mailto:troy.kisky@boundarydevices.com]
> Sent: Wednesday, May 20, 2009 3:07 PM
> To: Narnakaje, Snehaprabha
> Cc: linux-mtd@lists.infradead.org; davinci-linux-open-
> source@linux.davincidsp.com; dwmw2@infradead.org; tglx@linutronix.de;
> akpm@linux-foundation.org
> Subject: Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large
> page NAND chips
> 
> Narnakaje, Snehaprabha wrote:
> > Troy,
> >
> >> -----Original Message-----
> >> From: Troy Kisky [mailto:troy.kisky@boundarydevices.com]
> >> Sent: Monday, May 18, 2009 8:17 PM
> >> To: Narnakaje, Snehaprabha
> >> Cc: dwmw2@infradead.org; davinci-linux-open-
> source@linux.davincidsp.com;
> >> linux-mtd@lists.infradead.org; tglx@linutronix.de; akpm@linux-
> >> foundation.org
> >> Subject: Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for
> large
> >> page NAND chips
> >>
> >> Troy Kisky wrote:
> >>> nsnehaprabha@ti.com wrote:
> >>>> From: Sneha Narnakaje <nsnehaprabha@ti.com>
> >>>>
> >>>> This patch adds 4-bit ECC support for large page NAND chips using the
> >> new ECC
> >>>> mode NAND_ECC_HW_OOB_FIRST. The platform data from board-dm355-evm
> has
> >> been
> >>>> adjusted to use this mode.
> >>>>
> >>>> The patches have been verified on DM355 device with 2K Micron devices
> >> using
> >>>> mtd-tests and JFFS2. Error correction upto 4-bits has also been
> >> verified using
> >>>> nandwrite/nanddump utilities.
> >>>>
> >>>> Reviewed-by: David Brownell <dbrownell@users.sourceforge.net>
> >>>> Signed-off-by: Sneha Narnakaje <nsnehaprabha@ti.com>
> >>>> ---
> >>>>  drivers/mtd/nand/davinci_nand.c |   37
> >> +++++++++++++++++++++++++++++++------
> >>>>  1 files changed, 31 insertions(+), 6 deletions(-)
> >>>>
> 
> >>>> diff --git a/drivers/mtd/nand/davinci_nand.c
> >> b/drivers/mtd/nand/davinci_nand.c
> >>>> index ba6940d..4557b8d 100644
> >>>> --- a/drivers/mtd/nand/davinci_nand.c
> >>>> +++ b/drivers/mtd/nand/davinci_nand.c
> >>>> @@ -500,6 +500,24 @@ 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 = 40,
> >>>> +	.eccpos = { 0, 1, 2, 3, 4,
> >>>> +		/* offset 5 holds the badblock marker */
> >>
> >> I don't see any bad block overrides to move it from bytes 0,1
> >> to byte 5 in this patch. What am I missing?
> 
> > We are making sure we are not overwriting offset 5, that holds the
> badblock marker from nand manufacturer. Thus offset 5 is skipped in the
> eccpos.
> 
> Yes, I agree, you are. But how does Linux know where to look for the bad
> block marker?

I believe, it is defined in the nand_bbt.c.

BTW, looking closely at the nand_bbt.c, looks like I made a mistake :-)

static struct nand_bbt_descr smallpage_flashbased = {
        .options = NAND_BBT_SCAN2NDPAGE,
        .offs = 5,
        .len = 1,
        .pattern = scan_ff_pattern
};

static struct nand_bbt_descr largepage_flashbased = {
        .options = NAND_BBT_SCAN2NDPAGE,
        .offs = 0,
        .len = 2,
        .pattern = scan_ff_pattern
};

Since my patches are to support large page, Shouldn't I be skipping 2 bytes (offset 0 and 1)?

The 1 byte at offset 5 was for smallpage.

Thanks
Sneha

Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large page NAND chips

[Troy Kisky]

Narnakaje, Snehaprabha wrote:
> 
>> -----Original Message-----
>> From: Troy Kisky [mailto:troy.kisky@boundarydevices.com]
>> Sent: Wednesday, May 20, 2009 3:07 PM
>> To: Narnakaje, Snehaprabha
>> Cc: linux-mtd@lists.infradead.org; davinci-linux-open-
>> source@linux.davincidsp.com; dwmw2@infradead.org; tglx@linutronix.de;
>> akpm@linux-foundation.org
>> Subject: Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large
>> page NAND chips
>>
>> Narnakaje, Snehaprabha wrote:
>>> Troy,
>>>
>>>> -----Original Message-----
>>>> From: Troy Kisky [mailto:troy.kisky@boundarydevices.com]
>>>> Sent: Monday, May 18, 2009 8:17 PM
>>>> To: Narnakaje, Snehaprabha
>>>> Cc: dwmw2@infradead.org; davinci-linux-open-
>> source@linux.davincidsp.com;
>>>> linux-mtd@lists.infradead.org; tglx@linutronix.de; akpm@linux-
>>>> foundation.org
>>>> Subject: Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for
>> large
>>>> page NAND chips
>>>>
>>>> Troy Kisky wrote:
>>>>> nsnehaprabha@ti.com wrote:
>>>>>> From: Sneha Narnakaje <nsnehaprabha@ti.com>
>>>>>>
>>>>>> This patch adds 4-bit ECC support for large page NAND chips using the
>>>> new ECC
>>>>>> mode NAND_ECC_HW_OOB_FIRST. The platform data from board-dm355-evm
>> has
>>>> been
>>>>>> adjusted to use this mode.
>>>>>>
>>>>>> The patches have been verified on DM355 device with 2K Micron devices
>>>> using
>>>>>> mtd-tests and JFFS2. Error correction upto 4-bits has also been
>>>> verified using
>>>>>> nandwrite/nanddump utilities.
>>>>>>
>>>>>> Reviewed-by: David Brownell <dbrownell@users.sourceforge.net>
>>>>>> Signed-off-by: Sneha Narnakaje <nsnehaprabha@ti.com>
>>>>>> ---
>>>>>>  drivers/mtd/nand/davinci_nand.c |   37
>>>> +++++++++++++++++++++++++++++++------
>>>>>>  1 files changed, 31 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mtd/nand/davinci_nand.c
>>>> b/drivers/mtd/nand/davinci_nand.c
>>>>>> index ba6940d..4557b8d 100644
>>>>>> --- a/drivers/mtd/nand/davinci_nand.c
>>>>>> +++ b/drivers/mtd/nand/davinci_nand.c
>>>>>> @@ -500,6 +500,24 @@ 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 = 40,
>>>>>> +	.eccpos = { 0, 1, 2, 3, 4,
>>>>>> +		/* offset 5 holds the badblock marker */
>>>> I don't see any bad block overrides to move it from bytes 0,1
>>>> to byte 5 in this patch. What am I missing?
>>> We are making sure we are not overwriting offset 5, that holds the
>> badblock marker from nand manufacturer. Thus offset 5 is skipped in the
>> eccpos.
>>
>> Yes, I agree, you are. But how does Linux know where to look for the bad
>> block marker?
> 
> I believe, it is defined in the nand_bbt.c.
> 
> BTW, looking closely at the nand_bbt.c, looks like I made a mistake :-)
> 
> static struct nand_bbt_descr smallpage_flashbased = {
>         .options = NAND_BBT_SCAN2NDPAGE,
>         .offs = 5,
>         .len = 1,
>         .pattern = scan_ff_pattern
> };
> 
> static struct nand_bbt_descr largepage_flashbased = {
>         .options = NAND_BBT_SCAN2NDPAGE,
>         .offs = 0,
>         .len = 2,
>         .pattern = scan_ff_pattern
> };
> 
> Since my patches are to support large page, Shouldn't I be skipping 2 bytes (offset 0 and 1)?
> 
> The 1 byte at offset 5 was for smallpage.
> 
> Thanks
> Sneha
> 
> 

Yes, that is what I thought. If you can place the ecc anywhere, please put it at the end of the oob data
for large page devices.


Troy

RE: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large page NAND chips

[Narnakaje, Snehaprabha]

> -----Original Message-----
> From: Troy Kisky [mailto:troy.kisky@boundarydevices.com]
> Sent: Wednesday, May 20, 2009 4:19 PM
> To: Narnakaje, Snehaprabha
> Cc: linux-mtd@lists.infradead.org; davinci-linux-open-
> source@linux.davincidsp.com; dwmw2@infradead.org; tglx@linutronix.de;
> akpm@linux-foundation.org
> Subject: Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large
> page NAND chips
> 
> Narnakaje, Snehaprabha wrote:
> >
> >> -----Original Message-----
> >> From: Troy Kisky [mailto:troy.kisky@boundarydevices.com]
> >> Sent: Wednesday, May 20, 2009 3:07 PM
> >> To: Narnakaje, Snehaprabha
> >> Cc: linux-mtd@lists.infradead.org; davinci-linux-open-
> >> source@linux.davincidsp.com; dwmw2@infradead.org; tglx@linutronix.de;
> >> akpm@linux-foundation.org
> >> Subject: Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for
> large
> >> page NAND chips
> >>
> >> Narnakaje, Snehaprabha wrote:
> >>> Troy,
> >>>
> >>>> -----Original Message-----
> >>>> From: Troy Kisky [mailto:troy.kisky@boundarydevices.com]
> >>>> Sent: Monday, May 18, 2009 8:17 PM
> >>>> To: Narnakaje, Snehaprabha
> >>>> Cc: dwmw2@infradead.org; davinci-linux-open-
> >> source@linux.davincidsp.com;
> >>>> linux-mtd@lists.infradead.org; tglx@linutronix.de; akpm@linux-
> >>>> foundation.org
> >>>> Subject: Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for
> >> large
> >>>> page NAND chips
> >>>>
> >>>> Troy Kisky wrote:
> >>>>> nsnehaprabha@ti.com wrote:
> >>>>>> From: Sneha Narnakaje <nsnehaprabha@ti.com>
> >>>>>>
> >>>>>> This patch adds 4-bit ECC support for large page NAND chips using
> the
> >>>> new ECC
> >>>>>> mode NAND_ECC_HW_OOB_FIRST. The platform data from board-dm355-evm
> >> has
> >>>> been
> >>>>>> adjusted to use this mode.
> >>>>>>
> >>>>>> The patches have been verified on DM355 device with 2K Micron
> devices
> >>>> using
> >>>>>> mtd-tests and JFFS2. Error correction upto 4-bits has also been
> >>>> verified using
> >>>>>> nandwrite/nanddump utilities.
> >>>>>>
> >>>>>> Reviewed-by: David Brownell <dbrownell@users.sourceforge.net>
> >>>>>> Signed-off-by: Sneha Narnakaje <nsnehaprabha@ti.com>
> >>>>>> ---
> >>>>>>  drivers/mtd/nand/davinci_nand.c |   37
> >>>> +++++++++++++++++++++++++++++++------
> >>>>>>  1 files changed, 31 insertions(+), 6 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/mtd/nand/davinci_nand.c
> >>>> b/drivers/mtd/nand/davinci_nand.c
> >>>>>> index ba6940d..4557b8d 100644
> >>>>>> --- a/drivers/mtd/nand/davinci_nand.c
> >>>>>> +++ b/drivers/mtd/nand/davinci_nand.c
> >>>>>> @@ -500,6 +500,24 @@ 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 = 40,
> >>>>>> +	.eccpos = { 0, 1, 2, 3, 4,
> >>>>>> +		/* offset 5 holds the badblock marker */
> >>>> I don't see any bad block overrides to move it from bytes 0,1
> >>>> to byte 5 in this patch. What am I missing?
> >>> We are making sure we are not overwriting offset 5, that holds the
> >> badblock marker from nand manufacturer. Thus offset 5 is skipped in the
> >> eccpos.
> >>
> >> Yes, I agree, you are. But how does Linux know where to look for the
> bad
> >> block marker?
> >
> > I believe, it is defined in the nand_bbt.c.
> >
> > BTW, looking closely at the nand_bbt.c, looks like I made a mistake :-)
> >
> > static struct nand_bbt_descr smallpage_flashbased = {
> >         .options = NAND_BBT_SCAN2NDPAGE,
> >         .offs = 5,
> >         .len = 1,
> >         .pattern = scan_ff_pattern
> > };
> >
> > static struct nand_bbt_descr largepage_flashbased = {
> >         .options = NAND_BBT_SCAN2NDPAGE,
> >         .offs = 0,
> >         .len = 2,
> >         .pattern = scan_ff_pattern
> > };
> >
> > Since my patches are to support large page, Shouldn't I be skipping 2
> bytes (offset 0 and 1)?
> >
> > The 1 byte at offset 5 was for smallpage.
> >
> > Thanks
> > Sneha
> >
> >
> 
> Yes, that is what I thought. If you can place the ecc anywhere, please put
> it at the end of the oob data
> for large page devices.

Troy,

I explored nand_bbt, jffs2 using the OOB area.

This is what I found -
2 bytes at offset 0: BB markers
4 bytes at offset 8: BBT magic number/data
1 byte at offset 12: BBT version
8 bytes at offset 16: JFFS2 clear markers

So, it is better to start with offset 24 (there are few bytes in between, but it is better to leave them as-is).

Dave, 

Did you have any comments?

Thanks
Sneha


> 
> 
> Troy

Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large page NAND chips

[Troy Kisky]

> 
> I explored nand_bbt, jffs2 using the OOB area.
> 
> This is what I found -
> 2 bytes at offset 0: BB markers
> 4 bytes at offset 8: BBT magic number/data
> 1 byte at offset 12: BBT version
> 8 bytes at offset 16: JFFS2 clear markers
> 
> So, it is better to start with offset 24 (there are few bytes in between, but it is better to leave them as-is).
> 
> Dave, 
> 
> Did you have any comments?
> 
> Thanks
> Sneha


If you have to move the ECC anyway, there really isn't a further compatibility
problem with moving the JFFS2 bytes as well.

0-1 : BB marker
2-23 : Free space (JFFS2 will use)
24-63 : 4 x 10 bytes of ecc.


This way, if the default ecc layout patch that I submitted is ever accepted, the code in this
patch can be removed.

Troy

RE: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large page NAND chips

[Narnakaje, Snehaprabha]

Troy,

Thank you for reviewing these patches.

> -----Original Message-----
> From: Troy Kisky [mailto:troy.kisky@boundarydevices.com]
> Sent: Monday, May 18, 2009 7:58 PM
> To: Narnakaje, Snehaprabha
> Cc: linux-mtd@lists.infradead.org; davinci-linux-open-
> source@linux.davincidsp.com; dwmw2@infradead.org; tglx@linutronix.de;
> akpm@linux-foundation.org
> Subject: Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large
> page NAND chips
> 
> nsnehaprabha@ti.com wrote:
> > From: Sneha Narnakaje <nsnehaprabha@ti.com>
> >
> > This patch adds 4-bit ECC support for large page NAND chips using the
> new ECC
> > mode NAND_ECC_HW_OOB_FIRST. The platform data from board-dm355-evm has
> been
> > adjusted to use this mode.
> >
> > The patches have been verified on DM355 device with 2K Micron devices
> using
> > mtd-tests and JFFS2. Error correction upto 4-bits has also been verified
> using
> > nandwrite/nanddump utilities.
> >
> > Reviewed-by: David Brownell <dbrownell@users.sourceforge.net>
> > Signed-off-by: Sneha Narnakaje <nsnehaprabha@ti.com>
> > ---
> >  drivers/mtd/nand/davinci_nand.c |   37 +++++++++++++++++++++++++++++++-
> -----
> >  1 files changed, 31 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/davinci_nand.c
> b/drivers/mtd/nand/davinci_nand.c
> > index ba6940d..4557b8d 100644
> > --- a/drivers/mtd/nand/davinci_nand.c
> > +++ b/drivers/mtd/nand/davinci_nand.c
> > @@ -500,6 +500,24 @@ 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 = 40,
> > +	.eccpos = { 0, 1, 2, 3, 4,
> > +		/* offset 5 holds the badblock marker */
> 
> I don't see any bad block overrides to move it from bytes 0,1
> to byte 5 in this patch. What am I missing?

We are making sure we are not overwriting offset 5, that holds the badblock marker from nand manufacturer. Thus offset 5 is skipped in the eccpos.

> 
> 
> > +		6, 7, 8, 9, 10,	11, 12, 13, 14, 15,
> > +		/* 8 bytes at offset 16 hold JFFS2 clean markers */
> > +		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, },
> > +	},
> > +};
> >
> >  static int __init nand_davinci_probe(struct platform_device *pdev)
> >  {
> > @@ -689,15 +707,22 @@ 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->ecclayout.oobfree[1].offset instead of 49 would look better.
> or move the .offset = 49 here too.

OK, info->ecclayout.oobfree[1].offset can be used. I would like to keep the .offset = 49 in the nand_ecclayout structure, so that it relates to the eccpos.

Thanks
Sneha

> 
> > +			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
> > -		 * before reading the body of the page, to avoid
> > -		 * the "infix OOB" model of NAND_ECC_HW_SYNDROME
> > -		 * (and preserve manufacturer badblock markings).
> > +		/* 4K page chips are not yet supported. The eccpos from
> > +		 * nand_ecclayout cannot hold 80bytes and change to eccpos[]
> > +		 * breaks userspace ioctl interface with mtd-utils. Once we
> > +		 * resolve this issue, NAND_ECC_HW_OOB_FIRST mode can be used
> > +		 * for the 4K page chips.
> >  		 */
> >  		dev_warn(&pdev->dev, "no 4-bit ECC support yet "
> > -				"for large page NAND\n");
> > +				"for 4K page NAND\n");
> >  		ret = -EIO;
> >  		goto err_scan;
> >
> 

Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large page NAND chips

[David Brownell]

Minor comment:  $SUBJECT should say it adds this "for DaVinci".  ;)

To recap other discussion (I hope), this needs to be reissued since:

On Monday 18 May 2009, nsnehaprabha@ti.com wrote:
> +static struct nand_ecclayout hwecc4_2048 __initconst = {
> +       .eccbytes = 40,
> +       .eccpos = { 0, 1, 2, 3, 4,

Byte 0 -- for 8-bit large-page parts which need this patch -- is
the manufacturer bad block marker, and the idea was to *not* clobber
it.  Those bytes need to be skipped.

Will the 40 bytes of ECC data now be stored at offsets 24..63,
matching Troy's patches?

	0..1	manufacturer badblock markers
	8..11	BBT marker ("free" oob data)
	16..23	JFFS2 stuff ("free" oob data)
	24..63	ECC data

And for 4KB pages, it would be the same -- except that
there would be 80 bytes of ECC data, from 48..127 (too
much to fit into the eccpos array).


> +               /* offset 5 holds the badblock marker */
> +               6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
> +               /* 8 bytes at offset 16 hold JFFS2 clean markers */
> +               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, },
> +       },
> +};

RE: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large page NAND chips

[Narnakaje, Snehaprabha]

Hi,

I will be coming up with an updated patch for this (3/3) early next week to address -
1. Corrections to eccpos[]
2. Comments from Troy

Thanks
Sneha

> -----Original Message-----
> From: David Brownell [mailto:david-b@pacbell.net]
> Sent: Wednesday, May 20, 2009 8:38 PM
> To: davinci-linux-open-source@linux.davincidsp.com; Narnakaje, Snehaprabha
> Cc: linux-mtd@lists.infradead.org; dwmw2@infradead.org;
> tglx@linutronix.de; akpm@linux-foundation.org
> Subject: Re: [PATCH 2.6.30-rc6 3/3] NAND: Add 4-bit ECC support for large
> page NAND chips
> 
> Minor comment:  $SUBJECT should say it adds this "for DaVinci".  ;)
> 
> To recap other discussion (I hope), this needs to be reissued since:
> 
> On Monday 18 May 2009, nsnehaprabha@ti.com wrote:
> > +static struct nand_ecclayout hwecc4_2048 __initconst = {
> > +       .eccbytes = 40,
> > +       .eccpos = { 0, 1, 2, 3, 4,
> 
> Byte 0 -- for 8-bit large-page parts which need this patch -- is
> the manufacturer bad block marker, and the idea was to *not* clobber
> it.  Those bytes need to be skipped.
> 
> Will the 40 bytes of ECC data now be stored at offsets 24..63,
> matching Troy's patches?
> 
> 	0..1	manufacturer badblock markers
> 	8..11	BBT marker ("free" oob data)
> 	16..23	JFFS2 stuff ("free" oob data)
> 	24..63	ECC data
> 
> And for 4KB pages, it would be the same -- except that
> there would be 80 bytes of ECC data, from 48..127 (too
> much to fit into the eccpos array).
> 
> 
> > +               /* offset 5 holds the badblock marker */
> > +               6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
> > +               /* 8 bytes at offset 16 hold JFFS2 clean markers */
> > +               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, },
> > +       },
> > +};
> 
>