[Lucid] pull-request: mmc: fix all hangs related to mmc/sd card insert/removal during suspend/resume

Stefan Bader stefan.bader at canonical.com
Fri Sep 3 15:17:38 UTC 2010


NAK, you add a whole range of things that are not part of the upstream patch and
are not required to make it apply. AFAIKS the only issue is the usage of
pm_flags in the patch and it is set to 0 in both cases. As pm_flags does not
exist in 2.6.32 I don't see a reason to just drop it.

-Stefan

On 09/03/2010 04:48 PM, Lee Jones wrote:
> The following changes since commit 5470af5fd104192214b496a2736fd26200f4650d:
>   Steve Conklin (1):
>         UBUNTU: [Config] updateconfigs
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/lag/ubuntu-lucid.git lp477106
> 
> Lee Jones (1):
>       mmc: fix all hangs related to mmc/sd card insert/removal during suspend/resume
> 
>  drivers/mmc/core/core.c     |  107 +++++++++++++++++++++++++++++-------------
>  drivers/mmc/core/host.c     |    5 ++
>  drivers/mmc/core/sdio_ops.c |   64 +++++++++++++++++++++++++
>  drivers/mmc/core/sdio_ops.h |    2 +-
>  include/linux/mmc/host.h    |    6 ++
>  include/linux/mmc/pm.h      |   30 ++++++++++++
>  6 files changed, 180 insertions(+), 34 deletions(-)
>  create mode 100644 include/linux/mmc/pm.h
> 
> ------------------------------------------------------------------------
> 
> commit f81f5e32533573dbe601e31f4f1a07e3497a89e9
> Author: Lee Jones <lee.jones at canonical.com>
> Date:   Fri Sep 3 15:43:24 2010 +0100
> 
>     mmc: fix all hangs related to mmc/sd card insert/removal during suspend/resume
>     
>     Based on a patch-set originally written by Maxim Levitsky.
>     
>     If you don't use CONFIG_MMC_UNSAFE_RESUME, as soon as you attempt to
>     suspend, the card will be removed, therefore this patch doesn't change the
>     behavior of this option.
>     
>     However the removal will be done by pm notifier, which runs while
>     userspace is still not frozen and thus can freely use del_gendisk, without
>     the risk of deadlock which would happen otherwise.
>     
>     Card detect workqueue is now disabled while userspace is frozen, Therefore
>     if you do use CONFIG_MMC_UNSAFE_RESUME, and remove the card during
>     suspend, the removal will be detected as soon as userspace is unfrozen,
>     again at the moment it is safe to call del_gendisk.
>     
>     Tested with and without CONFIG_MMC_UNSAFE_RESUME with suspend and hibernate.
>     
>     [akpm at linux-foundation.org: clean up function prototype]
>     [akpm at linux-foundation.org: fix CONFIG_PM-n linkage, small cleanups]
>     [akpm at linux-foundation.org: coding-style fixes]
>     Signed-off-by: Maxim Levitsky <maximlevitsky at gmail.com>
>     Cc: David Brownell <david-b at pacbell.net>
>     Cc: Alan Stern <stern at rowland.harvard.edu>
>     Cc: <linux-mmc at vger.kernel.org>
>     Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
>     Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
>     
>     [Backported to Ubuntu-2.6.32-25.43 by Lee Jones]
>     Signed-off-by: Lee Jones <lee.jones at canonical.com>
>     
>     BugLink: http://bugs.launchpad.net/bugs/477106
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 30acd52..eb2f935 100644 (file)
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -106,8 +106,9 @@ void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq)
>                 cmd->error = 0;
>                 host->ops->request(host, mrq);
>         } else {
> +#ifdef CONFIG_LEDS_TRIGGERS
>                 led_trigger_event(host->led, LED_OFF);
> -
> +#endif
>                 pr_debug("%s: req done (CMD%u): %d: %08x %08x %08x %08x\n",
>                         mmc_hostname(host), cmd->opcode, err,
>                         cmd->resp[0], cmd->resp[1],
> @@ -163,7 +164,9 @@ mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
>  
>         WARN_ON(!host->claimed);
>  
> +#ifdef CONFIG_LEDS_TRIGGERS
>         led_trigger_event(host->led, LED_FULL);
> +#endif
>  
>         mrq->cmd->error = 0;
>         mrq->cmd->mrq = mrq;
> @@ -1057,6 +1060,17 @@ void mmc_rescan(struct work_struct *work)
>                 container_of(work, struct mmc_host, detect.work);
>         u32 ocr;
>         int err;
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&host->lock, flags);
> +
> +       if (host->rescan_disable) {
> +               spin_unlock_irqrestore(&host->lock, flags);
> +               return;
> +       }
> +
> +       spin_unlock_irqrestore(&host->lock, flags);
> +
>  
>         mmc_bus_get(host);
>  
> @@ -1087,8 +1101,9 @@ void mmc_rescan(struct work_struct *work)
>                 goto out;
>  
>         mmc_claim_host(host);
> -
>         mmc_power_up(host);
> +       sdio_reset(host);
> +
>         mmc_go_idle(host);
>  
>         mmc_send_if_cond(host, host->ocr_avail);
> @@ -1151,6 +1166,9 @@ void mmc_stop_host(struct mmc_host *host)
>         cancel_delayed_work(&host->detect);
>         mmc_flush_scheduled_work();
>  
> +       /* clear pm flags now and let card drivers set them as needed */
> +       host->pm_flags = 0;
> +
>         mmc_bus_get(host);
>         if (host->bus_ops && !host->bus_dead) {
>                 if (host->bus_ops->remove)
> @@ -1263,22 +1281,10 @@ int mmc_suspend_host(struct mmc_host *host, pm_message_t state)
>         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);
>  
> -       if (!err)
> +       if (!err && !(host->pm_flags & MMC_PM_KEEP_POWER))
>                 mmc_power_off(host);
>  
>         return err;
> @@ -1296,36 +1302,71 @@ int mmc_resume_host(struct mmc_host *host)
>  
>         mmc_bus_get(host);
>         if (host->bus_ops && !host->bus_dead) {
> -               mmc_power_up(host);
> -               mmc_select_voltage(host, host->ocr);
> +               if (!(host->pm_flags & MMC_PM_KEEP_POWER)) {
> +                       mmc_power_up(host);
> +                       mmc_select_voltage(host, host->ocr);
> +               }
>                 BUG_ON(!host->bus_ops->resume);
>                 err = host->bus_ops->resume(host);
>                 if (err) {
>                         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 */
> +                                  "(card was removed?)\n",
> +                                  mmc_hostname(host), err);
>                         err = 0;
>                 }
>         }
>         mmc_bus_put(host);
> -
> -       /*
> -        * We add a slight delay here so that resume can progress
> -        * in parallel.
> -        */
> -       mmc_detect_change(host, 1);
> -
> +       
>         return err;
>  }
> -
>  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);
> +       unsigned long flags;
> +
> +
> +       switch (mode) {
> +       case PM_HIBERNATION_PREPARE:
> +       case PM_SUSPEND_PREPARE:
> +
> +               spin_lock_irqsave(&host->lock, flags);
> +               host->rescan_disable = 1;
> +               spin_unlock_irqrestore(&host->lock, flags);
> +               cancel_delayed_work_sync(&host->detect);
> +
> +               if (!host->bus_ops || host->bus_ops->suspend)
> +                       break;
> +
> +               mmc_claim_host(host);
> +
> +               if (host->bus_ops->remove)
> +                       host->bus_ops->remove(host);
> +
> +               mmc_detach_bus(host);
> +               mmc_release_host(host);
> +               host->pm_flags = 0;
> +               break;
> +
> +       case PM_POST_SUSPEND:
> +       case PM_POST_HIBERNATION:
> +
> +               spin_lock_irqsave(&host->lock, flags);
> +               host->rescan_disable = 0;
> +               spin_unlock_irqrestore(&host->lock, flags);
> +               mmc_detect_change(host, 0);
> +
> +       }
> +
> +       return 0;
> +}
>  #endif
>  
>  static int __init mmc_init(void)
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index a268d12..0efe631 100644 (file)
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -16,6 +16,8 @@
>  #include <linux/idr.h>
>  #include <linux/pagemap.h>
>  #include <linux/leds.h>
> +#include <linux/slab.h>
> +#include <linux/suspend.h>
>  
>  #include <linux/mmc/host.h>
>  
> @@ -84,6 +86,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 +135,7 @@ int mmc_add_host(struct mmc_host *host)
>  #endif
>  
>         mmc_start_host(host);
> +       register_pm_notifier(&host->pm_notify);
>  
>         return 0;
>  }
> @@ -148,6 +152,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/drivers/mmc/core/sdio_ops.c b/drivers/mmc/core/sdio_ops.c
> index 4eb7825..77f1252 100644 (file)
> --- a/drivers/mmc/core/sdio_ops.c
> +++ b/drivers/mmc/core/sdio_ops.c
> @@ -67,6 +67,54 @@ int mmc_send_io_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
>         return err;
>  }
>  
> +static int mmc_io_rw_direct_host(struct mmc_host *host, int write, unsigned fn,
> +       unsigned addr, u8 in, u8 *out)
> +{
> +       struct mmc_command cmd;
> +       int err;
> +
> +       BUG_ON(!host);
> +       BUG_ON(fn > 7);
> +
> +       /* sanity check */
> +       if (addr & ~0x1FFFF)
> +               return -EINVAL;
> +
> +       memset(&cmd, 0, sizeof(struct mmc_command));
> +
> +       cmd.opcode = SD_IO_RW_DIRECT;
> +       cmd.arg = write ? 0x80000000 : 0x00000000;
> +       cmd.arg |= fn << 28;
> +       cmd.arg |= (write && out) ? 0x08000000 : 0x00000000;
> +       cmd.arg |= addr << 9;
> +       cmd.arg |= in;
> +       cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_AC;
> +
> +       err = mmc_wait_for_cmd(host, &cmd, 0);
> +       if (err)
> +               return err;
> +
> +       if (mmc_host_is_spi(host)) {
> +               /* host driver already reported errors */
> +       } else {
> +               if (cmd.resp[0] & R5_ERROR)
> +                       return -EIO;
> +               if (cmd.resp[0] & R5_FUNCTION_NUMBER)
> +                       return -EINVAL;
> +               if (cmd.resp[0] & R5_OUT_OF_RANGE)
> +                       return -ERANGE;
> +       }
> +
> +       if (out) {
> +               if (mmc_host_is_spi(host))
> +                       *out = (cmd.resp[0] >> 8) & 0xFF;
> +               else
> +                       *out = cmd.resp[0] & 0xFF;
> +       }
> +
> +       return 0;
> +}
> +
>  int mmc_io_rw_direct(struct mmc_card *card, int write, unsigned fn,
>         unsigned addr, u8 in, u8* out)
>  {
> @@ -182,3 +230,19 @@ int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
>         return 0;
>  }
>  
> +int sdio_reset(struct mmc_host *host)
> +{
> +       int ret;
> +       u8 abort;
> +
> +       /* SDIO Simplified Specification V2.0, 4.4 Reset for SDIO */
> +
> +       ret = mmc_io_rw_direct_host(host, 0, 0, SDIO_CCCR_ABORT, 0, &abort);
> +       if (ret)
> +               abort = 0x08;
> +       else
> +               abort |= 0x08;
> +
> +       ret = mmc_io_rw_direct_host(host, 1, 0, SDIO_CCCR_ABORT, abort, NULL);
> +       return ret;
> +}
> diff --git a/drivers/mmc/core/sdio_ops.h b/drivers/mmc/core/sdio_ops.h
> index e2e74b0..7e6c836 100644 (file)
> --- a/drivers/mmc/core/sdio_ops.h
> +++ b/drivers/mmc/core/sdio_ops.h
> @@ -17,6 +17,6 @@ int mmc_io_rw_direct(struct mmc_card *card, int write, unsigned fn,
>         unsigned addr, u8 in, u8* out);
>  int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
>         unsigned addr, int incr_addr, u8 *buf, unsigned blocks, unsigned blksz);
> -
> +int sdio_reset(struct mmc_host *host);
>  #endif
>  
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index eaf3636..c43bc21 100644 (file)
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -14,6 +14,7 @@
>  #include <linux/sched.h>
>  
>  #include <linux/mmc/core.h>
> +#include <linux/mmc/pm.h>
>  
>  struct mmc_ios {
>         unsigned int    clock;                  /* clock rate */
> @@ -120,6 +121,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 */
> @@ -177,6 +179,7 @@ struct mmc_host {
>  
>         /* Only used with MMC_CAP_DISABLE */
>         int                     enabled;        /* host is enabled */
> +       int                     rescan_disable; /* disable card detection */
>         int                     nesting_cnt;    /* "enable" nesting count */
>         int                     en_dis_recurs;  /* detect recursion */
>         unsigned int            disable_delay;  /* disable delay in msecs */
> @@ -197,6 +200,8 @@ struct mmc_host {
>         struct task_struct      *sdio_irq_thread;
>         atomic_t                sdio_irq_thread_abort;
>  
> +       mmc_pm_flag_t           pm_flags;       /* requested pm features */
> +
>  #ifdef CONFIG_LEDS_TRIGGERS
>         struct led_trigger      *led;           /* activity led */
>  #endif
> @@ -249,6 +254,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)
> diff --git a/include/linux/mmc/pm.h b/include/linux/mmc/pm.h
> new file mode 100644 (file)
> index 0000000..d37aac4
> --- /dev/null
> +++ b/include/linux/mmc/pm.h
> @@ -0,0 +1,30 @@
> +/*
> + * linux/include/linux/mmc/pm.h
> + *
> + * Author:     Nicolas Pitre
> + * Copyright:  (C) 2009 Marvell Technology Group Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef LINUX_MMC_PM_H
> +#define LINUX_MMC_PM_H
> +
> +/*
> + * These flags are used to describe power management features that
> + * some cards (typically SDIO cards) might wish to benefit from when
> + * the host system is being suspended.  There are several layers of
> + * abstractions involved, from the host controller driver, to the MMC core
> + * code, to the SDIO core code, to finally get to the actual SDIO function
> + * driver.  This file is therefore used for common definitions shared across
> + * all those layers.
> + */
> +
> +typedef unsigned int mmc_pm_flag_t;
> +
> +#define MMC_PM_KEEP_POWER      (1 << 0)        /* preserve card power during suspend */
> +#define MMC_PM_WAKE_SDIO_IRQ   (1 << 1)        /* wake up host system on SDIO IRQ assertion */
> +
> +#endif
> 





More information about the kernel-team mailing list