About the mxc nand driver

About the mxc nand driver

[Sascha Hauer]

Hi all,

I frequently get reports about bugs in the Freescale mxc nand driver.
Juergen Beisert and me invested some time to see what's going on in this
driver, so here are some results.

> 
> /* OOB placement block for use with hardware ecc generation */
> static struct nand_ecclayout nand_hw_eccoob_8 = {
> 	.eccbytes = 5,
> 	.eccpos = {6, 7, 8, 9, 10},
> 	.oobfree = {{0, 5}, {11, 5}, }
> };
> 
> static struct nand_ecclayout nand_hw_eccoob_16 = {
> 	.eccbytes = 5,
> 	.eccpos = {6, 7, 8, 9, 10},
> 	.oobfree = {{0, 5}, {11, 5}, }
> };
> 
> static struct nand_ecclayout nand_hw_eccoob_64 = {
> 	.eccbytes = 20,
> 	.eccpos = {6, 7, 8, 9, 10, 22, 23, 24, 25, 26,
> 		   38, 39, 40, 41, 42, 54, 55, 56, 57, 58},
> 	.oobfree = {{2, 4}, {11, 10}, {27, 10}, {43, 10}, {59, 5}, }
> };
> 
> 
> /* Define some generic bad / good block scan pattern which are used
>  * while scanning a device for factory marked good / bad blocks. */
> static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
> 
> static struct nand_bbt_descr smallpage_memorybased = {
> 	.options = NAND_BBT_SCAN2NDPAGE,
> 	.offs = 5,
> 	.len = 1,
> 	.pattern = scan_ff_pattern
> };

This has been introduced by Eric Benard. He said he needs this to make
the driver work on 2k flashes because otherwise all blocks on his nand
are marked as bad. Could it be that Eric somehow destroyed his oob data?
Without this largepage_memorybased would be used which has .offs = 0, .len = 2.
When Eric does not have 0xff,0xff here (but really *should* have, but
has used a broken driver some time ago) all blocks would be marked as bad.

Looking at it, the original Freescale driver passes .offset = 0, length = 5
to the 2k ecclayout, so jffs2 will probably overwrite bytes 0 and 1.
This does not hurt the Freescale driver since it uses a flash based
bbt but breaks when he starts using the Mainline driver which does not
use a flash based bbt.

> 
> 	if (this->ecc.mode == NAND_ECC_HW) {
> 		switch (mtd->oobsize) {
> 		case 8:
> 			this->ecc.layout = &nand_hw_eccoob_8;


This has been introduced by Vladimir Barinov and seems wrong. The
original Freescale driver has a nand_hw_eccoob_8 and a
nand_hw_eccoob_16, but it is used for 8bit bus width vs. 16bit bus width
and *not* for 8 byte oob size (do we have these chips anyway nowadays?)

The original Freescale driver uses this ecc layout for 8/16 bit busses:

static struct nand_ecclayout nand_hw_eccoob_8 = {
	.eccbytes = 5,
	.eccpos = {6, 7, 8, 9, 10},
	.oobfree = {{0, 5}, {11, 5}}
};

static struct nand_ecclayout nand_hw_eccoob_16 = {
	.eccbytes = 5,
	.eccpos = {6, 7, 8, 9, 10},
	.oobfree = {{0, 6}, {12, 4}}
};

I understand the first one, but doesn't the second one specify a oobfree
which overlaps with ecc data?


When I understand the mtd layer correctly it uses byte 5 of the oob data
for bad block detection, but this is passed to oobfree causing potential
corruption, so we should probably skip byte 5 from oobfree.

The original Freescale driver uses NAND_USE_FLASH_BBT and
NAND_BBT_CREATE | NAND_BBT_WRITE, so we do not need to store the bad
block information after the first scan anymore. Should we use this here
aswell?


> 			break;
> 		case 16:
> 			this->ecc.layout = &nand_hw_eccoob_16;
> 			break;
> 		case 64:
> 			this->ecc.layout = &nand_hw_eccoob_64;
> 			break;
> 		default:
> 			/* page size not handled by HW ECC */
> 			/* switching back to soft ECC */
> 			this->ecc.size = 512;
> 			this->ecc.bytes = 3;
> 			this->ecc.layout = &nand_hw_eccoob_8;
> 			this->ecc.mode = NAND_ECC_SOFT;
> 			this->ecc.calculate = NULL;
> 			this->ecc.correct = NULL;
> 			this->ecc.hwctl = NULL;
> 			tmp = readw(host->regs + NFC_CONFIG1);
> 			tmp &= ~NFC_ECC_EN;
> 			writew(tmp, host->regs + NFC_CONFIG1);
> 			break;

Which default does this code handle? Has anyone tested it?


As a conclusion I would:

- revert Erics patch
- make sure we do not pass byte 5 for small pages to oobfree
- make sure we do not pass bytes 0/1 for large pages to oobfree
- switch to use a flash based bbt?

I think doing this we break several systems out there, but do we have
another chance?

As a general note I really want to make this driver work on i.MX35/25. I
already have several patches in my queue, but they have to be rebased on
the current driver and I'm still working on it.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: About the mxc nand driver

[Juergen Beisert]

On Dienstag, 20. Oktober 2009, Sascha Hauer wrote:
> [...]
> The original Freescale driver uses this ecc layout for 8/16 bit busses:
>
> static struct nand_ecclayout nand_hw_eccoob_8 = {
> 	.eccbytes = 5,
> 	.eccpos = {6, 7, 8, 9, 10},
> 	.oobfree = {{0, 5}, {11, 5}}
> };

This one is bogus, as it describes an 8 byte OOB with more than 8 bytes... In 
the current driver this layout is used for soft ECC. Maybe this is the 
reason, why there wasn't any failure messages, because all people are using 
hardware ECC.

jbe

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-8766-939 228     |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |

Re: About the mxc nand driver

[Sascha Hauer]

On Tue, Oct 20, 2009 at 03:28:54PM +0200, Juergen Beisert wrote:
> On Dienstag, 20. Oktober 2009, Sascha Hauer wrote:
> > [...]
> > The original Freescale driver uses this ecc layout for 8/16 bit busses:
> >
> > static struct nand_ecclayout nand_hw_eccoob_8 = {
> > 	.eccbytes = 5,
> > 	.eccpos = {6, 7, 8, 9, 10},
> > 	.oobfree = {{0, 5}, {11, 5}}
> > };
> 
> This one is bogus, as it describes an 8 byte OOB with more than 8 bytes...

Well, in current mainline driver it is, but in the Freescale driver
where this snipped has been copied from it is used for 8bit busses
rather than 8bit oob size.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: About the mxc nand driver

[Gianluca Renzi]

On Tuesday 20 October 2009 15:11:58 Sascha Hauer wrote:
> Hi all,
>
> I frequently get reports about bugs in the Freescale mxc nand driver.
> Juergen Beisert and me invested some time to see what's going on in this
> driver, so here are some results.

The problem is caused by the fact the FreeScale's NAND Core Controller manages 
the 2K Largepage NAND as 4 SmallPage pages joined with their own ECC codes and 
badblock markers positions i.e. the smallpage badblock marker is located at 
6th byte of the oobpage data (offset 5) (repeated for each page).
To keep this NAND Controller fancy situation under kernel/mtd's control there 
are 2 ways:
1- Keep the location of the badblock marker at the first byte of oobpage data 
(offset 0) as mtd driver expects to be the badblock marker, but the mxc_nand 
driver MUST swap those locations *before* sending them to mtd.
2- Tell to mtd through its badblock descriptors to use 6th location (offset 5) 
as a badblock marker instead of the standard offset 0 (for each smallpage page 
area).
In the latter case, the hardware NAND Flash vendor MUST follow the same 
badblock marker layout (i.e.: Numonyx NAND02G-B2D 2-Gbit, 2112-byte/1056-word 
page, multiplane architecture, 1.8 V or 3 V, SLC NAND flash memories is using 
both locations as badblock marker (offset 0 and offset 5) of oobpage data)

In the first case we need to have a code like this to act the swap:
/*
 * This function does the trick of swapping the 464th byte in the last RAM
 * buffer in the main area with the 0th byte in the spare area. This seems
 * to be the optimal way of addressing the NFC imcompatibility problem with
 * the NAND flash out of factory in terms of BI field.
 * Note: this function only operates on the NFC's internal RAM buffers and
 * for 2K page only.
 */
static void mxc_swap_2k_bi_main_sp(void)
{
	u16 tmp1, tmp2, new_tmp1;
	tmp1 = BAD_BLK_MARKER_464;
	tmp2 = BAD_BLK_MARKER_SP_0;

	new_tmp1 = (tmp1 & 0xFF00) | (tmp2 >> 8);
	tmp2 = (tmp1 << 8) | (tmp2 & 0xFF);
	BAD_BLK_MARKER_464 = new_tmp1;
	BAD_BLK_MARKER_SP_0 = tmp2;
}
and you can use the standard layout of mtd.

We are using the second case, and to make it works we had to use the following 
ECCLayout as suggested by FreeScale using the HWECC:

/*
 * OOB placement block for use with hardware ecc generation
 */
static struct nand_ecclayout nand_hw_eccoob_8 = {
	.eccbytes = 5,
	.eccpos = {6, 7, 8, 9, 10},
	.oobfree = {{0, 5}, {11, 5}}
};

static struct nand_ecclayout nand_hw_eccoob_16 = {
	.eccbytes = 5,
	.eccpos = {6, 7, 8, 9, 10},
	.oobfree = {{0, 6}, {12, 4}}
};

static struct nand_ecclayout nand_hw_eccoob_2k = {
	.eccbytes = 20,
	.eccpos = {6, 7, 8, 9, 10, 22, 23, 24, 25, 26,
		   38, 39, 40, 41, 42, 54, 55, 56, 57, 58},
	.oobfree = {
		    {.offset = 0,
		     .length = 5},

		    {.offset = 11,
		     .length = 10},

		    {.offset = 27,
		     .length = 10},

		    {.offset = 43,
		     .length = 10},

		    {.offset = 59,
		     .length = 5}
		    }
};

We hope to see a common way to ensure this NAND Controller in the next kernel 
release, hopefully selected by a Kconfig entry (so to keep both cases valid).

Best regards,
-- 
           ,,,
          (o o)
======oOO==(_)==OOo======

Gianluca Renzi
R&D
phone: +39.0542.609120
fax: +39.0542.609212

      .oooO  Oooo.
======(   )==(   )=======
       \ (    ) /
        \_)  (_/

Re: About the mxc nand driver

[Juergen Beisert]

On Mittwoch, 21. Oktober 2009, Gianluca Renzi wrote:
> On Tuesday 20 October 2009 15:11:58 Sascha Hauer wrote:
> > Hi all,
> >
> > I frequently get reports about bugs in the Freescale mxc nand driver.
> > Juergen Beisert and me invested some time to see what's going on in this
> > driver, so here are some results.
>
> The problem is caused by the fact the FreeScale's NAND Core Controller
> manages the 2K Largepage NAND as 4 SmallPage pages joined with their own
> ECC codes and badblock markers positions i.e. the smallpage badblock marker
> is located at 6th byte of the oobpage data (offset 5) (repeated for each
> page).

I can currently speak only about the i.MX35 NFC. But it handles larger pages 
than 512 byte in the same way the i.MX27 NFC does. The tables in the 
datasheet suggests they define a fixed location for the bad block marker in 
the spare area and this will be repeated 4 times for a 2048 byte page. But it 
seems the NFS does not touch any other byte than the ones defined for ECC 
usage.

jbe

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-8766-939 228     |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |

Re: About the mxc nand driver

[alfred steele]

> I can currently speak only about the i.MX35 NFC. But it handles larger pages
> than 512 byte in the same way the i.MX27 NFC does. The tables in the
> datasheet suggests they define a fixed location for the bad block marker in
> the spare area and this will be repeated 4 times for a 2048 byte page. But it
> seems the NFS does not touch any other byte than the ones defined for ECC
> usage.
So does the mxc_nand driver without "Saschas" latest patches work for
NAND transactions on the mx35 nfc. This probably suggests that the for
a 4K page size NAND, it would boil down to 8 cycles. Does that still
hold true?

-Alfred

Re: About the mxc nand driver

[Juergen Beisert]

On Donnerstag, 22. Oktober 2009, alfred steele wrote:
> > I can currently speak only about the i.MX35 NFC. But it handles larger
> > pages than 512 byte in the same way the i.MX27 NFC does. The tables in
> > the datasheet suggests they define a fixed location for the bad block
> > marker in the spare area and this will be repeated 4 times for a 2048
> > byte page. But it seems the NFS does not touch any other byte than the
> > ones defined for ECC usage.
>
> So does the mxc_nand driver without "Saschas" latest patches work for
> NAND transactions on the mx35 nfc.

The current mainline mxc nand driver does not work on i.MX35. The i.MX35 NFC 
is of the next generation. We are on the way to merge the new features into 
this driver. 

> This probably suggests that the for 
> a 4K page size NAND, it would boil down to 8 cycles. Does that still
> hold true?

Yes, a 4 kiB page would generate 8 stripes of ECC data, 16 bytes each. 7 free, 
9 for ECC.

AFAIK i.MX35 (4 symbols ECC):

512 byte page:

* OOB offset: 00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
*      Usage: |<----------------->|<----------------------->|
*                      User                   ECC

2048 byte page:

* OOB offset: 00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
* OOB offset: 10 11 12 13 14 15 16 17 18 19 1A 1B 1C 1D 1E 1F
* OOB offset: 20 21 22 23 24 25 26 27 28 29 2A 2B 2C 2D 2E 2F
* OOB offset: 30 31 32 33 34 35 36 37 38 39 3A 3B 3C 3D 3E 3F
*      Usage: |<----------------->|<----------------------->|
*                       User                  ECC

4096 byte page:

* OOB offset: 00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
* OOB offset: 10 11 12 13 14 15 16 17 18 19 1A 1B 1C 1D 1E 1F
*             .. .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
* OOB offset: 60 61 62 63 64 65 66 67 68 69 6A 6B 6C 6D 6E 6F
* OOB offset: 70 71 72 73 74 75 76 77 78 79 7A 7B 7C 7D 7E 7F
*      Usage: |<----------------->|<----------------------->|
*                       User                  ECC

But recent 4 kiB NANDs also provide 218 bytes of OOB. In this case this NFS 
also supports 18 byte of ECC. But currently I'm not sure about its layout.

jbe

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-8766-939 228     |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |

Re: About the mxc nand driver

[Juergen Beisert]

On Dienstag, 20. Oktober 2009, Berland, Mathieu wrote:
> =============== About Freescale driver (NOT mainline driver)
>
> >The original Freescale driver uses this ecc layout for 8/16 bit busses:
> >
> >static struct nand_ecclayout nand_hw_eccoob_8 = {
> >	.eccbytes = 5,
> >	.eccpos = {6, 7, 8, 9, 10},
> >	.oobfree = {{0, 5}, {11, 5}}
> >};
> >
> >static struct nand_ecclayout nand_hw_eccoob_16 = {
> >	.eccbytes = 5,
> >	.eccpos = {6, 7, 8, 9, 10},
> >	.oobfree = {{0, 6}, {12, 4}}
> >};
> >
> >I understand the first one, but doesn't the second one specify a oobfree
>
> which overlaps with ecc data?
>
> ECC is not overlapped for 16 bits since the second fields of oobfree are
> lengths which is confusing.
>
> These settings comply with the MXC Nand Flash controller specification, at
> least with mine which might be outdated :
>
> 8 bits bus :
> 	Bad block marker @ 5
> 	ECC @ [6..10] (understand from 6 to 10, both included) = {6, 7, 8,
> 9, 10}
> 	Free @ [0..4] [11..15] = {{0, 5}, {11, 5}}
>
> 16 bits bus :
> 	Bad block marker @ 11
> 	ECC @ [6..10] = {6, 7, 8, 9, 10}
> 	Free @ [0..5] [12..15] = {{0, 6}, {12, 4}}
>
> The datasheet says :
> "The host can use all of the spare area except for the BI (bad block
> marker) and ECC code areas".
>
> The ECC is managed by the hardware. It seems that the bad block marker is
> not (not sure).

Bad block marker is handled by the NAND framework. And in the most worst case 
this bad block marker is overwritten by the ECC hardware...

> What's the problem with the old Freescale's driver ?
> The problem is that Flash chips with 2K pages are not really handled.
>
>
> =============== About "SPARE AREA ASSIGNMENT STANDARD" from Samsung (?)
>
> I found one document in Samsung website. It is said :
>
> Small pages (16B OOB) && 8 bits bus :
> 	Bad block marker @ 5 => In my Samsung chip, all the bytes of the OOB
> are set to 0 when a block is marked bad in factory.
> 	ECC @ [6..10] => Chip founders don't care I guess.

It seems every hardware vendor does it in a different way.

> Small pages (16B OOB) && 16 bits bus :
> 	Bad block marker @ 11
> 	ECC @ [6..10]

You must register your own bad block marker structure 
(member 'badblock_pattern' in struct nand_chip) to use offset 11 instead of 
the offset 5.

[...]
> =============== About the mainline driver
>
> Generic BBT code used by mxc_nand.c :
>
> Small pages
> 	Bad block marker @ 5
>
> Large pages
> 	Bad block marker @ [0..1]

These are the defaults if the driver keeps member 'badblock_pattern' in struct 
nand_chip NULL.

> Here is what Vladimir did :
>
> 8 bytes OOB or SW ECC (?) :
> 	Bad block marker @ [5]

Default for page size equal to 512.

> 	ECC @ [6..10]
> 	Free @ [0..4] [11..15]
>
> Small pages (16B OOB) :
> 	Bad block marker @ [5]

Default for page size equal to 512.

> 	ECC @ [6..10]
> 	Free @ [0..4] [11..15]
>
> Large pages (64B OOB) :
> 	Bad block marker @ [0..1]

Default if page size is larger than 512 bytes.

> 	ECC @ [6..10] [22..26] [38..42] [54..58]
> 	Free @ [2..5] [11..20] [27..36] [43..52] [59..63]
> [...]
> =============== Some improvements ?
>
> The only thing that's worrying me is the fact that the Nand Flash
> Controller might use these bad block marker bytes. In this case, we could
> experience problems.

That's why I'm using a separate bbt descriptor.

> I have somes proposals which should be compatible with current Flash
> loaders. This is *only* if someone is experiencing problems during the
> lifecyle of boards :
>
> 1)
>
> 8 bytes OOB :
> 	No more supported
>
> Small pages (16B OOB) && 8 bits bus :
> 	Requirement : keep default bad block marker location, same as today
>
> 	Bad block marker @ 5
> 	ECC @ [6..10]
> 	Free @ [0..4] [11..15]

Bad block marker at offset 5 is described in structure 'smallpage_flashbased' 
in 'nand_bbt.c' and will be used if the driver doesn't touch 
the 'badblock_pattern' structure member.

> Small pages (16B OOB) && 16 bits bus :
> 	Requirement : keep default bad block marker location, same as today
> 	Requirement : byte 11 is not free anymore because it might be used
> by HW for bad block flag (don't known)
>
> 	Bad block marker @ 5
> 	ECC @ [6..10]
> 	Free @ [0..4] [12..15]

Same here. because 'nand_bbt.c' does not distinguish between date width.

> Large pages (64B OOB) && 8 bits bus :
> 	Requirement : keep default bad block marker location, same as today
> 	Requirement : byte 5+16*n are not free because it might be used by
> HW for bad block flag (don't known)
>
> 	Bad block marker @ [0..1]

Yes, because here 'largepage_flashbased' is used for all page sizes larger 
than 512 bytes (until the driver touches the 'badblock_pattern' structure 
member).

> 	ECC & [6..10] [22..26] [38..42] [54..58]
> 	Free @ [2..4] [11..20] [27..36] [43..52] [59..63]
>
> Large pages (64B OOB) && 16 bits bus :
> 	Requirement : keep default bad block marker location, same as today
> 	Requirement : byte 11+16*n are not free because it might be used by
> HW for bad block flag (don't known)
>
> 	Bad block marker @ [0..1]

Yes. No difference between 8 or 16 bits.

jbe

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-8766-939 228     |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |