[PATCH 0/4] UBI: Use the whole NAND device to calculate max bad block number

[PATCH 0/4] UBI: Use the whole NAND device to calculate max bad block number

[Richard Genoud]

Hi,

This patch serie applies on top of Shmulik Ladkani's patch serie:
"[PATCH 0/5] ubi: Fix bad PEBs reserve caclulation"
(and on mtd-artem git tree)

The goal here is to make UBI reserve a number of PEB for bad block dependant
of the whole NAND chipset size, and not of the MTD partition size.

As explained in patch 3/4, the NAND manufacturers only tell how many bad blocks
there will be in the worst case on a NAND device, but not that these blocks
will be equally disposed on the flash.
I.E. if a NAND flash admits at max 40 bad blocks, and is cutted in two equal
MTD partitions, it doesn't mean that there will be at max only 20 bad blocks
per partition.
The 40 BEBs could be all on the first partition.

So, for each MTD partition, UBI should reserved the maximum expected number of
bad erase blocks.

Patches 1/2/3 are making that happend with a kernel option MTD_UBI_BEB_LIMIT
Then, patch 4 replace this option with user-space parameters (kernel parameter
and a UBI_IOCATT ioctl)

The patch on mtd-utils follows the serie.

Richard Genoud (4):
  mtd_is_partition: struct mtd_info should be const
  MTD parts: introduce mtd_get_device_size()
  UBI: use the whole MTD device size to get bad_peb_limit
  UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

 drivers/mtd/mtdpart.c          |   16 ++++++++-
 drivers/mtd/ubi/Kconfig        |   19 ----------
 drivers/mtd/ubi/build.c        |   73 ++++++++++++++++++++++++++++++---------
 drivers/mtd/ubi/cdev.c         |    9 ++++-
 drivers/mtd/ubi/ubi.h          |    6 +++-
 include/linux/mtd/partitions.h |    3 +-
 include/mtd/ubi-user.h         |   19 ++++++++++-
 7 files changed, 104 insertions(+), 41 deletions(-)

-- 
1.7.2.5

[PATCH 1/4] mtd_is_partition: struct mtd_info should be const

[Richard Genoud]

struct mtd_info is not modified by mtd_is_partition so it should be a
const.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/mtd/mtdpart.c          |    2 +-
 include/linux/mtd/partitions.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index d518e4d..8500584 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -744,7 +744,7 @@ int parse_mtd_partitions(struct mtd_info *master, const char **types,
 	return ret;
 }
 
-int mtd_is_partition(struct mtd_info *mtd)
+int mtd_is_partition(const struct mtd_info *mtd)
 {
 	struct mtd_part *part;
 	int ispart = 0;
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 2475228..02a5115 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -79,7 +79,7 @@ struct mtd_part_parser {
 extern int register_mtd_parser(struct mtd_part_parser *parser);
 extern int deregister_mtd_parser(struct mtd_part_parser *parser);
 
-int mtd_is_partition(struct mtd_info *mtd);
+int mtd_is_partition(const struct mtd_info *mtd);
 int mtd_add_partition(struct mtd_info *master, char *name,
 		      long long offset, long long length);
 int mtd_del_partition(struct mtd_info *master, int partno);
-- 
1.7.2.5

[PATCH 2/4] MTD parts: introduce mtd_get_device_size()

[Richard Genoud]

mtd_get_device_size() returns the size of the whole MTD device, that is
the mtd_info master size.
This is used by UBI to calculate the maximum number of bad blocks (MBB)
on a MTD device.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/mtd/mtdpart.c          |   14 ++++++++++++++
 include/linux/mtd/partitions.h |    1 +
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 8500584..d12f583 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -760,3 +760,17 @@ int mtd_is_partition(const struct mtd_info *mtd)
 	return ispart;
 }
 EXPORT_SYMBOL_GPL(mtd_is_partition);
+
+/* returns the size of an MTD device */
+uint64_t mtd_get_device_size(const struct mtd_info *mtd)
+{
+	struct mtd_part *part;
+
+	if (!mtd_is_partition(mtd))
+		return mtd->size;
+
+	part = PART(mtd);
+
+	return part->master->size;
+}
+EXPORT_SYMBOL_GPL(mtd_get_device_size);
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 02a5115..1f8d24b 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -83,5 +83,6 @@ int mtd_is_partition(const struct mtd_info *mtd);
 int mtd_add_partition(struct mtd_info *master, char *name,
 		      long long offset, long long length);
 int mtd_del_partition(struct mtd_info *master, int partno);
+uint64_t mtd_get_device_size(const struct mtd_info *mtd);
 
 #endif
-- 
1.7.2.5

[PATCH 3/4] UBI: use the whole MTD device size to get bad_peb_limit

[Richard Genoud]

On NAND flash devices, UBI reserves some physical erase blocks (PEB) for
bad block handling.
Today, the number of reserved PEB can only be set as a percentage of the
total number of PEB in each MTD partition.
For example, for a NAND flash with 128KiB PEB, 2 MTD partition of 20MiB
(mtd0) and 100MiB (mtd1) and 2% reserved PEB:
- the UBI device on mtd0 will have 2 PEB reserved
- the UBI device on mtd1 will have 16 PEB reserved

The problem with this behaviour is that NAND flash manufacturers give a
minimum number of valid block (NVB) during the endurance life of the
device.
E.G.:
Parameter             Symbol    Min    Max    Unit      Notes
--------------------------------------------------------------
Valid block number     NVB     1004    1024   Blocks     1
Note:
1. Invalid blocks are block that contain one or more bad bits beyond
ECC. The device may contain bad blocks upon shipment. Additional bad
blocks may develop over time; however, the total number of available
blocks will not drop below NVB during the endurance life of the device.

>From this number we can deduce the maximum number of bad PEB that a
device will contain during its endurance life :
A 128MiB NAND flash (1024 PEB) will not have less than 20 bad blocks
during the flash endurance life.

BUT, the manufacturer doesn't tell where those bad block will appear. He
doesn't say either if they will be equally disposed on the whole device
(and I'm pretty sure they won't).
So, according to the datasheets, we should reserve the maximum number of
bad PEB for each UBI device.
(Worst case scenario: 20 bad blocks appears on the smallest MTD
partition.)

So this patch make UBI use the whole MTD device size to calculate the
Maximum bad expected eraseblocks.

The Kconfig option is in per1024 blocks, thus it can have a default
value of 20 which is *very* common for NAND devices.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/mtd/ubi/Kconfig |   14 ++++++++++----
 drivers/mtd/ubi/build.c |   17 ++++++++++++++---
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index 145cda2..b77ffe1 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -28,15 +28,21 @@ config MTD_UBI_WL_THRESHOLD
 	  to 128 or 256, although it does not have to be power of 2).
 
 config MTD_UBI_BEB_LIMIT
-	int "Percentage of maximum expected bad eraseblocks"
-	default 2
-	range 0 25
+	int "Maximum expected bad eraseblocks per 1024 eraseblocks"
+	default 20
+	range 2 256
 	help
 	  If the MTD device admits of bad eraseblocks (e.g. NAND flash), UBI
 	  reserves some amount of physical eraseblocks to handle new bad
 	  eraseblocks.
 	  This option specifies the maximum bad eraseblocks UBI expects on the
-	  ubi device (percents of total number of flash eraseblocks).
+	  ubi device per 1024 eraseblocks.
+	  This value is often given in an other form in the NAND datasheet
+	  (min NVB i.e. minimal number of valid blocks). The maximum expected
+	  bad eraseblocks per 1024 is then:
+	    1024 * (1 - MinNVB / MaxNVB)
+	  Which gives 20 for most NAND devices.
+
 	  This limit is used in order to derive amount of eraseblock UBI
 	  reserves for handling new bad blocks.
 	  If the device has more bad eraseblocks than this limit, UBI does not
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 62cc6ce..87b39c2 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -36,6 +36,7 @@
 #include <linux/namei.h>
 #include <linux/stat.h>
 #include <linux/miscdevice.h>
+#include <linux/mtd/partitions.h>
 #include <linux/log2.h>
 #include <linux/kthread.h>
 #include <linux/kernel.h>
@@ -610,12 +611,22 @@ static int io_init(struct ubi_device *ubi)
 	if (mtd_can_have_bb(ubi->mtd)) {
 		ubi->bad_allowed = 1;
 		if (CONFIG_MTD_UBI_BEB_LIMIT > 0) {
-			int percent = CONFIG_MTD_UBI_BEB_LIMIT;
+			int per1024 = CONFIG_MTD_UBI_BEB_LIMIT;
+			int device_peb_count;
+			uint64_t device_size;
 			int beb_limit;
 
-			beb_limit = mult_frac(ubi->peb_count, percent, 100);
+			/* we are using here the whole MTD device size and not
+			 * the MTD partition size because the maximum number of
+			 * bad blocks is a percentage of the whole device and
+			 * the bad blocks are not fairly disposed on a flash
+			 * device
+			 */
+			device_size = mtd_get_device_size(ubi->mtd);
+			device_peb_count = mtd_div_by_eb(device_size, ubi->mtd);
+			beb_limit = mult_frac(device_peb_count, per1024, 1024);
 			/* round it up */
-			if (mult_frac(beb_limit, 100, percent) < ubi->peb_count)
+			if (mult_frac(beb_limit, 1024, per1024) < ubi->peb_count)
 				beb_limit++;
 			ubi->bad_peb_limit = beb_limit;
 		}
-- 
1.7.2.5

[PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Richard Genoud]

This patch provides the possibility to adjust the "maximum expected number of
bad blocks per 1024 blocks" (max_beb_per1024) for each mtd device.

The majority of NAND devices have their max_beb_per1024 equal to 20, but
sometimes it's more.
Now, we can adjust that via a kernel parameter:
ubi.mtd=<name|num|path>[,<vid_hdr_offs>[,max_beb_per1024]]
and via UBI_IOCATT ioctl which is now:
struct ubi_attach_req {
	__s32 ubi_num;
	__s32 mtd_num;
	__s32 vid_hdr_offset;
	__u8 max_beb_per1024;
	__s8 padding[11];
};

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/mtd/ubi/Kconfig |   25 --------------
 drivers/mtd/ubi/build.c |   82 +++++++++++++++++++++++++++++++---------------
 drivers/mtd/ubi/cdev.c  |    9 ++++-
 drivers/mtd/ubi/ubi.h   |    6 +++-
 include/mtd/ubi-user.h  |   19 ++++++++++-
 5 files changed, 86 insertions(+), 55 deletions(-)

diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index b77ffe1..7a57cc0 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -27,31 +27,6 @@ config MTD_UBI_WL_THRESHOLD
 	  life-cycle less than 10000, the threshold should be lessened (e.g.,
 	  to 128 or 256, although it does not have to be power of 2).
 
-config MTD_UBI_BEB_LIMIT
-	int "Maximum expected bad eraseblocks per 1024 eraseblocks"
-	default 20
-	range 2 256
-	help
-	  If the MTD device admits of bad eraseblocks (e.g. NAND flash), UBI
-	  reserves some amount of physical eraseblocks to handle new bad
-	  eraseblocks.
-	  This option specifies the maximum bad eraseblocks UBI expects on the
-	  ubi device per 1024 eraseblocks.
-	  This value is often given in an other form in the NAND datasheet
-	  (min NVB i.e. minimal number of valid blocks). The maximum expected
-	  bad eraseblocks per 1024 is then:
-	    1024 * (1 - MinNVB / MaxNVB)
-	  Which gives 20 for most NAND devices.
-
-	  This limit is used in order to derive amount of eraseblock UBI
-	  reserves for handling new bad blocks.
-	  If the device has more bad eraseblocks than this limit, UBI does not
-	  reserve any physical eraseblocks for new bad eraseblocks, but
-	  attempts to use available eraseblocks (if any).
-	  If the underlying flash does not admit of bad eraseblocks (e.g. NOR
-	  flash), this value is ignored.
-	  Leave the default value if unsure.
-
 config MTD_UBI_GLUEBI
 	tristate "MTD devices emulation driver (gluebi)"
 	help
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 87b39c2..76358e9 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -46,6 +46,8 @@
 /* Maximum length of the 'mtd=' parameter */
 #define MTD_PARAM_LEN_MAX 64
 
+#define MTD_PARAM_NB_MAX 3
+
 #ifdef CONFIG_MTD_UBI_MODULE
 #define ubi_is_module() 1
 #else
@@ -57,10 +59,12 @@
  * @name: MTD character device node path, MTD device name, or MTD device number
  *        string
  * @vid_hdr_offs: VID header offset
+ * @max_beb_per1024: maximum expected number of bad blocks per 1024 erase blocks
  */
 struct mtd_dev_param {
 	char name[MTD_PARAM_LEN_MAX];
 	int vid_hdr_offs;
+	int max_beb_per1024;
 };
 
 /* Numbers of elements set in the @mtd_dev_param array */
@@ -565,9 +569,37 @@ void ubi_free_internal_volumes(struct ubi_device *ubi)
 	}
 }
 
+static int get_bad_peb_limit(const struct ubi_device *ubi, int max_beb_per1024)
+{
+	int device_peb_count;
+	uint64_t device_size;
+	int beb_limit = 0;
+
+	/* this has already been checked in ioctl */
+	if (max_beb_per1024 <= 0)
+		goto out;
+
+	/* we are using here the whole MTD device size and not
+	 * the MTD partition size because the maximum number of
+	 * bad blocks is a percentage of the whole device and
+	 * the bad blocks are not fairly disposed on a flash
+	 * device
+	 */
+	device_size = mtd_get_device_size(ubi->mtd);
+	device_peb_count = mtd_div_by_eb(device_size, ubi->mtd);
+	beb_limit = mult_frac(device_peb_count, max_beb_per1024, 1024);
+	/* round it up */
+	if (mult_frac(beb_limit, 1024, max_beb_per1024) < ubi->peb_count)
+		beb_limit++;
+
+out:
+	return beb_limit;
+}
+
 /**
  * io_init - initialize I/O sub-system for a given UBI device.
  * @ubi: UBI device description object
+ * @max_beb_per1024: maximum expected number of bad PEB per 1024 PEB
  *
  * If @ubi->vid_hdr_offset or @ubi->leb_start is zero, default offsets are
  * assumed:
@@ -580,7 +612,7 @@ void ubi_free_internal_volumes(struct ubi_device *ubi)
  * This function returns zero in case of success and a negative error code in
  * case of failure.
  */
-static int io_init(struct ubi_device *ubi)
+static int io_init(struct ubi_device *ubi, int max_beb_per1024)
 {
 	if (ubi->mtd->numeraseregions != 0) {
 		/*
@@ -610,26 +642,7 @@ static int io_init(struct ubi_device *ubi)
 
 	if (mtd_can_have_bb(ubi->mtd)) {
 		ubi->bad_allowed = 1;
-		if (CONFIG_MTD_UBI_BEB_LIMIT > 0) {
-			int per1024 = CONFIG_MTD_UBI_BEB_LIMIT;
-			int device_peb_count;
-			uint64_t device_size;
-			int beb_limit;
-
-			/* we are using here the whole MTD device size and not
-			 * the MTD partition size because the maximum number of
-			 * bad blocks is a percentage of the whole device and
-			 * the bad blocks are not fairly disposed on a flash
-			 * device
-			 */
-			device_size = mtd_get_device_size(ubi->mtd);
-			device_peb_count = mtd_div_by_eb(device_size, ubi->mtd);
-			beb_limit = mult_frac(device_peb_count, per1024, 1024);
-			/* round it up */
-			if (mult_frac(beb_limit, 1024, per1024) < ubi->peb_count)
-				beb_limit++;
-			ubi->bad_peb_limit = beb_limit;
-		}
+		ubi->bad_peb_limit = get_bad_peb_limit(ubi, max_beb_per1024);
 	}
 
 	if (ubi->mtd->type == MTD_NORFLASH) {
@@ -822,6 +835,7 @@ static int autoresize(struct ubi_device *ubi, int vol_id)
  * @mtd: MTD device description object
  * @ubi_num: number to assign to the new UBI device
  * @vid_hdr_offset: VID header offset
+ * @max_beb_per1024: maximum number of expected bad blocks per 1024 eraseblocks
  *
  * This function attaches MTD device @mtd_dev to UBI and assign @ubi_num number
  * to the newly created UBI device, unless @ubi_num is %UBI_DEV_NUM_AUTO, in
@@ -832,7 +846,8 @@ static int autoresize(struct ubi_device *ubi, int vol_id)
  * Note, the invocations of this function has to be serialized by the
  * @ubi_devices_mutex.
  */
-int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset)
+int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
+		       int vid_hdr_offset, int max_beb_per1024)
 {
 	struct ubi_device *ubi;
 	int i, err, ref = 0;
@@ -905,7 +920,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset)
 	dbg_msg("sizeof(struct ubi_ainf_peb) %zu", sizeof(struct ubi_ainf_peb));
 	dbg_msg("sizeof(struct ubi_wl_entry) %zu", sizeof(struct ubi_wl_entry));
 
-	err = io_init(ubi);
+	err = io_init(ubi, max_beb_per1024);
 	if (err)
 		goto out_free;
 
@@ -1194,7 +1209,7 @@ static int __init ubi_init(void)
 
 		mutex_lock(&ubi_devices_mutex);
 		err = ubi_attach_mtd_dev(mtd, UBI_DEV_NUM_AUTO,
-					 p->vid_hdr_offs);
+					 p->vid_hdr_offs, p->max_beb_per1024);
 		mutex_unlock(&ubi_devices_mutex);
 		if (err < 0) {
 			ubi_err("cannot attach mtd%d", mtd->index);
@@ -1313,7 +1328,7 @@ static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp)
 	struct mtd_dev_param *p;
 	char buf[MTD_PARAM_LEN_MAX];
 	char *pbuf = &buf[0];
-	char *tokens[2] = {NULL, NULL};
+	char *tokens[MTD_PARAM_NB_MAX];
 
 	if (!val)
 		return -EINVAL;
@@ -1343,7 +1358,7 @@ static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp)
 	if (buf[len - 1] == '\n')
 		buf[len - 1] = '\0';
 
-	for (i = 0; i < 2; i++)
+	for (i = 0; i < MTD_PARAM_NB_MAX; i++)
 		tokens[i] = strsep(&pbuf, ",");
 
 	if (pbuf) {
@@ -1361,18 +1376,31 @@ static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp)
 	if (p->vid_hdr_offs < 0)
 		return p->vid_hdr_offs;
 
+	if (tokens[2])
+		p->max_beb_per1024 = bytes_str_to_int(tokens[2]);
+	/* a value of 0 is force to the default value to keep the same
+	 * behavior between ubiattach command and module parameter
+	 */
+	if (!p->max_beb_per1024)
+		p->max_beb_per1024 = MTD_UBI_DEFAULT_BEB_LIMIT;
+
 	mtd_devs += 1;
 	return 0;
 }
 
 module_param_call(mtd, ubi_mtd_param_parse, NULL, NULL, 000);
 MODULE_PARM_DESC(mtd, "MTD devices to attach. Parameter format: "
-		      "mtd=<name|num|path>[,<vid_hdr_offs>].\n"
+		      "mtd=<name|num|path>[,<vid_hdr_offs>[,"
+		      "max_beb_per1024]].\n"
 		      "Multiple \"mtd\" parameters may be specified.\n"
 		      "MTD devices may be specified by their number, name, or "
 		      "path to the MTD character device node.\n"
 		      "Optional \"vid_hdr_offs\" parameter specifies UBI VID "
 		      "header position to be used by UBI.\n"
+		      "Optional \"max_beb_per1024\" parameter specifies the "
+		      "maximum expected bad eraseblock per 1024 eraseblocks."
+		      "(default " __stringify(MTD_UBI_DEFAULT_BEB_LIMIT)
+		      " if 0 or not set)\n"
 		      "Example 1: mtd=/dev/mtd0 - attach MTD device "
 		      "/dev/mtd0.\n"
 		      "Example 2: mtd=content,1984 mtd=4 - attach MTD device "
diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c
index acec85d..ed5dd75 100644
--- a/drivers/mtd/ubi/cdev.c
+++ b/drivers/mtd/ubi/cdev.c
@@ -1005,12 +1005,19 @@ static long ctrl_cdev_ioctl(struct file *file, unsigned int cmd,
 			break;
 		}
 
+		/* for compatibility with older user-space tools,
+		 * a zero value should be treated like a default value
+		 */
+		if (!req.max_beb_per1024)
+			req.max_beb_per1024 = MTD_UBI_DEFAULT_BEB_LIMIT;
+
 		/*
 		 * Note, further request verification is done by
 		 * 'ubi_attach_mtd_dev()'.
 		 */
 		mutex_lock(&ubi_devices_mutex);
-		err = ubi_attach_mtd_dev(mtd, req.ubi_num, req.vid_hdr_offset);
+		err = ubi_attach_mtd_dev(mtd, req.ubi_num, req.vid_hdr_offset,
+					 req.max_beb_per1024);
 		mutex_unlock(&ubi_devices_mutex);
 		if (err < 0)
 			put_mtd_device(mtd);
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index c94612e..2148f35 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -50,6 +50,9 @@
 /* UBI name used for character devices, sysfs, etc */
 #define UBI_NAME_STR "ubi"
 
+/* Default number of maximum expected bad blocks per 1024 eraseblocks */
+#define MTD_UBI_DEFAULT_BEB_LIMIT 20
+
 /* Normal UBI messages */
 #define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__)
 /* UBI warning messages */
@@ -693,7 +696,8 @@ int ubi_io_write_vid_hdr(struct ubi_device *ubi, int pnum,
 			 struct ubi_vid_hdr *vid_hdr);
 
 /* build.c */
-int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset);
+int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
+		       int vid_hdr_offset, int max_beb_per1024);
 int ubi_detach_mtd_dev(int ubi_num, int anyway);
 struct ubi_device *ubi_get_device(int ubi_num);
 void ubi_put_device(struct ubi_device *ubi);
diff --git a/include/mtd/ubi-user.h b/include/mtd/ubi-user.h
index 8787349..77cd0d1 100644
--- a/include/mtd/ubi-user.h
+++ b/include/mtd/ubi-user.h
@@ -222,6 +222,7 @@ enum {
  * @ubi_num: UBI device number to create
  * @mtd_num: MTD device number to attach
  * @vid_hdr_offset: VID header offset (use defaults if %0)
+ * @max_beb_per1024: Maximum expected bad eraseblocks per 1024 eraseblocks
  * @padding: reserved for future, not used, has to be zeroed
  *
  * This data structure is used to specify MTD device UBI has to attach and the
@@ -245,12 +246,28 @@ enum {
  * be 2KiB-64 bytes = 1984. Note, that this position is not even 512-bytes
  * aligned, which is OK, as UBI is clever enough to realize this is 4th
  * sub-page of the first page and add needed padding.
+ *
+ * The @max_beb_per1024 is the maximum bad eraseblocks UBI expects on the ubi
+ * device per 1024 eraseblocks.
+ * This value is often given in an other form in the NAND datasheet (min NVB
+ * i.e. minimal number of valid blocks). The maximum expected bad eraseblocks
+ * per 1024 is then:
+ *   1024 * (1 - MinNVB / MaxNVB)
+ * Which gives 20 for most NAND devices.
+ * This limit is used in order to derive amount of eraseblock UBI reserves for
+ * handling new bad blocks.
+ * If the device has more bad eraseblocks than this limit, UBI does not reserve
+ * any physical eraseblocks for new bad eraseblocks, but attempts to use
+ * available eraseblocks (if any).
+ * The accepted range is 0-255. If 0 is given, the default value
+ * MTD_UBI_DEFAULT_BEB_LIMIT will be used for compatibility.
  */
 struct ubi_attach_req {
 	__s32 ubi_num;
 	__s32 mtd_num;
 	__s32 vid_hdr_offset;
-	__s8 padding[12];
+	__u8 max_beb_per1024;
+	__s8 padding[11];
 };
 
 /**
-- 
1.7.2.5

[PATCH] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Richard Genoud]

The ioctl UBI_IOCATT has been extended with max_beb_per1024 parameter.

This parameter is used for adjusting the "maximum expected number of
bad blocks per 1024 blocks" for each mtd device.
The number of physical erase blocks (PEB) that UBI will reserve for bad
block handling is now:
whole_flash_chipset__PEB_number * max_beb_per1024 / 1024

This means that for a 4096 PEB NAND device with 3 MTD partitions:
mtd0: 512 PEB
mtd1: 1536 PEB
mtd2: 2048 PEB

the commands:
ubiattach -m 0 -d 0 -b 20 /dev/ubi_ctrl
ubiattach -m 1 -d 1 -b 20 /dev/ubi_ctrl
ubiattach -m 2 -d 2 -b 20 /dev/ubi_ctrl
will attach mtdx to UBIx and reserve:
80 PEB for bad block handling on UBI0
80 PEB for bad block handling on UBI1
80 PEB for bad block handling on UBI2

=> for the whole device, 240 PEB will be reserved for bad block
handling.

This may seems a waste of space, but as far as the bad blocks can appear
every where on a flash device, in the worst case scenario they can
all appear in one MTD partition.
So the maximum number of expected erase blocks given by the NAND
manufacturer should be reserve on each MTD partition.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 include/mtd/ubi-user.h             |   20 ++++++++++++++++-
 tests/fs-tests/integrity/integck.c |    1 +
 ubi-utils/include/libubi.h         |    2 +
 ubi-utils/libubi.c                 |    2 +
 ubi-utils/ubiattach.c              |   41 +++++++++++++++++++++++++++++-------
 5 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/include/mtd/ubi-user.h b/include/mtd/ubi-user.h
index 296efae..5f77906 100644
--- a/include/mtd/ubi-user.h
+++ b/include/mtd/ubi-user.h
@@ -235,6 +235,7 @@ enum {
  * @ubi_num: UBI device number to create
  * @mtd_num: MTD device number to attach
  * @vid_hdr_offset: VID header offset (use defaults if %0)
+ * @max_beb_per1024: Maximum expected bad eraseblocks per 1024 eraseblocks
  * @padding: reserved for future, not used, has to be zeroed
  *
  * This data structure is used to specify MTD device UBI has to attach and the
@@ -258,12 +259,29 @@ enum {
  * be 2KiB-64 bytes = 1984. Note, that this position is not even 512-bytes
  * aligned, which is OK, as UBI is clever enough to realize this is 4th
  * sub-page of the first page and add needed padding.
+ *
+ * The @max_beb_per1024 is the maximum bad eraseblocks UBI expects on the ubi
+ * device per 1024 eraseblocks.
+ * This value is often given in an other form in the NAND datasheet (min NVB
+ * i.e. minimal number of valid blocks). The maximum expected bad eraseblocks
+ * per 1024 is then:
+ *   1024 * (1 - MinNVB / MaxNVB)
+ * Which gives 20 for most NAND devices.
+ * This limit is used in order to derive amount of eraseblock UBI reserves for
+ * handling new bad blocks.
+ * If the device has more bad eraseblocks than this limit, UBI does not reserve
+ * any physical eraseblocks for new bad eraseblocks, but attempts to use
+ * available eraseblocks (if any).
+ * The accepted range is 0-255. If 0 is given, the default kernel value
+ * MTD_UBI_DEFAULT_BEB_LIMIT will be used for compatibility with previous
+ * versions.
  */
 struct ubi_attach_req {
 	int32_t ubi_num;
 	int32_t mtd_num;
 	int32_t vid_hdr_offset;
-	int8_t padding[12];
+	int8_t max_beb_per1024;
+	int8_t padding[11];
 };
 
 /**
diff --git a/tests/fs-tests/integrity/integck.c b/tests/fs-tests/integrity/integck.c
index 30322cd..f12dfac 100644
--- a/tests/fs-tests/integrity/integck.c
+++ b/tests/fs-tests/integrity/integck.c
@@ -3152,6 +3152,7 @@ static int reattach(void)
 	req.mtd_num = args.mtdn;
 	req.vid_hdr_offset = 0;
 	req.mtd_dev_node = NULL;
+	req.max_beb_per1024 = 0;
 
 	err = ubi_attach(libubi, "/dev/ubi_ctrl", &req);
 	if (err)
diff --git a/ubi-utils/include/libubi.h b/ubi-utils/include/libubi.h
index dc03d02..1eadff8 100644
--- a/ubi-utils/include/libubi.h
+++ b/ubi-utils/include/libubi.h
@@ -50,6 +50,7 @@ typedef void * libubi_t;
  * @mtd_dev_node: path to MTD device node to attach
  * @vid_hdr_offset: VID header offset (%0 means default offset and this is what
  *                  most of the users want)
+ * @max_beb_per1024: Maximum expected bad eraseblocks per 1024 eraseblocks
  */
 struct ubi_attach_request
 {
@@ -57,6 +58,7 @@ struct ubi_attach_request
 	int mtd_num;
 	const char *mtd_dev_node;
 	int vid_hdr_offset;
+	unsigned char max_beb_per1024;
 };
 
 /**
diff --git a/ubi-utils/libubi.c b/ubi-utils/libubi.c
index c898e36..d3c333d 100644
--- a/ubi-utils/libubi.c
+++ b/ubi-utils/libubi.c
@@ -719,6 +719,7 @@ int ubi_attach_mtd(libubi_t desc, const char *node,
 	r.ubi_num = req->dev_num;
 	r.mtd_num = req->mtd_num;
 	r.vid_hdr_offset = req->vid_hdr_offset;
+	r.max_beb_per1024 = req->max_beb_per1024;
 
 	ret = do_attach(node, &r);
 	if (ret == 0)
@@ -780,6 +781,7 @@ int ubi_attach(libubi_t desc, const char *node, struct ubi_attach_request *req)
 	memset(&r, 0, sizeof(struct ubi_attach_req));
 	r.ubi_num = req->dev_num;
 	r.vid_hdr_offset = req->vid_hdr_offset;
+	r.max_beb_per1024 = req->max_beb_per1024;
 
 	/*
 	 * User has passed path to device node. Lets find out MTD device number
diff --git a/ubi-utils/ubiattach.c b/ubi-utils/ubiattach.c
index 27e7c09..2026c2e 100644
--- a/ubi-utils/ubiattach.c
+++ b/ubi-utils/ubiattach.c
@@ -42,6 +42,7 @@ struct args {
 	int vidoffs;
 	const char *node;
 	const char *dev;
+	int max_beb_per1024;
 };
 
 static struct args args = {
@@ -50,6 +51,7 @@ static struct args args = {
 	.vidoffs = 0,
 	.node = NULL,
 	.dev = NULL,
+	.max_beb_per1024 = 0,
 };
 
 static const char doc[] = PROGRAM_NAME " version " VERSION
@@ -63,6 +65,9 @@ static const char optionsstr[] =
 "                      if the character device node does not exist)\n"
 "-O, --vid-hdr-offset  VID header offset (do not specify this unless you really\n"
 "                      know what you are doing, the default should be optimal)\n"
+"-b, --max-beb-per1024 Maximum expected bad block number per 1024 eraseblock.\n"
+"                      The default value is correct for most NAND devices.\n"
+"                      (Range 1-255, 0 for default kernel value).\n"
 "-h, --help            print help message\n"
 "-V, --version         print program version";
 
@@ -71,19 +76,25 @@ static const char usage[] =
 "\t[-m <MTD device number>] [-d <UBI device number>] [-p <path to device>]\n"
 "\t[--mtdn=<MTD device number>] [--devn=<UBI device number>]\n"
 "\t[--dev-path=<path to device>]\n"
+"\t[--max-beb-per1024=<maximum bad block number per 1024 blocks>]\n"
 "UBI control device defaults to " DEFAULT_CTRL_DEV " if not supplied.\n"
 "Example 1: " PROGRAM_NAME " -p /dev/mtd0 - attach /dev/mtd0 to UBI\n"
 "Example 2: " PROGRAM_NAME " -m 0 - attach MTD device 0 (mtd0) to UBI\n"
 "Example 3: " PROGRAM_NAME " -m 0 -d 3 - attach MTD device 0 (mtd0) to UBI\n"
-"           and create UBI device number 3 (ubi3)";
+"           and create UBI device number 3 (ubi3)\n"
+"Example 4: " PROGRAM_NAME " -m 1 -b 25 - attach /dev/mtd1 to UBI and reserve \n"
+"           25*nand_size_in_blocks/1024 erase blocks for bad block handling.\n"
+"           (e.g. if the NAND *chipset* has 4096 PEB, 100 will be reserved \n"
+"           for this UBI device).";
 
 static const struct option long_options[] = {
-	{ .name = "devn",           .has_arg = 1, .flag = NULL, .val = 'd' },
-	{ .name = "dev-path",       .has_arg = 1, .flag = NULL, .val = 'p' },
-	{ .name = "mtdn",           .has_arg = 1, .flag = NULL, .val = 'm' },
-	{ .name = "vid-hdr-offset", .has_arg = 1, .flag = NULL, .val = 'O' },
-	{ .name = "help",           .has_arg = 0, .flag = NULL, .val = 'h' },
-	{ .name = "version",        .has_arg = 0, .flag = NULL, .val = 'V' },
+	{ .name = "devn",            .has_arg = 1, .flag = NULL, .val = 'd' },
+	{ .name = "dev-path",        .has_arg = 1, .flag = NULL, .val = 'p' },
+	{ .name = "mtdn",            .has_arg = 1, .flag = NULL, .val = 'm' },
+	{ .name = "vid-hdr-offset",  .has_arg = 1, .flag = NULL, .val = 'O' },
+	{ .name = "help",            .has_arg = 0, .flag = NULL, .val = 'h' },
+	{ .name = "version",         .has_arg = 0, .flag = NULL, .val = 'V' },
+	{ .name = "max-beb-per1024", .has_arg = 1, .flag = NULL, .val = 'b' },
 	{ NULL, 0, NULL, 0},
 };
 
@@ -92,7 +103,7 @@ static int parse_opt(int argc, char * const argv[])
 	while (1) {
 		int key, error = 0;
 
-		key = getopt_long(argc, argv, "p:m:d:O:hV", long_options, NULL);
+		key = getopt_long(argc, argv, "p:m:d:O:hVb:", long_options, NULL);
 		if (key == -1)
 			break;
 
@@ -134,6 +145,19 @@ static int parse_opt(int argc, char * const argv[])
 		case ':':
 			return errmsg("parameter is missing");
 
+		case 'b':
+			args.max_beb_per1024 = simple_strtoul(optarg, &error);
+			if (error || args.max_beb_per1024 < 0
+			    || args.max_beb_per1024 > 255)
+				return errmsg("bad maximum of expected bad "
+					      "blocks (0-255): \"%s\"",
+					      optarg);
+			if (args.max_beb_per1024 == 0)
+				warnmsg("default kernel value will be used for"
+					" maximum expected bad blocks\n");
+
+			break;
+
 		default:
 			fprintf(stderr, "Use -h for help\n");
 			return -1;
@@ -190,6 +214,7 @@ int main(int argc, char * const argv[])
 	req.mtd_num = args.mtdn;
 	req.vid_hdr_offset = args.vidoffs;
 	req.mtd_dev_node = args.dev;
+	req.max_beb_per1024 = args.max_beb_per1024;
 
 	err = ubi_attach(libubi, args.node, &req);
 	if (err) {
-- 
1.7.2.5

Re: [PATCH 3/4] UBI: use the whole MTD device size to get bad_peb_limit

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 481 bytes --]

On Tue, 2012-07-10 at 18:23 +0200, Richard Genoud wrote:
> 
> The Kconfig option is in per1024 blocks, thus it can have a default
> value of 20 which is *very* common for NAND devices. 

Why do you prefer per1024? I'd make it centi-percent instead, wouldn't
that be more human-friendly. It is just %*100. If I am a user, it is
easy for me to calculate % and multiply that by 100. This per1024 thing
would make me scratch my head...

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/4] UBI: use the whole MTD device size to get bad_peb_limit

[Richard Genoud]

2012/7/18 Artem Bityutskiy <dedekind1@gmail.com>:
> On Tue, 2012-07-10 at 18:23 +0200, Richard Genoud wrote:
>>
>> The Kconfig option is in per1024 blocks, thus it can have a default
>> value of 20 which is *very* common for NAND devices.
>
> Why do you prefer per1024? I'd make it centi-percent instead, wouldn't
> that be more human-friendly. It is just %*100. If I am a user, it is
> easy for me to calculate % and multiply that by 100. This per1024 thing
> would make me scratch my head...
All the NAND devices I've seen are a multiple of 1024 erase blocks, so
I thought it'll be easier for the humans to use per1024...
here are 2 random datasheets :
page 14: http://www.micron.com/~/media/Documents/Products/Data%20Sheet/NAND%20Flash/6691NANDXXX%20A%20128Mb_256Mb_528%20byte_or_264%20word_page_3V_SLC_90nm.pdf
2048 blocks 40BEB max => 20per1024
1024 blocks 20BEB max => 20per1024

page 24: http://www.hynix.com/datasheet/pdf/flash/HY27(U_S)S(08_16)121M(Rev0.6).pdf
4096 blocks 80BEB max => 20per1024

If we express the default exact value of 20 per1024 blocks in percent,
that would be 1.953125% (event in per-thousand, we'll have 19.53125).
=> the value have to be rounded to floor or ceiling and that will make
the user unsure about the effective number of bad blocks used for bad
block handling.
(even if we say that the default value 20 per-thousand block will be
used, as the NAND devices have a number of blocks multiple of 1024,
for a 4096 blocks device, we'll have 81.92 reserved blocks, rounded to
82 => we loose 2 blocks for each mtd partition.)
So the per1024 thing was really to stick to the device layout and to
be easier for users (IMHO)

Richard.

Re: [PATCH 3/4] UBI: use the whole MTD device size to get bad_peb_limit

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 222 bytes --]

On Wed, 2012-07-18 at 10:30 +0200, Richard Genoud wrote:
> So the per1024 thing was really to stick to the device layout and to
> be easier for users (IMHO)

Convinced, thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/4] mtd_is_partition: struct mtd_info should be const

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 489 bytes --]

On Tue, 2012-07-10 at 18:23 +0200, Richard Genoud wrote:
> struct mtd_info is not modified by mtd_is_partition so it should be a
> const.
> 
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>

I've pushed this and the next MTD patches to linux-ubi, thank you! I
hope David does not mind if it goes in via the MTD, or if he could take
them to the mtd tree, then I cold merge the mtd tree to the UBI tree.
But this is not your pain.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

On Tue, 2012-07-10 at 18:23 +0200, Richard Genoud wrote:
> This patch provides the possibility to adjust the "maximum expected number of
> bad blocks per 1024 blocks" (max_beb_per1024) for each mtd device.
> 
> The majority of NAND devices have their max_beb_per1024 equal to 20, but
> sometimes it's more.
> Now, we can adjust that via a kernel parameter:
> ubi.mtd=<name|num|path>[,<vid_hdr_offs>[,max_beb_per1024]]
> and via UBI_IOCATT ioctl which is now:
> struct ubi_attach_req {
> 	__s32 ubi_num;
> 	__s32 mtd_num;
> 	__s32 vid_hdr_offset;
> 	__u8 max_beb_per1024;
> 	__s8 padding[11];
> };
> 
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>

Would you please split this patch on 2 - first adds the kernel module
parameter, the second changes the ioctl.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/4] UBI: use the whole MTD device size to get bad_peb_limit

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 6524 bytes --]

On Tue, 2012-07-10 at 18:23 +0200, Richard Genoud wrote:
> +			/* we are using here the whole MTD device size and not
> +			 * the MTD partition size because the maximum number of
> +			 * bad blocks is a percentage of the whole device and
> +			 * the bad blocks are not fairly disposed on a flash
> +			 * device
> +			 */

Would you please use proper kernel-style comments instead, to be
consistent with the rest of the UBI code? I've amended this one, but
wanted to note for future.

I've re-based your patch against the latest UBI. I've also tried to
improve the Kconfig help message as well. Below is the patch I ended up
with.



From cb14c6c5455443cbe960a36e77b3fcd0e5bc7152 Mon Sep 17 00:00:00 2001
From: Richard Genoud <richard.genoud@gmail.com>
Date: Tue, 10 Jul 2012 18:23:41 +0200
Subject: [PATCH 2/2] UBI: use the whole MTD device size to get bad_peb_limit

On NAND flash devices, UBI reserves some physical erase blocks (PEB) for
bad block handling. Today, the number of reserved PEB can only be set as a
percentage of the total number of PEB in each MTD partition. For example, for a
NAND flash with 128KiB PEB, 2 MTD partition of 20MiB (mtd0) and 100MiB (mtd1)
and 2% reserved PEB:
 - the UBI device on mtd0 will have 2 PEB reserved
 - the UBI device on mtd1 will have 16 PEB reserved

The problem with this behaviour is that NAND flash manufacturers give a
minimum number of valid block (NVB) during the endurance life of the
device, e.g.:

Parameter             Symbol    Min    Max    Unit      Notes
--------------------------------------------------------------
Valid block number     NVB     1004    1024   Blocks     1
Note:
1. Invalid blocks are block that contain one or more bad bits beyond
ECC. The device may contain bad blocks upon shipment. Additional bad
blocks may develop over time; however, the total number of available
blocks will not drop below NVB during the endurance life of the device.

From this number we can deduce the maximum number of bad PEB that a device will
contain during its endurance life: a 128MiB NAND flash (1024 PEB) will not have
less than 20 bad blocks during the flash endurance life.

But the manufacturer doesn't tell where those bad block will appear. He doesn't
say either if they will be equally disposed on the whole device (and I'm pretty
sure they won't). So, according to the datasheets, we should reserve the
maximum number of bad PEB for each UBI device (worst case scenario: 20 bad
blocks appears on the smallest MTD partition).

So this patch make UBI use the whole MTD device size to calculate the maximum
bad expected eraseblocks.

The Kconfig option is in per1024 blocks, thus it can have a default value of 20
which is *very* common for NAND devices.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 drivers/mtd/ubi/Kconfig |   27 +++++++++++++++++++++------
 drivers/mtd/ubi/build.c |   21 ++++++++++++++++++---
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index b2f4f0f..98bda6c 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -28,14 +28,29 @@ config MTD_UBI_WL_THRESHOLD
 	  to 128 or 256, although it does not have to be power of 2).
 
 config MTD_UBI_BEB_LIMIT
-	int "Percentage of maximum expected bad eraseblocks"
-	default 2
-	range 0 25
+	int "Maximum expected bad eraseblock count per 1024 eraseblocks"
+	default 20
+	range 2 256
 	help
 	  This option specifies the maximum bad physical eraseblocks UBI
-	  expects on the UBI device (percents of total number of physical
-	  eraseblocks on this MTD partition). If the underlying flash does not
-	  admit of bad eraseblocks (e.g. NOR flash), this value is ignored.
+	  expects on the MTD device (per 1024 eraseblocks). If the underlying
+	  flash does not admit of bad eraseblocks (e.g. NOR flash), this value
+	  is ignored.
+
+	  NAND datasheets often specify the minimum and maximum NVM (Number of
+	  Valid Blocks) for the flashes' endurance lifetime. The maximum
+	  expected bad eraseblocks per 1024 eraseblocks then can be calculated
+	  as "1024 * (1 - MinNVB / MaxNVB)", which gives 20 for most NANDs
+	  (MaxNVB is basically the total count of eraseblocks on the chip).
+
+	  To put it differently, if this value is 20, UBI will try to reserve
+	  about 1.9% of physical eraseblocks for bad blocks handling. And that
+	  will be 1.9% of eraseblocks on the entire NAND chip, not just the MTD
+	  partition UBI attaches. This means that if you have, say,  if a NAND
+	  flash chip admits maximum 40 bad eraseblocks, and it is split on two
+	  MTD partitions of the same size, UBI will reserve 40 eraseblocks when
+	  attaching a partition.
+
 	  Leave the default value if unsure.
 
 config MTD_UBI_GLUEBI
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 7b6b5f9..9fd8d86 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -36,6 +36,7 @@
 #include <linux/namei.h>
 #include <linux/stat.h>
 #include <linux/miscdevice.h>
+#include <linux/mtd/partitions.h>
 #include <linux/log2.h>
 #include <linux/kthread.h>
 #include <linux/kernel.h>
@@ -610,11 +611,25 @@ static int io_init(struct ubi_device *ubi)
 	if (mtd_can_have_bb(ubi->mtd)) {
 		ubi->bad_allowed = 1;
 		if (CONFIG_MTD_UBI_BEB_LIMIT > 0) {
-			int percent = CONFIG_MTD_UBI_BEB_LIMIT;
-			int limit = mult_frac(ubi->peb_count, percent, 100);
+			int per1024 = CONFIG_MTD_UBI_BEB_LIMIT;
+			int limit, device_pebs;
+			uint64_t device_size;
+
+			/*
+			 * Here we are using size of the entire flash chip and
+			 * not just the MTD partition size because the maximum
+			 * number of bad eraseblocks is a percentage of the
+			 * whole device and bad eraseblocks are not fairly
+			 * distributed over the flash chip. So the worst case
+			 * is that all the bad eraseblocks of the chip are in
+			 * the MTD partition we are attaching (ubi->mtd).
+			 */
+			device_size = mtd_get_device_size(ubi->mtd);
+			device_pebs = mtd_div_by_eb(device_size, ubi->mtd);
+			limit = mult_frac(device_pebs, per1024, 1024);
 
 			/* Round it up */
-			if (mult_frac(limit, 100, percent) < ubi->peb_count)
+			if (mult_frac(limit, 1024, per1024) < ubi->peb_count)
 				limit += 1;
 			ubi->bad_peb_limit = limit;
 		}
-- 
1.7.10.4

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/4] UBI: use the whole MTD device size to get bad_peb_limit

[Richard Genoud]

2012/8/15 Artem Bityutskiy <dedekind1@gmail.com>:
> On Tue, 2012-07-10 at 18:23 +0200, Richard Genoud wrote:
>> +                     /* we are using here the whole MTD device size and not
>> +                      * the MTD partition size because the maximum number of
>> +                      * bad blocks is a percentage of the whole device and
>> +                      * the bad blocks are not fairly disposed on a flash
>> +                      * device
>> +                      */
>
> Would you please use proper kernel-style comments instead, to be
> consistent with the rest of the UBI code? I've amended this one, but
> wanted to note for future.
ok, sorry for that.

>
> I've re-based your patch against the latest UBI. I've also tried to
> improve the Kconfig help message as well. Below is the patch I ended up
> with.
>
>
> From cb14c6c5455443cbe960a36e77b3fcd0e5bc7152 Mon Sep 17 00:00:00 2001
> From: Richard Genoud <richard.genoud@gmail.com>
> Date: Tue, 10 Jul 2012 18:23:41 +0200
> Subject: [PATCH 2/2] UBI: use the whole MTD device size to get bad_peb_limit
>
> On NAND flash devices, UBI reserves some physical erase blocks (PEB) for
> bad block handling. Today, the number of reserved PEB can only be set as a
> percentage of the total number of PEB in each MTD partition. For example, for a
> NAND flash with 128KiB PEB, 2 MTD partition of 20MiB (mtd0) and 100MiB (mtd1)
> and 2% reserved PEB:
>  - the UBI device on mtd0 will have 2 PEB reserved
>  - the UBI device on mtd1 will have 16 PEB reserved
>
> The problem with this behaviour is that NAND flash manufacturers give a
> minimum number of valid block (NVB) during the endurance life of the
> device, e.g.:
>
> Parameter             Symbol    Min    Max    Unit      Notes
> --------------------------------------------------------------
> Valid block number     NVB     1004    1024   Blocks     1
> Note:
> 1. Invalid blocks are block that contain one or more bad bits beyond
> ECC. The device may contain bad blocks upon shipment. Additional bad
> blocks may develop over time; however, the total number of available
> blocks will not drop below NVB during the endurance life of the device.
>
> From this number we can deduce the maximum number of bad PEB that a device will
> contain during its endurance life: a 128MiB NAND flash (1024 PEB) will not have
> less than 20 bad blocks during the flash endurance life.
>
> But the manufacturer doesn't tell where those bad block will appear. He doesn't
> say either if they will be equally disposed on the whole device (and I'm pretty
> sure they won't). So, according to the datasheets, we should reserve the
> maximum number of bad PEB for each UBI device (worst case scenario: 20 bad
> blocks appears on the smallest MTD partition).
>
> So this patch make UBI use the whole MTD device size to calculate the maximum
> bad expected eraseblocks.
>
> The Kconfig option is in per1024 blocks, thus it can have a default value of 20
> which is *very* common for NAND devices.
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> ---
>  drivers/mtd/ubi/Kconfig |   27 +++++++++++++++++++++------
>  drivers/mtd/ubi/build.c |   21 ++++++++++++++++++---
>  2 files changed, 39 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
> index b2f4f0f..98bda6c 100644
> --- a/drivers/mtd/ubi/Kconfig
> +++ b/drivers/mtd/ubi/Kconfig
> @@ -28,14 +28,29 @@ config MTD_UBI_WL_THRESHOLD
>           to 128 or 256, although it does not have to be power of 2).
>
>  config MTD_UBI_BEB_LIMIT
> -       int "Percentage of maximum expected bad eraseblocks"
> -       default 2
> -       range 0 25
> +       int "Maximum expected bad eraseblock count per 1024 eraseblocks"
> +       default 20
> +       range 2 256
>         help
>           This option specifies the maximum bad physical eraseblocks UBI
> -         expects on the UBI device (percents of total number of physical
> -         eraseblocks on this MTD partition). If the underlying flash does not
> -         admit of bad eraseblocks (e.g. NOR flash), this value is ignored.
> +         expects on the MTD device (per 1024 eraseblocks). If the underlying
> +         flash does not admit of bad eraseblocks (e.g. NOR flash), this value
> +         is ignored.
> +
> +         NAND datasheets often specify the minimum and maximum NVM (Number of
> +         Valid Blocks) for the flashes' endurance lifetime. The maximum
> +         expected bad eraseblocks per 1024 eraseblocks then can be calculated
> +         as "1024 * (1 - MinNVB / MaxNVB)", which gives 20 for most NANDs
> +         (MaxNVB is basically the total count of eraseblocks on the chip).
> +
> +         To put it differently, if this value is 20, UBI will try to reserve
> +         about 1.9% of physical eraseblocks for bad blocks handling. And that
> +         will be 1.9% of eraseblocks on the entire NAND chip, not just the MTD
> +         partition UBI attaches. This means that if you have, say,  if a NAND
I don't quite understand this sentence.
Maybe you meant:
This means that if you have, say, a NAND flash chip that admits a
maximum of 40 bad eraseblocks [...]
(but I'm not a native english speaker !)
> +         flash chip admits maximum 40 bad eraseblocks, and it is split on two
> +         MTD partitions of the same size, UBI will reserve 40 eraseblocks when
> +         attaching a partition.
> +
>           Leave the default value if unsure.
>

Best regards,
Richard.


-- 
for me, ck means con kolivas and not calvin klein... does it mean I'm a geek ?

Re: [PATCH 3/4] UBI: use the whole MTD device size to get bad_peb_limit

[Shmulik Ladkani]

Hi Artem, Richard,

On Wed, 15 Aug 2012 18:08:51 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> 1. Invalid blocks are block that contain one or more bad bits beyond
> ECC.

I would remove this one sentence from the log, it is misleading; invalid
blocks are not necessarily related to ECC.

>  		if (CONFIG_MTD_UBI_BEB_LIMIT > 0) {
> -			int percent = CONFIG_MTD_UBI_BEB_LIMIT;
> -			int limit = mult_frac(ubi->peb_count, percent, 100);
> +			int per1024 = CONFIG_MTD_UBI_BEB_LIMIT;
> +			int limit, device_pebs;
> +			uint64_t device_size;
> +
> +			/*
> +			 * Here we are using size of the entire flash chip and
> +			 * not just the MTD partition size because the maximum
> +			 * number of bad eraseblocks is a percentage of the
> +			 * whole device and bad eraseblocks are not fairly
> +			 * distributed over the flash chip. So the worst case
> +			 * is that all the bad eraseblocks of the chip are in
> +			 * the MTD partition we are attaching (ubi->mtd).
> +			 */
> +			device_size = mtd_get_device_size(ubi->mtd);
> +			device_pebs = mtd_div_by_eb(device_size, ubi->mtd);
> +			limit = mult_frac(device_pebs, per1024, 1024);
>  
>  			/* Round it up */
> -			if (mult_frac(limit, 100, percent) < ubi->peb_count)
> +			if (mult_frac(limit, 1024, per1024) < ubi->peb_count)

Oops... should be: 

+			if (mult_frac(limit, 1024, per1024) < device_pebs)

Regards,
Shmulik

Re: [PATCH 3/4] UBI: use the whole MTD device size to get bad_peb_limit

[Shmulik Ladkani]

Hi,

One more thing...

On Wed, 15 Aug 2012 18:08:51 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
>  config MTD_UBI_BEB_LIMIT
> -	int "Percentage of maximum expected bad eraseblocks"
> -	default 2
> -	range 0 25
> +	int "Maximum expected bad eraseblock count per 1024 eraseblocks"
> +	default 20
> +	range 2 256

Those defconfigs that explicilty set an original LIMIT should be
adjusted as well, as the units have changed, no?

Regards,
Shmulik

Re: [PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Shmulik Ladkani]

Hi Richard,

Sorry for reviewing this late...

On Tue, 10 Jul 2012 18:23:42 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
> -config MTD_UBI_BEB_LIMIT
> -	int "Maximum expected bad eraseblocks per 1024 eraseblocks"
> -	default 20
> -	range 2 256

I see some benefit keeping the config.

For the simplest systems (those having one ubi device) that need a limit
*other* than the default (20 per 1024), they can simply set the config
to their chosen value, as they were used to.

With you approach, these system MUST pass the limit parameter via the
ioctl / module-parameter.

> +static int get_bad_peb_limit(const struct ubi_device *ubi, int max_beb_per1024)
> +{
> +	int device_peb_count;
> +	uint64_t device_size;
> +	int beb_limit = 0;
> +
> +	/* this has already been checked in ioctl */
> +	if (max_beb_per1024 <= 0)
> +		goto out;

Can you explain how 'max_beb_per1024 <= 0' may happen? 

It seems that all of your calls to 'ubi_attach_mtd_dev' pass a positive
max_beb_per1024 value (the default is always set). See your
'ubi_mtd_param_parse' and 'ctrl_cdev_ioctl'. Am I missing something?

Also, since max_beb_per1024 is always set, how one may specify a zero
limit?

Regards,
Shmulik

Re: [PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Richard Genoud]

2012/8/16 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> Hi Richard,
>
> Sorry for reviewing this late...
>
> On Tue, 10 Jul 2012 18:23:42 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
>> -config MTD_UBI_BEB_LIMIT
>> -     int "Maximum expected bad eraseblocks per 1024 eraseblocks"
>> -     default 20
>> -     range 2 256
>
> I see some benefit keeping the config.
>
> For the simplest systems (those having one ubi device) that need a limit
> *other* than the default (20 per 1024), they can simply set the config
> to their chosen value, as they were used to.
>
> With you approach, these system MUST pass the limit parameter via the
> ioctl / module-parameter.
That's right.
We can add a kernel config option to change the max_beb_per1024
default value (actually, this is almost the patch I send first).
But I see something disturbing with that:
It means that an ubi_attach call from userspace, without specifying
max_beb_per1024, won't have the same result, depending of the default
config value the kernel has been compiled with.
(Or maybe this behavior is acceptable).

>> +static int get_bad_peb_limit(const struct ubi_device *ubi, int max_beb_per1024)
>> +{
>> +     int device_peb_count;
>> +     uint64_t device_size;
>> +     int beb_limit = 0;
>> +
>> +     /* this has already been checked in ioctl */
>> +     if (max_beb_per1024 <= 0)
>> +             goto out;
>
> Can you explain how 'max_beb_per1024 <= 0' may happen?
>
> It seems that all of your calls to 'ubi_attach_mtd_dev' pass a positive
> max_beb_per1024 value (the default is always set). See your
> 'ubi_mtd_param_parse' and 'ctrl_cdev_ioctl'. Am I missing something?
Actually it can. But it's because I made a mistake:
p->max_beb_per1024 = bytes_str_to_int(tokens[2]);
=> I didn't check the return value of this. It can fail, and if it
does the return value is >0.
I'm going to change that.

>
> Also, since max_beb_per1024 is always set, how one may specify a zero
> limit?
You can't.
Do you think we need that ?
0 should be kept for "default value", not to break user-space.
But we can use another value for 0, like 1024 or 2048.
(but honestly, I think this will add complexity for an unlikely configuration.)

>
> Regards,
> Shmulik

Regards,
Richard.


-- 
for me, ck means con kolivas and not calvin klein... does it mean I'm a geek ?

Re: [PATCH 3/4] UBI: use the whole MTD device size to get bad_peb_limit

[Richard Genoud]

2012/8/16 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> Hi Artem, Richard,
>
> On Wed, 15 Aug 2012 18:08:51 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
>> 1. Invalid blocks are block that contain one or more bad bits beyond
>> ECC.
>
> I would remove this one sentence from the log, it is misleading; invalid
> blocks are not necessarily related to ECC.
I agree (even if this sentence is from a nand datasheet !)

>
>>               if (CONFIG_MTD_UBI_BEB_LIMIT > 0) {
>> -                     int percent = CONFIG_MTD_UBI_BEB_LIMIT;
>> -                     int limit = mult_frac(ubi->peb_count, percent, 100);
>> +                     int per1024 = CONFIG_MTD_UBI_BEB_LIMIT;
>> +                     int limit, device_pebs;
>> +                     uint64_t device_size;
>> +
>> +                     /*
>> +                      * Here we are using size of the entire flash chip and
>> +                      * not just the MTD partition size because the maximum
>> +                      * number of bad eraseblocks is a percentage of the
>> +                      * whole device and bad eraseblocks are not fairly
>> +                      * distributed over the flash chip. So the worst case
>> +                      * is that all the bad eraseblocks of the chip are in
>> +                      * the MTD partition we are attaching (ubi->mtd).
>> +                      */
>> +                     device_size = mtd_get_device_size(ubi->mtd);
>> +                     device_pebs = mtd_div_by_eb(device_size, ubi->mtd);
>> +                     limit = mult_frac(device_pebs, per1024, 1024);
>>
>>                       /* Round it up */
>> -                     if (mult_frac(limit, 100, percent) < ubi->peb_count)
>> +                     if (mult_frac(limit, 1024, per1024) < ubi->peb_count)
>
> Oops... should be:
>
> +                       if (mult_frac(limit, 1024, per1024) < device_pebs)
>
> Regards,
> Shmulik
Ok, I'll change that, may be in a separate patch, as it's a bug fix.
I'll add your signoff.

thanks !


-- 
for me, ck means con kolivas and not calvin klein... does it mean I'm a geek ?

Re: [PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Shmulik Ladkani]

Hi Richard, Artem,

On Thu, 16 Aug 2012 12:07:01 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
> > With you approach, these system MUST pass the limit parameter via the
> > ioctl / module-parameter.
> That's right.
> We can add a kernel config option to change the max_beb_per1024
> default value (actually, this is almost the patch I send first).
> But I see something disturbing with that:
> It means that an ubi_attach call from userspace, without specifying
> max_beb_per1024, won't have the same result, depending of the default
> config value the kernel has been compiled with.
> (Or maybe this behavior is acceptable).

Well, that was the previous behavior of MTD_UBI_BEB_RESERVE, long before
our patchsets.
I think it is acceptable, given the fact it simplifies the configuration
for most simple systems.

Anyway I'm just pointing out the consequences of your change and try to
suggest other alternatives.
Artem should decide as he's the maintainer.

> > Also, since max_beb_per1024 is always set, how one may specify a zero
> > limit?
> You can't.
> Do you think we need that ?

Well again, originally, prior our patchsets, one *could* set a zero
MTD_UBI_BEB_RESERVE for his system. So we're introducing a change that
affects the possible ways an ubi system can be configured, banning
a configuration that was valid in the past.

Does it make sense to set a zero limit? dunno.
For testing purposes, maybe.

Artem, what do you think? prohibit a zero beb limit?

Regards,
Shmulik

Re: [PATCH 3/4] UBI: use the whole MTD device size to get bad_peb_limit

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 307 bytes --]

On Thu, 2012-08-16 at 11:25 +0300, Shmulik Ladkani wrote:
> I would remove this one sentence from the log, it is misleading; invalid
> blocks are not necessarily related to ECC.

Done.

> +			if (mult_frac(limit, 1024, per1024) < device_pebs)

Done, thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/4] UBI: use the whole MTD device size to get bad_peb_limit

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 413 bytes --]

On Thu, 2012-08-16 at 12:35 +0200, Richard Genoud wrote:
> Ok, I'll change that, may be in a separate patch, as it's a bug fix.

Let me take care of this patch. I'll amend it myself, push to my tree
and re-send to the mailing list for your review. Please, provide an
updated version of the other patches instead.

P.S. Of course, your authorship will be preserved.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/4] UBI: use the whole MTD device size to get bad_peb_limit

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 397 bytes --]

On Thu, 2012-08-16 at 11:32 +0300, Shmulik Ladkani wrote:
> > -	default 2
> > -	range 0 25
> > +	int "Maximum expected bad eraseblock count per 1024 eraseblocks"
> > +	default 20
> > +	range 2 256
> 
> Those defconfigs that explicilty set an original LIMIT should be
> adjusted as well, as the units have changed, no?

Yes, I'll do this, thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/4] UBI: use the whole MTD device size to get bad_peb_limit

[Richard Genoud]

2012/8/16 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> Hi,
>
> One more thing...
>
> On Wed, 15 Aug 2012 18:08:51 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
>>  config MTD_UBI_BEB_LIMIT
>> -     int "Percentage of maximum expected bad eraseblocks"
>> -     default 2
>> -     range 0 25
>> +     int "Maximum expected bad eraseblock count per 1024 eraseblocks"
>> +     default 20
>> +     range 2 256
>
> Those defconfigs that explicilty set an original LIMIT should be
> adjusted as well, as the units have changed, no?

you mean that it should be between 0 and 256 ?

Re: [PATCH 3/4] UBI: use the whole MTD device size to get bad_peb_limit

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 678 bytes --]

On Thu, 2012-08-16 at 10:13 +0200, Richard Genoud wrote:
> > +         To put it differently, if this value is 20, UBI will try
> to reserve
> > +         about 1.9% of physical eraseblocks for bad blocks
> handling. And that
> > +         will be 1.9% of eraseblocks on the entire NAND chip, not
> just the MTD
> > +         partition UBI attaches. This means that if you have, say,
> if a NAND
> I don't quite understand this sentence.
> Maybe you meant:
> This means that if you have, say, a NAND flash chip that admits a
> maximum of 40 bad eraseblocks [...]
> (but I'm not a native english speaker !)

Fixed, thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 1/2] UBI: use the whole MTD device size to get bad_peb_limit

[Artem Bityutskiy]

From: Richard Genoud <richard.genoud@gmail.com>

On NAND flash devices, UBI reserves some physical erase blocks (PEB) for
bad block handling. Today, the number of reserved PEB can only be set as a
percentage of the total number of PEB in each MTD partition. For example, for a
NAND flash with 128KiB PEB, 2 MTD partition of 20MiB (mtd0) and 100MiB (mtd1)
and 2% reserved PEB:
 - the UBI device on mtd0 will have 2 PEB reserved
 - the UBI device on mtd1 will have 16 PEB reserved

The problem with this behaviour is that NAND flash manufacturers give a
minimum number of valid block (NVB) during the endurance life of the
device, e.g.:

Parameter             Symbol    Min    Max    Unit      Notes
--------------------------------------------------------------
Valid block number     NVB     1004    1024   Blocks     1

>From this number we can deduce the maximum number of bad PEB that a device will
contain during its endurance life: a 128MiB NAND flash (1024 PEB) will not have
less than 20 bad blocks during the flash endurance life.

But the manufacturer doesn't tell where those bad block will appear. He doesn't
say either if they will be equally disposed on the whole device (and I'm pretty
sure they won't). So, according to the datasheets, we should reserve the
maximum number of bad PEB for each UBI device (worst case scenario: 20 bad
blocks appears on the smallest MTD partition).

So this patch make UBI use the whole MTD device size to calculate the maximum
bad expected eraseblocks.

The Kconfig option is in per1024 blocks, thus it can have a default value of 20
which is *very* common for NAND devices.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 drivers/mtd/ubi/Kconfig |   27 +++++++++++++++++++++------
 drivers/mtd/ubi/build.c |   21 ++++++++++++++++++---
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index b2f4f0f..f326877 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -28,14 +28,29 @@ config MTD_UBI_WL_THRESHOLD
 	  to 128 or 256, although it does not have to be power of 2).
 
 config MTD_UBI_BEB_LIMIT
-	int "Percentage of maximum expected bad eraseblocks"
-	default 2
-	range 0 25
+	int "Maximum expected bad eraseblock count per 1024 eraseblocks"
+	default 20
+	range 2 256
 	help
 	  This option specifies the maximum bad physical eraseblocks UBI
-	  expects on the UBI device (percents of total number of physical
-	  eraseblocks on this MTD partition). If the underlying flash does not
-	  admit of bad eraseblocks (e.g. NOR flash), this value is ignored.
+	  expects on the MTD device (per 1024 eraseblocks). If the underlying
+	  flash does not admit of bad eraseblocks (e.g. NOR flash), this value
+	  is ignored.
+
+	  NAND datasheets often specify the minimum and maximum NVM (Number of
+	  Valid Blocks) for the flashes' endurance lifetime. The maximum
+	  expected bad eraseblocks per 1024 eraseblocks then can be calculated
+	  as "1024 * (1 - MinNVB / MaxNVB)", which gives 20 for most NANDs
+	  (MaxNVB is basically the total count of eraseblocks on the chip).
+
+	  To put it differently, if this value is 20, UBI will try to reserve
+	  about 1.9% of physical eraseblocks for bad blocks handling. And that
+	  will be 1.9% of eraseblocks on the entire NAND chip, not just the MTD
+	  partition UBI attaches. This means that if you have, say, a NAND
+	  flash chip admits maximum 40 bad eraseblocks, and it is split on two
+	  MTD partitions of the same size, UBI will reserve 40 eraseblocks when
+	  attaching a partition.
+
 	  Leave the default value if unsure.
 
 config MTD_UBI_GLUEBI
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 7b6b5f9..738772c 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -36,6 +36,7 @@
 #include <linux/namei.h>
 #include <linux/stat.h>
 #include <linux/miscdevice.h>
+#include <linux/mtd/partitions.h>
 #include <linux/log2.h>
 #include <linux/kthread.h>
 #include <linux/kernel.h>
@@ -610,11 +611,25 @@ static int io_init(struct ubi_device *ubi)
 	if (mtd_can_have_bb(ubi->mtd)) {
 		ubi->bad_allowed = 1;
 		if (CONFIG_MTD_UBI_BEB_LIMIT > 0) {
-			int percent = CONFIG_MTD_UBI_BEB_LIMIT;
-			int limit = mult_frac(ubi->peb_count, percent, 100);
+			int per1024 = CONFIG_MTD_UBI_BEB_LIMIT;
+			int limit, device_pebs;
+			uint64_t device_size;
+
+			/*
+			 * Here we are using size of the entire flash chip and
+			 * not just the MTD partition size because the maximum
+			 * number of bad eraseblocks is a percentage of the
+			 * whole device and bad eraseblocks are not fairly
+			 * distributed over the flash chip. So the worst case
+			 * is that all the bad eraseblocks of the chip are in
+			 * the MTD partition we are attaching (ubi->mtd).
+			 */
+			device_size = mtd_get_device_size(ubi->mtd);
+			device_pebs = mtd_div_by_eb(device_size, ubi->mtd);
+			limit = mult_frac(device_pebs, per1024, 1024);
 
 			/* Round it up */
-			if (mult_frac(limit, 100, percent) < ubi->peb_count)
+			if (mult_frac(limit, 1024, per1024) < device_pebs)
 				limit += 1;
 			ubi->bad_peb_limit = limit;
 		}
-- 
1.7.10.4

[PATCH 2/2] arm: sam9_l9260_defconfig: adjust UBI bad eraseblocks limit

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

UBI has changed the MTD_UBI_BEB_LIMIT semantics. It used to be a percent of
total amount of eraseblock in the partition, and now it is the maximum
amount of bad eraseblocks on the entire devise per 1024 eraseblocks. So not
only the units changed, but also the meaning. But anyway, old 3% roughly
correspond to new 30, so change the defconfig correspondingly.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 arch/arm/configs/sam9_l9260_defconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/configs/sam9_l9260_defconfig b/arch/arm/configs/sam9_l9260_defconfig
index da276f9..d11fea5 100644
--- a/arch/arm/configs/sam9_l9260_defconfig
+++ b/arch/arm/configs/sam9_l9260_defconfig
@@ -39,7 +39,7 @@ CONFIG_MTD_NAND=y
 CONFIG_MTD_NAND_ATMEL=y
 CONFIG_MTD_NAND_PLATFORM=y
 CONFIG_MTD_UBI=y
-CONFIG_MTD_UBI_BEB_LIMIT=3
+CONFIG_MTD_UBI_BEB_LIMIT=30
 CONFIG_MTD_UBI_GLUEBI=y
 CONFIG_BLK_DEV_LOOP=y
 CONFIG_BLK_DEV_RAM=y
-- 
1.7.10.4

Re: [PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 724 bytes --]

On Thu, 2012-08-16 at 11:57 +0300, Shmulik Ladkani wrote:
> 
> For the simplest systems (those having one ubi device) that need a
> limit
> *other* than the default (20 per 1024), they can simply set the config
> to their chosen value, as they were used to.
> 
> With you approach, these system MUST pass the limit parameter via the
> ioctl / module-parameter. 

Yeah, when you change the default, you usually need to do an extra step.
It does not feel too bad, and I would not keep additional configuration
option for a hypothetical user. If someone suffers, we can add an option
to change the default. But I'd start without it. So, I think it is OK to
remove this.


-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 497 bytes --]

On Thu, 2012-08-16 at 13:42 +0300, Shmulik Ladkani wrote:
> 
> Does it make sense to set a zero limit? dunno.
> For testing purposes, maybe.
> 
> Artem, what do you think? prohibit a zero beb limit? 

We do not have that big user-base. No one uses 0 in the tree, most use
the default. I never heard anyone using 0. I did not use it also. I
think it is OK to have the lower range start from non-zero. But why it
is 2 and not 1 - I am not sure :-)

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Shmulik Ladkani]

On Thu, 16 Aug 2012 16:28:38 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Thu, 2012-08-16 at 11:57 +0300, Shmulik Ladkani wrote:
> > 
> > For the simplest systems (those having one ubi device) that need a
> > limit
> > *other* than the default (20 per 1024), they can simply set the config
> > to their chosen value, as they were used to.
> > 
> > With you approach, these system MUST pass the limit parameter via the
> > ioctl / module-parameter. 
> 
> Yeah, when you change the default, you usually need to do an extra step.
> It does not feel too bad, and I would not keep additional configuration
> option for a hypothetical user. If someone suffers, we can add an option
> to change the default. But I'd start without it. So, I think it is OK to
> remove this.

Yes, but the main drawback I was referring to is those platforms already
setting MTD_UBI_BEB_RESERVE other than the default, by means of kernel
configuration.
(there's one platform known to do so in its defconfig, that's
sam9_l9260_defconfig, which uses 3% instead of the "standard" 2%).

These platforms must now change their usermode code to either pass a
module parameter during the insmod or change their attach ioctl of
their application.

We force these systems to change their usermode because we changed ubi's
default BEB limit to be 20/1024 _hardcoded_ (instead of kernel
configurable as previously was).

Is this ok?

Regards,
Shmulik

Re: [PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Richard Genoud]

2012/8/16 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> On Thu, 16 Aug 2012 16:28:38 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
>> On Thu, 2012-08-16 at 11:57 +0300, Shmulik Ladkani wrote:
>> >
>> > For the simplest systems (those having one ubi device) that need a
>> > limit
>> > *other* than the default (20 per 1024), they can simply set the config
>> > to their chosen value, as they were used to.
>> >
>> > With you approach, these system MUST pass the limit parameter via the
>> > ioctl / module-parameter.
>>
>> Yeah, when you change the default, you usually need to do an extra step.
>> It does not feel too bad, and I would not keep additional configuration
>> option for a hypothetical user. If someone suffers, we can add an option
>> to change the default. But I'd start without it. So, I think it is OK to
>> remove this.
>
> Yes, but the main drawback I was referring to is those platforms already
> setting MTD_UBI_BEB_RESERVE other than the default, by means of kernel
> configuration.
> (there's one platform known to do so in its defconfig, that's
> sam9_l9260_defconfig, which uses 3% instead of the "standard" 2%).
I found the board:
https://www.olimex.com/dev/sam9-L9260.html
and the nand datasheet:
http://www.rockbox.org/wiki/pub/Main/LyrePrototype/K9xxG08UXM.pdf
page 11, we can see that the max_bad_bebper1024 is 25 (100 for 4096)

> These platforms must now change their usermode code to either pass a
> module parameter during the insmod or change their attach ioctl of
> their application.
>
> We force these systems to change their usermode because we changed ubi's
> default BEB limit to be 20/1024 _hardcoded_ (instead of kernel
> configurable as previously was).
>
> Is this ok?
well, I don't know, Artem, you're the maintainer :)
I made a quick patch on this, if you decide to apply it.

Richard


-- 
for me, ck means con kolivas and not calvin klein... does it mean I'm a geek ?

[PATCH 0/2] splitting "UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter"

[Richard Genoud]

Artem,
Here are the 2 patches you requested in place of
"[PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter"


I made some changes to correct some things that Shmulik pointed out:

* max_beb_per1024 is now an unsigned.
* ubi_mtd_param_parse can fail on kstrtouint error.

Richard Genoud (2):
  UBI: replace MTD_UBI_BEB_LIMIT with module parameter
  UBI: add ioctl for max_beb_per1024

 arch/arm/configs/sam9_l9260_defconfig |    1 -
 drivers/mtd/ubi/Kconfig               |   26 ---------
 drivers/mtd/ubi/build.c               |   93 ++++++++++++++++++++++-----------
 drivers/mtd/ubi/cdev.c                |   10 +++-
 drivers/mtd/ubi/ubi.h                 |    6 ++-
 include/mtd/ubi-user.h                |   19 ++++++-
 6 files changed, 95 insertions(+), 60 deletions(-)

-- 
1.7.2.5

[PATCH 1/2] UBI: replace MTD_UBI_BEB_LIMIT with module parameter

[Richard Genoud]

This patch provides the possibility to adjust the "maximum expected number of
bad blocks per 1024 blocks" (max_beb_per1024) for each mtd device.

The majority of NAND devices have their max_beb_per1024 equal to 20, but
sometimes it's more.
Now, we can adjust that via a kernel parameter:
ubi.mtd=<name|num|path>[,<vid_hdr_offs>[,max_beb_per1024]]

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 arch/arm/configs/sam9_l9260_defconfig |    1 -
 drivers/mtd/ubi/Kconfig               |   26 ---------
 drivers/mtd/ubi/build.c               |   93 ++++++++++++++++++++++-----------
 drivers/mtd/ubi/cdev.c                |    3 +-
 drivers/mtd/ubi/ubi.h                 |    6 ++-
 5 files changed, 70 insertions(+), 59 deletions(-)

diff --git a/arch/arm/configs/sam9_l9260_defconfig b/arch/arm/configs/sam9_l9260_defconfig
index d11fea5..47dd71a 100644
--- a/arch/arm/configs/sam9_l9260_defconfig
+++ b/arch/arm/configs/sam9_l9260_defconfig
@@ -39,7 +39,6 @@ CONFIG_MTD_NAND=y
 CONFIG_MTD_NAND_ATMEL=y
 CONFIG_MTD_NAND_PLATFORM=y
 CONFIG_MTD_UBI=y
-CONFIG_MTD_UBI_BEB_LIMIT=30
 CONFIG_MTD_UBI_GLUEBI=y
 CONFIG_BLK_DEV_LOOP=y
 CONFIG_BLK_DEV_RAM=y
diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index f326877..7a57cc0 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -27,32 +27,6 @@ config MTD_UBI_WL_THRESHOLD
 	  life-cycle less than 10000, the threshold should be lessened (e.g.,
 	  to 128 or 256, although it does not have to be power of 2).
 
-config MTD_UBI_BEB_LIMIT
-	int "Maximum expected bad eraseblock count per 1024 eraseblocks"
-	default 20
-	range 2 256
-	help
-	  This option specifies the maximum bad physical eraseblocks UBI
-	  expects on the MTD device (per 1024 eraseblocks). If the underlying
-	  flash does not admit of bad eraseblocks (e.g. NOR flash), this value
-	  is ignored.
-
-	  NAND datasheets often specify the minimum and maximum NVM (Number of
-	  Valid Blocks) for the flashes' endurance lifetime. The maximum
-	  expected bad eraseblocks per 1024 eraseblocks then can be calculated
-	  as "1024 * (1 - MinNVB / MaxNVB)", which gives 20 for most NANDs
-	  (MaxNVB is basically the total count of eraseblocks on the chip).
-
-	  To put it differently, if this value is 20, UBI will try to reserve
-	  about 1.9% of physical eraseblocks for bad blocks handling. And that
-	  will be 1.9% of eraseblocks on the entire NAND chip, not just the MTD
-	  partition UBI attaches. This means that if you have, say, a NAND
-	  flash chip admits maximum 40 bad eraseblocks, and it is split on two
-	  MTD partitions of the same size, UBI will reserve 40 eraseblocks when
-	  attaching a partition.
-
-	  Leave the default value if unsure.
-
 config MTD_UBI_GLUEBI
 	tristate "MTD devices emulation driver (gluebi)"
 	help
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 738772c..65edbc0 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -46,6 +46,8 @@
 /* Maximum length of the 'mtd=' parameter */
 #define MTD_PARAM_LEN_MAX 64
 
+#define MTD_PARAM_NB_MAX 3
+
 #ifdef CONFIG_MTD_UBI_MODULE
 #define ubi_is_module() 1
 #else
@@ -57,10 +59,12 @@
  * @name: MTD character device node path, MTD device name, or MTD device number
  *        string
  * @vid_hdr_offs: VID header offset
+ * @max_beb_per1024: maximum expected number of bad blocks per 1024 erase blocks
  */
 struct mtd_dev_param {
 	char name[MTD_PARAM_LEN_MAX];
 	int vid_hdr_offs;
+	unsigned int max_beb_per1024;
 };
 
 /* Numbers of elements set in the @mtd_dev_param array */
@@ -565,9 +569,37 @@ void ubi_free_internal_volumes(struct ubi_device *ubi)
 	}
 }
 
+static int get_bad_peb_limit(const struct ubi_device *ubi,
+			     unsigned int max_beb_per1024)
+{
+	int device_pebs;
+	uint64_t device_size;
+	int limit = 0;
+
+	/*
+	 * Here we are using size of the entire flash chip and
+	 * not just the MTD partition size because the maximum
+	 * number of bad eraseblocks is a percentage of the
+	 * whole device and bad eraseblocks are not fairly
+	 * distributed over the flash chip. So the worst case
+	 * is that all the bad eraseblocks of the chip are in
+	 * the MTD partition we are attaching (ubi->mtd).
+	 */
+	device_size = mtd_get_device_size(ubi->mtd);
+	device_pebs = mtd_div_by_eb(device_size, ubi->mtd);
+	limit = mult_frac(device_pebs, max_beb_per1024, 1024);
+
+	/* Round it up */
+	if (mult_frac(limit, 1024, max_beb_per1024) < device_pebs)
+		limit += 1;
+
+	return limit;
+}
+
 /**
  * io_init - initialize I/O sub-system for a given UBI device.
  * @ubi: UBI device description object
+ * @max_beb_per1024: maximum expected number of bad PEB per 1024 PEB
  *
  * If @ubi->vid_hdr_offset or @ubi->leb_start is zero, default offsets are
  * assumed:
@@ -580,7 +612,7 @@ void ubi_free_internal_volumes(struct ubi_device *ubi)
  * This function returns zero in case of success and a negative error code in
  * case of failure.
  */
-static int io_init(struct ubi_device *ubi)
+static int io_init(struct ubi_device *ubi, unsigned int max_beb_per1024)
 {
 	if (ubi->mtd->numeraseregions != 0) {
 		/*
@@ -610,29 +642,7 @@ static int io_init(struct ubi_device *ubi)
 
 	if (mtd_can_have_bb(ubi->mtd)) {
 		ubi->bad_allowed = 1;
-		if (CONFIG_MTD_UBI_BEB_LIMIT > 0) {
-			int per1024 = CONFIG_MTD_UBI_BEB_LIMIT;
-			int limit, device_pebs;
-			uint64_t device_size;
-
-			/*
-			 * Here we are using size of the entire flash chip and
-			 * not just the MTD partition size because the maximum
-			 * number of bad eraseblocks is a percentage of the
-			 * whole device and bad eraseblocks are not fairly
-			 * distributed over the flash chip. So the worst case
-			 * is that all the bad eraseblocks of the chip are in
-			 * the MTD partition we are attaching (ubi->mtd).
-			 */
-			device_size = mtd_get_device_size(ubi->mtd);
-			device_pebs = mtd_div_by_eb(device_size, ubi->mtd);
-			limit = mult_frac(device_pebs, per1024, 1024);
-
-			/* Round it up */
-			if (mult_frac(limit, 1024, per1024) < device_pebs)
-				limit += 1;
-			ubi->bad_peb_limit = limit;
-		}
+		ubi->bad_peb_limit = get_bad_peb_limit(ubi, max_beb_per1024);
 	}
 
 	if (ubi->mtd->type == MTD_NORFLASH) {
@@ -825,6 +835,7 @@ static int autoresize(struct ubi_device *ubi, int vol_id)
  * @mtd: MTD device description object
  * @ubi_num: number to assign to the new UBI device
  * @vid_hdr_offset: VID header offset
+ * @max_beb_per1024: maximum number of expected bad blocks per 1024 eraseblocks
  *
  * This function attaches MTD device @mtd_dev to UBI and assign @ubi_num number
  * to the newly created UBI device, unless @ubi_num is %UBI_DEV_NUM_AUTO, in
@@ -835,7 +846,8 @@ static int autoresize(struct ubi_device *ubi, int vol_id)
  * Note, the invocations of this function has to be serialized by the
  * @ubi_devices_mutex.
  */
-int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset)
+int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
+		       int vid_hdr_offset, unsigned int max_beb_per1024)
 {
 	struct ubi_device *ubi;
 	int i, err, ref = 0;
@@ -908,7 +920,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset)
 	dbg_msg("sizeof(struct ubi_ainf_peb) %zu", sizeof(struct ubi_ainf_peb));
 	dbg_msg("sizeof(struct ubi_wl_entry) %zu", sizeof(struct ubi_wl_entry));
 
-	err = io_init(ubi);
+	err = io_init(ubi, max_beb_per1024);
 	if (err)
 		goto out_free;
 
@@ -1197,7 +1209,7 @@ static int __init ubi_init(void)
 
 		mutex_lock(&ubi_devices_mutex);
 		err = ubi_attach_mtd_dev(mtd, UBI_DEV_NUM_AUTO,
-					 p->vid_hdr_offs);
+					 p->vid_hdr_offs, p->max_beb_per1024);
 		mutex_unlock(&ubi_devices_mutex);
 		if (err < 0) {
 			ubi_err("cannot attach mtd%d", mtd->index);
@@ -1316,7 +1328,8 @@ static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp)
 	struct mtd_dev_param *p;
 	char buf[MTD_PARAM_LEN_MAX];
 	char *pbuf = &buf[0];
-	char *tokens[2] = {NULL, NULL};
+	char *tokens[MTD_PARAM_NB_MAX];
+	int err;
 
 	if (!val)
 		return -EINVAL;
@@ -1346,7 +1359,7 @@ static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp)
 	if (buf[len - 1] == '\n')
 		buf[len - 1] = '\0';
 
-	for (i = 0; i < 2; i++)
+	for (i = 0; i < MTD_PARAM_NB_MAX; i++)
 		tokens[i] = strsep(&pbuf, ",");
 
 	if (pbuf) {
@@ -1364,18 +1377,38 @@ static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp)
 	if (p->vid_hdr_offs < 0)
 		return p->vid_hdr_offs;
 
+	if (tokens[2]) {
+		err = kstrtouint(tokens[2], 10, &p->max_beb_per1024);
+		if (err) {
+			pr_err("UBI error: bad value for max_beb_per1024 parameter: %s",
+			       tokens[2]);
+			return -EINVAL;
+		}
+	}
+
+	/*
+	 * A value of 0 is forced to the default value to keep the same
+	 * behavior between ubiattach command and module parameter
+	 */
+	if (!p->max_beb_per1024)
+		p->max_beb_per1024 = MTD_UBI_DEFAULT_BEB_LIMIT;
+
 	mtd_devs += 1;
 	return 0;
 }
 
 module_param_call(mtd, ubi_mtd_param_parse, NULL, NULL, 000);
 MODULE_PARM_DESC(mtd, "MTD devices to attach. Parameter format: "
-		      "mtd=<name|num|path>[,<vid_hdr_offs>].\n"
+		      "mtd=<name|num|path>[,<vid_hdr_offs>[,max_beb_per1024]].\n"
 		      "Multiple \"mtd\" parameters may be specified.\n"
 		      "MTD devices may be specified by their number, name, or "
 		      "path to the MTD character device node.\n"
 		      "Optional \"vid_hdr_offs\" parameter specifies UBI VID "
 		      "header position to be used by UBI.\n"
+		      "Optional \"max_beb_per1024\" parameter specifies the "
+		      "maximum expected bad eraseblock per 1024 eraseblocks."
+		      "(default " __stringify(MTD_UBI_DEFAULT_BEB_LIMIT)
+		      " if 0 or not set)\n"
 		      "Example 1: mtd=/dev/mtd0 - attach MTD device "
 		      "/dev/mtd0.\n"
 		      "Example 2: mtd=content,1984 mtd=4 - attach MTD device "
diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c
index fb55678..e0027e7 100644
--- a/drivers/mtd/ubi/cdev.c
+++ b/drivers/mtd/ubi/cdev.c
@@ -1010,7 +1010,8 @@ static long ctrl_cdev_ioctl(struct file *file, unsigned int cmd,
 		 * 'ubi_attach_mtd_dev()'.
 		 */
 		mutex_lock(&ubi_devices_mutex);
-		err = ubi_attach_mtd_dev(mtd, req.ubi_num, req.vid_hdr_offset);
+		err = ubi_attach_mtd_dev(mtd, req.ubi_num, req.vid_hdr_offset,
+					 MTD_UBI_DEFAULT_BEB_LIMIT);
 		mutex_unlock(&ubi_devices_mutex);
 		if (err < 0)
 			put_mtd_device(mtd);
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index c94612e..f926343 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -50,6 +50,9 @@
 /* UBI name used for character devices, sysfs, etc */
 #define UBI_NAME_STR "ubi"
 
+/* Default number of maximum expected bad blocks per 1024 eraseblocks */
+#define MTD_UBI_DEFAULT_BEB_LIMIT 20
+
 /* Normal UBI messages */
 #define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__)
 /* UBI warning messages */
@@ -693,7 +696,8 @@ int ubi_io_write_vid_hdr(struct ubi_device *ubi, int pnum,
 			 struct ubi_vid_hdr *vid_hdr);
 
 /* build.c */
-int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset);
+int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
+		       int vid_hdr_offset, unsigned int max_beb_per1024);
 int ubi_detach_mtd_dev(int ubi_num, int anyway);
 struct ubi_device *ubi_get_device(int ubi_num);
 void ubi_put_device(struct ubi_device *ubi);
-- 
1.7.2.5

[PATCH 2/2] UBI: add ioctl for max_beb_per1024

[Richard Genoud]

This patch provides the possibility to adjust the "maximum expected number of
bad blocks per 1024 blocks" (max_beb_per1024) for each mtd device from
UBI_IOCATT ioctl.

The majority of NAND devices have their max_beb_per1024 equal to 20, but
sometimes it's more.
We already could adjust that via a kernel parameter, now we can also use
UBI_IOCATT ioctl:
struct ubi_attach_req {
	__s32 ubi_num;
	__s32 mtd_num;
	__s32 vid_hdr_offset;
	__u8 max_beb_per1024;
	__s8 padding[11];
};

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/mtd/ubi/cdev.c |    9 ++++++++-
 include/mtd/ubi-user.h |   19 ++++++++++++++++++-
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c
index e0027e7..d268b42 100644
--- a/drivers/mtd/ubi/cdev.c
+++ b/drivers/mtd/ubi/cdev.c
@@ -1006,12 +1006,19 @@ static long ctrl_cdev_ioctl(struct file *file, unsigned int cmd,
 		}
 
 		/*
+		 * For compatibility with older user-space tools,
+		 * a zero value should be treated like a default value
+		 */
+		if (!req.max_beb_per1024)
+			req.max_beb_per1024 = MTD_UBI_DEFAULT_BEB_LIMIT;
+
+		/*
 		 * Note, further request verification is done by
 		 * 'ubi_attach_mtd_dev()'.
 		 */
 		mutex_lock(&ubi_devices_mutex);
 		err = ubi_attach_mtd_dev(mtd, req.ubi_num, req.vid_hdr_offset,
-					 MTD_UBI_DEFAULT_BEB_LIMIT);
+					 req.max_beb_per1024);
 		mutex_unlock(&ubi_devices_mutex);
 		if (err < 0)
 			put_mtd_device(mtd);
diff --git a/include/mtd/ubi-user.h b/include/mtd/ubi-user.h
index 8787349..77cd0d1 100644
--- a/include/mtd/ubi-user.h
+++ b/include/mtd/ubi-user.h
@@ -222,6 +222,7 @@ enum {
  * @ubi_num: UBI device number to create
  * @mtd_num: MTD device number to attach
  * @vid_hdr_offset: VID header offset (use defaults if %0)
+ * @max_beb_per1024: Maximum expected bad eraseblocks per 1024 eraseblocks
  * @padding: reserved for future, not used, has to be zeroed
  *
  * This data structure is used to specify MTD device UBI has to attach and the
@@ -245,12 +246,28 @@ enum {
  * be 2KiB-64 bytes = 1984. Note, that this position is not even 512-bytes
  * aligned, which is OK, as UBI is clever enough to realize this is 4th
  * sub-page of the first page and add needed padding.
+ *
+ * The @max_beb_per1024 is the maximum bad eraseblocks UBI expects on the ubi
+ * device per 1024 eraseblocks.
+ * This value is often given in an other form in the NAND datasheet (min NVB
+ * i.e. minimal number of valid blocks). The maximum expected bad eraseblocks
+ * per 1024 is then:
+ *   1024 * (1 - MinNVB / MaxNVB)
+ * Which gives 20 for most NAND devices.
+ * This limit is used in order to derive amount of eraseblock UBI reserves for
+ * handling new bad blocks.
+ * If the device has more bad eraseblocks than this limit, UBI does not reserve
+ * any physical eraseblocks for new bad eraseblocks, but attempts to use
+ * available eraseblocks (if any).
+ * The accepted range is 0-255. If 0 is given, the default value
+ * MTD_UBI_DEFAULT_BEB_LIMIT will be used for compatibility.
  */
 struct ubi_attach_req {
 	__s32 ubi_num;
 	__s32 mtd_num;
 	__s32 vid_hdr_offset;
-	__s8 padding[12];
+	__u8 max_beb_per1024;
+	__s8 padding[11];
 };
 
 /**
-- 
1.7.2.5

[PATCH] UBI: use a config value for default BEB limit

[Richard Genoud]

This patch provides the possibility to adjust the default value for
"maximum expected number of bad blocks per 1024 blocks" from kernel
configuration.

sam9_l9260_defconfig has been adjusted as well. The nand device used by
this card has a NVB of 3996/4096 ie a MAX_BEB_LIMIT of 25/1024

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 arch/arm/configs/sam9_l9260_defconfig |    1 +
 drivers/mtd/ubi/Kconfig               |   29 +++++++++++++++++++++++++++++
 drivers/mtd/ubi/ubi.h                 |    2 +-
 3 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/arch/arm/configs/sam9_l9260_defconfig b/arch/arm/configs/sam9_l9260_defconfig
index 47dd71a..2c20c9e 100644
--- a/arch/arm/configs/sam9_l9260_defconfig
+++ b/arch/arm/configs/sam9_l9260_defconfig
@@ -39,6 +39,7 @@ CONFIG_MTD_NAND=y
 CONFIG_MTD_NAND_ATMEL=y
 CONFIG_MTD_NAND_PLATFORM=y
 CONFIG_MTD_UBI=y
+CONFIG_MTD_UBI_DEFAULT_BEB_LIMIT=25
 CONFIG_MTD_UBI_GLUEBI=y
 CONFIG_BLK_DEV_LOOP=y
 CONFIG_BLK_DEV_RAM=y
diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index 7a57cc0..30e8370 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -27,6 +27,35 @@ config MTD_UBI_WL_THRESHOLD
 	  life-cycle less than 10000, the threshold should be lessened (e.g.,
 	  to 128 or 256, although it does not have to be power of 2).
 
+config MTD_UBI_DEFAULT_BEB_LIMIT
+	int "Maximum expected bad eraseblock count per 1024 eraseblocks"
+	default 20
+	range 0 256
+	help
+	  This option specifies the default value for maximum bad physical
+	  eraseblocks UBI expects on the MTD device (per 1024 eraseblocks).
+	  It will be used when the maximum bad PEB value is null in the
+	  UBI_IOCATT ioctl (e.g. when calling ubiattach) or in the kernel
+	  parameter ubi.mtd.
+	  If the underlying flash does not admit of bad eraseblocks
+	  (e.g. NOR flash), this value is ignored.
+
+	  NAND datasheets often specify the minimum and maximum NVM (Number of
+	  Valid Blocks) for the flashes' endurance lifetime. The maximum
+	  expected bad eraseblocks per 1024 eraseblocks then can be calculated
+	  as "1024 * (1 - MinNVB / MaxNVB)", which gives 20 for most NANDs
+	  (MaxNVB is basically the total count of eraseblocks on the chip).
+
+	  To put it differently, if this value is 20, UBI will try to reserve
+	  about 1.9% of physical eraseblocks for bad blocks handling. And that
+	  will be 1.9% of eraseblocks on the entire NAND chip, not just the MTD
+	  partition UBI attaches. This means that if you have, say,  a NAND
+	  flash chip that admits a maximum 40 bad eraseblocks, and it is split
+	  on two MTD partitions of the same size, UBI will reserve 40
+	  eraseblocks when attaching a partition.
+
+	  Leave the default value if unsure.
+
 config MTD_UBI_GLUEBI
 	tristate "MTD devices emulation driver (gluebi)"
 	help
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index f926343..39a1e7e 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -51,7 +51,7 @@
 #define UBI_NAME_STR "ubi"
 
 /* Default number of maximum expected bad blocks per 1024 eraseblocks */
-#define MTD_UBI_DEFAULT_BEB_LIMIT 20
+#define MTD_UBI_DEFAULT_BEB_LIMIT CONFIG_MTD_UBI_DEFAULT_BEB_LIMIT
 
 /* Normal UBI messages */
 #define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__)
-- 
1.7.2.5

Re: [PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 852 bytes --]

On Thu, 2012-08-16 at 16:50 +0300, Shmulik Ladkani wrote:
> Yes, but the main drawback I was referring to is those platforms already
> setting MTD_UBI_BEB_RESERVE other than the default, by means of kernel
> configuration.
> (there's one platform known to do so in its defconfig, that's
> sam9_l9260_defconfig, which uses 3% instead of the "standard" 2%).

Yes, I understand this. I think there are few such systems and many of
them are fine to pass the parameter explicitly.

> We force these systems to change their usermode because we changed ubi's
> default BEB limit to be 20/1024 _hardcoded_ (instead of kernel
> configurable as previously was).

Not necessarily, they can come to us and ask to add the kernel option.

> Is this ok?

It is not ideal, but I am willing to take this risk.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 505 bytes --]

On Thu, 2012-08-16 at 16:30 +0200, Richard Genoud wrote:
> > (there's one platform known to do so in its defconfig, that's
> > sam9_l9260_defconfig, which uses 3% instead of the "standard" 2%).
> I found the board:
> https://www.olimex.com/dev/sam9-L9260.html
> and the nand datasheet:
> http://www.rockbox.org/wiki/pub/Main/LyrePrototype/K9xxG08UXM.pdf
> page 11, we can see that the max_bad_bebper1024 is 25 (100 for 4096)

OK, thanks for the research!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] UBI: replace MTD_UBI_BEB_LIMIT with module parameter

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 2636 bytes --]

Richard, would you please split this series differently:

1. Separate out the calculations to the get_bad_peb_limit() func.
2. Invent 
2. Add the module parameter
3. Extends the ioctl
4. Removes the Kconfig option

This will be much easier to review.

See also some comments below.

> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -46,6 +46,8 @@
>  /* Maximum length of the 'mtd=' parameter */
>  #define MTD_PARAM_LEN_MAX 64
>  
> +#define MTD_PARAM_NB_MAX 3

Please, make it to be

/* Maximum number of comma-separated items in ht 'mtd=' parameter */
#define MTD_PARAM_MAX_COUNT 3

instead.

> @@ -57,10 +59,12 @@
>   * @name: MTD character device node path, MTD device name, or MTD device number
>   *        string
>   * @vid_hdr_offs: VID header offset
> + * @max_beb_per1024: maximum expected number of bad blocks per 1024 erase blocks
>   */
>  struct mtd_dev_param {
>  	char name[MTD_PARAM_LEN_MAX];
>  	int vid_hdr_offs;
> +	unsigned int max_beb_per1024;
>  };

Please, make it to be just 'int' here and everywhere, just for
consistency with other parameters, which are 'int' (no other stronger
reason).

>  MODULE_PARM_DESC(mtd, "MTD devices to attach. Parameter format: "
> -		      "mtd=<name|num|path>[,<vid_hdr_offs>].\n"
> +		      "mtd=<name|num|path>[,<vid_hdr_offs>[,max_beb_per1024]].\n"
>  		      "Multiple \"mtd\" parameters may be specified.\n"
>  		      "MTD devices may be specified by their number, name, or "
>  		      "path to the MTD character device node.\n"
>  		      "Optional \"vid_hdr_offs\" parameter specifies UBI VID "
>  		      "header position to be used by UBI.\n"

This comment needs improvement. Consider a situation when I do not want
to specify vid_hdr_offs (want to use the default), but want to specify
max_beb_per1024. I think I can put 0 here which means "default". Would
you please verify this and add a comment about this in this help output.
Also, please, verify that the output looks OK using 'modinfo ubi'.

> +		      "Optional \"max_beb_per1024\" parameter specifies the "
> +		      "maximum expected bad eraseblock per 1024 eraseblocks."
> +		      "(default " __stringify(MTD_UBI_DEFAULT_BEB_LIMIT)
> +		      " if 0 or not set)\n"

Yeah, something like "if 0 or not set" is needed for 'vid_hdr_offs' as
well.

>  		      "Example 1: mtd=/dev/mtd0 - attach MTD device "
>  		      "/dev/mtd0.\n"
>  		      "Example 2: mtd=content,1984 mtd=4 - attach MTD device "

Could you also modify one of the examples and add "max_beb_per1024"
there?

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] UBI: add ioctl for max_beb_per1024

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

On Thu, 2012-08-16 at 16:52 +0200, Richard Genoud wrote:
>  		mutex_lock(&ubi_devices_mutex);
>  		err = ubi_attach_mtd_dev(mtd, req.ubi_num, req.vid_hdr_offset,
> -					 MTD_UBI_DEFAULT_BEB_LIMIT);
> +					 req.max_beb_per1024);

Do not forget to explicitely validate 'req.max_beb_per1024' valuse in
'ubi_attach_mtd_dev()', as we do for other parameters.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] ubiattach: introduce max_beb_per1024 in UBI_IOCATT

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 642 bytes --]

On Tue, 2012-07-10 at 18:23 +0200, Richard Genoud wrote:
> The ioctl UBI_IOCATT has been extended with max_beb_per1024 parameter.
> 
> This parameter is used for adjusting the "maximum expected number of
> bad blocks per 1024 blocks" for each mtd device.
> The number of physical erase blocks (PEB) that UBI will reserve for bad
> block handling is now:
> whole_flash_chipset__PEB_number * max_beb_per1024 / 1024

Could you please split this on 2 patches:

1. Update teh ubi-user.h file by syncing with the kernel. Do make
headers_install and just copy it to mtd-utils.
2. Do the change.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] UBI: replace MTD_UBI_BEB_LIMIT with module parameter

[Richard Genoud]

2012/8/17 Artem Bityutskiy <dedekind1@gmail.com>:
> Richard, would you please split this series differently:
>
> 1. Separate out the calculations to the get_bad_peb_limit() func.
> 2. Invent
> 2. Add the module parameter
> 3. Extends the ioctl
> 4. Removes the Kconfig option
>
> This will be much easier to review.
ok, will do that.

>
> See also some comments below.
>
>> --- a/drivers/mtd/ubi/build.c
>> +++ b/drivers/mtd/ubi/build.c
>> @@ -46,6 +46,8 @@
>>  /* Maximum length of the 'mtd=' parameter */
>>  #define MTD_PARAM_LEN_MAX 64
>>
>> +#define MTD_PARAM_NB_MAX 3
>
> Please, make it to be
>
> /* Maximum number of comma-separated items in ht 'mtd=' parameter */
> #define MTD_PARAM_MAX_COUNT 3
>
> instead.
done.
>
>> @@ -57,10 +59,12 @@
>>   * @name: MTD character device node path, MTD device name, or MTD device number
>>   *        string
>>   * @vid_hdr_offs: VID header offset
>> + * @max_beb_per1024: maximum expected number of bad blocks per 1024 erase blocks
>>   */
>>  struct mtd_dev_param {
>>       char name[MTD_PARAM_LEN_MAX];
>>       int vid_hdr_offs;
>> +     unsigned int max_beb_per1024;
>>  };
>
> Please, make it to be just 'int' here and everywhere, just for
> consistency with other parameters, which are 'int' (no other stronger
> reason).
ok, I put it back to an int.

>>  MODULE_PARM_DESC(mtd, "MTD devices to attach. Parameter format: "
>> -                   "mtd=<name|num|path>[,<vid_hdr_offs>].\n"
>> +                   "mtd=<name|num|path>[,<vid_hdr_offs>[,max_beb_per1024]].\n"
>>                     "Multiple \"mtd\" parameters may be specified.\n"
>>                     "MTD devices may be specified by their number, name, or "
>>                     "path to the MTD character device node.\n"
>>                     "Optional \"vid_hdr_offs\" parameter specifies UBI VID "
>>                     "header position to be used by UBI.\n"
>
> This comment needs improvement. Consider a situation when I do not want
> to specify vid_hdr_offs (want to use the default), but want to specify
> max_beb_per1024. I think I can put 0 here which means "default". Would
> you please verify this and add a comment about this in this help output.
you're right, if vid_hdr_offs is 0, the default value is taken.
I'll improve the comments accordingly.
> Also, please, verify that the output looks OK using 'modinfo ubi'.
ok.
>> +                   "Optional \"max_beb_per1024\" parameter specifies the "
>> +                   "maximum expected bad eraseblock per 1024 eraseblocks."
>> +                   "(default " __stringify(MTD_UBI_DEFAULT_BEB_LIMIT)
>> +                   " if 0 or not set)\n"
>
> Yeah, something like "if 0 or not set" is needed for 'vid_hdr_offs' as
> well.
>
>>                     "Example 1: mtd=/dev/mtd0 - attach MTD device "
>>                     "/dev/mtd0.\n"
>>                     "Example 2: mtd=content,1984 mtd=4 - attach MTD device "
>
> Could you also modify one of the examples and add "max_beb_per1024"
> there?
yes, of course.

> --
> Best Regards,
> Artem Bityutskiy



-- 
for me, ck means con kolivas and not calvin klein... does it mean I'm a geek ?

Re: [PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Shmulik Ladkani]

On Thu, 16 Aug 2012 16:33:32 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> We do not have that big user-base. No one uses 0 in the tree, most use
> the default. I never heard anyone using 0. I did not use it also. I
> think it is OK to have the lower range start from non-zero. But why it
> is 2 and not 1 - I am not sure :-)

Artem,

Note that originally range was 0..25 (percentage).

It was changed to be 2..256 (per 1024) by 07ad322 in linux-ubi.

Seems arbitrary change.
So if you'd like, we can fix to 1..256 (or 0..256) - as you prefer.

Regards,
Shmulik

Re: [PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 812 bytes --]

On Sun, 2012-08-19 at 10:09 +0300, Shmulik Ladkani wrote:
> On Thu, 16 Aug 2012 16:33:32 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > We do not have that big user-base. No one uses 0 in the tree, most use
> > the default. I never heard anyone using 0. I did not use it also. I
> > think it is OK to have the lower range start from non-zero. But why it
> > is 2 and not 1 - I am not sure :-)
> 
> Artem,
> 
> Note that originally range was 0..25 (percentage).
> 
> It was changed to be 2..256 (per 1024) by 07ad322 in linux-ubi.
> 
> Seems arbitrary change.
> So if you'd like, we can fix to 1..256 (or 0..256) - as you prefer.

Yeah, I wanted to make it 1..256 but forgot, will do now. 0..256 would
need some more work to avoid division by 0.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Richard Genoud]

Hi Artem,
2012/8/19 Artem Bityutskiy <dedekind1@gmail.com>:
> Yeah, I wanted to make it 1..256 but forgot, will do now. 0..256 would
> need some more work to avoid division by 0.
Division by 0 is handled in the get_bad_peb_limit() function, I don't
see another dangerous place.
So, I think that we can change back the range to 0..256.
(and if we want to be coherent with user-space, it should be 0..255,
as the range is coded with an u8)

Richard.

-- 
for me, ck means con kolivas and not calvin klein... does it mean I'm a geek ?

Re: [PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

On Mon, 2012-08-20 at 08:55 +0200, Richard Genoud wrote:
> Hi Artem,
> 2012/8/19 Artem Bityutskiy <dedekind1@gmail.com>:
> > Yeah, I wanted to make it 1..256 but forgot, will do now. 0..256 would
> > need some more work to avoid division by 0.
> Division by 0 is handled in the get_bad_peb_limit() function, I don't
> see another dangerous place.

        if (mult_frac(limit, 1024, max_beb_per1024) < device_pebs)

will divide by 0 if max_beb_per1024 is 0.

> (and if we want to be coherent with user-space, it should be 0..255,
> as the range is coded with an u8)

I think it should be uint16_t instead, because we are defining ABI here
and we should not assume no one will ever nee values higher than 255.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/4] UBI: replace MTD_UBI_BEB_LIMIT with user-space parameter

[Richard Genoud]

2012/8/20 Artem Bityutskiy <dedekind1@gmail.com>:
> On Mon, 2012-08-20 at 08:55 +0200, Richard Genoud wrote:
>> Hi Artem,
>> 2012/8/19 Artem Bityutskiy <dedekind1@gmail.com>:
>> > Yeah, I wanted to make it 1..256 but forgot, will do now. 0..256 would
>> > need some more work to avoid division by 0.
>> Division by 0 is handled in the get_bad_peb_limit() function, I don't
>> see another dangerous place.
>
>         if (mult_frac(limit, 1024, max_beb_per1024) < device_pebs)
>
> will divide by 0 if max_beb_per1024 is 0.
>
just a few lines before, you've got:
        /*
         * We don't want a division by 0, and having max_beb_per1024 == 0 is ok
         */
        if (!max_beb_per1024)
                return 0;
from commit abae1f1
(or I'm not looking at the right line ?)

>> (and if we want to be coherent with user-space, it should be 0..255,
>> as the range is coded with an u8)
>
> I think it should be uint16_t instead, because we are defining ABI here
> and we should not assume no one will ever nee values higher than 255.
I agree with you, even if 25% of reserved space for bad block seems
insane, we never know...
I'll update that.



-- 
for me, ck means con kolivas and not calvin klein... does it mean I'm a geek ?