linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: maximlevitsky@gmail.com (Maxim Levitsky)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] sdio: fix suspend/resume regression
Date: Sat, 23 Oct 2010 16:18:50 +0200	[thread overview]
Message-ID: <1287843531.29752.10.camel@maxim-laptop> (raw)
In-Reply-To: <AANLkTi=iC1uKyToEXrroOj+JxQ2mW97WRVEXjv+9oQEa@mail.gmail.com>

On Sat, 2010-10-23 at 12:09 +0200, Ohad Ben-Cohen wrote:
> Hi Maxim,
> 
> On Fri, Oct 22, 2010 at 1:47 AM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> > On Wed, 2010-10-13 at 09:31 +0200, Ohad Ben-Cohen wrote:
> >> Fix SDIO suspend/resume regression introduced by
> >> 4c2ef25fe0b847d2ae818f74758ddb0be1c27d8e "mmc: fix all hangs related to
> >> mmc/sd card insert/removal during suspend/resume":
> ...
> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> >> index c94565d..515ff39 100644
> >> --- a/drivers/mmc/core/core.c
> >> +++ b/drivers/mmc/core/core.c
> >> @@ -1682,6 +1682,19 @@ int mmc_suspend_host(struct mmc_host *host)
> >>       if (host->bus_ops && !host->bus_dead) {
> >>               if (host->bus_ops->suspend)
> >>                       err = host->bus_ops->suspend(host);
> >> +             if (err == -ENOSYS || !host->bus_ops->resume) {
> > This reintroduces the bug I fixed.
> >
> > if the CONFIG_MMC_UNSAFE_RESUME isn't set (and that is default
> > unfortunately), the host->bus_ops->resume will be NULL (see core/mmc.c
> > mmc_ops), and therefore card will be removed, that will trigger a block
> > device removal, sync, and deadlock).
> 
> Have you reproduced this or is it just a general concern ?
Only a general concern.
> 
> I'm asking this because if CONFIG_MMC_UNSAFE_RESUME isn't set, then
> mmc_pm_notify() will remove the card and detach the bus.
> 
> As a result, host->bus_ops will be unset, and host->bus_dead will be set.
> 
> In this case, mmc_suspend_host() should skip the code you are concerned about.
You are right here.
There is a very unlikely case that suspend of sd/mmc card fails (with
CONFIG_MMC_UNSAFE_RESUME set, and that will trigger the hang, but that
currently isn't possible because both mmc_sd_suspend and mmc_suspend
just return 0 unconditionally).

Btw, two above functions are identical, so some refactoring won't hurt.

So sorry for noise (and bug).

Best regards,
	Maxim Levitsky

  reply	other threads:[~2010-10-23 14:18 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-04  7:30 2.6.35.6 fails to suspend (pxa2xx-mci.0) Sven Neumann
2010-10-04  7:48 ` Eric Miao
2010-10-06 18:59 ` Maciej Rutecki
2010-10-06 23:55 ` Daniel Mack
2010-10-07  0:18   ` Rafael J. Wysocki
2010-10-07 15:03     ` Sven Neumann
2010-10-07 21:23       ` Rafael J. Wysocki
2010-10-08  8:23         ` Sven Neumann
2010-10-08 20:08           ` Rafael J. Wysocki
2010-10-09  1:07             ` Ohad Ben-Cohen
2010-10-09 23:20               ` Sven Neumann
2010-10-11  8:31               ` Sven Neumann
2010-10-11  8:45                 ` Ohad Ben-Cohen
2010-10-11  9:11                   ` Sven Neumann
2010-10-13  7:31                     ` [PATCH] sdio: fix suspend/resume regression Ohad Ben-Cohen
2010-10-13  7:54                       ` Vitaly Wool
2010-10-13  8:55                         ` Ohad Ben-Cohen
2010-10-13  9:06                           ` Vitaly Wool
2010-10-13  9:46                             ` Ohad Ben-Cohen
2010-10-13 20:00                         ` Nicolas Pitre
2010-10-13 20:08                       ` Nicolas Pitre
2010-10-14 15:28                         ` Ohad Ben-Cohen
2010-10-14  2:24                       ` Chris Ball
2010-10-14  4:49                         ` Ohad Ben-Cohen
2010-10-21 23:47                       ` Maxim Levitsky
2010-10-21 23:55                         ` Nicolas Pitre
2010-10-22  0:25                           ` Maxim Levitsky
2010-10-21 23:57                         ` Nicolas Pitre
2010-10-23 10:09                         ` Ohad Ben-Cohen
2010-10-23 14:18                           ` Maxim Levitsky [this message]
2010-10-23 14:44                             ` Ohad Ben-Cohen
2010-10-11  8:10             ` 2.6.35.6 fails to suspend (pxa2xx-mci.0) Sven Neumann

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=1287843531.29752.10.camel@maxim-laptop \
    --to=maximlevitsky@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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 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).