Re: [PATCH v5] mtd: gpmi: Deal with bitflips in erased regions regions

[Huang Shijie <shijie8@gmail.com>]

On Tue, Dec 17, 2013 at 02:45:42PM +0100, Elie De Brauwer wrote:
>  
> +	/* Set the tolerance for bitflips when reading erased blocks. */
> +	erase_threshold = gf_len / 2;
> +	if (erase_threshold > ecc_strength)
> +		erase_threshold = ecc_strength;
> +
I was about to give you my ACK, but i find you used a wrong ecc strength
here.  The "ecc_strength" is just half of the real ECC strength used by
the BCH. Please read this line in the function:
268	ecc_strength  = bch_geo->ecc_strength >> 1;

Could you please send a new version patch ?


> +	writel(erase_threshold & BM_BCH_MODE_ERASE_THRESHOLD_MASK,
> +		r->bch_regs + HW_BCH_MODE);
> +
>  	/* Set *all* chip selects to use layout 0. */
>  	writel(0, r->bch_regs + HW_BCH_LAYOUTSELECT);
>  
> @@ -1094,6 +1103,15 @@ int gpmi_is_ready(struct gpmi_nand_data *this, unsigned chip)
>  	return reg & mask;
>  }
>  
> +/*
> + * Count the number of 0 bits in a supposed to be
> + * erased region and correct them. Return the number
> + * of bitflips or zero when the region was correct.
> + */
> +static unsigned int erased_sector_bitflips(unsigned char *data,
> +					unsigned int chunk,
> +					struct bch_geometry *geo)
> +{
> +	unsigned int flip_bits = 0;
> +	int i;
> +	int base = geo->ecc_chunk_size * chunk;
> +
> +	/* Count bitflips */
> +	for (i = 0; i < geo->ecc_chunk_size; i++)
> +		flip_bits += hweight8(~data[base + i]);
> +
> +	/* Correct bitflips by 0xFF'ing this chunk. */
> +	if (flip_bits)
> +		memset(&data[base], 0xFF, geo->ecc_chunk_size);
> +
> +	return flip_bits;
> +}

Since a new version patch is inevitable, i want to give more comment
about this function.


Does the following code run faster then above?
static unsigned int erased_sector_bitflips(unsigned char *data,
					unsigned int chunk,
					struct bch_geometry *geo)
{
	unsigned int flip_bits = 0;
	int i;
	int base = geo->ecc_chunk_size * chunk;
	int tmp;

	for (i = 0; i < geo->ecc_chunk_size; i++) {
		tmp = hweight8(~data[base + i]);

		if (tmp) {
			data[base + i] = 0xff;
			flip_bits += tmp;
		}
	}

	return flip_bits;
}

I am not sure this code is faster then your code, i do not have time to
do a test to compare the two functions.

If you think your function is better, just ignore my code, it is okay to
me.

I really very appreciate at your work!

thanks
Huang Shijie