For ONFI-compliant NAND devices, the ONFI parameters report the maximum number
of bad blocks per LUN that will be encountered over the lifetime of the device,
so we can use that information to get a more accurate (and smaller) value for
the UBI bad PEB limit.
The ONFI parameter "maxiumum number of bad blocks per LUN" is the max number of
bad blocks that each individual LUN will ever ecounter. It is not the number of
bad blocks to reserve for the nand device per LUN in the device.
This means that in the worst case a UBI device spanning X LUNs will encounter
"maximum number of bad blocks per LUN" * X bad blocks. The implementation in
this patch assumes this worst case and allocates bad block accordingly.
These patches are ordered in terms of their dependencies, but ideally, all 5
would need to be applied for this to work as intended.
v2:
* Changed commit message to address concerns from v1[1] about this patch set
making best case assumptions.
v3:
* Provided helper function for _max_bad_blocks
* Two new patches
* First new patch adds bb_per_lun and blocks_per_lun to nand_chip struct
* Second new patch sets the new fields during nand_flash_detect_onfi
* Max bad blocks calculation now uses the new nand_chip fields
v4:
* Changed bb_per_lun and blocks_per_lun to bb_per_die and blocks_per_die
* Corrected type of bb_per_die and blocks_per_die from little endian to host
unsigned int
v5:
* Changed bb_per_die to max_bb_per_die
* Fixed spacing style issue
v6:
* Moved bounds checking from "part_max_bad_blocks" to "mtd_max_bad_blocks"
* Added check for 'ofs < 0' to bounds checking mentioned in change above.
* Moved assignment of slave->mtd._max_bad_blocks up to be next to the other
bad-block-related assignments
* s/lun/die in "nand_max_bad_blocks"
* Fixed comment style in "nand_max_bad_blocks"
* In "get_bad_peb_limit" made call to "mtd_max_bad_blocks" occurr only if
!max_beb_per1024. This makes the ordering the three different ways to set
the bad_peb_limit: cmdline > Kconfig > automatic with mtd_max_bad_blocks
Jeff Westfahl (2):
mtd: introduce function max_bad_blocks
mtd: ubi: use 'max_bad_blocks' to compute bad_peb_limit if available
Zach Brown (3):
mtd: nand: Add max_bb_per_die and blocks_per_die fields to nand_chip
mtd: nand: implement 'max_bad_blocks' mtd function
mtd: nand: set max_bb_per_die and blocks_per_die for ONFI compliant
chips
drivers/mtd/mtdpart.c | 10 ++++++++++
drivers/mtd/nand/nand_base.c | 40 ++++++++++++++++++++++++++++++++++++++++
drivers/mtd/ubi/build.c | 13 +++++++++++--
include/linux/mtd/mtd.h | 14 ++++++++++++++
include/linux/mtd/nand.h | 5 +++++
5 files changed, 80 insertions(+), 2 deletions(-)
--
2.7.4
From: Jeff Westfahl <[email protected]>
If implemented, 'max_bad_blocks' returns the maximum number of bad
blocks to reserve for a MTD. An implementation for NAND is coming soon.
Signed-off-by: Jeff Westfahl <[email protected]>
Signed-off-by: Zach Brown <[email protected]>
Acked-by: Boris Brezillon <[email protected]>
Acked-by: Brian Norris <[email protected]>
---
drivers/mtd/mtdpart.c | 10 ++++++++++
include/linux/mtd/mtd.h | 14 ++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index fccdd49..08925bb 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -349,6 +349,14 @@ static const struct mtd_ooblayout_ops part_ooblayout_ops = {
.free = part_ooblayout_free,
};
+static int part_max_bad_blocks(struct mtd_info *mtd, loff_t ofs, size_t len)
+{
+ struct mtd_part *part = mtd_to_part(mtd);
+
+ return part->master->_max_bad_blocks(part->master,
+ ofs + part->offset, len);
+}
+
static inline void free_partition(struct mtd_part *p)
{
kfree(p->mtd.name);
@@ -475,6 +483,8 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
slave->mtd._block_isbad = part_block_isbad;
if (master->_block_markbad)
slave->mtd._block_markbad = part_block_markbad;
+ if (master->_max_bad_blocks)
+ slave->mtd._max_bad_blocks = part_max_bad_blocks;
if (master->_get_device)
slave->mtd._get_device = part_get_device;
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 13f8052..da6d762 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -322,6 +322,7 @@ struct mtd_info {
int (*_block_isreserved) (struct mtd_info *mtd, loff_t ofs);
int (*_block_isbad) (struct mtd_info *mtd, loff_t ofs);
int (*_block_markbad) (struct mtd_info *mtd, loff_t ofs);
+ int (*_max_bad_blocks) (struct mtd_info *mtd, loff_t ofs, size_t len);
int (*_suspend) (struct mtd_info *mtd);
void (*_resume) (struct mtd_info *mtd);
void (*_reboot) (struct mtd_info *mtd);
@@ -402,6 +403,19 @@ int mtd_wunit_to_pairing_info(struct mtd_info *mtd, int wunit,
int mtd_pairing_info_to_wunit(struct mtd_info *mtd,
const struct mtd_pairing_info *info);
int mtd_pairing_groups(struct mtd_info *mtd);
+
+static inline int mtd_max_bad_blocks(struct mtd_info *mtd,
+ loff_t ofs, size_t len)
+{
+ if (!mtd->_max_bad_blocks)
+ return -ENOTSUPP;
+
+ if (mtd->size < (len + ofs) || ofs < 0)
+ return -EINVAL;
+
+ return mtd->_max_bad_blocks(mtd, ofs, len);
+}
+
int mtd_erase(struct mtd_info *mtd, struct erase_info *instr);
int mtd_point(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
void **virt, resource_size_t *phys);
--
2.7.4
From: Jeff Westfahl <[email protected]>
If the user has not set max_beb_per1024 using either the cmdline or
Kconfig options for doing so, use the MTD function 'max_bad_blocks' to
compute the UBI bad_peb_limit.
Signed-off-by: Jeff Westfahl <[email protected]>
Signed-off-by: Zach Brown <[email protected]>
Acked-by: Boris Brezillon <[email protected]>
---
drivers/mtd/ubi/build.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 85d54f3..3029219 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -584,8 +584,17 @@ static int get_bad_peb_limit(const struct ubi_device *ubi, int max_beb_per1024)
int limit, device_pebs;
uint64_t device_size;
- if (!max_beb_per1024)
- return 0;
+ if (!max_beb_per1024) {
+ /*
+ * Since max_beb_per1024 has not been set by the user in either
+ * the cmdline or Kconfig, use mtd_max_bad_blocks to set the
+ * limit if it is supported by the device.
+ */
+ limit = mtd_max_bad_blocks(ubi->mtd, 0, ubi->mtd->size);
+ if (limit < 0)
+ return 0;
+ return limit;
+ }
/*
* Here we are using size of the entire flash chip and
--
2.7.4
The fields max_bb_per_die and blocks_per_die are useful determining the
number of bad blocks a MTD needs to allocate. How they are set will
depend on if the chip is ONFI, JEDEC or a full-id entry in the nand_ids
table.
Signed-off-by: Zach Brown <[email protected]>
Acked-by: Boris Brezillon <[email protected]>
Acked-by: Brian Norris <[email protected]>
---
include/linux/mtd/nand.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index d8905a2..8e9dce1 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -771,6 +771,9 @@ nand_get_sdr_timings(const struct nand_data_interface *conf)
* supported, 0 otherwise.
* @jedec_params: [INTERN] holds the JEDEC parameter page when JEDEC is
* supported, 0 otherwise.
+ * @max_bb_per_die: [INTERN] the max number of bad blocks each die of a
+ * this nand device will encounter their life times.
+ * @blocks_per_die: [INTERN] The number of PEBs in a die
* @read_retries: [INTERN] the number of read retry modes supported
* @onfi_set_features: [REPLACEABLE] set the features for ONFI nand
* @onfi_get_features: [REPLACEABLE] get the features for ONFI nand
@@ -853,6 +856,8 @@ struct nand_chip {
struct nand_onfi_params onfi_params;
struct nand_jedec_params jedec_params;
};
+ u16 max_bb_per_die;
+ u32 blocks_per_die;
struct nand_data_interface *data_interface;
--
2.7.4
Implement the new mtd function 'max_bad_blocks'. Using the chip's
max_bb_per_die and blocks_per_die fields to determine the maximum bad
blocks to reserve for an MTD.
Signed-off-by: Jeff Westfahl <[email protected]>
Signed-off-by: Zach Brown <[email protected]>
Acked-by: Boris Brezillon <[email protected]>
Acked-by: Brian Norris <[email protected]>
---
drivers/mtd/nand/nand_base.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 3bde96a..b4a7c7f 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3236,6 +3236,42 @@ static int nand_block_markbad(struct mtd_info *mtd, loff_t ofs)
}
/**
+ * nand_max_bad_blocks - [MTD Interface] Max number of bad blocks for an mtd
+ * @mtd: MTD device structure
+ * @ofs: offset relative to mtd start
+ * @len: length of mtd
+ */
+static int nand_max_bad_blocks(struct mtd_info *mtd, loff_t ofs, size_t len)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+ uint32_t part_start_block;
+ uint32_t part_end_block;
+ uint32_t part_start_die;
+ uint32_t part_end_die;
+
+ /*
+ * max_bb_per_die and blocks_per_die used to determine
+ * the maximum bad block count.
+ */
+ if (!chip->max_bb_per_die || !chip->blocks_per_die)
+ return -ENOTSUPP;
+
+ /* Get the start and end of the partition in erase blocks. */
+ part_start_block = mtd_div_by_eb(ofs, mtd);
+ part_end_block = mtd_div_by_eb(len, mtd) + part_start_block - 1;
+
+ /* Get the start and end LUNs of the partition. */
+ part_start_die = part_start_block / chip->blocks_per_die;
+ part_end_die = part_end_block / chip->blocks_per_die;
+
+ /*
+ * Look up the bad blocks per unit and multiply by the number of units
+ * that the partition spans.
+ */
+ return chip->max_bb_per_die * (part_end_die - part_start_die + 1);
+}
+
+/**
* nand_onfi_set_features- [REPLACEABLE] set features for ONFI nand
* @mtd: MTD device structure
* @chip: nand chip info structure
@@ -4767,6 +4803,7 @@ int nand_scan_tail(struct mtd_info *mtd)
mtd->_block_isreserved = nand_block_isreserved;
mtd->_block_isbad = nand_block_isbad;
mtd->_block_markbad = nand_block_markbad;
+ mtd->_max_bad_blocks = nand_max_bad_blocks;
mtd->writebufsize = mtd->writesize;
/*
--
2.7.4
ONFI compliant chips contain the values for the max_bb_per_die and
blocks_per_die fields in the parameter page. When the ONFI paged is
retrieved/parsed the chip's fields are set by the corresponding fields
in the param page.
Signed-off-by: Zach Brown <[email protected]>
Acked-by: Boris Brezillon <[email protected]>
Acked-by: Brian Norris <[email protected]>
---
drivers/mtd/nand/nand_base.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index b4a7c7f..623cdc5 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3601,6 +3601,9 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
chip->bits_per_cell = p->bits_per_cell;
+ chip->max_bb_per_die = le16_to_cpu(p->bb_per_lun);
+ chip->blocks_per_die = le32_to_cpu(p->blocks_per_lun);
+
if (onfi_feature(chip) & ONFI_FEATURE_16_BIT_BUS)
*busw = NAND_BUSWIDTH_16;
else
--
2.7.4
Hi,
On Wed, Nov 30, 2016 at 10:57:27AM -0600, Zach Brown wrote:
> From: Jeff Westfahl <[email protected]>
>
> If implemented, 'max_bad_blocks' returns the maximum number of bad
> blocks to reserve for a MTD. An implementation for NAND is coming soon.
>
> Signed-off-by: Jeff Westfahl <[email protected]>
> Signed-off-by: Zach Brown <[email protected]>
> Acked-by: Boris Brezillon <[email protected]>
> Acked-by: Brian Norris <[email protected]>
I guess I had given my 'ack' conditional on fixing my comments. It
appears they have been fixed well enough. One small nit, but that's not
worth holding anything up:
> ---
> drivers/mtd/mtdpart.c | 10 ++++++++++
> include/linux/mtd/mtd.h | 14 ++++++++++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index fccdd49..08925bb 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -349,6 +349,14 @@ static const struct mtd_ooblayout_ops part_ooblayout_ops = {
> .free = part_ooblayout_free,
> };
>
> +static int part_max_bad_blocks(struct mtd_info *mtd, loff_t ofs, size_t len)
> +{
> + struct mtd_part *part = mtd_to_part(mtd);
> +
> + return part->master->_max_bad_blocks(part->master,
> + ofs + part->offset, len);
> +}
> +
> static inline void free_partition(struct mtd_part *p)
> {
> kfree(p->mtd.name);
> @@ -475,6 +483,8 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
> slave->mtd._block_isbad = part_block_isbad;
> if (master->_block_markbad)
> slave->mtd._block_markbad = part_block_markbad;
> + if (master->_max_bad_blocks)
> + slave->mtd._max_bad_blocks = part_max_bad_blocks;
>
> if (master->_get_device)
> slave->mtd._get_device = part_get_device;
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 13f8052..da6d762 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -322,6 +322,7 @@ struct mtd_info {
> int (*_block_isreserved) (struct mtd_info *mtd, loff_t ofs);
> int (*_block_isbad) (struct mtd_info *mtd, loff_t ofs);
> int (*_block_markbad) (struct mtd_info *mtd, loff_t ofs);
> + int (*_max_bad_blocks) (struct mtd_info *mtd, loff_t ofs, size_t len);
> int (*_suspend) (struct mtd_info *mtd);
> void (*_resume) (struct mtd_info *mtd);
> void (*_reboot) (struct mtd_info *mtd);
> @@ -402,6 +403,19 @@ int mtd_wunit_to_pairing_info(struct mtd_info *mtd, int wunit,
> int mtd_pairing_info_to_wunit(struct mtd_info *mtd,
> const struct mtd_pairing_info *info);
> int mtd_pairing_groups(struct mtd_info *mtd);
> +
> +static inline int mtd_max_bad_blocks(struct mtd_info *mtd,
> + loff_t ofs, size_t len)
> +{
> + if (!mtd->_max_bad_blocks)
> + return -ENOTSUPP;
> +
> + if (mtd->size < (len + ofs) || ofs < 0)
> + return -EINVAL;
> +
> + return mtd->_max_bad_blocks(mtd, ofs, len);
> +}
I don't really know what the reasoning is for putting functions like
this as 'static inline' in the header, vs. exported from the .c file.
But this seems to be the odd man out in that regard.
> +
> int mtd_erase(struct mtd_info *mtd, struct erase_info *instr);
> int mtd_point(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
> void **virt, resource_size_t *phys);
Anyway, it's all fine with me here. I think Richard suggested he'd like
to take the whole series (presuming he's happy with patch 2?) in his
tree, though IMO it's a bit late for 4.10. I guess I'm fine either way,
as it's only UBI that will suffer if this wasn't properly baked ;)
Brian