On Fri, Oct 11, 2013 at 07:06:41PM +0530, Pekon Gupta wrote: > This patch adds following two flavours of BCH4 ECC scheme in omap2-nand driver > - OMAP_ECC_BCH4_CODE_HW_DETECTION_SW > - uses GPMC H/W engine for calculating ECC. > - uses software library (lib/bch.h & nand_bch.h) for error correction. > > - OMAP_ECC_BCH4_CODE_HW > - uses GPMC H/W engine for calculating ECC. > - uses ELM H/W engine for error correction. > > With this patch omap2-nand driver supports following ECC schemes: > +---------------------------------------+---------------+---------------+ > | ECC scheme |ECC calculation|Error detection| > +---------------------------------------+---------------+---------------+ > |OMAP_ECC_HAM1_CODE_HW |H/W (GPMC) |S/W | > +---------------------------------------+---------------+---------------+ > |OMAP_ECC_BCH4_CODE_HW_DETECTION_SW |H/W (GPMC) |S/W (lib/bch.h)| > |OMAP_ECC_BCH4_CODE_HW |H/W (GPMC) |H/W (ELM) | > +---------------------------------------+---------------+---------------+ > |OMAP_ECC_BCH8_CODE_HW_DETECTION_SW |H/W (GPMC) |S/W (lib/bch.h)| > |OMAP_ECC_BCH8_CODE_HW |H/W (GPMC) |H/W (ELM) | > +---------------------------------------+---------------+---------------+ > Important: > - Selection of OMAP_ECC_BCHx_CODE_HW_DETECTION_SW requires, > Kconfig: CONFIG_MTD_NAND_ECC_BCH: enables S/W based BCH ECC algorithm. > > - Selection of OMAP_ECC_BCHx_CODE_HW requires, > Kconfig: CONFIG_MTD_NAND_OMAP_BCH: enables ELM H/W module. > > Signed-off-by: Pekon Gupta <pekon@xxxxxx> > --- > drivers/mtd/nand/Kconfig | 30 ++--------- > drivers/mtd/nand/omap2.c | 134 +++++++++++++++++++++-------------------------- > 2 files changed, 63 insertions(+), 101 deletions(-) > ... > diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c > index fb96251..a783dae 100644 > --- a/drivers/mtd/nand/omap2.c > +++ b/drivers/mtd/nand/omap2.c > @@ -1927,7 +1868,7 @@ static int omap_nand_probe(struct platform_device *pdev) > nand_chip->ecc.bytes = 7; > nand_chip->ecc.strength = 4; > nand_chip->ecc.hwctl = omap3_enable_hwecc_bch; > - nand_chip->ecc.correct = omap3_correct_data_bch; > + info->nand.ecc.correct = nand_bch_correct_data; Your patch description doesn't talk about this (and the deletion of omap3_correct_data_bch() above). Is the NAND BCH library a drop-in replacement for omap3_correct_data_bch()? If so, TELL me about it in the commit description. If not, this is an incompatible change and should at least be documented so that people can understand the change. These questions are being asked by the DT guys, so include it in your descriptions. Also, you're very inconsistent on using 'nand_chip' vs. 'info->nand'. You added 'nand_chip' amid the noise of patch 3, so if you have it, use it consistently throughout probe(). Or remove it and don't use it at all. (This would be an independent patch from patch 3 and 4, in case you're wondering, since it causes a lot of diff noise.) > nand_chip->ecc.calculate = omap3_calculate_ecc_bch4; > /* define custom ECC layout */ > ecclayout->eccbytes = nand_chip->ecc.bytes * > @@ -2061,7 +2036,12 @@ out_release_mem_region: > free_irq(info->gpmc_irq_fifo, info); > release_mem_region(info->phys_base, info->mem_size); > out_free_info: > - omap3_free_bch(&info->mtd); > +#ifdef CONFIG_MTD_NAND_ECC_BCH > + if (info->nand.ecc.priv) { > + nand_bch_free(info->nand.ecc.priv); > + info->nand.ecc.priv = NULL; > + } > +#endif As noted previously, the #ifdef should not be necessary. > kfree(info); > > return err; > @@ -2072,8 +2052,12 @@ static int omap_nand_remove(struct platform_device *pdev) > struct mtd_info *mtd = platform_get_drvdata(pdev); > struct omap_nand_info *info = container_of(mtd, struct omap_nand_info, > mtd); > - omap3_free_bch(&info->mtd); > - > +#ifdef CONFIG_MTD_NAND_ECC_BCH > + if (info->nand.ecc.priv) { > + nand_bch_free(info->nand.ecc.priv); > + info->nand.ecc.priv = NULL; > + } > +#endif Ditto. > if (info->dma) > dma_release_channel(info->dma); > Brian -- 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