* [PATCH] PM / MTD: Fix regressions related to mtd_suspend()
@ 2012-01-15 13:43 Rafael J. Wysocki
2012-01-15 14:21 ` Artem Bityutskiy
0 siblings, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2012-01-15 13:43 UTC (permalink / raw)
To: Linux PM list, David Woodhouse; +Cc: LKML, Artem Bityutskiy, linux-mtd
From: Rafael J. Wysocki <rjw@sisk.pl>
Commit 3fe4bae88460869a8e553397cd9057a4ee7ca341 (mtd: introduce
mtd_suspend interface) introduced the mtd_suspend() routine
which didn't check whether or not mtd and mtd->suspend were
both valid pointers. Later commit 079c985e7a6f4ce60f931cebfdd5ee3c3
(mtd: do not use mtd->suspend and mtd->resume directly) added the
check for mtd->suspend, but still it failed to check mtd. Moreover,
it caused mtd_suspend() to return an error code for NULL
mtd->suspend, which makes system suspend fail on one of my test
systems on every attempt and which is a regression from v3.2.
Fix mtd_suspend() by making it check if mtd is not NULL and
return 0 if mtd or mtd->suspend is NULL, which shouldn't prevent
the system from suspending.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
include/linux/mtd/mtd.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
Index: linux/include/linux/mtd/mtd.h
===================================================================
--- linux.orig/include/linux/mtd/mtd.h
+++ linux/include/linux/mtd/mtd.h
@@ -427,9 +427,7 @@ static inline int mtd_is_locked(struct m
static inline int mtd_suspend(struct mtd_info *mtd)
{
- if (!mtd->suspend)
- return -EOPNOTSUPP;
- return mtd->suspend(mtd);
+ return mtd && mtd->suspend ? mtd->suspend(mtd) : 0;
}
static inline void mtd_resume(struct mtd_info *mtd)
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] PM / MTD: Fix regressions related to mtd_suspend() 2012-01-15 13:43 [PATCH] PM / MTD: Fix regressions related to mtd_suspend() Rafael J. Wysocki @ 2012-01-15 14:21 ` Artem Bityutskiy 2012-01-15 23:18 ` Rafael J. Wysocki 0 siblings, 1 reply; 8+ messages in thread From: Artem Bityutskiy @ 2012-01-15 14:21 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, David Woodhouse, LKML, Artem Bityutskiy, linux-mtd [-- Attachment #1: Type: text/plain, Size: 2624 bytes --] On Sun, 2012-01-15 at 14:43 +0100, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rjw@sisk.pl> > > Commit 3fe4bae88460869a8e553397cd9057a4ee7ca341 (mtd: introduce > mtd_suspend interface) introduced the mtd_suspend() routine > which didn't check whether or not mtd and mtd->suspend were > both valid pointers. Later commit 079c985e7a6f4ce60f931cebfdd5ee3c3 > (mtd: do not use mtd->suspend and mtd->resume directly) added the > check for mtd->suspend, but still it failed to check mtd. Moreover, > it caused mtd_suspend() to return an error code for NULL > mtd->suspend, which makes system suspend fail on one of my test > systems on every attempt and which is a regression from v3.2. > > Fix mtd_suspend() by making it check if mtd is not NULL and > return 0 if mtd or mtd->suspend is NULL, which shouldn't prevent > the system from suspending. > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > --- > include/linux/mtd/mtd.h | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > Index: linux/include/linux/mtd/mtd.h > =================================================================== > --- linux.orig/include/linux/mtd/mtd.h > +++ linux/include/linux/mtd/mtd.h > @@ -427,9 +427,7 @@ static inline int mtd_is_locked(struct m > > static inline int mtd_suspend(struct mtd_info *mtd) > { > - if (!mtd->suspend) > - return -EOPNOTSUPP; > - return mtd->suspend(mtd); > + return mtd && mtd->suspend ? mtd->suspend(mtd) : 0; > } Thanks! But All the MTD API functions assume that the "mtd" pointer is non-NULL, and this function should to the same for consistency. Would you please give this patch a test? 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; } -- Best Regards, Artem Bityutskiy [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] PM / MTD: Fix regressions related to mtd_suspend() 2012-01-15 14:21 ` Artem Bityutskiy @ 2012-01-15 23:18 ` Rafael J. Wysocki 2012-01-16 9:17 ` Artem Bityutskiy 0 siblings, 1 reply; 8+ messages in thread From: Rafael J. Wysocki @ 2012-01-15 23:18 UTC (permalink / raw) To: dedekind1 Cc: Linux PM list, David Woodhouse, LKML, Artem Bityutskiy, linux-mtd On Sunday, January 15, 2012, Artem Bityutskiy wrote: > On Sun, 2012-01-15 at 14:43 +0100, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rjw@sisk.pl> > > > > Commit 3fe4bae88460869a8e553397cd9057a4ee7ca341 (mtd: introduce > > mtd_suspend interface) introduced the mtd_suspend() routine > > which didn't check whether or not mtd and mtd->suspend were > > both valid pointers. Later commit 079c985e7a6f4ce60f931cebfdd5ee3c3 > > (mtd: do not use mtd->suspend and mtd->resume directly) added the > > check for mtd->suspend, but still it failed to check mtd. Moreover, > > it caused mtd_suspend() to return an error code for NULL > > mtd->suspend, which makes system suspend fail on one of my test > > systems on every attempt and which is a regression from v3.2. > > > > Fix mtd_suspend() by making it check if mtd is not NULL and > > return 0 if mtd or mtd->suspend is NULL, which shouldn't prevent > > the system from suspending. > > > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > > --- > > include/linux/mtd/mtd.h | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > Index: linux/include/linux/mtd/mtd.h > > =================================================================== > > --- linux.orig/include/linux/mtd/mtd.h > > +++ linux/include/linux/mtd/mtd.h > > @@ -427,9 +427,7 @@ static inline int mtd_is_locked(struct m > > > > static inline int mtd_suspend(struct mtd_info *mtd) > > { > > - if (!mtd->suspend) > > - return -EOPNOTSUPP; > > - return mtd->suspend(mtd); > > + return mtd && mtd->suspend ? mtd->suspend(mtd) : 0; > > } > > Thanks! But All the MTD API functions assume that the "mtd" pointer is > non-NULL, and this function should to the same for consistency. > > Would you please give this patch a test? Yes, this works too. Thanks, Rafael > 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; > } > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PM / MTD: Fix regressions related to mtd_suspend() 2012-01-15 23:18 ` Rafael J. Wysocki @ 2012-01-16 9:17 ` Artem Bityutskiy 2012-01-16 9:23 ` Oliver Neukum ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Artem Bityutskiy @ 2012-01-16 9:17 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Linux PM list, David Woodhouse, LKML, linux-mtd [-- Attachment #1: Type: text/plain, Size: 2101 bytes --] Thanks Rafael! I've pushed this patch to l2-mtd-2.6.git (David, would you please merge it upstream?). Rafael, should I add some tags with your name like Tested-by or Acked-by? From d0eb0d769923d562781fcf43fdc6fa28e3823756 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>. 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.8.3 -- Best Regards, Artem Bityutskiy [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] PM / MTD: Fix regressions related to mtd_suspend() 2012-01-16 9:17 ` Artem Bityutskiy @ 2012-01-16 9:23 ` Oliver Neukum 2012-01-16 9:26 ` Artem Bityutskiy 2012-01-16 20:59 ` Rafael J. Wysocki 2012-01-27 22:51 ` Rafael J. Wysocki 2 siblings, 1 reply; 8+ messages in thread From: Oliver Neukum @ 2012-01-16 9:23 UTC (permalink / raw) To: dedekind1 Cc: Rafael J. Wysocki, Linux PM list, David Woodhouse, LKML, linux-mtd Am Montag, 16. Januar 2012, 10:17:21 schrieb Artem Bityutskiy: > Thanks Rafael! > > I've pushed this patch to l2-mtd-2.6.git (David, would you please merge > it upstream?). Does this need to go into stable? Regards Oliver ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PM / MTD: Fix regressions related to mtd_suspend() 2012-01-16 9:23 ` Oliver Neukum @ 2012-01-16 9:26 ` Artem Bityutskiy 0 siblings, 0 replies; 8+ messages in thread From: Artem Bityutskiy @ 2012-01-16 9:26 UTC (permalink / raw) To: Oliver Neukum Cc: Rafael J. Wysocki, Linux PM list, David Woodhouse, LKML, linux-mtd [-- Attachment #1: Type: text/plain, Size: 449 bytes --] On Mon, 2012-01-16 at 10:23 +0100, Oliver Neukum wrote: > Am Montag, 16. Januar 2012, 10:17:21 schrieb Artem Bityutskiy: > > Thanks Rafael! > > > > I've pushed this patch to l2-mtd-2.6.git (David, would you please merge > > it upstream?). > > Does this need to go into stable? No, this is a regression introduced during this merge window. I'll amend the commit message to make it clear, thanks. -- Best Regards, Artem Bityutskiy [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PM / MTD: Fix regressions related to mtd_suspend() 2012-01-16 9:17 ` Artem Bityutskiy 2012-01-16 9:23 ` Oliver Neukum @ 2012-01-16 20:59 ` Rafael J. Wysocki 2012-01-27 22:51 ` Rafael J. Wysocki 2 siblings, 0 replies; 8+ messages in thread From: Rafael J. Wysocki @ 2012-01-16 20:59 UTC (permalink / raw) To: dedekind1; +Cc: Linux PM list, David Woodhouse, LKML, linux-mtd On Monday, January 16, 2012, Artem Bityutskiy wrote: > Thanks Rafael! > > I've pushed this patch to l2-mtd-2.6.git (David, would you please merge > it upstream?). > > Rafael, should I add some tags with your name like Tested-by or > Acked-by? Traditionally, the "Reported-and-tested-by" tag is used for such things, so you can do that too ... Thanks, Rafael > From d0eb0d769923d562781fcf43fdc6fa28e3823756 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>. > > 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) > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PM / MTD: Fix regressions related to mtd_suspend() 2012-01-16 9:17 ` Artem Bityutskiy 2012-01-16 9:23 ` Oliver Neukum 2012-01-16 20:59 ` Rafael J. Wysocki @ 2012-01-27 22:51 ` Rafael J. Wysocki 2 siblings, 0 replies; 8+ messages in thread From: Rafael J. Wysocki @ 2012-01-27 22:51 UTC (permalink / raw) To: dedekind1; +Cc: Linux PM list, David Woodhouse, LKML, linux-mtd, Maciej Rutecki On Monday, January 16, 2012, Artem Bityutskiy wrote: > Thanks Rafael! > > I've pushed this patch to l2-mtd-2.6.git (David, would you please merge > it upstream?). > > Rafael, should I add some tags with your name like Tested-by or > Acked-by? > > > From d0eb0d769923d562781fcf43fdc6fa28e3823756 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>. Reported-and-tested-by: Rafael J. Wysocki <rjw@sisk.pl> Is there any plan to push this patch to Linus for 3.3? It fixes a rather nasty suspend regression. Thanks, Rafael > 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) > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-01-27 22:47 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-01-15 13:43 [PATCH] PM / MTD: Fix regressions related to mtd_suspend() Rafael J. Wysocki 2012-01-15 14:21 ` Artem Bityutskiy 2012-01-15 23:18 ` Rafael J. Wysocki 2012-01-16 9:17 ` Artem Bityutskiy 2012-01-16 9:23 ` Oliver Neukum 2012-01-16 9:26 ` Artem Bityutskiy 2012-01-16 20:59 ` Rafael J. Wysocki 2012-01-27 22:51 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).