Re: [PATCH] Add driver for M-sys / Sandisk diskonchip G4 nand flash

[Ivan Djelic <ivan.djelic@parrot.com>]

On Mon, Oct 10, 2011 at 04:51:18PM +0100, Marek Vasut wrote:
> On Monday, October 10, 2011 04:48:10 PM Mike Dunn wrote:
> > This is a driver for the diskonchip G4 in my Palm Treo680.  I've tested it
> > fairly well; it passes the nandtest utility, and I've been able to create a
> > ubifs using it.

Hello Mike,

I had a quick look at how you handle BCH ecc correction. If I understood
correctly:
- hw generates a 56-bit polynomial remainder (56 = 14*4, m=14, t=4)
- hw-generated polynomial terms are mapped to bits in a way not compatible with
the BCH library

Still, I do not understand why you go to great lengths computing syndromes in
your code, using a mix of generated remainder tables and Galois field tables.

Why don't you just reformat the hw-generated polynomial (cheap, only 56 bits
to shuffle) and let the BCH library do the whole computation ?

> > +
> > +     /* undo last step in BCH alg; currently this is a mystery to me */

Maybe I can help here :)
This last step exists because the BCH library represents nand data as a large
polynomial with high order terms first (this is consistent with several hw
generators). If N is the total bit length of array data[], then byte data[0]
will contain polynomial terms X^(N-1) (in bit 7), X^(N-2), ... X^(N-8) (in bit
0). Byte data[1] will contain X^(N-9) (bit 7) ... X^(N-16) (bit 0) and so on.

Thus, term X^i is located in byte data[(N-1-i)/8], in bit position
7-(N-1-i)%8. The last step:

   errloc[i] = (errloc[i] & ~7)|(7-(errloc[i] & 7));

was added just so that users can correct bit errors with byte access:

   data[errloc[i]/8] ^= 1 << (errloc[i] & 7);

instead of (the slightly more complicated):

   data[errloc[i]/8] ^= 1 << (7-(errloc[i] & 7));

And that's all there is to it, really.

Looking at your code, it seems that your hw generator represents nand data[] as
a 32-bit words array with high order terms in lower bits (assuming your machine
is little-endian):

data[0] = X^(N-32) (bit 31) ... X^(N-1) (bit 0),
data[1] = X^(N-64) (bit 31) ... X^(N-33) (bit 0)

.. and so on. In your case you don't need the modulo mirroring step.

BR,

--
Ivan