| Message ID | 1335766496-15966-1-git-send-email-girish.shivananjappa@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 04/30/2012 08:14 AM, 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). > > This patch sets POWER_OFF_NOTIFICATION to POWER_OFF_SHORT if it is sent from > the suspend sequence. If it is sent from shutdown sequence then it is set to > POWER_OFF_LONG. > > Previuos implementation of PowerOff Notify as a core function is replaced as > a device's bus operation. > > Signed-off-by: Saugata Das<saugata.das@linaro.org> > Signed-off-by: Girish K S<girish.shivananjappa@linaro.org> > --- > drivers/mmc/core/core.c | 95 +++++++++++++++------------------------------ > drivers/mmc/core/core.h | 1 + > drivers/mmc/core/mmc.c | 76 +++++++++++++++++++++++++++++++------ > include/linux/mmc/host.h | 1 + > 4 files changed, 98 insertions(+), 75 deletions(-) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index ba821fe..7f5461e 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -1100,48 +1100,6 @@ 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) > -{ > - struct mmc_card *card; > - unsigned int timeout; > - unsigned int notify_type = EXT_CSD_NO_POWER_NOTIFICATION; > - int err = 0; > - > - card = host->card; > - mmc_claim_host(host); > - > - /* > - * Send power notify command only if card > - * is mmc and notify state is powered ON > - */ > - if (card&& mmc_card_mmc(card)&& > - (card->poweroff_notify_state == MMC_POWERED_ON)) { > - > - if (host->power_notify_type == MMC_HOST_PW_NOTIFY_SHORT) { > - notify_type = EXT_CSD_POWER_OFF_SHORT; > - timeout = card->ext_csd.generic_cmd6_time; > - card->poweroff_notify_state = MMC_POWEROFF_SHORT; > - } else { > - notify_type = EXT_CSD_POWER_OFF_LONG; > - timeout = card->ext_csd.power_off_longtime; > - card->poweroff_notify_state = MMC_POWEROFF_LONG; > - } > - > - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, > - EXT_CSD_POWER_OFF_NOTIFICATION, > - notify_type, timeout); > - > - if (err&& err != -EBADMSG) > - pr_err("Device failed to respond within %d poweroff " > - "time. Forcefully powering down the device\n", > - timeout); > - > - /* Set the card state to no notification after the poweroff */ > - card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; > - } > - mmc_release_host(host); > -} > - > /* > * Apply power to the MMC stack. This is a two-stage process. > * First, we enable power to the card without the clock running. > @@ -1198,30 +1156,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. > */ > @@ -2084,6 +2024,14 @@ void mmc_stop_host(struct mmc_host *host) > > mmc_bus_get(host); > if (host->bus_ops&& !host->bus_dead) { > + > + if (host->bus_ops->poweroff_notify) { > + int err = host->bus_ops->poweroff_notify(host); > + if (err) > + pr_info("%s: error [%x] in poweroff notify\n", > + mmc_hostname(host), err); > + } > + > /* Calling bus_ops->remove() with a claimed host can deadlock */ > if (host->bus_ops->remove) > host->bus_ops->remove(host); > @@ -2093,6 +2041,7 @@ void mmc_stop_host(struct mmc_host *host) > mmc_power_off(host); > mmc_release_host(host); > mmc_bus_put(host); > + > return; > } > mmc_bus_put(host); > @@ -2119,6 +2068,13 @@ 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; > + if (host->bus_ops->poweroff_notify) { > + int err = host->bus_ops->poweroff_notify(host); > + if (err) > + pr_info("%s: error [%x] in poweroff notify\n", > + mmc_hostname(host), err); > + } > > mmc_bus_put(host); > > @@ -2142,7 +2098,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); > > @@ -2161,8 +2117,11 @@ int mmc_card_awake(struct mmc_host *host) > > mmc_bus_get(host); > > - if (host->bus_ops&& !host->bus_dead&& host->bus_ops->awake) > + 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); > > @@ -2179,8 +2138,11 @@ int mmc_card_sleep(struct mmc_host *host) > > mmc_bus_get(host); > > - if (host->bus_ops&& !host->bus_dead&& host->bus_ops->sleep) > + 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); > > @@ -2379,6 +2341,12 @@ int mmc_pm_notify(struct notifier_block *notify_block, > > if (!host->bus_ops || host->bus_ops->suspend) > break; > + if (host->bus_ops->poweroff_notify) { > + int err = host->bus_ops->poweroff_notify(host); > + if (err) > + pr_info("%s: error [%x] in poweroff notify\n", > + mmc_hostname(host), err); > + } > > /* Calling bus_ops->remove() with a claimed host can deadlock */ > if (host->bus_ops->remove) > @@ -2388,6 +2356,7 @@ int mmc_pm_notify(struct notifier_block *notify_block, > mmc_detach_bus(host); > mmc_power_off(host); > mmc_release_host(host); > + > host->pm_flags = 0; > break; > > diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h > index 3bdafbc..351cbbe 100644 > --- a/drivers/mmc/core/core.h > +++ b/drivers/mmc/core/core.h > @@ -25,6 +25,7 @@ struct mmc_bus_ops { > int (*power_save)(struct mmc_host *); > int (*power_restore)(struct mmc_host *); > int (*alive)(struct mmc_host *); > + int (*poweroff_notify)(struct mmc_host *); > }; > > void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops); > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index 54df5ad..11ab536 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1282,6 +1282,50 @@ err: > return err; > } > > +static 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 = -EOPNOTSUPP; > + > + card = host->card; > + mmc_claim_host(host); > + > + /* > + * Send power notify command only if card > + * is mmc and notify state is powered ON > + */ > + if (card&& mmc_card_mmc(card)&& > + (card->poweroff_notify_state == MMC_POWERED_ON)) { > + > + if (host->power_notify_type == MMC_HOST_PW_NOTIFY_SHORT) { > + notify_type = EXT_CSD_POWER_OFF_SHORT; > + timeout = card->ext_csd.generic_cmd6_time; > + card->poweroff_notify_state = MMC_POWEROFF_SHORT; > + } else { > + notify_type = EXT_CSD_POWER_OFF_LONG; > + timeout = card->ext_csd.power_off_longtime; > + card->poweroff_notify_state = MMC_POWEROFF_LONG; > + } > + > + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, > + EXT_CSD_POWER_OFF_NOTIFICATION, > + notify_type, timeout); > + > + if (err&& err != -EBADMSG) > + pr_err("%s: Device failed to respond within %d " > + "poweroff time. Forcefully powering down " > + "the device\n", mmc_hostname(host), timeout); > + > + /* Set the card state to no notification after the poweroff */ > + card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; > + } > + mmc_release_host(host); > + > + return err; > +} > + > /* > * Host is being removed. Free up the current card. > */ > @@ -1341,15 +1385,21 @@ static int mmc_suspend(struct mmc_host *host) > BUG_ON(!host); > BUG_ON(!host->card); > > - mmc_claim_host(host); > - if (mmc_card_can_sleep(host)) { > - err = mmc_card_sleep(host); > - if (!err) > - mmc_card_set_sleep(host->card); > - } else if (!mmc_host_is_spi(host)) > - mmc_deselect_cards(host); > - host->card->state&= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); > - mmc_release_host(host); > + if ((host->card)&& mmc_card_mmc(host->card)&& > + (host->caps2& MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND)) { > + err = mmc_poweroff_notify(host); This still wont work. What if the card supports sleep but not poweroff_notify. At the same time the host is able to cut VCCQ. Then the poweroff_notify will be issued and fail. Instead sleep should be issued. Again, a "mmc_card_can_poweroff_notify" would be useful here. > + } else { > + mmc_claim_host(host); > + if (mmc_card_can_sleep(host)) > + err = mmc_card_sleep(host); > + else if (!mmc_host_is_spi(host)) > + mmc_deselect_cards(host); > + mmc_release_host(host); > + } > + > + if (!err) > + host->card->state&= > + ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); > > return err; > } > @@ -1368,11 +1418,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; > @@ -1430,6 +1480,7 @@ static const struct mmc_bus_ops mmc_ops = { > .resume = NULL, > .power_restore = mmc_power_restore, > .alive = mmc_alive, > + .poweroff_notify = mmc_poweroff_notify, > }; > > static const struct mmc_bus_ops mmc_ops_unsafe = { > @@ -1441,6 +1492,7 @@ static const struct mmc_bus_ops mmc_ops_unsafe = { > .resume = mmc_resume, > .power_restore = mmc_power_restore, > .alive = mmc_alive, > + .poweroff_notify = mmc_poweroff_notify, > }; > > static void mmc_attach_bus_ops(struct mmc_host *host) > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h > index 0707d22..986e22b 100644 > --- a/include/linux/mmc/host.h > +++ b/include/linux/mmc/host.h > @@ -238,6 +238,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/30/2012 11:44 AM, 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. What the specification mandates? Does it expect the host to turn off both VCC and VCCQ rails after power off notification (short/long)? What if we send the power off notification (short/long) but still don't turn off either VCCQ or VCC or both? > > 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). > > This patch sets POWER_OFF_NOTIFICATION to POWER_OFF_SHORT if it is sent from > the suspend sequence. If it is sent from shutdown sequence then it is set to > POWER_OFF_LONG. > > Previuos implementation of PowerOff Notify as a core function is replaced as > a device's bus operation. > > Signed-off-by: Saugata Das<saugata.das@linaro.org> > Signed-off-by: Girish K S<girish.shivananjappa@linaro.org> > --- > drivers/mmc/core/core.c | 95 +++++++++++++++------------------------------ > drivers/mmc/core/core.h | 1 + > drivers/mmc/core/mmc.c | 76 +++++++++++++++++++++++++++++++------ > include/linux/mmc/host.h | 1 + > 4 files changed, 98 insertions(+), 75 deletions(-) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index ba821fe..7f5461e 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -1100,48 +1100,6 @@ 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) > -{ > - struct mmc_card *card; > - unsigned int timeout; > - unsigned int notify_type = EXT_CSD_NO_POWER_NOTIFICATION; > - int err = 0; > - > - card = host->card; > - mmc_claim_host(host); > - > - /* > - * Send power notify command only if card > - * is mmc and notify state is powered ON > - */ > - if (card&& mmc_card_mmc(card)&& > - (card->poweroff_notify_state == MMC_POWERED_ON)) { > - > - if (host->power_notify_type == MMC_HOST_PW_NOTIFY_SHORT) { > - notify_type = EXT_CSD_POWER_OFF_SHORT; > - timeout = card->ext_csd.generic_cmd6_time; > - card->poweroff_notify_state = MMC_POWEROFF_SHORT; > - } else { > - notify_type = EXT_CSD_POWER_OFF_LONG; > - timeout = card->ext_csd.power_off_longtime; > - card->poweroff_notify_state = MMC_POWEROFF_LONG; > - } > - > - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, > - EXT_CSD_POWER_OFF_NOTIFICATION, > - notify_type, timeout); > - > - if (err&& err != -EBADMSG) > - pr_err("Device failed to respond within %d poweroff " > - "time. Forcefully powering down the device\n", > - timeout); > - > - /* Set the card state to no notification after the poweroff */ > - card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; > - } > - mmc_release_host(host); > -} > - > /* > * Apply power to the MMC stack. This is a two-stage process. > * First, we enable power to the card without the clock running. > @@ -1198,30 +1156,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. > */ > @@ -2084,6 +2024,14 @@ void mmc_stop_host(struct mmc_host *host) > > mmc_bus_get(host); > if (host->bus_ops&& !host->bus_dead) { > + > + if (host->bus_ops->poweroff_notify) { > + int err = host->bus_ops->poweroff_notify(host); > + if (err) > + pr_info("%s: error [%x] in poweroff notify\n", > + mmc_hostname(host), err); > + } > + > /* Calling bus_ops->remove() with a claimed host can deadlock */ > if (host->bus_ops->remove) > host->bus_ops->remove(host); > @@ -2093,6 +2041,7 @@ void mmc_stop_host(struct mmc_host *host) > mmc_power_off(host); > mmc_release_host(host); > mmc_bus_put(host); > + > return; > } > mmc_bus_put(host); > @@ -2119,6 +2068,13 @@ 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; > + if (host->bus_ops->poweroff_notify) { > + int err = host->bus_ops->poweroff_notify(host); > + if (err) > + pr_info("%s: error [%x] in poweroff notify\n", > + mmc_hostname(host), err); > + } > > mmc_bus_put(host); > > @@ -2142,7 +2098,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; why are we setting the notify_type to NOTIFY_LONG here? Isn't this a resume? > mmc_power_up(host); > ret = host->bus_ops->power_restore(host); > > @@ -2161,8 +2117,11 @@ int mmc_card_awake(struct mmc_host *host) > > mmc_bus_get(host); > > - if (host->bus_ops&& !host->bus_dead&& host->bus_ops->awake) > + 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); > > @@ -2179,8 +2138,11 @@ int mmc_card_sleep(struct mmc_host *host) > > mmc_bus_get(host); > > - if (host->bus_ops&& !host->bus_dead&& host->bus_ops->sleep) > + 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); > > @@ -2379,6 +2341,12 @@ int mmc_pm_notify(struct notifier_block *notify_block, > > if (!host->bus_ops || host->bus_ops->suspend) > break; > + if (host->bus_ops->poweroff_notify) { > + int err = host->bus_ops->poweroff_notify(host); > + if (err) > + pr_info("%s: error [%x] in poweroff notify\n", > + mmc_hostname(host), err); err is printed in hex format? shouldn't it be %d ? Also can we move this message printing with in poweroff_notify() function before returning from it? > + } > > /* Calling bus_ops->remove() with a claimed host can deadlock */ > if (host->bus_ops->remove) > @@ -2388,6 +2356,7 @@ int mmc_pm_notify(struct notifier_block *notify_block, > mmc_detach_bus(host); > mmc_power_off(host); > mmc_release_host(host); > + why is this extra line space added? > host->pm_flags = 0; > break; > > diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h > index 3bdafbc..351cbbe 100644 > --- a/drivers/mmc/core/core.h > +++ b/drivers/mmc/core/core.h > @@ -25,6 +25,7 @@ struct mmc_bus_ops { > int (*power_save)(struct mmc_host *); > int (*power_restore)(struct mmc_host *); > int (*alive)(struct mmc_host *); > + int (*poweroff_notify)(struct mmc_host *); > }; > > void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops); > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index 54df5ad..11ab536 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1282,6 +1282,50 @@ err: > return err; > } > > +static 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 = -EOPNOTSUPP; > + > + card = host->card; > + mmc_claim_host(host); Shouldn't we really claim_host() when we are about to send command to card? Currently claim_host() is called even if card=NULL or poweroff_notify_state is already short/long. Actually if possible we should force the caller of this function to check if it's MMC card or not and they should only call this function if it's MMC card. Basically if this function is called with card=NULL or card_type!=MMC, this function should not return an error because caller of this function is printing pr_info() message in case of an error. > + > + /* > + * Send power notify command only if card > + * is mmc and notify state is powered ON > + */ > + if (card&& mmc_card_mmc(card)&& > + (card->poweroff_notify_state == MMC_POWERED_ON)) { > + > + if (host->power_notify_type == MMC_HOST_PW_NOTIFY_SHORT) { shouldn't the name should be "poweroff_notify_type" to be consisten with other variables like "poweroff_notify_state"? even MMC_HOST_PW_NOTIFY_SHORT should be named as MMC_HOST_PW_OFF_NOTIFY_SHORT? > + notify_type = EXT_CSD_POWER_OFF_SHORT; > + timeout = card->ext_csd.generic_cmd6_time; > + card->poweroff_notify_state = MMC_POWEROFF_SHORT; > + } else { > + notify_type = EXT_CSD_POWER_OFF_LONG; > + timeout = card->ext_csd.power_off_longtime; > + card->poweroff_notify_state = MMC_POWEROFF_LONG;. I don't see need of these 2 extra states (MMC_POWEROFF_SHORT and MMC_POWEROFF_SHORT). MMC_POWERED_ON and MMC_NO_POWER_NOTIFICATION should be enough. Basically if state is MMC_POWERED_ON, we can send the short/long power off notification. And if state is MMC_NO_POWER_NOTIFICATION, we can't send the short/long power off notification. > + } > + > + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, > + EXT_CSD_POWER_OFF_NOTIFICATION, > + notify_type, timeout); > + > + if (err&& err != -EBADMSG) > + pr_err("%s: Device failed to respond within %d " > + "poweroff time. Forcefully powering down " > + "the device\n", mmc_hostname(host), timeout); > + > + /* Set the card state to no notification after the poweroff */ > + card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; poweroff_notify_state should be set to MMC_NO_POWER_NOTIFICATION only if mmc_switch() command is successful or else it's state should remain MMC_POWERED_ON. Also, Shouldn't we set the poweroff_notify_state to MMC_NO_POWER_NOTIFICTION in mmc_init_card() if mmc_switch() returns error? > + } > + mmc_release_host(host); > + > + return err; > +} > + > /* > * Host is being removed. Free up the current card. > */ > @@ -1341,15 +1385,21 @@ static int mmc_suspend(struct mmc_host *host) > BUG_ON(!host); > BUG_ON(!host->card); > > - mmc_claim_host(host); > - if (mmc_card_can_sleep(host)) { > - err = mmc_card_sleep(host); > - if (!err) > - mmc_card_set_sleep(host->card); > - } else if (!mmc_host_is_spi(host)) > - mmc_deselect_cards(host); > - host->card->state&= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); > - mmc_release_host(host); > + if ((host->card)&& mmc_card_mmc(host->card)&& > + (host->caps2& MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND)) { > + err = mmc_poweroff_notify(host); can't we invoke the power_off_notify bus ops instead of calling mmc_poweroff_notify() directly ? > + } else { > + mmc_claim_host(host); > + if (mmc_card_can_sleep(host)) > + err = mmc_card_sleep(host); > + else if (!mmc_host_is_spi(host)) > + mmc_deselect_cards(host); > + mmc_release_host(host); > + } > + > + if (!err) > + host->card->state&= > + ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); > > return err; > } > @@ -1368,11 +1418,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; > @@ -1430,6 +1480,7 @@ static const struct mmc_bus_ops mmc_ops = { > .resume = NULL, > .power_restore = mmc_power_restore, > .alive = mmc_alive, > + .poweroff_notify = mmc_poweroff_notify, > }; > > static const struct mmc_bus_ops mmc_ops_unsafe = { > @@ -1441,6 +1492,7 @@ static const struct mmc_bus_ops mmc_ops_unsafe = { > .resume = mmc_resume, > .power_restore = mmc_power_restore, > .alive = mmc_alive, > + .poweroff_notify = mmc_poweroff_notify, > }; > > static void mmc_attach_bus_ops(struct mmc_host *host) > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h > index 0707d22..986e22b 100644 > --- a/include/linux/mmc/host.h > +++ b/include/linux/mmc/host.h > @@ -238,6 +238,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;
On 30 April 2012 14:09, Ulf Hansson <ulf.hansson@stericsson.com> wrote: > On 04/30/2012 08:14 AM, 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). >> >> This patch sets POWER_OFF_NOTIFICATION to POWER_OFF_SHORT if it is sent >> from >> the suspend sequence. If it is sent from shutdown sequence then it is set >> to >> POWER_OFF_LONG. >> >> Previuos implementation of PowerOff Notify as a core function is replaced >> as >> a device's bus operation. >> >> Signed-off-by: Saugata Das<saugata.das@linaro.org> >> Signed-off-by: Girish K S<girish.shivananjappa@linaro.org> >> --- >> drivers/mmc/core/core.c | 95 >> +++++++++++++++------------------------------ >> drivers/mmc/core/core.h | 1 + >> drivers/mmc/core/mmc.c | 76 +++++++++++++++++++++++++++++++------ >> include/linux/mmc/host.h | 1 + >> 4 files changed, 98 insertions(+), 75 deletions(-) >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> index ba821fe..7f5461e 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -1100,48 +1100,6 @@ 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) >> -{ >> - struct mmc_card *card; >> - unsigned int timeout; >> - unsigned int notify_type = EXT_CSD_NO_POWER_NOTIFICATION; >> - int err = 0; >> - >> - card = host->card; >> - mmc_claim_host(host); >> - >> - /* >> - * Send power notify command only if card >> - * is mmc and notify state is powered ON >> - */ >> - if (card&& mmc_card_mmc(card)&& >> - (card->poweroff_notify_state == MMC_POWERED_ON)) { >> - >> - if (host->power_notify_type == MMC_HOST_PW_NOTIFY_SHORT) { >> - notify_type = EXT_CSD_POWER_OFF_SHORT; >> - timeout = card->ext_csd.generic_cmd6_time; >> - card->poweroff_notify_state = MMC_POWEROFF_SHORT; >> - } else { >> - notify_type = EXT_CSD_POWER_OFF_LONG; >> - timeout = card->ext_csd.power_off_longtime; >> - card->poweroff_notify_state = MMC_POWEROFF_LONG; >> - } >> - >> - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >> - EXT_CSD_POWER_OFF_NOTIFICATION, >> - notify_type, timeout); >> - >> - if (err&& err != -EBADMSG) >> >> - pr_err("Device failed to respond within %d >> poweroff " >> - "time. Forcefully powering down the >> device\n", >> - timeout); >> - >> - /* Set the card state to no notification after the >> poweroff */ >> - card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; >> - } >> - mmc_release_host(host); >> -} >> - >> /* >> * Apply power to the MMC stack. This is a two-stage process. >> * First, we enable power to the card without the clock running. >> @@ -1198,30 +1156,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. >> */ >> @@ -2084,6 +2024,14 @@ void mmc_stop_host(struct mmc_host *host) >> >> mmc_bus_get(host); >> if (host->bus_ops&& !host->bus_dead) { >> >> + >> + if (host->bus_ops->poweroff_notify) { >> + int err = host->bus_ops->poweroff_notify(host); >> + if (err) >> + pr_info("%s: error [%x] in poweroff >> notify\n", >> + mmc_hostname(host), err); >> + } >> + >> /* Calling bus_ops->remove() with a claimed host can >> deadlock */ >> if (host->bus_ops->remove) >> host->bus_ops->remove(host); >> @@ -2093,6 +2041,7 @@ void mmc_stop_host(struct mmc_host *host) >> mmc_power_off(host); >> mmc_release_host(host); >> mmc_bus_put(host); >> + >> return; >> } >> mmc_bus_put(host); >> @@ -2119,6 +2068,13 @@ 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; >> + if (host->bus_ops->poweroff_notify) { >> + int err = host->bus_ops->poweroff_notify(host); >> + if (err) >> + pr_info("%s: error [%x] in poweroff notify\n", >> + mmc_hostname(host), err); >> + } >> >> mmc_bus_put(host); >> >> @@ -2142,7 +2098,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); >> >> @@ -2161,8 +2117,11 @@ int mmc_card_awake(struct mmc_host *host) >> >> mmc_bus_get(host); >> >> - if (host->bus_ops&& !host->bus_dead&& host->bus_ops->awake) >> + 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); >> >> @@ -2179,8 +2138,11 @@ int mmc_card_sleep(struct mmc_host *host) >> >> mmc_bus_get(host); >> >> - if (host->bus_ops&& !host->bus_dead&& host->bus_ops->sleep) >> + 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); >> >> @@ -2379,6 +2341,12 @@ int mmc_pm_notify(struct notifier_block >> *notify_block, >> >> if (!host->bus_ops || host->bus_ops->suspend) >> break; >> + if (host->bus_ops->poweroff_notify) { >> + int err = host->bus_ops->poweroff_notify(host); >> + if (err) >> + pr_info("%s: error [%x] in poweroff >> notify\n", >> + mmc_hostname(host), err); >> + } >> >> /* Calling bus_ops->remove() with a claimed host can >> deadlock */ >> if (host->bus_ops->remove) >> @@ -2388,6 +2356,7 @@ int mmc_pm_notify(struct notifier_block >> *notify_block, >> mmc_detach_bus(host); >> mmc_power_off(host); >> mmc_release_host(host); >> + >> host->pm_flags = 0; >> break; >> >> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h >> index 3bdafbc..351cbbe 100644 >> --- a/drivers/mmc/core/core.h >> +++ b/drivers/mmc/core/core.h >> @@ -25,6 +25,7 @@ struct mmc_bus_ops { >> int (*power_save)(struct mmc_host *); >> int (*power_restore)(struct mmc_host *); >> int (*alive)(struct mmc_host *); >> + int (*poweroff_notify)(struct mmc_host *); >> }; >> >> void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops >> *ops); >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> index 54df5ad..11ab536 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -1282,6 +1282,50 @@ err: >> return err; >> } >> >> +static 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 = -EOPNOTSUPP; >> + >> + card = host->card; >> + mmc_claim_host(host); >> + >> + /* >> + * Send power notify command only if card >> + * is mmc and notify state is powered ON >> + */ >> + if (card&& mmc_card_mmc(card)&& >> + (card->poweroff_notify_state == MMC_POWERED_ON)) { >> + >> + if (host->power_notify_type == MMC_HOST_PW_NOTIFY_SHORT) { >> + notify_type = EXT_CSD_POWER_OFF_SHORT; >> + timeout = card->ext_csd.generic_cmd6_time; >> + card->poweroff_notify_state = MMC_POWEROFF_SHORT; >> + } else { >> + notify_type = EXT_CSD_POWER_OFF_LONG; >> + timeout = card->ext_csd.power_off_longtime; >> + card->poweroff_notify_state = MMC_POWEROFF_LONG; >> + } >> + >> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >> + EXT_CSD_POWER_OFF_NOTIFICATION, >> + notify_type, timeout); >> + >> + if (err&& err != -EBADMSG) >> >> + pr_err("%s: Device failed to respond within %d " >> + "poweroff time. Forcefully powering down " >> + "the device\n", mmc_hostname(host), >> timeout); >> + >> + /* Set the card state to no notification after the >> poweroff */ >> + card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; >> + } >> + mmc_release_host(host); >> + >> + return err; >> +} >> + >> /* >> * Host is being removed. Free up the current card. >> */ >> @@ -1341,15 +1385,21 @@ static int mmc_suspend(struct mmc_host *host) >> BUG_ON(!host); >> BUG_ON(!host->card); >> >> - mmc_claim_host(host); >> - if (mmc_card_can_sleep(host)) { >> - err = mmc_card_sleep(host); >> - if (!err) >> - mmc_card_set_sleep(host->card); >> - } else if (!mmc_host_is_spi(host)) >> - mmc_deselect_cards(host); >> - host->card->state&= ~(MMC_STATE_HIGHSPEED | >> MMC_STATE_HIGHSPEED_200); >> >> - mmc_release_host(host); >> + if ((host->card)&& mmc_card_mmc(host->card)&& >> + (host->caps2& MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND)) { >> + err = mmc_poweroff_notify(host); > > This still wont work. What if the card supports sleep but not > poweroff_notify. At the same time the host is able to cut VCCQ. > Then the poweroff_notify will be issued and fail. Instead sleep should be > issued. > > Again, a "mmc_card_can_poweroff_notify" would be useful here. > The host needs to set MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND only if it is MMC-4.5 card (power Off notify is mandatory feature) and host can switch off VCCQ during suspend. > >> + } else { >> + mmc_claim_host(host); >> + if (mmc_card_can_sleep(host)) >> + err = mmc_card_sleep(host); >> + else if (!mmc_host_is_spi(host)) >> + mmc_deselect_cards(host); >> + mmc_release_host(host); >> + } >> + >> + if (!err) >> + host->card->state&= >> + ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); >> >> return err; >> } >> @@ -1368,11 +1418,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; >> @@ -1430,6 +1480,7 @@ static const struct mmc_bus_ops mmc_ops = { >> .resume = NULL, >> .power_restore = mmc_power_restore, >> .alive = mmc_alive, >> + .poweroff_notify = mmc_poweroff_notify, >> }; >> >> static const struct mmc_bus_ops mmc_ops_unsafe = { >> @@ -1441,6 +1492,7 @@ static const struct mmc_bus_ops mmc_ops_unsafe = { >> .resume = mmc_resume, >> .power_restore = mmc_power_restore, >> .alive = mmc_alive, >> + .poweroff_notify = mmc_poweroff_notify, >> }; >> >> static void mmc_attach_bus_ops(struct mmc_host *host) >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >> index 0707d22..986e22b 100644 >> --- a/include/linux/mmc/host.h >> +++ b/include/linux/mmc/host.h >> @@ -238,6 +238,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 30 April 2012 18:39, Subhash Jadavani <subhashj@codeaurora.org> wrote: > On 4/30/2012 11:44 AM, 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. > > What the specification mandates? Does it expect the host to turn off both > VCC and VCCQ rails after power off notification (short/long)? What if we > send the power off notification (short/long) but still don't turn off either > VCCQ or VCC or both? > The specification does not say that the power OFF notify has to be followed by switching off Vcc and Vccq. We however reinitialize eMMC before using it again. >> >> 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). >> >> This patch sets POWER_OFF_NOTIFICATION to POWER_OFF_SHORT if it is sent >> from >> the suspend sequence. If it is sent from shutdown sequence then it is set >> to >> POWER_OFF_LONG. >> >> Previuos implementation of PowerOff Notify as a core function is replaced >> as >> a device's bus operation. >> >> Signed-off-by: Saugata Das<saugata.das@linaro.org> >> Signed-off-by: Girish K S<girish.shivananjappa@linaro.org> >> --- >> drivers/mmc/core/core.c | 95 >> +++++++++++++++------------------------------ >> drivers/mmc/core/core.h | 1 + >> drivers/mmc/core/mmc.c | 76 +++++++++++++++++++++++++++++++------ >> include/linux/mmc/host.h | 1 + >> 4 files changed, 98 insertions(+), 75 deletions(-) >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> index ba821fe..7f5461e 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -1100,48 +1100,6 @@ 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) >> -{ >> - struct mmc_card *card; >> - unsigned int timeout; >> - unsigned int notify_type = EXT_CSD_NO_POWER_NOTIFICATION; >> - int err = 0; >> - >> - card = host->card; >> - mmc_claim_host(host); >> - >> - /* >> - * Send power notify command only if card >> - * is mmc and notify state is powered ON >> - */ >> - if (card&& mmc_card_mmc(card)&& >> - (card->poweroff_notify_state == MMC_POWERED_ON)) { >> - >> - if (host->power_notify_type == MMC_HOST_PW_NOTIFY_SHORT) { >> - notify_type = EXT_CSD_POWER_OFF_SHORT; >> - timeout = card->ext_csd.generic_cmd6_time; >> - card->poweroff_notify_state = MMC_POWEROFF_SHORT; >> - } else { >> - notify_type = EXT_CSD_POWER_OFF_LONG; >> - timeout = card->ext_csd.power_off_longtime; >> - card->poweroff_notify_state = MMC_POWEROFF_LONG; >> - } >> - >> - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >> - EXT_CSD_POWER_OFF_NOTIFICATION, >> - notify_type, timeout); >> - >> - if (err&& err != -EBADMSG) >> >> - pr_err("Device failed to respond within %d >> poweroff " >> - "time. Forcefully powering down the >> device\n", >> - timeout); >> - >> - /* Set the card state to no notification after the >> poweroff */ >> - card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; >> - } >> - mmc_release_host(host); >> -} >> - >> /* >> * Apply power to the MMC stack. This is a two-stage process. >> * First, we enable power to the card without the clock running. >> @@ -1198,30 +1156,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. >> */ >> @@ -2084,6 +2024,14 @@ void mmc_stop_host(struct mmc_host *host) >> >> mmc_bus_get(host); >> if (host->bus_ops&& !host->bus_dead) { >> >> + >> + if (host->bus_ops->poweroff_notify) { >> + int err = host->bus_ops->poweroff_notify(host); >> + if (err) >> + pr_info("%s: error [%x] in poweroff >> notify\n", >> + mmc_hostname(host), err); >> + } >> + >> /* Calling bus_ops->remove() with a claimed host can >> deadlock */ >> if (host->bus_ops->remove) >> host->bus_ops->remove(host); >> @@ -2093,6 +2041,7 @@ void mmc_stop_host(struct mmc_host *host) >> mmc_power_off(host); >> mmc_release_host(host); >> mmc_bus_put(host); >> + >> return; >> } >> mmc_bus_put(host); >> @@ -2119,6 +2068,13 @@ 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; >> + if (host->bus_ops->poweroff_notify) { >> + int err = host->bus_ops->poweroff_notify(host); >> + if (err) >> + pr_info("%s: error [%x] in poweroff notify\n", >> + mmc_hostname(host), err); >> + } >> >> mmc_bus_put(host); >> >> @@ -2142,7 +2098,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; > > why are we setting the notify_type to NOTIFY_LONG here? Isn't this a resume? >> >> mmc_power_up(host); >> ret = host->bus_ops->power_restore(host); >> >> @@ -2161,8 +2117,11 @@ int mmc_card_awake(struct mmc_host *host) >> >> mmc_bus_get(host); >> >> - if (host->bus_ops&& !host->bus_dead&& host->bus_ops->awake) >> + 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); >> >> @@ -2179,8 +2138,11 @@ int mmc_card_sleep(struct mmc_host *host) >> >> mmc_bus_get(host); >> >> - if (host->bus_ops&& !host->bus_dead&& host->bus_ops->sleep) >> + 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); >> >> @@ -2379,6 +2341,12 @@ int mmc_pm_notify(struct notifier_block >> *notify_block, >> >> if (!host->bus_ops || host->bus_ops->suspend) >> break; >> + if (host->bus_ops->poweroff_notify) { >> + int err = host->bus_ops->poweroff_notify(host); >> + if (err) >> + pr_info("%s: error [%x] in poweroff >> notify\n", >> + mmc_hostname(host), err); > > err is printed in hex format? shouldn't it be %d ? Also can we move this > message printing with in poweroff_notify() function before returning from > it? > >> + } >> >> /* Calling bus_ops->remove() with a claimed host can >> deadlock */ >> if (host->bus_ops->remove) >> @@ -2388,6 +2356,7 @@ int mmc_pm_notify(struct notifier_block >> *notify_block, >> mmc_detach_bus(host); >> mmc_power_off(host); >> mmc_release_host(host); >> + > > why is this extra line space added? > >> host->pm_flags = 0; >> break; >> >> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h >> index 3bdafbc..351cbbe 100644 >> --- a/drivers/mmc/core/core.h >> +++ b/drivers/mmc/core/core.h >> @@ -25,6 +25,7 @@ struct mmc_bus_ops { >> int (*power_save)(struct mmc_host *); >> int (*power_restore)(struct mmc_host *); >> int (*alive)(struct mmc_host *); >> + int (*poweroff_notify)(struct mmc_host *); >> }; >> >> void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops >> *ops); >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> index 54df5ad..11ab536 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -1282,6 +1282,50 @@ err: >> return err; >> } >> >> +static 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 = -EOPNOTSUPP; >> + >> + card = host->card; >> + mmc_claim_host(host); > > Shouldn't we really claim_host() when we are about to send command to card? > Currently claim_host() is called even if card=NULL or poweroff_notify_state > is already short/long. > Actually if possible we should force the caller of this function to check if > it's MMC card or not and they should only call this function if it's MMC > card. > > Basically if this function is called with card=NULL or card_type!=MMC, this > function should not return an error because caller of this function is > printing pr_info() message in case of an error. > > >> + >> + /* >> + * Send power notify command only if card >> + * is mmc and notify state is powered ON >> + */ >> + if (card&& mmc_card_mmc(card)&& >> + (card->poweroff_notify_state == MMC_POWERED_ON)) { >> + >> + if (host->power_notify_type == MMC_HOST_PW_NOTIFY_SHORT) { > > shouldn't the name should be "poweroff_notify_type" to be consisten with > other variables like "poweroff_notify_state"? even MMC_HOST_PW_NOTIFY_SHORT > should be named as MMC_HOST_PW_OFF_NOTIFY_SHORT? >> >> + notify_type = EXT_CSD_POWER_OFF_SHORT; >> + timeout = card->ext_csd.generic_cmd6_time; >> + card->poweroff_notify_state = MMC_POWEROFF_SHORT; >> + } else { >> + notify_type = EXT_CSD_POWER_OFF_LONG; >> + timeout = card->ext_csd.power_off_longtime; >> + card->poweroff_notify_state = MMC_POWEROFF_LONG;. > > I don't see need of these 2 extra states (MMC_POWEROFF_SHORT and > MMC_POWEROFF_SHORT). > MMC_POWERED_ON and MMC_NO_POWER_NOTIFICATION should be enough. > > Basically if state is MMC_POWERED_ON, we can send the short/long power off > notification. And if state is MMC_NO_POWER_NOTIFICATION, we can't send the > short/long power off notification. >> >> + } >> + >> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >> + EXT_CSD_POWER_OFF_NOTIFICATION, >> + notify_type, timeout); >> + >> + if (err&& err != -EBADMSG) >> >> + pr_err("%s: Device failed to respond within %d " >> + "poweroff time. Forcefully powering down " >> + "the device\n", mmc_hostname(host), >> timeout); >> + >> + /* Set the card state to no notification after the >> poweroff */ >> + card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; > > poweroff_notify_state should be set to MMC_NO_POWER_NOTIFICATION only if > mmc_switch() command is successful or else it's state should remain > MMC_POWERED_ON. > Also, Shouldn't we set the poweroff_notify_state to MMC_NO_POWER_NOTIFICTION > in mmc_init_card() if mmc_switch() returns error? >> >> + } >> + mmc_release_host(host); >> + >> + return err; >> +} >> + >> /* >> * Host is being removed. Free up the current card. >> */ >> @@ -1341,15 +1385,21 @@ static int mmc_suspend(struct mmc_host *host) >> BUG_ON(!host); >> BUG_ON(!host->card); >> >> - mmc_claim_host(host); >> - if (mmc_card_can_sleep(host)) { >> - err = mmc_card_sleep(host); >> - if (!err) >> - mmc_card_set_sleep(host->card); >> - } else if (!mmc_host_is_spi(host)) >> - mmc_deselect_cards(host); >> - host->card->state&= ~(MMC_STATE_HIGHSPEED | >> MMC_STATE_HIGHSPEED_200); >> - mmc_release_host(host); >> >> + if ((host->card)&& mmc_card_mmc(host->card)&& >> + (host->caps2& MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND)) { >> + err = mmc_poweroff_notify(host); > > can't we invoke the power_off_notify bus ops instead of calling > mmc_poweroff_notify() directly ? > >> + } else { >> + mmc_claim_host(host); >> + if (mmc_card_can_sleep(host)) >> + err = mmc_card_sleep(host); >> + else if (!mmc_host_is_spi(host)) >> + mmc_deselect_cards(host); >> + mmc_release_host(host); >> + } >> + >> + if (!err) >> + host->card->state&= >> + ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); >> >> return err; >> } >> @@ -1368,11 +1418,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; >> @@ -1430,6 +1480,7 @@ static const struct mmc_bus_ops mmc_ops = { >> .resume = NULL, >> .power_restore = mmc_power_restore, >> .alive = mmc_alive, >> + .poweroff_notify = mmc_poweroff_notify, >> }; >> >> static const struct mmc_bus_ops mmc_ops_unsafe = { >> @@ -1441,6 +1492,7 @@ static const struct mmc_bus_ops mmc_ops_unsafe = { >> .resume = mmc_resume, >> .power_restore = mmc_power_restore, >> .alive = mmc_alive, >> + .poweroff_notify = mmc_poweroff_notify, >> }; >> >> static void mmc_attach_bus_ops(struct mmc_host *host) >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >> index 0707d22..986e22b 100644 >> --- a/include/linux/mmc/host.h >> +++ b/include/linux/mmc/host.h >> @@ -238,6 +238,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; > >
> -----Original Message----- > From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc- > owner@vger.kernel.org] On Behalf Of Saugata Das > Sent: Monday, April 30, 2012 11:17 PM > To: Ulf Hansson > Cc: Girish K S; linux-mmc@vger.kernel.org; patches@linaro.org > Subject: Re: [PATCH V2] MMC-4.5 Power OFF Notify rework > > On 30 April 2012 14:09, Ulf Hansson <ulf.hansson@stericsson.com> wrote: > > On 04/30/2012 08:14 AM, 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). > >> > >> This patch sets POWER_OFF_NOTIFICATION to POWER_OFF_SHORT if it is > >> sent from the suspend sequence. If it is sent from shutdown sequence > >> then it is set to POWER_OFF_LONG. > >> > >> Previuos implementation of PowerOff Notify as a core function is > >> replaced as a device's bus operation. > >> > >> Signed-off-by: Saugata Das<saugata.das@linaro.org> > >> Signed-off-by: Girish K S<girish.shivananjappa@linaro.org> > >> --- > >> drivers/mmc/core/core.c | 95 > >> +++++++++++++++------------------------------ > >> drivers/mmc/core/core.h | 1 + > >> drivers/mmc/core/mmc.c | 76 > >> +++++++++++++++++++++++++++++++------ > >> include/linux/mmc/host.h | 1 + > >> 4 files changed, 98 insertions(+), 75 deletions(-) > >> > >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index > >> ba821fe..7f5461e 100644 > >> --- a/drivers/mmc/core/core.c > >> +++ b/drivers/mmc/core/core.c > >> @@ -1100,48 +1100,6 @@ 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) -{ > >> - struct mmc_card *card; > >> - unsigned int timeout; > >> - unsigned int notify_type = EXT_CSD_NO_POWER_NOTIFICATION; > >> - int err = 0; > >> - > >> - card = host->card; > >> - mmc_claim_host(host); > >> - > >> - /* > >> - * Send power notify command only if card > >> - * is mmc and notify state is powered ON > >> - */ > >> - if (card&& mmc_card_mmc(card)&& > >> - (card->poweroff_notify_state == MMC_POWERED_ON)) { > >> - > >> - if (host->power_notify_type == > >> MMC_HOST_PW_NOTIFY_SHORT) { > >> - notify_type = EXT_CSD_POWER_OFF_SHORT; > >> - timeout = card->ext_csd.generic_cmd6_time; > >> - card->poweroff_notify_state = > >> MMC_POWEROFF_SHORT; > >> - } else { > >> - notify_type = EXT_CSD_POWER_OFF_LONG; > >> - timeout = card->ext_csd.power_off_longtime; > >> - card->poweroff_notify_state = > >> MMC_POWEROFF_LONG; > >> - } > >> - > >> - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, > >> - EXT_CSD_POWER_OFF_NOTIFICATION, > >> - notify_type, timeout); > >> - > >> - if (err&& err != -EBADMSG) > >> > >> - pr_err("Device failed to respond within %d > >> poweroff " > >> - "time. Forcefully powering down the > >> device\n", > >> - timeout); > >> - > >> - /* Set the card state to no notification after the > >> poweroff */ > >> - card->poweroff_notify_state = > >> MMC_NO_POWER_NOTIFICATION; > >> - } > >> - mmc_release_host(host); > >> -} > >> - > >> /* > >> * Apply power to the MMC stack. This is a two-stage process. > >> * First, we enable power to the card without the clock running. > >> @@ -1198,30 +1156,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. > >> */ > >> @@ -2084,6 +2024,14 @@ void mmc_stop_host(struct mmc_host *host) > >> > >> mmc_bus_get(host); > >> if (host->bus_ops&& !host->bus_dead) { > >> > >> + > >> + if (host->bus_ops->poweroff_notify) { > >> + int err = > >> + host->bus_ops->poweroff_notify(host); > >> + if (err) > >> + pr_info("%s: error [%x] in poweroff > >> notify\n", > >> + mmc_hostname(host), err); > >> + } > >> + > >> /* Calling bus_ops->remove() with a claimed host can > >> deadlock */ > >> if (host->bus_ops->remove) > >> host->bus_ops->remove(host); @@ -2093,6 > >> +2041,7 @@ void mmc_stop_host(struct mmc_host *host) > >> mmc_power_off(host); > >> mmc_release_host(host); > >> mmc_bus_put(host); > >> + > >> return; > >> } > >> mmc_bus_put(host); > >> @@ -2119,6 +2068,13 @@ 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; > >> + if (host->bus_ops->poweroff_notify) { > >> + int err = host->bus_ops->poweroff_notify(host); > >> + if (err) > >> + pr_info("%s: error [%x] in poweroff > >> + notify\n", > >> + mmc_hostname(host), err); > >> + } > >> > >> mmc_bus_put(host); > >> > >> @@ -2142,7 +2098,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); > >> > >> @@ -2161,8 +2117,11 @@ int mmc_card_awake(struct mmc_host *host) > >> > >> mmc_bus_get(host); > >> > >> - if (host->bus_ops&& !host->bus_dead&& host->bus_ops->awake) > >> + 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); > >> > >> @@ -2179,8 +2138,11 @@ int mmc_card_sleep(struct mmc_host *host) > >> > >> mmc_bus_get(host); > >> > >> - if (host->bus_ops&& !host->bus_dead&& host->bus_ops->sleep) > >> + 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); > >> > >> @@ -2379,6 +2341,12 @@ int mmc_pm_notify(struct notifier_block > >> *notify_block, > >> > >> if (!host->bus_ops || host->bus_ops->suspend) > >> break; > >> + if (host->bus_ops->poweroff_notify) { > >> + int err = > >> + host->bus_ops->poweroff_notify(host); > >> + if (err) > >> + pr_info("%s: error [%x] in poweroff > >> notify\n", > >> + mmc_hostname(host), err); > >> + } > >> > >> /* Calling bus_ops->remove() with a claimed host can > >> deadlock */ > >> if (host->bus_ops->remove) @@ -2388,6 +2356,7 @@ int > >> mmc_pm_notify(struct notifier_block *notify_block, > >> mmc_detach_bus(host); > >> mmc_power_off(host); > >> mmc_release_host(host); > >> + > >> host->pm_flags = 0; > >> break; > >> > >> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index > >> 3bdafbc..351cbbe 100644 > >> --- a/drivers/mmc/core/core.h > >> +++ b/drivers/mmc/core/core.h > >> @@ -25,6 +25,7 @@ struct mmc_bus_ops { > >> int (*power_save)(struct mmc_host *); > >> int (*power_restore)(struct mmc_host *); > >> int (*alive)(struct mmc_host *); > >> + int (*poweroff_notify)(struct mmc_host *); > >> }; > >> > >> void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops > >> *ops); diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > >> index 54df5ad..11ab536 100644 > >> --- a/drivers/mmc/core/mmc.c > >> +++ b/drivers/mmc/core/mmc.c > >> @@ -1282,6 +1282,50 @@ err: > >> return err; > >> } > >> > >> +static 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 = -EOPNOTSUPP; > >> + > >> + card = host->card; > >> + mmc_claim_host(host); > >> + > >> + /* > >> + * Send power notify command only if card > >> + * is mmc and notify state is powered ON > >> + */ > >> + if (card&& mmc_card_mmc(card)&& > >> + (card->poweroff_notify_state == MMC_POWERED_ON)) { > >> + > >> + if (host->power_notify_type == > >> + MMC_HOST_PW_NOTIFY_SHORT) { > >> + notify_type = EXT_CSD_POWER_OFF_SHORT; > >> + timeout = card->ext_csd.generic_cmd6_time; > >> + card->poweroff_notify_state = > >> + MMC_POWEROFF_SHORT; > >> + } else { > >> + notify_type = EXT_CSD_POWER_OFF_LONG; > >> + timeout = card->ext_csd.power_off_longtime; > >> + card->poweroff_notify_state = > >> + MMC_POWEROFF_LONG; > >> + } > >> + > >> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, > >> + EXT_CSD_POWER_OFF_NOTIFICATION, > >> + notify_type, timeout); > >> + > >> + if (err&& err != -EBADMSG) > >> > >> + pr_err("%s: Device failed to respond within %d " > >> + "poweroff time. Forcefully powering down " > >> + "the device\n", mmc_hostname(host), > >> timeout); > >> + > >> + /* Set the card state to no notification after the > >> poweroff */ > >> + card->poweroff_notify_state = > >> + MMC_NO_POWER_NOTIFICATION; > >> + } > >> + mmc_release_host(host); > >> + > >> + return err; > >> +} > >> + > >> /* > >> * Host is being removed. Free up the current card. > >> */ > >> @@ -1341,15 +1385,21 @@ static int mmc_suspend(struct mmc_host > *host) > >> BUG_ON(!host); > >> BUG_ON(!host->card); > >> > >> - mmc_claim_host(host); > >> - if (mmc_card_can_sleep(host)) { > >> - err = mmc_card_sleep(host); > >> - if (!err) > >> - mmc_card_set_sleep(host->card); > >> - } else if (!mmc_host_is_spi(host)) > >> - mmc_deselect_cards(host); > >> - host->card->state&= ~(MMC_STATE_HIGHSPEED | > >> MMC_STATE_HIGHSPEED_200); > >> > >> - mmc_release_host(host); > >> + if ((host->card)&& mmc_card_mmc(host->card)&& > >> + (host->caps2& > >> + MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND)) { > >> + err = mmc_poweroff_notify(host); > > > > This still wont work. What if the card supports sleep but not > > poweroff_notify. At the same time the host is able to cut VCCQ. > > Then the poweroff_notify will be issued and fail. Instead sleep should > > be issued. > > > > Again, a "mmc_card_can_poweroff_notify" would be useful here. > > > > The host needs to set MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND > only if it is MMC-4.5 card (power Off notify is mandatory feature) and host can > switch off VCCQ during suspend. How would host driver know whether it's eMMC 4.5 card or not? It doesn't know that. Host driver just needs to set this cap during probe and then based on which card is detected, core layer will check this patch before sending the power off notify during suspend. > > > > > >> + } else { > >> + mmc_claim_host(host); > >> + if (mmc_card_can_sleep(host)) > >> + err = mmc_card_sleep(host); > >> + else if (!mmc_host_is_spi(host)) > >> + mmc_deselect_cards(host); > >> + mmc_release_host(host); > >> + } > >> + > >> + if (!err) > >> + host->card->state&= > >> + ~(MMC_STATE_HIGHSPEED | > >> + MMC_STATE_HIGHSPEED_200); > >> > >> return err; > >> } > >> @@ -1368,11 +1418,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; > >> @@ -1430,6 +1480,7 @@ static const struct mmc_bus_ops mmc_ops = { > >> .resume = NULL, > >> .power_restore = mmc_power_restore, > >> .alive = mmc_alive, > >> + .poweroff_notify = mmc_poweroff_notify, > >> }; > >> > >> static const struct mmc_bus_ops mmc_ops_unsafe = { @@ -1441,6 > >> +1492,7 @@ static const struct mmc_bus_ops mmc_ops_unsafe = { > >> .resume = mmc_resume, > >> .power_restore = mmc_power_restore, > >> .alive = mmc_alive, > >> + .poweroff_notify = mmc_poweroff_notify, > >> }; > >> > >> static void mmc_attach_bus_ops(struct mmc_host *host) diff --git > >> a/include/linux/mmc/host.h b/include/linux/mmc/host.h index > >> 0707d22..986e22b 100644 > >> --- a/include/linux/mmc/host.h > >> +++ b/include/linux/mmc/host.h > >> @@ -238,6 +238,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 > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body > of a message to majordomo@vger.kernel.org More majordomo info at > http://vger.kernel.org/majordomo-info.html
On 30 April 2012 18:39, Subhash Jadavani <subhashj@codeaurora.org> wrote: > On 4/30/2012 11:44 AM, 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. > > What the specification mandates? Does it expect the host to turn off both > VCC and VCCQ rails after power off notification (short/long)? What if we > send the power off notification (short/long) but still don't turn off either > VCCQ or VCC or both? > >> >> 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). >> >> This patch sets POWER_OFF_NOTIFICATION to POWER_OFF_SHORT if it is sent >> from >> the suspend sequence. If it is sent from shutdown sequence then it is set >> to >> POWER_OFF_LONG. >> >> Previuos implementation of PowerOff Notify as a core function is replaced >> as >> a device's bus operation. >> >> Signed-off-by: Saugata Das<saugata.das@linaro.org> >> Signed-off-by: Girish K S<girish.shivananjappa@linaro.org> >> --- >> drivers/mmc/core/core.c | 95 >> +++++++++++++++------------------------------ >> drivers/mmc/core/core.h | 1 + >> drivers/mmc/core/mmc.c | 76 +++++++++++++++++++++++++++++++------ >> include/linux/mmc/host.h | 1 + >> 4 files changed, 98 insertions(+), 75 deletions(-) >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> index ba821fe..7f5461e 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -1100,48 +1100,6 @@ 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) >> -{ >> - struct mmc_card *card; >> - unsigned int timeout; >> - unsigned int notify_type = EXT_CSD_NO_POWER_NOTIFICATION; >> - int err = 0; >> - >> - card = host->card; >> - mmc_claim_host(host); >> - >> - /* >> - * Send power notify command only if card >> - * is mmc and notify state is powered ON >> - */ >> - if (card&& mmc_card_mmc(card)&& >> - (card->poweroff_notify_state == MMC_POWERED_ON)) { >> - >> - if (host->power_notify_type == MMC_HOST_PW_NOTIFY_SHORT) { >> - notify_type = EXT_CSD_POWER_OFF_SHORT; >> - timeout = card->ext_csd.generic_cmd6_time; >> - card->poweroff_notify_state = MMC_POWEROFF_SHORT; >> - } else { >> - notify_type = EXT_CSD_POWER_OFF_LONG; >> - timeout = card->ext_csd.power_off_longtime; >> - card->poweroff_notify_state = MMC_POWEROFF_LONG; >> - } >> - >> - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >> - EXT_CSD_POWER_OFF_NOTIFICATION, >> - notify_type, timeout); >> - >> - if (err&& err != -EBADMSG) >> >> - pr_err("Device failed to respond within %d >> poweroff " >> - "time. Forcefully powering down the >> device\n", >> - timeout); >> - >> - /* Set the card state to no notification after the >> poweroff */ >> - card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; >> - } >> - mmc_release_host(host); >> -} >> - >> /* >> * Apply power to the MMC stack. This is a two-stage process. >> * First, we enable power to the card without the clock running. >> @@ -1198,30 +1156,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. >> */ >> @@ -2084,6 +2024,14 @@ void mmc_stop_host(struct mmc_host *host) >> >> mmc_bus_get(host); >> if (host->bus_ops&& !host->bus_dead) { >> >> + >> + if (host->bus_ops->poweroff_notify) { >> + int err = host->bus_ops->poweroff_notify(host); >> + if (err) >> + pr_info("%s: error [%x] in poweroff >> notify\n", >> + mmc_hostname(host), err); >> + } >> + >> /* Calling bus_ops->remove() with a claimed host can >> deadlock */ >> if (host->bus_ops->remove) >> host->bus_ops->remove(host); >> @@ -2093,6 +2041,7 @@ void mmc_stop_host(struct mmc_host *host) >> mmc_power_off(host); >> mmc_release_host(host); >> mmc_bus_put(host); >> + >> return; >> } >> mmc_bus_put(host); >> @@ -2119,6 +2068,13 @@ 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; >> + if (host->bus_ops->poweroff_notify) { >> + int err = host->bus_ops->poweroff_notify(host); >> + if (err) >> + pr_info("%s: error [%x] in poweroff notify\n", >> + mmc_hostname(host), err); >> + } >> >> mmc_bus_put(host); >> >> @@ -2142,7 +2098,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; > > why are we setting the notify_type to NOTIFY_LONG here? Isn't this a resume? I agree. I will remove it in all resume paths >> >> mmc_power_up(host); >> ret = host->bus_ops->power_restore(host); >> >> @@ -2161,8 +2117,11 @@ int mmc_card_awake(struct mmc_host *host) >> >> mmc_bus_get(host); >> >> - if (host->bus_ops&& !host->bus_dead&& host->bus_ops->awake) >> + 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); >> >> @@ -2179,8 +2138,11 @@ int mmc_card_sleep(struct mmc_host *host) >> >> mmc_bus_get(host); >> >> - if (host->bus_ops&& !host->bus_dead&& host->bus_ops->sleep) >> + 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); >> >> @@ -2379,6 +2341,12 @@ int mmc_pm_notify(struct notifier_block >> *notify_block, >> >> if (!host->bus_ops || host->bus_ops->suspend) >> break; >> + if (host->bus_ops->poweroff_notify) { >> + int err = host->bus_ops->poweroff_notify(host); >> + if (err) >> + pr_info("%s: error [%x] in poweroff >> notify\n", >> + mmc_hostname(host), err); > > err is printed in hex format? shouldn't it be %d ? Also can we move this will change it > message printing with in poweroff_notify() function before returning from > it? Here the err value is used only to print the info. > >> + } >> >> /* Calling bus_ops->remove() with a claimed host can >> deadlock */ >> if (host->bus_ops->remove) >> @@ -2388,6 +2356,7 @@ int mmc_pm_notify(struct notifier_block >> *notify_block, >> mmc_detach_bus(host); >> mmc_power_off(host); >> mmc_release_host(host); >> + > > why is this extra line space added? will delete > >> host->pm_flags = 0; >> break; >> >> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h >> index 3bdafbc..351cbbe 100644 >> --- a/drivers/mmc/core/core.h >> +++ b/drivers/mmc/core/core.h >> @@ -25,6 +25,7 @@ struct mmc_bus_ops { >> int (*power_save)(struct mmc_host *); >> int (*power_restore)(struct mmc_host *); >> int (*alive)(struct mmc_host *); >> + int (*poweroff_notify)(struct mmc_host *); >> }; >> >> void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops >> *ops); >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> index 54df5ad..11ab536 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -1282,6 +1282,50 @@ err: >> return err; >> } >> >> +static 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 = -EOPNOTSUPP; >> + >> + card = host->card; >> + mmc_claim_host(host); > > Shouldn't we really claim_host() when we are about to send command to card? > Currently claim_host() is called even if card=NULL or poweroff_notify_state > is already short/long. The poweroff_notify_state has to be initialized to one of the states before entering this function. I will move the claim code before sending the command > Actually if possible we should force the caller of this function to check if > it's MMC card or not and they should only call this function if it's MMC > card. Not necessary. As the condition is already done in this function. Instead of using it in multiple places (caller function), current place of conditional check is better > > Basically if this function is called with card=NULL or card_type!=MMC, this > function should not return an error because caller of this function is > printing pr_info() message in case of an error. I will check this again > > >> + >> + /* >> + * Send power notify command only if card >> + * is mmc and notify state is powered ON >> + */ >> + if (card&& mmc_card_mmc(card)&& >> + (card->poweroff_notify_state == MMC_POWERED_ON)) { >> + >> + if (host->power_notify_type == MMC_HOST_PW_NOTIFY_SHORT) { > > shouldn't the name should be "poweroff_notify_type" to be consisten with > other variables like "poweroff_notify_state"? even MMC_HOST_PW_NOTIFY_SHORT > should be named as MMC_HOST_PW_OFF_NOTIFY_SHORT? will change it for consistency >> >> + notify_type = EXT_CSD_POWER_OFF_SHORT; >> + timeout = card->ext_csd.generic_cmd6_time; >> + card->poweroff_notify_state = MMC_POWEROFF_SHORT; >> + } else { >> + notify_type = EXT_CSD_POWER_OFF_LONG; >> + timeout = card->ext_csd.power_off_longtime; >> + card->poweroff_notify_state = MMC_POWEROFF_LONG;. > > I don't see need of these 2 extra states (MMC_POWEROFF_SHORT and > MMC_POWEROFF_SHORT). > MMC_POWERED_ON and MMC_NO_POWER_NOTIFICATION should be enough. > > Basically if state is MMC_POWERED_ON, we can send the short/long power off > notification. And if state is MMC_NO_POWER_NOTIFICATION, we can't send the > short/long power off notification. Thanks for this one. >> >> + } >> + >> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >> + EXT_CSD_POWER_OFF_NOTIFICATION, >> + notify_type, timeout); >> + >> + if (err&& err != -EBADMSG) >> >> + pr_err("%s: Device failed to respond within %d " >> + "poweroff time. Forcefully powering down " >> + "the device\n", mmc_hostname(host), >> timeout); >> + >> + /* Set the card state to no notification after the >> poweroff */ >> + card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; > > poweroff_notify_state should be set to MMC_NO_POWER_NOTIFICATION only if > mmc_switch() command is successful or else it's state should remain > MMC_POWERED_ON. > Also, Shouldn't we set the poweroff_notify_state to MMC_NO_POWER_NOTIFICTION > in mmc_init_card() if mmc_switch() returns error? Will verify with the specification again. >> >> + } >> + mmc_release_host(host); >> + >> + return err; >> +} >> + >> /* >> * Host is being removed. Free up the current card. >> */ >> @@ -1341,15 +1385,21 @@ static int mmc_suspend(struct mmc_host *host) >> BUG_ON(!host); >> BUG_ON(!host->card); >> >> - mmc_claim_host(host); >> - if (mmc_card_can_sleep(host)) { >> - err = mmc_card_sleep(host); >> - if (!err) >> - mmc_card_set_sleep(host->card); >> - } else if (!mmc_host_is_spi(host)) >> - mmc_deselect_cards(host); >> - host->card->state&= ~(MMC_STATE_HIGHSPEED | >> MMC_STATE_HIGHSPEED_200); >> - mmc_release_host(host); >> >> + if ((host->card)&& mmc_card_mmc(host->card)&& >> + (host->caps2& MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND)) { >> + err = mmc_poweroff_notify(host); > > can't we invoke the power_off_notify bus ops instead of calling > mmc_poweroff_notify() directly ? since the function is local to this file it is invoked directly. why local function should be invoked from bus_ops? > >> + } else { >> + mmc_claim_host(host); >> + if (mmc_card_can_sleep(host)) >> + err = mmc_card_sleep(host); >> + else if (!mmc_host_is_spi(host)) >> + mmc_deselect_cards(host); >> + mmc_release_host(host); >> + } >> + >> + if (!err) >> + host->card->state&= >> + ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); >> >> return err; >> } >> @@ -1368,11 +1418,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; >> @@ -1430,6 +1480,7 @@ static const struct mmc_bus_ops mmc_ops = { >> .resume = NULL, >> .power_restore = mmc_power_restore, >> .alive = mmc_alive, >> + .poweroff_notify = mmc_poweroff_notify, >> }; >> >> static const struct mmc_bus_ops mmc_ops_unsafe = { >> @@ -1441,6 +1492,7 @@ static const struct mmc_bus_ops mmc_ops_unsafe = { >> .resume = mmc_resume, >> .power_restore = mmc_power_restore, >> .alive = mmc_alive, >> + .poweroff_notify = mmc_poweroff_notify, >> }; >> >> static void mmc_attach_bus_ops(struct mmc_host *host) >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >> index 0707d22..986e22b 100644 >> --- a/include/linux/mmc/host.h >> +++ b/include/linux/mmc/host.h >> @@ -238,6 +238,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; > >
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index ba821fe..7f5461e 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1100,48 +1100,6 @@ 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) -{ - struct mmc_card *card; - unsigned int timeout; - unsigned int notify_type = EXT_CSD_NO_POWER_NOTIFICATION; - int err = 0; - - card = host->card; - mmc_claim_host(host); - - /* - * Send power notify command only if card - * is mmc and notify state is powered ON - */ - if (card && mmc_card_mmc(card) && - (card->poweroff_notify_state == MMC_POWERED_ON)) { - - if (host->power_notify_type == MMC_HOST_PW_NOTIFY_SHORT) { - notify_type = EXT_CSD_POWER_OFF_SHORT; - timeout = card->ext_csd.generic_cmd6_time; - card->poweroff_notify_state = MMC_POWEROFF_SHORT; - } else { - notify_type = EXT_CSD_POWER_OFF_LONG; - timeout = card->ext_csd.power_off_longtime; - card->poweroff_notify_state = MMC_POWEROFF_LONG; - } - - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, - EXT_CSD_POWER_OFF_NOTIFICATION, - notify_type, timeout); - - if (err && err != -EBADMSG) - pr_err("Device failed to respond within %d poweroff " - "time. Forcefully powering down the device\n", - timeout); - - /* Set the card state to no notification after the poweroff */ - card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; - } - mmc_release_host(host); -} - /* * Apply power to the MMC stack. This is a two-stage process. * First, we enable power to the card without the clock running. @@ -1198,30 +1156,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. */ @@ -2084,6 +2024,14 @@ void mmc_stop_host(struct mmc_host *host) mmc_bus_get(host); if (host->bus_ops && !host->bus_dead) { + + if (host->bus_ops->poweroff_notify) { + int err = host->bus_ops->poweroff_notify(host); + if (err) + pr_info("%s: error [%x] in poweroff notify\n", + mmc_hostname(host), err); + } + /* Calling bus_ops->remove() with a claimed host can deadlock */ if (host->bus_ops->remove) host->bus_ops->remove(host); @@ -2093,6 +2041,7 @@ void mmc_stop_host(struct mmc_host *host) mmc_power_off(host); mmc_release_host(host); mmc_bus_put(host); + return; } mmc_bus_put(host); @@ -2119,6 +2068,13 @@ 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; + if (host->bus_ops->poweroff_notify) { + int err = host->bus_ops->poweroff_notify(host); + if (err) + pr_info("%s: error [%x] in poweroff notify\n", + mmc_hostname(host), err); + } mmc_bus_put(host); @@ -2142,7 +2098,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); @@ -2161,8 +2117,11 @@ int mmc_card_awake(struct mmc_host *host) mmc_bus_get(host); - if (host->bus_ops && !host->bus_dead && host->bus_ops->awake) + 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); @@ -2179,8 +2138,11 @@ int mmc_card_sleep(struct mmc_host *host) mmc_bus_get(host); - if (host->bus_ops && !host->bus_dead && host->bus_ops->sleep) + 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); @@ -2379,6 +2341,12 @@ int mmc_pm_notify(struct notifier_block *notify_block, if (!host->bus_ops || host->bus_ops->suspend) break; + if (host->bus_ops->poweroff_notify) { + int err = host->bus_ops->poweroff_notify(host); + if (err) + pr_info("%s: error [%x] in poweroff notify\n", + mmc_hostname(host), err); + } /* Calling bus_ops->remove() with a claimed host can deadlock */ if (host->bus_ops->remove) @@ -2388,6 +2356,7 @@ int mmc_pm_notify(struct notifier_block *notify_block, mmc_detach_bus(host); mmc_power_off(host); mmc_release_host(host); + host->pm_flags = 0; break; diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 3bdafbc..351cbbe 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -25,6 +25,7 @@ struct mmc_bus_ops { int (*power_save)(struct mmc_host *); int (*power_restore)(struct mmc_host *); int (*alive)(struct mmc_host *); + int (*poweroff_notify)(struct mmc_host *); }; void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops); diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 54df5ad..11ab536 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1282,6 +1282,50 @@ err: return err; } +static 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 = -EOPNOTSUPP; + + card = host->card; + mmc_claim_host(host); + + /* + * Send power notify command only if card + * is mmc and notify state is powered ON + */ + if (card && mmc_card_mmc(card) && + (card->poweroff_notify_state == MMC_POWERED_ON)) { + + if (host->power_notify_type == MMC_HOST_PW_NOTIFY_SHORT) { + notify_type = EXT_CSD_POWER_OFF_SHORT; + timeout = card->ext_csd.generic_cmd6_time; + card->poweroff_notify_state = MMC_POWEROFF_SHORT; + } else { + notify_type = EXT_CSD_POWER_OFF_LONG; + timeout = card->ext_csd.power_off_longtime; + card->poweroff_notify_state = MMC_POWEROFF_LONG; + } + + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, + EXT_CSD_POWER_OFF_NOTIFICATION, + notify_type, timeout); + + if (err && err != -EBADMSG) + pr_err("%s: Device failed to respond within %d " + "poweroff time. Forcefully powering down " + "the device\n", mmc_hostname(host), timeout); + + /* Set the card state to no notification after the poweroff */ + card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; + } + mmc_release_host(host); + + return err; +} + /* * Host is being removed. Free up the current card. */ @@ -1341,15 +1385,21 @@ static int mmc_suspend(struct mmc_host *host) BUG_ON(!host); BUG_ON(!host->card); - mmc_claim_host(host); - if (mmc_card_can_sleep(host)) { - err = mmc_card_sleep(host); - if (!err) - mmc_card_set_sleep(host->card); - } else if (!mmc_host_is_spi(host)) - mmc_deselect_cards(host); - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); - mmc_release_host(host); + if ((host->card) && mmc_card_mmc(host->card) && + (host->caps2 & MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND)) { + err = mmc_poweroff_notify(host); + } else { + mmc_claim_host(host); + if (mmc_card_can_sleep(host)) + err = mmc_card_sleep(host); + else if (!mmc_host_is_spi(host)) + mmc_deselect_cards(host); + mmc_release_host(host); + } + + if (!err) + host->card->state &= + ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); return err; } @@ -1368,11 +1418,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; @@ -1430,6 +1480,7 @@ static const struct mmc_bus_ops mmc_ops = { .resume = NULL, .power_restore = mmc_power_restore, .alive = mmc_alive, + .poweroff_notify = mmc_poweroff_notify, }; static const struct mmc_bus_ops mmc_ops_unsafe = { @@ -1441,6 +1492,7 @@ static const struct mmc_bus_ops mmc_ops_unsafe = { .resume = mmc_resume, .power_restore = mmc_power_restore, .alive = mmc_alive, + .poweroff_notify = mmc_poweroff_notify, }; static void mmc_attach_bus_ops(struct mmc_host *host) diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 0707d22..986e22b 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -238,6 +238,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;