Re: [PATCH 2/4] MTD: flash drivers set ecc strength

[Ivan Djelic <ivan.djelic@parrot.com>]

On Mon, Apr 02, 2012 at 06:34:12PM +0100, Mike Dunn wrote:
> On 03/29/2012 10:24 AM, Brian Norris wrote:
> >>
> >>        case NAND_ECC_SOFT_BCH:
> >> @@ -3384,6 +3385,8 @@ int nand_scan_tail(struct mtd_info *mtd)
> >>                        pr_warn("BCH ECC initialization failed!\n");
> >>                        BUG();
> >>                }
> >> +               chip->ecc.strength =
> >> +                       chip->ecc.bytes*8 / fls(8*chip->ecc.size);
> > 
> > Isn't this spacing against coding style? I'd suggest spaces around the
> > '*'. Also, after a few minutes, I have no idea where this calculation
> > comes from. But I'm not familiar with SOFT_BCH. Maybe a comment is in
> > order?
> 
> 
> I'm no bch expert myself (ping, Ivan), but I believe this is correct, and it is
> consistant with the equations used to determine the 't' parameter (i.e.,
> ecc_strength) from nand_bch_init() in drivers/mtd/nand/nand_bch.c.

Yes you are correct. Quoting nand_bch.c:

	m = fls(1+8*eccsize);
	t = (eccbytes*8)/m;

The first line computes the smallest 'm' such that
    2^m-1 > number_of_bits_in_ecc_block
hence
    m = fls(1+8*eccsize)

In practice, adding 1 to 8*eccsize has no effect on the fls() result (unless
eccsize is 0 :-), but I left it for consistency with nand_bch_init() doc.

Arguably it would have been less confusing to pass 'm' and 't' directly to nand_bch_init(),
but the present solution had the advantage of providing enough information, without
requiring any additional redundant field in mtd structures.

BR,
--
Ivan