* [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).