Skip to contentSkip to conversation

OneNAND: Runtime badblock check support

PATCH In review Kyungmin Park 3 participants 12 messages Jul 21, 2009 – Jul 24, 2009

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 OneNAND: Runtime badblock check support

At bootloader, we don't need to read full page. It takes too long time. Instead it only read pages required for boot.

---

drivers/mtd/onenand/onenand_base.c +3 -8
index 6e82909..f2b2d2d 100644
@@ -9,7 +9,7 @@
  *	auto-placement support, read-while load support, various fixes
  *	Copyright (C) Nokia Corporation, 2007
  *
- *	Vishak G <vishak.g at samsung.com>, Rohit Hagargundgi <h.rohit at samsung.com>
+ *	Vishak G <[email protected]>, Rohit Hagargundgi <[email protected]>
  *	Flex-OneNAND support
  *	Copyright (C) Samsung Electronics, 2008
  *
@@ -1477,6 +1477,7 @@ static int onenand_bbt_wait(struct mtd_info *mtd, int state)
  * @param ops		oob operation description structure
  *
  * OneNAND read out-of-band data from the spare area for bbt scan
+ * Note that it operates without lock
  */
 int onenand_bbt_read_oob(struct mtd_info *mtd, loff_t from, 
 			    struct mtd_oob_ops *ops)
@@ -1498,9 +1499,6 @@ int onenand_bbt_read_oob(struct mtd_info *mtd, loff_t from,
 		return ONENAND_BBT_READ_FATAL_ERROR;
 	}
 
-	/* Grab the lock and see if the device is available */
-	onenand_get_device(mtd, FL_READING);
-
 	column = from & (mtd->oobsize - 1);
 
 	readcmd = ONENAND_IS_MLC(this) ? ONENAND_CMD_READ : ONENAND_CMD_READOOB;
@@ -1537,9 +1535,6 @@ int onenand_bbt_read_oob(struct mtd_info *mtd, loff_t from,
 		}
 	}
 
-	/* Deselect and wake up anyone waiting on the device */
-	onenand_release_device(mtd);
-
 	ops->oobretlen = read;
 	return ret;
 }
@@ -3408,7 +3403,7 @@ static void onenand_resume(struct mtd_info *mtd)
 	if (this->state == FL_PM_SUSPENDED)
 		onenand_release_device(mtd);
 	else
-		printk(KERN_ERR "resume() called for the chip which is not"
+		printk(KERN_ERR "resume() called for the chip which is not "
 				"in suspended state\n");
 }
 
drivers/mtd/onenand/onenand_bbt.c +70 -5

133 lines not on this page · raw message

drivers/mtd/onenand/samsung.c +3 -0
index 4b92abc..e4b8505 100644
@@ -670,6 +670,9 @@ static int s3c_onenand_probe(struct platform_device *pdev)
 		goto oob_buf_fail;
 	}
 
+	/* Set runtime badblock check before onenand_scan() */
+	this->options |= ONENAND_RUNTIME_BADBLOCK_CHECK;
+
 	if (onenand_scan(mtd, 1)) {
 		err = -EFAULT;
 		goto scan_failed;
include/linux/mtd/onenand.h +1 -0
index 8ed8733..5697476 100644
@@ -189,6 +189,7 @@ struct onenand_chip {
 #define ONENAND_HAS_UNLOCK_ALL		(0x0002)
 #define ONENAND_HAS_2PLANE		(0x0004)
 #define ONENAND_SKIP_UNLOCK_CHECK	(0x0100)
+#define ONENAND_RUNTIME_BADBLOCK_CHECK	(0x0200)
 #define ONENAND_PAGEBUF_ALLOC		(0x1000)
 #define ONENAND_OOBBUF_ALLOC		(0x2000)
 
#1 lore raw

Hi,

Quoted 5 lines · Kyungmin Park

At bootloader, we don't need to read full page. It takes too long time. Instead it only read pages required for boot.

---

I do not understand the commit message. Could you please provide a better one which explains what the patch is really doing and why.

Signature · 2 lines
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
#2 lore raw

Hi

Quoted 4 lines · Artem Bityutskiy

Hi,

Quoted 1 line · Kyungmin Park
Quoted 8 lines
Quoted 5 lines

At bootloader, we don't need to read full page. It takes too long time. Instead it only read pages required for boot.

---

I do not understand the commit message. Could you please provide a better one which explains what the patch is really doing and why.

Right, the page means all blocks. It doesn't scan all blocks at boot time. only some block contains kernel part are read and boot.

The goal is simple, Avoid double scanning at boot time. One time for bootloader, Another is kernel.
As you know at bootloader, only load kernel in normal case. it means we only read the kernel bad blocks and handle it.
and jump to the kernel. that's all

Thank you,
Kyungmin Park

#3 lore raw

Hi,

Quoted 16 lines · Kyungmin Park

Hi

Quoted 4 lines · Artem Bityutskiy

Hi,

Quoted 1 line · Kyungmin Park
Quoted 8 lines
Quoted 5 lines

At bootloader, we don't need to read full page. It takes too long time. Instead it only read pages required for boot.

---

I do not understand the commit message. Could you please provide a better one which explains what the patch is really doing and why.

Quoted 9 lines

Right, the page means all blocks. It doesn't scan all blocks at boot time. only some block contains kernel part are read and boot.

The goal is simple, Avoid double scanning at boot time. One time for bootloader, Another is kernel.
As you know at bootloader, only load kernel in normal case. it means we only read the kernel bad blocks and handle it.
and jump to the kernel. that's all

Experimental idea.
How about to integrate the bbt scan and ubi scan?

At ubi scan time. it scans all ubi blocks. In different from NAND, OneNAND read main page and spare page both.
So if runtime badblock check enabled, read the page and check oob first and if it's valid. use main page. If not, set it's badblock. I'm not sure how much codes are modified. :)

How to you think?

Thank you,
Kyungmin Park

#4 lore raw
Quoted 28 lines · Kyungmin Park

Hi,

Quoted 25 lines · Kyungmin Park

Hi

Quoted 4 lines · Artem Bityutskiy

Hi,

Quoted 1 line · Kyungmin Park
Quoted 8 lines
Quoted 5 lines

At bootloader, we don't need to read full page. It takes too long time. Instead it only read pages required for boot.

---

I do not understand the commit message. Could you please provide a better one which explains what the patch is really doing and why.

Right, the page means all blocks. It doesn't scan all blocks at boot time. only some block contains kernel part are read and boot.

The goal is simple, Avoid double scanning at boot time. One time for bootloader, Another is kernel.
As you know at bootloader, only load kernel in normal case. it means we only read the kernel bad blocks and handle it.
and jump to the kernel. that's all

I understand the double-scanning issue. But I still do not understand how your patch is avoiding it? If the boot-loader has scanned the flash, it should pass the bad-block bitmap to the kernel. If you have on-flash BBT, then probably this is not a problem.

Quoted 2 lines

Experimental idea.
How about to integrate the bbt scan and ubi scan?

It is possible, but isn't it better to just have on-flash BBT and forget about any scanning for bad blocks at all?

We could go even further: If the boot-loader supports ubi, it could do the scanning and pass all UBI data to the kernel, in which case UBI would not need to do any scanning.

Quoted 7 lines

At ubi scan time. it scans all ubi blocks. In different from NAND, OneNAND read main page and spare page both.
So if runtime badblock check enabled, read the page and check oob first and if it's valid. use main page. If not, set it's badblock. I'm not sure how much codes are modified. :)

How to you think?

It is possible, but I think the implementation will add more ugliness... Why not to just use on-flash BBT instead?

Signature · 2 lines
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
#5 lore raw
Quoted 30 lines · Artem Bityutskiy
Quoted 28 lines · Kyungmin Park

Hi,

Quoted 25 lines · Kyungmin Park

Hi

Quoted 4 lines · Artem Bityutskiy

Hi,

Quoted 1 line · Kyungmin Park
Quoted 8 lines
Quoted 5 lines

At bootloader, we don't need to read full page. It takes too long time. Instead it only read pages required for boot.

---

I do not understand the commit message. Could you please provide a better one which explains what the patch is really doing and why.

Right, the page means all blocks. It doesn't scan all blocks at boot time. only some block contains kernel part are read and boot.

The goal is simple, Avoid double scanning at boot time. One time for bootloader, Another is kernel.
As you know at bootloader, only load kernel in normal case. it means we only read the kernel bad blocks and handle it.
and jump to the kernel. that's all

Quoted 4 lines

I understand the double-scanning issue. But I still do not understand how your patch is avoiding it? If the boot-loader has scanned the flash, it should pass the bad-block bitmap to the kernel. If you have on-flash BBT, then probably this is not a problem.

It don't avoid the double-scanning issue. only partial scan at bootloader. In my environemt kernel has 2MiB partition and we only scan this partition at load kernel command.
Exactly that it skip the bad block scanning except the kernel partition at bootloader
As bootcmd load kernel, it first check that read block is bad, if not read this block, next check again next block, and read next block and so on.

Quoted 10 lines
Quoted 3 lines

Experimental idea.
How about to integrate the bbt scan and ubi scan?

It is possible, but isn't it better to just have on-flash BBT and forget about any scanning for bad blocks at all?

We could go even further: If the boot-loader supports ubi, it could do the scanning and pass all UBI data to the kernel, in which case UBI would not need to do any scanning.

This is chicken and egg problem. actually the scan time is almost same either bootloader or kernel.
My main idea integrate the scanning and ubi scan.
In any case, we have to scan whole chip at least one.

Quoted 7 lines
Quoted 7 lines

At ubi scan time. it scans all ubi blocks. In different from NAND, OneNAND read main page and spare page both.
So if runtime badblock check enabled, read the page and check oob first and if it's valid. use main page. If not, set it's badblock. I'm not sure how much codes are modified. :)

Quoted 5 lines
Quoted 1 line

How to you think?

It is possible, but I think the implementation will add more ugliness... Why not to just use on-flash BBT instead?

I don't have any idea about on-flash BBT.

Thank you,
Kyungmin Park

#6 lore raw
Quoted 8 lines · Kyungmin Park

It don't avoid the double-scanning issue. only partial scan at bootloader. In my environemt kernel has 2MiB partition and we only scan this partition at load kernel command.
Exactly that it skip the bad block scanning except the kernel partition at bootloader
As bootcmd load kernel, it first check that read block is bad, if not read this block, next check again next block, and read next block and so on.

So you have a small kernel partition, you store the kernel there. Your boot-loader reads the kernel partition and loads the kernel from there. And of course the boot-loader scans the kernel partition for bad block. And your boot-loader does not need to scan anything else. I understand this part.

But what is next? Do you want to tell the kernel to avoid scanning the kernel partition because boot-loader has already scanned it? Sorry, I still cannot get the idea...

Quoted 1 line

I don't have any idea about on-flash BBT.

It is standard mechanism which exists in MTD from early tglx1 times. MTD can keep BBT on flash, AFAIR, in the last eraseblock. Look for the NAND_USE_FLASH_BBT constant. And this is the way to go if you want to optimize BBT scanning, IMO.

Signature · 2 lines
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
#7 lore raw
Quoted 11 lines · Artem Bityutskiy
Quoted 9 lines · Kyungmin Park

It don't avoid the double-scanning issue. only partial scan at bootloader. In my environemt kernel has 2MiB partition and we only scan this partition at load kernel command.
Exactly that it skip the bad block scanning except the kernel partition at bootloader
As bootcmd load kernel, it first check that read block is bad, if not read this block, next check again next block, and read next block and so on.

Quoted 5 lines

So you have a small kernel partition, you store the kernel there. Your boot-loader reads the kernel partition and loads the kernel from there. And of course the boot-loader scans the kernel partition for bad block. And your boot-loader does not need to scan anything else. I understand this part.

Right

Quoted 4 lines

But what is next? Do you want to tell the kernel to avoid scanning the kernel partition because boot-loader has already scanned it? Sorry, I still cannot get the idea...

Kernel part is same as before. Now I just optimize the boot loader part.

Quoted 6 lines
Quoted 1 line

I don't have any idea about on-flash BBT.

It is standard mechanism which exists in MTD from early tglx1 times. MTD can keep BBT on flash, AFAIR, in the last eraseblock. Look for the NAND_USE_FLASH_BBT constant. And this is the way to go if you want to optimize BBT scanning, IMO.

Yes I know, I mean no plan to implement it now. Sorry

Thank you,
Kyungmin Park

#8 lore raw
In reply to Artem Bityutskiy

Hi ,

Quoted 40 lines · Kyungmin Park
Quoted 38 lines

Hi,

Quoted 34 lines · Kyungmin Park

Hi

Quoted 8 lines · Artem Bityutskiy

Hi,

Quoted 5 lines · Kyungmin Park

At bootloader, we don't need to read full page. It takes too long time.
Instead it only read pages required for boot.

Quoted 8 lines
Quoted 3 lines

Signed-off-by: Kyungmin
Park<[email protected]>
---

I do not understand the commit message. Could you please provide a better
one which explains what the patch is really doing and why.

Right, the page means all blocks. It doesn't scan all blocks at boot
time. only some block contains kernel part are read and boot.

The goal is simple, Avoid double scanning at boot time. One time for
bootloader, Another is kernel.
As you know at bootloader, only load kernel in normal case. it means
we only read the kernel bad blocks and handle it. and jump to the kernel. that's all

Quoted 11 lines

I understand the double-scanning issue. But I still do not understand
how your patch is avoiding it? If the boot-loader has scanned the
flash, it should pass the bad-block bitmap to the kernel. If you have
on-flash BBT, then probably this is not a problem.

Quoted 2 lines

Experimental idea.
How about to integrate the bbt scan and ubi scan?

In my opinion integrating bbt scan and ubi scan is a valid idea.
It will save our double scanning issue that is available in UBi at apresent.

Quoted 19 lines

It is possible, but isn't it better to just have on-flash BBT and
forget about any scanning for bad blocks at all?

We could go even further: If the boot-loader supports ubi, it could do
the scanning and pass all UBI data to the kernel, in which case UBI
would not need to do any scanning.

Quoted 9 lines

At ubi scan time. it scans all ubi blocks. In different from NAND,
OneNAND read main page and spare page both.
So if runtime badblock check enabled, read the page and check oob
first and if it's valid. use main page. If not, set it's badblock.
I'm not sure how much codes are modified. :)

Quoted 5 lines
Quoted 1 line

How to you think?

It is possible, but I think the implementation will add more ugliness...
Why not to just use on-flash BBT instead?

implmentaing on flash bab block table will save our bbt
scan time but flash bbt table may go huge bceuase you have to set full
mapping table and in big device it may be to huge.

Quoted 10 lines
Signature · 8 lines
-- 
Best Regards,
Artem Bityutskiy (????? ????????)


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
#9 lore raw
Quoted 4 lines · Amit Kumar Sharma

implmentaing on flash bab block table will save our bbt
scan time but flash bbt table may go huge bceuase you have to set full
mapping table and in big device it may be to huge.

If it is huge, then doing any even one scanning is crazy anyway.

Signature · 2 lines
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
#10 lore raw
In reply to Kyungmin Park
Quoted 4 lines · Kyungmin Park

At bootloader, we don't need to read full page. It takes too long time. Instead it only read pages required for boot.

I still did not get it, why you change boot-loader behavior by patching the kernel. Or you use the kernel as a bootloader?

Also, the mtd-2.6.git tree does not havemtd/onenand/samsung.c

Anyway, could you please re-send the patch with a nice description, for silly people like me :-)

Signature · 2 lines
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
#11 lore raw
Quoted 9 lines · Artem Bityutskiy
Quoted 5 lines · Kyungmin Park

At bootloader, we don't need to read full page. It takes too long time. Instead it only read pages required for boot.

I still did not get it, why you change boot-loader behavior by patching the kernel. Or you use the kernel as a bootloader?

Since code consistency. I also send a same patch to u-boot. actually u-boot OneNAND code are almost same as kernel one.

Quoted 2 lines

Also, the mtd-2.6.git tree does not havemtd/onenand/samsung.c

Right, samsung.c is for s3c64xx and s5pc100 SoCs. I will send it after more tests.

Quoted 3 lines

Anyway, could you please re-send the patch with a nice description, for silly people like me :-)

Sure.

Thank you,
Kyungmin Park