| Message ID | 1383186575-31328-1-git-send-email-computersforpeace@gmail.com |
|---|---|
| State | Accepted |
| Commit | 5961ad2cb4dd14933889f5219e0d8505669d752d |
| Headers | show |
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?
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
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?
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
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 >
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>
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
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 */
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(-)