Re: abuse of nand_correct_data in tmio_nand driver

[vimal singh <vimal.newwork@gmail.com>]

On Tue, Jul 21, 2009 at 8:50 PM, Atsushi Nemoto<anemo@mba.ocn.ne.jp> wrote:
> On Mon, 20 Jul 2009 10:45:20 +0530, vimal singh <vimal.newwork@gmail.com> wrote:
>> Seems like this driver may be reading 512 bytes at a times, but still
>> calculates 256-byte sector ECC. (1-bit error correction for each 256
>> bytes of data).
>> In that case, nand_correct_data() should be called twice, once for
>> each 256 byte data.
>
> But unfortunately nand_correct_data() cannot be used as is because it
> depends on chip->ecc.size (which is 512 for tmio_nand driver).
>
>> This can be handled by overriding 'chip->ecc.correct' in driver. You
>> may reuse 'nand_correct_data()' code in the driver of 256 byte sector
>> ECC.
>
> Yes, the driver can reuse nand_ecc code to implement its own
> nand_correct rountine.
>
> Or how about adding support for 6byte-ecc/512byte-data to nand_ecc.c
> to avoid code duplication?
>
> I mean something like this.  If it looks acceptable, I will prepare a
> patch including similer change to nand_calculate_ecc.

I personally do not like any HW specific implementation going into the
generic part
of the code. This particular issue is specific to your HW, so better
handle it in the
driver only.


>
> diff --git a/drivers/mtd/nand/nand_ecc.c b/drivers/mtd/nand/nand_ecc.c
> index c0cb87d..77e13c8 100644
> --- a/drivers/mtd/nand/nand_ecc.c
> +++ b/drivers/mtd/nand/nand_ecc.c
> @@ -425,14 +425,12 @@ EXPORT_SYMBOL(nand_calculate_ecc);
>  *
>  * Detect and correct a 1 bit error for 256/512 byte block
>  */
> -int nand_correct_data(struct mtd_info *mtd, unsigned char *buf,
> -                     unsigned char *read_ecc, unsigned char *calc_ecc)
> +static int nand_correct_data_sub(unsigned char *buf,
> +       unsigned char *read_ecc, unsigned char *calc_ecc,
> +       const uint32_t eccsize_mult)
>  {
>        unsigned char b0, b1, b2, bit_addr;
>        unsigned int byte_addr;
> -       /* 256 or 512 bytes/ecc  */
> -       const uint32_t eccsize_mult =
> -                       (((struct nand_chip *)mtd->priv)->ecc.size) >> 8;
>
>        /*
>         * b0 to b2 indicate which bit is faulty (if any)
> @@ -495,6 +493,26 @@ int nand_correct_data(struct mtd_info *mtd, unsigned char *buf,
>        printk(KERN_ERR "uncorrectable error : ");
>        return -1;
>  }
> +int nand_correct_data(struct mtd_info *mtd, unsigned char *buf,
> +                     unsigned char *read_ecc, unsigned char *calc_ecc)
> +{
> +       struct nand_chip *chip = mtd->priv;
> +       /* 256 or 512 bytes/ecc  */
> +       const uint32_t eccsize_mult = (chip->ecc.size) >> 8;
> +       int r0, r1;
> +
> +       if (eccsize_mult == 2 && chip->ecc.bytes == 6) {
> +               r0 = nand_correct_data_sub(buf, read_ecc, calc_ecc, 1);
> +               if (r0 < 0)
> +                       return r0;
> +               r1 = nand_correct_data_sub(buf + 256,
> +                                          read_ecc + 3, calc_ecc + 3, 1);
> +               if (r1 < 0)
> +                       return r1;
> +               return r0 | r1;
> +       }
> +       return nand_correct_data_sub(buf, read_ecc, calc_ecc, eccsize_mult);
> +}
>  EXPORT_SYMBOL(nand_correct_data);
>
>  MODULE_LICENSE("GPL");
>



-- 
---
Regards,
\/ | |\/| /-\ |_

____      __o
------   -\<,
-----  ( )/ ( )