[U-Boot] [U-Boot, v2, 2/3] mtd: nand: omap: add support for BCH16_ECC - NAND driver updates

[Scott Wood]

On Tue, 2013-11-19 at 13:21 +0000, Gupta, Pekon wrote:
> > From: Scott Wood [mailto:scottwood at freescale.com]
> > > On Tue, Sep 10, 2013 at 12:55:07PM +0530, pekon gupta wrote:
> > > With increase in NAND flash densities occurence of bit-flips has increased.
> > > Thus stronger ECC schemes are required for detecting and correcting
> > multiple
> > > simultaneous bit-flips in same NAND page. But stronger ECC schemes have
> > large
> > > ECC syndrome which require more space in OOB/Spare.
> > > This patch add support for BCH16_ECC:
> > > (a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data.
> > > (b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B.
> > >
> > > Due to (b) this scheme can only be used with NAND devices which have
> > enough
> > > OOB to satisfy following equation:
> > > OOBsize per page >= 26 * (page-size / 512)
> > >
> > > Signed-off-by: Pekon Gupta <pekon at ti.com>
> > >
> > > ---
> > > arch/arm/include/asm/arch-am33xx/cpu.h       | 15 ++++-
> > >  arch/arm/include/asm/arch-am33xx/omap_gpmc.h |  4 +-
> > >  drivers/mtd/nand/omap_gpmc.c                 | 87
> > +++++++++++++++++++++++-----
> > >  include/mtd/mtd-abi.h                        |  3 +-
> > >  4 files changed, 90 insertions(+), 19 deletions(-)
> > 
> > This doesn't apply cleanly.
> > 
> This one is on top of previous patch series like..
> [Part 1] http://lists.denx.de/pipermail/u-boot/2013-November/167393.html
> [Part 2] http://lists.denx.de/pipermail/u-boot/2013-November/167445.html
> 
> So once the above series are cleaned and accepted. I'll rebase and re-send this
> series for BCH16 ecc-scheme again..
> 
> 
> > > diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h
> > b/arch/arm/include/asm/arch-am33xx/cpu.h
> > > index 10b56e0..1de92e6 100644
> > > --- a/arch/arm/include/asm/arch-am33xx/cpu.h
> > > +++ b/arch/arm/include/asm/arch-am33xx/cpu.h
> > > @@ -63,7 +63,16 @@ struct gpmc_cs {
> > >  };
> > >
> > >  struct bch_res_0_3 {
> > > -	u32 bch_result_x[4];
> > > +	u32 bch_result0;
> > > +	u32 bch_result1;
> > > +	u32 bch_result2;
> > > +	u32 bch_result3;
> > > +};
> > 
> > Is this really an improvement?
> > 
> Good for readability to match it to actual IP spec.. 
> Each of bch_resultx map to individual 32-bit registers named accordingly
> in IP spec.

Hardware often names things "FOO1, FOO2, etc", but in C it's often
better to express as foo[n] rather than foo1, foo2, etc.

> > It would also be nice if headers for things in drivers/mtd/nand weren't
> > in arch.
> > 
> I agree.. but I would take that separately, as I need to test all other
> platforms also for this change .. (like am335x, am43xx, omap3.. )

Yes, of course it would be separate.

-Scott