On 14 June 2012 18:43, Per Forlin <per.lkml@xxxxxxxxx> wrote:
> Hi Girish and Suagata,
>
> I have run some regression tests with this patch on our board (ux500
> family) running suspend and resume of the eMMC 4.41 device.
>
> The two patches I have looked at are:
> 1. "mmc: core: Fix PowerOff Notify suspend/resume" (merged)
> 2. " MMC-4.5 Power OFF Notify Rework"
>
> With only patch #1 the eMMC doesn't power up after in resume() after
> being suspended. The eMMC doesn't respond at all after suspend. It's
> not powered up.
> Running tests with #1 and #2, the card is powered up but it doesn't
> wake up after CMD5. Commands that arrive are after resume/CMD5
> timeouts. I even tried by increasing the awake timeout to 5 seconds
> but i didn't help.
>
> The eMMC on my board successfully suspends and resumes with patch #1
> and #2 if waking up the card using CMD0 (mmc_card_init()) instead of
> CMD5.
>
> Have anyone else seen the same issue?
> Have this patch been verified on a board together with eMMC 4.41 that
> supports card power off.
This rework patch is still under progress. we are modifying it. In our
earlier discussions subhash has posted the
same issue and a solution for this. we should save ios context before
sleep and restore ios before awake. soon rework patch will be
posted with the above recomenedded solution.
>
> BR
> Per
>
> On Tue, May 22, 2012 at 1:45 PM, Girish K S
> <girish.shivananjappa@xxxxxxxxxx> wrote:
>> From: Saugata Das <saugata.das@xxxxxxxxxx>
>>
>> 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.
>>
>> Earlier implementation of PowerOff Notify as a core function is replaced as
>> a device's bus operation.
>>
>> Signed-off-by: Saugata Das <saugata.das@xxxxxxxxxx>
>> Signed-off-by: Girish K S <girish.shivananjappa@xxxxxxxxxx>
>>
>> changes in v5:
>> modified the the handling of return value in mmc_poweroff_notify.
>> changes in v4:
>> As suggested in review,
>> - Moved mmc_can_poweroff_notify to core.c
>> - Moved mmc_claim_host, mmc_release_host outside mmc_poweroff_notify
>> - Added check for wrong initialization for poweroff_notify_type
>> - mmc_poweroff_notify is modified to take as 2nd parameter
>> changes in v3:
>> This version addresses the review comments given by Subhash and Ulf
>> changes in v2:
>> This version addresses the changes suggested by Ulf
>> ---
>> drivers/mmc/core/core.c | 108 ++++++++++++++++++--------------------------
>> drivers/mmc/core/core.h | 1 +
>> drivers/mmc/core/mmc.c | 56 ++++++++++++++++++++---
>> drivers/mmc/host/dw_mmc.c | 5 --
>> drivers/mmc/host/sdhci.c | 9 ----
>> include/linux/mmc/card.h | 5 +-
>> include/linux/mmc/core.h | 1 +
>> include/linux/mmc/host.h | 5 +--
>> include/linux/mmc/mmc.h | 7 +++
>> 9 files changed, 104 insertions(+), 93 deletions(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 0b6141d..fe616b9 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -1101,48 +1101,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.
>> @@ -1202,8 +1160,6 @@ static void mmc_power_up(struct mmc_host *host)
>>
>> void mmc_power_off(struct mmc_host *host)
>> {
>> - int err = 0;
>> -
>> if (host->ios.power_mode == MMC_POWER_OFF)
>> return;
>>
>> @@ -1212,22 +1168,6 @@ void mmc_power_off(struct mmc_host *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
>> @@ -1726,6 +1666,15 @@ int mmc_can_secure_erase_trim(struct mmc_card *card)
>> }
>> EXPORT_SYMBOL(mmc_can_secure_erase_trim);
>>
>> +int mmc_can_poweroff_notify(const struct mmc_card *card)
>> +{
>> + return card &&
>> + mmc_card_mmc(card) &&
>> + card->host->bus_ops->poweroff_notify &&
>> + (card->poweroff_notify_state == MMC_POWERED_ON);
>> +}
>> +EXPORT_SYMBOL(mmc_can_poweroff_notify);
>> +
>> int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from,
>> unsigned int nr)
>> {
>> @@ -2096,6 +2045,15 @@ void mmc_stop_host(struct mmc_host *host)
>>
>> mmc_bus_get(host);
>> if (host->bus_ops && !host->bus_dead) {
>> + mmc_claim_host(host);
>> + if (mmc_can_poweroff_notify(host->card)) {
>> + int err = host->bus_ops->poweroff_notify(host,
>> + MMC_PW_OFF_NOTIFY_LONG);
>> + if (err)
>> + pr_info("%s: error [%d] in poweroff notify\n",
>> + mmc_hostname(host), err);
>> + }
>> + mmc_release_host(host);
>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>> if (host->bus_ops->remove)
>> host->bus_ops->remove(host);
>> @@ -2131,6 +2089,15 @@ int mmc_power_save_host(struct mmc_host *host)
>>
>> if (host->bus_ops->power_save)
>> ret = host->bus_ops->power_save(host);
>> + mmc_claim_host(host);
>> + if (mmc_can_poweroff_notify(host->card)) {
>> + int err = host->bus_ops->poweroff_notify(host,
>> + MMC_PW_OFF_NOTIFY_SHORT);
>> + if (err)
>> + pr_info("%s: error [%d] in poweroff notify\n",
>> + mmc_hostname(host), err);
>> + }
>> + mmc_release_host(host);
>>
>> mmc_bus_put(host);
>>
>> @@ -2173,8 +2140,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);
>>
>> @@ -2191,8 +2161,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);
>>
>> @@ -2385,12 +2358,20 @@ int mmc_pm_notify(struct notifier_block *notify_block,
>>
>> spin_lock_irqsave(&host->lock, flags);
>> host->rescan_disable = 1;
>> - host->power_notify_type = MMC_HOST_PW_NOTIFY_SHORT;
>> spin_unlock_irqrestore(&host->lock, flags);
>> cancel_delayed_work_sync(&host->detect);
>>
>> if (!host->bus_ops || host->bus_ops->suspend)
>> break;
>> + mmc_claim_host(host);
>> + if (mmc_can_poweroff_notify(host->card)) {
>> + int err = host->bus_ops->poweroff_notify(host,
>> + MMC_PW_OFF_NOTIFY_SHORT);
>> + if (err)
>> + pr_info("%s: error [%d] in poweroff notify\n",
>> + mmc_hostname(host), err);
>> + }
>> + mmc_release_host(host);
>>
>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>> if (host->bus_ops->remove)
>> @@ -2409,7 +2390,6 @@ int mmc_pm_notify(struct notifier_block *notify_block,
>>
>> spin_lock_irqsave(&host->lock, flags);
>> host->rescan_disable = 0;
>> - host->power_notify_type = MMC_HOST_PW_NOTIFY_LONG;
>> spin_unlock_irqrestore(&host->lock, flags);
>> mmc_detect_change(host, 0);
>>
>> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
>> index 3bdafbc..15b918d 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 *, int notify);
>> };
>>
>> 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 2f0e11c..042dbdc 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1259,6 +1259,40 @@ err:
>> return err;
>> }
>>
>> +static int mmc_poweroff_notify(struct mmc_host *host, int notify)
>> +{
>> + struct mmc_card *card;
>> + unsigned int timeout;
>> + unsigned int notify_type = EXT_CSD_NO_POWER_NOTIFICATION;
>> + int err;
>> +
>> + card = host->card;
>> +
>> + if (notify == MMC_PW_OFF_NOTIFY_SHORT) {
>> + notify_type = EXT_CSD_POWER_OFF_SHORT;
>> + timeout = card->ext_csd.generic_cmd6_time;
>> + } else if (notify == MMC_PW_OFF_NOTIFY_LONG) {
>> + notify_type = EXT_CSD_POWER_OFF_LONG;
>> + timeout = card->ext_csd.power_off_longtime;
>> + } else {
>> + pr_info("%s: mmc_poweroff_notify called "
>> + "with notify type %d\n", mmc_hostname(host), notify);
>> + return -EINVAL;
>> + }
>> +
>> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> + EXT_CSD_POWER_OFF_NOTIFICATION,
>> + notify_type, timeout);
>> +
>> + if (err)
>> + pr_err("%s: Device failed to respond within %d "
>> + "poweroff timeout.\n", mmc_hostname(host), timeout);
>> + else
>> + card->poweroff_notify_state =
>> + MMC_NO_POWER_NOTIFICATION;
>> +
>> + return err;
>> +}
>> /*
>> * Host is being removed. Free up the current card.
>> */
>> @@ -1319,13 +1353,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 (!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);
>> + if (mmc_can_poweroff_notify(host->card) &&
>> + (host->caps2 & MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND)) {
>> + err = mmc_poweroff_notify(host, MMC_PW_OFF_NOTIFY_SHORT);
>> + } else {
>> + if (mmc_card_can_sleep(host))
>> + err = mmc_card_sleep(host);
>> + else if (!mmc_host_is_spi(host))
>> + mmc_deselect_cards(host);
>> + }
>> + if (!err)
>> + host->card->state &=
>> + ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200);
>> mmc_release_host(host);
>>
>> return err;
>> @@ -1347,7 +1386,6 @@ static int mmc_resume(struct mmc_host *host)
>> mmc_claim_host(host);
>> if (mmc_card_is_sleep(host->card)) {
>> err = mmc_card_awake(host);
>> - mmc_card_clr_sleep(host->card);
>> } else
>> err = mmc_init_card(host, host->ocr, host->card);
>> mmc_release_host(host);
>> @@ -1407,6 +1445,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 = {
>> @@ -1418,6 +1457,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/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 1532357..463130f 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1788,11 +1788,6 @@ static int __init dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>> if (host->pdata->quirks & DW_MCI_QUIRK_HIGHSPEED)
>> mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
>>
>> - if (mmc->caps2 & MMC_CAP2_POWEROFF_NOTIFY)
>> - mmc->power_notify_type = MMC_HOST_PW_NOTIFY_SHORT;
>> - else
>> - mmc->power_notify_type = MMC_HOST_PW_NOTIFY_NONE;
>> -
>> if (host->pdata->blk_settings) {
>> mmc->max_segs = host->pdata->blk_settings->max_segs;
>> mmc->max_blk_size = host->pdata->blk_settings->max_blk_size;
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index e626732..c0a5a91 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -2812,15 +2812,6 @@ int sdhci_add_host(struct sdhci_host *host)
>> if (caps[1] & SDHCI_DRIVER_TYPE_D)
>> mmc->caps |= MMC_CAP_DRIVER_TYPE_D;
>>
>> - /*
>> - * If Power Off Notify capability is enabled by the host,
>> - * set notify to short power off notify timeout value.
>> - */
>> - if (mmc->caps2 & MMC_CAP2_POWEROFF_NOTIFY)
>> - mmc->power_notify_type = MMC_HOST_PW_NOTIFY_SHORT;
>> - else
>> - mmc->power_notify_type = MMC_HOST_PW_NOTIFY_NONE;
>> -
>> /* Initial value for re-tuning timer count */
>> host->tuning_count = (caps[1] & SDHCI_RETUNING_TIMER_COUNT_MASK) >>
>> SDHCI_RETUNING_TIMER_COUNT_SHIFT;
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> index d76513b..040eec4 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -239,11 +239,10 @@ struct mmc_card {
>> #define MMC_QUIRK_BROKEN_BYTE_MODE_512 (1<<8) /* Avoid sending 512 bytes in */
>> #define MMC_QUIRK_LONG_READ_TIME (1<<9) /* Data read time > CSD says */
>> /* byte mode */
>> - unsigned int poweroff_notify_state; /* eMMC4.5 notify feature */
>> + unsigned int poweroff_notify_state; /* MMC-4.5 poweroff
>> + notify feature */
>> #define MMC_NO_POWER_NOTIFICATION 0
>> #define MMC_POWERED_ON 1
>> -#define MMC_POWEROFF_SHORT 2
>> -#define MMC_POWEROFF_LONG 3
>>
>> unsigned int erase_size; /* erase size in sectors */
>> unsigned int erase_shift; /* if erase unit is power 2 */
>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
>> index 1b431c7..54894d6 100644
>> --- a/include/linux/mmc/core.h
>> +++ b/include/linux/mmc/core.h
>> @@ -161,6 +161,7 @@ extern int mmc_can_trim(struct mmc_card *card);
>> extern int mmc_can_discard(struct mmc_card *card);
>> extern int mmc_can_sanitize(struct mmc_card *card);
>> extern int mmc_can_secure_erase_trim(struct mmc_card *card);
>> +extern int mmc_can_poweroff_notify(const struct mmc_card *card);
>> extern int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from,
>> unsigned int nr);
>> extern unsigned int mmc_calc_max_discard(struct mmc_card *card);
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index 0707d22..0e9adac 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -238,12 +238,9 @@ 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;
>> -#define MMC_HOST_PW_NOTIFY_NONE 0
>> -#define MMC_HOST_PW_NOTIFY_SHORT 1
>> -#define MMC_HOST_PW_NOTIFY_LONG 2
>>
>> #ifdef CONFIG_MMC_CLKGATE
>> int clk_requests; /* internal reference counter */
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index d425cab..b11876b 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -386,4 +386,11 @@ struct _mmc_csd {
>> #define MMC_SWITCH_MODE_CLEAR_BITS 0x02 /* Clear bits which are 1 in value */
>> #define MMC_SWITCH_MODE_WRITE_BYTE 0x03 /* Set target to value */
>>
>> +/*
>> + * MMC Poweroff Notify types
>> + */
>> +#define MMC_PW_OFF_NOTIFY_NONE 0
>> +#define MMC_PW_OFF_NOTIFY_SHORT 1
>> +#define MMC_PW_OFF_NOTIFY_LONG 2
>> +
>> #endif /* LINUX_MMC_MMC_H */
>> --
>> 1.7.4.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
