[PATCH 0/8] UBI: add max_beb_per1024 parameter / ioctl

[PATCH 0/8] UBI: add max_beb_per1024 parameter / ioctl

[Richard Genoud]

Artem,
This patch serie introduce, step by step the kernel module parameter
max_beb_per1024, then the ioctl and finally drop the kernel config option
CONFIG_MTD_UBI_BEB_LIMIT.

It's based on top of linux-ubi/master (and I added your patch on
sam9_l9260_defconfig).
BTW, I let the BEB value at 30 in sam9_l9260_defconfig, you can set it at 25 if
you want (as it's the value from the datasheet).

Best Regards

Richard.

Artem Bityutskiy (1):
  arm: sam9_l9260_defconfig: adjust UBI bad eraseblocks limit

Richard Genoud (7):
  UBI: separate bad_peb_limit in a function
  UBI: introduce MTD_PARAM_MAX_COUNT
  UBI: prepare for max_beb_per1024 module parameter addition
  UBI: check max_beb_per1024 value in ubi_attach_mtd_dev
  UBI: replace MTD_UBI_BEB_LIMIT with module parameter
  UBI: add ioctl for max_beb_per1024
  UBI: drop CONFIG_MTD_UBI_BEB_LIMIT

 arch/arm/configs/sam9_l9260_defconfig |    1 -
 drivers/mtd/ubi/Kconfig               |   26 -------
 drivers/mtd/ubi/build.c               |  116 +++++++++++++++++++++-----------
 drivers/mtd/ubi/cdev.c                |    3 +-
 drivers/mtd/ubi/ubi.h                 |    6 ++-
 include/mtd/ubi-user.h                |   19 +++++-
 6 files changed, 101 insertions(+), 70 deletions(-)

-- 
1.7.2.5

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

[Richard Genoud]

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 files changed, 1 insertions(+), 1 deletions(-)

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.2.5

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

[Artem Bityutskiy]

On Fri, 2012-08-17 at 16:35 +0200, Richard Genoud wrote:
> 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

I'll just remove this from from the defconfig and make it use the
default, which is more sensible for this device, as your research
showed.

-- 
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120819/65142d0c/attachment-0001.sig>

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

[Artem Bityutskiy]

On Sun, 2012-08-19 at 22:21 +0300, Artem Bityutskiy wrote:
> I'll just remove this from from the defconfig and make it use the
> default, which is more sensible for this device, as your research
> showed.

Actually, pushed this patch to l2-mtd.git:


From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Date: Sun, 19 Aug 2012 22:22:23 +0300
Subject: [PATCH] arm: sam9_l9260_defconfig: correct CONFIG_MTD_UBI_BEB_LIMIT

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.

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

"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)"

Thus, use "25" for sam9.

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..b4384af 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=25
 CONFIG_MTD_UBI_GLUEBI=y
 CONFIG_BLK_DEV_LOOP=y
 CONFIG_BLK_DEV_RAM=y
-- 
1.7.11.2

-- 
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120819/ae63b25d/attachment.sig>

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

[Jean-Christophe PLAGNIOL-VILLARD]

On 16:35 Fri 17 Aug     , Richard Genoud wrote:
> 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>
simply drop this defconfig

Best Regards,
J.

[PATCH 2/8] UBI: separate bad_peb_limit in a function

[Richard Genoud]

No functional changes here, just to prepare for next patch.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/mtd/ubi/build.c |   57 ++++++++++++++++++++++++++++-------------------
 1 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 738772c..82d11e1 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -565,6 +565,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 limit, device_pebs;
+	uint64_t device_size;
+
+	/*
+	 * We don't want a division by 0, and having max_beb_per1024 == 0 is ok
+	 */
+	if (!max_beb_per1024)
+		return 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
@@ -582,6 +613,8 @@ void ubi_free_internal_volumes(struct ubi_device *ubi)
  */
 static int io_init(struct ubi_device *ubi)
 {
+	const int max_beb_per1024 = CONFIG_MTD_UBI_BEB_LIMIT;
+
 	if (ubi->mtd->numeraseregions != 0) {
 		/*
 		 * Some flashes have several erase regions. Different regions
@@ -610,29 +643,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) {
-- 
1.7.2.5

[PATCH 2/8] UBI: separate bad_peb_limit in a function

[Shmulik Ladkani]

On Fri, 17 Aug 2012 16:35:18 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
> No functional changes here, just to prepare for next patch.
> 
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>

Reviewed-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>

[PATCH 2/8] UBI: separate bad_peb_limit in a function

[Artem Bityutskiy]

On Fri, 2012-08-17 at 16:35 +0200, Richard Genoud wrote:
> No functional changes here, just to prepare for next patch.
> 
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>

Pushed this one to linux-ubi, thanks!

-- 
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120819/1403a498/attachment.sig>

[PATCH 3/8] UBI: introduce MTD_PARAM_MAX_COUNT

[Richard Genoud]

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

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 82d11e1..1f45f51 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -46,6 +46,9 @@
 /* Maximum length of the 'mtd=' parameter */
 #define MTD_PARAM_LEN_MAX 64
 
+/* Maximum number of comma-separated items in ht 'mtd=' parameter */
+#define MTD_PARAM_MAX_COUNT 2
+
 #ifdef CONFIG_MTD_UBI_MODULE
 #define ubi_is_module() 1
 #else
@@ -1327,7 +1330,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_MAX_COUNT];
 
 	if (!val)
 		return -EINVAL;
@@ -1357,7 +1360,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_MAX_COUNT; i++)
 		tokens[i] = strsep(&pbuf, ",");
 
 	if (pbuf) {
-- 
1.7.2.5

[PATCH 3/8] UBI: introduce MTD_PARAM_MAX_COUNT

[Artem Bityutskiy]

On Fri, 2012-08-17 at 16:35 +0200, Richard Genoud wrote:
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> ---
>  drivers/mtd/ubi/build.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index 82d11e1..1f45f51 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -46,6 +46,9 @@
>  /* Maximum length of the 'mtd=' parameter */
>  #define MTD_PARAM_LEN_MAX 64
>  
> +/* Maximum number of comma-separated items in ht 'mtd=' parameter */
> +#define MTD_PARAM_MAX_COUNT 2

Fixed the spelling of "the" and pushed to l2-mtd.git, thanks!

-- 
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120819/d1b0a538/attachment.sig>

[PATCH 3/8] UBI: introduce MTD_PARAM_MAX_COUNT

[Artem Bityutskiy]

On Fri, 2012-08-17 at 16:35 +0200, Richard Genoud wrote:
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> ---
>  drivers/mtd/ubi/build.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index 82d11e1..1f45f51 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -46,6 +46,9 @@
>  /* Maximum length of the 'mtd=' parameter */
>  #define MTD_PARAM_LEN_MAX 64
>  
> +/* Maximum number of comma-separated items in ht 'mtd=' parameter */
> +#define MTD_PARAM_MAX_COUNT 2

Fixed the spelling of "the" and pushed to linux-ubi.git, thanks!

-- 
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120819/43454bc0/attachment.sig>

[PATCH 4/8] UBI: prepare for max_beb_per1024 module parameter addition

[Richard Genoud]

This patch prepare the way for the addition of max_beb_per1024 module
parameter.
There's no functional change.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/mtd/ubi/build.c |   14 ++++++++------
 drivers/mtd/ubi/cdev.c  |    3 ++-
 drivers/mtd/ubi/ubi.h   |    3 ++-
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 1f45f51..58fe53d 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -602,6 +602,7 @@ static int get_bad_peb_limit(const struct ubi_device *ubi, int max_beb_per1024)
 /**
  * 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:
@@ -614,10 +615,8 @@ static int get_bad_peb_limit(const struct ubi_device *ubi, int max_beb_per1024)
  * 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)
 {
-	const int max_beb_per1024 = CONFIG_MTD_UBI_BEB_LIMIT;
-
 	if (ubi->mtd->numeraseregions != 0) {
 		/*
 		 * Some flashes have several erase regions. Different regions
@@ -839,6 +838,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
@@ -849,7 +849,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;
@@ -922,7 +923,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;
 
@@ -1211,7 +1212,8 @@ 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,
+					 CONFIG_MTD_UBI_BEB_LIMIT);
 		mutex_unlock(&ubi_devices_mutex);
 		if (err < 0) {
 			ubi_err("cannot attach mtd%d", mtd->index);
diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c
index fb55678..619f914 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,
+					 CONFIG_MTD_UBI_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..2a2475b 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -693,7 +693,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);
-- 
1.7.2.5

[PATCH 4/8] UBI: prepare for max_beb_per1024 module parameter addition

[Shmulik Ladkani]

On Fri, 17 Aug 2012 16:35:20 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
> This patch prepare the way for the addition of max_beb_per1024 module
> parameter.
> There's no functional change.
> 
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>

Reviewed-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>

[PATCH 5/8] UBI: check max_beb_per1024 value in ubi_attach_mtd_dev

[Richard Genoud]

max_beb_per1024 shouldn't be negative, and a 0 value will be treated as
the default value.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/mtd/ubi/build.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 58fe53d..ec7311f 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -855,6 +855,16 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
 	struct ubi_device *ubi;
 	int i, err, ref = 0;
 
+	if (max_beb_per1024 < 0)
+		return -EINVAL;
+
+	/*
+	 * A value of 0 is forced to the default value to keep the same
+	 * behavior between ubiattach command and module parameter
+	 */
+	if (!max_beb_per1024)
+		max_beb_per1024 = CONFIG_MTD_UBI_BEB_LIMIT;
+
 	/*
 	 * Check if we already have the same MTD device attached.
 	 *
-- 
1.7.2.5

[PATCH 5/8] UBI: check max_beb_per1024 value in ubi_attach_mtd_dev

[Shmulik Ladkani]

Hi Richard,

On Fri, 17 Aug 2012 16:35:21 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
> +	/*
> +	 * A value of 0 is forced to the default value to keep the same
> +	 * behavior between ubiattach command and module parameter
> +	 */

Minor thing.

Since the module parameter is not yet introduced (only in a later
patch), and since last part of sentence isn't that important, I would
simply state:

+	 * Use the default if max_beb_per1024 isn't provided.

or alike.

Regards,
Shmulik

[PATCH 5/8] UBI: check max_beb_per1024 value in ubi_attach_mtd_dev

[Richard Genoud]

Hi Shmulik,

2012/8/19 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> Hi Richard,
>
> On Fri, 17 Aug 2012 16:35:21 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
>> +     /*
>> +      * A value of 0 is forced to the default value to keep the same
>> +      * behavior between ubiattach command and module parameter
>> +      */
>
> Minor thing.
>
> Since the module parameter is not yet introduced (only in a later
> patch), and since last part of sentence isn't that important, I would
> simply state:
>
> +        * Use the default if max_beb_per1024 isn't provided.
>
That's right, I'll correct that in next version.
Thanks !

[PATCH 5/8] UBI: check max_beb_per1024 value in ubi_attach_mtd_dev

[Artem Bityutskiy]

On Mon, 2012-08-20 at 08:31 +0200, Richard Genoud wrote:
> That's right, I'll correct that in next version.
> Thanks !

I've amended this patch, added Shmulik's reviewed-by and pushed to
linux-ubi.git, thanks!

-- 
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120820/167a9045/attachment.sig>

[PATCH 5/8] UBI: check max_beb_per1024 value in ubi_attach_mtd_dev

[Artem Bityutskiy]

On Mon, 2012-08-20 at 11:25 +0300, Artem Bityutskiy wrote:
> On Mon, 2012-08-20 at 08:31 +0200, Richard Genoud wrote:
> > That's right, I'll correct that in next version.
> > Thanks !
> 
> I've amended this patch, added Shmulik's reviewed-by and pushed to
> linux-ubi.git, thanks!

Sorry, actually I've dropped it - it does not compile :-) Please, send
bisectable patch-sets. Anyway, I expect you to re-send patches 5-8.

P.S. I am not sure we need to CC arm mailing list and lkml - I think we
spam them unnecessarily and the MTD list is enough.

-- 
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120820/b1a6f574/attachment.sig>

[PATCH 6/8] 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>
---
 drivers/mtd/ubi/Kconfig |    2 ++
 drivers/mtd/ubi/build.c |   38 ++++++++++++++++++++++++--------------
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index f326877..8b67bac 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -51,6 +51,8 @@ config MTD_UBI_BEB_LIMIT
 	  MTD partitions of the same size, UBI will reserve 40 eraseblocks when
 	  attaching a partition.
 
+	  This option can be overridden by the kernel parameter ubi.mtd.
+
 	  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 ec7311f..cc05022 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -47,7 +47,7 @@
 #define MTD_PARAM_LEN_MAX 64
 
 /* Maximum number of comma-separated items in ht 'mtd=' parameter */
-#define MTD_PARAM_MAX_COUNT 2
+#define MTD_PARAM_MAX_COUNT 3
 
 #ifdef CONFIG_MTD_UBI_MODULE
 #define ubi_is_module() 1
@@ -60,10 +60,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 */
@@ -1222,8 +1224,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,
-					 CONFIG_MTD_UBI_BEB_LIMIT);
+					 p->vid_hdr_offs, p->max_beb_per1024);
 		mutex_unlock(&ubi_devices_mutex);
 		if (err < 0) {
 			ubi_err("cannot attach mtd%d", mtd->index);
@@ -1343,6 +1344,7 @@ static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp)
 	char buf[MTD_PARAM_LEN_MAX];
 	char *pbuf = &buf[0];
 	char *tokens[MTD_PARAM_MAX_COUNT];
+	int err;
 
 	if (!val)
 		return -EINVAL;
@@ -1390,23 +1392,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]) {
+		err = kstrtoint(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;
+		}
+	}
+
 	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"
+MODULE_PARM_DESC(mtd, "MTD devices to attach. Parameter format: 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"
-		      "Example 1: mtd=/dev/mtd0 - attach MTD device "
-		      "/dev/mtd0.\n"
-		      "Example 2: mtd=content,1984 mtd=4 - attach MTD device "
-		      "with name \"content\" using VID header offset 1984, and "
-		      "MTD device number 4 with default VID header offset.");
+		      "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. (default value if 0 or not set)\n"
+		      "Optional \"max_beb_per1024\" parameter specifies the maximum expected bad eraseblock per 1024 eraseblocks. (default value ("
+		      __stringify(CONFIG_MTD_UBI_BEB_LIMIT) ") if 0 or not set)\n"
+		      "\n"
+		      "Example 1: mtd=/dev/mtd0 - attach MTD device /dev/mtd0.\n"
+		      "Example 2: mtd=content,1984 mtd=4 - attach MTD device with name \"content\" using VID header offset 1984, and MTD device number 4 with default VID header offset.\n"
+		      "Example 3: mtd=/dev/mtd1,0,25 - attach MTD device /dev/mtd1 using default VID header offset and reserve 25*nand_size_in_blocks/1024 erase blocks for bad block handling.\n"
+		      "\t(e.g. if the NAND *chipset* has 4096 PEB, 100 will be reserved for this UBI device).");
 
 MODULE_VERSION(__stringify(UBI_VERSION));
 MODULE_DESCRIPTION("UBI - Unsorted Block Images");
-- 
1.7.2.5

[PATCH 6/8] UBI: replace MTD_UBI_BEB_LIMIT with module parameter

[Shmulik Ladkani]

Hi Richard,

On Fri, 17 Aug 2012 16:35:22 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
> +		      "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. (default value if 0 or not set)\n"
> +		      "Optional \"max_beb_per1024\" parameter specifies the maximum expected bad eraseblock per 1024 eraseblocks. (default value ("
> +		      __stringify(CONFIG_MTD_UBI_BEB_LIMIT) ") if 0 or not set)\n"

Looks like "mtd=/dev/mtd1,,25" is invalid, although we state "default
value if 0 or not set".

Also, lines are exceeding 80 chars, not sure this is more readable than
if they were broken.

Regards,
Shmulik

[PATCH 6/8] UBI: replace MTD_UBI_BEB_LIMIT with module parameter

[Richard Genoud]

2012/8/19 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> Hi Richard,
>
> On Fri, 17 Aug 2012 16:35:22 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
>> +                   "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. (default value if 0 or not set)\n"
>> +                   "Optional \"max_beb_per1024\" parameter specifies the maximum expected bad eraseblock per 1024 eraseblocks. (default value ("
>> +                   __stringify(CONFIG_MTD_UBI_BEB_LIMIT) ") if 0 or not set)\n"
>
> Looks like "mtd=/dev/mtd1,,25" is invalid, although we state "default
> value if 0 or not set".
I'm going to check that.
>
> Also, lines are exceeding 80 chars, not sure this is more readable than
> if they were broken.
Actually, I didn't split those lines to make checkpatch happy.
Checkpatch.pl doesn't like split strings because they are hard to grep.
So there's an exception on the 80 column rule for strings.

>
> Regards,
> Shmulik



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

[PATCH 6/8] UBI: replace MTD_UBI_BEB_LIMIT with module parameter

[Richard Genoud]

2012/8/20 Richard Genoud <richard.genoud@gmail.com>:
> 2012/8/19 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
>> Hi Richard,
>>
>> On Fri, 17 Aug 2012 16:35:22 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
>>> +                   "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. (default value if 0 or not set)\n"
>>> +                   "Optional \"max_beb_per1024\" parameter specifies the maximum expected bad eraseblock per 1024 eraseblocks. (default value ("
>>> +                   __stringify(CONFIG_MTD_UBI_BEB_LIMIT) ") if 0 or not set)\n"
>>
>> Looks like "mtd=/dev/mtd1,,25" is invalid, although we state "default
>> value if 0 or not set".
> I'm going to check that.
I made a patch to accept empty string value.
I'll send it with the rest of the patch serie.

Regards,
Richard.

[PATCH 7/8] 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/Kconfig |    3 ++-
 drivers/mtd/ubi/cdev.c  |    2 +-
 include/mtd/ubi-user.h  |   19 ++++++++++++++++++-
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index 8b67bac..56531a7 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -51,7 +51,8 @@ config MTD_UBI_BEB_LIMIT
 	  MTD partitions of the same size, UBI will reserve 40 eraseblocks when
 	  attaching a partition.
 
-	  This option can be overridden by the kernel parameter ubi.mtd.
+	  This option can be overridden by the kernel parameter ubi.mtd and the
+	  ioctl UBI_IOCATT.
 
 	  Leave the default value if unsure.
 
diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c
index 619f914..7885dc0 100644
--- a/drivers/mtd/ubi/cdev.c
+++ b/drivers/mtd/ubi/cdev.c
@@ -1011,7 +1011,7 @@ static long ctrl_cdev_ioctl(struct file *file, unsigned int cmd,
 		 */
 		mutex_lock(&ubi_devices_mutex);
 		err = ubi_attach_mtd_dev(mtd, req.ubi_num, req.vid_hdr_offset,
-					 CONFIG_MTD_UBI_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..8168d9b 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 kernel config value
+ * CONFIG_MTD_UBI_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 8/8] UBI: drop CONFIG_MTD_UBI_BEB_LIMIT

[Richard Genoud]

This option can be set by a kernel parameter and an ioctl, so we may not
need the kernel config option any more.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 arch/arm/configs/sam9_l9260_defconfig |    1 -
 drivers/mtd/ubi/Kconfig               |   29 -----------------------------
 drivers/mtd/ubi/build.c               |    4 ++--
 drivers/mtd/ubi/ubi.h                 |    3 +++
 include/mtd/ubi-user.h                |    2 +-
 5 files changed, 6 insertions(+), 33 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 56531a7..7a57cc0 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -27,35 +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.
-
-	  This option can be overridden by the kernel parameter ubi.mtd and the
-	  ioctl UBI_IOCATT.
-
-	  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 cc05022..d940a12 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -865,7 +865,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
 	 * behavior between ubiattach command and module parameter
 	 */
 	if (!max_beb_per1024)
-		max_beb_per1024 = CONFIG_MTD_UBI_BEB_LIMIT;
+		max_beb_per1024 = MTD_UBI_DEFAULT_BEB_LIMIT;
 
 	/*
 	 * Check if we already have the same MTD device attached.
@@ -1411,7 +1411,7 @@ MODULE_PARM_DESC(mtd, "MTD devices to attach. Parameter format: mtd=<name|num|pa
 		      "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. (default value if 0 or not set)\n"
 		      "Optional \"max_beb_per1024\" parameter specifies the maximum expected bad eraseblock per 1024 eraseblocks. (default value ("
-		      __stringify(CONFIG_MTD_UBI_BEB_LIMIT) ") if 0 or not set)\n"
+		      __stringify(MTD_UBI_DEFAULT_BEB_LIMIT) ") if 0 or not set)\n"
 		      "\n"
 		      "Example 1: mtd=/dev/mtd0 - attach MTD device /dev/mtd0.\n"
 		      "Example 2: mtd=content,1984 mtd=4 - attach MTD device with name \"content\" using VID header offset 1984, and MTD device number 4 with default VID header offset.\n"
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 2a2475b..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 */
diff --git a/include/mtd/ubi-user.h b/include/mtd/ubi-user.h
index 8168d9b..0b25f4c 100644
--- a/include/mtd/ubi-user.h
+++ b/include/mtd/ubi-user.h
@@ -260,7 +260,7 @@ enum {
  * 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 config value
- * CONFIG_MTD_UBI_BEB_LIMIT will be used for compatibility.
+ * MTD_UBI_DEFAULT_BEB_LIMIT will be used for compatibility.
  */
 struct ubi_attach_req {
 	__s32 ubi_num;
-- 
1.7.2.5