Patchwork mtd: nand: Rename create_bbt()'s 'len' variable to 'numpages'

login
register
mail settings
Submitter Shmulik Ladkani
Date Aug. 20, 2012, 1:29 p.m.
Message ID <20120820162915.7f93e6c4@pixies.home.jungo.com>
Download mbox | patch
Permalink /patch/178776/
State New
Headers show

Comments

Shmulik Ladkani - Aug. 20, 2012, 1:29 p.m.
Rename 'len' variable of create_bbt/scan_block_fast/scan_block_full to
'numpages', since it really means number of pages to scan when
searching for the BBM (and not the byte length of the scan).

Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
Brian Norris - Aug. 25, 2012, 12:18 a.m.
On Mon, Aug 20, 2012 at 6:29 AM, Shmulik Ladkani
<shmulik.ladkani@gmail.com> wrote:
> Rename 'len' variable of create_bbt/scan_block_fast/scan_block_full to
> 'numpages', since it really means number of pages to scan when
> searching for the BBM (and not the byte length of the scan).
>
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>

Thanks, this kind of fix is actually very helpful! It's kinda annoying
that I always have to translate in my head the fact that 'len' means
'number of pages.' And now that you mention it, I should have just
changed this myself a long time ago :)

BTW, some (all?) of this might die if I (or Matthieu) ever end up
polishing Matthieu's patches to utilize nand_chip->block_bad instead
of much of this code. But that's no issue.

I'm not sure what goes here. Acked-by? Reviewed-by? Gladly-welcomed-by?

Reviewed-by: Brian Norris <computersforpeace@gmail.com>
Shmulik Ladkani - Aug. 25, 2012, 8:32 a.m.
On Fri, 24 Aug 2012 17:18:22 -0700 Brian Norris <computersforpeace@gmail.com> wrote:
> On Mon, Aug 20, 2012 at 6:29 AM, Shmulik Ladkani
> Thanks, this kind of fix is actually very helpful! It's kinda annoying
> that I always have to translate in my head the fact that 'len' means
> 'number of pages.'

Exactly.

It was triggered from reviewing some of the latest bbt patches; I was
summarizing on a _paper_: "len is numpages, scanlen is ... readlen is ..."
and I thought to myself, isn't this crazy?

So yes, naming is important ;-)

(BTW scanlen and readlen are annoying, too... but they don't have a
concise meaning)

Regards,
Shmulik
Shmulik Ladkani - Sept. 22, 2012, 6:54 p.m.
Ping.

On Mon, 20 Aug 2012 16:29:15 +0300 Shmulik Ladkani <shmulik.ladkani@gmail.com> wrote:
> Rename 'len' variable of create_bbt/scan_block_fast/scan_block_full to
> 'numpages', since it really means number of pages to scan when
> searching for the BBM (and not the byte length of the scan).
> 
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> ---
> diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
> index 9a5402e..24b0a4c 100644
> --- a/drivers/mtd/nand/nand_bbt.c
> +++ b/drivers/mtd/nand/nand_bbt.c
> @@ -401,7 +401,7 @@ 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 len)
> +			   int scanlen, int numpages)
>  {
>  	int ret, j;
>  
> @@ -410,7 +410,7 @@ static int scan_block_full(struct mtd_info *mtd, struct nand_bbt_descr *bd,
>  	if (ret && !mtd_is_bitflip_or_eccerr(ret))
>  		return ret;
>  
> -	for (j = 0; j < len; j++, buf += scanlen) {
> +	for (j = 0; j < numpages; j++, buf += scanlen) {
>  		if (check_pattern(buf, scanlen, mtd->writesize, bd))
>  			return 1;
>  	}
> @@ -419,7 +419,7 @@ static int scan_block_full(struct mtd_info *mtd, struct nand_bbt_descr *bd,
>  
>  /* 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 len)
> +			   loff_t offs, uint8_t *buf, int numpages)
>  {
>  	struct mtd_oob_ops ops;
>  	int j, ret;
> @@ -430,7 +430,7 @@ static int scan_block_fast(struct mtd_info *mtd, struct nand_bbt_descr *bd,
>  	ops.datbuf = NULL;
>  	ops.mode = MTD_OPS_PLACE_OOB;
>  
> -	for (j = 0; j < len; j++) {
> +	for (j = 0; j < numpages; j++) {
>  		/*
>  		 * Read the full oob until read_oob is fixed to handle single
>  		 * byte reads for 16 bit buswidth.
> @@ -463,7 +463,7 @@ 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, len, scanlen;
> +	int i, numblocks, numpages, scanlen;
>  	int startblock;
>  	loff_t from;
>  	size_t readlen;
> @@ -471,11 +471,11 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
>  	pr_info("Scanning device for bad blocks\n");
>  
>  	if (bd->options & NAND_BBT_SCANALLPAGES)
> -		len = 1 << (this->bbt_erase_shift - this->page_shift);
> +		numpages = 1 << (this->bbt_erase_shift - this->page_shift);
>  	else if (bd->options & NAND_BBT_SCAN2NDPAGE)
> -		len = 2;
> +		numpages = 2;
>  	else
> -		len = 1;
> +		numpages = 1;
>  
>  	if (!(bd->options & NAND_BBT_SCANEMPTY)) {
>  		/* We need only read few bytes from the OOB area */
> @@ -484,7 +484,7 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
>  	} else {
>  		/* Full page content should be read */
>  		scanlen = mtd->writesize + mtd->oobsize;
> -		readlen = len * mtd->writesize;
> +		readlen = numpages * mtd->writesize;
>  	}
>  
>  	if (chip == -1) {
> @@ -508,7 +508,7 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
>  	}
>  
>  	if (this->bbt_options & NAND_BBT_SCANLASTPAGE)
> -		from += mtd->erasesize - (mtd->writesize * len);
> +		from += mtd->erasesize - (mtd->writesize * numpages);
>  
>  	for (i = startblock; i < numblocks;) {
>  		int ret;
> @@ -517,9 +517,9 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
>  
>  		if (bd->options & NAND_BBT_SCANALLPAGES)
>  			ret = scan_block_full(mtd, bd, from, buf, readlen,
> -					      scanlen, len);
> +					      scanlen, numpages);
>  		else
> -			ret = scan_block_fast(mtd, bd, from, buf, len);
> +			ret = scan_block_fast(mtd, bd, from, buf, numpages);
>  
>  		if (ret < 0)
>  			return ret;
Artem Bityutskiy - Sept. 23, 2012, 12:02 p.m.
On Sat, 2012-09-22 at 21:54 +0300, Shmulik Ladkani wrote:
> Ping.

It is in my tree for a long time, I just somehow forgot to notify you.

commit 88b9e2c45297129863e0befc47881b3899402850
Author: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Date:   Mon Aug 20 16:29:15 2012 +0300

    mtd: nand: rename create_bbt()'s 'len' variable to 'numpages'
    
    Rename 'len' variable of create_bbt/scan_block_fast/scan_block_full to
    'numpages', since it really means number of pages to scan when
    searching for the BBM (and not the byte length of the scan).
    
    Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
    Reviewed-by: Brian Norris <computersforpeace@gmail.com>
    Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Shmulik Ladkani - Sept. 23, 2012, 2:38 p.m.
On Sun, 23 Sep 2012 15:02:29 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> It is in my tree for a long time, I just somehow forgot to notify you.

OK, Thanks.

Patch

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 9a5402e..24b0a4c 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -401,7 +401,7 @@  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 len)
+			   int scanlen, int numpages)
 {
 	int ret, j;
 
@@ -410,7 +410,7 @@  static int scan_block_full(struct mtd_info *mtd, struct nand_bbt_descr *bd,
 	if (ret && !mtd_is_bitflip_or_eccerr(ret))
 		return ret;
 
-	for (j = 0; j < len; j++, buf += scanlen) {
+	for (j = 0; j < numpages; j++, buf += scanlen) {
 		if (check_pattern(buf, scanlen, mtd->writesize, bd))
 			return 1;
 	}
@@ -419,7 +419,7 @@  static int scan_block_full(struct mtd_info *mtd, struct nand_bbt_descr *bd,
 
 /* 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 len)
+			   loff_t offs, uint8_t *buf, int numpages)
 {
 	struct mtd_oob_ops ops;
 	int j, ret;
@@ -430,7 +430,7 @@  static int scan_block_fast(struct mtd_info *mtd, struct nand_bbt_descr *bd,
 	ops.datbuf = NULL;
 	ops.mode = MTD_OPS_PLACE_OOB;
 
-	for (j = 0; j < len; j++) {
+	for (j = 0; j < numpages; j++) {
 		/*
 		 * Read the full oob until read_oob is fixed to handle single
 		 * byte reads for 16 bit buswidth.
@@ -463,7 +463,7 @@  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, len, scanlen;
+	int i, numblocks, numpages, scanlen;
 	int startblock;
 	loff_t from;
 	size_t readlen;
@@ -471,11 +471,11 @@  static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
 	pr_info("Scanning device for bad blocks\n");
 
 	if (bd->options & NAND_BBT_SCANALLPAGES)
-		len = 1 << (this->bbt_erase_shift - this->page_shift);
+		numpages = 1 << (this->bbt_erase_shift - this->page_shift);
 	else if (bd->options & NAND_BBT_SCAN2NDPAGE)
-		len = 2;
+		numpages = 2;
 	else
-		len = 1;
+		numpages = 1;
 
 	if (!(bd->options & NAND_BBT_SCANEMPTY)) {
 		/* We need only read few bytes from the OOB area */
@@ -484,7 +484,7 @@  static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
 	} else {
 		/* Full page content should be read */
 		scanlen = mtd->writesize + mtd->oobsize;
-		readlen = len * mtd->writesize;
+		readlen = numpages * mtd->writesize;
 	}
 
 	if (chip == -1) {
@@ -508,7 +508,7 @@  static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
 	}
 
 	if (this->bbt_options & NAND_BBT_SCANLASTPAGE)
-		from += mtd->erasesize - (mtd->writesize * len);
+		from += mtd->erasesize - (mtd->writesize * numpages);
 
 	for (i = startblock; i < numblocks;) {
 		int ret;
@@ -517,9 +517,9 @@  static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
 
 		if (bd->options & NAND_BBT_SCANALLPAGES)
 			ret = scan_block_full(mtd, bd, from, buf, readlen,
-					      scanlen, len);
+					      scanlen, numpages);
 		else
-			ret = scan_block_fast(mtd, bd, from, buf, len);
+			ret = scan_block_fast(mtd, bd, from, buf, numpages);
 
 		if (ret < 0)
 			return ret;