[Lucid] UBUNTU: [regression] lucid alpha-2 and earlier freeze upon suspend with sd card plugged in with some hardware

Hi all,

Solves all 'suspend crashes while SD card is inserted' bugs. Of which there are >20, with >130 people affected.

BugLink: http://bugs.launchpad.net/bugs/477106

The following changes since commit 84ee32fb89f127545f52d23e53a3f6d0c59a7878:
  Keng-Yu Lin (1):
        UBUNTU: SAUCE: dell-laptop: fire SMI when toggling hardware killswitch (revised)

are available in the git repository at:

  git://kernel.ubuntu.com/lag/ubuntu-lucid.git lp477106

Lee Jones (1):
      UBUNTU: [regression] lucid alpha-2 and earlier freeze upon suspend with sd card plugged in with some hardware

 drivers/mmc/core/core.c  |   51 +++++++++++++++++++++++++++-------------------
 drivers/mmc/core/host.c  |    4 +++
 include/linux/mmc/host.h |    2 +
 3 files changed, 36 insertions(+), 21 deletions(-)

Hi Lee,

for the record just a few additional hints on the submission of SRU patches:
 * as a subject I'd use [Lucid] SRU: ...
 * above the output of git request-pull you should have the following (which
   should also be in the bug report. I usually modify the description and
   place it before anything else):

   SRU justification:

   Impact: What is the problem?

   Fix: Where does the fix come from? What does it roughly do?

   Testcase: How to trigger/verify the issue.

 * When you append the patch content (which is good), take the whole output
   of git format-patch. This also gives a quick look on the headers and should
   also give some insights on the patch history.

Like it looks now, I would wonder (again, because I forgot) where exactly the
patch originates from, whether it is targeted for upstream (with stable?) and
whether you have modified it.
The commit message does not sound like you picked the commit from another tree
(if we take upstream patches, they should remain unmodified, except the added
s-o-bs). If you did the patch, you should use a description suitable for
upstream submission (not the title of the bug report) and if it is intended to
go to upstream stable have a '(pre-stable)' prefix. If it was tried to submit
upstream but was rejected (maybe because upstream works differently now) you
should note this in the justifications fix section and prefix the patch with
'SAUCE:'.

So the patch header needs update. To the patch, I know I already looked at it
but I don't remember whether I made comments in email...
Most of it is sort of straight forward. Using a freezable workqueue avoids the
need to flush it. The notifier will do the eject of the card before userspace is
frozen. The one bit that is unclear is the removal of the code to eject in the
resume function, which looks like handling an error case on resume which feels
like still being valid.

-Stefan

On 06/29/2010 09:39 AM, Lee Jones wrote:
> Hi all,
> 
> Solves all 'suspend crashes while SD card is inserted' bugs. Of which there are >20, with >130 people affected.
> 
> BugLink: http://bugs.launchpad.net/bugs/477106
> 
> The following changes since commit 84ee32fb89f127545f52d23e53a3f6d0c59a7878:
>   Keng-Yu Lin (1):
>         UBUNTU: SAUCE: dell-laptop: fire SMI when toggling hardware killswitch (revised)
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/lag/ubuntu-lucid.git lp477106
> 
> Lee Jones (1):
>       UBUNTU: [regression] lucid alpha-2 and earlier freeze upon suspend with sd card plugged in with some hardware
> 
>  drivers/mmc/core/core.c  |   51 +++++++++++++++++++++++++++-------------------
>  drivers/mmc/core/host.c  |    4 +++
>  include/linux/mmc/host.h |    2 +
>  3 files changed, 36 insertions(+), 21 deletions(-)
> 
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 30acd52..2e8208c 100644 (file)
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1256,25 +1256,11 @@ int mmc_suspend_host(struct mmc_host *host, pm_message_t state)
>  
>         if (host->caps & MMC_CAP_DISABLE)
>                 cancel_delayed_work(&host->disable);
> -       cancel_delayed_work(&host->detect);
> -       mmc_flush_scheduled_work();
>  
>         mmc_bus_get(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) {
> -                       /*
> -                        * We simply "remove" the card in this case.
> -                        * It will be redetected on resume.
> -                        */
> -                       if (host->bus_ops->remove)
> -                               host->bus_ops->remove(host);
> -                       mmc_claim_host(host);
> -                       mmc_detach_bus(host);
> -                       mmc_release_host(host);
> -                       err = 0;
> -               }
>         }
>         mmc_bus_put(host);
>  
> @@ -1304,12 +1290,6 @@ int mmc_resume_host(struct mmc_host *host)
>                         printk(KERN_WARNING "%s: error %d during resume "
>                                             "(card was removed?)\n",
>                                             mmc_hostname(host), err);
> -                       if (host->bus_ops->remove)
> -                               host->bus_ops->remove(host);
> -                       mmc_claim_host(host);
> -                       mmc_detach_bus(host);
> -                       mmc_release_host(host);
> -                       /* no need to bother upper layers */
>                         err = 0;
>                 }
>         }
> @@ -1326,13 +1306,42 @@ int mmc_resume_host(struct mmc_host *host)
>  
>  EXPORT_SYMBOL(mmc_resume_host);
>  
> +/* Do the card removal on suspend if card is assumed removeable
> + * Do that in pm notifier while userspace isn't yet frozen, so we will be able to sync the card.
> + */
> +int mmc_pm_notify(struct notifier_block *notify_block,
> +                                 unsigned long mode, void *unused)
> +{
> +       struct mmc_host *host = container_of(
> +               notify_block, struct mmc_host, pm_notify);
> +       
> +       
> +       switch (mode) {
> +       case PM_HIBERNATION_PREPARE:
> +       case PM_SUSPEND_PREPARE:
> +               
> +               if (!host->bus_ops || host->bus_ops->suspend)
> +                       break;
> +               
> +               if (host->bus_ops->remove)
> +                       host->bus_ops->remove(host);
> +               mmc_claim_host(host);
> +               mmc_detach_bus(host);
> +               mmc_release_host(host);
> +               break;
> +               
> +       }
> +       
> +       return 0;
> +}
> +               
>  #endif
>  
>  static int __init mmc_init(void)
>  {
>         int ret;
>  
> -       workqueue = create_singlethread_workqueue("kmmcd");
> +       workqueue = create_freezeable_workqueue("kmmcd");
>         if (!workqueue)
>                 return -ENOMEM;
>  
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index a268d12..b88fc3d 100644 (file)
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -16,6 +16,7 @@
>  #include <linux/idr.h>
>  #include <linux/pagemap.h>
>  #include <linux/leds.h>
> +#include <linux/suspend.h>
>  
>  #include <linux/mmc/host.h>
>  
> @@ -84,6 +85,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
>         init_waitqueue_head(&host->wq);
>         INIT_DELAYED_WORK(&host->detect, mmc_rescan);
>         INIT_DELAYED_WORK_DEFERRABLE(&host->disable, mmc_host_deeper_disable);
> +       host->pm_notify.notifier_call = mmc_pm_notify;
>  
>         /*
>          * By default, hosts do not support SGIO or large requests.
> @@ -132,6 +134,7 @@ int mmc_add_host(struct mmc_host *host)
>  #endif
>  
>         mmc_start_host(host);
> +       register_pm_notifier(&host->pm_notify);
>  
>         return 0;
>  }
> @@ -148,6 +151,7 @@ EXPORT_SYMBOL(mmc_add_host);
>   */
>  void mmc_remove_host(struct mmc_host *host)
>  {
> +       unregister_pm_notifier(&host->pm_notify);
>         mmc_stop_host(host);
>  
>  #ifdef CONFIG_DEBUG_FS
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index eaf3636..473817f 100644 (file)
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -120,6 +120,7 @@ struct mmc_host {
>         unsigned int            f_min;
>         unsigned int            f_max;
>         u32                     ocr_avail;
> +       struct notifier_block   pm_notify;
>  
>  #define MMC_VDD_165_195                0x00000080      /* VDD voltage 1.65 - 1.95 */
>  #define MMC_VDD_20_21          0x00000100      /* VDD voltage 2.0 ~ 2.1 */
> @@ -249,6 +250,7 @@ int mmc_card_can_sleep(struct mmc_host *host);
>  int mmc_host_enable(struct mmc_host *host);
>  int mmc_host_disable(struct mmc_host *host);
>  int mmc_host_lazy_disable(struct mmc_host *host);
> +int mmc_pm_notify(struct notifier_block *notify_block, unsigned long, void *);
>  
>  static inline void mmc_set_disable_delay(struct mmc_host *host,
>                                          unsigned int disable_delay)
>