[PATCH v3 1/4] mtd: nand: omap: optimized chip->ecc.correct() for H/W ECC schemes

[Gupta, Pekon]

> From: Brian Norris [mailto:computersforpeace at gmail.com]
> + Philip and Ivan, who did relevant work
> 
Including Avinash Philip's new email-id... <avinashphilipk at gmail.com>


> > On Sat, Nov 02, 2013 at 03:16:13PM +0530, Pekon Gupta wrote:
> > chip->ecc.correct() is used for detecting and correcting bit-flips during read
> > operations. In omap2-nand driver this is done usingt following functions:
> >
> > - omap_correct_data(): for H/W based HAM1_ECC schemes
> > 	(Un-Touched in current patch)
> >
> > - omap_elm_correct_data(): for H/W based BCHx_ECC scheme
> > 	Current implementation of this function is not scalable for newer ECC
> > 	schemes because:
> > 	- It depends on a specific byte-position in OOB area (reserved as
> 0x00)
> > 	  to differentiates between programmed-pages and erased-pages.
> > 	  This reserved byte-position cannot be accomodated in all ECC
> schemes.
> > 	- Current code is not scalable for future ECC schemes due to tweaks
> for
> > 	  BCH4_ECC and BCH8_ECC at multiple places.
> > 	- It checks for bit-flips in Erased-pages using check_erased_page().
> > 	  This is over-work, as sanity of Erased-page can be verified by just
> > 	  comparing them to a pre-defined ECC-syndrome for all_0xFF data.
> >
> > 	This patch optimizes omap_elm_correct_data() in following ways:
> > 	(1) Removes dependency on specific reserved-byte (0x00) in OOB
> area,
> > 	    instead Erased-page is identified by matching calc_ecc with a
> > 	    pre-defined ECC syndrome of all(0xFF) data
> > 	(2) merges common code for BCH4_ECC and BCH8_ECC for scalability.
> > 	(3) handles incorrect elm_error_location beyond data+oob buffer.
> > 	(4) removes check_erased_page(): Bit-flips in erased-page are
> handled
> > 	    in same way as for programmed-page
> 
> Are you sure that this new scheme is equivalent to the old? i.e., is
> this backwards compatible?
>
Yes, this is backward compatible. I'm also testing it using old boot-loaders.
With this patch NAND boots (ROM-> boot-loader (SPL + u-boot) -> kernel).

[...]

>  Are you totally dropping the "reserved byte"  marker?
> 
No, The "reserved byte" is part of ecc.layout which is defined in ROM code.
(please refer AM335x TRM [1] (b) Figure which explains ECC layout)

I'm just replacing below check in which driver determines whether page is
programmed or not based on number of '0' bits present in "reserved" byte.
> > -	/*
> > -	 * Check data area is programmed by counting
> > -	 * number of 0's at fixed offset in spare area.
> > -	 * Checking count of 0's against threshold.
> > -	 * In case programmed page expects at least threshold
> > -	 * zeros in byte.
> > -	 * If zeros are less than threshold for programmed page/
> > -	 * zeros are more than threshold erased page, either
> > -	 * case page reported as uncorrectable.
> > -	 */
> > -	if (hweight8(~read_ecc[eccsize]) >= threshold) {
> > -		/*
> > -		 * Update elm error vector as
> > -		 * data area is programmed
> > -		 */
> > -		err_vec[i].error_reported = true;
> > -		is_error_reported = true;
> > -	} else {
> > -		/* Error reported in erased page */
> > -		int bitflip_count;
> > -		u_char *buf = &data[info->nand.ecc.size * i];
> > -
> > -		if (memcmp(calc_ecc, erased_ecc_vec, eccsize)) {
> > -			bitflip_count = erased_sector_bitflips(
> > -					buf, read_ecc, info);

Also, based on following discussion, erased pages also need to be checked
for bit-flips.
https://lkml.org/lkml/2012/10/5/288

So instead of depending on "reserved" byte marker in OOB area, 
I'm identifying the erased page based 
if ECC_of(read_data + read_ecc) == ECC_of( all 0xff * (ecc.size + ecc.bytes))
	 then page is erased.

[...]

> Is this compatible with your bootloaders? I don't have a very
> clear mental picture of exactly what your driver is doing to check
> programmed/erased pages, and I'm certainly not familiar with your
> bootloader/ROM code.
> 
Pointing to AM335x TRM which I'm referring for this ..
- http://www.ti.com/product/am3359
- [1] latest AM3359 TRM  http://www.ti.com/litv/pdf/spruh73i

(a) Section: 26.1.7.4 NAND
	This section explains ROM code behavior for booting from NAND.
	ROM code also fetches device geometry from ONFI params.
	In addition there is a separate boot mode (called NANDI2C) where
	device parameters can be fetched from on-board EEPROM.

(b) Figure 26-15. ECC Data Mapping for 2 KB Page and 8b BCH Encoding
	This figure explains ecc layout used by ROM code, which is what
	I'm using as reference for BCH8_HW algorithm.


with regards, pekon