Message ID | 20120820162915.7f93e6c4@pixies.home.jungo.com |
---|---|
State | Accepted |
Commit | eceb84b1886acb38e618f3dfb51cd4e53f2ddb97 |
Headers | show |
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>
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
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;
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>
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.
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;
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> ---