Had a conversation with Ash @ Gumstix and he pointed out relying on CONFIG_NAND_OMAP_ECCSCHEME could be dangerous as it could be anything other than the two SW ECC schemes available for OMAP3.
Also it looks like making a selection between OMAP_ECC_BCH8_CODE_HW and OMAP_ECC_BCH8_CODE_HW_DETECTION_SW may not make sense as the latter clearly requires CONFIG_BCH, which enables "software" library for BCH. On Wed, Feb 18, 2015 at 9:33 AM, Adam Lee <adam.yh....@gmail.com> wrote: > Hi Andreas, for OMAP3 and AM35xx boards, it would have been ok omitting > the > CONFIG_BCH check and simply use CONFIG_NAND_OMAP_ECCSCHEME. > Those boards use the ecc scheme config already. However I just wasn't 100% > sure if I could rely on this config for all TI OMAP/AM based boards. I > know OMAP3 > and AM35xx board configs already have CONFIG_NAND_OMAP_ECCSCHEME. > Should I check for other OMAP and AM series? The original ecc detection > function > seems to be written with an assumption that config is nonexistent - hence > defaulting > to HAM1. > > That said, I agree that the whole omap_nand_switch_ecc() could be improved. > However, to me, the confusion arised from the fact that OMAP3 can do BCH8 > hw ECC > calculation but needs software to do the correction. Hence my patch > changes the > software part of the omap_nand_switch_ecc(), and not the hardware part. > > Just to clarify, what you are saying is that I should leave the software > part as it was (defaulting > to HAM1), and in the hardware part I should check for ELM support and > choose a BCH8 > scheme accordingly, regardless of what's defined by > CONFIG_NAND_OMAP_ECCSCHEME? > > In other words, I will run 'nandecc hw' to enable BCH8? > > Let me know, > > Thanks, > > Adam > > > > On Wed, Feb 18, 2015 at 6:14 AM, Andreas Bießmann < > andreas.de...@googlemail.com> wrote: > >> Hi Adam, >> >> On 02/18/2015 03:58 AM, Adam YH Lee wrote: >> > The ECC scheme selection algorithm in OMAP GPMC appears to be left >> untested when >> > BCH8 handling code was added. Running 'nandecc sw' defaults to HAM1 >> even if >> > the board is using another scheme (ex. >> OMAP_ECC_BCH8_CODE_HW_DETECTION_SW on >> > OMAP3). This results in unrecoverable ECC errors when reading data. >> This commit >> > fixes the behavior by checking for CONFIG_BCH and using the scheme >> defined by >> > CONFIG_NAND_OMAP_ECCSCHEME in the board configuration file. >> > >> > This has been tested on Gumstix Overo (OMAP3). >> > >> > Signed-off-by: Adam YH Lee <adam.yh....@gmail.com> >> > --- >> > drivers/mtd/nand/omap_gpmc.c | 5 +++++ >> > 1 file changed, 5 insertions(+) >> > >> > diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c >> > index fc64f48..5daf932 100644 >> > --- a/drivers/mtd/nand/omap_gpmc.c >> > +++ b/drivers/mtd/nand/omap_gpmc.c >> > @@ -901,8 +901,13 @@ int __maybe_unused omap_nand_switch_ecc(uint32_t >> hardware, uint32_t eccstrength) >> > return -EINVAL; >> > } >> > } else { >> > + #ifdef CONFIG_BCH >> > + err = omap_select_ecc_scheme(nand, >> CONFIG_NAND_OMAP_ECCSCHEME, >> > + mtd->writesize, mtd->oobsize); >> > + #else >> > err = omap_select_ecc_scheme(nand, OMAP_ECC_HAM1_CODE_SW, >> > mtd->writesize, mtd->oobsize); >> >> Couldn't we just use the CONFIG_NAND_OMAP_ECCSCHEME instead of >> OMAP_ECC_HAM1_CODE_SW here and omit the CONFIG_BCH define? >> >> On the other hand I think the whole function omap_nand_switch_ecc() is >> wrong. AFAIR it should only be used on omap3 aka am35xx/dm37xx devices >> witch the nandecc command. >> These SoC do not have a ELM. Therefore I decided to say BCH8 on those >> devices is always 'HW ECC' when introduced BCH8 on omap3 first. Nowadays >> it is the so called BCH8_CODE_HW_DETECTION_SW. Coming from this mindset >> the right solution is to use some detection if the ELM is supported or >> not and switch in the HW part of omap_nand_switch_ecc() between >> OMAP_ECC_BCH8_CODE_HW and OMAP_ECC_BCH8_CODE_HW_DETECTION_SW. >> >> Best regards >> >> Andreas Bießmann >> > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot