[PATCH] MTD-NAND: Changes to read_page APIs to support NAND_ECC_HW_SYNDROME mode on TI DaVinci DM355

[Narnakaje, Snehaprabha]

Thomas,

> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx at linutronix.de]
> Sent: Tuesday, April 07, 2009 12:46 PM
> To: Narnakaje, Snehaprabha
> Cc: David Brownell; davinci-linux-open-source at linux.davincidsp.com; linux-
> mtd at lists.infradead.org; linux-kernel at vger.kernel.org
> Subject: RE: [PATCH] MTD-NAND: Changes to read_page APIs to support
> NAND_ECC_HW_SYNDROME mode on TI DaVinci DM355
> 
> On Tue, 7 Apr 2009, Narnakaje, Snehaprabha wrote:
> > > And that's exactly what these patches do:  enable just such choices.
> > > The $SUBJECT patch would be needed to use NAND_ECC_HW with this 4-bit
> > > ECC hardware, and not be forced to "choose" the infix-OOB policy.
> >
> > I had looked read_page/write_page APIs of the NAND_ECC_HW mode to
> > see if we can use this mode for DaVinci DM355 device.
> >
> > The read_page handler for NAND_ECC_HW mode reads the data and ECC
> > from hardware for each chunk. It then reads the OOB and extracts ecc
> > code from it, before using the ECC from hardware and ecc code from
> > OOB for data correction.
> >
> > On DaVinci DM355 device, we need to pass the ecc code/syndrome
> > extracted from OOB area, in order to read the HW ECC for each data
> > chunk. This is where we differ from NAND_ECC_HW mode.
> 
> I have to admit that I'm slightly confused. Is the below description
> correct?
> 
> On write you generate the ECC via hardware and store it in the OOB
> area, right ?

Yes. For a large page (e.g. 2K+64), ECC should be stored in the 64bytes OOB region. The NAND controller on DaVinci DM355 device is capable of generating HW ECC (4-bit) for every 512bytes. This means, we have 4 eccsteps. The ECC should be generated by the HW for every 512byte chunk write and stored temporarily in a buffer until all 4 eccsteps have been tried. The ECC from the temporary buffer is then written to the OOB area.

> 
> On read you read the oob data first and extract the ECC. Then you feed
> the extracted ECC into the HW generator and read the data. Is this
> correct ?

Almost, read OOB, read data and then feed the extracted ECC into HW generator. Read the ECC status to see any correction required on the read data. Again, reading data, feeding the extracted ECC into HW generator and the correction will have to be repeated for # of times - eccsteps.

> 
> > The read_page/write_page APIs for NAND_ECC_HW_SYNDROME have the
> > other problem that David mentioned - overwriting NAND manufacturers
> > bad block meta data, when used with large page NAND chips. These
> > APIs do not handle the "eccsteps" properly. The OOB is read/written
> > after every data chunk, thus you actually have overwritten the
> > factory bad block information, when these APIs handle the last data
> > chunk. OOB should be read/written after the entire data (a page) is
> > read/written.
> 
> Again, that is _how_ the NAND_ECC_HW_SYNDROME functions work. And if
> your hardware needs a completely different mode, then we need to
> analyse what's the best solution for it.

Based on the description above, NAND_ECC_HW_SYNDROME fitted well, with the exception of overriding the read_page/write_page APIs (to fix the bugs mentioned above). I have not sent the actual NAND driver to the linux-mtd yet, since the initial version (supported only 1-bit HW ECC) submitted by David Brownell was still in review (no comments and not approved yet). While coming up with read_page API in the DaVinci NAND driver, we realized the need to pass "page" parameter. The read_page API in the DaVinci NAND driver required to call chip->cmdfunc to adjust the read location (page vs. OOB). The "page" parameter has to be passed to the chip->cmdfunc.
This patch was a "preparation" patch for the DaVinci NAND driver 4-bit ECC support. It added the "page" parameter to the "read_page" and "read_page_raw" handlers, fixed few other bugs.

> 
> Right now the provided information is way to diffuse to do that.
> 
> You provided a patch without explaining what your hardware needs and
> showing how the actual user of the modified API looks like and how the
> functionality is implemented.
> 
> Thanks,
> 
> 	tglx