Re: [PATCH] mtd: nand_omap_gpmc: fix BCH error correction

[Daniel Schultz <d.schultz@phytec.de>]

Hi Matt,

could you give me some more informations about how you figured out this 
problem and how you tested it?

The Linux kernel and Barebox differs a lot in this driver. Your 
conditional assignment of eccsteps is from the omap_correct_data 
function, which will only be called if Hamming error correction is used, 
but BCH uses either omap_elm_correct_data or nand_bch_correct_data. 
These two function use the old way of getting eccsteps.

After I applied your patch, only the first 512 Bytes of a page was 
corrected (I made omap_correct_bch more verbose), before that the whole 
page was corrected. BCH in Barebox (which is default by the DTS) can 
only use the ROMCODE one, which has a default ECC size of 512 Bytes. So, 
eccsteps would always be 1, instead of 4.

Since hamming is using the omap_correct_hamming function, maybe this 
patch isn't needed?

-- 
Mit freundlichen Grüßen,
With best regards,
   Daniel Schultz

Am 14.04.2017 um 20:32 schrieb Matt Reimer:
> BCH error detection and correction was only looking at the first
> of four syndrome polynomials, which meant it was failing to detect
> and correct bitflips in the last 3/4 of the data. In effect, only
> the first 512 bytes of a 2048 byte page were being protected by ECC.
> 
> The syndrome polynomials (BCH error codes) are stored in the NAND's
> OOB, each of which protects 512 bytes of data. The driver used
> eccsteps = 1 which effectively made it only use the first polynomial,
> and therefore was only protecting the first 512 bytes of the page.
> 
> The fix is to pull over a bit of code from the kernel's
> omap_correct_data() that sets eccsteps = 4 when the page size is 2048
> bytes and hardware ECC is being used.
> 
> Signed-off-by: Matt Reimer <mreimer@sdgsystems.com>
> ---
>   drivers/mtd/nand/nand_omap_gpmc.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/nand_omap_gpmc.c b/drivers/mtd/nand/nand_omap_gpmc.c
> index 9d9d27e..2fe6a10 100644
> --- a/drivers/mtd/nand/nand_omap_gpmc.c
> +++ b/drivers/mtd/nand/nand_omap_gpmc.c
> @@ -302,7 +302,8 @@ static int omap_correct_bch(struct mtd_info *mtd, uint8_t *dat,
>   	unsigned int err_loc[8];
>   	int select_4_8;
>   
> -	int eccsteps = oinfo->nand.ecc.steps;
> +        int eccsteps = (nand->ecc.mode == NAND_ECC_HW) &&
> +                (nand->ecc.size == 2048) ? 4 : 1;
>   	int eccsize = oinfo->nand.ecc.bytes;
>   
>   	switch (oinfo->ecc_mode) {
> 



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox