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

[Troy Kisky <troy.kisky@boundarydevices.com>]

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