* [PATCH 0/4] mtd: nand: cleanups to BBT
@ 2013-07-24 6:27 Brian Norris
2013-07-24 6:27 ` [PATCH 1/4] mtd: nand: add accessors, macros for in-memory BBT Brian Norris
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Brian Norris @ 2013-07-24 6:27 UTC (permalink / raw)
To: linux-mtd; +Cc: Brian Norris, David Woodhouse, Artem Bityutskiy
A few random BBT cleanups, some of which I've had sitting around for a
while. There will probably be some more in the near future.
Brian Norris (4):
mtd: nand: add accessors, macros for in-memory BBT
mtd: nand: refactor chip->block_markbad interface
mtd: nand: hide in-memory BBT implementation details
mtd: nand: remove NAND_BBT_SCANEMPTY
Documentation/DocBook/mtdnand.tmpl | 2 -
drivers/mtd/nand/docg4.c | 6 -
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 44 +++-----
drivers/mtd/nand/nand_base.c | 113 ++++++++++---------
drivers/mtd/nand/nand_bbt.c | 194 +++++++++++++++++----------------
drivers/mtd/nand/omap2.c | 2 +-
drivers/mtd/nand/sm_common.c | 9 +-
drivers/mtd/onenand/onenand_bbt.c | 1 -
include/linux/mtd/bbm.h | 2 -
include/linux/mtd/nand.h | 2 +-
10 files changed, 185 insertions(+), 190 deletions(-)
--
1.8.3.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/4] mtd: nand: add accessors, macros for in-memory BBT
2013-07-24 6:27 [PATCH 0/4] mtd: nand: cleanups to BBT Brian Norris
@ 2013-07-24 6:27 ` Brian Norris
2013-07-30 22:02 ` Ezequiel Garcia
2013-07-24 6:27 ` [PATCH 2/4] mtd: nand: refactor chip->block_markbad interface Brian Norris
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Brian Norris @ 2013-07-24 6:27 UTC (permalink / raw)
To: linux-mtd; +Cc: Brian Norris, David Woodhouse, Artem Bityutskiy
There is an abundance of magic numbers and complicated shifting/masking
logic in the in-memory BBT code, and due to the complicated
shifting/masking, we often store the block number multiplied by 2.
Together, these features make the code unnecessary complex and hard to
read.
This patch adds macros to represent the 00b, 01b, 10b, and 11b
memory-BBT magic numbers, as well as two accessor functions for reading
and marking the memory-BBT bitfield for a given block.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
drivers/mtd/nand/nand_bbt.c | 132 ++++++++++++++++++++++++--------------------
1 file changed, 72 insertions(+), 60 deletions(-)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 2672643..3f18776 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -71,6 +71,28 @@
#include <linux/export.h>
#include <linux/string.h>
+#define BBT_BLOCK_GOOD 0x00
+#define BBT_BLOCK_WORN 0x01
+#define BBT_BLOCK_RESERVED 0x02
+#define BBT_BLOCK_FACTORY_BAD 0x03
+
+#define BBT_ENTRY_MASK 0x03
+#define BBT_ENTRY_SHIFT 2
+
+static inline uint8_t bbt_get_entry(struct nand_chip *chip, int block)
+{
+ uint8_t entry = chip->bbt[block >> BBT_ENTRY_SHIFT];
+ entry >>= (block & BBT_ENTRY_MASK) * 2;
+ return entry & BBT_ENTRY_MASK;
+}
+
+static inline void bbt_mark_entry(struct nand_chip *chip, int block,
+ uint8_t mark)
+{
+ uint8_t msk = (mark & BBT_ENTRY_MASK) << ((block & BBT_ENTRY_MASK) * 2);
+ chip->bbt[block >> BBT_ENTRY_SHIFT] |= msk;
+}
+
static int check_pattern_no_oob(uint8_t *buf, struct nand_bbt_descr *td)
{
if (memcmp(buf, td->pattern, td->len))
@@ -159,7 +181,7 @@ static u32 add_marker_len(struct nand_bbt_descr *td)
* @page: the starting page
* @num: the number of bbt descriptors to read
* @td: the bbt describtion table
- * @offs: offset in the memory table
+ * @offs: block number offset in the table
*
* Read the bad block table starting from page.
*/
@@ -209,14 +231,16 @@ static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num,
/* Analyse data */
for (i = 0; i < len; i++) {
uint8_t dat = buf[i];
- for (j = 0; j < 8; j += bits, act += 2) {
+ for (j = 0; j < 8; j += bits, act++) {
uint8_t tmp = (dat >> j) & msk;
if (tmp == msk)
continue;
if (reserved_block_code && (tmp == reserved_block_code)) {
pr_info("nand_read_bbt: reserved block at 0x%012llx\n",
- (loff_t)((offs << 2) + (act >> 1)) << this->bbt_erase_shift);
- this->bbt[offs + (act >> 3)] |= 0x2 << (act & 0x06);
+ (loff_t)(offs + act) <<
+ this->bbt_erase_shift);
+ bbt_mark_entry(this, offs + act,
+ BBT_BLOCK_RESERVED);
mtd->ecc_stats.bbtblocks++;
continue;
}
@@ -225,12 +249,15 @@ static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num,
* move this message to pr_debug.
*/
pr_info("nand_read_bbt: bad block at 0x%012llx\n",
- (loff_t)((offs << 2) + (act >> 1)) << this->bbt_erase_shift);
+ (loff_t)(offs + act) <<
+ this->bbt_erase_shift);
/* Factory marked bad or worn out? */
if (tmp == 0)
- this->bbt[offs + (act >> 3)] |= 0x3 << (act & 0x06);
+ bbt_mark_entry(this, offs + act,
+ BBT_BLOCK_FACTORY_BAD);
else
- this->bbt[offs + (act >> 3)] |= 0x1 << (act & 0x06);
+ bbt_mark_entry(this, offs + act,
+ BBT_BLOCK_WORN);
mtd->ecc_stats.badblocks++;
}
}
@@ -265,7 +292,7 @@ static int read_abs_bbt(struct mtd_info *mtd, uint8_t *buf, struct nand_bbt_desc
td, offs);
if (res)
return res;
- offs += this->chipsize >> (this->bbt_erase_shift + 2);
+ offs += this->chipsize >> this->bbt_erase_shift;
}
} else {
res = read_bbt(mtd, buf, td->pages[0],
@@ -489,11 +516,7 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
}
if (chip == -1) {
- /*
- * Note that numblocks is 2 * (real numblocks) here, see i+=2
- * below as it makes shifting and masking less painful
- */
- numblocks = mtd->size >> (this->bbt_erase_shift - 1);
+ numblocks = mtd->size >> this->bbt_erase_shift;
startblock = 0;
from = 0;
} else {
@@ -502,16 +525,16 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
chip + 1, this->numchips);
return -EINVAL;
}
- numblocks = this->chipsize >> (this->bbt_erase_shift - 1);
+ numblocks = this->chipsize >> this->bbt_erase_shift;
startblock = chip * numblocks;
numblocks += startblock;
- from = (loff_t)startblock << (this->bbt_erase_shift - 1);
+ from = (loff_t)startblock << this->bbt_erase_shift;
}
if (this->bbt_options & NAND_BBT_SCANLASTPAGE)
from += mtd->erasesize - (mtd->writesize * numpages);
- for (i = startblock; i < numblocks;) {
+ for (i = startblock; i < numblocks; i++) {
int ret;
BUG_ON(bd->options & NAND_BBT_NO_OOB);
@@ -526,13 +549,12 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
return ret;
if (ret) {
- this->bbt[i >> 3] |= 0x03 << (i & 0x6);
+ bbt_mark_entry(this, i, BBT_BLOCK_FACTORY_BAD);
pr_warn("Bad eraseblock %d at 0x%012llx\n",
- i >> 1, (unsigned long long)from);
+ i, (unsigned long long)from);
mtd->ecc_stats.badblocks++;
}
- i += 2;
from += (1 << this->bbt_erase_shift);
}
return 0;
@@ -655,9 +677,9 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
{
struct nand_chip *this = mtd->priv;
struct erase_info einfo;
- int i, j, res, chip = 0;
+ int i, res, chip = 0;
int bits, startblock, dir, page, offs, numblocks, sft, sftmsk;
- int nrchips, bbtoffs, pageoffs, ooboffs;
+ int nrchips, pageoffs, ooboffs;
uint8_t msk[4];
uint8_t rcode = td->reserved_block_code;
size_t retlen, len = 0;
@@ -713,10 +735,9 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
for (i = 0; i < td->maxblocks; i++) {
int block = startblock + dir * i;
/* Check, if the block is bad */
- switch ((this->bbt[block >> 2] >>
- (2 * (block & 0x03))) & 0x03) {
- case 0x01:
- case 0x03:
+ switch (bbt_get_entry(this, block)) {
+ case BBT_BLOCK_WORN:
+ case BBT_BLOCK_FACTORY_BAD:
continue;
}
page = block <<
@@ -748,8 +769,6 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
default: return -EINVAL;
}
- bbtoffs = chip * (numblocks >> 2);
-
to = ((loff_t)page) << this->page_shift;
/* Must we save the block contents? */
@@ -814,16 +833,12 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
buf[ooboffs + td->veroffs] = td->version[chip];
/* Walk through the memory table */
- for (i = 0; i < numblocks;) {
+ for (i = 0; i < numblocks; i++) {
uint8_t dat;
- dat = this->bbt[bbtoffs + (i >> 2)];
- for (j = 0; j < 4; j++, i++) {
- int sftcnt = (i << (3 - sft)) & sftmsk;
- /* Do not store the reserved bbt blocks! */
- buf[offs + (i >> sft)] &=
- ~(msk[dat & 0x03] << sftcnt);
- dat >>= 2;
- }
+ int sftcnt = (i << (3 - sft)) & sftmsk;
+ dat = bbt_get_entry(this, chip * numblocks + i);
+ /* Do not store the reserved bbt blocks! */
+ buf[offs + (i >> sft)] &= ~(msk[dat] << sftcnt);
}
memset(&einfo, 0, sizeof(einfo));
@@ -1009,7 +1024,7 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)
{
struct nand_chip *this = mtd->priv;
int i, j, chips, block, nrblocks, update;
- uint8_t oldval, newval;
+ uint8_t oldval;
/* Do we have a bbt per chip? */
if (td->options & NAND_BBT_PERCHIP) {
@@ -1026,12 +1041,12 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)
if (td->pages[i] == -1)
continue;
block = td->pages[i] >> (this->bbt_erase_shift - this->page_shift);
- block <<= 1;
- oldval = this->bbt[(block >> 3)];
- newval = oldval | (0x2 << (block & 0x06));
- this->bbt[(block >> 3)] = newval;
- if ((oldval != newval) && td->reserved_block_code)
- nand_update_bbt(mtd, (loff_t)block << (this->bbt_erase_shift - 1));
+ oldval = bbt_get_entry(this, block);
+ bbt_mark_entry(this, block, BBT_BLOCK_RESERVED);
+ if ((oldval != BBT_BLOCK_RESERVED) &&
+ td->reserved_block_code)
+ nand_update_bbt(mtd, (loff_t)block <<
+ this->bbt_erase_shift);
continue;
}
update = 0;
@@ -1039,14 +1054,12 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)
block = ((i + 1) * nrblocks) - td->maxblocks;
else
block = i * nrblocks;
- block <<= 1;
for (j = 0; j < td->maxblocks; j++) {
- oldval = this->bbt[(block >> 3)];
- newval = oldval | (0x2 << (block & 0x06));
- this->bbt[(block >> 3)] = newval;
- if (oldval != newval)
+ oldval = bbt_get_entry(this, block);
+ bbt_mark_entry(this, block, BBT_BLOCK_RESERVED);
+ if (oldval != BBT_BLOCK_RESERVED)
update = 1;
- block += 2;
+ block++;
}
/*
* If we want reserved blocks to be recorded to flash, and some
@@ -1054,7 +1067,8 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)
* bbts. This should only happen once.
*/
if (update && td->reserved_block_code)
- nand_update_bbt(mtd, (loff_t)(block - 2) << (this->bbt_erase_shift - 1));
+ nand_update_bbt(mtd, (loff_t)(block - 1) <<
+ this->bbt_erase_shift);
}
}
@@ -1356,23 +1370,21 @@ int nand_default_bbt(struct mtd_info *mtd)
int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)
{
struct nand_chip *this = mtd->priv;
- int block;
- uint8_t res;
+ int block, res;
- /* Get block number * 2 */
- block = (int)(offs >> (this->bbt_erase_shift - 1));
- res = (this->bbt[block >> 3] >> (block & 0x06)) & 0x03;
+ block = (int)(offs >> this->bbt_erase_shift);
+ res = bbt_get_entry(this, block);
pr_debug("nand_isbad_bbt(): bbt info for offs 0x%08x: "
"(block %d) 0x%02x\n",
- (unsigned int)offs, block >> 1, res);
+ (unsigned int)offs, block, res);
- switch ((int)res) {
- case 0x00:
+ switch (res) {
+ case BBT_BLOCK_GOOD:
return 0;
- case 0x01:
+ case BBT_BLOCK_WORN:
return 1;
- case 0x02:
+ case BBT_BLOCK_RESERVED:
return allowbbt ? 0 : 1;
}
return 1;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/4] mtd: nand: refactor chip->block_markbad interface
2013-07-24 6:27 [PATCH 0/4] mtd: nand: cleanups to BBT Brian Norris
2013-07-24 6:27 ` [PATCH 1/4] mtd: nand: add accessors, macros for in-memory BBT Brian Norris
@ 2013-07-24 6:27 ` Brian Norris
2013-07-24 6:59 ` Huang Shijie
2013-07-24 6:27 ` [PATCH 3/4] mtd: nand: hide in-memory BBT implementation details Brian Norris
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Brian Norris @ 2013-07-24 6:27 UTC (permalink / raw)
To: linux-mtd; +Cc: Huang Shijie, Brian Norris, David Woodhouse, Artem Bityutskiy
The chip->block_markbad pointer should really only be responsible for
writing a bad block marker for new bad blocks. It should not take care
of BBT-related functionality, nor should it handle bookkeeping of bad
block stats.
This patch refactors the 3 users of the block_markbad interface (plus
the default nand_base implementation) so that the common code is kept in
nand_block_markbad_lowlevel(). It removes some inconsistencies between
the various implementations and should allow for more centralized
improvements in the future.
Because gpmi-nand no longer needs the nand_update_bbt() function, let's
stop exporting it as well.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: Huang Shijie <b32955@freescale.com>
---
drivers/mtd/nand/docg4.c | 6 ---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 44 +++++++----------
drivers/mtd/nand/nand_base.c | 87 ++++++++++++++++++++--------------
drivers/mtd/nand/nand_bbt.c | 1 -
drivers/mtd/nand/sm_common.c | 9 ++--
5 files changed, 72 insertions(+), 75 deletions(-)
diff --git a/drivers/mtd/nand/docg4.c b/drivers/mtd/nand/docg4.c
index dc86d4a..548db23 100644
--- a/drivers/mtd/nand/docg4.c
+++ b/drivers/mtd/nand/docg4.c
@@ -1093,7 +1093,6 @@ static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)
struct nand_chip *nand = mtd->priv;
struct docg4_priv *doc = nand->priv;
struct nand_bbt_descr *bbtd = nand->badblock_pattern;
- int block = (int)(ofs >> nand->bbt_erase_shift);
int page = (int)(ofs >> nand->page_shift);
uint32_t g4_addr = mtd_to_docg4_address(page, 0);
@@ -1108,9 +1107,6 @@ static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)
if (buf == NULL)
return -ENOMEM;
- /* update bbt in memory */
- nand->bbt[block / 4] |= 0x01 << ((block & 0x03) * 2);
-
/* write bit-wise negation of pattern to oob buffer */
memset(nand->oob_poi, 0xff, mtd->oobsize);
for (i = 0; i < bbtd->len; i++)
@@ -1120,8 +1116,6 @@ static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)
write_page_prologue(mtd, g4_addr);
docg4_write_page(mtd, nand, buf, 1);
ret = pageprog(mtd);
- if (!ret)
- mtd->ecc_stats.badblocks++;
kfree(buf);
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index b741a6c..5947575 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1157,43 +1157,31 @@ static int gpmi_block_markbad(struct mtd_info *mtd, loff_t ofs)
{
struct nand_chip *chip = mtd->priv;
struct gpmi_nand_data *this = chip->priv;
- int block, ret = 0;
+ int ret = 0;
uint8_t *block_mark;
int column, page, status, chipnr;
- /* Get block number */
- block = (int)(ofs >> chip->bbt_erase_shift);
- if (chip->bbt)
- chip->bbt[block >> 2] |= 0x01 << ((block & 0x03) << 1);
+ chipnr = (int)(ofs >> chip->chip_shift);
+ chip->select_chip(mtd, chipnr);
- /* Do we have a flash based bad block table ? */
- if (chip->bbt_options & NAND_BBT_USE_FLASH)
- ret = nand_update_bbt(mtd, ofs);
- else {
- chipnr = (int)(ofs >> chip->chip_shift);
- chip->select_chip(mtd, chipnr);
+ column = this->swap_block_mark ? mtd->writesize : 0;
- column = this->swap_block_mark ? mtd->writesize : 0;
+ /* Write the block mark. */
+ block_mark = this->data_buffer_dma;
+ block_mark[0] = 0; /* bad block marker */
- /* Write the block mark. */
- block_mark = this->data_buffer_dma;
- block_mark[0] = 0; /* bad block marker */
+ /* Shift to get page */
+ page = (int)(ofs >> chip->page_shift);
- /* Shift to get page */
- page = (int)(ofs >> chip->page_shift);
+ chip->cmdfunc(mtd, NAND_CMD_SEQIN, column, page);
+ chip->write_buf(mtd, block_mark, 1);
+ chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
- chip->cmdfunc(mtd, NAND_CMD_SEQIN, column, page);
- chip->write_buf(mtd, block_mark, 1);
- chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+ status = chip->waitfunc(mtd, chip);
+ if (status & NAND_STATUS_FAIL)
+ ret = -EIO;
- status = chip->waitfunc(mtd, chip);
- if (status & NAND_STATUS_FAIL)
- ret = -EIO;
-
- chip->select_chip(mtd, -1);
- }
- if (!ret)
- mtd->ecc_stats.badblocks++;
+ chip->select_chip(mtd, -1);
return ret;
}
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 1cbacff..a9119f7 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -324,13 +324,58 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
}
/**
- * nand_default_block_markbad - [DEFAULT] mark a block bad
+ * nand_default_block_markbad - [DEFAULT] mark a block bad via bad block marker
* @mtd: MTD device structure
* @ofs: offset from device start
*
* This is the default implementation, which can be overridden by a hardware
- * specific driver. We try operations in the following order, according to our
- * bbt_options (NAND_BBT_NO_OOB_BBM and NAND_BBT_USE_FLASH):
+ * specific driver. It provides the details for writing a bad block marker to a
+ * block.
+ */
+static int nand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)
+{
+ struct nand_chip *chip = mtd->priv;
+ struct mtd_oob_ops ops;
+ uint8_t buf[2] = { 0, 0 };
+ int ret = 0, res, i = 0;
+
+ ops.datbuf = NULL;
+ ops.oobbuf = buf;
+ ops.ooboffs = chip->badblockpos;
+ if (chip->options & NAND_BUSWIDTH_16) {
+ ops.ooboffs &= ~0x01;
+ ops.len = ops.ooblen = 2;
+ } else {
+ ops.len = ops.ooblen = 1;
+ }
+ ops.mode = MTD_OPS_PLACE_OOB;
+
+ /* Write to first/last page(s) if necessary */
+ if (chip->bbt_options & NAND_BBT_SCANLASTPAGE)
+ ofs += mtd->erasesize - mtd->writesize;
+ do {
+ res = nand_do_write_oob(mtd, ofs, &ops);
+ if (!ret)
+ ret = res;
+
+ i++;
+ ofs += mtd->writesize;
+ } while ((chip->bbt_options & NAND_BBT_SCAN2NDPAGE) && i < 2);
+
+ return ret;
+}
+
+/**
+ * nand_block_markbad_lowlevel - mark a block bad
+ * @mtd: MTD device structure
+ * @ofs: offset from device start
+ *
+ * This function performs the generic NAND bad block marking steps (i.e., bad
+ * block table(s) and/or marker(s)). We only allow the hardware driver to
+ * specify how to write bad block markers to OOB (chip->block_markbad).
+ *
+ * We try operations in the following order, according to our bbt_options
+ * (NAND_BBT_NO_OOB_BBM and NAND_BBT_USE_FLASH):
* (1) erase the affected block, to allow OOB marker to be written cleanly
* (2) update in-memory BBT
* (3) write bad block marker to OOB area of affected block
@@ -338,11 +383,10 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
* Note that we retain the first error encountered in (3) or (4), finish the
* procedures, and dump the error in the end.
*/
-static int nand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)
+static int nand_block_markbad_lowlevel(struct mtd_info *mtd, loff_t ofs)
{
struct nand_chip *chip = mtd->priv;
- uint8_t buf[2] = { 0, 0 };
- int block, res, ret = 0, i = 0;
+ int block, res, ret = 0;
int write_oob = !(chip->bbt_options & NAND_BBT_NO_OOB_BBM);
if (write_oob) {
@@ -364,34 +408,8 @@ static int nand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)
/* Write bad block marker to OOB */
if (write_oob) {
- struct mtd_oob_ops ops;
- loff_t wr_ofs = ofs;
-
nand_get_device(mtd, FL_WRITING);
-
- ops.datbuf = NULL;
- ops.oobbuf = buf;
- ops.ooboffs = chip->badblockpos;
- if (chip->options & NAND_BUSWIDTH_16) {
- ops.ooboffs &= ~0x01;
- ops.len = ops.ooblen = 2;
- } else {
- ops.len = ops.ooblen = 1;
- }
- ops.mode = MTD_OPS_PLACE_OOB;
-
- /* 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);
-
+ ret = chip->block_markbad(mtd, ofs);
nand_release_device(mtd);
}
@@ -2683,7 +2701,6 @@ static int nand_block_isbad(struct mtd_info *mtd, loff_t offs)
*/
static int nand_block_markbad(struct mtd_info *mtd, loff_t ofs)
{
- struct nand_chip *chip = mtd->priv;
int ret;
ret = nand_block_isbad(mtd, ofs);
@@ -2694,7 +2711,7 @@ static int nand_block_markbad(struct mtd_info *mtd, loff_t ofs)
return ret;
}
- return chip->block_markbad(mtd, ofs);
+ return nand_block_markbad_lowlevel(mtd, ofs);
}
/**
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 3f18776..bac481a 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -1392,4 +1392,3 @@ int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)
EXPORT_SYMBOL(nand_scan_bbt);
EXPORT_SYMBOL(nand_default_bbt);
-EXPORT_SYMBOL_GPL(nand_update_bbt);
diff --git a/drivers/mtd/nand/sm_common.c b/drivers/mtd/nand/sm_common.c
index e8181ed..e06b5e5 100644
--- a/drivers/mtd/nand/sm_common.c
+++ b/drivers/mtd/nand/sm_common.c
@@ -42,7 +42,7 @@ static int sm_block_markbad(struct mtd_info *mtd, loff_t ofs)
{
struct mtd_oob_ops ops;
struct sm_oob oob;
- int ret, error = 0;
+ int ret;
memset(&oob, -1, SM_OOB_SIZE);
oob.block_status = 0x0F;
@@ -61,11 +61,10 @@ static int sm_block_markbad(struct mtd_info *mtd, loff_t ofs)
printk(KERN_NOTICE
"sm_common: can't mark sector at %i as bad\n",
(int)ofs);
- error = -EIO;
- } else
- mtd->ecc_stats.badblocks++;
+ return -EIO;
+ }
- return error;
+ return 0;
}
static struct nand_flash_dev nand_smartmedia_flash_ids[] = {
--
1.8.3.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/4] mtd: nand: hide in-memory BBT implementation details
2013-07-24 6:27 [PATCH 0/4] mtd: nand: cleanups to BBT Brian Norris
2013-07-24 6:27 ` [PATCH 1/4] mtd: nand: add accessors, macros for in-memory BBT Brian Norris
2013-07-24 6:27 ` [PATCH 2/4] mtd: nand: refactor chip->block_markbad interface Brian Norris
@ 2013-07-24 6:27 ` Brian Norris
2013-07-24 6:27 ` [PATCH 4/4] mtd: nand: remove NAND_BBT_SCANEMPTY Brian Norris
2013-07-31 0:52 ` [PATCH v2 0/6] mtd: nand: cleanups to BBT Brian Norris
4 siblings, 0 replies; 16+ messages in thread
From: Brian Norris @ 2013-07-24 6:27 UTC (permalink / raw)
To: linux-mtd; +Cc: Brian Norris, David Woodhouse, Artem Bityutskiy
nand_base.c shouldn't have to know the implementation details of
nand_bbt's in-memory BBT. Specifically, nand_base shouldn't perform the
bit masking and shifting to isolate a BBT entry.
Instead, just move some of the BBT code into a new nand_markbad_bbt()
interface. This interface allows external users (i.e., nand_base) to
mark a single block as bad in the BBT. Then nand_bbt will take care of
modifying the in-memory BBT and updating the flash-based BBT (if
applicable).
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
drivers/mtd/nand/nand_base.c | 32 +++++++++++---------------------
drivers/mtd/nand/nand_bbt.c | 28 ++++++++++++++++++++++++++--
include/linux/mtd/nand.h | 2 +-
3 files changed, 38 insertions(+), 24 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index a9119f7..da3ba77 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -374,22 +374,20 @@ static int nand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)
* block table(s) and/or marker(s)). We only allow the hardware driver to
* specify how to write bad block markers to OOB (chip->block_markbad).
*
- * We try operations in the following order, according to our bbt_options
- * (NAND_BBT_NO_OOB_BBM and NAND_BBT_USE_FLASH):
+ * We try operations in the following order:
* (1) erase the affected block, to allow OOB marker to be written cleanly
- * (2) update in-memory BBT
- * (3) write bad block marker to OOB area of affected block
- * (4) update flash-based BBT
- * Note that we retain the first error encountered in (3) or (4), finish the
+ * (2) write bad block marker to OOB area of affected block (unless flag
+ * NAND_BBT_NO_OOB_BBM is present)
+ * (3) update the BBT
+ * Note that we retain the first error encountered in (2) or (3), finish the
* procedures, and dump the error in the end.
*/
static int nand_block_markbad_lowlevel(struct mtd_info *mtd, loff_t ofs)
{
struct nand_chip *chip = mtd->priv;
- int block, res, ret = 0;
- int write_oob = !(chip->bbt_options & NAND_BBT_NO_OOB_BBM);
+ int res, ret = 0;
- if (write_oob) {
+ if (!(chip->bbt_options & NAND_BBT_NO_OOB_BBM)) {
struct erase_info einfo;
/* Attempt erase before marking OOB */
@@ -398,24 +396,16 @@ static int nand_block_markbad_lowlevel(struct mtd_info *mtd, loff_t ofs)
einfo.addr = ofs;
einfo.len = 1 << chip->phys_erase_shift;
nand_erase_nand(mtd, &einfo, 0);
- }
-
- /* Get block number */
- block = (int)(ofs >> chip->bbt_erase_shift);
- /* Mark block bad in memory-based BBT */
- if (chip->bbt)
- chip->bbt[block >> 2] |= 0x01 << ((block & 0x03) << 1);
- /* Write bad block marker to OOB */
- if (write_oob) {
+ /* Write bad block marker to OOB */
nand_get_device(mtd, FL_WRITING);
ret = chip->block_markbad(mtd, ofs);
nand_release_device(mtd);
}
- /* Update flash-based bad block table */
- if (chip->bbt_options & NAND_BBT_USE_FLASH) {
- res = nand_update_bbt(mtd, ofs);
+ /* Mark block bad in BBT */
+ if (chip->bbt) {
+ res = nand_markbad_bbt(mtd, ofs);
if (!ret)
ret = res;
}
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index bac481a..ae3fb58 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -79,6 +79,8 @@
#define BBT_ENTRY_MASK 0x03
#define BBT_ENTRY_SHIFT 2
+static int nand_update_bbt(struct mtd_info *mtd, loff_t offs);
+
static inline uint8_t bbt_get_entry(struct nand_chip *chip, int block)
{
uint8_t entry = chip->bbt[block >> BBT_ENTRY_SHIFT];
@@ -1194,13 +1196,13 @@ int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
}
/**
- * nand_update_bbt - [NAND Interface] update bad block table(s)
+ * nand_update_bbt - update bad block table(s)
* @mtd: MTD device structure
* @offs: the offset of the newly marked block
*
* The function updates the bad block table(s).
*/
-int nand_update_bbt(struct mtd_info *mtd, loff_t offs)
+static int nand_update_bbt(struct mtd_info *mtd, loff_t offs)
{
struct nand_chip *this = mtd->priv;
int len, res = 0;
@@ -1390,5 +1392,27 @@ int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)
return 1;
}
+/**
+ * nand_markbad_bbt - [NAND Interface] Mark a block bad in the BBT
+ * @mtd: MTD device structure
+ * @offs: offset of the bad block
+ */
+int nand_markbad_bbt(struct mtd_info *mtd, loff_t offs)
+{
+ struct nand_chip *this = mtd->priv;
+ int block, ret = 0;
+
+ block = (int)(offs >> this->bbt_erase_shift);
+
+ /* Mark bad block in memory */
+ bbt_mark_entry(this, block, BBT_BLOCK_WORN);
+
+ /* Update flash-based bad block table */
+ if (this->bbt_options & NAND_BBT_USE_FLASH)
+ ret = nand_update_bbt(mtd, offs);
+
+ return ret;
+}
+
EXPORT_SYMBOL(nand_scan_bbt);
EXPORT_SYMBOL(nand_default_bbt);
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 0745a42..9f7b248 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -628,8 +628,8 @@ extern struct nand_flash_dev nand_flash_ids[];
extern struct nand_manufacturers nand_manuf_ids[];
extern int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd);
-extern int nand_update_bbt(struct mtd_info *mtd, loff_t offs);
extern int nand_default_bbt(struct mtd_info *mtd);
+extern int nand_markbad_bbt(struct mtd_info *mtd, loff_t offs);
extern int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt);
extern int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
int allowbbt);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/4] mtd: nand: remove NAND_BBT_SCANEMPTY
2013-07-24 6:27 [PATCH 0/4] mtd: nand: cleanups to BBT Brian Norris
` (2 preceding siblings ...)
2013-07-24 6:27 ` [PATCH 3/4] mtd: nand: hide in-memory BBT implementation details Brian Norris
@ 2013-07-24 6:27 ` Brian Norris
2013-07-31 0:52 ` [PATCH v2 0/6] mtd: nand: cleanups to BBT Brian Norris
4 siblings, 0 replies; 16+ messages in thread
From: Brian Norris @ 2013-07-24 6:27 UTC (permalink / raw)
To: linux-mtd; +Cc: Ivan Djelic, Brian Norris, David Woodhouse, Artem Bityutskiy
NAND_BBT_SCANEMPTY is a strange, badly-supported option with omap as its
single remaining user.
NAND_BBT_SCANEMPTY was likely used by accident in omap2[1]. And anyway,
omap2 doesn't scan the chip for bad blocks (courtesy of
NAND_SKIP_BBTSCAN), and so its use of this option is irrelevant.
This patch drops the NAND_BBT_SCANEMPTY option.
[1] http://lists.infradead.org/pipermail/linux-mtd/2012-July/042902.html
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: Ivan Djelic <ivan.djelic@parrot.com>
---
Documentation/DocBook/mtdnand.tmpl | 2 --
drivers/mtd/nand/nand_bbt.c | 33 +++++----------------------------
drivers/mtd/nand/omap2.c | 2 +-
drivers/mtd/onenand/onenand_bbt.c | 1 -
include/linux/mtd/bbm.h | 2 --
5 files changed, 6 insertions(+), 34 deletions(-)
diff --git a/Documentation/DocBook/mtdnand.tmpl b/Documentation/DocBook/mtdnand.tmpl
index fe122d6..a248f42 100644
--- a/Documentation/DocBook/mtdnand.tmpl
+++ b/Documentation/DocBook/mtdnand.tmpl
@@ -1224,8 +1224,6 @@ in this page</entry>
#define NAND_BBT_CREATE 0x00000200
/* Search good / bad pattern through all pages of a block */
#define NAND_BBT_SCANALLPAGES 0x00000400
-/* Scan block empty during good / bad block scan */
-#define NAND_BBT_SCANEMPTY 0x00000800
/* 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 ae3fb58..bc06196 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -110,33 +110,17 @@ static int check_pattern_no_oob(uint8_t *buf, struct nand_bbt_descr *td)
* @td: search pattern descriptor
*
* Check for a pattern at the given place. Used to search bad block tables and
- * good / bad block identifiers. If the SCAN_EMPTY option is set then check, if
- * all bytes except the pattern area contain 0xff.
+ * good / bad block identifiers.
*/
static int check_pattern(uint8_t *buf, int len, int paglen, struct nand_bbt_descr *td)
{
- int end = 0;
- uint8_t *p = buf;
-
if (td->options & NAND_BBT_NO_OOB)
return check_pattern_no_oob(buf, td);
- end = paglen + td->offs;
- if (td->options & NAND_BBT_SCANEMPTY)
- if (memchr_inv(p, 0xff, end))
- return -1;
- p += end;
-
/* Compare the pattern */
- if (memcmp(p, td->pattern, td->len))
+ if (memcmp(buf + paglen + td->offs, td->pattern, td->len))
return -1;
- if (td->options & NAND_BBT_SCANEMPTY) {
- p += td->len;
- end += td->len;
- if (memchr_inv(p, 0xff, len - end))
- return -1;
- }
return 0;
}
@@ -507,15 +491,9 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
else
numpages = 1;
- if (!(bd->options & NAND_BBT_SCANEMPTY)) {
- /* We need only read few bytes from the OOB area */
- scanlen = 0;
- readlen = bd->len;
- } else {
- /* Full page content should be read */
- scanlen = mtd->writesize + mtd->oobsize;
- readlen = numpages * mtd->writesize;
- }
+ /* 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;
@@ -882,7 +860,6 @@ static inline int nand_memory_bbt(struct mtd_info *mtd, struct nand_bbt_descr *b
{
struct nand_chip *this = mtd->priv;
- bd->options &= ~NAND_BBT_SCANEMPTY;
return create_bbt(mtd, this->buffers->databuf, bd, -1);
}
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index c4c7e0d..d6b9cc1 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -159,7 +159,7 @@ static struct nand_ecclayout omap_oobinfo;
*/
static uint8_t scan_ff_pattern[] = { 0xff };
static struct nand_bbt_descr bb_descrip_flashbased = {
- .options = NAND_BBT_SCANEMPTY | NAND_BBT_SCANALLPAGES,
+ .options = NAND_BBT_SCANALLPAGES,
.offs = 0,
.len = 1,
.pattern = scan_ff_pattern,
diff --git a/drivers/mtd/onenand/onenand_bbt.c b/drivers/mtd/onenand/onenand_bbt.c
index 66fe3b7..08d0085 100644
--- a/drivers/mtd/onenand/onenand_bbt.c
+++ b/drivers/mtd/onenand/onenand_bbt.c
@@ -133,7 +133,6 @@ static inline int onenand_memory_bbt (struct mtd_info *mtd, struct nand_bbt_desc
{
struct onenand_chip *this = mtd->priv;
- bd->options &= ~NAND_BBT_SCANEMPTY;
return create_bbt(mtd, this->page_buf, bd, -1);
}
diff --git a/include/linux/mtd/bbm.h b/include/linux/mtd/bbm.h
index 211ff67..95fc482 100644
--- a/include/linux/mtd/bbm.h
+++ b/include/linux/mtd/bbm.h
@@ -93,8 +93,6 @@ struct nand_bbt_descr {
#define NAND_BBT_CREATE_EMPTY 0x00000400
/* Search good / bad pattern through all pages of a block */
#define NAND_BBT_SCANALLPAGES 0x00000800
-/* Scan block empty during good / bad block scan */
-#define NAND_BBT_SCANEMPTY 0x00001000
/* Write bbt if neccecary */
#define NAND_BBT_WRITE 0x00002000
/* Read and write back block contents when writing bbt */
--
1.8.3.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] mtd: nand: refactor chip->block_markbad interface
2013-07-24 6:27 ` [PATCH 2/4] mtd: nand: refactor chip->block_markbad interface Brian Norris
@ 2013-07-24 6:59 ` Huang Shijie
0 siblings, 0 replies; 16+ messages in thread
From: Huang Shijie @ 2013-07-24 6:59 UTC (permalink / raw)
To: Brian Norris; +Cc: David Woodhouse, linux-mtd, Artem Bityutskiy
于 2013年07月24日 14:27, Brian Norris 写道:
> The chip->block_markbad pointer should really only be responsible for
> writing a bad block marker for new bad blocks. It should not take care
> of BBT-related functionality, nor should it handle bookkeeping of bad
> block stats.
yes.
the gpmi driver should not do the BBT-related jobs.
> This patch refactors the 3 users of the block_markbad interface (plus
> the default nand_base implementation) so that the common code is kept in
> nand_block_markbad_lowlevel(). It removes some inconsistencies between
> the various implementations and should allow for more centralized
> improvements in the future.
>
> Because gpmi-nand no longer needs the nand_update_bbt() function, let's
> stop exporting it as well.
for the gpmi-nand part:
Acked-by: Huang Shijie <b32955@freescale.com>
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] mtd: nand: add accessors, macros for in-memory BBT
2013-07-24 6:27 ` [PATCH 1/4] mtd: nand: add accessors, macros for in-memory BBT Brian Norris
@ 2013-07-30 22:02 ` Ezequiel Garcia
2013-07-31 0:40 ` Brian Norris
0 siblings, 1 reply; 16+ messages in thread
From: Ezequiel Garcia @ 2013-07-30 22:02 UTC (permalink / raw)
To: Brian Norris; +Cc: David Woodhouse, linux-mtd, Artem Bityutskiy
Hi Brian,
Here's my attempt at reviewing this patchset, given the recent
discussion about needing more MTD reviewing.
I don't have enough experience with the NAND core to say anything about this,
but I noticed you're doing a bunch of related but not necessarily tied changes.
IMHO, splitting this patch further might make reviewing a lot easier, see below.
On Tue, Jul 23, 2013 at 11:27:56PM -0700, Brian Norris wrote:
> There is an abundance of magic numbers and complicated shifting/masking
> logic in the in-memory BBT code, and due to the complicated
> shifting/masking, we often store the block number multiplied by 2.
> Together, these features make the code unnecessary complex and hard to
> read.
>
> This patch adds macros to represent the 00b, 01b, 10b, and 11b
> memory-BBT magic numbers, as well as two accessor functions for reading
> and marking the memory-BBT bitfield for a given block.
>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
> drivers/mtd/nand/nand_bbt.c | 132 ++++++++++++++++++++++++--------------------
> 1 file changed, 72 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
> index 2672643..3f18776 100644
> --- a/drivers/mtd/nand/nand_bbt.c
> +++ b/drivers/mtd/nand/nand_bbt.c
> @@ -71,6 +71,28 @@
> #include <linux/export.h>
> #include <linux/string.h>
>
> +#define BBT_BLOCK_GOOD 0x00
> +#define BBT_BLOCK_WORN 0x01
> +#define BBT_BLOCK_RESERVED 0x02
> +#define BBT_BLOCK_FACTORY_BAD 0x03
> +
> +#define BBT_ENTRY_MASK 0x03
> +#define BBT_ENTRY_SHIFT 2
> +
You can have one patch to change all the magic numbers into this nice
macros.
[...]
> - for (j = 0; j < 8; j += bits, act += 2) {
> + for (j = 0; j < 8; j += bits, act++) {
> uint8_t tmp = (dat >> j) & msk;
> if (tmp == msk)
> continue;
> if (reserved_block_code && (tmp == reserved_block_code)) {
> pr_info("nand_read_bbt: reserved block at 0x%012llx\n",
> - (loff_t)((offs << 2) + (act >> 1)) << this->bbt_erase_shift);
> - this->bbt[offs + (act >> 3)] |= 0x2 << (act & 0x06);
> + (loff_t)(offs + act) <<
> + this->bbt_erase_shift);
> + bbt_mark_entry(this, offs + act,
> + BBT_BLOCK_RESERVED);
And then another patch to use the new accesors.
[...]
> int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)
> {
> struct nand_chip *this = mtd->priv;
> - int block;
> - uint8_t res;
> + int block, res;
>
> - /* Get block number * 2 */
And then a third patch to make the block * 2 -> block change.
> - block = (int)(offs >> (this->bbt_erase_shift - 1));
> - res = (this->bbt[block >> 3] >> (block & 0x06)) & 0x03;
> + block = (int)(offs >> this->bbt_erase_shift);
> + res = bbt_get_entry(this, block);
>
> pr_debug("nand_isbad_bbt(): bbt info for offs 0x%08x: "
> "(block %d) 0x%02x\n",
> - (unsigned int)offs, block >> 1, res);
> + (unsigned int)offs, block, res);
>
> - switch ((int)res) {
> - case 0x00:
> + switch (res) {
Mmm.. and then if you are really paranoid (like me) you can
make the uint8_t -> int type change in another patch.
> + case BBT_BLOCK_GOOD:
> return 0;
> - case 0x01:
> + case BBT_BLOCK_WORN:
> return 1;
> - case 0x02:
> + case BBT_BLOCK_RESERVED:
> return allowbbt ? 0 : 1;
> }
> return 1;
> --
> 1.8.3.2
>
Hope this is of any help!
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] mtd: nand: add accessors, macros for in-memory BBT
2013-07-30 22:02 ` Ezequiel Garcia
@ 2013-07-31 0:40 ` Brian Norris
0 siblings, 0 replies; 16+ messages in thread
From: Brian Norris @ 2013-07-31 0:40 UTC (permalink / raw)
To: Ezequiel Garcia; +Cc: David Woodhouse, linux-mtd, Artem Bityutskiy
Hi Ezequiel,
On 07/30/2013 03:02 PM, Ezequiel Garcia wrote:
> Hi Brian,
>
> Here's my attempt at reviewing this patchset, given the recent
> discussion about needing more MTD reviewing.
>
> I don't have enough experience with the NAND core to say anything about this,
> but I noticed you're doing a bunch of related but not necessarily tied changes.
>
> IMHO, splitting this patch further might make reviewing a lot easier, see below.
At first I disagreed, but after re-reading my work, I agree in part. The
original code was quite ugly, but that doesn't excuse me for not making
a little effort to make it easier to review.
> On Tue, Jul 23, 2013 at 11:27:56PM -0700, Brian Norris wrote:
>> diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
>> index 2672643..3f18776 100644
>> --- a/drivers/mtd/nand/nand_bbt.c
>> +++ b/drivers/mtd/nand/nand_bbt.c
>> @@ -71,6 +71,28 @@
>> #include <linux/export.h>
>> #include <linux/string.h>
>>
>> +#define BBT_BLOCK_GOOD 0x00
>> +#define BBT_BLOCK_WORN 0x01
>> +#define BBT_BLOCK_RESERVED 0x02
>> +#define BBT_BLOCK_FACTORY_BAD 0x03
>> +
>> +#define BBT_ENTRY_MASK 0x03
>> +#define BBT_ENTRY_SHIFT 2
>> +
>
> You can have one patch to change all the magic numbers into this nice
> macros.
>
> [...]
>
> And then another patch to use the new accesors.
I plan to still lump the macros + accessors together, since most of the
shifting and masking ugliness is very intertwined. It's easier (IMO) to
just reason about a hunk like this:
[combined hunk]
- this->bbt[offs + (act >> 3)] |= 0x2 << (act & 0x06);
+ bbt_mark_entry(this, (offs << 2) + (act >> 1), BBT_BLOCK_RESERVED);
Than the next two (as if they are split to 2 patches):
[hunk for patch 1]
- this->bbt[offs + (act >> 3)] |= 0x2 << (act & 0x06);
+ this->bbt[offs + ((act >> 1) >> BBT_ENTRY_SHIFT)] |=
+ BBT_BLOCK_RESERVED << ((act >> 1) & BBT_ENTRY_MASK);
[hunk for patch 2]
- this->bbt[offs + ((act >> 1) >> BBT_ENTRY_SHIFT)] |=
- BBT_BLOCK_RESERVED << ((act >> 1) & BBT_ENTRY_MASK);
+ bbt_mark_entry(this, (offs << 2) + (act >> 1), BBT_BLOCK_RESERVED);
But I can split again, if the next series is still hard to review.
> [...]
>> int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)
>> {
>> struct nand_chip *this = mtd->priv;
>> - int block;
>> - uint8_t res;
>> + int block, res;
>>
>> - /* Get block number * 2 */
>
> And then a third patch to make the block * 2 -> block change.
Yes, this one can be kept pretty separate, and it makes the intermediate
changes easier to read.
>> - block = (int)(offs >> (this->bbt_erase_shift - 1));
>> - res = (this->bbt[block >> 3] >> (block & 0x06)) & 0x03;
>> + block = (int)(offs >> this->bbt_erase_shift);
>> + res = bbt_get_entry(this, block);
>>
>> pr_debug("nand_isbad_bbt(): bbt info for offs 0x%08x: "
>> "(block %d) 0x%02x\n",
>> - (unsigned int)offs, block >> 1, res);
>> + (unsigned int)offs, block, res);
>>
>> - switch ((int)res) {
>> - case 0x00:
>> + switch (res) {
>
> Mmm.. and then if you are really paranoid (like me) you can
> make the uint8_t -> int type change in another patch.
I'll consider it, but I don't consider myself quite that paranoid ;)
>> + case BBT_BLOCK_GOOD:
>> return 0;
>> - case 0x01:
>> + case BBT_BLOCK_WORN:
>> return 1;
>> - case 0x02:
>> + case BBT_BLOCK_RESERVED:
>> return allowbbt ? 0 : 1;
>> }
>> return 1;
>
> Hope this is of any help!
Yes, thanks for the review! I'll send a split v2 series, and I hope it
will be more reviewable.
Brian
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 0/6] mtd: nand: cleanups to BBT
2013-07-24 6:27 [PATCH 0/4] mtd: nand: cleanups to BBT Brian Norris
` (3 preceding siblings ...)
2013-07-24 6:27 ` [PATCH 4/4] mtd: nand: remove NAND_BBT_SCANEMPTY Brian Norris
@ 2013-07-31 0:52 ` Brian Norris
2013-07-31 0:52 ` [PATCH v2 1/6] mtd: nand: add accessors, macros for in-memory BBT Brian Norris
` (6 more replies)
4 siblings, 7 replies; 16+ messages in thread
From: Brian Norris @ 2013-07-31 0:52 UTC (permalink / raw)
To: linux-mtd
Cc: Artem Bityutskiy, Huang Shijie, Ezequiel Garcia, Ivan Djelic,
Brian Norris, David Woodhouse
Hi all,
This is v2 of my NAND BBT cleanup. The net diff is exactly the same as v1, but
it splits up a few logical changes.
v1 -> v2:
Split up first patch into 3 patches, according to Ezequiel's comments.
Hopefully this makes it easier to read and review.
Brian Norris (6):
mtd: nand: add accessors, macros for in-memory BBT
mtd: nand: remove multiplied-by-2 block logic
mtd: nand: eliminate cast
mtd: nand: refactor chip->block_markbad interface
mtd: nand: hide in-memory BBT implementation details
mtd: nand: remove NAND_BBT_SCANEMPTY
Documentation/DocBook/mtdnand.tmpl | 2 -
drivers/mtd/nand/docg4.c | 6 -
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 44 +++-----
drivers/mtd/nand/nand_base.c | 113 ++++++++++---------
drivers/mtd/nand/nand_bbt.c | 194 +++++++++++++++++---------------
drivers/mtd/nand/omap2.c | 2 +-
drivers/mtd/nand/sm_common.c | 9 +-
drivers/mtd/onenand/onenand_bbt.c | 1 -
include/linux/mtd/bbm.h | 2 -
include/linux/mtd/nand.h | 2 +-
10 files changed, 185 insertions(+), 190 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/6] mtd: nand: add accessors, macros for in-memory BBT
2013-07-31 0:52 ` [PATCH v2 0/6] mtd: nand: cleanups to BBT Brian Norris
@ 2013-07-31 0:52 ` Brian Norris
2013-07-31 0:52 ` [PATCH v2 2/6] mtd: nand: remove multiplied-by-2 block logic Brian Norris
` (5 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Brian Norris @ 2013-07-31 0:52 UTC (permalink / raw)
To: linux-mtd
Cc: Brian Norris, David Woodhouse, Ezequiel Garcia, Artem Bityutskiy
There is an abundance of magic numbers and complicated shifting/masking
logic in the in-memory BBT code which makes the code unnecessary complex
and hard to read.
This patch adds macros to represent the 00b, 01b, 10b, and 11b
memory-BBT magic numbers, as well as two accessor functions for reading
and marking the memory-BBT bitfield for a given block.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
drivers/mtd/nand/nand_bbt.c | 70 +++++++++++++++++++++++++++++--------------
1 file changed, 48 insertions(+), 22 deletions(-)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 2672643..4c57a9b 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -71,6 +71,28 @@
#include <linux/export.h>
#include <linux/string.h>
+#define BBT_BLOCK_GOOD 0x00
+#define BBT_BLOCK_WORN 0x01
+#define BBT_BLOCK_RESERVED 0x02
+#define BBT_BLOCK_FACTORY_BAD 0x03
+
+#define BBT_ENTRY_MASK 0x03
+#define BBT_ENTRY_SHIFT 2
+
+static inline uint8_t bbt_get_entry(struct nand_chip *chip, int block)
+{
+ uint8_t entry = chip->bbt[block >> BBT_ENTRY_SHIFT];
+ entry >>= (block & BBT_ENTRY_MASK) * 2;
+ return entry & BBT_ENTRY_MASK;
+}
+
+static inline void bbt_mark_entry(struct nand_chip *chip, int block,
+ uint8_t mark)
+{
+ uint8_t msk = (mark & BBT_ENTRY_MASK) << ((block & BBT_ENTRY_MASK) * 2);
+ chip->bbt[block >> BBT_ENTRY_SHIFT] |= msk;
+}
+
static int check_pattern_no_oob(uint8_t *buf, struct nand_bbt_descr *td)
{
if (memcmp(buf, td->pattern, td->len))
@@ -216,7 +238,9 @@ static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num,
if (reserved_block_code && (tmp == reserved_block_code)) {
pr_info("nand_read_bbt: reserved block at 0x%012llx\n",
(loff_t)((offs << 2) + (act >> 1)) << this->bbt_erase_shift);
- this->bbt[offs + (act >> 3)] |= 0x2 << (act & 0x06);
+ bbt_mark_entry(this, (offs << 2) +
+ (act >> 1),
+ BBT_BLOCK_RESERVED);
mtd->ecc_stats.bbtblocks++;
continue;
}
@@ -228,9 +252,13 @@ static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num,
(loff_t)((offs << 2) + (act >> 1)) << this->bbt_erase_shift);
/* Factory marked bad or worn out? */
if (tmp == 0)
- this->bbt[offs + (act >> 3)] |= 0x3 << (act & 0x06);
+ bbt_mark_entry(this, (offs << 2) +
+ (act >> 1),
+ BBT_BLOCK_FACTORY_BAD);
else
- this->bbt[offs + (act >> 3)] |= 0x1 << (act & 0x06);
+ bbt_mark_entry(this, (offs << 2) +
+ (act >> 1),
+ BBT_BLOCK_WORN);
mtd->ecc_stats.badblocks++;
}
}
@@ -526,7 +554,7 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
return ret;
if (ret) {
- this->bbt[i >> 3] |= 0x03 << (i & 0x6);
+ bbt_mark_entry(this, i >> 1, BBT_BLOCK_FACTORY_BAD);
pr_warn("Bad eraseblock %d at 0x%012llx\n",
i >> 1, (unsigned long long)from);
mtd->ecc_stats.badblocks++;
@@ -713,10 +741,9 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
for (i = 0; i < td->maxblocks; i++) {
int block = startblock + dir * i;
/* Check, if the block is bad */
- switch ((this->bbt[block >> 2] >>
- (2 * (block & 0x03))) & 0x03) {
- case 0x01:
- case 0x03:
+ switch (bbt_get_entry(this, block)) {
+ case BBT_BLOCK_WORN:
+ case BBT_BLOCK_FACTORY_BAD:
continue;
}
page = block <<
@@ -816,7 +843,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
/* Walk through the memory table */
for (i = 0; i < numblocks;) {
uint8_t dat;
- dat = this->bbt[bbtoffs + (i >> 2)];
+ dat = bbt_get_entry(this, (bbtoffs << 2) + i);
for (j = 0; j < 4; j++, i++) {
int sftcnt = (i << (3 - sft)) & sftmsk;
/* Do not store the reserved bbt blocks! */
@@ -1009,7 +1036,7 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)
{
struct nand_chip *this = mtd->priv;
int i, j, chips, block, nrblocks, update;
- uint8_t oldval, newval;
+ uint8_t oldval;
/* Do we have a bbt per chip? */
if (td->options & NAND_BBT_PERCHIP) {
@@ -1027,10 +1054,10 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)
continue;
block = td->pages[i] >> (this->bbt_erase_shift - this->page_shift);
block <<= 1;
- oldval = this->bbt[(block >> 3)];
- newval = oldval | (0x2 << (block & 0x06));
- this->bbt[(block >> 3)] = newval;
- if ((oldval != newval) && td->reserved_block_code)
+ oldval = bbt_get_entry(this, block >> 1);
+ bbt_mark_entry(this, block >> 1, BBT_BLOCK_RESERVED);
+ if ((oldval != BBT_BLOCK_RESERVED) &&
+ td->reserved_block_code)
nand_update_bbt(mtd, (loff_t)block << (this->bbt_erase_shift - 1));
continue;
}
@@ -1041,10 +1068,9 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)
block = i * nrblocks;
block <<= 1;
for (j = 0; j < td->maxblocks; j++) {
- oldval = this->bbt[(block >> 3)];
- newval = oldval | (0x2 << (block & 0x06));
- this->bbt[(block >> 3)] = newval;
- if (oldval != newval)
+ oldval = bbt_get_entry(this, block >> 1);
+ bbt_mark_entry(this, block >> 1, BBT_BLOCK_RESERVED);
+ if (oldval != BBT_BLOCK_RESERVED)
update = 1;
block += 2;
}
@@ -1361,18 +1387,18 @@ int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)
/* Get block number * 2 */
block = (int)(offs >> (this->bbt_erase_shift - 1));
- res = (this->bbt[block >> 3] >> (block & 0x06)) & 0x03;
+ res = bbt_get_entry(this, block >> 1);
pr_debug("nand_isbad_bbt(): bbt info for offs 0x%08x: "
"(block %d) 0x%02x\n",
(unsigned int)offs, block >> 1, res);
switch ((int)res) {
- case 0x00:
+ case BBT_BLOCK_GOOD:
return 0;
- case 0x01:
+ case BBT_BLOCK_WORN:
return 1;
- case 0x02:
+ case BBT_BLOCK_RESERVED:
return allowbbt ? 0 : 1;
}
return 1;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/6] mtd: nand: remove multiplied-by-2 block logic
2013-07-31 0:52 ` [PATCH v2 0/6] mtd: nand: cleanups to BBT Brian Norris
2013-07-31 0:52 ` [PATCH v2 1/6] mtd: nand: add accessors, macros for in-memory BBT Brian Norris
@ 2013-07-31 0:52 ` Brian Norris
2013-07-31 0:52 ` [PATCH v2 3/6] mtd: nand: eliminate cast Brian Norris
` (4 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Brian Norris @ 2013-07-31 0:52 UTC (permalink / raw)
To: linux-mtd
Cc: Brian Norris, David Woodhouse, Ezequiel Garcia, Artem Bityutskiy
The previous patch makes the following comment obsolete:
/*
* Note that numblocks is 2 * (real numblocks) here, see i+=2
* below as it makes shifting and masking less painful
*/
I don't think it ever could have been "less painful" to have to shift an
extra bit (or 2, or 3) at various points in nand_bbt.c (and even
outside, since we leak our in-memory format). But now it is certainly
more painful, since we have nice macros and functions to retrieve the
relevant portions of the BBT.
This patch removes any points where the block number is
doubled/halved/otherwise-shifted, instead representing the block number
in its most natural form: as the actual block number.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
drivers/mtd/nand/nand_bbt.c | 83 ++++++++++++++++++-------------------------
1 file changed, 35 insertions(+), 48 deletions(-)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 4c57a9b..3ff9d36 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -181,7 +181,7 @@ static u32 add_marker_len(struct nand_bbt_descr *td)
* @page: the starting page
* @num: the number of bbt descriptors to read
* @td: the bbt describtion table
- * @offs: offset in the memory table
+ * @offs: block number offset in the table
*
* Read the bad block table starting from page.
*/
@@ -231,15 +231,15 @@ static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num,
/* Analyse data */
for (i = 0; i < len; i++) {
uint8_t dat = buf[i];
- for (j = 0; j < 8; j += bits, act += 2) {
+ for (j = 0; j < 8; j += bits, act++) {
uint8_t tmp = (dat >> j) & msk;
if (tmp == msk)
continue;
if (reserved_block_code && (tmp == reserved_block_code)) {
pr_info("nand_read_bbt: reserved block at 0x%012llx\n",
- (loff_t)((offs << 2) + (act >> 1)) << this->bbt_erase_shift);
- bbt_mark_entry(this, (offs << 2) +
- (act >> 1),
+ (loff_t)(offs + act) <<
+ this->bbt_erase_shift);
+ bbt_mark_entry(this, offs + act,
BBT_BLOCK_RESERVED);
mtd->ecc_stats.bbtblocks++;
continue;
@@ -249,15 +249,14 @@ static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num,
* move this message to pr_debug.
*/
pr_info("nand_read_bbt: bad block at 0x%012llx\n",
- (loff_t)((offs << 2) + (act >> 1)) << this->bbt_erase_shift);
+ (loff_t)(offs + act) <<
+ this->bbt_erase_shift);
/* Factory marked bad or worn out? */
if (tmp == 0)
- bbt_mark_entry(this, (offs << 2) +
- (act >> 1),
+ bbt_mark_entry(this, offs + act,
BBT_BLOCK_FACTORY_BAD);
else
- bbt_mark_entry(this, (offs << 2) +
- (act >> 1),
+ bbt_mark_entry(this, offs + act,
BBT_BLOCK_WORN);
mtd->ecc_stats.badblocks++;
}
@@ -293,7 +292,7 @@ static int read_abs_bbt(struct mtd_info *mtd, uint8_t *buf, struct nand_bbt_desc
td, offs);
if (res)
return res;
- offs += this->chipsize >> (this->bbt_erase_shift + 2);
+ offs += this->chipsize >> this->bbt_erase_shift;
}
} else {
res = read_bbt(mtd, buf, td->pages[0],
@@ -517,11 +516,7 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
}
if (chip == -1) {
- /*
- * Note that numblocks is 2 * (real numblocks) here, see i+=2
- * below as it makes shifting and masking less painful
- */
- numblocks = mtd->size >> (this->bbt_erase_shift - 1);
+ numblocks = mtd->size >> this->bbt_erase_shift;
startblock = 0;
from = 0;
} else {
@@ -530,16 +525,16 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
chip + 1, this->numchips);
return -EINVAL;
}
- numblocks = this->chipsize >> (this->bbt_erase_shift - 1);
+ numblocks = this->chipsize >> this->bbt_erase_shift;
startblock = chip * numblocks;
numblocks += startblock;
- from = (loff_t)startblock << (this->bbt_erase_shift - 1);
+ from = (loff_t)startblock << this->bbt_erase_shift;
}
if (this->bbt_options & NAND_BBT_SCANLASTPAGE)
from += mtd->erasesize - (mtd->writesize * numpages);
- for (i = startblock; i < numblocks;) {
+ for (i = startblock; i < numblocks; i++) {
int ret;
BUG_ON(bd->options & NAND_BBT_NO_OOB);
@@ -554,13 +549,12 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
return ret;
if (ret) {
- bbt_mark_entry(this, i >> 1, BBT_BLOCK_FACTORY_BAD);
+ bbt_mark_entry(this, i, BBT_BLOCK_FACTORY_BAD);
pr_warn("Bad eraseblock %d at 0x%012llx\n",
- i >> 1, (unsigned long long)from);
+ i, (unsigned long long)from);
mtd->ecc_stats.badblocks++;
}
- i += 2;
from += (1 << this->bbt_erase_shift);
}
return 0;
@@ -683,9 +677,9 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
{
struct nand_chip *this = mtd->priv;
struct erase_info einfo;
- int i, j, res, chip = 0;
+ int i, res, chip = 0;
int bits, startblock, dir, page, offs, numblocks, sft, sftmsk;
- int nrchips, bbtoffs, pageoffs, ooboffs;
+ int nrchips, pageoffs, ooboffs;
uint8_t msk[4];
uint8_t rcode = td->reserved_block_code;
size_t retlen, len = 0;
@@ -775,8 +769,6 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
default: return -EINVAL;
}
- bbtoffs = chip * (numblocks >> 2);
-
to = ((loff_t)page) << this->page_shift;
/* Must we save the block contents? */
@@ -841,16 +833,12 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
buf[ooboffs + td->veroffs] = td->version[chip];
/* Walk through the memory table */
- for (i = 0; i < numblocks;) {
+ for (i = 0; i < numblocks; i++) {
uint8_t dat;
- dat = bbt_get_entry(this, (bbtoffs << 2) + i);
- for (j = 0; j < 4; j++, i++) {
- int sftcnt = (i << (3 - sft)) & sftmsk;
- /* Do not store the reserved bbt blocks! */
- buf[offs + (i >> sft)] &=
- ~(msk[dat & 0x03] << sftcnt);
- dat >>= 2;
- }
+ int sftcnt = (i << (3 - sft)) & sftmsk;
+ dat = bbt_get_entry(this, chip * numblocks + i);
+ /* Do not store the reserved bbt blocks! */
+ buf[offs + (i >> sft)] &= ~(msk[dat] << sftcnt);
}
memset(&einfo, 0, sizeof(einfo));
@@ -1053,12 +1041,12 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)
if (td->pages[i] == -1)
continue;
block = td->pages[i] >> (this->bbt_erase_shift - this->page_shift);
- block <<= 1;
- oldval = bbt_get_entry(this, block >> 1);
- bbt_mark_entry(this, block >> 1, BBT_BLOCK_RESERVED);
+ oldval = bbt_get_entry(this, block);
+ bbt_mark_entry(this, block, BBT_BLOCK_RESERVED);
if ((oldval != BBT_BLOCK_RESERVED) &&
td->reserved_block_code)
- nand_update_bbt(mtd, (loff_t)block << (this->bbt_erase_shift - 1));
+ nand_update_bbt(mtd, (loff_t)block <<
+ this->bbt_erase_shift);
continue;
}
update = 0;
@@ -1066,13 +1054,12 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)
block = ((i + 1) * nrblocks) - td->maxblocks;
else
block = i * nrblocks;
- block <<= 1;
for (j = 0; j < td->maxblocks; j++) {
- oldval = bbt_get_entry(this, block >> 1);
- bbt_mark_entry(this, block >> 1, BBT_BLOCK_RESERVED);
+ oldval = bbt_get_entry(this, block);
+ bbt_mark_entry(this, block, BBT_BLOCK_RESERVED);
if (oldval != BBT_BLOCK_RESERVED)
update = 1;
- block += 2;
+ block++;
}
/*
* If we want reserved blocks to be recorded to flash, and some
@@ -1080,7 +1067,8 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)
* bbts. This should only happen once.
*/
if (update && td->reserved_block_code)
- nand_update_bbt(mtd, (loff_t)(block - 2) << (this->bbt_erase_shift - 1));
+ nand_update_bbt(mtd, (loff_t)(block - 1) <<
+ this->bbt_erase_shift);
}
}
@@ -1385,13 +1373,12 @@ int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)
int block;
uint8_t res;
- /* Get block number * 2 */
- block = (int)(offs >> (this->bbt_erase_shift - 1));
- res = bbt_get_entry(this, block >> 1);
+ block = (int)(offs >> this->bbt_erase_shift);
+ res = bbt_get_entry(this, block);
pr_debug("nand_isbad_bbt(): bbt info for offs 0x%08x: "
"(block %d) 0x%02x\n",
- (unsigned int)offs, block >> 1, res);
+ (unsigned int)offs, block, res);
switch ((int)res) {
case BBT_BLOCK_GOOD:
--
1.7.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 3/6] mtd: nand: eliminate cast
2013-07-31 0:52 ` [PATCH v2 0/6] mtd: nand: cleanups to BBT Brian Norris
2013-07-31 0:52 ` [PATCH v2 1/6] mtd: nand: add accessors, macros for in-memory BBT Brian Norris
2013-07-31 0:52 ` [PATCH v2 2/6] mtd: nand: remove multiplied-by-2 block logic Brian Norris
@ 2013-07-31 0:52 ` Brian Norris
2013-07-31 0:52 ` [PATCH v2 4/6] mtd: nand: refactor chip->block_markbad interface Brian Norris
` (3 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Brian Norris @ 2013-07-31 0:52 UTC (permalink / raw)
To: linux-mtd
Cc: Brian Norris, David Woodhouse, Ezequiel Garcia, Artem Bityutskiy
Just make 'res' an int.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
drivers/mtd/nand/nand_bbt.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 3ff9d36..3f18776 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -1370,8 +1370,7 @@ int nand_default_bbt(struct mtd_info *mtd)
int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)
{
struct nand_chip *this = mtd->priv;
- int block;
- uint8_t res;
+ int block, res;
block = (int)(offs >> this->bbt_erase_shift);
res = bbt_get_entry(this, block);
@@ -1380,7 +1379,7 @@ int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)
"(block %d) 0x%02x\n",
(unsigned int)offs, block, res);
- switch ((int)res) {
+ switch (res) {
case BBT_BLOCK_GOOD:
return 0;
case BBT_BLOCK_WORN:
--
1.7.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 4/6] mtd: nand: refactor chip->block_markbad interface
2013-07-31 0:52 ` [PATCH v2 0/6] mtd: nand: cleanups to BBT Brian Norris
` (2 preceding siblings ...)
2013-07-31 0:52 ` [PATCH v2 3/6] mtd: nand: eliminate cast Brian Norris
@ 2013-07-31 0:52 ` Brian Norris
2013-07-31 0:52 ` [PATCH v2 5/6] mtd: nand: hide in-memory BBT implementation details Brian Norris
` (2 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Brian Norris @ 2013-07-31 0:52 UTC (permalink / raw)
To: linux-mtd; +Cc: Huang Shijie, Brian Norris, David Woodhouse, Artem Bityutskiy
The chip->block_markbad pointer should really only be responsible for
writing a bad block marker for new bad blocks. It should not take care
of BBT-related functionality, nor should it handle bookkeeping of bad
block stats.
This patch refactors the 3 users of the block_markbad interface (plus
the default nand_base implementation) so that the common code is kept in
nand_block_markbad_lowlevel(). It removes some inconsistencies between
the various implementations and should allow for more centralized
improvements in the future.
Because gpmi-nand no longer needs the nand_update_bbt() function, let's
stop exporting it as well.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Acked-by: Huang Shijie <b32955@freescale.com> (for gpmi-nand parts)
---
drivers/mtd/nand/docg4.c | 6 ---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 44 ++++++----------
drivers/mtd/nand/nand_base.c | 87 +++++++++++++++++++-------------
drivers/mtd/nand/nand_bbt.c | 1 -
drivers/mtd/nand/sm_common.c | 9 ++--
5 files changed, 72 insertions(+), 75 deletions(-)
diff --git a/drivers/mtd/nand/docg4.c b/drivers/mtd/nand/docg4.c
index dc86d4a..548db23 100644
--- a/drivers/mtd/nand/docg4.c
+++ b/drivers/mtd/nand/docg4.c
@@ -1093,7 +1093,6 @@ static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)
struct nand_chip *nand = mtd->priv;
struct docg4_priv *doc = nand->priv;
struct nand_bbt_descr *bbtd = nand->badblock_pattern;
- int block = (int)(ofs >> nand->bbt_erase_shift);
int page = (int)(ofs >> nand->page_shift);
uint32_t g4_addr = mtd_to_docg4_address(page, 0);
@@ -1108,9 +1107,6 @@ static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)
if (buf == NULL)
return -ENOMEM;
- /* update bbt in memory */
- nand->bbt[block / 4] |= 0x01 << ((block & 0x03) * 2);
-
/* write bit-wise negation of pattern to oob buffer */
memset(nand->oob_poi, 0xff, mtd->oobsize);
for (i = 0; i < bbtd->len; i++)
@@ -1120,8 +1116,6 @@ static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)
write_page_prologue(mtd, g4_addr);
docg4_write_page(mtd, nand, buf, 1);
ret = pageprog(mtd);
- if (!ret)
- mtd->ecc_stats.badblocks++;
kfree(buf);
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index b741a6c..5947575 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1157,43 +1157,31 @@ static int gpmi_block_markbad(struct mtd_info *mtd, loff_t ofs)
{
struct nand_chip *chip = mtd->priv;
struct gpmi_nand_data *this = chip->priv;
- int block, ret = 0;
+ int ret = 0;
uint8_t *block_mark;
int column, page, status, chipnr;
- /* Get block number */
- block = (int)(ofs >> chip->bbt_erase_shift);
- if (chip->bbt)
- chip->bbt[block >> 2] |= 0x01 << ((block & 0x03) << 1);
+ chipnr = (int)(ofs >> chip->chip_shift);
+ chip->select_chip(mtd, chipnr);
- /* Do we have a flash based bad block table ? */
- if (chip->bbt_options & NAND_BBT_USE_FLASH)
- ret = nand_update_bbt(mtd, ofs);
- else {
- chipnr = (int)(ofs >> chip->chip_shift);
- chip->select_chip(mtd, chipnr);
+ column = this->swap_block_mark ? mtd->writesize : 0;
- column = this->swap_block_mark ? mtd->writesize : 0;
+ /* Write the block mark. */
+ block_mark = this->data_buffer_dma;
+ block_mark[0] = 0; /* bad block marker */
- /* Write the block mark. */
- block_mark = this->data_buffer_dma;
- block_mark[0] = 0; /* bad block marker */
+ /* Shift to get page */
+ page = (int)(ofs >> chip->page_shift);
- /* Shift to get page */
- page = (int)(ofs >> chip->page_shift);
+ chip->cmdfunc(mtd, NAND_CMD_SEQIN, column, page);
+ chip->write_buf(mtd, block_mark, 1);
+ chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
- chip->cmdfunc(mtd, NAND_CMD_SEQIN, column, page);
- chip->write_buf(mtd, block_mark, 1);
- chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+ status = chip->waitfunc(mtd, chip);
+ if (status & NAND_STATUS_FAIL)
+ ret = -EIO;
- status = chip->waitfunc(mtd, chip);
- if (status & NAND_STATUS_FAIL)
- ret = -EIO;
-
- chip->select_chip(mtd, -1);
- }
- if (!ret)
- mtd->ecc_stats.badblocks++;
+ chip->select_chip(mtd, -1);
return ret;
}
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 1cbacff..a9119f7 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -324,13 +324,58 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
}
/**
- * nand_default_block_markbad - [DEFAULT] mark a block bad
+ * nand_default_block_markbad - [DEFAULT] mark a block bad via bad block marker
* @mtd: MTD device structure
* @ofs: offset from device start
*
* This is the default implementation, which can be overridden by a hardware
- * specific driver. We try operations in the following order, according to our
- * bbt_options (NAND_BBT_NO_OOB_BBM and NAND_BBT_USE_FLASH):
+ * specific driver. It provides the details for writing a bad block marker to a
+ * block.
+ */
+static int nand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)
+{
+ struct nand_chip *chip = mtd->priv;
+ struct mtd_oob_ops ops;
+ uint8_t buf[2] = { 0, 0 };
+ int ret = 0, res, i = 0;
+
+ ops.datbuf = NULL;
+ ops.oobbuf = buf;
+ ops.ooboffs = chip->badblockpos;
+ if (chip->options & NAND_BUSWIDTH_16) {
+ ops.ooboffs &= ~0x01;
+ ops.len = ops.ooblen = 2;
+ } else {
+ ops.len = ops.ooblen = 1;
+ }
+ ops.mode = MTD_OPS_PLACE_OOB;
+
+ /* Write to first/last page(s) if necessary */
+ if (chip->bbt_options & NAND_BBT_SCANLASTPAGE)
+ ofs += mtd->erasesize - mtd->writesize;
+ do {
+ res = nand_do_write_oob(mtd, ofs, &ops);
+ if (!ret)
+ ret = res;
+
+ i++;
+ ofs += mtd->writesize;
+ } while ((chip->bbt_options & NAND_BBT_SCAN2NDPAGE) && i < 2);
+
+ return ret;
+}
+
+/**
+ * nand_block_markbad_lowlevel - mark a block bad
+ * @mtd: MTD device structure
+ * @ofs: offset from device start
+ *
+ * This function performs the generic NAND bad block marking steps (i.e., bad
+ * block table(s) and/or marker(s)). We only allow the hardware driver to
+ * specify how to write bad block markers to OOB (chip->block_markbad).
+ *
+ * We try operations in the following order, according to our bbt_options
+ * (NAND_BBT_NO_OOB_BBM and NAND_BBT_USE_FLASH):
* (1) erase the affected block, to allow OOB marker to be written cleanly
* (2) update in-memory BBT
* (3) write bad block marker to OOB area of affected block
@@ -338,11 +383,10 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
* Note that we retain the first error encountered in (3) or (4), finish the
* procedures, and dump the error in the end.
*/
-static int nand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)
+static int nand_block_markbad_lowlevel(struct mtd_info *mtd, loff_t ofs)
{
struct nand_chip *chip = mtd->priv;
- uint8_t buf[2] = { 0, 0 };
- int block, res, ret = 0, i = 0;
+ int block, res, ret = 0;
int write_oob = !(chip->bbt_options & NAND_BBT_NO_OOB_BBM);
if (write_oob) {
@@ -364,34 +408,8 @@ static int nand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)
/* Write bad block marker to OOB */
if (write_oob) {
- struct mtd_oob_ops ops;
- loff_t wr_ofs = ofs;
-
nand_get_device(mtd, FL_WRITING);
-
- ops.datbuf = NULL;
- ops.oobbuf = buf;
- ops.ooboffs = chip->badblockpos;
- if (chip->options & NAND_BUSWIDTH_16) {
- ops.ooboffs &= ~0x01;
- ops.len = ops.ooblen = 2;
- } else {
- ops.len = ops.ooblen = 1;
- }
- ops.mode = MTD_OPS_PLACE_OOB;
-
- /* 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);
-
+ ret = chip->block_markbad(mtd, ofs);
nand_release_device(mtd);
}
@@ -2683,7 +2701,6 @@ static int nand_block_isbad(struct mtd_info *mtd, loff_t offs)
*/
static int nand_block_markbad(struct mtd_info *mtd, loff_t ofs)
{
- struct nand_chip *chip = mtd->priv;
int ret;
ret = nand_block_isbad(mtd, ofs);
@@ -2694,7 +2711,7 @@ static int nand_block_markbad(struct mtd_info *mtd, loff_t ofs)
return ret;
}
- return chip->block_markbad(mtd, ofs);
+ return nand_block_markbad_lowlevel(mtd, ofs);
}
/**
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 3f18776..bac481a 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -1392,4 +1392,3 @@ int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)
EXPORT_SYMBOL(nand_scan_bbt);
EXPORT_SYMBOL(nand_default_bbt);
-EXPORT_SYMBOL_GPL(nand_update_bbt);
diff --git a/drivers/mtd/nand/sm_common.c b/drivers/mtd/nand/sm_common.c
index e8181ed..e06b5e5 100644
--- a/drivers/mtd/nand/sm_common.c
+++ b/drivers/mtd/nand/sm_common.c
@@ -42,7 +42,7 @@ static int sm_block_markbad(struct mtd_info *mtd, loff_t ofs)
{
struct mtd_oob_ops ops;
struct sm_oob oob;
- int ret, error = 0;
+ int ret;
memset(&oob, -1, SM_OOB_SIZE);
oob.block_status = 0x0F;
@@ -61,11 +61,10 @@ static int sm_block_markbad(struct mtd_info *mtd, loff_t ofs)
printk(KERN_NOTICE
"sm_common: can't mark sector at %i as bad\n",
(int)ofs);
- error = -EIO;
- } else
- mtd->ecc_stats.badblocks++;
+ return -EIO;
+ }
- return error;
+ return 0;
}
static struct nand_flash_dev nand_smartmedia_flash_ids[] = {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 5/6] mtd: nand: hide in-memory BBT implementation details
2013-07-31 0:52 ` [PATCH v2 0/6] mtd: nand: cleanups to BBT Brian Norris
` (3 preceding siblings ...)
2013-07-31 0:52 ` [PATCH v2 4/6] mtd: nand: refactor chip->block_markbad interface Brian Norris
@ 2013-07-31 0:52 ` Brian Norris
2013-07-31 0:53 ` [PATCH v2 6/6] mtd: nand: remove NAND_BBT_SCANEMPTY Brian Norris
2013-08-06 14:08 ` [PATCH v2 0/6] mtd: nand: cleanups to BBT Artem Bityutskiy
6 siblings, 0 replies; 16+ messages in thread
From: Brian Norris @ 2013-07-31 0:52 UTC (permalink / raw)
To: linux-mtd; +Cc: Brian Norris, David Woodhouse, Artem Bityutskiy
nand_base.c shouldn't have to know the implementation details of
nand_bbt's in-memory BBT. Specifically, nand_base shouldn't perform the
bit masking and shifting to isolate a BBT entry.
Instead, just move some of the BBT code into a new nand_markbad_bbt()
interface. This interface allows external users (i.e., nand_base) to
mark a single block as bad in the BBT. Then nand_bbt will take care of
modifying the in-memory BBT and updating the flash-based BBT (if
applicable).
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
drivers/mtd/nand/nand_base.c | 32 +++++++++++---------------------
drivers/mtd/nand/nand_bbt.c | 28 ++++++++++++++++++++++++++--
include/linux/mtd/nand.h | 2 +-
3 files changed, 38 insertions(+), 24 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index a9119f7..da3ba77 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -374,22 +374,20 @@ static int nand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)
* block table(s) and/or marker(s)). We only allow the hardware driver to
* specify how to write bad block markers to OOB (chip->block_markbad).
*
- * We try operations in the following order, according to our bbt_options
- * (NAND_BBT_NO_OOB_BBM and NAND_BBT_USE_FLASH):
+ * We try operations in the following order:
* (1) erase the affected block, to allow OOB marker to be written cleanly
- * (2) update in-memory BBT
- * (3) write bad block marker to OOB area of affected block
- * (4) update flash-based BBT
- * Note that we retain the first error encountered in (3) or (4), finish the
+ * (2) write bad block marker to OOB area of affected block (unless flag
+ * NAND_BBT_NO_OOB_BBM is present)
+ * (3) update the BBT
+ * Note that we retain the first error encountered in (2) or (3), finish the
* procedures, and dump the error in the end.
*/
static int nand_block_markbad_lowlevel(struct mtd_info *mtd, loff_t ofs)
{
struct nand_chip *chip = mtd->priv;
- int block, res, ret = 0;
- int write_oob = !(chip->bbt_options & NAND_BBT_NO_OOB_BBM);
+ int res, ret = 0;
- if (write_oob) {
+ if (!(chip->bbt_options & NAND_BBT_NO_OOB_BBM)) {
struct erase_info einfo;
/* Attempt erase before marking OOB */
@@ -398,24 +396,16 @@ static int nand_block_markbad_lowlevel(struct mtd_info *mtd, loff_t ofs)
einfo.addr = ofs;
einfo.len = 1 << chip->phys_erase_shift;
nand_erase_nand(mtd, &einfo, 0);
- }
-
- /* Get block number */
- block = (int)(ofs >> chip->bbt_erase_shift);
- /* Mark block bad in memory-based BBT */
- if (chip->bbt)
- chip->bbt[block >> 2] |= 0x01 << ((block & 0x03) << 1);
- /* Write bad block marker to OOB */
- if (write_oob) {
+ /* Write bad block marker to OOB */
nand_get_device(mtd, FL_WRITING);
ret = chip->block_markbad(mtd, ofs);
nand_release_device(mtd);
}
- /* Update flash-based bad block table */
- if (chip->bbt_options & NAND_BBT_USE_FLASH) {
- res = nand_update_bbt(mtd, ofs);
+ /* Mark block bad in BBT */
+ if (chip->bbt) {
+ res = nand_markbad_bbt(mtd, ofs);
if (!ret)
ret = res;
}
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index bac481a..ae3fb58 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -79,6 +79,8 @@
#define BBT_ENTRY_MASK 0x03
#define BBT_ENTRY_SHIFT 2
+static int nand_update_bbt(struct mtd_info *mtd, loff_t offs);
+
static inline uint8_t bbt_get_entry(struct nand_chip *chip, int block)
{
uint8_t entry = chip->bbt[block >> BBT_ENTRY_SHIFT];
@@ -1194,13 +1196,13 @@ int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
}
/**
- * nand_update_bbt - [NAND Interface] update bad block table(s)
+ * nand_update_bbt - update bad block table(s)
* @mtd: MTD device structure
* @offs: the offset of the newly marked block
*
* The function updates the bad block table(s).
*/
-int nand_update_bbt(struct mtd_info *mtd, loff_t offs)
+static int nand_update_bbt(struct mtd_info *mtd, loff_t offs)
{
struct nand_chip *this = mtd->priv;
int len, res = 0;
@@ -1390,5 +1392,27 @@ int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)
return 1;
}
+/**
+ * nand_markbad_bbt - [NAND Interface] Mark a block bad in the BBT
+ * @mtd: MTD device structure
+ * @offs: offset of the bad block
+ */
+int nand_markbad_bbt(struct mtd_info *mtd, loff_t offs)
+{
+ struct nand_chip *this = mtd->priv;
+ int block, ret = 0;
+
+ block = (int)(offs >> this->bbt_erase_shift);
+
+ /* Mark bad block in memory */
+ bbt_mark_entry(this, block, BBT_BLOCK_WORN);
+
+ /* Update flash-based bad block table */
+ if (this->bbt_options & NAND_BBT_USE_FLASH)
+ ret = nand_update_bbt(mtd, offs);
+
+ return ret;
+}
+
EXPORT_SYMBOL(nand_scan_bbt);
EXPORT_SYMBOL(nand_default_bbt);
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 0745a42..9f7b248 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -628,8 +628,8 @@ extern struct nand_flash_dev nand_flash_ids[];
extern struct nand_manufacturers nand_manuf_ids[];
extern int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd);
-extern int nand_update_bbt(struct mtd_info *mtd, loff_t offs);
extern int nand_default_bbt(struct mtd_info *mtd);
+extern int nand_markbad_bbt(struct mtd_info *mtd, loff_t offs);
extern int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt);
extern int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
int allowbbt);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 6/6] mtd: nand: remove NAND_BBT_SCANEMPTY
2013-07-31 0:52 ` [PATCH v2 0/6] mtd: nand: cleanups to BBT Brian Norris
` (4 preceding siblings ...)
2013-07-31 0:52 ` [PATCH v2 5/6] mtd: nand: hide in-memory BBT implementation details Brian Norris
@ 2013-07-31 0:53 ` Brian Norris
2013-08-06 14:08 ` [PATCH v2 0/6] mtd: nand: cleanups to BBT Artem Bityutskiy
6 siblings, 0 replies; 16+ messages in thread
From: Brian Norris @ 2013-07-31 0:53 UTC (permalink / raw)
To: linux-mtd; +Cc: Ivan Djelic, Brian Norris, David Woodhouse, Artem Bityutskiy
NAND_BBT_SCANEMPTY is a strange, badly-supported option with omap as its
single remaining user.
NAND_BBT_SCANEMPTY was likely used by accident in omap2[1]. And anyway,
omap2 doesn't scan the chip for bad blocks (courtesy of
NAND_SKIP_BBTSCAN), and so its use of this option is irrelevant.
This patch drops the NAND_BBT_SCANEMPTY option.
[1] http://lists.infradead.org/pipermail/linux-mtd/2012-July/042902.html
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: Ivan Djelic <ivan.djelic@parrot.com>
---
Documentation/DocBook/mtdnand.tmpl | 2 --
drivers/mtd/nand/nand_bbt.c | 33 +++++----------------------------
drivers/mtd/nand/omap2.c | 2 +-
drivers/mtd/onenand/onenand_bbt.c | 1 -
include/linux/mtd/bbm.h | 2 --
5 files changed, 6 insertions(+), 34 deletions(-)
diff --git a/Documentation/DocBook/mtdnand.tmpl b/Documentation/DocBook/mtdnand.tmpl
index fe122d6..a248f42 100644
--- a/Documentation/DocBook/mtdnand.tmpl
+++ b/Documentation/DocBook/mtdnand.tmpl
@@ -1224,8 +1224,6 @@ in this page</entry>
#define NAND_BBT_CREATE 0x00000200
/* Search good / bad pattern through all pages of a block */
#define NAND_BBT_SCANALLPAGES 0x00000400
-/* Scan block empty during good / bad block scan */
-#define NAND_BBT_SCANEMPTY 0x00000800
/* 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 ae3fb58..bc06196 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -110,33 +110,17 @@ static int check_pattern_no_oob(uint8_t *buf, struct nand_bbt_descr *td)
* @td: search pattern descriptor
*
* Check for a pattern at the given place. Used to search bad block tables and
- * good / bad block identifiers. If the SCAN_EMPTY option is set then check, if
- * all bytes except the pattern area contain 0xff.
+ * good / bad block identifiers.
*/
static int check_pattern(uint8_t *buf, int len, int paglen, struct nand_bbt_descr *td)
{
- int end = 0;
- uint8_t *p = buf;
-
if (td->options & NAND_BBT_NO_OOB)
return check_pattern_no_oob(buf, td);
- end = paglen + td->offs;
- if (td->options & NAND_BBT_SCANEMPTY)
- if (memchr_inv(p, 0xff, end))
- return -1;
- p += end;
-
/* Compare the pattern */
- if (memcmp(p, td->pattern, td->len))
+ if (memcmp(buf + paglen + td->offs, td->pattern, td->len))
return -1;
- if (td->options & NAND_BBT_SCANEMPTY) {
- p += td->len;
- end += td->len;
- if (memchr_inv(p, 0xff, len - end))
- return -1;
- }
return 0;
}
@@ -507,15 +491,9 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
else
numpages = 1;
- if (!(bd->options & NAND_BBT_SCANEMPTY)) {
- /* We need only read few bytes from the OOB area */
- scanlen = 0;
- readlen = bd->len;
- } else {
- /* Full page content should be read */
- scanlen = mtd->writesize + mtd->oobsize;
- readlen = numpages * mtd->writesize;
- }
+ /* 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;
@@ -882,7 +860,6 @@ static inline int nand_memory_bbt(struct mtd_info *mtd, struct nand_bbt_descr *b
{
struct nand_chip *this = mtd->priv;
- bd->options &= ~NAND_BBT_SCANEMPTY;
return create_bbt(mtd, this->buffers->databuf, bd, -1);
}
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index c4c7e0d..d6b9cc1 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -159,7 +159,7 @@ static struct nand_ecclayout omap_oobinfo;
*/
static uint8_t scan_ff_pattern[] = { 0xff };
static struct nand_bbt_descr bb_descrip_flashbased = {
- .options = NAND_BBT_SCANEMPTY | NAND_BBT_SCANALLPAGES,
+ .options = NAND_BBT_SCANALLPAGES,
.offs = 0,
.len = 1,
.pattern = scan_ff_pattern,
diff --git a/drivers/mtd/onenand/onenand_bbt.c b/drivers/mtd/onenand/onenand_bbt.c
index 66fe3b7..08d0085 100644
--- a/drivers/mtd/onenand/onenand_bbt.c
+++ b/drivers/mtd/onenand/onenand_bbt.c
@@ -133,7 +133,6 @@ static inline int onenand_memory_bbt (struct mtd_info *mtd, struct nand_bbt_desc
{
struct onenand_chip *this = mtd->priv;
- bd->options &= ~NAND_BBT_SCANEMPTY;
return create_bbt(mtd, this->page_buf, bd, -1);
}
diff --git a/include/linux/mtd/bbm.h b/include/linux/mtd/bbm.h
index 211ff67..95fc482 100644
--- a/include/linux/mtd/bbm.h
+++ b/include/linux/mtd/bbm.h
@@ -93,8 +93,6 @@ struct nand_bbt_descr {
#define NAND_BBT_CREATE_EMPTY 0x00000400
/* Search good / bad pattern through all pages of a block */
#define NAND_BBT_SCANALLPAGES 0x00000800
-/* Scan block empty during good / bad block scan */
-#define NAND_BBT_SCANEMPTY 0x00001000
/* Write bbt if neccecary */
#define NAND_BBT_WRITE 0x00002000
/* Read and write back block contents when writing bbt */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/6] mtd: nand: cleanups to BBT
2013-07-31 0:52 ` [PATCH v2 0/6] mtd: nand: cleanups to BBT Brian Norris
` (5 preceding siblings ...)
2013-07-31 0:53 ` [PATCH v2 6/6] mtd: nand: remove NAND_BBT_SCANEMPTY Brian Norris
@ 2013-08-06 14:08 ` Artem Bityutskiy
6 siblings, 0 replies; 16+ messages in thread
From: Artem Bityutskiy @ 2013-08-06 14:08 UTC (permalink / raw)
To: Brian Norris
Cc: David Woodhouse, Ivan Djelic, linux-mtd, Ezequiel Garcia,
Huang Shijie
On Tue, 2013-07-30 at 17:52 -0700, Brian Norris wrote:
> Hi all,
>
> This is v2 of my NAND BBT cleanup. The net diff is exactly the same as v1, but
> it splits up a few logical changes.
>
> v1 -> v2:
> Split up first patch into 3 patches, according to Ezequiel's comments.
> Hopefully this makes it easier to read and review.
Pushed to l2-mtd.git, thanks!
--
Best Regards,
Artem Bityutskiy
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2013-08-06 14:03 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-24 6:27 [PATCH 0/4] mtd: nand: cleanups to BBT Brian Norris
2013-07-24 6:27 ` [PATCH 1/4] mtd: nand: add accessors, macros for in-memory BBT Brian Norris
2013-07-30 22:02 ` Ezequiel Garcia
2013-07-31 0:40 ` Brian Norris
2013-07-24 6:27 ` [PATCH 2/4] mtd: nand: refactor chip->block_markbad interface Brian Norris
2013-07-24 6:59 ` Huang Shijie
2013-07-24 6:27 ` [PATCH 3/4] mtd: nand: hide in-memory BBT implementation details Brian Norris
2013-07-24 6:27 ` [PATCH 4/4] mtd: nand: remove NAND_BBT_SCANEMPTY Brian Norris
2013-07-31 0:52 ` [PATCH v2 0/6] mtd: nand: cleanups to BBT Brian Norris
2013-07-31 0:52 ` [PATCH v2 1/6] mtd: nand: add accessors, macros for in-memory BBT Brian Norris
2013-07-31 0:52 ` [PATCH v2 2/6] mtd: nand: remove multiplied-by-2 block logic Brian Norris
2013-07-31 0:52 ` [PATCH v2 3/6] mtd: nand: eliminate cast Brian Norris
2013-07-31 0:52 ` [PATCH v2 4/6] mtd: nand: refactor chip->block_markbad interface Brian Norris
2013-07-31 0:52 ` [PATCH v2 5/6] mtd: nand: hide in-memory BBT implementation details Brian Norris
2013-07-31 0:53 ` [PATCH v2 6/6] mtd: nand: remove NAND_BBT_SCANEMPTY Brian Norris
2013-08-06 14:08 ` [PATCH v2 0/6] mtd: nand: cleanups to BBT Artem Bityutskiy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox