Hi Aaron,

This patch is tested on Intel platform, and SDIO function driver's 
suspend/resume callback will only be called once, which fixed this issue. 
Previously, they can be called twice.

Here is the tested-by:

Tested-by: xiaoming wang <xiaoming.w...@intel.com>
Tested-by: Chuanxiao Dong <chuanxiao.d...@intel.com>

Thanks
Chuanxiao

> -----Original Message-----
> From: Lu, Aaron
> Sent: Wednesday, March 12, 2014 10:36 AM
> To: Ulf Hansson; linux-...@vger.kernel.org; Chris Ball; Liu, Chuansheng; Dong,
> Chuanxiao
> Cc: linux-kernel@vger.kernel.org; NeilBrown; Rafael J. Wysocki
> Subject: Re: [RFC PATCH] mmc: core: Invoke sdio func driver's PM callbacks 
> from the
> sdio bus
> 
> Hi Chuansheng & Chuanxiao,
> 
> Can you please help us testing this patch on your platform and let us know 
> the test
> result? Thanks.
> 
> -Aaron
> 
> On 02/28/2014 07:49 PM, Ulf Hansson wrote:
> > The sdio func device is added to the driver model after the card
> > device.
> >
> > This means the sdio func device will be suspend before the card device
> > and thus resumed after. The consequence are the mmc core don't
> > explicity need to protect itself from receiving sdio requests in
> > suspended state. Instead that can be handled from the sdio bus, which
> > is thus invokes the PM callbacks instead of old dummy function.
> >
> > In the case were the sdio func driver don't implement the PM callbacks
> > the mmc core will in the early phase of system suspend, remove the
> > card from the driver model and thus power off it.
> >
> > Cc: Aaron Lu <aaron...@intel.com>
> > Cc: NeilBrown <ne...@suse.de>
> > Cc: Rafael J. Wysocki <r...@rjwysocki.net>
> > Signed-off-by: Ulf Hansson <ulf.hans...@linaro.org>
> > ---
> >
> > Note, this patch has only been compile tested. Would appreciate if
> > some with SDIO and a sdio func driver could help out to test this.
> > Especially the libertas driver would be nice.
> >
> > ---
> >  drivers/mmc/core/sdio.c     |   45 
> > ++++---------------------------------------
> >  drivers/mmc/core/sdio_bus.c |   14 +-------------
> >  2 files changed, 5 insertions(+), 54 deletions(-)
> >
> > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index
> > 4d721c6..9933e42 100644
> > --- a/drivers/mmc/core/sdio.c
> > +++ b/drivers/mmc/core/sdio.c
> > @@ -943,40 +943,21 @@ static int mmc_sdio_pre_suspend(struct mmc_host
> *host)
> >   */
> >  static int mmc_sdio_suspend(struct mmc_host *host)  {
> > -   int i, err = 0;
> > -
> > -   for (i = 0; i < host->card->sdio_funcs; i++) {
> > -           struct sdio_func *func = host->card->sdio_func[i];
> > -           if (func && sdio_func_present(func) && func->dev.driver) {
> > -                   const struct dev_pm_ops *pmops = func->dev.driver->pm;
> > -                   err = pmops->suspend(&func->dev);
> > -                   if (err)
> > -                           break;
> > -           }
> > -   }
> > -   while (err && --i >= 0) {
> > -           struct sdio_func *func = host->card->sdio_func[i];
> > -           if (func && sdio_func_present(func) && func->dev.driver) {
> > -                   const struct dev_pm_ops *pmops = func->dev.driver->pm;
> > -                   pmops->resume(&func->dev);
> > -           }
> > -   }
> > -
> > -   if (!err && mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host))
> {
> > +   if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) {
> >             mmc_claim_host(host);
> >             sdio_disable_wide(host->card);
> >             mmc_release_host(host);
> >     }
> >
> > -   if (!err && !mmc_card_keep_power(host))
> > +   if (!mmc_card_keep_power(host))
> >             mmc_power_off(host);
> >
> > -   return err;
> > +   return 0;
> >  }
> >
> >  static int mmc_sdio_resume(struct mmc_host *host)  {
> > -   int i, err = 0;
> > +   int err = 0;
> >
> >     BUG_ON(!host);
> >     BUG_ON(!host->card);
> > @@ -1019,24 +1000,6 @@ static int mmc_sdio_resume(struct mmc_host *host)
> >             wake_up_process(host->sdio_irq_thread);
> >     mmc_release_host(host);
> >
> > -   /*
> > -    * If the card looked to be the same as before suspending, then
> > -    * we proceed to resume all card functions.  If one of them returns
> > -    * an error then we simply return that error to the core and the
> > -    * card will be redetected as new.  It is the responsibility of
> > -    * the function driver to perform further tests with the extra
> > -    * knowledge it has of the card to confirm the card is indeed the
> > -    * same as before suspending (same MAC address for network cards,
> > -    * etc.) and return an error otherwise.
> > -    */
> > -   for (i = 0; !err && i < host->card->sdio_funcs; i++) {
> > -           struct sdio_func *func = host->card->sdio_func[i];
> > -           if (func && sdio_func_present(func) && func->dev.driver) {
> > -                   const struct dev_pm_ops *pmops = func->dev.driver->pm;
> > -                   err = pmops->resume(&func->dev);
> > -           }
> > -   }
> > -
> >     host->pm_flags &= ~MMC_PM_KEEP_POWER;
> >     return err;
> >  }
> > diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
> > index 92d1ba8..4fa8fef9 100644
> > --- a/drivers/mmc/core/sdio_bus.c
> > +++ b/drivers/mmc/core/sdio_bus.c
> > @@ -197,20 +197,8 @@ static int sdio_bus_remove(struct device *dev)
> >
> >  #ifdef CONFIG_PM
> >
> > -#ifdef CONFIG_PM_SLEEP
> > -static int pm_no_operation(struct device *dev) -{
> > -   /*
> > -    * Prevent the PM core from calling SDIO device drivers' suspend
> > -    * callback routines, which it is not supposed to do, by using this
> > -    * empty function as the bus type suspend callaback for SDIO.
> > -    */
> > -   return 0;
> > -}
> > -#endif
> > -
> >  static const struct dev_pm_ops sdio_bus_pm_ops = {
> > -   SET_SYSTEM_SLEEP_PM_OPS(pm_no_operation, pm_no_operation)
> > +   SET_SYSTEM_SLEEP_PM_OPS(pm_generic_suspend, pm_generic_resume)
> >     SET_RUNTIME_PM_OPS(
> >             pm_generic_runtime_suspend,
> >             pm_generic_runtime_resume,
> >

Reply via email to