Patchwork [2] mtd/nand, BB detect: factory marker in page 1, 2, last and byte 1 or 6

login
register
mail settings
Submitter Brian Norris
Date June 25, 2010, 9:36 p.m.
Message ID <1277501791-3146-1-git-send-email-norris@broadcom.com>
Download mbox | patch
Permalink /patch/56990/
State New
Headers show

Comments

Brian Norris - June 25, 2010, 9:36 p.m.
Some level of support for various scanning locations was already built in,
but this required clean-up. First, BB marker location cannot be determined
_only_ by the page size. Instead, I implemented some heuristic detection
based on data sheets from various manufacturers (all found in
nand_base.c:nand_get_flash_type()).

Second, once these options were identified, they were not handled properly
by nand_bbt.c:nand_default_bbt(). I believe there are only 5 combinations
of page location/byte offset, so I updated the nand_bbt_desc structs
accordingly.

I have tested these changes with several chips, but particularly a Micron
29F8G08MAA chip. We use a memory-based, not flash-based, BBT; hence the
edits only to the "memory_based" side of things. Most edits could be
applicable to both, but I can't verify that. For instance, I do not
understand why the small discrepancies between the "memory_based" and
"flash_based" nand_bbt_descr structs.

Finally, please provide any feedback you can. I'm sure there's something
I have overlooked, but I feel this is a step in the right direction.

Signed-off-by: Brian Norris <norris@broadcom.com>
---
 drivers/mtd/nand/nand_base.c |   23 +++++++++++++++++++----
 drivers/mtd/nand/nand_bbt.c  |   41 ++++++++++++++++++++++++++++++++++-------
 2 files changed, 53 insertions(+), 11 deletions(-)
Artem Bityutskiy - July 13, 2010, 8:21 a.m.
On Fri, 2010-06-25 at 14:36 -0700, Brian Norris wrote:
> @@ -2920,9 +2920,13 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
>  		chip->chip_shift = ffs((unsigned)(chip->chipsize >> 32)) + 32 - 1;
>  
>  	/* Set the bad block position */
> -	chip->badblockpos = mtd->writesize > 512 ?
> -		NAND_LARGE_BADBLOCK_POS : NAND_SMALL_BADBLOCK_POS;
> -	chip->badblockbits = 8;
> +	chip->badblockpos = (!(busw & NAND_BUSWIDTH_16) &&
> +			(*maf_id == NAND_MFR_STMICRO ||
> +			 (*maf_id == NAND_MFR_SAMSUNG &&
> +			  mtd->writesize == 512) ||
> +			 *maf_id == NAND_MFR_AMD))
> +		? NAND_SMALL_BADBLOCK_POS : NAND_LARGE_BADBLOCK_POS;

I'd prefer if () else, ? : is not appropriate for such huge conditions.

Nevertheless, you patch looks indeed like a step in the right direction,
but I cannot really test it and do not have time to review.

But could you please re-send it without trailing white-spaces - I'll put
it to my l2 tree then? You can do a 'git am' to your own patches - it'll
check for trailing white-spaces for you.
Brian Norris - July 13, 2010, 10:12 p.m.
This is a two-patch correction and update to my previous attempt
to address a few issues with bad block scanning. Overall, they should
help renovate some incorrect and unwieldy code.

I included the top few maintainers from get_maintainers.pl. Sorry if these
were incorrect addressees.

Brian Norris (2):
  mtd/nand, BB detect: factory marker in page 1, 2, last and byte 1 or 6
  mtd/nand: More BB Detection, dynamic scan options

 drivers/mtd/nand/nand_base.c |   38 ++++++++++++++++--
 drivers/mtd/nand/nand_bbt.c  |   89 +++++++++++++++++++++++++++++------------
 include/linux/mtd/bbm.h      |    4 ++
 3 files changed, 101 insertions(+), 30 deletions(-)
Artem Bityutskiy - July 18, 2010, 4:38 p.m.
On Tue, 2010-07-13 at 15:12 -0700, Brian Norris wrote:
> This is a two-patch correction and update to my previous attempt
> to address a few issues with bad block scanning. Overall, they should
> help renovate some incorrect and unwieldy code.
> 
> I included the top few maintainers from get_maintainers.pl. Sorry if these
> were incorrect addressees.
> 
> Brian Norris (2):
>   mtd/nand, BB detect: factory marker in page 1, 2, last and byte 1 or 6
>   mtd/nand: More BB Detection, dynamic scan options
> 
>  drivers/mtd/nand/nand_base.c |   38 ++++++++++++++++--
>  drivers/mtd/nand/nand_bbt.c  |   89 +++++++++++++++++++++++++++++------------
>  include/linux/mtd/bbm.h      |    4 ++
>  3 files changed, 101 insertions(+), 30 deletions(-)

I did not _really_ review this, it the patches look good. How did you
test them? Did you test on both large and small page NANDs?

I pushed your patches to my l2-mtd-2.6.git / master
Brian Norris - July 19, 2010, 7:32 p.m.
On 07/18/2010 09:38 AM, Artem Bityutskiy wrote:
> I did not _really_ review this, it the patches look good. How did you
> test them? Did you test on both large and small page NANDs?

I referenced 30+ data sheets (covering 100+ parts), and I tested a 
selection of 10 different chips to varying degrees. Particularly, I 
tested the creation of bad-block descriptors and basic BB scanning on 
three parts:

ST NAND04GW3B2D, 2K page
ST NAND128W3A, 512B page
Samsung K9F1G08U0A, 2K page

To test these, I wrote some fake bad block markers to the flash (in OOB 
bytes 1, 6, and elsewhere) to see if the scanning routine would detect 
them properly. However, this method was somewhat limited because the 
driver I am using has some bugs in its OOB write functionality.
Artem Bityutskiy - July 21, 2010, 9:49 a.m.
On Mon, 2010-07-19 at 12:32 -0700, Brian Norris wrote:
> On 07/18/2010 09:38 AM, Artem Bityutskiy wrote:
> > I did not _really_ review this, it the patches look good. How did you
> > test them? Did you test on both large and small page NANDs?
> 
> I referenced 30+ data sheets (covering 100+ parts), and I tested a 
> selection of 10 different chips to varying degrees. Particularly, I 
> tested the creation of bad-block descriptors and basic BB scanning on 
> three parts:
> 
> ST NAND04GW3B2D, 2K page
> ST NAND128W3A, 512B page
> Samsung K9F1G08U0A, 2K page
> 
> To test these, I wrote some fake bad block markers to the flash (in OOB 
> bytes 1, 6, and elsewhere) to see if the scanning routine would detect 
> them properly. However, this method was somewhat limited because the 
> driver I am using has some bugs in its OOB write functionality.

Sounds like you did a lot of work. I'll add this information to the
patch description.
karl beldan - July 22, 2010, 7:44 p.m.
On Wed, Jul 21, 2010 at 11:49 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Mon, 2010-07-19 at 12:32 -0700, Brian Norris wrote:
>> On 07/18/2010 09:38 AM, Artem Bityutskiy wrote:
>> > I did not _really_ review this, it the patches look good. How did you
>> > test them? Did you test on both large and small page NANDs?
>>
>> I referenced 30+ data sheets (covering 100+ parts), and I tested a
>> selection of 10 different chips to varying degrees. Particularly, I
>> tested the creation of bad-block descriptors and basic BB scanning on
>> three parts:
>>
>> ST NAND04GW3B2D, 2K page
>> ST NAND128W3A, 512B page
>> Samsung K9F1G08U0A, 2K page
>>
>> To test these, I wrote some fake bad block markers to the flash (in OOB
>> bytes 1, 6, and elsewhere) to see if the scanning routine would detect
>> them properly. However, this method was somewhat limited because the
>> driver I am using has some bugs in its OOB write functionality.
>
> Sounds like you did a lot of work. I'll add this information to the
> patch description.
>
I haven't seen many nandsim feedback, though it allows precise test scenarios
and easy flash forensics.
Still, FWIW, I too laud the effort.

Patch

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index e6cf9ae..2d1ee3c 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2920,9 +2920,13 @@  static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 		chip->chip_shift = ffs((unsigned)(chip->chipsize >> 32)) + 32 - 1;
 
 	/* Set the bad block position */
-	chip->badblockpos = mtd->writesize > 512 ?
-		NAND_LARGE_BADBLOCK_POS : NAND_SMALL_BADBLOCK_POS;
-	chip->badblockbits = 8;
+	chip->badblockpos = (!(busw & NAND_BUSWIDTH_16) &&
+			(*maf_id == NAND_MFR_STMICRO ||
+			 (*maf_id == NAND_MFR_SAMSUNG &&
+			  mtd->writesize == 512) ||
+			 *maf_id == NAND_MFR_AMD))
+		? NAND_SMALL_BADBLOCK_POS : NAND_LARGE_BADBLOCK_POS;
+
 
 	/* Get chip options, preserve non chip based options */
 	chip->options &= ~NAND_CHIPOPTIONS_MSK;
@@ -2941,12 +2945,23 @@  static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 
 	/*
 	 * Bad block marker is stored in the last page of each block
-	 * on Samsung and Hynix MLC devices
+	 * on Samsung and Hynix MLC devices; stored in first two pages 
+	 * of each block on Micron devices with 2KiB pages and on 
+	 * SLC Samsung, Hynix, and AMD/Spansion. All others scan only 
+	 * the first page.
 	 */
 	if ((chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
 			(*maf_id == NAND_MFR_SAMSUNG ||
 			 *maf_id == NAND_MFR_HYNIX))
 		chip->options |= NAND_BBT_SCANLASTPAGE;
+	else if ((!(chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
+				(*maf_id == NAND_MFR_SAMSUNG ||
+				 *maf_id == NAND_MFR_HYNIX ||
+				 *maf_id == NAND_MFR_AMD)) ||
+			(mtd->writesize == 2048 &&
+			 *maf_id == NAND_MFR_MICRON))
+		chip->options |= NAND_BBT_SCAN2NDPAGE;
+
 
 	/* Check for AND chips with 4 page planes */
 	if (chip->options & NAND_4PAGE_ARRAY)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 71d83be..1719c56 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -1093,29 +1093,50 @@  int nand_update_bbt(struct mtd_info *mtd, loff_t offs)
 static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
 
 static struct nand_bbt_descr smallpage_memorybased = {
-	.options = NAND_BBT_SCAN2NDPAGE,
-	.offs = 5,
+	.options = 0,
+	.offs = NAND_SMALL_BADBLOCK_POS,
 	.len = 1,
 	.pattern = scan_ff_pattern
 };
 
+static struct nand_bbt_descr smallpage_scan2nd_memorybased = {
+	.options = NAND_BBT_SCAN2NDPAGE,
+	.offs = NAND_SMALL_BADBLOCK_POS,
+	.len = 2,
+	.pattern = scan_ff_pattern
+};
+
 static struct nand_bbt_descr largepage_memorybased = {
 	.options = 0,
-	.offs = 0,
+	.offs = NAND_LARGE_BADBLOCK_POS,
+	.len = 1,
+	.pattern = scan_ff_pattern
+};
+
+static struct nand_bbt_descr largepage_scan2nd_memorybased = {
+	.options = NAND_BBT_SCAN2NDPAGE,
+	.offs = NAND_LARGE_BADBLOCK_POS,
 	.len = 2,
 	.pattern = scan_ff_pattern
 };
 
+static struct nand_bbt_descr lastpage_memorybased = {
+	.options = NAND_BBT_SCANLASTPAGE,
+	.offs = 0,
+	.len = 1,
+	.pattern = scan_ff_pattern
+};
+
 static struct nand_bbt_descr smallpage_flashbased = {
 	.options = NAND_BBT_SCAN2NDPAGE,
-	.offs = 5,
+	.offs = NAND_SMALL_BADBLOCK_POS,
 	.len = 1,
 	.pattern = scan_ff_pattern
 };
 
 static struct nand_bbt_descr largepage_flashbased = {
 	.options = NAND_BBT_SCAN2NDPAGE,
-	.offs = 0,
+	.offs = NAND_LARGE_BADBLOCK_POS,
 	.len = 2,
 	.pattern = scan_ff_pattern
 };
@@ -1197,8 +1218,14 @@  int nand_default_bbt(struct mtd_info *mtd)
 		this->bbt_td = NULL;
 		this->bbt_md = NULL;
 		if (!this->badblock_pattern) {
-			this->badblock_pattern = (mtd->writesize > 512) ?
-			    &largepage_memorybased : &smallpage_memorybased;
+			if (this->options & NAND_BBT_SCANLASTPAGE)
+				this->badblock_pattern = &lastpage_memorybased;
+			else if (this->options & NAND_BBT_SCAN2NDPAGE)
+				this->badblock_pattern = this->badblockpos == NAND_SMALL_BADBLOCK_POS ? 
+					&smallpage_scan2nd_memorybased : &largepage_scan2nd_memorybased;
+			else
+				this->badblock_pattern = this->badblockpos == NAND_SMALL_BADBLOCK_POS ? 
+					&smallpage_memorybased : &largepage_memorybased;
 		}
 	}
 	return nand_scan_bbt(mtd, this->badblock_pattern);