Date: Tue, 7 Apr 2009 21:48:14 +0200 (CEST)
From: Thomas Gleixner <tglx@linutronix.de>
To: "Narnakaje, Snehaprabha" <nsnehaprabha@ti.com>
cc: David Brownell <david-b@pacbell.net>,
       "davinci-linux-open-source@linux.davincidsp.com" 
	<davinci-linux-open-source@linux.davincidsp.com>,
       "linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
       "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] MTD-NAND: Changes to read_page APIs to support
 NAND_ECC_HW_SYNDROME mode on TI DaVinci DM355
In-Reply-To: <7A436F7769CA33409C6B44B358BFFF0CFFAA9253@dlee02.ent.ti.com>
Message-ID: <alpine.LFD.2.00.0904072123320.21577@localhost.localdomain>
References: <1238601784-9563-1-git-send-email-nsnehaprabha@ti.com> <200904051556.05368.david-b@pacbell.net> <alpine.LFD.2.00.0904070042300.21577@localhost.localdomain> <200904061849.23110.david-b@pacbell.net> <7A436F7769CA33409C6B44B358BFFF0CFFAA90C0@dlee02.ent.ti.com>
 <alpine.LFD.2.00.0904071831550.21577@localhost.localdomain> <7A436F7769CA33409C6B44B358BFFF0CFFAA9253@dlee02.ent.ti.com>
User-Agent: Alpine 2.00 (LFD 1167 2008-08-23)
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3535
Lines: 83

On Tue, 7 Apr 2009, Narnakaje, Snehaprabha wrote:
> > > 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 ?

Your mailer still does not insert line breaks at around 78 chars.
 
> 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.

That's exactly what NAND_ECC_HW does.

> > 
> > 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.

Ok.

> > 
> > > 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,

No, it does not fit 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

Those are not bugs. You abused that interface and now you claim it has
bugs. It does _not_. You introduced the bugs by using a function for
something it was never designed for.

> 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 is the wrong approach. 

What you need is a NAND_ECC_HW_OOB_FIRST mode, which uses the
NAND_ECC_HW write function and implements a separate read function
which reads the oob and then reads the data chunks.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/