i.MX25 NFC with 8 bit ecc strength

I'm trying to get nand_ecclayout right on i.MX25 with the Micron MT29F8G08ABABA (page size: 4096, oob size: 224). The large OOB size allows

using hardware ecc strength of 8bit per ecc step (512 bytes). The mxc_nand driver code (get_eccsize()) and the reference manual seems to indicate that enabling 8 bit ecc mode requires 26 oob bytes per ecc step. However, this seems to contradict the actual hardware test as the shown in the dump below of a zero filled page + oob:

# hexdump -C dump4
00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00001000  ff ff ff ff ff ff ff 91  c4 45 be 32 45 6f 5d b1  |.........E.2Eo].|
00001010  b1 b9 13 61 59 7d 42 58  eb ff ff ff ff ff ff ff  |...aY}BX........|
00001020  ff ff ff 91 c4 45 be 32  45 6f 5d b1 b1 b9 13 61  |.....E.2Eo]....a|
00001030  59 7d 42 58 eb ff ff ff  ff ff ff ff ff ff ff 91  |Y}BX............|
00001040  c4 45 be 32 45 6f 5d b1  b1 b9 13 61 59 7d 42 58  |.E.2Eo]....aY}BX|
00001050  eb ff ff ff ff ff ff ff  ff ff ff 91 c4 45 be 32  |.............E.2|
00001060  45 6f 5d b1 b1 b9 13 61  59 7d 42 58 eb ff ff ff  |Eo]....aY}BX....|
00001070  ff ff ff ff ff ff ff 91  c4 45 be 32 45 6f 5d b1  |.........E.2Eo].|
00001080  b1 b9 13 61 59 7d 42 58  eb ff ff ff ff ff ff ff  |...aY}BX........|
00001090  ff ff ff 91 c4 45 be 32  45 6f 5d b1 b1 b9 13 61  |.....E.2Eo]....a|
000010a0  59 7d 42 58 eb ff ff ff  ff ff ff ff ff ff ff 91  |Y}BX............|
000010b0  c4 45 be 32 45 6f 5d b1  b1 b9 13 61 59 7d 42 58  |.E.2Eo]....aY}BX|
000010c0  eb ff ff ff ff ff ff ff  ff ff ff 91 c4 45 be 32  |.............E.2|
000010d0  45 6f 5d b1 b1 b9 13 61  59 7d 42 58 eb ff ff ff  |Eo]....aY}BX....|

As you can easily see, ecc steps start at 28 bytes interval, with 18 bytes for ecc (matches documentation), and 10 bytes free.

Has anyone ever tried using 8 bit ecc mode with any variant of the i.MX NFC?

Quoted 2 lines · Baruch Siach

I'm trying to get nand_ecclayout right on i.MX25 with the Micron MT29F8G08ABABA (page size: 4096, oob size: 224). The large OOB size allows

Just for me to understand your plan: To support the big ecc variant you need another set of struct nand_ecclayout. The expectation for your flash would be:

	.eccpos = { 8, ... 25,
			34, ... 51,
			60, ... 77,
			86, ... 103,
			112, ... 129,
			138, ... 155,
			164, ... 181,
			190, ... 207 }

right?

Quoted 24 lines

using hardware ecc strength of 8bit per ecc step (512 bytes). The mxc_nand driver code (get_eccsize()) and the reference manual seems to indicate that enabling 8 bit ecc mode requires 26 oob bytes per ecc step. However, this seems to contradict the actual hardware test as the shown in the dump below of a zero filled page + oob:

# hexdump -C dump4
00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00001000  ff ff ff ff ff ff ff 91  c4 45 be 32 45 6f 5d b1  |.........E.2Eo].|
00001010  b1 b9 13 61 59 7d 42 58  eb ff ff ff ff ff ff ff  |...aY}BX........|
00001020  ff ff ff 91 c4 45 be 32  45 6f 5d b1 b1 b9 13 61  |.....E.2Eo]....a|
00001030  59 7d 42 58 eb ff ff ff  ff ff ff ff ff ff ff 91  |Y}BX............|
00001040  c4 45 be 32 45 6f 5d b1  b1 b9 13 61 59 7d 42 58  |.E.2Eo]....aY}BX|
00001050  eb ff ff ff ff ff ff ff  ff ff ff 91 c4 45 be 32  |.............E.2|
00001060  45 6f 5d b1 b1 b9 13 61  59 7d 42 58 eb ff ff ff  |Eo]....aY}BX....|
00001070  ff ff ff ff ff ff ff 91  c4 45 be 32 45 6f 5d b1  |.........E.2Eo].|
00001080  b1 b9 13 61 59 7d 42 58  eb ff ff ff ff ff ff ff  |...aY}BX........|
00001090  ff ff ff 91 c4 45 be 32  45 6f 5d b1 b1 b9 13 61  |.....E.2Eo]....a|
000010a0  59 7d 42 58 eb ff ff ff  ff ff ff ff ff ff ff 91  |Y}BX............|
000010b0  c4 45 be 32 45 6f 5d b1  b1 b9 13 61 59 7d 42 58  |.E.2Eo]....aY}BX|
000010c0  eb ff ff ff ff ff ff ff  ff ff ff 91 c4 45 be 32  |.............E.2|
000010d0  45 6f 5d b1 b1 b9 13 61  59 7d 42 58 eb ff ff ff  |Eo]....aY}BX....|

Quoted 2 lines

As you can easily see, ecc steps start at 28 bytes interval, with 18 bytes for ecc (matches documentation), and 10 bytes free.

How did you extract this page+oob from the nand flash? From Linux I assume?

Can you try from barebox something like:

	mw -w 0xbb001e08 0x0000 # READ0
	mw -w 0xbb001e1c 0x01 # CMD cycle
	mw -w 0xbb001e06 0x00 # Address = 0
	mw -w 0xbb001e1c 0x02 # Address cycle
	mw -w 0xbb001e1c 0x02 # Address cycle
	mw -w 0xbb001e1c 0x02 # Address cycle (do we need three? [1])
	mw -w 0xbb001e04 0x00
	mw -w 0xbb001e1c 0x08 # NAND OUTPUT
	md -w 0xbb000000+0x10f0

with ecc being disabled (i.e. CONFIG1, bit 3 = 0). Does this show the 28 bytes offset, too?

Quoted 16 lines · Uwe Kleine-König

Quoted 2 lines · Baruch Siach

I'm trying to get nand_ecclayout right on i.MX25 with the Micron MT29F8G08ABABA (page size: 4096, oob size: 224). The large OOB size allows

Just for me to understand your plan: To support the big ecc variant you need another set of struct nand_ecclayout. The expectation for your flash would be:

	.eccpos = { 8, ... 25,
			34, ... 51,
			60, ... 77,
			86, ... 103,
			112, ... 129,
			138, ... 155,
			164, ... 181,
			190, ... 207 }

Quoted 1 line

right?

According to the dump below (28 bytes interval) it should be off by one:

	eccpos = { 7, ... 24,
		35, ... 52,
		63, ... 80,
		91, ... 108,
		119, ... 136,
		147, ... 164,
		175, ... 192,
		203, ... 220 };

Quoted 24 lines

Quoted 24 lines

using hardware ecc strength of 8bit per ecc step (512 bytes). The mxc_nand driver code (get_eccsize()) and the reference manual seems to indicate that enabling 8 bit ecc mode requires 26 oob bytes per ecc step. However, this seems to contradict the actual hardware test as the shown in the dump below of a zero filled page + oob:

# hexdump -C dump4
00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00001000  ff ff ff ff ff ff ff 91  c4 45 be 32 45 6f 5d b1  |.........E.2Eo].|
00001010  b1 b9 13 61 59 7d 42 58  eb ff ff ff ff ff ff ff  |...aY}BX........|
00001020  ff ff ff 91 c4 45 be 32  45 6f 5d b1 b1 b9 13 61  |.....E.2Eo]....a|
00001030  59 7d 42 58 eb ff ff ff  ff ff ff ff ff ff ff 91  |Y}BX............|
00001040  c4 45 be 32 45 6f 5d b1  b1 b9 13 61 59 7d 42 58  |.E.2Eo]....aY}BX|
00001050  eb ff ff ff ff ff ff ff  ff ff ff 91 c4 45 be 32  |.............E.2|
00001060  45 6f 5d b1 b1 b9 13 61  59 7d 42 58 eb ff ff ff  |Eo]....aY}BX....|
00001070  ff ff ff ff ff ff ff 91  c4 45 be 32 45 6f 5d b1  |.........E.2Eo].|
00001080  b1 b9 13 61 59 7d 42 58  eb ff ff ff ff ff ff ff  |...aY}BX........|
00001090  ff ff ff 91 c4 45 be 32  45 6f 5d b1 b1 b9 13 61  |.....E.2Eo]....a|
000010a0  59 7d 42 58 eb ff ff ff  ff ff ff ff ff ff ff 91  |Y}BX............|
000010b0  c4 45 be 32 45 6f 5d b1  b1 b9 13 61 59 7d 42 58  |.E.2Eo]....aY}BX|
000010c0  eb ff ff ff ff ff ff ff  ff ff ff 91 c4 45 be 32  |.............E.2|
000010d0  45 6f 5d b1 b1 b9 13 61  59 7d 42 58 eb ff ff ff  |Eo]....aY}BX....|

Quoted 4 lines

Quoted 2 lines

As you can easily see, ecc steps start at 28 bytes interval, with 18 bytes for ecc (matches documentation), and 10 bytes free.

How did you extract this page+oob from the nand flash? From Linux I assume?

Got it form nanddump using:

	nanddump -s 5238784 -f dump4 -o /dev/mtd2

Quoted 12 lines

Can you try from barebox something like:

	mw -w 0xbb001e08 0x0000 # READ0
	mw -w 0xbb001e1c 0x01 # CMD cycle
	mw -w 0xbb001e06 0x00 # Address = 0
	mw -w 0xbb001e1c 0x02 # Address cycle
	mw -w 0xbb001e1c 0x02 # Address cycle
	mw -w 0xbb001e1c 0x02 # Address cycle (do we need three? [1])
	mw -w 0xbb001e04 0x00
	mw -w 0xbb001e1c 0x08 # NAND OUTPUT
	md -w 0xbb000000+0x10f0

Quoted 2 lines

with ecc being disabled (i.e. CONFIG1, bit 3 = 0). Does this show the 28 bytes offset, too?

I (hopefuly) disabled ECC with:

	mw -w 0xbb001e1a 0x0010

Then, for the same page (using three address cycles, 0xff, 0x04, 0x00), I got all zeros up to 0xbb001000 (inclusive), and then:

bb001010: 0000 0000 0000 0000 0000 ffff 28a7 428a .............(.B
bb001020: 89fa 2cd4 4640 560a 2634 ac7e e5d8 20ea ...,@F.V4&~....
bb001030: caaa c809 0195 8411 6972 6bfc 84d6 10af ........ri.k....
bb001040: 0000 0000 0000 0000 0000 0000 0000 0000 ................
bb001050: 0000 0000 0000 0000 0000 ffff f1da 19c3 ................
bb001060: 21b2 0832 09c6 3c55 638c 3bb8 fd54 2983 .!2...U<.c.;T..)
bb001070: 8325 6d98 0814 0d64 ee73 675e 1943 5cf2 %..m..d.s.^gC..\
bb001080: 0000 0000 0000 0000 0000 0000 0000 0000 ................
bb001090: 0000 0000 0000 0000 0000 ffff c1bc 31fe ...............1
bb0010a0: 16ee ab6b 34a5 acad 0771 048c ac58 3f19 ..k..4..q...X..?
bb0010b0: b699 a88f eb5a 00ae 7e3c 9c6d 2ba8 d72e ....Z...<~m..+..
bb0010c0: 0000 0000 0000 0000 0000 0000 0000 0000 ................
bb0010d0: 0000 0000 0000 0000 0000 ffff ae14 06c7 ................
bb0010e0: 89b9 eb67 00d0 3648 daeb 15f5 77ca 8c09 ..g...H6.....w..

Quoted 31 lines · Baruch Siach

Quoted 30 lines · Uwe Kleine-König

Quoted 29 lines · Baruch Siach

I'm trying to get nand_ecclayout right on i.MX25 with the Micron MT29F8G08ABABA (page size: 4096, oob size: 224). The large OOB size allows using hardware ecc strength of 8bit per ecc step (512 bytes). The mxc_nand driver code (get_eccsize()) and the reference manual seems to indicate that enabling 8 bit ecc mode requires 26 oob bytes per ecc step. However, this seems to contradict the actual hardware test as the shown in the dump
below of a zero filled page + oob:

# hexdump -C dump4
00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00001000  ff ff ff ff ff ff ff 91  c4 45 be 32 45 6f 5d b1  |.........E.2Eo].|
00001010  b1 b9 13 61 59 7d 42 58  eb ff ff ff ff ff ff ff  |...aY}BX........|
00001020  ff ff ff 91 c4 45 be 32  45 6f 5d b1 b1 b9 13 61  |.....E.2Eo]....a|
00001030  59 7d 42 58 eb ff ff ff  ff ff ff ff ff ff ff 91  |Y}BX............|
00001040  c4 45 be 32 45 6f 5d b1  b1 b9 13 61 59 7d 42 58  |.E.2Eo]....aY}BX|
00001050  eb ff ff ff ff ff ff ff  ff ff ff 91 c4 45 be 32  |.............E.2|
00001060  45 6f 5d b1 b1 b9 13 61  59 7d 42 58 eb ff ff ff  |Eo]....aY}BX....|
00001070  ff ff ff ff ff ff ff 91  c4 45 be 32 45 6f 5d b1  |.........E.2Eo].|
00001080  b1 b9 13 61 59 7d 42 58  eb ff ff ff ff ff ff ff  |...aY}BX........|
00001090  ff ff ff 91 c4 45 be 32  45 6f 5d b1 b1 b9 13 61  |.....E.2Eo]....a|
000010a0  59 7d 42 58 eb ff ff ff  ff ff ff ff ff ff ff 91  |Y}BX............|
000010b0  c4 45 be 32 45 6f 5d b1  b1 b9 13 61 59 7d 42 58  |.E.2Eo]....aY}BX|
000010c0  eb ff ff ff ff ff ff ff  ff ff ff 91 c4 45 be 32  |.............E.2|
000010d0  45 6f 5d b1 b1 b9 13 61  59 7d 42 58 eb ff ff ff  |Eo]....aY}BX....|

As you can easily see, ecc steps start at 28 bytes interval, with 18 bytes for ecc (matches documentation), and 10 bytes free.

Quoted 6 lines

Quoted 2 lines

How did you extract this page+oob from the nand flash? From Linux I assume?

Got it form nanddump using:

	nanddump -s 5238784 -f dump4 -o /dev/mtd2

OK, then there is also the driver in between the hardware and your image. Can you show the changes you did to mxc_nand?

Quoted 12 lines

Quoted 12 lines

Can you try from barebox something like:

	mw -w 0xbb001e08 0x0000 # READ0
	mw -w 0xbb001e1c 0x01 # CMD cycle
	mw -w 0xbb001e06 0x00 # Address = 0
	mw -w 0xbb001e1c 0x02 # Address cycle
	mw -w 0xbb001e1c 0x02 # Address cycle
	mw -w 0xbb001e1c 0x02 # Address cycle (do we need three? [1])
	mw -w 0xbb001e04 0x00
	mw -w 0xbb001e1c 0x08 # NAND OUTPUT
	md -w 0xbb000000+0x10f0

Quoted 6 lines

Quoted 2 lines

with ecc being disabled (i.e. CONFIG1, bit 3 = 0). Does this show the 28 bytes offset, too?

I (hopefuly) disabled ECC with:

	mw -w 0xbb001e1a 0x0010

looks ok.

Quoted 17 lines

Then, for the same page (using three address cycles, 0xff, 0x04, 0x00), I got all zeros up to 0xbb001000 (inclusive), and then:

bb001010: 0000 0000 0000 0000 0000 ffff 28a7 428a .............(.B
bb001020: 89fa 2cd4 4640 560a 2634 ac7e e5d8 20ea ...,@F.V4&~....
bb001030: caaa c809 0195 8411 6972 6bfc 84d6 10af ........ri.k....
bb001040: 0000 0000 0000 0000 0000 0000 0000 0000 ................
bb001050: 0000 0000 0000 0000 0000 ffff f1da 19c3 ................
bb001060: 21b2 0832 09c6 3c55 638c 3bb8 fd54 2983 .!2...U<.c.;T..)
bb001070: 8325 6d98 0814 0d64 ee73 675e 1943 5cf2 %..m..d.s.^gC..\
bb001080: 0000 0000 0000 0000 0000 0000 0000 0000 ................
bb001090: 0000 0000 0000 0000 0000 ffff c1bc 31fe ...............1
bb0010a0: 16ee ab6b 34a5 acad 0771 048c ac58 3f19 ..k..4..q...X..?
bb0010b0: b699 a88f eb5a 00ae 7e3c 9c6d 2ba8 d72e ....Z...<~m..+..
bb0010c0: 0000 0000 0000 0000 0000 0000 0000 0000 ................
bb0010d0: 0000 0000 0000 0000 0000 ffff ae14 06c7 ................
bb0010e0: 89b9 eb67 00d0 3648 daeb 15f5 77ca 8c09 ..g...H6.....w..

This is more or less expected. The "more" part is: Matching the hardware description the (virtual) spare area is sorted into the spare area buffers, so the first spare area is written to 0xbb001000, the 2nd to 0xbb001040 etc. (See table 36-3 in the manual.) So probably it's the driver who doesn't get the sorting right.

The "less" part however is that the data doesn't match what you see in linux with nanddump. Hmm.

Can you retry the above commands after initializing the NFC buffer with some pattern:

	memset -w 0xbb000000 0x55 0x1200

Quoted 32 lines · Uwe Kleine-König

Quoted 31 lines · Baruch Siach

Quoted 30 lines · Uwe Kleine-König

Quoted 29 lines · Baruch Siach

I'm trying to get nand_ecclayout right on i.MX25 with the Micron MT29F8G08ABABA (page size: 4096, oob size: 224). The large OOB size allows using hardware ecc strength of 8bit per ecc step (512 bytes). The mxc_nand driver code (get_eccsize()) and the reference manual seems to indicate that enabling 8 bit ecc mode requires 26 oob bytes per ecc step. However, this seems to contradict the actual hardware test as the shown in the dump
below of a zero filled page + oob:

# hexdump -C dump4
00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00001000  ff ff ff ff ff ff ff 91  c4 45 be 32 45 6f 5d b1  |.........E.2Eo].|
00001010  b1 b9 13 61 59 7d 42 58  eb ff ff ff ff ff ff ff  |...aY}BX........|
00001020  ff ff ff 91 c4 45 be 32  45 6f 5d b1 b1 b9 13 61  |.....E.2Eo]....a|
00001030  59 7d 42 58 eb ff ff ff  ff ff ff ff ff ff ff 91  |Y}BX............|
00001040  c4 45 be 32 45 6f 5d b1  b1 b9 13 61 59 7d 42 58  |.E.2Eo]....aY}BX|
00001050  eb ff ff ff ff ff ff ff  ff ff ff 91 c4 45 be 32  |.............E.2|
00001060  45 6f 5d b1 b1 b9 13 61  59 7d 42 58 eb ff ff ff  |Eo]....aY}BX....|
00001070  ff ff ff ff ff ff ff 91  c4 45 be 32 45 6f 5d b1  |.........E.2Eo].|
00001080  b1 b9 13 61 59 7d 42 58  eb ff ff ff ff ff ff ff  |...aY}BX........|
00001090  ff ff ff 91 c4 45 be 32  45 6f 5d b1 b1 b9 13 61  |.....E.2Eo]....a|
000010a0  59 7d 42 58 eb ff ff ff  ff ff ff ff ff ff ff 91  |Y}BX............|
000010b0  c4 45 be 32 45 6f 5d b1  b1 b9 13 61 59 7d 42 58  |.E.2Eo]....aY}BX|
000010c0  eb ff ff ff ff ff ff ff  ff ff ff 91 c4 45 be 32  |.............E.2|
000010d0  45 6f 5d b1 b1 b9 13 61  59 7d 42 58 eb ff ff ff  |Eo]....aY}BX....|

As you can easily see, ecc steps start at 28 bytes interval, with 18 bytes for ecc (matches documentation), and 10 bytes free.

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

How did you extract this page+oob from the nand flash? From Linux I assume?

Got it form nanddump using:

	nanddump -s 5238784 -f dump4 -o /dev/mtd2

OK, then there is also the driver in between the hardware and your image. Can you show the changes you did to mxc_nand?

I generated the dump above on an old kernel (v2.6.36.4) that includes a number of changes needed to support 4K pages and large OOB.

I have generated an almost exact same dump using v4.0-rc1 based l2-mtd master as of d09957fbb4d (which includes your mxc_nand ONFI support code; works perfectly, BTW). My only local change is commenting out the copy_spare() call in the NAND_CMD_PAGEPROG case at mxc_nand_command().

Quoted 38 lines

Quoted 12 lines

Quoted 12 lines

Can you try from barebox something like:

	mw -w 0xbb001e08 0x0000 # READ0
	mw -w 0xbb001e1c 0x01 # CMD cycle
	mw -w 0xbb001e06 0x00 # Address = 0
	mw -w 0xbb001e1c 0x02 # Address cycle
	mw -w 0xbb001e1c 0x02 # Address cycle
	mw -w 0xbb001e1c 0x02 # Address cycle (do we need three? [1])
	mw -w 0xbb001e04 0x00
	mw -w 0xbb001e1c 0x08 # NAND OUTPUT
	md -w 0xbb000000+0x10f0

Quoted 6 lines

Quoted 2 lines

with ecc being disabled (i.e. CONFIG1, bit 3 = 0). Does this show the 28 bytes offset, too?

I (hopefuly) disabled ECC with:

	mw -w 0xbb001e1a 0x0010

looks ok.

Quoted 17 lines

Then, for the same page (using three address cycles, 0xff, 0x04, 0x00), I got all zeros up to 0xbb001000 (inclusive), and then:

bb001010: 0000 0000 0000 0000 0000 ffff 28a7 428a .............(.B
bb001020: 89fa 2cd4 4640 560a 2634 ac7e e5d8 20ea ...,@F.V4&~....
bb001030: caaa c809 0195 8411 6972 6bfc 84d6 10af ........ri.k....
bb001040: 0000 0000 0000 0000 0000 0000 0000 0000 ................
bb001050: 0000 0000 0000 0000 0000 ffff f1da 19c3 ................
bb001060: 21b2 0832 09c6 3c55 638c 3bb8 fd54 2983 .!2...U<.c.;T..)
bb001070: 8325 6d98 0814 0d64 ee73 675e 1943 5cf2 %..m..d.s.^gC..\
bb001080: 0000 0000 0000 0000 0000 0000 0000 0000 ................
bb001090: 0000 0000 0000 0000 0000 ffff c1bc 31fe ...............1
bb0010a0: 16ee ab6b 34a5 acad 0771 048c ac58 3f19 ..k..4..q...X..?
bb0010b0: b699 a88f eb5a 00ae 7e3c 9c6d 2ba8 d72e ....Z...<~m..+..
bb0010c0: 0000 0000 0000 0000 0000 0000 0000 0000 ................
bb0010d0: 0000 0000 0000 0000 0000 ffff ae14 06c7 ................
bb0010e0: 89b9 eb67 00d0 3648 daeb 15f5 77ca 8c09 ..g...H6.....w..

Quoted 5 lines

This is more or less expected. The "more" part is: Matching the hardware description the (virtual) spare area is sorted into the spare area buffers, so the first spare area is written to 0xbb001000, the 2nd to 0xbb001040 etc. (See table 36-3 in the manual.) So probably it's the driver who doesn't get the sorting right.

OK. I see what you mean. The 28 bytes interval has noting to do with hardware. It comes from this line in copy_spare():

	j = (mtd->oobsize / n >> 1) << 1;

In my case oobsize = 224, and n = 8 (512 bytes steps), so j == 28. This means that we must generate nand_ecclayout at run time according to the actual oobsize. This is probably also true for the 4 bit ecc case.

Quoted 7 lines

The "less" part however is that the data doesn't match what you see in linux with nanddump. Hmm.

Can you retry the above commands after initializing the NFC buffer with some pattern:

	memset -w 0xbb000000 0x55 0x1200

With this I get:

bb001010: 0000 0000 0000 0000 0000 5555 5555 5555 ..........UUUUUU
bb001020: 5555 5555 5555 5555 5555 5555 5555 5555 UUUUUUUUUUUUUUUU
bb001030: 5555 5555 5555 5555 5555 5555 5555 5555 UUUUUUUUUUUUUUUU
bb001040: 0000 0000 0000 0000 0000 0000 0000 0000 ................
bb001050: 0000 0000 0000 0000 0000 5555 5555 5555 ..........UUUUUU
bb001060: 5555 5555 5555 5555 5555 5555 5555 5555 UUUUUUUUUUUUUUUU
bb001070: 5555 5555 5555 5555 5555 5555 5555 5555 UUUUUUUUUUUUUUUU
bb001080: 0000 0000 0000 0000 0000 0000 0000 0000 ................
bb001090: 0000 0000 0000 0000 0000 5555 5555 5555 ..........UUUUUU
bb0010a0: 5555 5555 5555 5555 5555 5555 5555 5555 UUUUUUUUUUUUUUUU
bb0010b0: 5555 5555 5555 5555 5555 5555 5555 5555 UUUUUUUUUUUUUUUU
bb0010c0: 0000 0000 0000 0000 0000 0000 0000 0000 ................
bb0010d0: 0000 0000 0000 0000 0000 5555 5555 5555 ..........UUUUUU
bb0010e0: 5555 5555 5555 5555 5555 5555 5555 5555 UUUUUUUUUUUUUUUU

with anything else zero filled. This is probably misleading, since I get the same output (i.e. all zeros) even for pages that contain data.

Quoted 7 lines · Baruch Siach

Quoted 5 lines · Uwe Kleine-König

This is more or less expected. The "more" part is: Matching the hardware description the (virtual) spare area is sorted into the spare area buffers, so the first spare area is written to 0xbb001000, the 2nd to 0xbb001040 etc. (See table 36-3 in the manual.) So probably it's the driver who doesn't get the sorting right.

Quoted 8 lines

OK. I see what you mean. The 28 bytes interval has noting to do with hardware. It comes from this line in copy_spare():

	j = (mtd->oobsize / n >> 1) << 1;

In my case oobsize = 224, and n = 8 (512 bytes steps), so j == 28. This means that we must generate nand_ecclayout at run time according to the actual oobsize. This is probably also true for the 4 bit ecc case.

I think you're only partly right here. The NFC only supports 128 or 218 bytes spare area for 4k NAND flashes (initialized by BT_SPARE_SIZE). For you chip the controller uses the 218 bytes setting, so 26 bytes are read for the first 7 oob chunks each (last one: 36) As the driver assumes the real oob size of 224 bytes you get that offset of 28 instead.

So looking again on your hexdump:

	00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
	*

This is the data part, everything in order

	00001000  ff ff ff ff ff ff ff 91  c4 45 be 32 45 6f 5d b1  |.........E.2Eo].|
	00001010  b1 b9 13 61 59 7d 42 58  eb ff ff ff ff ff ff ff  |...aY}BX........|

Up to the 26th byte --------------------------^^ this is data coming from the flash. The following two 0xff are just what happened to be written in the NFC buffer. Starting with the four last 0xff in that line we have real data again.

	00001020  ff ff ff 91 c4 45 be 32  45 6f 5d b1 b1 b9 13 61  |.....E.2Eo]....a|

which makes the first ecc byte again the 8th of the oob chunk similar to the one above.

While understanding the problem I produced the following (untested) patch:

drivers/mtd/nand/mxc_nand.c +27 -10

index dca63a70e783..fc835d352e1c 100644
@@ -807,32 +807,49 @@ static void mxc_nand_select_chip_v2(struct mtd_info *mtd, int chip)
 }
 
 /*
- * Function to transfer data to/from spare area.
+ * The controller splits a page into data chunks of 512 bytes + partial oob.
+ * There are writesize / 512 such chunks, the size of the partial oob parts is
+ * oobsize / #chunks rounded down to a multiple of 2. The last oob chunk then
+ * contains additionally the byte lost by rounding (if any).
+ * This function handles the needed shuffling between host->data_buf (which
+ * holds a page in natural order, i.e. writesize bytes data + oobsize bytes
+ * spare) and the NFC buffer.
  */
 static void copy_spare(struct mtd_info *mtd, bool bfrom)
 {
 	struct nand_chip *this = mtd->priv;
 	struct mxc_nand_host *host = this->priv;
 	u16 i, j;
-	u16 n = mtd->writesize >> 9;
+
+	u16 num_chunks = mtd->writesize / 512;
+
 	u8 *d = host->data_buf + mtd->writesize;
 	u8 __iomem *s = host->spare0;
-	u16 t = host->devtype_data->spare_len;
+	u16 sparebuf_size = host->devtype_data->spare_len;
 
-	j = (mtd->oobsize / n >> 1) << 1;
+	/* size of oob chunk for all but possibly the last one */
+	oob_chunk_size = (mtd->oobsize / num_chunks >> 1) << 1;
 
 	if (bfrom) {
-		for (i = 0; i < n - 1; i++)
-			memcpy32_fromio(d + i * j, s + i * t, j);
+		for (i = 0; i < num_chunks - 1; i++)
+			memcpy32_fromio(d + i * oob_chunk_size,
+					s + i * sparebuf_size,
+					oob_chunk_size);
 
 		/* the last section */
-		memcpy32_fromio(d + i * j, s + i * t, mtd->oobsize - i * j);
+		memcpy32_fromio(d + i * oob_chunk_size,
+				s + i * sparebuf_size,
+				mtd->oobsize - i * oob_chunk_size);
 	} else {
-		for (i = 0; i < n - 1; i++)
-			memcpy32_toio(&s[i * t], &d[i * j], j);
+		for (i = 0; i < num_chunks - 1; i++)
+			memcpy32_toio(&s[i * sparebuf_size],
+				      &d[i * oob_chunk_size],
+				      oob_chunk_size);
 
 		/* the last section */
-		memcpy32_toio(&s[i * t], &d[i * j], mtd->oobsize - i * j);
+		memcpy32_toio(&s[oob_chunk_size * sparebuf_size],
+			      &d[i * oob_chunk_size],
+			      mtd->oobsize - i * oob_chunk_size);
 	}
 }
 

What is needed now on top of this (untested and noop) change is to use the oob size the controller assumes instead of the real one and somehow explain that to the mtd layer and maintainers :-)

Hi Uwe,

Quoted 30 lines · Uwe Kleine-König

Quoted 7 lines · Baruch Siach

Quoted 5 lines · Uwe Kleine-König

This is more or less expected. The "more" part is: Matching the hardware description the (virtual) spare area is sorted into the spare area buffers, so the first spare area is written to 0xbb001000, the 2nd to 0xbb001040 etc. (See table 36-3 in the manual.) So probably it's the driver who doesn't get the sorting right.

Quoted 8 lines

OK. I see what you mean. The 28 bytes interval has noting to do with hardware. It comes from this line in copy_spare():

	j = (mtd->oobsize / n >> 1) << 1;

In my case oobsize = 224, and n = 8 (512 bytes steps), so j == 28. This means that we must generate nand_ecclayout at run time according to the actual oobsize. This is probably also true for the 4 bit ecc case.

I think you're only partly right here. The NFC only supports 128 or 218 bytes spare area for 4k NAND flashes (initialized by BT_SPARE_SIZE). For you chip the controller uses the 218 bytes setting, so 26 bytes are read for the first 7 oob chunks each (last one: 36) As the driver assumes the real oob size of 224 bytes you get that offset of 28 instead.

So looking again on your hexdump:

	00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
	*

This is the data part, everything in order

	00001000  ff ff ff ff ff ff ff 91  c4 45 be 32 45 6f 5d b1  |.........E.2Eo].|
	00001010  b1 b9 13 61 59 7d 42 58  eb ff ff ff ff ff ff ff  |...aY}BX........|

Quoted 8 lines

Up to the 26th byte --------------------------^^ this is data coming from the flash. The following two 0xff are just what happened to be written in the NFC buffer. Starting with the four last 0xff in that line we have real data again.

	00001020  ff ff ff 91 c4 45 be 32  45 6f 5d b1 b1 b9 13 61  |.....E.2Eo]....a|

which makes the first ecc byte again the 8th of the oob chunk similar to the one above.

Thanks for your explanation.

[...]

Quoted 67 lines

While understanding the problem I produced the following (untested) patch:

drivers/mtd/nand/mxc_nand.c +27 -10

index dca63a70e783..fc835d352e1c 100644
@@ -807,32 +807,49 @@ static void mxc_nand_select_chip_v2(struct mtd_info *mtd, int chip)
 }
 
 /*
- * Function to transfer data to/from spare area.
+ * The controller splits a page into data chunks of 512 bytes + partial oob.
+ * There are writesize / 512 such chunks, the size of the partial oob parts is
+ * oobsize / #chunks rounded down to a multiple of 2. The last oob chunk then
+ * contains additionally the byte lost by rounding (if any).
+ * This function handles the needed shuffling between host->data_buf (which
+ * holds a page in natural order, i.e. writesize bytes data + oobsize bytes
+ * spare) and the NFC buffer.
  */
 static void copy_spare(struct mtd_info *mtd, bool bfrom)
 {
 	struct nand_chip *this = mtd->priv;
 	struct mxc_nand_host *host = this->priv;
 	u16 i, j;
-	u16 n = mtd->writesize >> 9;
+
+	u16 num_chunks = mtd->writesize / 512;
+
 	u8 *d = host->data_buf + mtd->writesize;
 	u8 __iomem *s = host->spare0;
-	u16 t = host->devtype_data->spare_len;
+	u16 sparebuf_size = host->devtype_data->spare_len;
 
-	j = (mtd->oobsize / n >> 1) << 1;
+	/* size of oob chunk for all but possibly the last one */
+	oob_chunk_size = (mtd->oobsize / num_chunks >> 1) << 1;
 
 	if (bfrom) {
-		for (i = 0; i < n - 1; i++)
-			memcpy32_fromio(d + i * j, s + i * t, j);
+		for (i = 0; i < num_chunks - 1; i++)
+			memcpy32_fromio(d + i * oob_chunk_size,
+					s + i * sparebuf_size,
+					oob_chunk_size);
 
 		/* the last section */
-		memcpy32_fromio(d + i * j, s + i * t, mtd->oobsize - i * j);
+		memcpy32_fromio(d + i * oob_chunk_size,
+				s + i * sparebuf_size,
+				mtd->oobsize - i * oob_chunk_size);
 	} else {
-		for (i = 0; i < n - 1; i++)
-			memcpy32_toio(&s[i * t], &d[i * j], j);
+		for (i = 0; i < num_chunks - 1; i++)
+			memcpy32_toio(&s[i * sparebuf_size],
+				      &d[i * oob_chunk_size],
+				      oob_chunk_size);
 
 		/* the last section */
-		memcpy32_toio(&s[i * t], &d[i * j], mtd->oobsize - i * j);
+		memcpy32_toio(&s[oob_chunk_size * sparebuf_size],
+			      &d[i * oob_chunk_size],
+			      mtd->oobsize - i * oob_chunk_size);
 	}
 }
 

Quoted 3 lines

What is needed now on top of this (untested and noop) change is to use the oob size the controller assumes instead of the real one and somehow explain that to the mtd layer and maintainers :-)

Can't we just limit oobsize to 128 or 218? Something like (on top of your patch):

drivers/mtd/nand/mxc_nand.c +1 -1

index cc0eb79a177c..ae63f06fe99e 100644
@@ -819,7 +819,7 @@ static void copy_spare(struct mtd_info *mtd, bool bfrom)
 {
 	struct nand_chip *this = mtd->priv;
 	struct mxc_nand_host *host = this->priv;
-	u16 i, j;
+	u16 i, oob_chunk_size, used_oobsize;
 
 	u16 num_chunks = mtd->writesize / 512;
 

@@ -828,7 +828,13 @@ static void copy_spare(struct mtd_info *mtd, bool bfrom)
 	u16 sparebuf_size = host->devtype_data->spare_len;

Quoted 91 lines · Baruch Siach

Quoted 67 lines · Uwe Kleine-König

While understanding the problem I produced the following (untested) patch:

drivers/mtd/nand/mxc_nand.c +27 -10

index dca63a70e783..fc835d352e1c 100644
@@ -807,32 +807,49 @@ static void mxc_nand_select_chip_v2(struct mtd_info *mtd, int chip)
 }
 
 /*
- * Function to transfer data to/from spare area.
+ * The controller splits a page into data chunks of 512 bytes + partial oob.
+ * There are writesize / 512 such chunks, the size of the partial oob parts is
+ * oobsize / #chunks rounded down to a multiple of 2. The last oob chunk then
+ * contains additionally the byte lost by rounding (if any).
+ * This function handles the needed shuffling between host->data_buf (which
+ * holds a page in natural order, i.e. writesize bytes data + oobsize bytes
+ * spare) and the NFC buffer.
  */
 static void copy_spare(struct mtd_info *mtd, bool bfrom)
 {
 	struct nand_chip *this = mtd->priv;
 	struct mxc_nand_host *host = this->priv;
 	u16 i, j;
-	u16 n = mtd->writesize >> 9;
+
+	u16 num_chunks = mtd->writesize / 512;
+
 	u8 *d = host->data_buf + mtd->writesize;
 	u8 __iomem *s = host->spare0;
-	u16 t = host->devtype_data->spare_len;
+	u16 sparebuf_size = host->devtype_data->spare_len;
 
-	j = (mtd->oobsize / n >> 1) << 1;
+	/* size of oob chunk for all but possibly the last one */
+	oob_chunk_size = (mtd->oobsize / num_chunks >> 1) << 1;
 
 	if (bfrom) {
-		for (i = 0; i < n - 1; i++)
-			memcpy32_fromio(d + i * j, s + i * t, j);
+		for (i = 0; i < num_chunks - 1; i++)
+			memcpy32_fromio(d + i * oob_chunk_size,
+					s + i * sparebuf_size,
+					oob_chunk_size);
 
 		/* the last section */
-		memcpy32_fromio(d + i * j, s + i * t, mtd->oobsize - i * j);
+		memcpy32_fromio(d + i * oob_chunk_size,
+				s + i * sparebuf_size,
+				mtd->oobsize - i * oob_chunk_size);
 	} else {
-		for (i = 0; i < n - 1; i++)
-			memcpy32_toio(&s[i * t], &d[i * j], j);
+		for (i = 0; i < num_chunks - 1; i++)
+			memcpy32_toio(&s[i * sparebuf_size],
+				      &d[i * oob_chunk_size],
+				      oob_chunk_size);
 
 		/* the last section */
-		memcpy32_toio(&s[i * t], &d[i * j], mtd->oobsize - i * j);
+		memcpy32_toio(&s[oob_chunk_size * sparebuf_size],
+			      &d[i * oob_chunk_size],
+			      mtd->oobsize - i * oob_chunk_size);
 	}
 }
 

Quoted 3 lines

What is needed now on top of this (untested and noop) change is to use the oob size the controller assumes instead of the real one and somehow explain that to the mtd layer and maintainers :-)

Can't we just limit oobsize to 128 or 218? Something like (on top of your patch):

drivers/mtd/nand/mxc_nand.c +1 -1

index cc0eb79a177c..ae63f06fe99e 100644
@@ -819,7 +819,7 @@ static void copy_spare(struct mtd_info *mtd, bool bfrom)
 {
 	struct nand_chip *this = mtd->priv;
 	struct mxc_nand_host *host = this->priv;
-	u16 i, j;
+	u16 i, oob_chunk_size, used_oobsize;
 
 	u16 num_chunks = mtd->writesize / 512;
 

@@ -828,7 +828,13 @@ static void copy_spare(struct mtd_info *mtd, bool bfrom)
 	u16 sparebuf_size = host->devtype_data->spare_len;
 

drivers/mtd/nand/mxc_nand.c static void copy_spare(struct mtd_info *mtd, bool bfrom) · line 837

Quoted 9 lines

drivers/mtd/nand/mxc_nand.c +7 -1

 	/* size of oob chunk for all but possibly the last one */
-	oob_chunk_size = (mtd->oobsize / num_chunks >> 1) << 1;
+	if (mtd->oobsize >= 218)
+		used_oobsize = 218;
+	else if (mtd->oobsize >= 128)
+		used_oobsize = 128;
+	else
+		used_oobsize = mtd->oobsize;
+	oob_chunk_size = (used_oobsize / num_chunks >> 1) << 1;

something like that, yes. I'd make that conditional on writesize=4k and the register setting that is actually used by the controler however.

Also you need to adapt the reading/setting of the last chunk to make use of used_oobsize instead of mtd->oobsize.

While understanding the problem I produced the following (untested) patch:

drivers/mtd/nand/mxc_nand.c +27 -10

index dca63a70e783..fc835d352e1c 100644
@@ -807,32 +807,49 @@ static void mxc_nand_select_chip_v2(struct mtd_info *mtd, int chip)
 }
 
 /*
- * Function to transfer data to/from spare area.
+ * The controller splits a page into data chunks of 512 bytes + partial oob.
+ * There are writesize / 512 such chunks, the size of the partial oob parts is
+ * oobsize / #chunks rounded down to a multiple of 2. The last oob chunk then
+ * contains additionally the byte lost by rounding (if any).
+ * This function handles the needed shuffling between host->data_buf (which
+ * holds a page in natural order, i.e. writesize bytes data + oobsize bytes
+ * spare) and the NFC buffer.
  */
 static void copy_spare(struct mtd_info *mtd, bool bfrom)
 {
 	struct nand_chip *this = mtd->priv;
 	struct mxc_nand_host *host = this->priv;
 	u16 i, j;
-	u16 n = mtd->writesize >> 9;
+
+	u16 num_chunks = mtd->writesize / 512;
+
 	u8 *d = host->data_buf + mtd->writesize;
 	u8 __iomem *s = host->spare0;
-	u16 t = host->devtype_data->spare_len;
+	u16 sparebuf_size = host->devtype_data->spare_len;
 
-	j = (mtd->oobsize / n >> 1) << 1;
+	/* size of oob chunk for all but possibly the last one */
+	oob_chunk_size = (mtd->oobsize / num_chunks >> 1) << 1;
 
 	if (bfrom) {
-		for (i = 0; i < n - 1; i++)
-			memcpy32_fromio(d + i * j, s + i * t, j);
+		for (i = 0; i < num_chunks - 1; i++)
+			memcpy32_fromio(d + i * oob_chunk_size,
+					s + i * sparebuf_size,
+					oob_chunk_size);
 
 		/* the last section */
-		memcpy32_fromio(d + i * j, s + i * t, mtd->oobsize - i * j);
+		memcpy32_fromio(d + i * oob_chunk_size,
+				s + i * sparebuf_size,
+				mtd->oobsize - i * oob_chunk_size);
 	} else {
-		for (i = 0; i < n - 1; i++)
-			memcpy32_toio(&s[i * t], &d[i * j], j);
+		for (i = 0; i < num_chunks - 1; i++)
+			memcpy32_toio(&s[i * sparebuf_size],
+				      &d[i * oob_chunk_size],
+				      oob_chunk_size);
 
 		/* the last section */
-		memcpy32_toio(&s[i * t], &d[i * j], mtd->oobsize - i * j);
+		memcpy32_toio(&s[oob_chunk_size * sparebuf_size],
+			      &d[i * oob_chunk_size],
+			      mtd->oobsize - i * oob_chunk_size);
 	}
 }

Hi Uwe,

Quoted 2 lines · Uwe Kleine-König

While understanding the problem I produced the following (untested) patch:

I intend to use this patch as part of a series fixing the large oob issue. Can you add your SoB please?

drivers/mtd/nand/mxc_nand.c static void mxc_nand_select_chip_v2(struct mtd_info *mtd, int chip) · line 831

Quoted 3 lines

drivers/mtd/nand/mxc_nand.c +2 -1

-	j = (mtd->oobsize / n >> 1) << 1;
+	/* size of oob chunk for all but possibly the last one */
+	oob_chunk_size = (mtd->oobsize / num_chunks >> 1) << 1;

Quoted 6 lines · Baruch Siach

Quoted 2 lines · Uwe Kleine-König

While understanding the problem I produced the following (untested) patch:

I intend to use this patch as part of a series fixing the large oob issue. Can you add your SoB please?

You can even get a commit log :-)

mtd: nand: mxc_nand: cleanup copy_spare function

To give people without the reference manual at hand a chance to understand the spare are handling of the i.MX nand controller, improve commenting, naming of variables and coding style.

No functional change intended.

Signed-off-by Uwe Kleine-König <[email protected]>

Quoted 3 lines

drivers/mtd/nand/mxc_nand.c static void mxc_nand_select_chip_v2(struct mtd_info *mtd, int chip) · line 831

Quoted 3 lines

drivers/mtd/nand/mxc_nand.c +2 -1

-	j = (mtd->oobsize / n >> 1) << 1;
+	/* size of oob chunk for all but possibly the last one */
+	oob_chunk_size = (mtd->oobsize / num_chunks >> 1) << 1;

BTW, I considered to change this to:

	oob_chunk_size = (mtd->oobsize / num_chunks) & ~1;

which is IMHO still clearer. Feel free to squash this in my patch if you agree.

Hi Shawn, Sascha, all,

I'm trying to get nand_ecclayout right on i.MX25 with the Micron MT29F8G08ABABA (page size: 4096, oob size: 224). The large OOB size allows using hardware ecc strength of 8bit per ecc step (512 bytes). The mxc_nand driver code (get_eccsize()) and the reference manual seems to indicate that enabling 8 bit ecc mode requires 26 oob bytes per ecc step.

Quoted 5 lines · Baruch Siach

I'm trying to get nand_ecclayout right on i.MX25 with the Micron MT29F8G08ABABA (page size: 4096, oob size: 224). The large OOB size allows using hardware ecc strength of 8bit per ecc step (512 bytes). The mxc_nand driver code (get_eccsize()) and the reference manual seems to indicate that enabling 8 bit ecc mode requires 26 oob bytes per ecc step.

Note sure if these really is relevant to this thread, but using BCH, 8 bit error correction per 512 byte ECC step requires 13 bytes of ECC codes per ECC step. (Depending on other factors, this will actually fit nicely even in a 64 byte OOB).

Quoted 7 lines · Ricard Wanderlof

Quoted 5 lines · Baruch Siach

I'm trying to get nand_ecclayout right on i.MX25 with the Micron MT29F8G08ABABA (page size: 4096, oob size: 224). The large OOB size allows using hardware ecc strength of 8bit per ecc step (512 bytes). The mxc_nand driver code (get_eccsize()) and the reference manual seems to indicate that enabling 8 bit ecc mode requires 26 oob bytes per ecc step.

Quoted 4 lines

Note sure if these really is relevant to this thread, but using BCH, 8 bit error correction per 512 byte ECC step requires 13 bytes of ECC codes per ECC step. (Depending on other factors, this will actually fit nicely even in a 64 byte OOB).

The i.MX25 NFC hardware uses Reed-Solomon ECC, according to the documentation, and needs 18 ECC bytes per 512 step. But even with 13 bytes per step, you'll need at least 104 (13*8) OOB bytes for a 4K page.