| Message ID | 1242682705-19845-1-git-send-email-nsnehaprabha@ti.com |
|---|---|
| State | New, archived |
| Headers | show |
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; >
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?????
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; > > >
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 > > >
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
> -----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
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
> -----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
> > 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
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, }, > + }, > +};
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, }, > > + }, > > +}; > >
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;