[patch 4/4] mtd/nand: fix multi-chip suspend problem

[patch 4/4] mtd/nand: fix multi-chip suspend problem

[akpm]

From: Li Yang <leoli@freescale.com>

Symptom:
device_suspend(): mtd_cls_suspend+0x0/0x58 returns -11
PM: Device mtd14 failed to suspend: error -11
PM: Some devices failed to suspend

This patch enables other chips to be suspended if the active chip of
the controller has been suspended.

Signed-off-by: Jin Qing <b24347@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/mtd/nand/nand_base.c |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff -puN drivers/mtd/nand/nand_base.c~mtd-nand-fix-multi-chip-suspend-problem drivers/mtd/nand/nand_base.c
--- a/drivers/mtd/nand/nand_base.c~mtd-nand-fix-multi-chip-suspend-problem
+++ a/drivers/mtd/nand/nand_base.c
@@ -697,10 +697,16 @@ nand_get_device(struct nand_chip *chip, 
 		spin_unlock(lock);
 		return 0;
 	}
-	if (new_state == FL_PM_SUSPENDED) {
-		spin_unlock(lock);
-		return (chip->state == FL_PM_SUSPENDED) ? 0 : -EAGAIN;
-	}
+	if (new_state == FL_PM_SUSPENDED)
+		if (chip->controller->active->state == FL_PM_SUSPENDED) {
+			chip->state = FL_PM_SUSPENDED;
+			spin_unlock(lock);
+			return 0;
+		} else {
+			spin_unlock(lock);
+			return -EAGAIN;
+		}
+
 	set_current_state(TASK_UNINTERRUPTIBLE);
 	add_wait_queue(wq, &wait);
 	spin_unlock(lock);
_

Re: [patch 4/4] mtd/nand: fix multi-chip suspend problem

[Artem Bityutskiy]

On Tue, 2009-11-17 at 14:45 -0800, akpm@linux-foundation.org wrote:
> From: Li Yang <leoli@freescale.com>
> 
> Symptom:
> device_suspend(): mtd_cls_suspend+0x0/0x58 returns -11
> PM: Device mtd14 failed to suspend: error -11
> PM: Some devices failed to suspend
> 
> This patch enables other chips to be suspended if the active chip of
> the controller has been suspended.
> 
> Signed-off-by: Jin Qing <b24347@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  drivers/mtd/nand/nand_base.c |   14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff -puN drivers/mtd/nand/nand_base.c~mtd-nand-fix-multi-chip-suspend-problem drivers/mtd/nand/nand_base.c
> --- a/drivers/mtd/nand/nand_base.c~mtd-nand-fix-multi-chip-suspend-problem
> +++ a/drivers/mtd/nand/nand_base.c
> @@ -697,10 +697,16 @@ nand_get_device(struct nand_chip *chip, 
>  		spin_unlock(lock);
>  		return 0;
>  	}
> -	if (new_state == FL_PM_SUSPENDED) {
> -		spin_unlock(lock);
> -		return (chip->state == FL_PM_SUSPENDED) ? 0 : -EAGAIN;
> -	}
> +	if (new_state == FL_PM_SUSPENDED)
> +		if (chip->controller->active->state == FL_PM_SUSPENDED) {
> +			chip->state = FL_PM_SUSPENDED;
> +			spin_unlock(lock);
> +			return 0;
> +		} else {
> +			spin_unlock(lock);
> +			return -EAGAIN;
> +		}
> +
>  	set_current_state(TASK_UNINTERRUPTIBLE);
>  	add_wait_queue(wq, &wait);
>  	spin_unlock(lock);

This patch casing the following gcc warning:

drivers/mtd/nand/nand_base.c: In function ‘nand_get_device’:
drivers/mtd/nand/nand_base.c:700: warning: suggest explicit braces to
avoid ambiguous ‘else’

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Ubiformat --erase-counter not always used

[Darwin Rambo]

Hello Artem, 

This is not a bug report, just an observation on ubiformat. It only seems to take the --erase-counter parameter in certain situations. If you have alien data it seems to take it. If the blocks are FF erased or previously formatted correctly it seems to ignore the --erase-counter argument, even though it says it is using it (see below).

Should we add something in the online help or documentation to explain this better and maybe say in the ubiformat tool itself when the switch is getting used or ignored and why?

# ./ubiformat /dev/mtd4 -n --erase-counter=1234 -v -y
ubiformat: mtd4 (NAND), size 2097152 bytes (2.0 MiB), 524288 eraseblocks of 524288 bytes (512.0 KiB), min. I/O size 4096 bytes
libscan: start scanning eraseblocks 0-4
libscan: scanning eraseblock 0: alien
libscan: scanning eraseblock 1: alien
libscan: scanning eraseblock 2: alien
libscan: scanning eraseblock 3: alien
libscan: finished, mean EC 0, 0 OK, 0 corrupted, 0 empty, 4 alien, bad 0
ubiformat: warning!: 4 of 4 eraseblocks contain non-ubifs data
ubiformat: use erase counter 1234 for all eraseblocks
ubiformat: eraseblock 0: erase, write EC 1234
ubiformat: eraseblock 1: erase, write EC 1234
ubiformat: eraseblock 2: erase, write EC 1234
ubiformat: eraseblock 3: erase, write EC 1234
192.168.1.105 # ./ubiformat /dev/mtd4 -n --erase-counter=5432 -v -y
ubiformat: mtd4 (NAND), size 2097152 bytes (2.0 MiB), 524288 eraseblocks of 524288 bytes (512.0 KiB), min. I/O size 4096 bytes
libscan: start scanning eraseblocks 0-4
libscan: scanning eraseblock 0: OK, erase counter 1234
libscan: scanning eraseblock 1: OK, erase counter 1234
libscan: scanning eraseblock 2: OK, erase counter 1234
libscan: scanning eraseblock 3: OK, erase counter 1234
libscan: finished, mean EC 1234, 4 OK, 0 corrupted, 0 empty, 0 alien, bad 0
ubiformat: 4 eraseblocks have valid erase counter, mean value is 1234
ubiformat: use erase counter 5432 for all eraseblocks    <------------------ Didn't take this value
ubiformat: eraseblock 0: erase, write EC 1235
ubiformat: eraseblock 1: erase, write EC 1235
ubiformat: eraseblock 2: erase, write EC 1235
ubiformat: eraseblock 3: erase, write EC 1235 

Thanks.

Regards,
Darwin

Re: Ubiformat --erase-counter not always used

[Artem Bityutskiy]

On Mon, 2009-11-23 at 10:57 -0800, Darwin Rambo wrote:
> Hello Artem, 
> 
> This is not a bug report, just an observation on ubiformat. It only seems to take the --erase-counter parameter in certain situations. If you have alien data it seems to take it. If the blocks are FF erased or previously formatted correctly it seems to ignore the --erase-counter argument, even though it says it is using it (see below).
> 
> Should we add something in the online help or documentation to explain this better and maybe say in the ubiformat tool itself when the switch is getting used or ignored and why?
> 
> # ./ubiformat /dev/mtd4 -n --erase-counter=1234 -v -y
> ubiformat: mtd4 (NAND), size 2097152 bytes (2.0 MiB), 524288 eraseblocks of 524288 bytes (512.0 KiB), min. I/O size 4096 bytes
> libscan: start scanning eraseblocks 0-4
> libscan: scanning eraseblock 0: alien
> libscan: scanning eraseblock 1: alien
> libscan: scanning eraseblock 2: alien
> libscan: scanning eraseblock 3: alien
> libscan: finished, mean EC 0, 0 OK, 0 corrupted, 0 empty, 4 alien, bad 0
> ubiformat: warning!: 4 of 4 eraseblocks contain non-ubifs data
> ubiformat: use erase counter 1234 for all eraseblocks
> ubiformat: eraseblock 0: erase, write EC 1234
> ubiformat: eraseblock 1: erase, write EC 1234
> ubiformat: eraseblock 2: erase, write EC 1234
> ubiformat: eraseblock 3: erase, write EC 1234
> 192.168.1.105 # ./ubiformat /dev/mtd4 -n --erase-counter=5432 -v -y
> ubiformat: mtd4 (NAND), size 2097152 bytes (2.0 MiB), 524288 eraseblocks of 524288 bytes (512.0 KiB), min. I/O size 4096 bytes
> libscan: start scanning eraseblocks 0-4
> libscan: scanning eraseblock 0: OK, erase counter 1234
> libscan: scanning eraseblock 1: OK, erase counter 1234
> libscan: scanning eraseblock 2: OK, erase counter 1234
> libscan: scanning eraseblock 3: OK, erase counter 1234
> libscan: finished, mean EC 1234, 4 OK, 0 corrupted, 0 empty, 0 alien, bad 0
> ubiformat: 4 eraseblocks have valid erase counter, mean value is 1234
> ubiformat: use erase counter 5432 for all eraseblocks    <------------------ Didn't take this value
> ubiformat: eraseblock 0: erase, write EC 1235
> ubiformat: eraseblock 1: erase, write EC 1235
> ubiformat: eraseblock 2: erase, write EC 1235
> ubiformat: eraseblock 3: erase, write EC 1235 

Looks like a bug to me. I think it should take the EC you ask it in all
cases. I'll look at this when I have a bit more time, thanks for the
report.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: Ubiformat --erase-counter not always used

[Artem Bityutskiy]

On Mon, 2009-11-23 at 10:57 -0800, Darwin Rambo wrote:
> Hello Artem, 
> 
> This is not a bug report, just an observation on ubiformat. It only seems to take the --erase-counter parameter in certain situations. If you have alien data it seems to take it. If the blocks are FF erased or previously formatted correctly it seems to ignore the --erase-counter argument, even though it says it is using it (see below).
> 
> Should we add something in the online help or documentation to explain this better and maybe say in the ubiformat tool itself when the switch is getting used or ignored and why?

Should be fixed in the mtd-utils repository now, thanks.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [patch 4/4] mtd/nand: fix multi-chip suspend problem

[Andrew Morton]

On Mon, 23 Nov 2009 17:19:04 +0200
Artem Bityutskiy <dedekind1@gmail.com> wrote:

> On Tue, 2009-11-17 at 14:45 -0800, akpm@linux-foundation.org wrote:
> > From: Li Yang <leoli@freescale.com>
> > 
> > Symptom:
> > device_suspend(): mtd_cls_suspend+0x0/0x58 returns -11
> > PM: Device mtd14 failed to suspend: error -11
> > PM: Some devices failed to suspend
> > 
> > This patch enables other chips to be suspended if the active chip of
> > the controller has been suspended.
> > 
> > Signed-off-by: Jin Qing <b24347@freescale.com>
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > ---
> > 
> >  drivers/mtd/nand/nand_base.c |   14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff -puN drivers/mtd/nand/nand_base.c~mtd-nand-fix-multi-chip-suspend-problem drivers/mtd/nand/nand_base.c
> > --- a/drivers/mtd/nand/nand_base.c~mtd-nand-fix-multi-chip-suspend-problem
> > +++ a/drivers/mtd/nand/nand_base.c
> > @@ -697,10 +697,16 @@ nand_get_device(struct nand_chip *chip, 
> >  		spin_unlock(lock);
> >  		return 0;
> >  	}
> > -	if (new_state == FL_PM_SUSPENDED) {
> > -		spin_unlock(lock);
> > -		return (chip->state == FL_PM_SUSPENDED) ? 0 : -EAGAIN;
> > -	}
> > +	if (new_state == FL_PM_SUSPENDED)
> > +		if (chip->controller->active->state == FL_PM_SUSPENDED) {
> > +			chip->state = FL_PM_SUSPENDED;
> > +			spin_unlock(lock);
> > +			return 0;
> > +		} else {
> > +			spin_unlock(lock);
> > +			return -EAGAIN;
> > +		}
> > +
> >  	set_current_state(TASK_UNINTERRUPTIBLE);
> >  	add_wait_queue(wq, &wait);
> >  	spin_unlock(lock);
> 
> This patch casing the following gcc warning:
> 
> drivers/mtd/nand/nand_base.c: In function ___nand_get_device___:
> drivers/mtd/nand/nand_base.c:700: warning: suggest explicit braces to
> avoid ambiguous ___else___
> 

Stupid gcc.

--- a/drivers/mtd/nand/nand_base.c~mtd-nand-fix-multi-chip-suspend-problem-fix
+++ a/drivers/mtd/nand/nand_base.c
@@ -697,7 +697,7 @@ nand_get_device(struct nand_chip *chip, 
 		spin_unlock(lock);
 		return 0;
 	}
-	if (new_state == FL_PM_SUSPENDED)
+	if (new_state == FL_PM_SUSPENDED) {
 		if (chip->controller->active->state == FL_PM_SUSPENDED) {
 			chip->state = FL_PM_SUSPENDED;
 			spin_unlock(lock);
@@ -706,7 +706,7 @@ nand_get_device(struct nand_chip *chip, 
 			spin_unlock(lock);
 			return -EAGAIN;
 		}
-
+	}
 	set_current_state(TASK_UNINTERRUPTIBLE);
 	add_wait_queue(wq, &wait);
 	spin_unlock(lock);
_