2013/10/17 Gupta, Pekon <pekon@xxxxxx>: >> From: Brian Norris [mailto:computersforpeace@xxxxxxxxx] >> > On Tue, Oct 15, 2013 at 11:19:55AM +0530, Pekon Gupta wrote: > [snip] > >> > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig >> > index d885298..5836039 100644 >> > --- a/drivers/mtd/nand/Kconfig >> > +++ b/drivers/mtd/nand/Kconfig >> > @@ -96,35 +96,13 @@ config MTD_NAND_OMAP2 >> > >> > config MTD_NAND_OMAP_BCH >> > depends on MTD_NAND && MTD_NAND_OMAP2 && ARCH_OMAP3 >> > - tristate "Enable support for hardware BCH error correction" >> > + tristate "Support hardware based BCH error correction" >> > default n >> > select BCH >> > - select BCH_CONST_PARAMS >> >> Do you know what will happen now if someone configures >> BCH_CONST_PARAMS? >> Would this cause problems? >> > As per comments in lib/bch.c > --------------------------- > * Option CONFIG_BCH_CONST_PARAMS can be used to force fixed values of > * parameters m and t; thus allowing extra compiler optimizations and providing > * better (up to 2x) encoding performance. Using this option makes sense when > * (m,t) are fixed and known in advance, e.g. when using BCH error correction > * on a particular NAND flash device. > --------------------------- > 'BCH_CONST_PARAMS' is required for optimization when BCH algorithm > is fixed. But in omap-nand case selection of type of BCH algorithm > (BCH4 or BCH8) comes from DT binding "ti,nand-ecc-opts". > > If enabled (CONFIG_BCH_CONST_PARAMS) will optimize lib/bch.c > for BCH8 algorithm by default, so > CASE: if BCH8 is selected by DT, then no issues > CASE: if BCH4 is selected then nand_bch_init() fails with following error > + if (!info->nand.ecc.priv) { > pr_err("nand: error: unable to use s/w BCH library\n"); > err = -EINVAL; > goto out_release_mem_region; > } > > [snip] > >> > if MTD_NAND_OMAP_BCH >> > config BCH_CONST_M >> >> Do you need to to also kill of the Kconfig stuff for BCH_CONST_M and >> BCH_CONST_T, which were tied to the MTD_NAND_OMAP_BCH4 and >> MTD_NAND_OMAP_BCH8 configs you just removed? >> > Thanks, good catch. I dint really notice. > So, the driver is now updated to separate out two flavours of BCHx scheme. > (a) OMAP_ECC_BCHx_CODE_HW: which uses ELM hardware > (b) OMAP_ECC_BCHx_CODE_HW_DETECTION_SW: which uses lib/bch.c > These BCH_CONST_M and BCH_CONST_T now belongs to (b) family only. > - if MTD_NAND_OMAP_BCH > + if MTD_NAND_ECC_BCH > config BCH_CONST_M > (I'll update that).. > > >> > diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c > [snip] >> > - info->bch = init_bch(nand_chip->ecc.bytes, >> > - nand_chip->ecc.strength, >> > - OMAP_ECC_BCH8_POLYNOMIAL); >> > - if (!info->bch) { >> > + info->nand.ecc.priv = nand_bch_init(mtd, >> > + info->nand.ecc.size, >> > + info->nand.ecc.bytes, >> > + &info->nand.ecc.layout); >> >> Are you sure nand_bch_init() is a proper drop-in replacement for the >> implementation you had based on init_bch()? It looks to me like they at >> least use a differnt polynomial value (0x201b vs. 0). Is this a problem >> for backwards compatibility? >> > It's not the polynomial value = 0. Rather 0x201b is selected in both cases > Refer below code. > --------------- > When init_bch(m,t, 0) is called from nand_bch_init() then, > lib/bch.c @@ init_bch(int m, int t, unsigned int prim_poly) > (a) /* default primitive polynomials */ > static const unsigned int prim_poly_tab[] = { > 0x25, 0x43, 0x83, 0x11d, 0x211, 0x409, 0x805, 0x1053, 0x201b, > 0x402b, 0x8003, > }; > (b) /* select a primitive polynomial for generating GF(2^m) */ > if (prim_poly == 0) > prim_poly = prim_poly_tab[m-min_m]; > (c) And, const int min_m = 5; > > So, for BCH8 m=13, min_m=5; So > prim_poly = prim_poly_tab[13-5] = prim_poly_tab[8] = 0x201b > --------------- > > Hence, there is no change in polynomial, it's just that instead of > hard-coding the value, polynomial selection depends on 'm' and 't'. > > > >> [...] >> >> A related question: do we have any current users of this driver that can >> provide testing results for this series? Or is this work just tested >> with new hardware? >> > Got a tested-by: jp.francois@xxxxxxxxxx for BCH4 > But that was in May,2013 :-) > http://lists.infradead.org/pipermail/linux-mtd/2013-May/047065.html > > > with regards, pekon Yes, but I am currently stuck on 3.10, and I need to migrate my out of tree board file to a device tree, and I currently have no bandwidth for this, so I won't be able to test the whole serie, but may be some parts are testable independently ? I will take a look and tell you. Regards, Jean-Philippe François -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
