lkml.org 
[lkml]   [2014]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [RFC PATCH] mmc: core: Invoke sdio func driver's PM callbacks from the sdio bus
    From
    On 12 March 2014 07:26, Aaron Lu <aaron.lu@intel.com> wrote:
    > On 03/12/2014 11:44 AM, Dong, Chuanxiao wrote:
    >> 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.wang@intel.com>
    >> Tested-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
    >
    > Thanks a lot for the test!
    >
    > -Aaron
    >

    Thanks for helping out testing!

    Just out of curiosity, which sdio func driver did you use (or maybe it
    hasn't been upstreamed yet)?

    Anyway, I suppose it's ->suspend callback don't return -ENOSYS with
    the expectation of the card to be removed?

    So I assume you want this to go to stable as well, right?

    Kind regards
    Uffe

    >>
    >> Thanks
    >> Chuanxiao
    >>
    >>> -----Original Message-----
    >>> From: Lu, Aaron
    >>> Sent: Wednesday, March 12, 2014 10:36 AM
    >>> To: Ulf Hansson; linux-mmc@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.lu@intel.com>
    >>>> Cc: NeilBrown <neilb@suse.de>
    >>>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
    >>>> Signed-off-by: Ulf Hansson <ulf.hansson@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,
    >>>>
    >>
    >


    \
     
     \ /
      Last update: 2014-03-12 14:21    [W:5.749 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site