RE: [PATCH v5 1/4] ARM: OMAP2+: cleaned-up DT support of various ECC schemes

Gupta, Pekon
2013-09-12T11:57:58Z

> On Thu, Aug 22, 2013 at 07:56:57AM +0000, Gupta, Pekon wrote:

> If anything, the device entry should somehow describe the various ecc
> options
> that the hardware implements (if you can't derive that from the compatible
> value, which I think you can?).
>
> > Also I'll try to explain how below ecc-scheme selection is linked to TI
> Hardware.
> > TI SoC uses two separate H/W engines for calculating and correcting ECC.
> > (a) GPMC: General Purpose Memory Controller which calculates ECC also.
> > (b) ELM: Error Locator Module which just locates errors in BCHx code only.
> >
> > *Reason-1*: All OMAP platforms have (a) GPMC h/w engine, but some
> > legacy OMAP platforms do not have (b) ELM h/w engine. Such older
> > platforms use S/W lib/bch.c libraries for ECC error detection.
>
> Ok, so then you just need a binary "elm-engine" property to indicate
> that the hardware does have the engine.
>
> > Therefore in-order to keep the driver consistent for all platforms we
> > needed to keep so many ECC options alive. Like below you would see
> > two versions of BCH8 and BCH4
> > - bch8_code_hw (supported on new devices with ELM h/w engine)
> > - bch8_code_hw_detection_sw (kept for legacy devices)
>
> All you need to specify is what ECC format is used. I.e. BCH8. Whether the
> hardware or software will be used to calculate the checksums and
> detect/correct
> errors is a driver decision, and not something that the device tree needs to
> specify.
>
> > *Reason-2*: Also H/W ECC schemes have different ECC layout, which is
> > compatible to ROM code. Thus end-to-end NAND boot would work
> > only with xx_code_hw schemes only.
>
> So you should describe what the layout in use is. Wouldn't it be
> possible to make the software handle the same layout as the hardware
> engine if needed? I.e. the decision of using HW or SW is not a property
> of the hardware and shouldn't be described in the device tree.
>

Sorry I was caught up in some other priority work, therefore could not
update this. But I have recently posted another version with your feedbacks
incorporated. Please review them, whenever possible.
http://lists.infradead.org/pipermail/linux-mtd/2013-September/048611.html

Patch series at:
http://lists.infradead.org/pipermail/linux-mtd/2013-September/048613.html

Thank you
With regards, pekon