答复: [PATCH] mtd: cmdlinepart: allow fill-up partition at any point

答复: [PATCH] mtd: cmdlinepart: allow fill-up partition at any point

[Yanjiantao]

Hi,
In linux-3.10 and later kernel versions, block 'markbad' or 'checkbad' method is to mark or check 1st, 2nd/last page, which depends on chip->bbt_options:
markbad flows is as follows:
                /* Write to first/last page(s) if necessary */
                if (chip->bbt_options & NAND_BBT_SCANLASTPAGE)
                        wr_ofs += mtd->erasesize - mtd->writesize;
                do {
                        res = nand_do_write_oob(mtd, wr_ofs, &ops);
                        if (!ret)
                                ret = res;

                        i++;
                        wr_ofs += mtd->writesize;
                } while ((chip->bbt_options & NAND_BBT_SCAN2NDPAGE) && i < 2);
Check bad flows is the same.

1. when NAND_BBT_SCANLASTPAGE is set, markbad and checkbad on the last page only.
2. when NAND_BBT_SCAN2NDPAGE is set, markbad and checkbad on the 1st and 2nd page.

Questions:
1. for some NAND Flash, badblock marker is on 1st or 2nd or last page(spansion), in this case , check badblock my fail.
2. for some NAND Flash, bad block marker is on 1st or last page(hynix H27UBG8T2CTR) , in this case, check badblock may fail too.
3. set NAND_BBT_SCANLASTPAGE means only mark or check the last page, which is not compliant with SAMSUNG/ TOSHIBA/ HYNIX/MICRON/spansion. they claim bad block marker is on 1st/2nd, or 1st/last, or 1st/2nd/last, or the 1st only. 

-----邮件原件-----
发件人: Caizhiyong 
发送时间: 2015年10月13日 15:35
收件人: Yanjiantao
主题: FW: [PATCH] mtd: cmdlinepart: allow fill-up partition at any point



Looking forward to your feedback.
Best regards.
Cai Zhiyong.
http://www.huawei.com

> -----Original Message-----
> From: Brian Norris [mailto:computersforpeace@gmail.com]
> Sent: Wednesday, September 30, 2015 3:56 AM
> To: Ben Shelton
> Cc: dmw2@infradead.org; linux-mtd@lists.infradead.org;
> linux-kernel@vger.kernel.org; Caizhiyong
> Subject: Re: [PATCH] mtd: cmdlinepart: allow fill-up partition at any point
> 
> Sorry for delay... a lot of things came on my plate, so things got
> buried.
> 
> I fixed up a few conflicts locally with some of my patches, but I have a
> few problems with your patch, so I'll have to request another revision.
> 
> On Thu, May 07, 2015 at 09:57:41PM -0500, Ben Shelton wrote:
> > Currently, a fill-up partition (indicated by '-') must be the last
> > partition, and no other partitions can go after it.  Change the
> > cmdlinepart parsing code to allow a fill-up partition at any point.
> > This is useful, for example, if you want to reserve a partition at the
> > end of the flash where the bad block table will go.
> >
> > Signed-off-by: Ben Shelton <ben.shelton@ni.com>
> 
> For one, I think we need a little update to the parser documentation
> (i.e., code comments at the top) to show at least an example of this new
> allowed syntax.
> 
> > ---
> >  drivers/mtd/cmdlinepart.c | 33 ++++++++++++++++++++++++++-------
> >  1 file changed, 26 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c
> > index c850300..2d0eda2 100644
> > --- a/drivers/mtd/cmdlinepart.c
> > +++ b/drivers/mtd/cmdlinepart.c
> > @@ -97,6 +97,7 @@ static struct mtd_partition * newpart(char *s,
> >  				      char **retptr,
> >  				      int *num_parts,
> >  				      int this_part,
> > +				      int size_remaining_found,
> >  				      unsigned char **extra_mem_ptr,
> >  				      int extra_mem_size)
> >  {
> > @@ -110,9 +111,16 @@ static struct mtd_partition * newpart(char *s,
> >
> >  	/* fetch the partition size */
> >  	if (*s == '-') {
> > +		if (size_remaining_found) {
> > +			printk(KERN_ERR ERRP
> 
> Trivial: we can use pr_err() now (see l2-mtd.git).
> 
> > +			       "more than one '-' partition specified\n");
> > +			return ERR_PTR(-EINVAL);
> > +		}
> > +
> >  		/* assign all remaining space to this partition */
> >  		size = SIZE_REMAINING;
> >  		s++;
> > +		size_remaining_found = 1;
> >  	} else {
> >  		size = memparse(s, &s);
> >  		if (size < PAGE_SIZE) {
> > @@ -169,13 +177,10 @@ static struct mtd_partition * newpart(char *s,
> >
> >  	/* test if more partitions are following */
> >  	if (*s == ',') {
> > -		if (size == SIZE_REMAINING) {
> > -			printk(KERN_ERR ERRP "no partitions allowed after a fill-up
> partition\n");
> > -			return ERR_PTR(-EINVAL);
> > -		}
> >  		/* more partitions follow, parse them */
> >  		parts = newpart(s + 1, &s, num_parts, this_part + 1,
> > -				&extra_mem, extra_mem_size);
> > +				size_remaining_found, &extra_mem,
> > +				extra_mem_size);
> >  		if (IS_ERR(parts))
> >  			return parts;
> >  	} else {
> > @@ -252,6 +257,7 @@ static int mtdpart_setup_real(char *s)
> >  				&s,		/* out: updated cmdline ptr */
> >  				&num_parts,	/* out: number of parts */
> >  				0,		/* first partition */
> > +				0,		/* size_remaining not found */
> >  				(unsigned char**)&this_mtd, /* out: extra mem */
> >  				mtd_id_len + 1 + sizeof(*this_mtd) +
> >  				sizeof(void*)-1 /*alignment*/);
> > @@ -313,6 +319,7 @@ static int parse_cmdline_partitions(struct mtd_info
> *master,
> >  	int i, err;
> >  	struct cmdline_mtd_partition *part;
> >  	const char *mtd_id = master->name;
> > +	int sr_part_num = -1;
> >
> >  	/* parse command line */
> >  	if (!cmdline_parsed) {
> > @@ -339,8 +346,10 @@ static int parse_cmdline_partitions(struct mtd_info
> *master,
> >  		else
> >  			offset = part->parts[i].offset;
> >
> > -		if (part->parts[i].size == SIZE_REMAINING)
> > -			part->parts[i].size = master->size - offset;
> > +		if (part->parts[i].size == SIZE_REMAINING) {
> > +			sr_part_num = i;
> > +			continue;
> > +		}
> 
> Here, you are dropping this property for fill partitions:
> 
>   "if specified or truncated size is 0 the part is skipped"
> 
> Since the size is no longer computed in this loop, it will never match
> the 'size == 0' check.
> 
> >
> >  		if (offset + part->parts[i].size > master->size) {
> >  			printk(KERN_WARNING ERRP
> > @@ -361,6 +370,16 @@ static int parse_cmdline_partitions(struct mtd_info
> *master,
> >  		}
> >  	}
> >
> > +	/* if a partition was marked as SIZE_REMAINING */
> > +	if (sr_part_num != -1) {
> > +		/* fix up the size of the SIZE_REMAINING partition */
> > +		part->parts[sr_part_num].size = master->size - offset;
> 
> The use of 'offset' doesn't look correct. At this point, 'offset' is the
> address of the end of the last partition. That looks like you're still
> sizing the fill partition to be only at the end of the flash, which is
> not the point of this patch. Am I missing something?
> 
> > +
> > +		/* fix up the offsets of the subsequent partitions */
> > +		for (i = (sr_part_num + 1); i < part->num_parts; i++)
> > +			part->parts[i].offset += part->parts[sr_part_num].size;
> 
> Is this a legal adjustment? Is it possible to have fixed address (not
> OFFSET_CONTINUOUS) partitions after the fill partition? I see that would
> actually make this kind of ambiguous. So maybe you need to specify this
> in the documentation and check for it in the parsing -- see that once
> size_remaining_found > 0, we don't allow any more partitions
> '@<address'> (i.e., not OFFSET_CONTINUOUS).
> 
> > +	}
> > +
> >  	*pparts = kmemdup(part->parts, sizeof(*part->parts) * part->num_parts,
> >  			  GFP_KERNEL);
> >  	if (!*pparts)
> 
> I think this patch speaks to our need for unit tests... that way we can
> clearly and definitively show pass/fail for any regressions.
> 
> This reminds me of Cai Zhiyong's attempts to unify this parser with the
> new block/cmdline-parser, without getting any test results for the
> variety of existing MTD use cases...
> 
> Brian

Re: 答复: [PATCH] mtd: cmdlinepart: allow fill-up partition at any point

[Brian Norris]

Hi,

First of all, I don't know why this message is in reply to the $subject
thread. Bad block markers have nothing to do with cmdlineparts.

On Tue, Oct 13, 2015 at 08:53:04AM +0000, Yanjiantao wrote:
> Hi,
> In linux-3.10 and later kernel versions, block 'markbad' or 'checkbad' method is to mark or check 1st, 2nd/last page, which depends on chip->bbt_options:
> markbad flows is as follows:
>                 /* Write to first/last page(s) if necessary */
>                 if (chip->bbt_options & NAND_BBT_SCANLASTPAGE)
>                         wr_ofs += mtd->erasesize - mtd->writesize;
>                 do {
>                         res = nand_do_write_oob(mtd, wr_ofs, &ops);
>                         if (!ret)
>                                 ret = res;
> 
>                         i++;
>                         wr_ofs += mtd->writesize;
>                 } while ((chip->bbt_options & NAND_BBT_SCAN2NDPAGE) && i < 2);
> Check bad flows is the same.
> 
> 1. when NAND_BBT_SCANLASTPAGE is set, markbad and checkbad on the last page only.
> 2. when NAND_BBT_SCAN2NDPAGE is set, markbad and checkbad on the 1st and 2nd page.

Sounds right.

> Questions:
> 1. for some NAND Flash, badblock marker is on 1st or 2nd or last page(spansion), in this case , check badblock my fail.
> 2. for some NAND Flash, bad block marker is on 1st or last page(hynix H27UBG8T2CTR) , in this case, check badblock may fail too.
> 3. set NAND_BBT_SCANLASTPAGE means only mark or check the last page, which is not compliant with SAMSUNG/ TOSHIBA/ HYNIX/MICRON/spansion. they claim bad block marker is on 1st/2nd, or 1st/last, or 1st/2nd/last, or the 1st only. 

We try our best to get the BBM locations correct. But we have
acknowledged that there are corner cases out there that we don't get
right. So far, I guess nobody really cares.

FWIW, in practice it seems that even when the datasheet says something
specific like the descriptions you mention above, the factory process
just fuses all bytes to zero (not just OOB, and not just in certain
pages; but ALL bytes in the factory-marked bad block). Have you seen any
failures as a result of bad BBM scans? Or is the problem just
theoretical?

Also, you can refer to this (old) table of NAND flash [1] that I and a few
others catalogued, and it notes a few flash that are not supported
correctly. It may be a bit out of date.

Brian

[1] http://linux-mtd.infradead.org/nand-data/nanddata.html

答复: 答复: [PATCH] mtd: cmdlinepart: allow fill-up partition at any point

[Yanjiantao]

Hi, Brian,
Thank you for your reply!
I noticed the mistake by sending you the 1st email attached with the cmdlineparts issue, so I resend you a new one. Sorry for the inconvenient.

This problem is not just theoretical, we came to meet badblock scans fail issue using spansion nand, which factory bad block is only in one byte of OOB, all the other bytes are all 0xFFs.
As you say, maybe this kind of nand flash has not been surppoted correctly yet, no big deal.

-----邮件原件-----
发件人: Brian Norris [mailto:computersforpeace@gmail.com] 
发送时间: 2015年10月14日 1:52
收件人: Yanjiantao
抄送: dmw2@infradead.org; linux-mtd@lists.infradead.org; linux-kernel@vger.kernel.org; Caizhiyong; pengjiancheng
主题: Re: 答复: [PATCH] mtd: cmdlinepart: allow fill-up partition at any point

Hi,

First of all, I don't know why this message is in reply to the $subject
thread. Bad block markers have nothing to do with cmdlineparts.

On Tue, Oct 13, 2015 at 08:53:04AM +0000, Yanjiantao wrote:
> Hi,
> In linux-3.10 and later kernel versions, block 'markbad' or 'checkbad' method is to mark or check 1st, 2nd/last page, which depends on chip->bbt_options:
> markbad flows is as follows:
>                 /* Write to first/last page(s) if necessary */
>                 if (chip->bbt_options & NAND_BBT_SCANLASTPAGE)
>                         wr_ofs += mtd->erasesize - mtd->writesize;
>                 do {
>                         res = nand_do_write_oob(mtd, wr_ofs, &ops);
>                         if (!ret)
>                                 ret = res;
> 
>                         i++;
>                         wr_ofs += mtd->writesize;
>                 } while ((chip->bbt_options & NAND_BBT_SCAN2NDPAGE) && i < 2);
> Check bad flows is the same.
> 
> 1. when NAND_BBT_SCANLASTPAGE is set, markbad and checkbad on the last page only.
> 2. when NAND_BBT_SCAN2NDPAGE is set, markbad and checkbad on the 1st and 2nd page.

Sounds right.

> Questions:
> 1. for some NAND Flash, badblock marker is on 1st or 2nd or last page(spansion), in this case , check badblock my fail.
> 2. for some NAND Flash, bad block marker is on 1st or last page(hynix H27UBG8T2CTR) , in this case, check badblock may fail too.
> 3. set NAND_BBT_SCANLASTPAGE means only mark or check the last page, which is not compliant with SAMSUNG/ TOSHIBA/ HYNIX/MICRON/spansion. they claim bad block marker is on 1st/2nd, or 1st/last, or 1st/2nd/last, or the 1st only. 

We try our best to get the BBM locations correct. But we have
acknowledged that there are corner cases out there that we don't get
right. So far, I guess nobody really cares.

FWIW, in practice it seems that even when the datasheet says something
specific like the descriptions you mention above, the factory process
just fuses all bytes to zero (not just OOB, and not just in certain
pages; but ALL bytes in the factory-marked bad block). Have you seen any
failures as a result of bad BBM scans? Or is the problem just
theoretical?

Also, you can refer to this (old) table of NAND flash [1] that I and a few
others catalogued, and it notes a few flash that are not supported
correctly. It may be a bit out of date.

Brian

[1] http://linux-mtd.infradead.org/nand-data/nanddata.html

Re: 答复: [PATCH] mtd: cmdlinepart: allow fill-up partition at any point

[Sheng Yong]

On 10/14/2015 1:51 AM, Brian Norris wrote:
> Hi,
> 
> First of all, I don't know why this message is in reply to the $subject
> thread. Bad block markers have nothing to do with cmdlineparts.
> 
> On Tue, Oct 13, 2015 at 08:53:04AM +0000, Yanjiantao wrote:
>> Hi,
>> In linux-3.10 and later kernel versions, block 'markbad' or 'checkbad' method is to mark or check 1st, 2nd/last page, which depends on chip->bbt_options:
>> markbad flows is as follows:
>>                 /* Write to first/last page(s) if necessary */
>>                 if (chip->bbt_options & NAND_BBT_SCANLASTPAGE)
>>                         wr_ofs += mtd->erasesize - mtd->writesize;
>>                 do {
>>                         res = nand_do_write_oob(mtd, wr_ofs, &ops);
>>                         if (!ret)
>>                                 ret = res;
>>
>>                         i++;
>>                         wr_ofs += mtd->writesize;
>>                 } while ((chip->bbt_options & NAND_BBT_SCAN2NDPAGE) && i < 2);
>> Check bad flows is the same.
>>
>> 1. when NAND_BBT_SCANLASTPAGE is set, markbad and checkbad on the last page only.
>> 2. when NAND_BBT_SCAN2NDPAGE is set, markbad and checkbad on the 1st and 2nd page.
> 
> Sounds right.
> 
>> Questions:
>> 1. for some NAND Flash, badblock marker is on 1st or 2nd or last page(spansion), in this case , check badblock my fail.
>> 2. for some NAND Flash, bad block marker is on 1st or last page(hynix H27UBG8T2CTR) , in this case, check badblock may fail too.
>> 3. set NAND_BBT_SCANLASTPAGE means only mark or check the last page, which is not compliant with SAMSUNG/ TOSHIBA/ HYNIX/MICRON/spansion. they claim bad block marker is on 1st/2nd, or 1st/last, or 1st/2nd/last, or the 1st only. 
> 
> We try our best to get the BBM locations correct. But we have
> acknowledged that there are corner cases out there that we don't get
> right. So far, I guess nobody really cares.
> 
> FWIW, in practice it seems that even when the datasheet says something
> specific like the descriptions you mention above, the factory process
> just fuses all bytes to zero (not just OOB, and not just in certain
> pages; but ALL bytes in the factory-marked bad block). Have you seen any
> failures as a result of bad BBM scans? Or is the problem just
> theoretical?
> 
Hi, Brian, Cai, Yan,

We triggered a BBM scan failure because of power cut. The chip is Micron
MT29F1G08ABAEAWP:E, whose BBM locates in the OOB of the first page in a
block.

However, nand_decode_bbm_options() sets NAND_BBT_SCAN2NDPAGE option, because
the chip satisifies: 1) manufacture is NAND_MFR_MICRON, 2) page size is 2048.

When checking bad, the BBM in the first OOB is correct, while the BBM in the
second OOB is 0. This leads to a lot of blocks are regarded as bad blocks.

I don't know what that means if the BBM in the second OOB is not 0xff. Can
we say that the block is good (the datasheet indicates "the first spare area
location in each bad block is guaranteed to contain the bad block mark")?

If we should follow the datasheet, maybe we could add some code to fix this:
* add a fixup table, which describes where the BBM locates
* when decoding bbm, lookup the table, and fix some specific chips

---
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index d87c7d0..44f0240 100644
index d87c7d0..44f0240 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3701,6 +3701,39 @@ static void nand_decode_id(struct mtd_info *mtd, struct nand_chip *chip,
        }
 }

+static void nand_decode_bbm_fixup(struct nand_chip *chip, u8 id_data[8])
+{
+       struct nand_bbm_page *bbm;
+       int i, j;
+
+       for (i = 0;;i++) {
+               bbm = &nand_bbm_page_fixup[i];
+               if (!bbm->name)
+                       /* At the end of the table */
+                       break;
+               for (j = 0; j < NAND_MAX_ID_LEN; i++) {
+                       if (id_data[i] != bbm->id[i])
+                               continue;
+               }
+               /* Now we find the chip */
+               break;
+       }
+
+       if (bbm) {
+               if (bbm->bbm_pages & NAND_BBM_FIRSTPAGE) {
+                       chip->bbt_options &= ~NAND_BBT_SCANLASTPAGE;
+                       chip->bbt_options &= ~NAND_BBT_SCAN2NDPAGE;
+               }
+               if (bbm->bbm_pages & NAND_BBM_SECONDPAGE) {
+                       chip->bbt_options &= ~NAND_BBT_SCANLASTPAGE;
+                       chip->bbt_options |= NAND_BBT_SCAN_SCAN2NDPAGE;
+               }
+               if (bbm->bbm_pages & NAND_BBM_LASTPAGE) {
+                       chip->bbt_options |= NAND_BBT_SCANLASTPAGE;
+               }
+       }
+}
+
 /*
  * Set the bad block marker/indicator (BBM/BBI) patterns according to some
  * heuristic patterns using various detected parameters (e.g., manufacturer,
@@ -3736,6 +3769,8 @@ static void nand_decode_bbm_options(struct mtd_info *mtd,
                        (mtd->writesize == 2048 &&
                         maf_id == NAND_MFR_MICRON))
                chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
+
s00292274@linux-4hy3:linux$ git diff > 1
s00292274@linux-4hy3:linux$ cat 1
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index d87c7d0..44f0240 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3701,6 +3701,39 @@ static void nand_decode_id(struct mtd_info *mtd, struct nand_chip *chip,
 	}
 }

+static void nand_decode_bbm_fixup(struct nand_chip *chip, u8 id_data[8])
+{
+	struct nand_bbm_page *bbm;
+	int i, j;
+
+	for (i = 0;;i++) {
+		bbm = &nand_bbm_page_fixup[i];
+		if (!bbm->name)
+			/* At the end of the table */
+			break;
+		for (j = 0; j < NAND_MAX_ID_LEN; i++) {
+			if (id_data[i] != bbm->id[i])
+				continue;
+		}
+		/* Now we find the chip */
+		break;
+	}
+
+	if (bbm) {
+		if (bbm->bbm_pages & NAND_BBM_FIRSTPAGE) {
+			chip->bbt_options &= ~NAND_BBT_SCANLASTPAGE;
+			chip->bbt_options &= ~NAND_BBT_SCAN2NDPAGE;
+		}
+		if (bbm->bbm_pages & NAND_BBM_SECONDPAGE) {
+			chip->bbt_options &= ~NAND_BBT_SCANLASTPAGE;
+			chip->bbt_options |= NAND_BBT_SCAN_SCAN2NDPAGE;
+		}
+		if (bbm->bbm_pages & NAND_BBM_LASTPAGE) {
+			chip->bbt_options |= NAND_BBT_SCANLASTPAGE;
+		}
+	}
+}
+
 /*
  * Set the bad block marker/indicator (BBM/BBI) patterns according to some
  * heuristic patterns using various detected parameters (e.g., manufacturer,
@@ -3736,6 +3769,8 @@ static void nand_decode_bbm_options(struct mtd_info *mtd,
 			(mtd->writesize == 2048 &&
 			 maf_id == NAND_MFR_MICRON))
 		chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
+
+	nand_decode_bbm_fixup(chip, id_data)
 }

 static inline bool is_full_id_nand(struct nand_flash_dev *type)
diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index a8804a3..56e59b4 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -184,8 +184,18 @@ struct nand_manufacturers nand_manuf_ids[] = {
 	{0x0, "Unknown"}
 };

+/* BBM page fixup table */
+struct nand_bbm_page nand_bbm_page_fixup[] = {
+	{"MT29F1G08ABAEAWP:E",
+	 {0x2C, 0xF1, 0x80, 0x95, 0x04, 0x00, 0x00, 0x00},
+	 NAND_BBM_FIRSTPAGE
+	},
+	{NULL}
+};
+
 EXPORT_SYMBOL(nand_manuf_ids);
 EXPORT_SYMBOL(nand_flash_ids);
+EXPORT_SYMBOL(nand_bbm_page_fixup);

 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Thomas Gleixner <tglx@linutronix.de>");
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 5a9d1d4..287874b 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -829,8 +829,28 @@ struct nand_manufacturers {
 	char *name;
 };

+/**
+ * struct nand_bbm_page - NAND Flash Bad Block Marker Position
+ * @name:	NAND chip name
+ * @id:		Full device ID
+ * @bbm_page:	Which page the BBM locates: NAND_BBM_FIRSTPAGE,
+ *              NAND_BBM_SECONDPAGE, NAND_BBM_LASTPAGE
+ */
+struct nand_bbm_page {
+	char *name;
+	union {
+		struct {
+			uint8_t mfr_id;
+			uint8_t dev_id;
+		};
+		uint8_t id[NAND_MAX_ID_LEN];
+	};
+	unsigned int bbm_page;
+};
+
 extern struct nand_flash_dev nand_flash_ids[];
 extern struct nand_manufacturers nand_manuf_ids[];
+extern struct nand_bbm_page nand_bbm_page_fixup[];

 extern int nand_default_bbt(struct mtd_info *mtd);
 extern int nand_markbad_bbt(struct mtd_info *mtd, loff_t offs);

thanks,
Sheng

> Also, you can refer to this (old) table of NAND flash [1] that I and a few
> others catalogued, and it notes a few flash that are not supported
> correctly. It may be a bit out of date.
> 
> Brian
> 
> [1] http://linux-mtd.infradead.org/nand-data/nanddata.html
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
> 

questions about [PATCH]mtd: nand: fix SCAN2NDPAGE check for BBM

[Yanjiantao]

Dear Brian,

i read the patch:  " mtd: nand: fix SCAN2NDPAGE check for BBM "
And i am confused about the markbad flows.

		xxxxx;
        if (chip->bbt_options & NAND_BBT_SCANLASTPAGE)
                ofs += mtd->erasesize - mtd->writesize;

		xxxxx;

        do { 
                if (chip->options & NAND_BUSWIDTH_16) {
                        chip->cmdfunc(mtd, NAND_CMD_READOOB,
                                        chip->badblockpos & 0xFE, page);
                        bad = cpu_to_le16(chip->read_word(mtd));
                        if (chip->badblockpos & 0x1) 
                                bad >>= 8;
                        else 
                                bad &= 0xFF;
                } else {
                        chip->cmdfunc(mtd, NAND_CMD_READOOB, chip->badblockpos,
                                        page);
                        bad = chip->read_byte(mtd);
                }    

                if (likely(chip->badblockbits == 8))
                        res = bad != 0xFF;
                else 
                        res = hweight8(bad) < chip->badblockbits;
                ofs += mtd->writesize;
                page = (int)(ofs >> chip->page_shift) & chip->pagemask;
                i++; 
        } while (!res && i < 2 && (chip->bbt_options & NAND_BBT_SCAN2NDPAGE)); 
mark bad flows is the same.

If I understand the patch right, then I think that:
1. when NAND_BBT_SCANLASTPAGE is set, markbad and checkbad on the last page only.
2. when NAND_BBT_SCAN2NDPAGE is set, markbad and checkbad on the 1st and 2nd page.

I checked some datasheets of different manufacturers(such as SAMSUNG/HYNIX/MICRON/TOSHIBA), they claim that bad block marker is on 1st/2nd, or 1st/last, or 1st/2nd/last(for spansion).
the checkbad method may check fail in some cases:
for example,:
1. for spansion nand(bad block marker is on 1st or 2nd or last page), set NAND_BBT_SCAN2NDPAGE while badblock marker is on last page or set NAND_BBT_SCANLASTPAGE while badblock marker is on 1st/2nd page may cause badblock chech failed.
2. for HYNIX (H27UBG8T2CTR) nand (bad block marker is on 1st or last page), check may failed whether NAND_BBT_SCANLASTPAGE is set or not.

Re: questions about [PATCH]mtd: nand: fix SCAN2NDPAGE check for BBM

[Brian Norris]

On Tue, Oct 13, 2015 at 11:33:54AM +0000, Yanjiantao wrote:
> Dear Brian,
> 
> i read the patch:  " mtd: nand: fix SCAN2NDPAGE check for BBM "
> And i am confused about the markbad flows.
> 
> 		xxxxx;
>         if (chip->bbt_options & NAND_BBT_SCANLASTPAGE)
>                 ofs += mtd->erasesize - mtd->writesize;
> 
> 		xxxxx;
> 
>         do { 
>                 if (chip->options & NAND_BUSWIDTH_16) {
>                         chip->cmdfunc(mtd, NAND_CMD_READOOB,
>                                         chip->badblockpos & 0xFE, page);
>                         bad = cpu_to_le16(chip->read_word(mtd));
>                         if (chip->badblockpos & 0x1) 
>                                 bad >>= 8;
>                         else 
>                                 bad &= 0xFF;
>                 } else {
>                         chip->cmdfunc(mtd, NAND_CMD_READOOB, chip->badblockpos,
>                                         page);
>                         bad = chip->read_byte(mtd);
>                 }    
> 
>                 if (likely(chip->badblockbits == 8))
>                         res = bad != 0xFF;
>                 else 
>                         res = hweight8(bad) < chip->badblockbits;
>                 ofs += mtd->writesize;
>                 page = (int)(ofs >> chip->page_shift) & chip->pagemask;
>                 i++; 
>         } while (!res && i < 2 && (chip->bbt_options & NAND_BBT_SCAN2NDPAGE)); 
> mark bad flows is the same.
> 
> If I understand the patch right, then I think that:
> 1. when NAND_BBT_SCANLASTPAGE is set, markbad and checkbad on the last page only.
> 2. when NAND_BBT_SCAN2NDPAGE is set, markbad and checkbad on the 1st and 2nd page.
> 
> I checked some datasheets of different manufacturers(such as SAMSUNG/HYNIX/MICRON/TOSHIBA), they claim that bad block marker is on 1st/2nd, or 1st/last, or 1st/2nd/last(for spansion).
> the checkbad method may check fail in some cases:
> for example,:
> 1. for spansion nand(bad block marker is on 1st or 2nd or last page), set NAND_BBT_SCAN2NDPAGE while badblock marker is on last page or set NAND_BBT_SCANLASTPAGE while badblock marker is on 1st/2nd page may cause badblock chech failed.
> 2. for HYNIX (H27UBG8T2CTR) nand (bad block marker is on 1st or last page), check may failed whether NAND_BBT_SCANLASTPAGE is set or not.

Are you repeating the question from here [1]? The refer to my response
here [2].

Brian

[1] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062486.html
[2] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062509.html