Re: [PATCH] mtd: nand: gpmi: Fix ecc strength calculation

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Wed, 22 Jun 2016 08:33:41 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> On Tue, Jun 21, 2016 at 10:52:09AM -0500, Han Xu wrote:
> > On Tue, Jun 21, 2016 at 9:46 AM, Boris Brezillon
> > <boris.brezillon@free-electrons.com> wrote:  
> > > On Tue, 21 Jun 2016 16:35:49 +0200
> > > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > >  
> > >> BCH ECC correction works in chunks of 512 bytes, so a 2k page size nand
> > >> is divided into 4 chunks. Hardware requires that each chunk has a full
> > >> number of bytes, so when we need 9 bits per chunk we must round up to
> > >> two bytes. The current code misses that and calculates a ECC strength
> > >> of 18 for a 2048+128 byte page size NAND. ECC strength of 18 requires
> > >> 30 bytes per chunk, so a total of 4 * (512 + 30) + 10 = 2178 bytes when
> > >> the device only has a page size of 2176 bytes.  
> > >
> > > AFAIR, the GPMI/ECC engine operates at the bit level (which is a pain
> > > to deal with BTW), and is only requiring a byte alignment on the total
> > > number of ECC bits. So here, DIV_ROUND_UP(18 * 13 * 4, 8) = 117, which
> > > fits in the 118 bytes (128 bytes - 10 bytes of 'metadata').
> > >
> > > Han, can you confirm that?  
> > 
> > Correct, BCH module works at bit level, 18bit ECC won't exceed the oob size.  
> 
> I see, only subpage reads fail here in my case. The driver does only subpage
> reads when the ECC size is byte aligned. This completely disables the subpage
> read feature on certain Nand types. Is that really what we want?

I'd definitely prefer to see only byte aligned settings, but the driver
already supports non-byte aligned configs, so this is not something we
can simply remove.

This being said, you have several solutions to address that:
1/ Patch the gpmi driver to take the information extracted from the
   nand-ecc-strength/step-size DT properties into account, and define
   these properties in your DT.
2/ Define 'fsl,use-minimum-ecc' in your DT and see what's happening. If
   you're lucky the ECC requirements will fall into a byte aligned case.
3/ Take advantage of the recent introduction of the generic
   'nand-ecc-maximize' property [1] and implement a different ECC
   maximization logic where you make sure your config generates
   byte-aligned ECC words

[1]https://lkml.org/lkml/2016/6/8/744