OneNAND: Simple Bad Block handling support

Hi

As I mentioned before we want to use another bad block handling method.

But now we have to push OneNAND to official MTD not experimental so I implement simple bad block table (BBT) routine
It's almost same as NAND does.
After merge. we have to split BBT into core and chip specific code. As the first step, I add bad block header file (bbm.h). It has bad block data structure, macro and etc.
I don't know which directory are used as bbt_core.c(?) location?

Please let me know how to use as official OneNAND MTD support and when?

Any comments are welcome.

Thank you.

Kyungmin Park

@@ -3,7 +3,9 @@
 #
 
 # Core functionality.
-obj-$(CONFIG_MTD_ONENAND)		+= onenand_base.o
+obj-$(CONFIG_MTD_ONENAND)		+= onenand.o
 
 # Board specific.
 obj-$(CONFIG_MTD_ONENAND_OMAP)		+= omap-onenand.o
+
+onenand-objs = onenand_base.o onenand_bbt.o

@@ -14,6 +14,7 @@
 
 #include <linux/spinlock.h>
 #include <linux/mtd/onenand_regs.h>
+#include <linux/mtd/bbm.h>
 
 #define MAX_BUFFERRAM		2
 

@@ -67,10 +68,14 @@ struct onenand_bufferram {
  * @param wait		[REPLACEABLE] hardware specific function for wait on ready
  * @param read_bufferram	[REPLACEABLE] hardware specific function for BufferRAM Area
  * @param write_bufferram	[REPLACEABLE] hardware specific function for BufferRAM Area
+ * @param read_word	[REPLACEABLE] hardware specific function for read register of OneNAND
+ * @param write_word	[REPLACEABLE] hardware specific function for write register of OneNAND
+ * @param scan_bbt	[REPLACEALBE] hardware specific function for scaning Bad block Table
  * @param chip_lock	[INTERN] spinlock used to protect access to this structure and the chip
  * @param wq		[INTERN] wait queue to sleep on if a OneNAND operation is in progress
  * @param state		[INTERN] the current state of the OneNAND device
  * @param autooob	[REPLACEABLE] the default (auto)placement scheme
+ * @param bbm		[REPLACEABLE] pointer to Bad Block Management 
  * @param priv		[OPTIONAL] pointer to private chip date
  */
 struct onenand_chip {

@@ -96,6 +101,8 @@ struct onenand_chip {
 	unsigned short (*read_word)(void __iomem *addr);
 	void (*write_word)(unsigned short value, void __iomem *addr);
 	void (*mmcontrol)(struct mtd_info *mtd, int sync_read);
+	int (*block_markbad)(struct mtd_info *mtd, loff_t ofs);
+	int (*scan_bbt)(struct mtd_info *mtd);
 
 	spinlock_t		chip_lock;
 	wait_queue_head_t	wq;

@@ -103,6 +110,8 @@ struct onenand_chip {
 
 	struct nand_oobinfo	*autooob;
 
+	void 			*bbm;
+
 	void			*priv;
 };
 

Hi,

trying patching a today's snapshot I get:

patch -p1 < onenand-bbm.patch
patching file drivers/mtd/onenand/Makefile
Hunk #1 FAILED at 3.
1 out of 1 hunk FAILED -- saving rejects to file drivers/mtd/onenand/Makefile.rej patching file drivers/mtd/onenand/onenand_base.c
Hunk #1 FAILED at 311.
Hunk #2 succeeded at 1033 (offset -29 lines).
Hunk #3 succeeded at 1101 (offset -29 lines).
Hunk #4 succeeded at 1158 (offset -29 lines).
Hunk #5 succeeded at 1444 (offset -29 lines).
Hunk #6 succeeded at 1504 (offset -35 lines).
1 out of 6 hunks FAILED -- saving rejects to file drivers/mtd/onenand/onenand_base.c.rej patching file drivers/mtd/onenand/onenand_bbt.c
patching file include/linux/mtd/bbm.h
patching file include/linux/mtd/onenand.h
Hunk #3 FAILED at 101.
Hunk #4 succeeded at 109 (offset -1 lines).
1 out of 4 hunks FAILED -- saving rejects to file include/linux/mtd/onenand.h.rej

.rej files are available.
Perhaps it's a silly fault... I'm new to diff/patch, sorry.

Regards,
Bernhard

Hi

Which directory are you patch? MTD-CVS or kernel?

In MTD-CVS I also have similar message

$ patch --dry-run -p1 < ../onenand-bbm.patch
patching file drivers/mtd/onenand/Makefile
Hunk #1 FAILED at 3.
1 out of 1 hunk FAILED -- saving rejects to file
drivers/mtd/onenand/Makefile.rej
patching file drivers/mtd/onenand/onenand_base.c
Hunk #1 FAILED at 311.
Hunk #2 succeeded at 1033 (offset -29 lines).
Hunk #4 succeeded at 1158 (offset -29 lines).
Hunk #6 succeeded at 1504 (offset -35 lines).
1 out of 6 hunks FAILED -- saving rejects to file
drivers/mtd/onenand/onenand_base.c.rej
patching file drivers/mtd/onenand/onenand_bbt.c
patching file include/linux/mtd/bbm.h
patching file include/linux/mtd/onenand.h
Hunk #3 FAILED at 101.
Hunk #4 succeeded at 109 (offset -1 lines).
1 out of 4 hunks FAILED -- saving rejects to file
include/linux/mtd/onenand.h.rej

I think the patch scenario is following

1. patching onenand-syncread.patch (check previous e-mail)
2. onenand-bbm.patch
3. omap-onenand.patch

Try again and If you have same problem. please let me know

In my test there's two .rej file

./drivers/mtd/onenand/Makefile.rej
./drivers/mtd/onenand/onenand_base.c.rej

***************
*** 3,9 ****
#

# Core functionality.
- obj-$(CONFIG_MTD_ONENAND) += onenand_base.o

# Board specific.
obj-$(CONFIG_MTD_ONENAND_OMAP) += omap-onenand.o
--- 3,11 ----
#

# Core functionality.
+ obj-$(CONFIG_MTD_ONENAND) += onenand.o

# Board specific.
obj-$(CONFIG_MTD_ONENAND_OMAP) += omap-onenand.o
+
+ onenand-objs = onenand_base.o onenand_bbt.o

***************
*** 311,329 ****

        ctrl = this->read_word(this->base + ONENAND_REG_CTRL_STATUS);

        if (ctrl & ONENAND_CTRL_ERROR) {
-               DEBUG(MTD_DEBUG_LEVEL0, "onenand_wait: controller error =
0x%04x", ctrl);
-               return -EAGAIN;
        }

        if (ctrl & ONENAND_CTRL_LOCK) {
-               DEBUG(MTD_DEBUG_LEVEL0, "onenand_wait: it's locked error =
0x%04x", ctrl);
-               return -EIO;
        }

        if (interrupt & ONENAND_INT_READ) {
                ecc = this->read_word(this->base + ONENAND_REG_ECC_STATUS);
                if (ecc & ONENAND_ECC_2BIT_ALL) {
-                       DEBUG(MTD_DEBUG_LEVEL0, "onenand_wait: ECC error =
0x%04x", ecc);
                        return -EBADMSG;
                }
        }
--- 311,331 ----
        ctrl = this->read_word(this->base + ONENAND_REG_CTRL_STATUS);

        if (ctrl & ONENAND_CTRL_ERROR) {

+ /* It maybe occur at initial bad block */
+ DEBUG(MTD_DEBUG_LEVEL0, "onenand_wait: controller error =
0x%04x\n", ctrl);
+ /* Clear other interrupt bits for preventing ECC error */

+               interrupt &= ONENAND_INT_MASTER;
        }

        if (ctrl & ONENAND_CTRL_LOCK) {
+               DEBUG(MTD_DEBUG_LEVEL0, "onenand_wait: it's locked error =
0x%04x\n", ctrl);
+               return -EACCES;
        }

        if (interrupt & ONENAND_INT_READ) {
                ecc = this->read_word(this->base + ONENAND_REG_ECC_STATUS);
                if (ecc & ONENAND_ECC_2BIT_ALL) {
+                       DEBUG(MTD_DEBUG_LEVEL0, "onenand_wait: ECC error =
0x%04x\n", ecc);
                        return -EBADMSG;
                }
        }

I tried to solve this problem

Thank you

Kyungmin Park

I ran it outside the kernel tree:
[...]/mtd/> patch -p1 onenand-bbm.patch
Trying it inside the kernel tree results in the same errors:
/usr/src/linux> patch -p1 /[...]/mtd/onenand-bbm.patch

This order works better in some way: fewer fails :-) But errors anyway.

Did you try to apply the patches? In which way exactly? Applying them in the correct order helps a bit, but I can't see any difference between inside/outside kernel tree. Should I?... But I'm not sure, because:

:-)

Ah, one thing: inline mail attaching onenand-syncread.patch causes some incorrect line wrappings which have to be corrected manually before running patch. Perhaps it would be better to send them really as .patch files.

Regards,
Bernhard

OK, sync read patch seems to work when applied directly to the kernel tree, e.g.

BBM patch, e.g.

runs through nearly successful. I get a rej in onenand_base.c, line 311 (see attached file). On line 315 you are looking for "return -EAGAIN;", but there's a "return -EIO;" -> patch fails. I don't know what's wrong with line 319/320, the patch fails anyway... Same thing for line 326.

Is the following code valid (assuming patch has been run)?

1) interrupt &= ONENAND_INT_MASTER instead of return?
2) return -EACCES instead of return -EIO?

Bernhard

*************** static int onenand_wait(struct mtd_info *** 311,329 ****
ctrl = this->read_word(this->base + ONENAND_REG_CTRL_STATUS);

if (ctrl & ONENAND_CTRL_ERROR) {
- DEBUG(MTD_DEBUG_LEVEL0, "onenand_wait: controller error = 0x%04x", ctrl);
- return -EAGAIN;
}

if (ctrl & ONENAND_CTRL_LOCK) {
- DEBUG(MTD_DEBUG_LEVEL0, "onenand_wait: it's locked error = 0x%04x", ctrl);
- return -EIO;
}

if (interrupt & ONENAND_INT_READ) {
ecc = this->read_word(this->base + ONENAND_REG_ECC_STATUS); if (ecc & ONENAND_ECC_2BIT_ALL) {
- DEBUG(MTD_DEBUG_LEVEL0, "onenand_wait: ECC error = 0x%04x", ecc);
return -EBADMSG;
}
}
--- 311,331 ----
ctrl = this->read_word(this->base + ONENAND_REG_CTRL_STATUS);

if (ctrl & ONENAND_CTRL_ERROR) {
+ /* It maybe occur at initial bad block */
+ DEBUG(MTD_DEBUG_LEVEL0, "onenand_wait: controller error = 0x%04x\n", ctrl);
+ /* Clear other interrupt bits for preventing ECC error */
+ interrupt &= ONENAND_INT_MASTER;
}

if (ctrl & ONENAND_CTRL_LOCK) {
+ DEBUG(MTD_DEBUG_LEVEL0, "onenand_wait: it's locked error = 0x%04x\n", ctrl);
+ return -EACCES;
}

if (interrupt & ONENAND_INT_READ) {
ecc = this->read_word(this->base + ONENAND_REG_ECC_STATUS); if (ecc & ONENAND_ECC_2BIT_ALL) {
+ DEBUG(MTD_DEBUG_LEVEL0, "onenand_wait: ECC error = 0x%04x\n", ecc);
return -EBADMSG;
}
}

Thank you for replay and check the code

=> Yes It's valid. During the initial bad block scanning, the
OneNAND controller return ctrl error. If we use return -EIO then OneNAND scan failed, then we can't detect OneNAND chip. So we don't return any ctrl error.
=> interrupt &= ONENAND_INT_MASTER. we always set master interrupt
bit in any case

=> I don't know which error code are correct. but I think locked
block access is EACCES instead of EIO

Kyungmin Park

Alright.

1) You could update your mtd sources and diff again, OR simply replace BBM patch's line 23 "return -EAGAIN" with "return -EIO".

2) In syncread-patch you add "void (*mmcontrol)" on line 103 of include/linux/onenand.h.
BBM patch is looking for this "void (*mmcontrol)" to add some new stuff here. E.g. it doesn't search the "original" onenand.h (official MTD) but the already syncread-patched onenand.h, bbm-patch cannot be run before sync-patch was run. No problem at all if one know's all this :-) Perhaps it would be good to change the struct onenand_chip declaration order in a way that both patches can be run independent of each other. Until they are put to MTD CVS anyway...

-> THE RULES:
+ Run all the patches inside the kernel tree
+ Be aware of the right order:
1. onenand-syncread.patch
2. onenand-bbm.patch
3. omap-onenand.patch

Regards,
Bernhard

Hi Kyungmin,

please let me bother you with another question.
It's about scanning for bad blocks during reading/writing flash (/* TODO handling oob */...)
What's the possibility of having a bad block at r/w? OneNAND spec isn't very extensive regarding handling bad blocks beside the initial scan. Which things must/should be done by the flash driver, which ones by the file system in this case? I would be very grateful if you could give me a brief outline, I'm afraid BBM is a bit "too new" for us because we are used to operate on plain NOR... :-)

Thanks,
Bernhard

PS: Kyungmin, sorry for double mail

Hi

'TODO handling oob' is for YAFFS filesystem
In JFFS2, they only use erase mark at oob area But YAFFS use some many files,
We needs to test YAFFS. currently not yet run and tested.

OneNAND spec isn't

I don't know the exactly but I heard that JFFS2 handles runtime badblock internally.
So OneNAND MTD don't care badblock handling in read/write function. MTD only care the initial bad block at erase operation.

In our commercial software, we use another bad block handling, known as Bad Block Management (BBM) based on replace instead of skip as BBT So we don't care the intial bad and runtime bad.

Now we are trying to implement our scheme and will be released at open source. But not yet available in this time.

If do you have any question. please let me know :)

Regards.

Kyungmin Park

Isn't this rather important? :-) As I said, I'm rather new to the bad block stuff, so I'm ashamed of don't knowing it at all. Can JFFS2 recognize a bad block without OneNAND driver explicitely reporting one?? I'm very interested in this thing; presumably it may be easy to find out having a look on the list's NAND BBT threads...

You are speaking about RFS? I'm wondering for quite a time about the technical and commercial parallels/differences of MTD/JFFS2 and RFS...

Do you have any (public) comment on the occurence of (runtime) bad blocks? When, why, ...?

Too many to write down, but I'll try ;-)

Regards,
Bernhard

Hi all,

Kyungmin, I'm still not completely clear about your bad block handling, sorry for bothering you another time.

In onenand_scan() you call scan_bbt() which calls onenand_memory_bbt()
which calls create_bbt() which builds the BBT using OOB data.
Fine so far.

But: the only time onenand_block_checkbad() is called is in onenand_erase() before issueing the (erase-)command to prevent erasing already scanned bad blocks.

Question: what about reading and writing (onenand_write_ecc(), onenand_read_ecc())? There is no onenand_block_checkbad() before issueing the write/read command. Shouldn't this be done?? scan_bbt() is only called on initial scan, e.g. device boot. What if this happens very rarely and bad blocks due to wear occur; that meant you cannot provide data integrity.

I wonder why this is not done in nand_base.c too. I'm afraid I missed some BBT stuff to gather in my investigations :-)... So please kindly excuse me if I'm wrong with my very limited MTD/OneNAND experience!

Best Regards,
Bernhard

Yes, The bbt scheme is same as NAND does. and OneNAND bbt is subset of NAND bbt.
IMHO, There's some historical reason. If I was worng. please correct it MTD only provides basic operations and handles initial badblock. The rest is the part of FTL or Filesystem. Is it right?

Regards,

Kyungmin Park