[PATCH] ubi: attach: do not return -EINVAL if the mtd->numeraseregions is 1

[PATCH] ubi: attach: do not return -EINVAL if the mtd->numeraseregions is 1

[Huang Shijie]

If the master mtd does not have any slave mtd partitions,
and its numeraseregions is one(only has one erease block), and
we attach the master mtd with : ubiattach -m 0 -d 0

We will meet the error:
-------------------------------------------------------
root@freescale ~$ ubiattach /dev/ubi_ctrl -m 0 -d 0
UBI: attaching mtd0 to ubi0
UBI error: io_init: multiple regions, not implemented
ubiattach: error!: cannot attach mtd0
           error 22 (Invalid argument)
-------------------------------------------------------

In fact, if there is only one "erase block", we should not
prevent the attach.

This patch fixes it.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/ubi/build.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index a561335..006f98a 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -638,7 +638,7 @@ static int io_init(struct ubi_device *ubi, int max_beb_per1024)
 	dbg_gen("sizeof(struct ubi_ainf_peb) %zu", sizeof(struct ubi_ainf_peb));
 	dbg_gen("sizeof(struct ubi_wl_entry) %zu", sizeof(struct ubi_wl_entry));
 
-	if (ubi->mtd->numeraseregions != 0) {
+	if (ubi->mtd->numeraseregions > 1) {
 		/*
 		 * Some flashes have several erase regions. Different regions
 		 * may have different eraseblock size and other
-- 
1.7.1

Re: [PATCH] ubi: attach: do not return -EINVAL if the mtd->numeraseregions is 1

[Huang Shijie]

于 2013年09月18日 10:17, Huang Shijie 写道:
> If the master mtd does not have any slave mtd partitions,
> and its numeraseregions is one(only has one erease block), and
> we attach the master mtd with : ubiattach -m 0 -d 0
>
> We will meet the error:
> -------------------------------------------------------
> root@freescale ~$ ubiattach /dev/ubi_ctrl -m 0 -d 0
> UBI: attaching mtd0 to ubi0
> UBI error: io_init: multiple regions, not implemented
> ubiattach: error!: cannot attach mtd0
>            error 22 (Invalid argument)
> -------------------------------------------------------
>
> In fact, if there is only one "erase block", we should not
> prevent the attach.
>
> This patch fixes it.
>
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/ubi/build.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index a561335..006f98a 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -638,7 +638,7 @@ static int io_init(struct ubi_device *ubi, int max_beb_per1024)
>  	dbg_gen("sizeof(struct ubi_ainf_peb) %zu", sizeof(struct ubi_ainf_peb));
>  	dbg_gen("sizeof(struct ubi_wl_entry) %zu", sizeof(struct ubi_wl_entry));
>  
> -	if (ubi->mtd->numeraseregions != 0) {
> +	if (ubi->mtd->numeraseregions > 1) {
>  		/*
>  		 * Some flashes have several erase regions. Different regions
>  		 * may have different eraseblock size and other
Hi Artem:
could you please check this patch?


thanks
Huang Shijie

Re: [PATCH] ubi: attach: do not return -EINVAL if the mtd->numeraseregions is 1

[Artem Bityutskiy]

On Wed, 2013-09-18 at 10:17 +0800, Huang Shijie wrote:
> -	if (ubi->mtd->numeraseregions != 0) {
> +	if (ubi->mtd->numeraseregions > 1) {

The purpose of this code is to refuse "composite" mtd devices, simply
because UBIFS authors never had any of them and did not know how to deal
with them.

numeraseregions == 0 means "no erase regions". This is the ABI, as we
expose this piece of information via sysfs, see
"Documentation/ABI/testing/sysfs-class-mtd".

numeraseregions > 1 means that this is some kind of a composite flash
consisting of several regions with different eraseblock sizes. I've
never saw these, but I guess there is always one large "main" region and
some small special-purpose regions.

What is 'numeraseregions == 1'? How is it different to 'numeraseregions
== 0'?

Is the "main" area covered by mtd->size, or 'mtd->size' spans the entire
chip and includes all the regions?

What is the flash you are dealing with? Surely this is not gpmi-nand?

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH] ubi: attach: do not return -EINVAL if the mtd->numeraseregions is 1

[Huang Shijie]

于 2013年10月26日 17:47, Artem Bityutskiy 写道:
> What is 'numeraseregions == 1'? How is it different to 'numeraseregions
> == 0'?
>
> Is the "main" area covered by mtd->size, or 'mtd->size' spans the entire
> chip and includes all the regions?
>
i think the mtd->size spans the entire chip and include the all the regions.


> What is the flash you are dealing with? Surely this is not gpmi-nand?
I meet this bug with a Micron's Parallel Nor: M29W256GL
it is not used by the gpmi-nand, it's used by weim driver: 
drivers/bus/imx-weim.c

this NOR's Number of erase block regions is 1, i quote the test here, 
see page 50
(i will send you the datasheet in a private email):

"
   It specifes the number of regions containning contiguous erase blocks 
of the same size.

"

please see allocate_partition():

   
---------------------------------------------------------------------------------
   if (master->numeraseregions > 1) {
       ................
   } else {
       /* Single erase size */
   }

   
---------------------------------------------------------------------------------

this NOR is 32MB, its master partion has 1 for "numeraseregions". if we 
add a 32MB slave partition
  to this NOR, the slave partition does NOT inherit the numeraseregions 
of the master partition.
So the ubiattach will succeed with the slave partition.

But if we attach the master partition directly, we will fails.

thanks
Huang Shijie

Re: [PATCH] ubi: attach: do not return -EINVAL if the mtd->numeraseregions is 1

[Huang Shijie]

于 2013年09月18日 10:17, Huang Shijie 写道:
> If the master mtd does not have any slave mtd partitions,
> and its numeraseregions is one(only has one erease block), and
> we attach the master mtd with : ubiattach -m 0 -d 0
>
> We will meet the error:
> -------------------------------------------------------
> root@freescale ~$ ubiattach /dev/ubi_ctrl -m 0 -d 0
> UBI: attaching mtd0 to ubi0
> UBI error: io_init: multiple regions, not implemented
> ubiattach: error!: cannot attach mtd0
>            error 22 (Invalid argument)
> -------------------------------------------------------
>
> In fact, if there is only one "erase block", we should not
> prevent the attach.
>
> This patch fixes it.
>
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/ubi/build.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index a561335..006f98a 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -638,7 +638,7 @@ static int io_init(struct ubi_device *ubi, int max_beb_per1024)
>  	dbg_gen("sizeof(struct ubi_ainf_peb) %zu", sizeof(struct ubi_ainf_peb));
>  	dbg_gen("sizeof(struct ubi_wl_entry) %zu", sizeof(struct ubi_wl_entry));
>  
> -	if (ubi->mtd->numeraseregions != 0) {
> +	if (ubi->mtd->numeraseregions > 1) {
>  		/*
>  		 * Some flashes have several erase regions. Different regions
>  		 * may have different eraseblock size and other
hi Artem:

Any comment about this patch?

thanks
Huang Shijie