| Message ID | 1333101747-3200-1-git-send-email-girish.shivananjappa@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Mar 30, 2012 at 12:02 PM, Girish K S <girish.shivananjappa@linaro.org> wrote: > This is a rework of the existing POWER OFF NOTIFY patch. The current problem > with the patch comes from the ambiguity on the usage of POWER OFF NOTIFY > together with SLEEP and misunderstanding on the usage of MMC_POWER_OFF > power_mode from mmc_set_ios in different host controller drivers. > > This new patch works around this problem by adding a new host CAP, > MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND, which when set sends a > POWER OFF NOTIFY from mmc_suspend instead of SLEEP. It is expected that host > controller drivers will set this CAP, if they switch off both Vcc and Vccq > from MMC_POWER_OFF condition within mmc_set_ios. However, note that there > is no harm in sending MMC_POWER_NOTIFY even if Vccq is not switched off. > > This patch also sends POWER OFF NOTIFY from power management routines (e.g. > mmc_power_save_host, mmc_pm_notify/PM_SUSPEND_PREPARE, mmc_stop_host), which > does reinitialization of the eMMC on the return path of the power management > routines (e.g. mmc_power_restore_host, mmc_pm_notify/PM_POST_RESTORE, > mmc_start_host). > > Signed-off-by: Saugata Das <saugata.das@linaro.org> > Signed-off-by: Girish K S <girish.shivananjappa@linaro.org> Overall this looks good! I think it may be possible to split the patch. First a patch that moves mmc_card_set_sleep() and mmc_card_clr_sleep() into mmc_card_sleep() and mmc_card_awake(), which is a good refactoring in its own right. Then a patch doing the rest of the changes. (But that's no big deal to me if Chris is OK with this.) > -static void mmc_poweroff_notify(struct mmc_host *host) > +int mmc_poweroff_notify(struct mmc_host *host) So now this function will return an errorcode if notification fails, maybe add some kerneldoc... > @@ -2112,7 +2096,8 @@ int mmc_power_save_host(struct mmc_host *host) > > if (host->bus_ops->power_save) > ret = host->bus_ops->power_save(host); > - > + host->power_notify_type = MMC_HOST_PW_NOTIFY_SHORT; > + mmc_poweroff_notify(host); So no risk in ignoring poweroff notification failure here I guess.. (Just thinking aloud.) > @@ -2135,7 +2120,7 @@ int mmc_power_restore_host(struct mmc_host *host) > mmc_bus_put(host); > return -EINVAL; > } > - > + host->power_notify_type = MMC_HOST_PW_NOTIFY_LONG; This looks new, can you explain in the code as comments or in the commit message when SHORT and LONG notifications are used and why? > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index 02914d6..885ad61 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1338,12 +1338,18 @@ static int mmc_suspend(struct mmc_host *host) > BUG_ON(!host->card); > > mmc_claim_host(host); > - if (mmc_card_can_sleep(host)) { > - err = mmc_card_sleep(host); > + if (host->caps2 & MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) { > + err = mmc_poweroff_notify(host); > if (!err) > - mmc_card_set_sleep(host->card); > - } else if (!mmc_host_is_spi(host)) > + goto out; > + } > + > + if (mmc_card_can_sleep(host)) > + err = mmc_card_sleep(host); > + else if (!mmc_host_is_spi(host)) > mmc_deselect_cards(host); Are you sure you should not deselect the card on an SPI host also if you power off? (I'm just confused, better to ask...) > + > +out: So if I understand correctly we power off if possible, else we set the card to sleep. (Looks good.) > mmc_claim_host(host); > - if (mmc_card_is_sleep(host->card)) { > + if (mmc_card_is_sleep(host->card)) > err = mmc_card_awake(host); > - mmc_card_clr_sleep(host->card); > - } else > + else > err = mmc_init_card(host, host->ocr, host->card); So a powered-off card will be reinitialized here, nice! Yours, Linus Walleij
On 30 March 2012 21:19, Linus Walleij <linus.walleij@linaro.org> wrote: > On Fri, Mar 30, 2012 at 12:02 PM, Girish K S > <girish.shivananjappa@linaro.org> wrote: > >> This is a rework of the existing POWER OFF NOTIFY patch. The current problem >> with the patch comes from the ambiguity on the usage of POWER OFF NOTIFY >> together with SLEEP and misunderstanding on the usage of MMC_POWER_OFF >> power_mode from mmc_set_ios in different host controller drivers. >> >> This new patch works around this problem by adding a new host CAP, >> MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND, which when set sends a >> POWER OFF NOTIFY from mmc_suspend instead of SLEEP. It is expected that host >> controller drivers will set this CAP, if they switch off both Vcc and Vccq >> from MMC_POWER_OFF condition within mmc_set_ios. However, note that there >> is no harm in sending MMC_POWER_NOTIFY even if Vccq is not switched off. >> >> This patch also sends POWER OFF NOTIFY from power management routines (e.g. >> mmc_power_save_host, mmc_pm_notify/PM_SUSPEND_PREPARE, mmc_stop_host), which >> does reinitialization of the eMMC on the return path of the power management >> routines (e.g. mmc_power_restore_host, mmc_pm_notify/PM_POST_RESTORE, >> mmc_start_host). >> >> Signed-off-by: Saugata Das <saugata.das@linaro.org> >> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org> > > Overall this looks good! > > I think it may be possible to split the patch. > First a patch that moves mmc_card_set_sleep() and mmc_card_clr_sleep() > into mmc_card_sleep() and mmc_card_awake(), which is a good > refactoring in its own right. Then a patch doing the rest of the changes. > > (But that's no big deal to me if Chris is OK with this.) > >> -static void mmc_poweroff_notify(struct mmc_host *host) >> +int mmc_poweroff_notify(struct mmc_host *host) > > So now this function will return an errorcode if notification fails, > maybe add some kerneldoc... > >> @@ -2112,7 +2096,8 @@ int mmc_power_save_host(struct mmc_host *host) >> >> if (host->bus_ops->power_save) >> ret = host->bus_ops->power_save(host); >> - >> + host->power_notify_type = MMC_HOST_PW_NOTIFY_SHORT; >> + mmc_poweroff_notify(host); > > So no risk in ignoring poweroff notification failure here I guess.. > (Just thinking aloud.) > >> @@ -2135,7 +2120,7 @@ int mmc_power_restore_host(struct mmc_host *host) >> mmc_bus_put(host); >> return -EINVAL; >> } >> - >> + host->power_notify_type = MMC_HOST_PW_NOTIFY_LONG; > > This looks new, can you explain in the code as comments > or in the commit message when SHORT and LONG notifications are > used and why? > The mmc_poweroff_notify with MMC_HOST_PW_NOTIFY_LONG will take longer time to complete than MMC_HOST_PW_NOTIFY_SHORT. So, the idea is that if we use mmc_poweroff_notify on suspend path, then we should use MMC_HOST_PW_NOTIFY_SHORT and if we use mmc_poweroff_notify from the shutdown path, then we should use MMC_HOST_PW_NOTIFY_LONG. We will add this in the description. >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> index 02914d6..885ad61 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -1338,12 +1338,18 @@ static int mmc_suspend(struct mmc_host *host) >> BUG_ON(!host->card); >> >> mmc_claim_host(host); >> - if (mmc_card_can_sleep(host)) { >> - err = mmc_card_sleep(host); >> + if (host->caps2 & MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) { >> + err = mmc_poweroff_notify(host); >> if (!err) >> - mmc_card_set_sleep(host->card); >> - } else if (!mmc_host_is_spi(host)) >> + goto out; >> + } >> + >> + if (mmc_card_can_sleep(host)) >> + err = mmc_card_sleep(host); >> + else if (!mmc_host_is_spi(host)) >> mmc_deselect_cards(host); > > Are you sure you should not deselect the card on an SPI host also if > you power off? (I'm just confused, better to ask...) > eMMC does not support SPI mode. So, the POWER OFF NOTIFY, which is an eMMC feature, can not be used on SPI mode. The above code (which you are referring) puts the eMMC to low power "standby" state with mmc_deselect_cards if sleep is not allowed. This logic has not been modified by the POWER OFF NOTIFY patch. >> + >> +out: > > So if I understand correctly we power off if possible, else we > set the card to sleep. (Looks good.) > >> mmc_claim_host(host); >> - if (mmc_card_is_sleep(host->card)) { >> + if (mmc_card_is_sleep(host->card)) >> err = mmc_card_awake(host); >> - mmc_card_clr_sleep(host->card); >> - } else >> + else >> err = mmc_init_card(host, host->ocr, host->card); > > So a powered-off card will be reinitialized here, nice! > > Yours, > Linus Walleij
On Mon, Apr 2, 2012 at 10:47 AM, Saugata Das <saugata.das@linaro.org> wrote: >>> + host->power_notify_type = MMC_HOST_PW_NOTIFY_LONG; >> >> This looks new, can you explain in the code as comments >> or in the commit message when SHORT and LONG notifications are >> used and why? >> > > The mmc_poweroff_notify with MMC_HOST_PW_NOTIFY_LONG will take longer > time to complete than MMC_HOST_PW_NOTIFY_SHORT. So, the idea is that > if we use mmc_poweroff_notify on suspend path, then we should use > MMC_HOST_PW_NOTIFY_SHORT and if we use mmc_poweroff_notify from the > shutdown path, then we should use MMC_HOST_PW_NOTIFY_LONG. > > We will add this in the description. OK sounds good. >>> @@ -1338,12 +1338,18 @@ static int mmc_suspend(struct mmc_host *host) >>> BUG_ON(!host->card); >>> >>> mmc_claim_host(host); >>> - if (mmc_card_can_sleep(host)) { >>> - err = mmc_card_sleep(host); >>> + if (host->caps2 & MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) { >>> + err = mmc_poweroff_notify(host); >>> if (!err) >>> - mmc_card_set_sleep(host->card); >>> - } else if (!mmc_host_is_spi(host)) >>> + goto out; >>> + } >>> + >>> + if (mmc_card_can_sleep(host)) >>> + err = mmc_card_sleep(host); >>> + else if (!mmc_host_is_spi(host)) >>> mmc_deselect_cards(host); >> >> Are you sure you should not deselect the card on an SPI host also if >> you power off? (I'm just confused, better to ask...) >> > > eMMC does not support SPI mode. So, the POWER OFF NOTIFY, which is an > eMMC feature, can not be used on SPI mode. The above code (which you > are referring) puts the eMMC to low power "standby" state with > mmc_deselect_cards if sleep is not allowed. This logic has not been > modified by the POWER OFF NOTIFY patch. OK I would add a small comment above the else if (!mmc_host_is_spi(host)) such as /* SPI mode is only used external cards */ or so, it helsps when reading the code. Yours, Linus Walleij
On 2 April 2012 21:26, Linus Walleij <linus.walleij@linaro.org> wrote: > On Mon, Apr 2, 2012 at 10:47 AM, Saugata Das <saugata.das@linaro.org> wrote: > >>>> + host->power_notify_type = MMC_HOST_PW_NOTIFY_LONG; >>> >>> This looks new, can you explain in the code as comments >>> or in the commit message when SHORT and LONG notifications are >>> used and why? >>> >> >> The mmc_poweroff_notify with MMC_HOST_PW_NOTIFY_LONG will take longer >> time to complete than MMC_HOST_PW_NOTIFY_SHORT. So, the idea is that >> if we use mmc_poweroff_notify on suspend path, then we should use >> MMC_HOST_PW_NOTIFY_SHORT and if we use mmc_poweroff_notify from the >> shutdown path, then we should use MMC_HOST_PW_NOTIFY_LONG. >> >> We will add this in the description. > > OK sounds good. > >>>> @@ -1338,12 +1338,18 @@ static int mmc_suspend(struct mmc_host *host) >>>> BUG_ON(!host->card); >>>> >>>> mmc_claim_host(host); >>>> - if (mmc_card_can_sleep(host)) { >>>> - err = mmc_card_sleep(host); >>>> + if (host->caps2 & MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) { >>>> + err = mmc_poweroff_notify(host); >>>> if (!err) >>>> - mmc_card_set_sleep(host->card); >>>> - } else if (!mmc_host_is_spi(host)) >>>> + goto out; >>>> + } >>>> + >>>> + if (mmc_card_can_sleep(host)) >>>> + err = mmc_card_sleep(host); >>>> + else if (!mmc_host_is_spi(host)) >>>> mmc_deselect_cards(host); >>> >>> Are you sure you should not deselect the card on an SPI host also if >>> you power off? (I'm just confused, better to ask...) >>> >> >> eMMC does not support SPI mode. So, the POWER OFF NOTIFY, which is an >> eMMC feature, can not be used on SPI mode. The above code (which you >> are referring) puts the eMMC to low power "standby" state with >> mmc_deselect_cards if sleep is not allowed. This logic has not been >> modified by the POWER OFF NOTIFY patch. > > OK I would add a small comment above the else if (!mmc_host_is_spi(host)) > such as /* SPI mode is only used external cards */ or so, it helsps when > reading the code. > I understand but power off notify patch did not introduce this. May be, we can add this comment in a different patch. If no other comments, then we will resubmit this patch with modification of the description. > Yours, > Linus Walleij
On 03/30/2012 12:02 PM, Girish K S wrote: > This is a rework of the existing POWER OFF NOTIFY patch. The current problem > with the patch comes from the ambiguity on the usage of POWER OFF NOTIFY > together with SLEEP and misunderstanding on the usage of MMC_POWER_OFF > power_mode from mmc_set_ios in different host controller drivers. > > This new patch works around this problem by adding a new host CAP, > MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND, which when set sends a > POWER OFF NOTIFY from mmc_suspend instead of SLEEP. It is expected that host > controller drivers will set this CAP, if they switch off both Vcc and Vccq > from MMC_POWER_OFF condition within mmc_set_ios. However, note that there > is no harm in sending MMC_POWER_NOTIFY even if Vccq is not switched off. > > This patch also sends POWER OFF NOTIFY from power management routines (e.g. > mmc_power_save_host, mmc_pm_notify/PM_SUSPEND_PREPARE, mmc_stop_host), which > does reinitialization of the eMMC on the return path of the power management > routines (e.g. mmc_power_restore_host, mmc_pm_notify/PM_POST_RESTORE, > mmc_start_host). > > Signed-off-by: Saugata Das<saugata.das@linaro.org> > Signed-off-by: Girish K S<girish.shivananjappa@linaro.org> > --- > drivers/mmc/core/core.c | 36 ++++++++++++++---------------------- > drivers/mmc/core/core.h | 1 + > drivers/mmc/core/mmc.c | 20 +++++++++++++------- > include/linux/mmc/host.h | 1 + > 4 files changed, 29 insertions(+), 29 deletions(-) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 14f262e..dc85ee1 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -1096,12 +1096,12 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type) > mmc_host_clk_release(host); > } > > -static void mmc_poweroff_notify(struct mmc_host *host) > +int mmc_poweroff_notify(struct mmc_host *host) A more generic comment; why is not this function implemented as a bus_ops function, similar how sleep for mmc is done? That would be more preferred I think. > { > struct mmc_card *card; > unsigned int timeout; > unsigned int notify_type = EXT_CSD_NO_POWER_NOTIFICATION; > - int err = 0; > + int err = -EINVAL; > > card = host->card; > mmc_claim_host(host); > @@ -1136,6 +1136,7 @@ static void mmc_poweroff_notify(struct mmc_host *host) > card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; > } > mmc_release_host(host); > + return err; > } > > /* > @@ -1194,30 +1195,12 @@ static void mmc_power_up(struct mmc_host *host) > > void mmc_power_off(struct mmc_host *host) > { > - int err = 0; > mmc_host_clk_hold(host); > > host->ios.clock = 0; > host->ios.vdd = 0; > > /* > - * For eMMC 4.5 device send AWAKE command before > - * POWER_OFF_NOTIFY command, because in sleep state > - * eMMC 4.5 devices respond to only RESET and AWAKE cmd > - */ > - if (host->card&& mmc_card_is_sleep(host->card)&& > - host->bus_ops->resume) { > - err = host->bus_ops->resume(host); > - > - if (!err) > - mmc_poweroff_notify(host); > - else > - pr_warning("%s: error %d during resume " > - "(continue with poweroff sequence)\n", > - mmc_hostname(host), err); > - } > - > - /* > * Reset ocr mask to be the highest possible voltage supported for > * this mmc host. This value will be used at next power up. > */ > @@ -2076,6 +2059,7 @@ void mmc_stop_host(struct mmc_host *host) > host->pm_flags = 0; > > mmc_bus_get(host); > + mmc_poweroff_notify(host); We must not do mmc_poweroff_notify, without knowing we have a card attached through the bus_ops. > if (host->bus_ops&& !host->bus_dead) { > /* Calling bus_ops->remove() with a claimed host can deadlock */ > if (host->bus_ops->remove) > @@ -2112,7 +2096,8 @@ int mmc_power_save_host(struct mmc_host *host) > > if (host->bus_ops->power_save) > ret = host->bus_ops->power_save(host); > - > + host->power_notify_type = MMC_HOST_PW_NOTIFY_SHORT; > + mmc_poweroff_notify(host); Same comment as above. Moreover, mmc_power_save_host is not working for eMMC cards supporting the SLEEP command (violating the eMMC spec for how VCC is cut). So, I am not sure we should introduce the poweroff_notify for the mmc_power_save_host right now. Better to fix SLEEP first, if needed at all. Additionally, this function is only used for SDIO if runtime PM is enabled at the moment. > mmc_bus_put(host); > > mmc_power_off(host); > @@ -2135,7 +2120,7 @@ int mmc_power_restore_host(struct mmc_host *host) > mmc_bus_put(host); > return -EINVAL; > } > - > + host->power_notify_type = MMC_HOST_PW_NOTIFY_LONG; Similar comments as for mmc_power_save_host > mmc_power_up(host); > ret = host->bus_ops->power_restore(host); > > @@ -2157,6 +2142,9 @@ int mmc_card_awake(struct mmc_host *host) > if (host->bus_ops&& !host->bus_dead&& host->bus_ops->awake) > err = host->bus_ops->awake(host); > > + if (!err) > + mmc_card_clr_sleep(host->card); > + This code must be handled from withing the bus_ops function instead, or you need to move it inside the "if" above, since the awake bus_ops only exist for (e)MMC. Moreover, you do not want to set the sleep state unless the sleep cmd really has been executed. > mmc_bus_put(host); > > return err; > @@ -2175,6 +2163,9 @@ int mmc_card_sleep(struct mmc_host *host) > if (host->bus_ops&& !host->bus_dead&& host->bus_ops->sleep) > err = host->bus_ops->sleep(host); > > + if (!err) > + mmc_card_set_sleep(host->card); > + I think this code should be handled from withing the bus_ops function instead, or you need to move it inside the "if" above, since the awake bus_ops only exist for (e)MMC. > mmc_bus_put(host); > > return err; > @@ -2393,6 +2384,7 @@ int mmc_pm_notify(struct notifier_block *notify_block, > host->bus_ops->remove(host); > > mmc_claim_host(host); > + mmc_poweroff_notify(host); This wont work if the card has been removed from the bus_ops->remove above. > mmc_detach_bus(host); > mmc_power_off(host); > mmc_release_host(host); > diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h > index 3bdafbc..0bdc0fd 100644 > --- a/drivers/mmc/core/core.h > +++ b/drivers/mmc/core/core.h > @@ -45,6 +45,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, > void mmc_set_timing(struct mmc_host *host, unsigned int timing); > void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); > void mmc_power_off(struct mmc_host *host); > +int mmc_poweroff_notify(struct mmc_host *host); > > static inline void mmc_delay(unsigned int ms) > { > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index 02914d6..885ad61 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1338,12 +1338,18 @@ static int mmc_suspend(struct mmc_host *host) > BUG_ON(!host->card); > > mmc_claim_host(host); > - if (mmc_card_can_sleep(host)) { > - err = mmc_card_sleep(host); > + if (host->caps2& MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) { 1. missing whitespace after caps2 2. Can't we have an mmc_card_can_poweroff_notify to check instead, which also considering this new cap together with the card features. > + err = mmc_poweroff_notify(host); > if (!err) > - mmc_card_set_sleep(host->card); > - } else if (!mmc_host_is_spi(host)) > + goto out; Suggest to remove the goto and just have another if-else, as we already have for sleep. > + } > + > + if (mmc_card_can_sleep(host)) > + err = mmc_card_sleep(host); > + else if (!mmc_host_is_spi(host)) > mmc_deselect_cards(host); > + > +out: > host->card->state&= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); > mmc_release_host(host); > > @@ -1364,11 +1370,11 @@ static int mmc_resume(struct mmc_host *host) > BUG_ON(!host->card); > > mmc_claim_host(host); > - if (mmc_card_is_sleep(host->card)) { > + if (mmc_card_is_sleep(host->card)) > err = mmc_card_awake(host); > - mmc_card_clr_sleep(host->card); > - } else > + else > err = mmc_init_card(host, host->ocr, host->card); > + > mmc_release_host(host); > > return err; > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h > index 33a4d08..3749a42 100644 > --- a/include/linux/mmc/host.h > +++ b/include/linux/mmc/host.h > @@ -237,6 +237,7 @@ struct mmc_host { > #define MMC_CAP2_BROKEN_VOLTAGE (1<< 7) /* Use the broken voltage */ > #define MMC_CAP2_DETECT_ON_ERR (1<< 8) /* On I/O err check card removal */ > #define MMC_CAP2_HC_ERASE_SZ (1<< 9) /* High-capacity erase size */ > +#define MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND (1<< 10) > > mmc_pm_flag_t pm_caps; /* supported pm features */ > unsigned int power_notify_type; Kind regards Ulf Hansson
On 4 April 2012 17:03, Ulf Hansson <ulf.hansson@stericsson.com> wrote: > On 03/30/2012 12:02 PM, Girish K S wrote: >> >> This is a rework of the existing POWER OFF NOTIFY patch. The current >> problem >> with the patch comes from the ambiguity on the usage of POWER OFF NOTIFY >> together with SLEEP and misunderstanding on the usage of MMC_POWER_OFF >> power_mode from mmc_set_ios in different host controller drivers. >> >> This new patch works around this problem by adding a new host CAP, >> MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND, which when set sends a >> POWER OFF NOTIFY from mmc_suspend instead of SLEEP. It is expected that >> host >> controller drivers will set this CAP, if they switch off both Vcc and Vccq >> from MMC_POWER_OFF condition within mmc_set_ios. However, note that there >> is no harm in sending MMC_POWER_NOTIFY even if Vccq is not switched off. >> >> This patch also sends POWER OFF NOTIFY from power management routines >> (e.g. >> mmc_power_save_host, mmc_pm_notify/PM_SUSPEND_PREPARE, mmc_stop_host), >> which >> does reinitialization of the eMMC on the return path of the power >> management >> routines (e.g. mmc_power_restore_host, mmc_pm_notify/PM_POST_RESTORE, >> mmc_start_host). >> >> Signed-off-by: Saugata Das<saugata.das@linaro.org> >> Signed-off-by: Girish K S<girish.shivananjappa@linaro.org> >> --- >> drivers/mmc/core/core.c | 36 ++++++++++++++---------------------- >> drivers/mmc/core/core.h | 1 + >> drivers/mmc/core/mmc.c | 20 +++++++++++++------- >> include/linux/mmc/host.h | 1 + >> 4 files changed, 29 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> index 14f262e..dc85ee1 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -1096,12 +1096,12 @@ void mmc_set_driver_type(struct mmc_host *host, >> unsigned int drv_type) >> mmc_host_clk_release(host); >> } >> >> -static void mmc_poweroff_notify(struct mmc_host *host) >> +int mmc_poweroff_notify(struct mmc_host *host) > > > A more generic comment; why is not this function implemented as a bus_ops > function, similar how sleep for mmc is done? That would be more preferred I > think. > > >> { >> struct mmc_card *card; >> unsigned int timeout; >> unsigned int notify_type = EXT_CSD_NO_POWER_NOTIFICATION; >> - int err = 0; >> + int err = -EINVAL; >> >> card = host->card; >> mmc_claim_host(host); >> @@ -1136,6 +1136,7 @@ static void mmc_poweroff_notify(struct mmc_host >> *host) >> card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; >> } >> mmc_release_host(host); >> + return err; >> } >> >> /* >> @@ -1194,30 +1195,12 @@ static void mmc_power_up(struct mmc_host *host) >> >> void mmc_power_off(struct mmc_host *host) >> { >> - int err = 0; >> mmc_host_clk_hold(host); >> >> host->ios.clock = 0; >> host->ios.vdd = 0; >> >> /* >> - * For eMMC 4.5 device send AWAKE command before >> - * POWER_OFF_NOTIFY command, because in sleep state >> - * eMMC 4.5 devices respond to only RESET and AWAKE cmd >> - */ >> - if (host->card&& mmc_card_is_sleep(host->card)&& >> - host->bus_ops->resume) { >> - err = host->bus_ops->resume(host); >> - >> - if (!err) >> - mmc_poweroff_notify(host); >> - else >> - pr_warning("%s: error %d during resume " >> - "(continue with poweroff sequence)\n", >> - mmc_hostname(host), err); >> - } >> - >> - /* >> * Reset ocr mask to be the highest possible voltage supported for >> * this mmc host. This value will be used at next power up. >> */ >> @@ -2076,6 +2059,7 @@ void mmc_stop_host(struct mmc_host *host) >> host->pm_flags = 0; >> >> mmc_bus_get(host); >> + mmc_poweroff_notify(host); > > > We must not do mmc_poweroff_notify, without knowing we have a card attached > through the bus_ops. > >> if (host->bus_ops&& !host->bus_dead) { >> >> /* Calling bus_ops->remove() with a claimed host can >> deadlock */ >> if (host->bus_ops->remove) >> @@ -2112,7 +2096,8 @@ int mmc_power_save_host(struct mmc_host *host) >> >> if (host->bus_ops->power_save) >> ret = host->bus_ops->power_save(host); >> - >> + host->power_notify_type = MMC_HOST_PW_NOTIFY_SHORT; >> + mmc_poweroff_notify(host); > > > Same comment as above. > > Moreover, mmc_power_save_host is not working for eMMC cards supporting the > SLEEP command (violating the eMMC spec for how VCC is cut). So, I am not > sure we should introduce the poweroff_notify for the mmc_power_save_host > right now. Better to fix SLEEP first, if needed at all. > > Additionally, this function is only used for SDIO if runtime PM is enabled > at the moment. > > >> mmc_bus_put(host); >> >> mmc_power_off(host); >> @@ -2135,7 +2120,7 @@ int mmc_power_restore_host(struct mmc_host *host) >> mmc_bus_put(host); >> return -EINVAL; >> } >> - >> + host->power_notify_type = MMC_HOST_PW_NOTIFY_LONG; > > > Similar comments as for mmc_power_save_host > >> mmc_power_up(host); >> ret = host->bus_ops->power_restore(host); >> >> @@ -2157,6 +2142,9 @@ int mmc_card_awake(struct mmc_host *host) >> if (host->bus_ops&& !host->bus_dead&& host->bus_ops->awake) >> >> err = host->bus_ops->awake(host); >> >> + if (!err) >> + mmc_card_clr_sleep(host->card); >> + > > > This code must be handled from withing the bus_ops function instead, or you > need to move it inside the "if" above, since the awake bus_ops only exist > for (e)MMC. Moreover, you do not want to set the sleep state unless the > sleep cmd really has been executed. > >> mmc_bus_put(host); >> >> return err; >> @@ -2175,6 +2163,9 @@ int mmc_card_sleep(struct mmc_host *host) >> if (host->bus_ops&& !host->bus_dead&& host->bus_ops->sleep) >> >> err = host->bus_ops->sleep(host); >> >> + if (!err) >> + mmc_card_set_sleep(host->card); >> + > > > I think this code should be handled from withing the bus_ops function > instead, or you need to move it inside the "if" above, since the awake > bus_ops only exist for (e)MMC. > > >> mmc_bus_put(host); >> >> return err; >> @@ -2393,6 +2384,7 @@ int mmc_pm_notify(struct notifier_block >> *notify_block, >> host->bus_ops->remove(host); >> >> mmc_claim_host(host); >> + mmc_poweroff_notify(host); > > > This wont work if the card has been removed from the bus_ops->remove above. > > >> mmc_detach_bus(host); >> mmc_power_off(host); >> mmc_release_host(host); >> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h >> index 3bdafbc..0bdc0fd 100644 >> --- a/drivers/mmc/core/core.h >> +++ b/drivers/mmc/core/core.h >> @@ -45,6 +45,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int >> signal_voltage, >> void mmc_set_timing(struct mmc_host *host, unsigned int timing); >> void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); >> void mmc_power_off(struct mmc_host *host); >> +int mmc_poweroff_notify(struct mmc_host *host); >> >> static inline void mmc_delay(unsigned int ms) >> { >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> index 02914d6..885ad61 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -1338,12 +1338,18 @@ static int mmc_suspend(struct mmc_host *host) >> BUG_ON(!host->card); >> >> mmc_claim_host(host); >> - if (mmc_card_can_sleep(host)) { >> - err = mmc_card_sleep(host); >> + if (host->caps2& MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) { > > 1. missing whitespace after caps2 > 2. Can't we have an mmc_card_can_poweroff_notify to check instead, which > also considering this new cap together with the card features. > >> + err = mmc_poweroff_notify(host); >> if (!err) >> - mmc_card_set_sleep(host->card); >> - } else if (!mmc_host_is_spi(host)) >> + goto out; > > Suggest to remove the goto and just have another if-else, as we already have > for sleep. >> >> + } >> + >> + if (mmc_card_can_sleep(host)) >> + err = mmc_card_sleep(host); >> + else if (!mmc_host_is_spi(host)) >> mmc_deselect_cards(host); >> + >> +out: >> host->card->state&= ~(MMC_STATE_HIGHSPEED | >> MMC_STATE_HIGHSPEED_200); >> >> mmc_release_host(host); >> >> @@ -1364,11 +1370,11 @@ static int mmc_resume(struct mmc_host *host) >> BUG_ON(!host->card); >> >> mmc_claim_host(host); >> - if (mmc_card_is_sleep(host->card)) { >> + if (mmc_card_is_sleep(host->card)) >> err = mmc_card_awake(host); >> - mmc_card_clr_sleep(host->card); >> - } else >> + else >> err = mmc_init_card(host, host->ocr, host->card); >> + >> mmc_release_host(host); >> >> return err; >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >> index 33a4d08..3749a42 100644 >> --- a/include/linux/mmc/host.h >> +++ b/include/linux/mmc/host.h >> @@ -237,6 +237,7 @@ struct mmc_host { >> #define MMC_CAP2_BROKEN_VOLTAGE (1<< 7) /* Use the broken >> voltage */ >> #define MMC_CAP2_DETECT_ON_ERR (1<< 8) /* On I/O err check >> card removal */ >> #define MMC_CAP2_HC_ERASE_SZ (1<< 9) /* High-capacity erase size >> */ >> +#define MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND (1<< 10) >> >> mmc_pm_flag_t pm_caps; /* supported pm features */ >> unsigned int power_notify_type; > > I will check it once i am back from business trip. > Kind regards > Ulf Hansson
On 4 April 2012 17:03, Ulf Hansson <ulf.hansson@stericsson.com> wrote: > On 03/30/2012 12:02 PM, Girish K S wrote: >> >> This is a rework of the existing POWER OFF NOTIFY patch. The current >> problem >> with the patch comes from the ambiguity on the usage of POWER OFF NOTIFY >> together with SLEEP and misunderstanding on the usage of MMC_POWER_OFF >> power_mode from mmc_set_ios in different host controller drivers. >> >> This new patch works around this problem by adding a new host CAP, >> MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND, which when set sends a >> POWER OFF NOTIFY from mmc_suspend instead of SLEEP. It is expected that >> host >> controller drivers will set this CAP, if they switch off both Vcc and Vccq >> from MMC_POWER_OFF condition within mmc_set_ios. However, note that there >> is no harm in sending MMC_POWER_NOTIFY even if Vccq is not switched off. >> >> This patch also sends POWER OFF NOTIFY from power management routines >> (e.g. >> mmc_power_save_host, mmc_pm_notify/PM_SUSPEND_PREPARE, mmc_stop_host), >> which >> does reinitialization of the eMMC on the return path of the power >> management >> routines (e.g. mmc_power_restore_host, mmc_pm_notify/PM_POST_RESTORE, >> mmc_start_host). >> >> Signed-off-by: Saugata Das<saugata.das@linaro.org> >> Signed-off-by: Girish K S<girish.shivananjappa@linaro.org> >> --- >> drivers/mmc/core/core.c | 36 ++++++++++++++---------------------- >> drivers/mmc/core/core.h | 1 + >> drivers/mmc/core/mmc.c | 20 +++++++++++++------- >> include/linux/mmc/host.h | 1 + >> 4 files changed, 29 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> index 14f262e..dc85ee1 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -1096,12 +1096,12 @@ void mmc_set_driver_type(struct mmc_host *host, >> unsigned int drv_type) >> mmc_host_clk_release(host); >> } >> >> -static void mmc_poweroff_notify(struct mmc_host *host) >> +int mmc_poweroff_notify(struct mmc_host *host) > > > A more generic comment; why is not this function implemented as a bus_ops > function, similar how sleep for mmc is done? That would be more preferred I > think. > I have nothing for or against the idea. We just wanted to limit the changes in the patch. In this patch lets focus on the functional aspects of the power off notify and we can work on moving this function to bus_ops in a separate patch later. > >> { >> struct mmc_card *card; >> unsigned int timeout; >> unsigned int notify_type = EXT_CSD_NO_POWER_NOTIFICATION; >> - int err = 0; >> + int err = -EINVAL; >> >> card = host->card; >> mmc_claim_host(host); >> @@ -1136,6 +1136,7 @@ static void mmc_poweroff_notify(struct mmc_host >> *host) >> card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; >> } >> mmc_release_host(host); >> + return err; >> } >> >> /* >> @@ -1194,30 +1195,12 @@ static void mmc_power_up(struct mmc_host *host) >> >> void mmc_power_off(struct mmc_host *host) >> { >> - int err = 0; >> mmc_host_clk_hold(host); >> >> host->ios.clock = 0; >> host->ios.vdd = 0; >> >> /* >> - * For eMMC 4.5 device send AWAKE command before >> - * POWER_OFF_NOTIFY command, because in sleep state >> - * eMMC 4.5 devices respond to only RESET and AWAKE cmd >> - */ >> - if (host->card&& mmc_card_is_sleep(host->card)&& >> - host->bus_ops->resume) { >> - err = host->bus_ops->resume(host); >> - >> - if (!err) >> - mmc_poweroff_notify(host); >> - else >> - pr_warning("%s: error %d during resume " >> - "(continue with poweroff sequence)\n", >> - mmc_hostname(host), err); >> - } >> - >> - /* >> * Reset ocr mask to be the highest possible voltage supported for >> * this mmc host. This value will be used at next power up. >> */ >> @@ -2076,6 +2059,7 @@ void mmc_stop_host(struct mmc_host *host) >> host->pm_flags = 0; >> >> mmc_bus_get(host); >> + mmc_poweroff_notify(host); > > > We must not do mmc_poweroff_notify, without knowing we have a card attached > through the bus_ops. > >> if (host->bus_ops&& !host->bus_dead) { >> >> /* Calling bus_ops->remove() with a claimed host can >> deadlock */ >> if (host->bus_ops->remove) >> @@ -2112,7 +2096,8 @@ int mmc_power_save_host(struct mmc_host *host) >> >> if (host->bus_ops->power_save) >> ret = host->bus_ops->power_save(host); >> - >> + host->power_notify_type = MMC_HOST_PW_NOTIFY_SHORT; >> + mmc_poweroff_notify(host); > > > Same comment as above. > > Moreover, mmc_power_save_host is not working for eMMC cards supporting the > SLEEP command (violating the eMMC spec for how VCC is cut). So, I am not > sure we should introduce the poweroff_notify for the mmc_power_save_host > right now. Better to fix SLEEP first, if needed at all. > > Additionally, this function is only used for SDIO if runtime PM is enabled > at the moment. > > >> mmc_bus_put(host); >> >> mmc_power_off(host); >> @@ -2135,7 +2120,7 @@ int mmc_power_restore_host(struct mmc_host *host) >> mmc_bus_put(host); >> return -EINVAL; >> } >> - >> + host->power_notify_type = MMC_HOST_PW_NOTIFY_LONG; > > > Similar comments as for mmc_power_save_host > >> mmc_power_up(host); >> ret = host->bus_ops->power_restore(host); >> >> @@ -2157,6 +2142,9 @@ int mmc_card_awake(struct mmc_host *host) >> if (host->bus_ops&& !host->bus_dead&& host->bus_ops->awake) >> >> err = host->bus_ops->awake(host); >> >> + if (!err) >> + mmc_card_clr_sleep(host->card); >> + > > > This code must be handled from withing the bus_ops function instead, or you > need to move it inside the "if" above, since the awake bus_ops only exist > for (e)MMC. Moreover, you do not want to set the sleep state unless the > sleep cmd really has been executed. > >> mmc_bus_put(host); >> >> return err; >> @@ -2175,6 +2163,9 @@ int mmc_card_sleep(struct mmc_host *host) >> if (host->bus_ops&& !host->bus_dead&& host->bus_ops->sleep) >> >> err = host->bus_ops->sleep(host); >> >> + if (!err) >> + mmc_card_set_sleep(host->card); >> + > > > I think this code should be handled from withing the bus_ops function > instead, or you need to move it inside the "if" above, since the awake > bus_ops only exist for (e)MMC. > > >> mmc_bus_put(host); >> >> return err; >> @@ -2393,6 +2384,7 @@ int mmc_pm_notify(struct notifier_block >> *notify_block, >> host->bus_ops->remove(host); >> >> mmc_claim_host(host); >> + mmc_poweroff_notify(host); > > > This wont work if the card has been removed from the bus_ops->remove above. > > >> mmc_detach_bus(host); >> mmc_power_off(host); >> mmc_release_host(host); >> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h >> index 3bdafbc..0bdc0fd 100644 >> --- a/drivers/mmc/core/core.h >> +++ b/drivers/mmc/core/core.h >> @@ -45,6 +45,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int >> signal_voltage, >> void mmc_set_timing(struct mmc_host *host, unsigned int timing); >> void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); >> void mmc_power_off(struct mmc_host *host); >> +int mmc_poweroff_notify(struct mmc_host *host); >> >> static inline void mmc_delay(unsigned int ms) >> { >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> index 02914d6..885ad61 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -1338,12 +1338,18 @@ static int mmc_suspend(struct mmc_host *host) >> BUG_ON(!host->card); >> >> mmc_claim_host(host); >> - if (mmc_card_can_sleep(host)) { >> - err = mmc_card_sleep(host); >> + if (host->caps2& MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) { > > 1. missing whitespace after caps2 > 2. Can't we have an mmc_card_can_poweroff_notify to check instead, which > also considering this new cap together with the card features. The test of capability of the device is done inside the function mmc_poweroff_notify, which is invoked from different places. It is not needed to have additional capability check within this "if" condition. > >> + err = mmc_poweroff_notify(host); >> if (!err) >> - mmc_card_set_sleep(host->card); >> - } else if (!mmc_host_is_spi(host)) >> + goto out; > > Suggest to remove the goto and just have another if-else, as we already have > for sleep. >> >> + } >> + >> + if (mmc_card_can_sleep(host)) >> + err = mmc_card_sleep(host); >> + else if (!mmc_host_is_spi(host)) >> mmc_deselect_cards(host); >> + >> +out: >> host->card->state&= ~(MMC_STATE_HIGHSPEED | >> MMC_STATE_HIGHSPEED_200); >> >> mmc_release_host(host); >> >> @@ -1364,11 +1370,11 @@ static int mmc_resume(struct mmc_host *host) >> BUG_ON(!host->card); >> >> mmc_claim_host(host); >> - if (mmc_card_is_sleep(host->card)) { >> + if (mmc_card_is_sleep(host->card)) >> err = mmc_card_awake(host); >> - mmc_card_clr_sleep(host->card); >> - } else >> + else >> err = mmc_init_card(host, host->ocr, host->card); >> + >> mmc_release_host(host); >> >> return err; >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >> index 33a4d08..3749a42 100644 >> --- a/include/linux/mmc/host.h >> +++ b/include/linux/mmc/host.h >> @@ -237,6 +237,7 @@ struct mmc_host { >> #define MMC_CAP2_BROKEN_VOLTAGE (1<< 7) /* Use the broken >> voltage */ >> #define MMC_CAP2_DETECT_ON_ERR (1<< 8) /* On I/O err check >> card removal */ >> #define MMC_CAP2_HC_ERASE_SZ (1<< 9) /* High-capacity erase size >> */ >> +#define MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND (1<< 10) >> >> mmc_pm_flag_t pm_caps; /* supported pm features */ >> unsigned int power_notify_type; > > > Kind regards > Ulf Hansson
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 14f262e..dc85ee1 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1096,12 +1096,12 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type) mmc_host_clk_release(host); } -static void mmc_poweroff_notify(struct mmc_host *host) +int mmc_poweroff_notify(struct mmc_host *host) { struct mmc_card *card; unsigned int timeout; unsigned int notify_type = EXT_CSD_NO_POWER_NOTIFICATION; - int err = 0; + int err = -EINVAL; card = host->card; mmc_claim_host(host); @@ -1136,6 +1136,7 @@ static void mmc_poweroff_notify(struct mmc_host *host) card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; } mmc_release_host(host); + return err; } /* @@ -1194,30 +1195,12 @@ static void mmc_power_up(struct mmc_host *host) void mmc_power_off(struct mmc_host *host) { - int err = 0; mmc_host_clk_hold(host); host->ios.clock = 0; host->ios.vdd = 0; /* - * For eMMC 4.5 device send AWAKE command before - * POWER_OFF_NOTIFY command, because in sleep state - * eMMC 4.5 devices respond to only RESET and AWAKE cmd - */ - if (host->card && mmc_card_is_sleep(host->card) && - host->bus_ops->resume) { - err = host->bus_ops->resume(host); - - if (!err) - mmc_poweroff_notify(host); - else - pr_warning("%s: error %d during resume " - "(continue with poweroff sequence)\n", - mmc_hostname(host), err); - } - - /* * Reset ocr mask to be the highest possible voltage supported for * this mmc host. This value will be used at next power up. */ @@ -2076,6 +2059,7 @@ void mmc_stop_host(struct mmc_host *host) host->pm_flags = 0; mmc_bus_get(host); + mmc_poweroff_notify(host); if (host->bus_ops && !host->bus_dead) { /* Calling bus_ops->remove() with a claimed host can deadlock */ if (host->bus_ops->remove) @@ -2112,7 +2096,8 @@ int mmc_power_save_host(struct mmc_host *host) if (host->bus_ops->power_save) ret = host->bus_ops->power_save(host); - + host->power_notify_type = MMC_HOST_PW_NOTIFY_SHORT; + mmc_poweroff_notify(host); mmc_bus_put(host); mmc_power_off(host); @@ -2135,7 +2120,7 @@ int mmc_power_restore_host(struct mmc_host *host) mmc_bus_put(host); return -EINVAL; } - + host->power_notify_type = MMC_HOST_PW_NOTIFY_LONG; mmc_power_up(host); ret = host->bus_ops->power_restore(host); @@ -2157,6 +2142,9 @@ int mmc_card_awake(struct mmc_host *host) if (host->bus_ops && !host->bus_dead && host->bus_ops->awake) err = host->bus_ops->awake(host); + if (!err) + mmc_card_clr_sleep(host->card); + mmc_bus_put(host); return err; @@ -2175,6 +2163,9 @@ int mmc_card_sleep(struct mmc_host *host) if (host->bus_ops && !host->bus_dead && host->bus_ops->sleep) err = host->bus_ops->sleep(host); + if (!err) + mmc_card_set_sleep(host->card); + mmc_bus_put(host); return err; @@ -2393,6 +2384,7 @@ int mmc_pm_notify(struct notifier_block *notify_block, host->bus_ops->remove(host); mmc_claim_host(host); + mmc_poweroff_notify(host); mmc_detach_bus(host); mmc_power_off(host); mmc_release_host(host); diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 3bdafbc..0bdc0fd 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -45,6 +45,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, void mmc_set_timing(struct mmc_host *host, unsigned int timing); void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); void mmc_power_off(struct mmc_host *host); +int mmc_poweroff_notify(struct mmc_host *host); static inline void mmc_delay(unsigned int ms) { diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 02914d6..885ad61 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1338,12 +1338,18 @@ static int mmc_suspend(struct mmc_host *host) BUG_ON(!host->card); mmc_claim_host(host); - if (mmc_card_can_sleep(host)) { - err = mmc_card_sleep(host); + if (host->caps2 & MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) { + err = mmc_poweroff_notify(host); if (!err) - mmc_card_set_sleep(host->card); - } else if (!mmc_host_is_spi(host)) + goto out; + } + + if (mmc_card_can_sleep(host)) + err = mmc_card_sleep(host); + else if (!mmc_host_is_spi(host)) mmc_deselect_cards(host); + +out: host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); mmc_release_host(host); @@ -1364,11 +1370,11 @@ static int mmc_resume(struct mmc_host *host) BUG_ON(!host->card); mmc_claim_host(host); - if (mmc_card_is_sleep(host->card)) { + if (mmc_card_is_sleep(host->card)) err = mmc_card_awake(host); - mmc_card_clr_sleep(host->card); - } else + else err = mmc_init_card(host, host->ocr, host->card); + mmc_release_host(host); return err; diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 33a4d08..3749a42 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -237,6 +237,7 @@ struct mmc_host { #define MMC_CAP2_BROKEN_VOLTAGE (1 << 7) /* Use the broken voltage */ #define MMC_CAP2_DETECT_ON_ERR (1 << 8) /* On I/O err check card removal */ #define MMC_CAP2_HC_ERASE_SZ (1 << 9) /* High-capacity erase size */ +#define MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND (1 << 10) mmc_pm_flag_t pm_caps; /* supported pm features */ unsigned int power_notify_type;