[PATCH v3 0/5] mtd: use ONFI bad blocks per LUN to calculate UBI bad PEB limit

[PATCH v3 0/5] mtd: use ONFI bad blocks per LUN to calculate UBI bad PEB limit

[Zach Brown]

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.

v1:
 * Changed commit message to address concerns from v1[1] about this patch set
   making best case assumptions.
v2:
 * 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

[1]
http://lkml.iu.edu/hypermail/linux/kernel/1505.1/04822.html

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 bb_per_lun and blocks_per_lun fields to nand_chip
  mtd: nand: implement 'max_bad_blocks' mtd function
  mtd: nand: set bb_per_lun and blocks_per_lun for ONFI compliant chips

 drivers/mtd/mtdpart.c        | 13 +++++++++++++
 drivers/mtd/nand/nand_base.c | 40 ++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/ubi/build.c      |  4 ++++
 include/linux/mtd/mtd.h      | 11 +++++++++++
 include/linux/mtd/nand.h     |  5 +++++
 5 files changed, 73 insertions(+)

-- 
2.7.4

[PATCH v3 1/5] mtd: introduce function max_bad_blocks

[Zach Brown]

From: Jeff Westfahl <jeff.westfahl@ni.com>

If implemented, 'max_bad_blocks' returns the maximum number of bad
blocks to reserve for an MTD. An implementation for NAND is coming soon.

Signed-off-by: Jeff Westfahl <jeff.westfahl@ni.com>
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
 drivers/mtd/mtdpart.c   | 13 +++++++++++++
 include/linux/mtd/mtd.h | 11 +++++++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index fccdd49..2106bd5 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -349,6 +349,16 @@ 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);
+
+	if ((len + ofs) > mtd->size)
+		return -EINVAL;
+	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);
@@ -481,6 +491,9 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
 	if (master->_put_device)
 		slave->mtd._put_device = part_put_device;
 
+	if (master->_max_bad_blocks)
+		slave->mtd._max_bad_blocks = part_max_bad_blocks;
+
 	slave->mtd._erase = part_erase;
 	slave->master = master;
 	slave->offset = part->offset;
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 13f8052..c02d3c2 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,16 @@ 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 mtd->_max_bad_blocks(mtd, ofs, len);
+
+	return -ENOTSUPP;
+}
+
 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

[PATCH v3 2/5] mtd: ubi: use 'max_bad_blocks' to compute bad_peb_limit if available

[Zach Brown]

From: Jeff Westfahl <jeff.westfahl@ni.com>

Use the MTD function 'max_bad_blocks' to compute the UBI bad_peb_limit,
if the function is implemented for an MTD and doesn't return an error.

Signed-off-by: Jeff Westfahl <jeff.westfahl@ni.com>
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
 drivers/mtd/ubi/build.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 85d54f3..e9940a9 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -584,6 +584,10 @@ static int get_bad_peb_limit(const struct ubi_device *ubi, int max_beb_per1024)
 	int limit, device_pebs;
 	uint64_t device_size;
 
+	limit = mtd_max_bad_blocks(ubi->mtd, 0, ubi->mtd->size);
+	if (limit > 0)
+		return limit;
+
 	if (!max_beb_per1024)
 		return 0;
 
-- 
2.7.4

[PATCH v3 3/5] mtd: nand: Add bb_per_lun and blocks_per_lun fields to nand_chip

[Zach Brown]

The fields bb_per_lun and blocks_per_lun 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 fuill-id entry in the nand_ids
table.

Signed-off-by: Zach Brown <zach.brown@ni.com>
---
 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 c5d3d502..efbe439 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.
+ * @bb_per_lun:	[INTERN] the max number of bad blocks each LUN of a
+ *			this nand device will encounter their life times.
+ * @blocks_per_lun:	[INTERN] The number of PEBs in a LUN
  * @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;
 	};
+	__le16 bb_per_lun;
+	__le32 blocks_per_lun;
 
 	struct nand_data_interface *data_interface;
 
-- 
2.7.4

[PATCH v3 4/5] mtd: nand: implement 'max_bad_blocks' mtd function

[Zach Brown]

Implement the new mtd function 'max_bad_blocks'. Using the chip's
bb_per_lun and blocks_per_lun fields to determine the maximum bad blocks
to reserve for an MTD.

Signed-off-by: Jeff Westfahl <jeff.westfahl@ni.com>
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
 drivers/mtd/nand/nand_base.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index e5718e5..ed8dc89 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3226,6 +3226,40 @@ 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_lun;
+	uint32_t part_end_lun;
+
+	/* bb_per_lun and blocks_per_lun used to determine
+	 * the maximum bad block count.
+	 */
+	if (!chip->bb_per_lun || !chip->blocks_per_lun)
+		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_lun = part_start_block / chip->blocks_per_lun;
+	part_end_lun = part_end_block / chip->blocks_per_lun;
+
+	/* Look up the bad blocks per unit and multiply by the number of units
+	 * that the partition spans.
+	 */
+	return chip->bb_per_lun * (part_end_lun - part_start_lun + 1);
+}
+
+/**
  * nand_onfi_set_features- [REPLACEABLE] set features for ONFI nand
  * @mtd: MTD device structure
  * @chip: nand chip info structure
@@ -4743,6 +4777,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

[PATCH v3 5/5] mtd: nand: set bb_per_lun and blocks_per_lun for ONFI compliant chips

[Zach Brown]

ONFI compliant chips contain the values for the bb_per_lun and
blocks_per_lun 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 <zach.brown@ni.com>
---
 drivers/mtd/nand/nand_base.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ed8dc89..c4ced91 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3619,6 +3619,11 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
 	if (p->jedec_id == NAND_MFR_MICRON)
 		nand_onfi_detect_micron(chip, p);
 
+	if (p->bb_per_lun)
+		chip->bb_per_lun = p->bb_per_lun;
+	if (p->blocks_per_lun)
+		chip->blocks_per_lun = p->blocks_per_lun;
+
 	return 1;
 }
 
-- 
2.7.4

Re: [PATCH v3 3/5] mtd: nand: Add bb_per_lun and blocks_per_lun fields to nand_chip

[Boris Brezillon]

On Fri, 28 Oct 2016 15:27:42 -0500
Zach Brown <zach.brown@ni.com> wrote:

> The fields bb_per_lun and blocks_per_lun 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 fuill-id entry in the nand_ids
> table.
> 
> Signed-off-by: Zach Brown <zach.brown@ni.com>
> ---
>  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 c5d3d502..efbe439 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.
> + * @bb_per_lun:	[INTERN] the max number of bad blocks each LUN of a
> + *			this nand device will encounter their life times.
> + * @blocks_per_lun:	[INTERN] The number of PEBs in a LUN
>   * @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;
>  	};
> +	__le16 bb_per_lun;
> +	__le32 blocks_per_lun;

Two things I don't like here:
- you use little-endian types, while it should use native endianness.
  Make it easier, and just declare those fields as int (or unsigned
  int).
- you stick to the ONFI spec, while I'd prefer to see the term lun
  replaced by die, and I wonder if we don't already have a field
  storing the number of blocks per die (I might be wrong though).

>  
>  	struct nand_data_interface *data_interface;
>  

Re: [PATCH v3 3/5] mtd: nand: Add bb_per_lun and blocks_per_lun fields to nand_chip

[Zach Brown]

On Tue, Nov 01, 2016 at 02:50:58PM +0100, Boris Brezillon wrote:
> On Fri, 28 Oct 2016 15:27:42 -0500
> Zach Brown <zach.brown@ni.com> wrote:
>
> > The fields bb_per_lun and blocks_per_lun 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 fuill-id entry in the nand_ids
> > table.
> >
> > Signed-off-by: Zach Brown <zach.brown@ni.com>
> > ---
> >  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 c5d3d502..efbe439 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.
> > + * @bb_per_lun:	[INTERN] the max number of bad blocks each LUN of a
> > + *			this nand device will encounter their life times.
> > + * @blocks_per_lun:	[INTERN] The number of PEBs in a LUN
> >   * @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;
> >  	};
> > +	__le16 bb_per_lun;
> > +	__le32 blocks_per_lun;
>
> Two things I don't like here:
> - you use little-endian types, while it should use native endianness.
>   Make it easier, and just declare those fields as int (or unsigned
>   int).
> - you stick to the ONFI spec, while I'd prefer to see the term lun
>   replaced by die, and I wonder if we don't already have a field
>   storing the number of blocks per die (I might be wrong though).
>

I looked for an existing field for number of blocks per die and could not find
it. Perhaps you were remembering chipsize? or numchips? I looked at how they
were calculated and chipsize is a multiple of (blocks_per_lun * lun_count) so
it can't be used to find blocks_per_lun without storing lun_count anyways.
Let me know if you think I missed something.

Re: [PATCH v3 3/5] mtd: nand: Add bb_per_lun and blocks_per_lun fields to nand_chip

[Boris Brezillon]

On Tue, 1 Nov 2016 13:32:32 -0500
Zach Brown <zach.brown@ni.com> wrote:

> On Tue, Nov 01, 2016 at 02:50:58PM +0100, Boris Brezillon wrote:
> > On Fri, 28 Oct 2016 15:27:42 -0500
> > Zach Brown <zach.brown@ni.com> wrote:
> >  
> > > The fields bb_per_lun and blocks_per_lun 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 fuill-id entry in the nand_ids
> > > table.
> > >
> > > Signed-off-by: Zach Brown <zach.brown@ni.com>
> > > ---
> > >  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 c5d3d502..efbe439 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.
> > > + * @bb_per_lun:	[INTERN] the max number of bad blocks each LUN of a
> > > + *			this nand device will encounter their life times.
> > > + * @blocks_per_lun:	[INTERN] The number of PEBs in a LUN
> > >   * @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;
> > >  	};
> > > +	__le16 bb_per_lun;
> > > +	__le32 blocks_per_lun;  
> >
> > Two things I don't like here:
> > - you use little-endian types, while it should use native endianness.
> >   Make it easier, and just declare those fields as int (or unsigned
> >   int).
> > - you stick to the ONFI spec, while I'd prefer to see the term lun
> >   replaced by die, and I wonder if we don't already have a field
> >   storing the number of blocks per die (I might be wrong though).
> >  
> 
> I looked for an existing field for number of blocks per die and could not find
> it. Perhaps you were remembering chipsize? or numchips? I looked at how they
> were calculated and chipsize is a multiple of (blocks_per_lun * lun_count) so
> it can't be used to find blocks_per_lun without storing lun_count anyways.
> Let me know if you think I missed something.

Probably not, we so many fields in the nand_chip and mtd_info struct
that I don't remember which ones we are missing ;-).