Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1291880 > unrolled thread

[PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT Peter Pan <peterpansjtu@gmail.com> - 2015-12-15 07:20 +0100
    [PATCH v2 09/12] mtd: nand_bbt: remove old API definitions Peter Pan <peterpansjtu@gmail.com> - 2015-12-15 07:20 +0100
    [PATCH v2 05/12] mtd: nand: use new BBT API instead of old ones Peter Pan <peterpansjtu@gmail.com> - 2015-12-15 07:20 +0100
    [PATCH v2 04/12] mtd: nand_bbt: add nand_bbt_markbad_factory() interface Peter Pan <peterpansjtu@gmail.com> - 2015-12-15 07:20 +0100
    [PATCH v2 02/12] mtd: nand_bbt: introduce struct nand_bbt Peter Pan <peterpansjtu@gmail.com> - 2015-12-15 07:20 +0100
    Re: [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> - 2015-12-28 21:50 +0100
      Re: [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-12-29 10:40 +0100
        Re: [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> - 2015-12-29 16:10 +0100
          Re: [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-12-29 16:20 +0100
            Re: [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT 潘栋 <peterpansjtu@gmail.com> - 2015-12-30 08:20 +0100
              Re: [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-12-30 09:40 +0100

#1291880 — [PATCH v2 00/12] mtd: nand_bbt: introduce independent nand BBT

Currently nand_bbt.c is tied with struct nand_chip, and it makes other
NAND family chips hard to use nand_bbt.c. Maybe it's the reason why
onenand has own bbt(onenand_bbt.c).

Separate struct nand_chip from BBT code can make current BBT shareable.
We create struct nand_bbt to take place of nand_chip in nand_bbt.c.
Struct nand_bbt contains all the information BBT needed from outside and
it should be embedded into NAND family chip struct (such as struct nand_chip).
NAND family driver should allocate, initialize and free struct nand_bbt.

Below is mtd folder structure we want:
	mtd
	├── Kconfig
	├── Makefile
	├── ...
	├── nand_bbt.c
	├── nand
	│   ├── Kconfig
	│   ├── Makefile
	│   ├── nand_base.c
	│   ├── nand_ids.c
	│   ├── ...
	│   └── xway_nand.c
	├── spi-nand
	│   ├── Kconfig
	│   ├── Makefile
	│   ├── spi-nand-base.c
	│   ├── ...
	│   └── spi-nand-device.c
	└── ...

Most of the patch is borrowed from Brian Norris <computersforpeace@gmail.com>.
http://git.infradead.org/users/norris/linux-mtd.git/shortlog/refs/heads/nand-bbt
Based on Brian's suggestion, I make my previous BBT patch into 12 independent
patches. Previous patch is http://patchwork.ozlabs.org/patch/492066/
Beside the patch split, I also moved nand_bbt.c to mtd folder, which didn't in
previous patch.

Patch 3, 7, 8, 9, 10 and 11 are totally borrowed from Brian's git tree. I just
test and split the code into independent patch. Patch 1, 2, 5 and 6 are partial
borrowed. I make some changes from Brian's git tree and the changes are recorded
in commit log. Patch 4 and 12 are written by me.

The patch is tested on Zed board.

v2 changes:
	rebase patch series on master branch of l2-mtd.git

Brian Norris (10):
  mtd: nand_bbt: new header for nand family BBT
  mtd: nand_bbt: introduce struct nand_bbt
  mtd: nand_bbt: add new API definitions
  mtd: nand: use new BBT API instead of old ones
  mtd: nand_bbt: use erase() and is_bad_bbm() hook in BBT
  mtd: nand: make nand_erase_nand() static
  mtd: nand_bbt: remove struct nand_chip from nand_bbt.c
  mtd: nand_bbt: remove old API definitions
  mtd: nand_bbt: remove NAND_BBT_DYNAMICSTRUCT macro
  mtd: nand: remove nand_chip.bbt

Peter Pan (2):
  mtd: nand_bbt: add nand_bbt_markbad_factory() interface
  mtd: nand-bbt: move nand_bbt.c to mtd folder

 drivers/mtd/Kconfig               |   7 +
 drivers/mtd/Makefile              |   1 +
 drivers/mtd/nand/Kconfig          |   2 +-
 drivers/mtd/nand/Makefile         |   2 +-
 drivers/mtd/nand/docg4.c          |   6 +-
 drivers/mtd/nand/nand_base.c      | 145 +++++++++-
 drivers/mtd/{nand => }/nand_bbt.c | 542 ++++++++++++++++----------------------
 include/linux/mtd/bbm.h           |  96 +------
 include/linux/mtd/nand.h          |  16 +-
 include/linux/mtd/nand_bbt.h      | 177 +++++++++++++
 10 files changed, 562 insertions(+), 432 deletions(-)
 rename drivers/mtd/{nand => }/nand_bbt.c (68%)
 create mode 100644 include/linux/mtd/nand_bbt.h

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1291883 — [PATCH v2 09/12] mtd: nand_bbt: remove old API definitions

From: Brian Norris <computersforpeace@gmail.com>

remove old BBT APIs

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Peter Pan <peterpandong@micron.com>
---
 drivers/mtd/nand/nand_bbt.c | 110 +++++++++-----------------------------------
 include/linux/mtd/nand.h    |   3 --
 2 files changed, 22 insertions(+), 91 deletions(-)

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index b46b4ae..22861a1 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -1166,13 +1166,14 @@ static struct nand_bbt_descr bbt_mirror_no_oob_descr = {
 };
 
 /**
- * nand_default_bbt - [NAND Interface] Select a default bad block table for the device
+ * nand_bbt_init - [NAND BBT Interface] Initialize and locate/create a bad block
+ * table
  * @bbt: NAND BBT structure
  *
  * This function selects the default bad block table support for the device and
- * calls the nand_scan_bbt function.
+ * scans for an existing table, or else creates one.
  */
-int nand_default_bbt(struct nand_bbt *bbt)
+int nand_bbt_init(struct nand_bbt *bbt)
 {
 	/* Is a flash based bad block table requested? */
 	if (bbt->bbt_options & NAND_BBT_USE_FLASH) {
@@ -1193,46 +1194,54 @@ int nand_default_bbt(struct nand_bbt *bbt)
 
 	return nand_scan_bbt(bbt);
 }
+EXPORT_SYMBOL(nand_bbt_init);
+
+void nand_bbt_release(struct nand_bbt *bbt)
+{
+	kfree(bbt->bbt);
+}
+EXPORT_SYMBOL(nand_bbt_release);
 
 /**
- * nand_isreserved_bbt - [NAND Interface] Check if a block is reserved
+ * nand_bbt_isreserved - [NAND BBT Interface] Check if a block is reserved
  * @bbt: NAND BBT structure
  * @offs: offset in the device
  */
-int nand_isreserved_bbt(struct nand_bbt *bbt, loff_t offs)
+int nand_bbt_isreserved(struct nand_bbt *bbt, loff_t offs)
 {
 	int block;
 
 	block = (int)(offs >> bbt->bbt_erase_shift);
 	return bbt_get_entry(bbt, block) == BBT_BLOCK_RESERVED;
 }
+EXPORT_SYMBOL(nand_bbt_isreserved);
 
 /**
- * nand_isbad_bbt - [NAND Interface] Check if a block is bad
+ * nand_bbt_isbad - [NAND BBT Interface] Check if a block is bad
  * @bbt: NAND BBT structure
  * @offs: offset in the device
- * @allowbbt: allow access to bad block table region
  */
-int nand_isbad_bbt(struct nand_bbt *bbt, loff_t offs, int allowbbt)
+int nand_bbt_isbad(struct nand_bbt *bbt, loff_t offs)
 {
 	int block, res;
 
 	block = (int)(offs >> bbt->bbt_erase_shift);
 	res = bbt_get_entry(bbt, block);
 
-	pr_debug("nand_isbad_bbt(): bbt info for offs 0x%08x: (block %d) 0x%02x\n",
+	pr_debug("nand_bbt_isbad(): bbt info for offs 0x%08x: (block %d) 0x%02x\n",
 		 (unsigned int)offs, block, res);
 
 	switch (res) {
 	case BBT_BLOCK_GOOD:
+	case BBT_BLOCK_RESERVED:
 		return 0;
 	case BBT_BLOCK_WORN:
+	case BBT_BLOCK_FACTORY_BAD:
+	default:
 		return 1;
-	case BBT_BLOCK_RESERVED:
-		return allowbbt ? 0 : 1;
 	}
-	return 1;
 }
+EXPORT_SYMBOL(nand_bbt_isbad);
 
 /**
  * nand_bbt_update_mark - update mark in the BBT
@@ -1257,84 +1266,13 @@ static int nand_bbt_update_mark(struct nand_bbt *bbt, loff_t offs, uint8_t mark)
 }
 
 /**
- * nand_markbad_bbt - [NAND Interface] Mark a block bad in the BBT
- * @bbt: NAND BBT structure
- * @offs: offset of the bad block
- */
-int nand_markbad_bbt(struct nand_bbt *bbt, loff_t offs)
-{
-	return nand_bbt_update_mark(bbt, offs, BBT_BLOCK_WORN);
-}
-EXPORT_SYMBOL(nand_scan_bbt);
-
-/**
- * nand_bbt_init - [NAND BBT Interface] Initialize and locate/create a bad block
- * table
- * @bbt: NAND BBT structure
- *
- * This function selects the default bad block table support for the device and
- * scans for an existing table, or else creates one.
- */
-int nand_bbt_init(struct nand_bbt *bbt)
-{
-	/*
-	 * FIXME: For now, we call nand_default_bbt() directly. It will change
-	 * when we use struct nand_bbt instead of struct nand_chip.
-	 */
-	return nand_default_bbt(bbt);
-}
-EXPORT_SYMBOL(nand_bbt_init);
-
-void nand_bbt_release(struct nand_bbt *bbt)
-{
-	kfree(bbt->bbt);
-}
-EXPORT_SYMBOL(nand_bbt_release);
-
-/**
- * nand_bbt_isreserved - [NAND BBT Interface] Check if a block is reserved
- * @bbt: NAND BBT structure
- * @offs: offset in the device
- */
-int nand_bbt_isreserved(struct nand_bbt *bbt, loff_t offs)
-{
-	/*
-	 * FIXME: For now, we call nand_isreserved_bbt() directly. It will
-	 * change when we use struct nand_bbt instead of struct nand_chip.
-	 */
-	return nand_isreserved_bbt(bbt, offs);
-}
-EXPORT_SYMBOL(nand_bbt_isreserved);
-
-/**
- * nand_bbt_isbad - [NAND BBT Interface] Check if a block is bad
- * @bbt: NAND BBT structure
- * @offs: offset in the device
- */
-int nand_bbt_isbad(struct nand_bbt *bbt, loff_t offs)
-{
-	/*
-	 * FIXME: For now, we call nand_isbad_bbt() directly. It will change
-	 * when we use struct nand_bbt instead of struct nand_chip.
-	 * Since we already have nand_bbt_isreserved(), we don't need to
-	 * check pass down allow_bbt.
-	 */
-	return nand_isbad_bbt(bbt, offs, 1);
-}
-EXPORT_SYMBOL(nand_bbt_isbad);
-
-/**
  * nand_bbt_markbad - [NAND BBT Interface] Mark a block bad in the BBT
  * @bbt: NAND BBT structure
  * @offs: offset of the bad block
  */
 int nand_bbt_markbad(struct nand_bbt *bbt, loff_t offs)
 {
-	/*
-	 * FIXME: For now, we call nand_markbad_bbt() directly. It will change
-	 * when we use struct nand_bbt instead of struct nand_chip.
-	 */
-	return nand_markbad_bbt(bbt, offs);
+	return nand_bbt_update_mark(bbt, offs, BBT_BLOCK_WORN);
 }
 EXPORT_SYMBOL(nand_bbt_markbad);
 
@@ -1346,10 +1284,6 @@ EXPORT_SYMBOL(nand_bbt_markbad);
  */
 int nand_bbt_markbad_factory(struct nand_bbt *bbt, loff_t offs)
 {
-	/*
-	 * FIXME: For now, we call nand_markbad_bbt() directly. It will change
-	 * when we use struct nand_bbt instead of struct nand_chip.
-	 */
 	return nand_bbt_update_mark(bbt, offs, BBT_BLOCK_FACTORY_BAD);
 }
 EXPORT_SYMBOL(nand_bbt_markbad_factory);
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 2051bd9..ee15f7d 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -863,9 +863,6 @@ struct nand_manufacturers {
 extern struct nand_flash_dev nand_flash_ids[];
 extern struct nand_manufacturers nand_manuf_ids[];
 
-extern int nand_markbad_bbt(struct mtd_info *mtd, loff_t offs);
-extern int nand_isreserved_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_do_read(struct mtd_info *mtd, loff_t from, size_t len,
 			size_t *retlen, uint8_t *buf);
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291884 — [PATCH v2 05/12] mtd: nand: use new BBT API instead of old ones

From: Brian Norris <computersforpeace@gmail.com>

Use new BBT APIs (nand_bbt_*()) in NAND. Keep old APIs (nand_*_bbt())
exist temporarily.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
[Peter: 1. use nand_bbt_markbad_factory() in docg4.c and implement
        2. nand_create_factory_badblock_pattern(), nand_is_bad_bbm() and
        nand_default_bbt() in nand_base.c
        3. add NAND_BADBLOCK_PATTERN_ALLOC macro]
Signed-off-by: Peter Pan <peterpandong@micron.com>
---
 drivers/mtd/nand/docg4.c     |   6 +-
 drivers/mtd/nand/nand_base.c | 140 ++++++++++++++++++++++++++++++++++++++++---
 include/linux/mtd/nand.h     |   9 ++-
 3 files changed, 143 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/docg4.c b/drivers/mtd/nand/docg4.c
index da93d7f..1be7fa2 100644
--- a/drivers/mtd/nand/docg4.c
+++ b/drivers/mtd/nand/docg4.c
@@ -1037,7 +1037,7 @@ static int __init read_factory_bbt(struct mtd_info *mtd)
 	 * operation after device power-up.  The above read ensures it never is.
 	 * Ugly, I know.
 	 */
-	if (nand->bbt == NULL)  /* no memory-based bbt */
+	if (nand->nand_bbt == NULL)  /* no memory-based bbt */
 		goto exit;
 
 	if (mtd->ecc_stats.failed > eccfailed_stats) {
@@ -1064,8 +1064,8 @@ static int __init read_factory_bbt(struct mtd_info *mtd)
 		unsigned long bits = ~buf[i];
 		for_each_set_bit(bitnum, &bits, 8) {
 			int badblock = block + 7 - bitnum;
-			nand->bbt[badblock / 4] |=
-				0x03 << ((badblock % 4) * 2);
+			nand_bbt_markbad_factory(nand->nand_bbt,
+					badblock << nand->bbt_erase_shift);
 			mtd->ecc_stats.badblocks++;
 			dev_notice(doc->dev, "factory-marked bad block: %d\n",
 				   badblock);
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 5aec154..28d7a58 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -450,8 +450,8 @@ static int nand_block_markbad_lowlevel(struct mtd_info *mtd, loff_t ofs)
 	}
 
 	/* Mark block bad in BBT */
-	if (chip->bbt) {
-		res = nand_markbad_bbt(mtd, ofs);
+	if (chip->nand_bbt) {
+		res = nand_bbt_markbad(chip->nand_bbt, ofs);
 		if (!ret)
 			ret = res;
 	}
@@ -493,10 +493,10 @@ static int nand_block_isreserved(struct mtd_info *mtd, loff_t ofs)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
 
-	if (!chip->bbt)
+	if (!chip->nand_bbt)
 		return 0;
 	/* Return info from the table */
-	return nand_isreserved_bbt(mtd, ofs);
+	return nand_bbt_isreserved(chip->nand_bbt, ofs);
 }
 
 /**
@@ -513,12 +513,18 @@ static int nand_block_checkbad(struct mtd_info *mtd, loff_t ofs, int getchip,
 			       int allowbbt)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct nand_bbt *bbt = chip->nand_bbt;
 
-	if (!chip->bbt)
+	if (!bbt)
 		return chip->block_bad(mtd, ofs, getchip);
 
 	/* Return info from the table */
-	return nand_isbad_bbt(mtd, ofs, allowbbt);
+	if (nand_bbt_isbad(bbt, ofs))
+		return 1;
+	else if (allowbbt)
+		return 0;
+	else
+		return nand_bbt_isreserved(bbt, ofs);
 }
 
 /**
@@ -2982,6 +2988,122 @@ erase_exit:
 	return ret;
 }
 
+/* NAND BBT helper - erase a block, including reserved blocks */
+static int nand_bbt_erase_block(struct mtd_info *mtd, loff_t addr)
+{
+	struct erase_info einfo;
+	struct nand_chip *chip = mtd->priv;
+
+	memset(&einfo, 0, sizeof(einfo));
+	einfo.mtd = mtd;
+	einfo.addr = addr;
+	einfo.len = 1ULL << chip->phys_erase_shift;
+
+	return nand_erase_nand(mtd, &einfo, 1);
+}
+
+static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
+#define BADBLOCK_SCAN_MASK (~NAND_BBT_NO_OOB)
+/**
+ * nand_create_factory_badblock_pattern - [INTERN] Creates a BBT descriptor
+ * structure for factory bad block marker
+ * @chip: NAND chip to create descriptor for
+ *
+ * This function allocates and initializes a badblock_pattern for factory bad
+ * block marker based on the properties of @chip when chip.badblock_pattern
+ * is NULL.
+ */
+static int nand_create_factory_badblock_pattern(struct nand_chip *chip)
+{
+	struct nand_bbt_descr *bd;
+
+	if (chip->badblock_pattern) {
+		pr_warn("Bad block pattern already allocated; not replacing\n");
+		return -EINVAL;
+	}
+	bd = kzalloc(sizeof(*bd), GFP_KERNEL);
+	if (!bd)
+		return -ENOMEM;
+	bd->options = chip->bbt_options & BADBLOCK_SCAN_MASK;
+	bd->offs = chip->badblockpos;
+	bd->len = (chip->options & NAND_BUSWIDTH_16) ? 2 : 1;
+	bd->pattern = scan_ff_pattern;
+	bd->options |= NAND_BADBLOCK_PATTERN_ALLOC;
+	chip->badblock_pattern = bd;
+
+	return 0;
+}
+
+static int nand_is_bad_bbm(struct mtd_info *mtd, loff_t addr)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct mtd_oob_ops ops;
+	struct nand_bbt_descr *bd = chip->badblock_pattern;
+	int j, ret;
+	int numpages;
+
+	if (bd->options & NAND_BBT_SCAN2NDPAGE)
+		numpages = 2;
+	else
+		numpages = 1;
+
+	if (bd->options & NAND_BBT_SCANLASTPAGE)
+		addr += mtd->erasesize - (mtd->writesize * numpages);
+
+	ops.ooblen = mtd->oobsize;
+	ops.oobbuf = chip->buffers->databuf;
+	ops.ooboffs = 0;
+	ops.datbuf = NULL;
+	ops.mode = MTD_OPS_PLACE_OOB;
+
+	for (j = 0; j < numpages; j++) {
+		/*
+		 * Read the full oob until read_oob is fixed to handle single
+		 * byte reads for 16 bit buswidth.
+		 */
+		ret = nand_do_read_oob(mtd, addr, &ops);
+		/* Ignore ECC errors when checking for BBM */
+		if (ret && !mtd_is_bitflip_or_eccerr(ret))
+			return ret;
+
+		if (memcmp(chip->buffers->databuf + bd->offs,
+				bd->pattern, bd->len))
+			return 1;
+
+		addr += mtd->writesize;
+	}
+
+	return 0;
+}
+
+static int nand_default_bbt(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nand_bbt *bbt = kzalloc(sizeof(struct nand_bbt), GFP_KERNEL);
+
+	if (!bbt)
+		return -ENOMEM;
+
+	bbt->bbt_options = chip->bbt_options;
+	bbt->mtd = mtd;
+	bbt->numchips = chip->numchips;
+	bbt->chipsize = chip->chipsize;
+	bbt->chip_shift = chip->chip_shift;
+	bbt->bbt_erase_shift = chip->phys_erase_shift;
+	bbt->page_shift = chip->page_shift;
+	bbt->bbt_td = chip->bbt_td;
+	bbt->bbt_md = chip->bbt_md;
+	bbt->is_bad_bbm = nand_is_bad_bbm;
+	bbt->erase = nand_bbt_erase_block;
+	chip->nand_bbt = bbt;
+
+	if (!chip->badblock_pattern &&
+		nand_create_factory_badblock_pattern(chip))
+		return -ENOMEM;
+
+	return nand_bbt_init(chip->nand_bbt);
+}
+
 /**
  * nand_sync - [MTD Interface] sync
  * @mtd: MTD device structure
@@ -4437,13 +4559,15 @@ void nand_release(struct mtd_info *mtd)
 	mtd_device_unregister(mtd);
 
 	/* Free bad block table memory */
-	kfree(chip->bbt);
+	if (chip->nand_bbt)
+		nand_bbt_release(chip->nand_bbt);
+	kfree(chip->nand_bbt);
 	if (!(chip->options & NAND_OWN_BUFFERS))
 		kfree(chip->buffers);
 
 	/* Free bad block descriptor memory */
 	if (chip->badblock_pattern && chip->badblock_pattern->options
-			& NAND_BBT_DYNAMICSTRUCT)
+				& NAND_BADBLOCK_PATTERN_ALLOC)
 		kfree(chip->badblock_pattern);
 }
 EXPORT_SYMBOL_GPL(nand_release);
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index b614ed2..12c9c07 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -190,6 +190,11 @@ typedef enum {
  */
 #define NAND_USE_BOUNCE_BUFFER	0x00100000
 
+/*
+ * Flag to mark that the badblock_pattern is allocated dynamicaly and must
+ * be freed in nand_release().
+ */
+#define NAND_BADBLOCK_PATTERN_ALLOC	0x08000000
 /* Options set by nand scan */
 /* Nand scan has allocated controller struct */
 #define NAND_CONTROLLER_ALLOC	0x80000000
@@ -625,6 +630,8 @@ struct nand_buffers {
  * @onfi_set_features:	[REPLACEABLE] set the features for ONFI nand
  * @onfi_get_features:	[REPLACEABLE] get the features for ONFI nand
  * @bbt:		[INTERN] bad block table pointer
+ * @nand_bbt:		[INTERN] pointer to bad block table structure, which
+ *			includes all information needed by Bad Block Management
  * @bbt_td:		[REPLACEABLE] bad block table descriptor for flash
  *			lookup.
  * @bbt_md:		[REPLACEABLE] bad block table mirror descriptor
@@ -713,6 +720,7 @@ struct nand_chip {
 	struct nand_hw_control hwcontrol;
 
 	uint8_t *bbt;
+	struct nand_bbt *nand_bbt;
 	struct nand_bbt_descr *bbt_td;
 	struct nand_bbt_descr *bbt_md;
 
@@ -855,7 +863,6 @@ struct nand_manufacturers {
 extern struct nand_flash_dev nand_flash_ids[];
 extern struct nand_manufacturers nand_manuf_ids[];
 
-extern int nand_default_bbt(struct mtd_info *mtd);
 extern int nand_markbad_bbt(struct mtd_info *mtd, loff_t offs);
 extern int nand_isreserved_bbt(struct mtd_info *mtd, loff_t offs);
 extern int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291885 — [PATCH v2 04/12] mtd: nand_bbt: add nand_bbt_markbad_factory() interface

Some nand controller drivers(like docg4.c) mark factory bad block
mark by accessing bbt buffer directly, so create this API to avoid
breaking layer.

Signed-off-by: Peter Pan <peterpandong@micron.com>
---
 drivers/mtd/nand/nand_bbt.c  | 32 +++++++++++++++++++++++++++++---
 include/linux/mtd/nand_bbt.h |  1 +
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 72aed20..f256be1 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -1353,11 +1353,12 @@ int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)
 }
 
 /**
- * nand_markbad_bbt - [NAND Interface] Mark a block bad in the BBT
+ * nand_bbt_update_mark - update mark in the BBT
  * @mtd: MTD device structure
  * @offs: offset of the bad block
+ * @mark: block type mark
  */
-int nand_markbad_bbt(struct mtd_info *mtd, loff_t offs)
+static int nand_bbt_update_mark(struct mtd_info *mtd, loff_t offs, uint8_t mark)
 {
 	struct nand_chip *this = mtd_to_nand(mtd);
 	int block, ret = 0;
@@ -1365,7 +1366,7 @@ int nand_markbad_bbt(struct mtd_info *mtd, loff_t offs)
 	block = (int)(offs >> this->bbt_erase_shift);
 
 	/* Mark bad block in memory */
-	bbt_mark_entry(this, block, BBT_BLOCK_WORN);
+	bbt_mark_entry(this, block, mark);
 
 	/* Update flash-based bad block table */
 	if (this->bbt_options & NAND_BBT_USE_FLASH)
@@ -1374,6 +1375,15 @@ int nand_markbad_bbt(struct mtd_info *mtd, loff_t offs)
 	return ret;
 }
 
+/**
+ * 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)
+{
+	return nand_bbt_update_mark(mtd, offs, BBT_BLOCK_WORN);
+}
 EXPORT_SYMBOL(nand_scan_bbt);
 
 /**
@@ -1446,3 +1456,19 @@ int nand_bbt_markbad(struct nand_bbt *bbt, loff_t offs)
 	return nand_markbad_bbt(bbt->mtd, offs);
 }
 EXPORT_SYMBOL(nand_bbt_markbad);
+
+/**
+ * nand_bbt_markbad_factory - [NAND BBT Interface] Mark a block as factory bad
+ * in the BBT
+ * @bbt: NAND BBT structure
+ * @offs: offset of the bad block
+ */
+int nand_bbt_markbad_factory(struct nand_bbt *bbt, loff_t offs)
+{
+	/*
+	 * FIXME: For now, we call nand_markbad_bbt() directly. It will change
+	 * when we use struct nand_bbt instead of struct nand_chip.
+	 */
+	return nand_bbt_update_mark(bbt->mtd, offs, BBT_BLOCK_FACTORY_BAD);
+}
+EXPORT_SYMBOL(nand_bbt_markbad_factory);
diff --git a/include/linux/mtd/nand_bbt.h b/include/linux/mtd/nand_bbt.h
index 150c49a..48a0620 100644
--- a/include/linux/mtd/nand_bbt.h
+++ b/include/linux/mtd/nand_bbt.h
@@ -179,5 +179,6 @@ void nand_bbt_release(struct nand_bbt *bbt);
 int nand_bbt_markbad(struct nand_bbt *bbt, loff_t offs);
 int nand_bbt_isreserved(struct nand_bbt *bbt, loff_t offs);
 int nand_bbt_isbad(struct nand_bbt *bbt, loff_t offs);
+int nand_bbt_markbad_factory(struct nand_bbt *bbt, loff_t offs);
 
 #endif	/* __LINUX_MTD_NAND_BBT_H */
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291886 — [PATCH v2 02/12] mtd: nand_bbt: introduce struct nand_bbt

From: Brian Norris <computersforpeace@gmail.com>

Currently nand_bbt.c is tied with struct nand_chip, and it makes other
NAND family chips hard to use nand_bbt.c. Maybe it's the reason why
onenand has own bbt(onenand_bbt.c).

Separate struct nand_chip from BBT code can make current BBT shareable.
We create struct nand_bbt to take place of nand_chip in nand_bbt.c

Below is mtd folder structure we want:
	mtd
	├── Kconfig
	├── Makefile
	├── ...
	├── nand_bbt.c
	├── nand
	│   ├── Kconfig
	│   ├── Makefile
	│   ├── nand_base.c
	│   ├── nand_ids.c
	│   ├── ...
	│   └── xway_nand.c
	├── spi-nand
	│   ├── Kconfig
	│   ├── Makefile
	│   ├── spi-nand-base.c
	│   ├── ...
	│   └── spi-nand-device.c
	└── ...

We put every information nand_bbt.c needed from outside into struct
nand_bbt, include:
	@mtd:	pointer to MTD device structure
	@is_bad_bbm:	check if a block is factory bad block
	@mark_bad_bbm:	imitate a block as factory bad block
	@erase:	erase block bypassing resvered checks
	@bbt_options:	bad block specific options. All options used
			here must come from nand_bbt.h.
	@numchips:	number of physical chips, required for NAND_BBT_PERCHIP
	@bbt_td:	bad block table descriptor for flash lookup.
	@bbt_md:	bad block table mirror descriptor
	@chipsize:	the size of one chip for multichip arrays
	@chip_shift:	number of address bits in one chip
	@bbt_erase_shift:	number of address bits in a bbt entry
	@page_shift:	number of address bits in a page
	@bbt:	bad block table pointer

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
[Peter: correct comment style]
Signed-off-by: Peter Pan <peterpandong@micron.com>
---
 include/linux/mtd/nand_bbt.h | 59 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/include/linux/mtd/nand_bbt.h b/include/linux/mtd/nand_bbt.h
index 5a65230..e468571 100644
--- a/include/linux/mtd/nand_bbt.h
+++ b/include/linux/mtd/nand_bbt.h
@@ -18,6 +18,8 @@
 #ifndef __LINUX_MTD_NAND_BBT_H
 #define __LINUX_MTD_NAND_BBT_H
 
+struct mtd_info;
+
 /* The maximum number of NAND chips in an array */
 #define NAND_MAX_CHIPS		8
 
@@ -115,4 +117,61 @@ struct nand_bbt_descr {
 /* The maximum number of blocks to scan for a bbt */
 #define NAND_BBT_SCAN_MAXBLOCKS	4
 
+/**
+ * struct nand_bbt - bad block table structure
+ * @mtd:	pointer to MTD device structure
+ * @is_bad_bbm:	check if a block is factory bad block
+ * @mark_bad_bbm:	imitate a block as factory bad block
+ * @erase:	erase block bypassing resvered checks
+ * @bbt_options:	bad block specific options. All options used
+ *			here must come from nand_bbt.h.
+ * @numchips:	number of physical chips, required for NAND_BBT_PERCHIP
+ * @bbt_td:	bad block table descriptor for flash lookup.
+ * @bbt_md:	bad block table mirror descriptor
+ * @chipsize:	the size of one chip for multichip arrays
+ * @chip_shift:	number of address bits in one chip
+ * @bbt_erase_shift:	number of address bits in a bbt entry
+ * @page_shift:	number of address bits in a page
+ * @bbt:	bad block table pointer
+ *
+ */
+struct nand_bbt {
+	struct mtd_info *mtd;
+
+	/*
+	 * This is important to abstract out of nand_bbt.c and provide
+	 * separately in nand_base.c and spi-nand-base.c -- it's sort of
+	 * duplicated in nand_block_bad() (nand_base) and
+	 * scan_block_fast() (nand_bbt) right now
+	 *
+	 * Note that this also means nand_chip.badblock_pattern should
+	 * be removed from nand_bbt.c
+	 */
+	int (*is_bad_bbm)(struct mtd_info *mtd, loff_t ofs);
+
+	/*
+	 * Only required if the driver wants to attempt to program new
+	 * bad block markers that imitate the factory-marked BBMs
+	 */
+	int (*mark_bad_bbm)(struct mtd_info *mtd, loff_t ofs);
+
+	/* Erase a block, bypassing reserved checks */
+	int (*erase)(struct mtd_info *mtd, loff_t ofs);
+
+	unsigned int bbt_options;
+	int numchips;
+
+	/*
+	 * Discourage new custom usages here; suggest usage of the
+	 * relevant NAND_BBT_* options instead
+	 */
+	struct nand_bbt_descr *bbt_td;
+	struct nand_bbt_descr *bbt_md;
+	u64 chipsize;
+	int chip_shift;
+	int bbt_erase_shift;
+	int page_shift;
+	u8 *bbt;
+};
+
 #endif	/* __LINUX_MTD_NAND_BBT_H */
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1298748

This is looking a lot better, thanks for the good work!

On 15 December 2015 at 02:59, Peter Pan <peterpansjtu@gmail.com> wrote:
> Currently nand_bbt.c is tied with struct nand_chip, and it makes other
> NAND family chips hard to use nand_bbt.c. Maybe it's the reason why
> onenand has own bbt(onenand_bbt.c).
>
> Separate struct nand_chip from BBT code can make current BBT shareable.
> We create struct nand_bbt to take place of nand_chip in nand_bbt.c.
> Struct nand_bbt contains all the information BBT needed from outside and
> it should be embedded into NAND family chip struct (such as struct nand_chip).
> NAND family driver should allocate, initialize and free struct nand_bbt.
>
> Below is mtd folder structure we want:
>         mtd
>         ├── Kconfig
>         ├── Makefile
>         ├── ...
>         ├── nand_bbt.c

Hm.. I'm not sure about having nand_bbt.c in drivers/mtd.
What's wrong with drivers/mtd/nand ?

In fact, I  was thinking we could go further and clean up the directories a bit
by separating core code, from controllers code, from SPI NAND code:

drivers/mtd/nand/
drivers/mtd/nand/controllers
drivers/mtd/nand/spi

Makes any sense?
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1298922

Hi,

On Mon, 28 Dec 2015 17:42:50 -0300
Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:

> This is looking a lot better, thanks for the good work!
> 
> On 15 December 2015 at 02:59, Peter Pan <peterpansjtu@gmail.com> wrote:
> > Currently nand_bbt.c is tied with struct nand_chip, and it makes other
> > NAND family chips hard to use nand_bbt.c. Maybe it's the reason why
> > onenand has own bbt(onenand_bbt.c).
> >
> > Separate struct nand_chip from BBT code can make current BBT shareable.
> > We create struct nand_bbt to take place of nand_chip in nand_bbt.c.
> > Struct nand_bbt contains all the information BBT needed from outside and
> > it should be embedded into NAND family chip struct (such as struct nand_chip).
> > NAND family driver should allocate, initialize and free struct nand_bbt.
> >
> > Below is mtd folder structure we want:
> >         mtd
> >         ├── Kconfig
> >         ├── Makefile
> >         ├── ...
> >         ├── nand_bbt.c
> 
> Hm.. I'm not sure about having nand_bbt.c in drivers/mtd.
> What's wrong with drivers/mtd/nand ?

I haven't reviewed the series yet, but I agree. If the BBT code is only
meant to be used on NAND based devices, it should probably stay in
drivers/mtd/nand.

> 
> In fact, I  was thinking we could go further and clean up the directories a bit
> by separating core code, from controllers code, from SPI NAND code:
> 
> drivers/mtd/nand/
> drivers/mtd/nand/controllers
> drivers/mtd/nand/spi
> 
> Makes any sense?

Actually I had the secret plan of moving all (raw) NAND controller
drivers into the drivers/mtd/nand/controllers directory, though this
was for a different reason: I'd like to create another directory for
manufacturer specific code in order to support some advanced features
on NANDs that do not implement (or only partially implement) the ONFI
standard.

The separation you're talking about here is more related to the
interface used to communicate with the NAND chip.

How about using the following hierarchy?

drivers/mtd/nand/<nand-core-code>
drivers/mtd/nand/interfaces/raw/<raw-nand-core-code>
drivers/mtd/nand/interfaces/raw/controllers/<raw-nand-controller-drivers>
drivers/mtd/nand/interfaces/spi/<spi-nand-code>
drivers/mtd/nand/interfaces/onenand/<onenand-code>
drivers/mtd/nand/chips/<manufacturer-spcific-code>

What do you think?

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1299032

On 29 December 2015 at 06:35, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> Hi,
>
> On Mon, 28 Dec 2015 17:42:50 -0300
> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:
>
>> This is looking a lot better, thanks for the good work!
>>
>> On 15 December 2015 at 02:59, Peter Pan <peterpansjtu@gmail.com> wrote:
>> > Currently nand_bbt.c is tied with struct nand_chip, and it makes other
>> > NAND family chips hard to use nand_bbt.c. Maybe it's the reason why
>> > onenand has own bbt(onenand_bbt.c).
>> >
>> > Separate struct nand_chip from BBT code can make current BBT shareable.
>> > We create struct nand_bbt to take place of nand_chip in nand_bbt.c.
>> > Struct nand_bbt contains all the information BBT needed from outside and
>> > it should be embedded into NAND family chip struct (such as struct nand_chip).
>> > NAND family driver should allocate, initialize and free struct nand_bbt.
>> >
>> > Below is mtd folder structure we want:
>> >         mtd
>> >         ├── Kconfig
>> >         ├── Makefile
>> >         ├── ...
>> >         ├── nand_bbt.c
>>
>> Hm.. I'm not sure about having nand_bbt.c in drivers/mtd.
>> What's wrong with drivers/mtd/nand ?
>
> I haven't reviewed the series yet, but I agree. If the BBT code is only
> meant to be used on NAND based devices, it should probably stay in
> drivers/mtd/nand.
>
>>
>> In fact, I  was thinking we could go further and clean up the directories a bit
>> by separating core code, from controllers code, from SPI NAND code:
>>
>> drivers/mtd/nand/
>> drivers/mtd/nand/controllers
>> drivers/mtd/nand/spi
>>
>> Makes any sense?
>
> Actually I had the secret plan of moving all (raw) NAND controller
> drivers into the drivers/mtd/nand/controllers directory, though this
> was for a different reason: I'd like to create another directory for
> manufacturer specific code in order to support some advanced features
> on NANDs that do not implement (or only partially implement) the ONFI
> standard.
>
> The separation you're talking about here is more related to the
> interface used to communicate with the NAND chip.
>
> How about using the following hierarchy?
>
> drivers/mtd/nand/<nand-core-code>
> drivers/mtd/nand/interfaces/raw/<raw-nand-core-code>
> drivers/mtd/nand/interfaces/raw/controllers/<raw-nand-controller-drivers>
> drivers/mtd/nand/interfaces/spi/<spi-nand-code>
> drivers/mtd/nand/interfaces/onenand/<onenand-code>
> drivers/mtd/nand/chips/<manufacturer-spcific-code>
>
> What do you think?
>

I believe we are bikeshedding here, but what the heck.

That seems too involved. A simpler hierarchy could be clear enough,
and seems to follow what other subsystems do:

drivers/mtd/nand/<all-nand-core-code>
drivers/mtd/nand/raw/<raw-nand-controller-drivers>
drivers/mtd/nand/spi/<spi-nand-code>
drivers/mtd/nand/onenand/<onenand-code>
drivers/mtd/nand/chips/<manufacturer-spcific-code>

-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1299035

On Tue, 29 Dec 2015 12:07:50 -0300
Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:

> On 29 December 2015 at 06:35, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > Hi,
> >
> > On Mon, 28 Dec 2015 17:42:50 -0300
> > Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:
> >
> >> This is looking a lot better, thanks for the good work!
> >>
> >> On 15 December 2015 at 02:59, Peter Pan <peterpansjtu@gmail.com> wrote:
> >> > Currently nand_bbt.c is tied with struct nand_chip, and it makes other
> >> > NAND family chips hard to use nand_bbt.c. Maybe it's the reason why
> >> > onenand has own bbt(onenand_bbt.c).
> >> >
> >> > Separate struct nand_chip from BBT code can make current BBT shareable.
> >> > We create struct nand_bbt to take place of nand_chip in nand_bbt.c.
> >> > Struct nand_bbt contains all the information BBT needed from outside and
> >> > it should be embedded into NAND family chip struct (such as struct nand_chip).
> >> > NAND family driver should allocate, initialize and free struct nand_bbt.
> >> >
> >> > Below is mtd folder structure we want:
> >> >         mtd
> >> >         ├── Kconfig
> >> >         ├── Makefile
> >> >         ├── ...
> >> >         ├── nand_bbt.c
> >>
> >> Hm.. I'm not sure about having nand_bbt.c in drivers/mtd.
> >> What's wrong with drivers/mtd/nand ?
> >
> > I haven't reviewed the series yet, but I agree. If the BBT code is only
> > meant to be used on NAND based devices, it should probably stay in
> > drivers/mtd/nand.
> >
> >>
> >> In fact, I  was thinking we could go further and clean up the directories a bit
> >> by separating core code, from controllers code, from SPI NAND code:
> >>
> >> drivers/mtd/nand/
> >> drivers/mtd/nand/controllers
> >> drivers/mtd/nand/spi
> >>
> >> Makes any sense?
> >
> > Actually I had the secret plan of moving all (raw) NAND controller
> > drivers into the drivers/mtd/nand/controllers directory, though this
> > was for a different reason: I'd like to create another directory for
> > manufacturer specific code in order to support some advanced features
> > on NANDs that do not implement (or only partially implement) the ONFI
> > standard.
> >
> > The separation you're talking about here is more related to the
> > interface used to communicate with the NAND chip.
> >
> > How about using the following hierarchy?
> >
> > drivers/mtd/nand/<nand-core-code>
> > drivers/mtd/nand/interfaces/raw/<raw-nand-core-code>
> > drivers/mtd/nand/interfaces/raw/controllers/<raw-nand-controller-drivers>
> > drivers/mtd/nand/interfaces/spi/<spi-nand-code>
> > drivers/mtd/nand/interfaces/onenand/<onenand-code>
> > drivers/mtd/nand/chips/<manufacturer-spcific-code>
> >
> > What do you think?
> >
> 
> I believe we are bikeshedding here, but what the heck.
> 
> That seems too involved. A simpler hierarchy could be clear enough,
> and seems to follow what other subsystems do:
> 
> drivers/mtd/nand/<all-nand-core-code>
> drivers/mtd/nand/raw/<raw-nand-controller-drivers>

And probably some common logic in there too.

> drivers/mtd/nand/spi/<spi-nand-code>
> drivers/mtd/nand/onenand/<onenand-code>
> drivers/mtd/nand/chips/<manufacturer-spcific-code>
> 

I'm fine with this one too ;-).

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1299262

Hi Boris and Ezequiel,

2015-12-29 23:11 GMT+08:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> On Tue, 29 Dec 2015 12:07:50 -0300
> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:
>
>> On 29 December 2015 at 06:35, Boris Brezillon
>> <boris.brezillon@free-electrons.com> wrote:
>> > Hi,
>> >
>> > On Mon, 28 Dec 2015 17:42:50 -0300
>> > Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:
>> >
>> >> This is looking a lot better, thanks for the good work!
>> >>
>> >> On 15 December 2015 at 02:59, Peter Pan <peterpansjtu@gmail.com> wrote:
>> >> > Currently nand_bbt.c is tied with struct nand_chip, and it makes other
>> >> > NAND family chips hard to use nand_bbt.c. Maybe it's the reason why
>> >> > onenand has own bbt(onenand_bbt.c).
>> >> >
>> >> > Separate struct nand_chip from BBT code can make current BBT shareable.
>> >> > We create struct nand_bbt to take place of nand_chip in nand_bbt.c.
>> >> > Struct nand_bbt contains all the information BBT needed from outside and
>> >> > it should be embedded into NAND family chip struct (such as struct nand_chip).
>> >> > NAND family driver should allocate, initialize and free struct nand_bbt.
>> >> >
>> >> > Below is mtd folder structure we want:
>> >> >         mtd
>> >> >         ├── Kconfig
>> >> >         ├── Makefile
>> >> >         ├── ...
>> >> >         ├── nand_bbt.c
>> >>
>> >> Hm.. I'm not sure about having nand_bbt.c in drivers/mtd.
>> >> What's wrong with drivers/mtd/nand ?
>> >
>> > I haven't reviewed the series yet, but I agree. If the BBT code is only
>> > meant to be used on NAND based devices, it should probably stay in
>> > drivers/mtd/nand.
>> >
>> >>
>> >> In fact, I  was thinking we could go further and clean up the directories a bit
>> >> by separating core code, from controllers code, from SPI NAND code:
>> >>
>> >> drivers/mtd/nand/
>> >> drivers/mtd/nand/controllers
>> >> drivers/mtd/nand/spi
>> >>
>> >> Makes any sense?
>> >
>> > Actually I had the secret plan of moving all (raw) NAND controller
>> > drivers into the drivers/mtd/nand/controllers directory, though this
>> > was for a different reason: I'd like to create another directory for
>> > manufacturer specific code in order to support some advanced features
>> > on NANDs that do not implement (or only partially implement) the ONFI
>> > standard.
>> >
>> > The separation you're talking about here is more related to the
>> > interface used to communicate with the NAND chip.
>> >
>> > How about using the following hierarchy?
>> >
>> > drivers/mtd/nand/<nand-core-code>
>> > drivers/mtd/nand/interfaces/raw/<raw-nand-core-code>
>> > drivers/mtd/nand/interfaces/raw/controllers/<raw-nand-controller-drivers>
>> > drivers/mtd/nand/interfaces/spi/<spi-nand-code>
>> > drivers/mtd/nand/interfaces/onenand/<onenand-code>
>> > drivers/mtd/nand/chips/<manufacturer-spcific-code>
>> >
>> > What do you think?
>> >
>>
>> I believe we are bikeshedding here, but what the heck.
>>
>> That seems too involved. A simpler hierarchy could be clear enough,
>> and seems to follow what other subsystems do:
>>
>> drivers/mtd/nand/<all-nand-core-code>
>> drivers/mtd/nand/raw/<raw-nand-controller-drivers>
>
> And probably some common logic in there too.
>
>> drivers/mtd/nand/spi/<spi-nand-code>
>> drivers/mtd/nand/onenand/<onenand-code>
>> drivers/mtd/nand/chips/<manufacturer-spcific-code>
>>
>
> I'm fine with this one too ;-).

I'm fine with this structure too. drivers/mtd/nand folder becomes top folder for
all NAND based devices. Because (raw)NAND, SPI-NAND and ONENAND have
different command set and feature, each has its own core - nand_base.c
spi-nand-base.c
and onenand_base.c. So maybe it'll take a lot effort to abstract a
all-nand-core-code
(of course BBT should be one of them). What's your opinion?

Also, please review the BBT patch if you have time. I think it's
helpful on the new NAND code
hierarchy.

>
> --
> Boris Brezillon, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

Thanks
Peter Pan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1299266

Hi Peter,

On Wed, 30 Dec 2015 15:18:39 +0800
潘栋 <peterpansjtu@gmail.com> wrote:

> Hi Boris and Ezequiel,
> 
> 2015-12-29 23:11 GMT+08:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> > On Tue, 29 Dec 2015 12:07:50 -0300
> > Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:
> >
> >> On 29 December 2015 at 06:35, Boris Brezillon
> >> <boris.brezillon@free-electrons.com> wrote:
> >> > Hi,
> >> >
> >> > On Mon, 28 Dec 2015 17:42:50 -0300
> >> > Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:
> >> >
> >> >> This is looking a lot better, thanks for the good work!
> >> >>
> >> >> On 15 December 2015 at 02:59, Peter Pan <peterpansjtu@gmail.com> wrote:
> >> >> > Currently nand_bbt.c is tied with struct nand_chip, and it makes other
> >> >> > NAND family chips hard to use nand_bbt.c. Maybe it's the reason why
> >> >> > onenand has own bbt(onenand_bbt.c).
> >> >> >
> >> >> > Separate struct nand_chip from BBT code can make current BBT shareable.
> >> >> > We create struct nand_bbt to take place of nand_chip in nand_bbt.c.
> >> >> > Struct nand_bbt contains all the information BBT needed from outside and
> >> >> > it should be embedded into NAND family chip struct (such as struct nand_chip).
> >> >> > NAND family driver should allocate, initialize and free struct nand_bbt.
> >> >> >
> >> >> > Below is mtd folder structure we want:
> >> >> >         mtd
> >> >> >         ├── Kconfig
> >> >> >         ├── Makefile
> >> >> >         ├── ...
> >> >> >         ├── nand_bbt.c
> >> >>
> >> >> Hm.. I'm not sure about having nand_bbt.c in drivers/mtd.
> >> >> What's wrong with drivers/mtd/nand ?
> >> >
> >> > I haven't reviewed the series yet, but I agree. If the BBT code is only
> >> > meant to be used on NAND based devices, it should probably stay in
> >> > drivers/mtd/nand.
> >> >
> >> >>
> >> >> In fact, I  was thinking we could go further and clean up the directories a bit
> >> >> by separating core code, from controllers code, from SPI NAND code:
> >> >>
> >> >> drivers/mtd/nand/
> >> >> drivers/mtd/nand/controllers
> >> >> drivers/mtd/nand/spi
> >> >>
> >> >> Makes any sense?
> >> >
> >> > Actually I had the secret plan of moving all (raw) NAND controller
> >> > drivers into the drivers/mtd/nand/controllers directory, though this
> >> > was for a different reason: I'd like to create another directory for
> >> > manufacturer specific code in order to support some advanced features
> >> > on NANDs that do not implement (or only partially implement) the ONFI
> >> > standard.
> >> >
> >> > The separation you're talking about here is more related to the
> >> > interface used to communicate with the NAND chip.
> >> >
> >> > How about using the following hierarchy?
> >> >
> >> > drivers/mtd/nand/<nand-core-code>
> >> > drivers/mtd/nand/interfaces/raw/<raw-nand-core-code>
> >> > drivers/mtd/nand/interfaces/raw/controllers/<raw-nand-controller-drivers>
> >> > drivers/mtd/nand/interfaces/spi/<spi-nand-code>
> >> > drivers/mtd/nand/interfaces/onenand/<onenand-code>
> >> > drivers/mtd/nand/chips/<manufacturer-spcific-code>
> >> >
> >> > What do you think?
> >> >
> >>
> >> I believe we are bikeshedding here, but what the heck.
> >>
> >> That seems too involved. A simpler hierarchy could be clear enough,
> >> and seems to follow what other subsystems do:
> >>
> >> drivers/mtd/nand/<all-nand-core-code>
> >> drivers/mtd/nand/raw/<raw-nand-controller-drivers>
> >
> > And probably some common logic in there too.
> >
> >> drivers/mtd/nand/spi/<spi-nand-code>
> >> drivers/mtd/nand/onenand/<onenand-code>
> >> drivers/mtd/nand/chips/<manufacturer-spcific-code>
> >>
> >
> > I'm fine with this one too ;-).
> 
> I'm fine with this structure too. drivers/mtd/nand folder becomes top folder for
> all NAND based devices. Because (raw)NAND, SPI-NAND and ONENAND have
> different command set and feature, each has its own core - nand_base.c
> spi-nand-base.c
> and onenand_base.c. So maybe it'll take a lot effort to abstract a
> all-nand-core-code
> (of course BBT should be one of them). What's your opinion?

Absolutely, that was the idea: move everything into the
drivers/mtd/nand directory (with the structure described above), keep
some specific logic for each interface type, and see if we can factor
out some common code (I noticed that SPI NAND devices have a parameter
page which looks similar to the one exposed by ONFI compliant devices,
except this parameter page is retrieved using a different command, the
same goes for the ->{set,get}_features() functions).
But let's focus on the nand_bbt code for now.

> 
> Also, please review the BBT patch if you have time. I think it's
> helpful on the new NAND code
> hierarchy.

I'll try to review it this week.

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web