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

[Narnakaje, Snehaprabha]

> -----Original Message-----
> From: David Brownell [mailto:david-b at pacbell.net]
> Sent: Thursday, May 07, 2009 1:11 PM
> To: Narnakaje, Snehaprabha
> Cc: davinci-linux-open-source at linux.davincidsp.com; linux-
> mtd at lists.infradead.org; dwmw2 at infradead.org; tglx at linutronix.de;
> akpm at linux-foundation.org
> Subject: Re: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large
> page NAND chips
> 
> On Thursday 07 May 2009, Narnakaje, Snehaprabha wrote:
> 
> > > 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.
> >
> > Yes, I had to use the .calculate in the new read_page handler
> > to stop the ECC though.
> 
> Oh, now I see what you were talking about off-list.  That
> doesn't seem quite right.  I'll take a look.

Looks like you had to implement is_readmode option in the .calculate function, to use the NAND_ECC_HW mode for small pages. The read_page handler for NAND_ECC_HW calls the .calculate function, and in our case we shouldn't be doing anything here for read. We use the .calculate function from write_page.

> 
> 
> > > 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;
> >
> > Sorry, I didn't realize this. I will separate the board-dm355-evm.c
> > from this patch and submit the patch #3 for board-dm355-evm.c to go
> > to davinci-linux-open-source.
> 
> Let's wait until we see the previous davinci_nand.c patches get
> queued up somewhere "official" though ... in fact, I don't see
> why they shouldn't merge for 2.6.30-soon.

I was hoping my patch (dependent on your patch set) would at least trigger the review/acceptance :-)

> 
> 
> > > (b) as noted elsewhere, there are still issues with 4K pages
> > > and the NAND core infrastructure.
> >
> > Yes, this is still an issue, if we make the read_page handler
> > use .eccpos[] for positioning the ECC bytes in the OOB area.
> > If we had fixed prepad+ecc nand_ecclayout we would avoid using
> > eccpos[] (This is what my initial set of 4-bit ECC patches did
> > to support 4K). But this is not a generic solution.
> >
> > The main problem is with nand_ooblayout structure that is not
> > extendable for large pages 4K or more, without breaking the
> > userspace IOCTLs that is dependent on the size of this structure.
> >
> > Question to linux-mtd list: Are there plans in the linux-mtd to
> > address this generic issue, now that NAND manufacturers are
> > coming up NAND chips > 4K page size?
> 
> They've been doing it for a while now, actually.  :)
> 
> And it wouldn't be an issue here ... *IF* this 4-bit ECC didn't
> need to use 80 bytes (8 chunks of 512 bytes, 10 bytes ECC per
> chunk) from a 64 byte field.  Doesn't fit.
> 
> Maybe the ecclayout stuff should migrate to sysfs, and just
> drop the old ioctl stuff for parts that can't use it ... as
> is being done to support chips that are 4 GByte and bigger.

I will explore more on this option.

> 
> 
> > > > +static struct nand_ecclayout hwecc4_2048 __initconst = {
> > > > +       .eccbytes = 10,
> > >
> > > Not ".eccbytes = 40"?  This is 4 chunks, 10 ecc bytes each...
> >
> > No, .eccbytes is for each chips->ecc.steps.
> 
> So it's the same as ecc.bytes, which is not exported to userspace.
> 
> 
> > And all nand read/write
> > APIs, we handle ecc.steps (for loop). There is chips->ecc.total that
> > is initialized as (chips->ecc.steps * chips->ecc.bytes).
> >
> > It is strange that .eccbytes is for each chunk, while eccpos[] and
> > .oobfree[] have to handle/cover all chunks.
> 
> Also strange:  when ECC_HW_SYNDROME is in use, nothing even
> looks at the nand_ecclayout to make sure it matches the actual
> layout being used ... which derives entirely from the prepad,
> postpad, and bytes fields of "ecc".  (Except that some of the
> OOB bytes not used for ECC may be hidden, e.g. manufacturer
> bad block markers are usually not listed.)

If you leave enough prepad to cover manufacturer's BB and JFFS2 markers, this mechanism was OK, I guess.

Thanks
Sneha

> 
> - Dave
> 
> 
>