Patchwork [RFC,2/2] mtd : Make the mtd_suspend return 0 if the suspend is not implemented

login
register
mail settings
Submitter Artem Bityutskiy
Date Jan. 30, 2012, 11:45 a.m.
Message ID <1327923953.2487.13.camel@sauron.fi.intel.com>
Download mbox | patch
Permalink /patch/138551/
State RFC
Headers show

Comments

Artem Bityutskiy - Jan. 30, 2012, 11:45 a.m.
On Tue, 2012-01-24 at 09:06 +0000, Russell King - ARM Linux wrote:
> > However, the bug made it into the 3.3 merge window, so shouldn't this
> > bugfix be sent upstream immediately?
> 
> David is the MTD maintainer, and Artem just helps out.  I believe Artem
> is waiting for David to finish travelling before asking David (last seen
> at Hong Kong airport) to pull these fixes.  David in turn will pass them
> onto Linus.  Plus, Linus only started adding to -rc1 yesterday, so its a
> little early to expect this to be fixed.

Hi,

here is the latest version of the fix.

http://git.infradead.org/users/dedekind/l2-mtd-2.6.git/commit/283d43b9ce2952535aa89c0195085e2a1b7e5fce

Also attached.

From 283d43b9ce2952535aa89c0195085e2a1b7e5fce Mon Sep 17 00:00:00 2001
From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Date: Mon, 16 Jan 2012 11:07:16 +0200
Subject: [PATCH] mtd: fix MTD suspend

Commits 3fe4bae88460869a8e553397cd9057a4ee7ca341 and
079c985e7a6f4ce60f931cebfdd5ee3c3 broke MTD suspend in 2 ways:

1. When the '->suspend' method is not present, we return -EOPNOTSUPP,
but
   the callers of 'mtd_suspend()' expects 0 instead.
2. Checking of the 'mtd' parameter against NULL has been incorrectly
removed
   in 'mtd_cls_suspend()'.

This patch fixes the breakages. This has been found, analyzed, reported
and tested by Rafael J. Wysocki <rjw@sisk.pl>.

Note, this patch is not needed in the stable tree because it causes a
regression introduced during the v3.3 merge window.

Reported-by: Rafael J. Wysocki <rjw@sisk.pl>
Tested-by: Rafael J. Wysocki <rjw@sisk.pl>
Tested-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 drivers/mtd/mtdcore.c   |    2 +-
 include/linux/mtd/mtd.h |    4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

 static inline void mtd_resume(struct mtd_info *mtd)
Rafael J. Wysocki - Jan. 30, 2012, 10:58 p.m.
On Monday, January 30, 2012, Artem Bityutskiy wrote:
> On Tue, 2012-01-24 at 09:06 +0000, Russell King - ARM Linux wrote:
> > > However, the bug made it into the 3.3 merge window, so shouldn't this
> > > bugfix be sent upstream immediately?
> > 
> > David is the MTD maintainer, and Artem just helps out.  I believe Artem
> > is waiting for David to finish travelling before asking David (last seen
> > at Hong Kong airport) to pull these fixes.  David in turn will pass them
> > onto Linus.  Plus, Linus only started adding to -rc1 yesterday, so its a
> > little early to expect this to be fixed.
> 
> Hi,
> 
> here is the latest version of the fix.
> 
> http://git.infradead.org/users/dedekind/l2-mtd-2.6.git/commit/283d43b9ce2952535aa89c0195085e2a1b7e5fce
> 
> Also attached.
> 
> From 283d43b9ce2952535aa89c0195085e2a1b7e5fce Mon Sep 17 00:00:00 2001
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> Date: Mon, 16 Jan 2012 11:07:16 +0200
> Subject: [PATCH] mtd: fix MTD suspend
> 
> Commits 3fe4bae88460869a8e553397cd9057a4ee7ca341 and
> 079c985e7a6f4ce60f931cebfdd5ee3c3 broke MTD suspend in 2 ways:
> 
> 1. When the '->suspend' method is not present, we return -EOPNOTSUPP,
> but
>    the callers of 'mtd_suspend()' expects 0 instead.
> 2. Checking of the 'mtd' parameter against NULL has been incorrectly
> removed
>    in 'mtd_cls_suspend()'.
> 
> This patch fixes the breakages. This has been found, analyzed, reported
> and tested by Rafael J. Wysocki <rjw@sisk.pl>.
> 
> Note, this patch is not needed in the stable tree because it causes a
> regression introduced during the v3.3 merge window.
> 
> Reported-by: Rafael J. Wysocki <rjw@sisk.pl>
> Tested-by: Rafael J. Wysocki <rjw@sisk.pl>
> Tested-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Thanks, but is anyone actually going to push it to Linus any time soon?

If not, then I can do that through the linux-pm tree.  I'll be sending
at least one more pull request with fixes for v3.3.

Thanks,
Rafael


> ---
>  drivers/mtd/mtdcore.c   |    2 +-
>  include/linux/mtd/mtd.h |    4 +---
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index b265188..de96865 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -119,7 +119,7 @@ static int mtd_cls_suspend(struct device *dev,
> pm_message_t state)
>  {
>  	struct mtd_info *mtd = dev_get_drvdata(dev);
>  
> -	return mtd_suspend(mtd);
> +	return mtd ? mtd_suspend(mtd) : 0;
>  }
>  
>  static int mtd_cls_resume(struct device *dev)
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 1a81fde..d8c7aad 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -427,9 +427,7 @@ static inline int mtd_is_locked(struct mtd_info
> *mtd, loff_t ofs, uint64_t len)
>  
>  static inline int mtd_suspend(struct mtd_info *mtd)
>  {
> -	if (!mtd->suspend)
> -		return -EOPNOTSUPP;
> -	return mtd->suspend(mtd);
> +	return mtd->suspend ? mtd->suspend(mtd) : 0;
>  }
>  
>  static inline void mtd_resume(struct mtd_info *mtd)
>
Artem Bityutskiy - Feb. 2, 2012, 11:43 a.m.
On Mon, 2012-01-30 at 23:58 +0100, Rafael J. Wysocki wrote:
> Thanks, but is anyone actually going to push it to Linus any time soon?

I agree, but I am not the maintainer so cannot answer. David Woodhouse
is aware of this, but I do not know when he gonna send the pull request.
Rafael J. Wysocki - Feb. 2, 2012, 6:40 p.m.
On Thursday, February 02, 2012, Artem Bityutskiy wrote:
> On Mon, 2012-01-30 at 23:58 +0100, Rafael J. Wysocki wrote:
> > Thanks, but is anyone actually going to push it to Linus any time soon?
> 
> I agree, but I am not the maintainer so cannot answer. David Woodhouse
> is aware of this, but I do not know when he gonna send the pull request.

Well, the problem is being reported by more and more people, so I'd
like to push the fix quickly.

David, please let me know if you don't want me to include the $subject patch
into the next PM fixes pull request.  If I don't hear from you, I'll push it
next week for -rc4.

Thanks,
Rafael
Russell King - ARM Linux - Feb. 2, 2012, 8:21 p.m.
On Thu, Feb 02, 2012 at 07:40:26PM +0100, Rafael J. Wysocki wrote:
> On Thursday, February 02, 2012, Artem Bityutskiy wrote:
> > On Mon, 2012-01-30 at 23:58 +0100, Rafael J. Wysocki wrote:
> > > Thanks, but is anyone actually going to push it to Linus any time soon?
> > 
> > I agree, but I am not the maintainer so cannot answer. David Woodhouse
> > is aware of this, but I do not know when he gonna send the pull request.
> 
> Well, the problem is being reported by more and more people, so I'd
> like to push the fix quickly.
> 
> David, please let me know if you don't want me to include the $subject patch
> into the next PM fixes pull request.  If I don't hear from you, I'll push it
> next week for -rc4.

I too have it in my tree as a patch which Artem sent me, as it's rather
fundamental to not oopsing the kernel when you suspend/resume on any ARM
platform.
Rafael J. Wysocki - Feb. 2, 2012, 8:36 p.m.
On Thursday, February 02, 2012, Russell King - ARM Linux wrote:
> On Thu, Feb 02, 2012 at 07:40:26PM +0100, Rafael J. Wysocki wrote:
> > On Thursday, February 02, 2012, Artem Bityutskiy wrote:
> > > On Mon, 2012-01-30 at 23:58 +0100, Rafael J. Wysocki wrote:
> > > > Thanks, but is anyone actually going to push it to Linus any time soon?
> > > 
> > > I agree, but I am not the maintainer so cannot answer. David Woodhouse
> > > is aware of this, but I do not know when he gonna send the pull request.
> > 
> > Well, the problem is being reported by more and more people, so I'd
> > like to push the fix quickly.
> > 
> > David, please let me know if you don't want me to include the $subject patch
> > into the next PM fixes pull request.  If I don't hear from you, I'll push it
> > next week for -rc4.
> 
> I too have it in my tree as a patch which Artem sent me, as it's rather
> fundamental to not oopsing the kernel when you suspend/resume on any ARM
> platform.

It actually affects everyone using MTD and suspend (not only ARM) and that's
why I thought I'd take it, but I'm fine with any other resolution as long as
the patch goes to Linus before 3.3-final.

Thanks,
Rafael

Patch

From 283d43b9ce2952535aa89c0195085e2a1b7e5fce Mon Sep 17 00:00:00 2001
From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Date: Mon, 16 Jan 2012 11:07:16 +0200
Subject: [PATCH] mtd: fix MTD suspend

Commits 3fe4bae88460869a8e553397cd9057a4ee7ca341 and
079c985e7a6f4ce60f931cebfdd5ee3c3 broke MTD suspend in 2 ways:

1. When the '->suspend' method is not present, we return -EOPNOTSUPP, but
   the callers of 'mtd_suspend()' expects 0 instead.
2. Checking of the 'mtd' parameter against NULL has been incorrectly removed
   in 'mtd_cls_suspend()'.

This patch fixes the breakages. This has been found, analyzed, reported
and tested by Rafael J. Wysocki <rjw@sisk.pl>.

Note, this patch is not needed in the stable tree because it causes a
regression introduced during the v3.3 merge window.

Reported-by: Rafael J. Wysocki <rjw@sisk.pl>
Tested-by: Rafael J. Wysocki <rjw@sisk.pl>
Tested-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---

 drivers/mtd/mtdcore.c   |    2 +-
 include/linux/mtd/mtd.h |    4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c

index b265188..de96865 100644

--- a/drivers/mtd/mtdcore.c

+++ b/drivers/mtd/mtdcore.c

@@ -119,7 +119,7 @@  static int mtd_cls_suspend(struct device *dev, pm_message_t state)

 {
 	struct mtd_info *mtd = dev_get_drvdata(dev);
 
-	return mtd_suspend(mtd);

+	return mtd ? mtd_suspend(mtd) : 0;

 }
 
 static int mtd_cls_resume(struct device *dev)
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h

index 1a81fde..d8c7aad 100644

--- a/include/linux/mtd/mtd.h

+++ b/include/linux/mtd/mtd.h

@@ -427,9 +427,7 @@  static inline int mtd_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)

 
 static inline int mtd_suspend(struct mtd_info *mtd)
 {
-	if (!mtd->suspend)

-		return -EOPNOTSUPP;

-	return mtd->suspend(mtd);

+	return mtd->suspend ? mtd->suspend(mtd) : 0;

 }
 
 static inline void mtd_resume(struct mtd_info *mtd)
-- 

1.7.7.6