[PATCH] mtd: nand_bbt: kill NAND_BBT_SCANALLPAGES

[PATCH] mtd: nand_bbt: kill NAND_BBT_SCANALLPAGES

[Brian Norris]

Now that the last user of NAND_BBT_SCANALLPAGES has been removed, let's
kill this peculiar BBT feature flag.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 Documentation/DocBook/mtdnand.tmpl |  2 --
 drivers/mtd/nand/nand_bbt.c        | 37 +++----------------------------------
 include/linux/mtd/bbm.h            |  2 --
 3 files changed, 3 insertions(+), 38 deletions(-)

diff --git a/Documentation/DocBook/mtdnand.tmpl b/Documentation/DocBook/mtdnand.tmpl
index a248f42a121e..cd11926e07c7 100644
--- a/Documentation/DocBook/mtdnand.tmpl
+++ b/Documentation/DocBook/mtdnand.tmpl
@@ -1222,8 +1222,6 @@ in this page</entry>
 #define NAND_BBT_VERSION	0x00000100
 /* Create a bbt if none axists */
 #define NAND_BBT_CREATE		0x00000200
-/* Search good / bad pattern through all pages of a block */
-#define NAND_BBT_SCANALLPAGES	0x00000400
 /* Write bbt if neccecary */
 #define NAND_BBT_WRITE		0x00001000
 /* Read and write back block contents when writing bbt */
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index c75b6a7c6ea4..c0615d1526f9 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -412,25 +412,6 @@ static void read_abs_bbts(struct mtd_info *mtd, uint8_t *buf,
 	}
 }
 
-/* Scan a given block full */
-static int scan_block_full(struct mtd_info *mtd, struct nand_bbt_descr *bd,
-			   loff_t offs, uint8_t *buf, size_t readlen,
-			   int scanlen, int numpages)
-{
-	int ret, j;
-
-	ret = scan_read_oob(mtd, buf, offs, readlen);
-	/* Ignore ECC errors when checking for BBM */
-	if (ret && !mtd_is_bitflip_or_eccerr(ret))
-		return ret;
-
-	for (j = 0; j < numpages; j++, buf += scanlen) {
-		if (check_pattern(buf, scanlen, mtd->writesize, bd))
-			return 1;
-	}
-	return 0;
-}
-
 /* Scan a given block partially */
 static int scan_block_fast(struct mtd_info *mtd, struct nand_bbt_descr *bd,
 			   loff_t offs, uint8_t *buf, int numpages)
@@ -477,24 +458,17 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
 	struct nand_bbt_descr *bd, int chip)
 {
 	struct nand_chip *this = mtd->priv;
-	int i, numblocks, numpages, scanlen;
+	int i, numblocks, numpages;
 	int startblock;
 	loff_t from;
-	size_t readlen;
 
 	pr_info("Scanning device for bad blocks\n");
 
-	if (bd->options & NAND_BBT_SCANALLPAGES)
-		numpages = 1 << (this->bbt_erase_shift - this->page_shift);
-	else if (bd->options & NAND_BBT_SCAN2NDPAGE)
+	if (bd->options & NAND_BBT_SCAN2NDPAGE)
 		numpages = 2;
 	else
 		numpages = 1;
 
-	/* We need only read few bytes from the OOB area */
-	scanlen = 0;
-	readlen = bd->len;
-
 	if (chip == -1) {
 		numblocks = mtd->size >> this->bbt_erase_shift;
 		startblock = 0;
@@ -519,12 +493,7 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
 
 		BUG_ON(bd->options & NAND_BBT_NO_OOB);
 
-		if (bd->options & NAND_BBT_SCANALLPAGES)
-			ret = scan_block_full(mtd, bd, from, buf, readlen,
-					      scanlen, numpages);
-		else
-			ret = scan_block_fast(mtd, bd, from, buf, numpages);
-
+		ret = scan_block_fast(mtd, bd, from, buf, numpages);
 		if (ret < 0)
 			return ret;
 
diff --git a/include/linux/mtd/bbm.h b/include/linux/mtd/bbm.h
index 95fc482cef36..36bb6a503f19 100644
--- a/include/linux/mtd/bbm.h
+++ b/include/linux/mtd/bbm.h
@@ -91,8 +91,6 @@ struct nand_bbt_descr {
  * with NAND_BBT_CREATE.
  */
 #define NAND_BBT_CREATE_EMPTY	0x00000400
-/* Search good / bad pattern through all pages of a block */
-#define NAND_BBT_SCANALLPAGES	0x00000800
 /* Write bbt if neccecary */
 #define NAND_BBT_WRITE		0x00002000
 /* Read and write back block contents when writing bbt */
-- 
1.8.1.2

Re: [PATCH] mtd: nand_bbt: kill NAND_BBT_SCANALLPAGES

[Ezequiel Garcia]

On Wed, Oct 30, 2013 at 10:29:35PM -0400, Brian Norris wrote:
> Now that the last user of NAND_BBT_SCANALLPAGES has been removed, let's
> kill this peculiar BBT feature flag.
> 

I must admit I also find this option a bit puzzling.

However, I'm wondering what happens if a manufacturer specifies
the bad block mark is in some page at the middle of a block.

AFAIK, some of them do exactly that, and I've always thought
this option was the solution for such cases.

So, is this really no longer needed?

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH] mtd: nand_bbt: kill NAND_BBT_SCANALLPAGES

[Brian Norris]

On Thu, Oct 31, 2013 at 07:20:39AM -0300, Ezequiel Garcia wrote:
> On Wed, Oct 30, 2013 at 10:29:35PM -0400, Brian Norris wrote:
> > Now that the last user of NAND_BBT_SCANALLPAGES has been removed, let's
> > kill this peculiar BBT feature flag.
> > 
> 
> I must admit I also find this option a bit puzzling.
> 
> However, I'm wondering what happens if a manufacturer specifies
> the bad block mark is in some page at the middle of a block.
> 
> AFAIK, some of them do exactly that, and I've always thought
> this option was the solution for such cases.

Well, it's not really a *good* solution for a marker in the middle of
the page, as it's both inaccurate and heavily inefficient. But honestly,
I've never heard of such a device. (There is a rare one or two that uses
the 2nd-to-last or 3rd-to-last page.) Do you have any examples?

> So, is this really no longer needed?

I think it just adds unnecessary complexity to nand_bbt.c. Generally,
the fewer (unused) options the better. Also, this helps clear the way
for some nand_base/nand_bbt simplification that I have planned.

Additionally, I think its only users were either accidental (in the case
of omap2.c) or lazy (which didn't want to figure out the correct pages
to scan for bad block markers). And now there are no users.

Brian

Re: [PATCH] mtd: nand_bbt: kill NAND_BBT_SCANALLPAGES

[Ezequiel Garcia]

On Thu, Oct 31, 2013 at 11:22:45AM -0400, Brian Norris wrote:
> On Thu, Oct 31, 2013 at 07:20:39AM -0300, Ezequiel Garcia wrote:
> > On Wed, Oct 30, 2013 at 10:29:35PM -0400, Brian Norris wrote:
> > > Now that the last user of NAND_BBT_SCANALLPAGES has been removed, let's
> > > kill this peculiar BBT feature flag.
> > > 
> > 
> > I must admit I also find this option a bit puzzling.
> > 
> > However, I'm wondering what happens if a manufacturer specifies
> > the bad block mark is in some page at the middle of a block.
> > 
> > AFAIK, some of them do exactly that, and I've always thought
> > this option was the solution for such cases.
> 
> Well, it's not really a *good* solution for a marker in the middle of
> the page, as it's both inaccurate and heavily inefficient.

Granted.

> I've never heard of such a device. (There is a rare one or two that uses
> the 2nd-to-last or 3rd-to-last page.) Do you have any examples?
> 

Not at hand.

> > So, is this really no longer needed?
> 
> I think it just adds unnecessary complexity to nand_bbt.c. Generally,
> the fewer (unused) options the better. Also, this helps clear the way
> for some nand_base/nand_bbt simplification that I have planned.
> 
> Additionally, I think its only users were either accidental (in the case
> of omap2.c) or lazy (which didn't want to figure out the correct pages
> to scan for bad block markers). And now there are no users.
> 

Ok, agreed. But do we have any mechanism to scan a *particular*
page?

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH] mtd: nand_bbt: kill NAND_BBT_SCANALLPAGES

[Brian Norris]

On Thu, Oct 31, 2013 at 12:32:43PM -0300, Ezequiel Garcia wrote:
> But do we have any mechanism to scan a *particular*
> page?

No. But if we need it, we can develop a proper one. For instance, I
think a properly flexible approach could involve some kind of bitmap,
where each bit represents a particular page (e.g., a 32-bit bitmap where
the first 16 bits represent the first 16 pages and the last 16 represent
the last 16 pages).

I think, though, that modern NAND are converging to only a few types of
bad block marker locations, and our currently available flags (without
SCANALLPAGES) are sufficient.

Brian

Re: [PATCH] mtd: nand_bbt: kill NAND_BBT_SCANALLPAGES

[Ezequiel Garcia]

On Wed, Oct 30, 2013 at 10:29:35PM -0400, Brian Norris wrote:
> Now that the last user of NAND_BBT_SCANALLPAGES has been removed, let's
> kill this peculiar BBT feature flag.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Reviewed-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>

> ---
>  Documentation/DocBook/mtdnand.tmpl |  2 --
>  drivers/mtd/nand/nand_bbt.c        | 37 +++----------------------------------
>  include/linux/mtd/bbm.h            |  2 --
>  3 files changed, 3 insertions(+), 38 deletions(-)
> 
> diff --git a/Documentation/DocBook/mtdnand.tmpl b/Documentation/DocBook/mtdnand.tmpl
> index a248f42a121e..cd11926e07c7 100644
> --- a/Documentation/DocBook/mtdnand.tmpl
> +++ b/Documentation/DocBook/mtdnand.tmpl
> @@ -1222,8 +1222,6 @@ in this page</entry>
>  #define NAND_BBT_VERSION	0x00000100
>  /* Create a bbt if none axists */
>  #define NAND_BBT_CREATE		0x00000200
> -/* Search good / bad pattern through all pages of a block */
> -#define NAND_BBT_SCANALLPAGES	0x00000400
>  /* Write bbt if neccecary */
>  #define NAND_BBT_WRITE		0x00001000
>  /* Read and write back block contents when writing bbt */
> diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
> index c75b6a7c6ea4..c0615d1526f9 100644
> --- a/drivers/mtd/nand/nand_bbt.c
> +++ b/drivers/mtd/nand/nand_bbt.c
> @@ -412,25 +412,6 @@ static void read_abs_bbts(struct mtd_info *mtd, uint8_t *buf,
>  	}
>  }
>  
> -/* Scan a given block full */
> -static int scan_block_full(struct mtd_info *mtd, struct nand_bbt_descr *bd,
> -			   loff_t offs, uint8_t *buf, size_t readlen,
> -			   int scanlen, int numpages)
> -{
> -	int ret, j;
> -
> -	ret = scan_read_oob(mtd, buf, offs, readlen);
> -	/* Ignore ECC errors when checking for BBM */
> -	if (ret && !mtd_is_bitflip_or_eccerr(ret))
> -		return ret;
> -
> -	for (j = 0; j < numpages; j++, buf += scanlen) {
> -		if (check_pattern(buf, scanlen, mtd->writesize, bd))
> -			return 1;
> -	}
> -	return 0;
> -}
> -
>  /* Scan a given block partially */
>  static int scan_block_fast(struct mtd_info *mtd, struct nand_bbt_descr *bd,
>  			   loff_t offs, uint8_t *buf, int numpages)
> @@ -477,24 +458,17 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
>  	struct nand_bbt_descr *bd, int chip)
>  {
>  	struct nand_chip *this = mtd->priv;
> -	int i, numblocks, numpages, scanlen;
> +	int i, numblocks, numpages;
>  	int startblock;
>  	loff_t from;
> -	size_t readlen;
>  
>  	pr_info("Scanning device for bad blocks\n");
>  
> -	if (bd->options & NAND_BBT_SCANALLPAGES)
> -		numpages = 1 << (this->bbt_erase_shift - this->page_shift);
> -	else if (bd->options & NAND_BBT_SCAN2NDPAGE)
> +	if (bd->options & NAND_BBT_SCAN2NDPAGE)
>  		numpages = 2;
>  	else
>  		numpages = 1;
>  
> -	/* We need only read few bytes from the OOB area */
> -	scanlen = 0;
> -	readlen = bd->len;
> -
>  	if (chip == -1) {
>  		numblocks = mtd->size >> this->bbt_erase_shift;
>  		startblock = 0;
> @@ -519,12 +493,7 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
>  
>  		BUG_ON(bd->options & NAND_BBT_NO_OOB);
>  
> -		if (bd->options & NAND_BBT_SCANALLPAGES)
> -			ret = scan_block_full(mtd, bd, from, buf, readlen,
> -					      scanlen, numpages);
> -		else
> -			ret = scan_block_fast(mtd, bd, from, buf, numpages);
> -
> +		ret = scan_block_fast(mtd, bd, from, buf, numpages);
>  		if (ret < 0)
>  			return ret;
>  
> diff --git a/include/linux/mtd/bbm.h b/include/linux/mtd/bbm.h
> index 95fc482cef36..36bb6a503f19 100644
> --- a/include/linux/mtd/bbm.h
> +++ b/include/linux/mtd/bbm.h
> @@ -91,8 +91,6 @@ struct nand_bbt_descr {
>   * with NAND_BBT_CREATE.
>   */
>  #define NAND_BBT_CREATE_EMPTY	0x00000400
> -/* Search good / bad pattern through all pages of a block */
> -#define NAND_BBT_SCANALLPAGES	0x00000800
>  /* Write bbt if neccecary */
>  #define NAND_BBT_WRITE		0x00002000
>  /* Read and write back block contents when writing bbt */
> -- 
> 1.8.1.2
> 

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH] mtd: nand_bbt: kill NAND_BBT_SCANALLPAGES

[Artem Bityutskiy]

On Thu, 2013-10-31 at 16:04 -0300, Ezequiel Garcia wrote:
> On Wed, Oct 30, 2013 at 10:29:35PM -0400, Brian Norris wrote:
> > Now that the last user of NAND_BBT_SCANALLPAGES has been removed, let's
> > kill this peculiar BBT feature flag.
> > 
> > Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> 
> Reviewed-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>

Yes, I do agree that we do not need to keep dead code just in case.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH] mtd: nand_bbt: kill NAND_BBT_SCANALLPAGES

[Brian Norris]

On Fri, Nov 1, 2013 at 5:01 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Thu, 2013-10-31 at 16:04 -0300, Ezequiel Garcia wrote:
>> On Wed, Oct 30, 2013 at 10:29:35PM -0400, Brian Norris wrote:
>> > Now that the last user of NAND_BBT_SCANALLPAGES has been removed, let's
>> > kill this peculiar BBT feature flag.
>> >
>> > Signed-off-by: Brian Norris <computersforpeace@gmail.com>
>>
>> Reviewed-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>
> Yes, I do agree that we do not need to keep dead code just in case.
>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Pushed to l2-mtd.git.

Brian