> 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
