All of lore.kernel.org
 help / color / mirror / Atom feed
From: Artem Bityutskiy <dedekind1@gmail.com>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Artem Bityutskiy <artem.bityutskiy@linux.intel.com>,
	David Woodhouse <David.Woodhouse@intel.com>,
	linux-mtd@lists.infradead.org,
	Linux PM list <linux-pm@vger.kernel.org>
Subject: Re: [PATCH] PM / MTD: Fix regressions related to mtd_suspend()
Date: Sun, 15 Jan 2012 16:21:45 +0200	[thread overview]
Message-ID: <1326637306.2201.22.camel@koala> (raw)
In-Reply-To: <201201151443.30897.rjw@sisk.pl>

[-- 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 --]

WARNING: multiple messages have this Message-ID (diff)
From: Artem Bityutskiy <dedekind1@gmail.com>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Linux PM list <linux-pm@vger.kernel.org>,
	David Woodhouse <David.Woodhouse@intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Artem Bityutskiy <artem.bityutskiy@linux.intel.com>,
	linux-mtd@lists.infradead.org
Subject: Re: [PATCH] PM / MTD: Fix regressions related to mtd_suspend()
Date: Sun, 15 Jan 2012 16:21:45 +0200	[thread overview]
Message-ID: <1326637306.2201.22.camel@koala> (raw)
In-Reply-To: <201201151443.30897.rjw@sisk.pl>

[-- 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 --]

  reply	other threads:[~2012-01-15 14:21 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-15 13:43 [PATCH] PM / MTD: Fix regressions related to mtd_suspend() Rafael J. Wysocki
2012-01-15 13:43 ` Rafael J. Wysocki
2012-01-15 14:21 ` Artem Bityutskiy [this message]
2012-01-15 14:21   ` Artem Bityutskiy
2012-01-15 23:18   ` Rafael J. Wysocki
2012-01-15 23:18     ` Rafael J. Wysocki
2012-01-16  9:17     ` Artem Bityutskiy
2012-01-16  9:17       ` Artem Bityutskiy
2012-01-16  9:23       ` Oliver Neukum
2012-01-16  9:23         ` Oliver Neukum
2012-01-16  9:26         ` Artem Bityutskiy
2012-01-16  9:26           ` Artem Bityutskiy
2012-01-16 20:59       ` Rafael J. Wysocki
2012-01-16 20:59         ` Rafael J. Wysocki
2012-01-27 22:51       ` Rafael J. Wysocki
2012-01-27 22:51         ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1326637306.2201.22.camel@koala \
    --to=dedekind1@gmail.com \
    --cc=David.Woodhouse@intel.com \
    --cc=artem.bityutskiy@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rjw@sisk.pl \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.