* [PATCH] MMC-4.5 Power OFF Notify rework
@ 2012-04-19 12:41 Girish K S
2012-04-20 11:33 ` Girish K S
0 siblings, 1 reply; 12+ messages in thread
From: Girish K S @ 2012-04-19 12:41 UTC (permalink / raw)
To: linux-mmc; +Cc: patches, ulf.hansson, saugata.das, girish.shivananjappa
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 | 83 ++++++++++-----------------------------------
drivers/mmc/core/core.h | 1 +
drivers/mmc/core/mmc.c | 75 +++++++++++++++++++++++++++++++++++-------
include/linux/mmc/host.h | 1 +
4 files changed, 84 insertions(+), 76 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index e541efb..4b8b2c1 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,9 @@ 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)
+ host->bus_ops->poweroff_notify(host);
+
/* Calling bus_ops->remove() with a claimed host can deadlock */
if (host->bus_ops->remove)
host->bus_ops->remove(host);
@@ -2093,6 +2036,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,7 +2063,9 @@ 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)
+ host->bus_ops->poweroff_notify(host);
mmc_bus_put(host);
mmc_power_off(host);
@@ -2142,7 +2088,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 +2107,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 +2128,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);
@@ -2394,6 +2346,8 @@ int mmc_pm_notify(struct notifier_block *notify_block,
if (!host->bus_ops || host->bus_ops->suspend)
break;
+ if (host->bus_ops->poweroff_notify)
+ host->bus_ops->poweroff_notify(host);
/* Calling bus_ops->remove() with a claimed host can deadlock */
if (host->bus_ops->remove)
@@ -2403,6 +2357,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..62bdee6 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 = -EINVAL;
+
+ 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,20 @@ 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->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 +1417,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 +1479,7 @@ static const struct mmc_bus_ops mmc_ops = {
.resume = NULL,
.power_restore = mmc_power_restore,
.alive = mmc_alive,
+ .poweroff_notify = NULL,
};
static const struct mmc_bus_ops mmc_ops_unsafe = {
@@ -1441,6 +1491,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 cbde4b7..cf44a6f 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;
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] MMC-4.5 Power OFF Notify rework
2012-04-19 12:41 [PATCH] MMC-4.5 Power OFF Notify rework Girish K S
@ 2012-04-20 11:33 ` Girish K S
2012-04-20 12:55 ` Ulf Hansson
2012-04-23 14:11 ` Ulf Hansson
0 siblings, 2 replies; 12+ messages in thread
From: Girish K S @ 2012-04-20 11:33 UTC (permalink / raw)
To: linux-mmc; +Cc: patches, ulf.hansson, saugata.das, girish.shivananjappa
On 19 April 2012 18:11, Girish K S <girish.shivananjappa@linaro.org> wrote:
> This is a rework of the existing POWER OFF NOTIFY patch. The current problem
> with the patch comes from the ambiguity on the usage of POWER OFF NOTIFY
> together with SLEEP and misunderstanding on the usage of MMC_POWER_OFF
> power_mode from mmc_set_ios in different host controller drivers.
>
> This new patch works around this problem by adding a new host CAP,
> MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND, which when set sends a
> POWER OFF NOTIFY from mmc_suspend instead of SLEEP. It is expected that host
> controller drivers will set this CAP, if they switch off both Vcc and Vccq
> from MMC_POWER_OFF condition within mmc_set_ios. However, note that there
> is no harm in sending MMC_POWER_NOTIFY even if Vccq is not switched off.
>
> This patch also sends POWER OFF NOTIFY from power management routines (e.g.
> mmc_power_save_host, mmc_pm_notify/PM_SUSPEND_PREPARE, mmc_stop_host), which
> does reinitialization of the eMMC on the return path of the power management
> routines (e.g. mmc_power_restore_host, mmc_pm_notify/PM_POST_RESTORE,
> mmc_start_host).
>
> 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 | 83 ++++++++++-----------------------------------
> drivers/mmc/core/core.h | 1 +
> drivers/mmc/core/mmc.c | 75 +++++++++++++++++++++++++++++++++++-------
> include/linux/mmc/host.h | 1 +
> 4 files changed, 84 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index e541efb..4b8b2c1 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,9 @@ 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)
> + host->bus_ops->poweroff_notify(host);
> +
> /* Calling bus_ops->remove() with a claimed host can deadlock */
> if (host->bus_ops->remove)
> host->bus_ops->remove(host);
> @@ -2093,6 +2036,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,7 +2063,9 @@ 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)
> + host->bus_ops->poweroff_notify(host);
> mmc_bus_put(host);
>
> mmc_power_off(host);
> @@ -2142,7 +2088,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 +2107,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 +2128,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);
>
> @@ -2394,6 +2346,8 @@ int mmc_pm_notify(struct notifier_block *notify_block,
>
> if (!host->bus_ops || host->bus_ops->suspend)
> break;
> + if (host->bus_ops->poweroff_notify)
> + host->bus_ops->poweroff_notify(host);
>
> /* Calling bus_ops->remove() with a claimed host can deadlock */
> if (host->bus_ops->remove)
> @@ -2403,6 +2357,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..62bdee6 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 = -EINVAL;
> +
> + 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,20 @@ 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->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 +1417,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 +1479,7 @@ static const struct mmc_bus_ops mmc_ops = {
> .resume = NULL,
> .power_restore = mmc_power_restore,
> .alive = mmc_alive,
> + .poweroff_notify = NULL,
> };
>
> static const struct mmc_bus_ops mmc_ops_unsafe = {
> @@ -1441,6 +1491,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 cbde4b7..cf44a6f 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;
Hi Ulf,
need your comment on the above.
> --
> 1.7.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] MMC-4.5 Power OFF Notify rework
2012-04-20 11:33 ` Girish K S
@ 2012-04-20 12:55 ` Ulf Hansson
2012-04-23 14:11 ` Ulf Hansson
1 sibling, 0 replies; 12+ messages in thread
From: Ulf Hansson @ 2012-04-20 12:55 UTC (permalink / raw)
To: Girish K S; +Cc: linux-mmc, patches, saugata.das
On 04/20/2012 01:33 PM, Girish K S wrote:
> On 19 April 2012 18:11, Girish K S<girish.shivananjappa@linaro.org> wrote:
>> This is a rework of the existing POWER OFF NOTIFY patch. The current problem
>> with the patch comes from the ambiguity on the usage of POWER OFF NOTIFY
>> together with SLEEP and misunderstanding on the usage of MMC_POWER_OFF
>> power_mode from mmc_set_ios in different host controller drivers.
>>
>> This new patch works around this problem by adding a new host CAP,
>> MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND, which when set sends a
>> POWER OFF NOTIFY from mmc_suspend instead of SLEEP. It is expected that host
>> controller drivers will set this CAP, if they switch off both Vcc and Vccq
>> from MMC_POWER_OFF condition within mmc_set_ios. However, note that there
>> is no harm in sending MMC_POWER_NOTIFY even if Vccq is not switched off.
>>
>> This patch also sends POWER OFF NOTIFY from power management routines (e.g.
>> mmc_power_save_host, mmc_pm_notify/PM_SUSPEND_PREPARE, mmc_stop_host), which
>> does reinitialization of the eMMC on the return path of the power management
>> routines (e.g. mmc_power_restore_host, mmc_pm_notify/PM_POST_RESTORE,
>> mmc_start_host).
>>
>> 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 | 83 ++++++++++-----------------------------------
>> drivers/mmc/core/core.h | 1 +
>> drivers/mmc/core/mmc.c | 75 +++++++++++++++++++++++++++++++++++-------
>> include/linux/mmc/host.h | 1 +
>> 4 files changed, 84 insertions(+), 76 deletions(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index e541efb..4b8b2c1 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,9 @@ 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)
>> + host->bus_ops->poweroff_notify(host);
>> +
>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>> if (host->bus_ops->remove)
>> host->bus_ops->remove(host);
>> @@ -2093,6 +2036,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,7 +2063,9 @@ 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)
>> + host->bus_ops->poweroff_notify(host);
>> mmc_bus_put(host);
>>
>> mmc_power_off(host);
>> @@ -2142,7 +2088,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 +2107,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 +2128,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);
>>
>> @@ -2394,6 +2346,8 @@ int mmc_pm_notify(struct notifier_block *notify_block,
>>
>> if (!host->bus_ops || host->bus_ops->suspend)
>> break;
>> + if (host->bus_ops->poweroff_notify)
>> + host->bus_ops->poweroff_notify(host);
>>
>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>> if (host->bus_ops->remove)
>> @@ -2403,6 +2357,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..62bdee6 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 = -EINVAL;
>> +
>> + 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,20 @@ 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->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 +1417,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 +1479,7 @@ static const struct mmc_bus_ops mmc_ops = {
>> .resume = NULL,
>> .power_restore = mmc_power_restore,
>> .alive = mmc_alive,
>> + .poweroff_notify = NULL,
>> };
>>
>> static const struct mmc_bus_ops mmc_ops_unsafe = {
>> @@ -1441,6 +1491,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 cbde4b7..cf44a6f 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;
>
> Hi Ulf,
> need your comment on the above.
>> --
>> 1.7.1
>>
I will definitely look into it beginning of next week. Please be
patient. :-)
Kind regards
Ulf Hansson
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] MMC-4.5 Power OFF Notify rework
2012-04-20 11:33 ` Girish K S
2012-04-20 12:55 ` Ulf Hansson
@ 2012-04-23 14:11 ` Ulf Hansson
2012-04-27 4:40 ` Girish K S
1 sibling, 1 reply; 12+ messages in thread
From: Ulf Hansson @ 2012-04-23 14:11 UTC (permalink / raw)
To: Girish K S; +Cc: linux-mmc, patches, saugata.das
Hi Girish,
Please see some comments below...
On 04/20/2012 01:33 PM, Girish K S wrote:
> On 19 April 2012 18:11, Girish K S<girish.shivananjappa@linaro.org> wrote:
>> This is a rework of the existing POWER OFF NOTIFY patch. The current problem
>> with the patch comes from the ambiguity on the usage of POWER OFF NOTIFY
>> together with SLEEP and misunderstanding on the usage of MMC_POWER_OFF
>> power_mode from mmc_set_ios in different host controller drivers.
>>
>> This new patch works around this problem by adding a new host CAP,
>> MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND, which when set sends a
>> POWER OFF NOTIFY from mmc_suspend instead of SLEEP. It is expected that host
>> controller drivers will set this CAP, if they switch off both Vcc and Vccq
>> from MMC_POWER_OFF condition within mmc_set_ios. However, note that there
>> is no harm in sending MMC_POWER_NOTIFY even if Vccq is not switched off.
>>
>> This patch also sends POWER OFF NOTIFY from power management routines (e.g.
>> mmc_power_save_host, mmc_pm_notify/PM_SUSPEND_PREPARE, mmc_stop_host), which
>> does reinitialization of the eMMC on the return path of the power management
>> routines (e.g. mmc_power_restore_host, mmc_pm_notify/PM_POST_RESTORE,
>> mmc_start_host).
>>
>> 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.
Great, I like this! :-)
>>
>> Signed-off-by: Saugata Das<saugata.das@linaro.org>
>> Signed-off-by: Girish K S<girish.shivananjappa@linaro.org>
>> ---
>> drivers/mmc/core/core.c | 83 ++++++++++-----------------------------------
>> drivers/mmc/core/core.h | 1 +
>> drivers/mmc/core/mmc.c | 75 +++++++++++++++++++++++++++++++++++-------
>> include/linux/mmc/host.h | 1 +
>> 4 files changed, 84 insertions(+), 76 deletions(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index e541efb..4b8b2c1 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,9 @@ 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)
>> + host->bus_ops->poweroff_notify(host);
>> +
>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>> if (host->bus_ops->remove)
>> host->bus_ops->remove(host);
>> @@ -2093,6 +2036,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,7 +2063,9 @@ 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)
>> + host->bus_ops->poweroff_notify(host);
>> mmc_bus_put(host);
>>
>> mmc_power_off(host);
>> @@ -2142,7 +2088,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 +2107,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 +2128,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);
>>
>> @@ -2394,6 +2346,8 @@ int mmc_pm_notify(struct notifier_block *notify_block,
>>
>> if (!host->bus_ops || host->bus_ops->suspend)
>> break;
>> + if (host->bus_ops->poweroff_notify)
>> + host->bus_ops->poweroff_notify(host);
>>
>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>> if (host->bus_ops->remove)
>> @@ -2403,6 +2357,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..62bdee6 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 = -EINVAL;
Should we consider it an error if the card does not support poweroff
notification?
Moreover, this function returns an error code which is good. Although
everywhere you call this function you do not consider the error. Please
look into changing this.
>> +
>> + 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)&&
Some white spaces..
>> + (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)
Some white spaces..
>> + 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,20 @@ static int mmc_suspend(struct mmc_host *host)
>> BUG_ON(!host);
>> BUG_ON(!host->card);
>>
>> - mmc_claim_host(host);
nested claims is OK, so you can keep this if the code becomes easier to
read!? The corresponding mmc_release_host then as well of course...
>> - 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->caps2& MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) {
Some white spaces..
This will not work. If the card does not support "poweroff_notify", but
sleep, then the card will not be put to sleep. The cap should not be
used to determine what the card supports, only the host.
Maybe an new function "mmc_card_can_poweroff_notify" can help out in
taking the right decision here. The you also might be able to get rid of
the host->poweroff_notify_state variable, don't know.
>> + 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 +1417,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 +1479,7 @@ static const struct mmc_bus_ops mmc_ops = {
>> .resume = NULL,
>> .power_restore = mmc_power_restore,
>> .alive = mmc_alive,
>> + .poweroff_notify = NULL,
Is there any problems with adding the function here? Even if an eMMC
always should be considered non-removable, and thus the unsafe bus_ops
should be used. I don't know the reason to why sleep/awake has been
added to this bus_ops, but I think poweroff_notify should be handled
similarly...
>> };
>>
>> static const struct mmc_bus_ops mmc_ops_unsafe = {
>> @@ -1441,6 +1491,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 cbde4b7..cf44a6f 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)
Don't know if we would like to use the old caps(1) bit 7, since
MMC_CAP_DISABLE has been removed and thus this bit is now available...
>>
>> mmc_pm_flag_t pm_caps; /* supported pm features */
>> unsigned int power_notify_type;
>
> Hi Ulf,
> need your comment on the above.
>> --
>> 1.7.1
>>
Overall, I like this patch, we are getting close to a final version I think.
Kind regards
Ulf Hansson
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] MMC-4.5 Power OFF Notify rework
2012-04-23 14:11 ` Ulf Hansson
@ 2012-04-27 4:40 ` Girish K S
2012-04-27 7:40 ` Ulf Hansson
0 siblings, 1 reply; 12+ messages in thread
From: Girish K S @ 2012-04-27 4:40 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, patches, saugata.das
On 23 April 2012 19:41, Ulf Hansson <ulf.hansson@stericsson.com> wrote:
> Hi Girish,
>
> Please see some comments below...
>
>
> On 04/20/2012 01:33 PM, Girish K S wrote:
>>
>> On 19 April 2012 18:11, Girish K S<girish.shivananjappa@linaro.org>
>> wrote:
>>>
>>> This is a rework of the existing POWER OFF NOTIFY patch. The current
>>> problem
>>> with the patch comes from the ambiguity on the usage of POWER OFF NOTIFY
>>> together with SLEEP and misunderstanding on the usage of MMC_POWER_OFF
>>> power_mode from mmc_set_ios in different host controller drivers.
>>>
>>> This new patch works around this problem by adding a new host CAP,
>>> MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND, which when set sends a
>>> POWER OFF NOTIFY from mmc_suspend instead of SLEEP. It is expected that
>>> host
>>> controller drivers will set this CAP, if they switch off both Vcc and
>>> Vccq
>>> from MMC_POWER_OFF condition within mmc_set_ios. However, note that there
>>> is no harm in sending MMC_POWER_NOTIFY even if Vccq is not switched off.
>>>
>>> This patch also sends POWER OFF NOTIFY from power management routines
>>> (e.g.
>>> mmc_power_save_host, mmc_pm_notify/PM_SUSPEND_PREPARE, mmc_stop_host),
>>> which
>>> does reinitialization of the eMMC on the return path of the power
>>> management
>>> routines (e.g. mmc_power_restore_host, mmc_pm_notify/PM_POST_RESTORE,
>>> mmc_start_host).
>>>
>>> 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.
>
> Great, I like this! :-)
>
>>>
>>> Signed-off-by: Saugata Das<saugata.das@linaro.org>
>>> Signed-off-by: Girish K S<girish.shivananjappa@linaro.org>
>>> ---
>>> drivers/mmc/core/core.c | 83
>>> ++++++++++-----------------------------------
>>> drivers/mmc/core/core.h | 1 +
>>> drivers/mmc/core/mmc.c | 75
>>> +++++++++++++++++++++++++++++++++++-------
>>> include/linux/mmc/host.h | 1 +
>>> 4 files changed, 84 insertions(+), 76 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>> index e541efb..4b8b2c1 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,9 @@ 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)
>>> + host->bus_ops->poweroff_notify(host);
>>> +
>>> /* Calling bus_ops->remove() with a claimed host can
>>> deadlock */
>>> if (host->bus_ops->remove)
>>> host->bus_ops->remove(host);
>>> @@ -2093,6 +2036,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,7 +2063,9 @@ 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)
>>> + host->bus_ops->poweroff_notify(host);
>>> mmc_bus_put(host);
>>>
>>> mmc_power_off(host);
>>> @@ -2142,7 +2088,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 +2107,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 +2128,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);
>>>
>>> @@ -2394,6 +2346,8 @@ int mmc_pm_notify(struct notifier_block
>>> *notify_block,
>>>
>>> if (!host->bus_ops || host->bus_ops->suspend)
>>> break;
>>> + if (host->bus_ops->poweroff_notify)
>>> + host->bus_ops->poweroff_notify(host);
>>>
>>> /* Calling bus_ops->remove() with a claimed host can
>>> deadlock */
>>> if (host->bus_ops->remove)
>>> @@ -2403,6 +2357,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..62bdee6 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 = -EINVAL;
>
> Should we consider it an error if the card does not support poweroff
> notification?
To make it more meaningful I will change the error value to -EOPNOTSUPP.
>
> Moreover, this function returns an error code which is good. Although
> everywhere you call this function you do not consider the error. Please look
> into changing this.
Also will consider the return value from the places it is called to
display the info "card doesnt support po notify"
>
>
>>> +
>>> + 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)&&
>
> Some white spaces..
will rum cleanpatch
>
>
>>> + (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)
>
> Some white spaces..
>
>
>>> + 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,20 @@ static int mmc_suspend(struct mmc_host *host)
>>> BUG_ON(!host);
>>> BUG_ON(!host->card);
>>>
>>> - mmc_claim_host(host);
>
> nested claims is OK, so you can keep this if the code becomes easier to
> read!? The corresponding mmc_release_host then as well of course...
>
>>> - 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->caps2& MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) {
>
> Some white spaces..
>
> This will not work. If the card does not support "poweroff_notify", but
> sleep, then the card will not be put to sleep. The cap should not be used to
> determine what the card supports, only the host.
I will remove th host caps option. and directly call the power off
notify function. This function will execute the switch command only if
the card supports the power off notify.
so comparison with host caps can be omitted.
>
> Maybe an new function "mmc_card_can_poweroff_notify" can help out in taking
> the right decision here. The you also might be able to get rid of the
> host->poweroff_notify_state variable, don't know.
not necessary to add a new function as said above the notify function
takes care of card support
>
>
>>> + 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 +1417,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 +1479,7 @@ static const struct mmc_bus_ops mmc_ops = {
>>> .resume = NULL,
>>> .power_restore = mmc_power_restore,
>>> .alive = mmc_alive,
>>> + .poweroff_notify = NULL,
>
> Is there any problems with adding the function here? Even if an eMMC always
> should be considered non-removable, and thus the unsafe bus_ops should be
> used. I don't know the reason to why sleep/awake has been added to this
> bus_ops, but I think poweroff_notify should be handled similarly...
No problem in adding. The only reason for not adding is that none of
the removable cards support poweroff notify. this feature is supported
above 4.5 version. If you have come across any 4.5 removable device,
it would more convinient to add here.
>
>
>>> };
>>>
>>> static const struct mmc_bus_ops mmc_ops_unsafe = {
>>> @@ -1441,6 +1491,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 cbde4b7..cf44a6f 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)
>
> Don't know if we would like to use the old caps(1) bit 7, since
> MMC_CAP_DISABLE has been removed and thus this bit is now available...
I am planning to remove this caps.
>
>
>>>
>>> mmc_pm_flag_t pm_caps; /* supported pm features
>>> */
>>> unsigned int power_notify_type;
>>
>>
>> Hi Ulf,
>> need your comment on the above.
>>>
>>> --
>>> 1.7.1
>>>
>
> Overall, I like this patch, we are getting close to a final version I think.
>
> Kind regards
> Ulf Hansson
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] MMC-4.5 Power OFF Notify rework
2012-04-27 4:40 ` Girish K S
@ 2012-04-27 7:40 ` Ulf Hansson
2012-04-27 8:42 ` Saugata Das
0 siblings, 1 reply; 12+ messages in thread
From: Ulf Hansson @ 2012-04-27 7:40 UTC (permalink / raw)
To: Girish K S; +Cc: linux-mmc, patches, saugata.das
On 04/27/2012 06:40 AM, Girish K S wrote:
> On 23 April 2012 19:41, Ulf Hansson<ulf.hansson@stericsson.com> wrote:
>> Hi Girish,
>>
>> Please see some comments below...
>>
>>
>> On 04/20/2012 01:33 PM, Girish K S wrote:
>>>
>>> On 19 April 2012 18:11, Girish K S<girish.shivananjappa@linaro.org>
>>> wrote:
>>>>
>>>> This is a rework of the existing POWER OFF NOTIFY patch. The current
>>>> problem
>>>> with the patch comes from the ambiguity on the usage of POWER OFF NOTIFY
>>>> together with SLEEP and misunderstanding on the usage of MMC_POWER_OFF
>>>> power_mode from mmc_set_ios in different host controller drivers.
>>>>
>>>> This new patch works around this problem by adding a new host CAP,
>>>> MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND, which when set sends a
>>>> POWER OFF NOTIFY from mmc_suspend instead of SLEEP. It is expected that
>>>> host
>>>> controller drivers will set this CAP, if they switch off both Vcc and
>>>> Vccq
>>>> from MMC_POWER_OFF condition within mmc_set_ios. However, note that there
>>>> is no harm in sending MMC_POWER_NOTIFY even if Vccq is not switched off.
>>>>
>>>> This patch also sends POWER OFF NOTIFY from power management routines
>>>> (e.g.
>>>> mmc_power_save_host, mmc_pm_notify/PM_SUSPEND_PREPARE, mmc_stop_host),
>>>> which
>>>> does reinitialization of the eMMC on the return path of the power
>>>> management
>>>> routines (e.g. mmc_power_restore_host, mmc_pm_notify/PM_POST_RESTORE,
>>>> mmc_start_host).
>>>>
>>>> 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.
>>
>> Great, I like this! :-)
>>
>>>>
>>>> Signed-off-by: Saugata Das<saugata.das@linaro.org>
>>>> Signed-off-by: Girish K S<girish.shivananjappa@linaro.org>
>>>> ---
>>>> drivers/mmc/core/core.c | 83
>>>> ++++++++++-----------------------------------
>>>> drivers/mmc/core/core.h | 1 +
>>>> drivers/mmc/core/mmc.c | 75
>>>> +++++++++++++++++++++++++++++++++++-------
>>>> include/linux/mmc/host.h | 1 +
>>>> 4 files changed, 84 insertions(+), 76 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>> index e541efb..4b8b2c1 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,9 @@ 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)
>>>> + host->bus_ops->poweroff_notify(host);
>>>> +
>>>> /* Calling bus_ops->remove() with a claimed host can
>>>> deadlock */
>>>> if (host->bus_ops->remove)
>>>> host->bus_ops->remove(host);
>>>> @@ -2093,6 +2036,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,7 +2063,9 @@ 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)
>>>> + host->bus_ops->poweroff_notify(host);
>>>> mmc_bus_put(host);
>>>>
>>>> mmc_power_off(host);
>>>> @@ -2142,7 +2088,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 +2107,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 +2128,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);
>>>>
>>>> @@ -2394,6 +2346,8 @@ int mmc_pm_notify(struct notifier_block
>>>> *notify_block,
>>>>
>>>> if (!host->bus_ops || host->bus_ops->suspend)
>>>> break;
>>>> + if (host->bus_ops->poweroff_notify)
>>>> + host->bus_ops->poweroff_notify(host);
>>>>
>>>> /* Calling bus_ops->remove() with a claimed host can
>>>> deadlock */
>>>> if (host->bus_ops->remove)
>>>> @@ -2403,6 +2357,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..62bdee6 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 = -EINVAL;
>>
>> Should we consider it an error if the card does not support poweroff
>> notification?
> To make it more meaningful I will change the error value to -EOPNOTSUPP.
>>
>> Moreover, this function returns an error code which is good. Although
>> everywhere you call this function you do not consider the error. Please look
>> into changing this.
> Also will consider the return value from the places it is called to
> display the info "card doesnt support po notify"
>>
>>
>>>> +
>>>> + 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)&&
>>
>> Some white spaces..
> will rum cleanpatch
>>
>>
>>>> + (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)
>>
>> Some white spaces..
>>
>>
>>>> + 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,20 @@ static int mmc_suspend(struct mmc_host *host)
>>>> BUG_ON(!host);
>>>> BUG_ON(!host->card);
>>>>
>>>> - mmc_claim_host(host);
>>
>> nested claims is OK, so you can keep this if the code becomes easier to
>> read!? The corresponding mmc_release_host then as well of course...
>>
>>>> - 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->caps2& MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) {
>>
>> Some white spaces..
>>
>> This will not work. If the card does not support "poweroff_notify", but
>> sleep, then the card will not be put to sleep. The cap should not be used to
>> determine what the card supports, only the host.
> I will remove th host caps option. and directly call the power off
> notify function. This function will execute the switch command only if
> the card supports the power off notify.
> so comparison with host caps can be omitted.
I don't think removing the host cap i a good option. If the host only
supports removing VCC and not VCCQ, we would like to issue the sleep cmd
in favor of poweroff_notify cmd. Both conditions must thus be checked.
Thus I think a "mmc_card_can_poweroff_notify" could be feasible to use
here for simplifying things.
>>
>> Maybe an new function "mmc_card_can_poweroff_notify" can help out in taking
>> the right decision here. The you also might be able to get rid of the
>> host->poweroff_notify_state variable, don't know.
> not necessary to add a new function as said above the notify function
> takes care of card support
>>
>>
>>>> + 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 +1417,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 +1479,7 @@ static const struct mmc_bus_ops mmc_ops = {
>>>> .resume = NULL,
>>>> .power_restore = mmc_power_restore,
>>>> .alive = mmc_alive,
>>>> + .poweroff_notify = NULL,
>>
>> Is there any problems with adding the function here? Even if an eMMC always
>> should be considered non-removable, and thus the unsafe bus_ops should be
>> used. I don't know the reason to why sleep/awake has been added to this
>> bus_ops, but I think poweroff_notify should be handled similarly...
> No problem in adding. The only reason for not adding is that none of
> the removable cards support poweroff notify. this feature is supported
> above 4.5 version. If you have come across any 4.5 removable device,
> it would more convinient to add here.
For testing I believe 4.5 eMMC in an SD-card adapter can be used. But
just for testing...
>>
>>
>>>> };
>>>>
>>>> static const struct mmc_bus_ops mmc_ops_unsafe = {
>>>> @@ -1441,6 +1491,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 cbde4b7..cf44a6f 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)
>>
>> Don't know if we would like to use the old caps(1) bit 7, since
>> MMC_CAP_DISABLE has been removed and thus this bit is now available...
> I am planning to remove this caps.
>>
>>
>>>>
>>>> mmc_pm_flag_t pm_caps; /* supported pm features
>>>> */
>>>> unsigned int power_notify_type;
>>>
>>>
>>> Hi Ulf,
>>> need your comment on the above.
>>>>
>>>> --
>>>> 1.7.1
>>>>
>>
>> Overall, I like this patch, we are getting close to a final version I think.
>>
>> Kind regards
>> Ulf Hansson
Kind regards
Ulf Hansson
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] MMC-4.5 Power OFF Notify rework
2012-04-27 7:40 ` Ulf Hansson
@ 2012-04-27 8:42 ` Saugata Das
2012-04-27 8:53 ` Girish K S
0 siblings, 1 reply; 12+ messages in thread
From: Saugata Das @ 2012-04-27 8:42 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Girish K S, linux-mmc, patches
On 27 April 2012 13:10, Ulf Hansson <ulf.hansson@stericsson.com> wrote:
> On 04/27/2012 06:40 AM, Girish K S wrote:
>>
>> On 23 April 2012 19:41, Ulf Hansson<ulf.hansson@stericsson.com> wrote:
>>>
>>> Hi Girish,
>>>
>>> Please see some comments below...
>>>
>>>
>>> On 04/20/2012 01:33 PM, Girish K S wrote:
>>>>
>>>>
>>>> On 19 April 2012 18:11, Girish K S<girish.shivananjappa@linaro.org>
>>>> wrote:
>>>>>
>>>>>
>>>>> This is a rework of the existing POWER OFF NOTIFY patch. The current
>>>>> problem
>>>>> with the patch comes from the ambiguity on the usage of POWER OFF
>>>>> NOTIFY
>>>>> together with SLEEP and misunderstanding on the usage of MMC_POWER_OFF
>>>>> power_mode from mmc_set_ios in different host controller drivers.
>>>>>
>>>>> This new patch works around this problem by adding a new host CAP,
>>>>> MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND, which when set sends a
>>>>> POWER OFF NOTIFY from mmc_suspend instead of SLEEP. It is expected that
>>>>> host
>>>>> controller drivers will set this CAP, if they switch off both Vcc and
>>>>> Vccq
>>>>> from MMC_POWER_OFF condition within mmc_set_ios. However, note that
>>>>> there
>>>>> is no harm in sending MMC_POWER_NOTIFY even if Vccq is not switched
>>>>> off.
>>>>>
>>>>> This patch also sends POWER OFF NOTIFY from power management routines
>>>>> (e.g.
>>>>> mmc_power_save_host, mmc_pm_notify/PM_SUSPEND_PREPARE, mmc_stop_host),
>>>>> which
>>>>> does reinitialization of the eMMC on the return path of the power
>>>>> management
>>>>> routines (e.g. mmc_power_restore_host, mmc_pm_notify/PM_POST_RESTORE,
>>>>> mmc_start_host).
>>>>>
>>>>> 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.
>>>
>>>
>>> Great, I like this! :-)
>>>
>>>>>
>>>>> Signed-off-by: Saugata Das<saugata.das@linaro.org>
>>>>> Signed-off-by: Girish K S<girish.shivananjappa@linaro.org>
>>>>> ---
>>>>> drivers/mmc/core/core.c | 83
>>>>> ++++++++++-----------------------------------
>>>>> drivers/mmc/core/core.h | 1 +
>>>>> drivers/mmc/core/mmc.c | 75
>>>>> +++++++++++++++++++++++++++++++++++-------
>>>>> include/linux/mmc/host.h | 1 +
>>>>> 4 files changed, 84 insertions(+), 76 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>> index e541efb..4b8b2c1 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,9 @@ 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)
>>>>> + host->bus_ops->poweroff_notify(host);
>>>>> +
>>>>> /* Calling bus_ops->remove() with a claimed host can
>>>>> deadlock */
>>>>> if (host->bus_ops->remove)
>>>>> host->bus_ops->remove(host);
>>>>> @@ -2093,6 +2036,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,7 +2063,9 @@ 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)
>>>>> + host->bus_ops->poweroff_notify(host);
>>>>> mmc_bus_put(host);
>>>>>
>>>>> mmc_power_off(host);
>>>>> @@ -2142,7 +2088,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 +2107,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 +2128,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);
>>>>>
>>>>> @@ -2394,6 +2346,8 @@ int mmc_pm_notify(struct notifier_block
>>>>> *notify_block,
>>>>>
>>>>> if (!host->bus_ops || host->bus_ops->suspend)
>>>>> break;
>>>>> + if (host->bus_ops->poweroff_notify)
>>>>> + host->bus_ops->poweroff_notify(host);
>>>>>
>>>>> /* Calling bus_ops->remove() with a claimed host can
>>>>> deadlock */
>>>>> if (host->bus_ops->remove)
>>>>> @@ -2403,6 +2357,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..62bdee6 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 = -EINVAL;
>>>
>>>
>>> Should we consider it an error if the card does not support poweroff
>>> notification?
>>
>> To make it more meaningful I will change the error value to -EOPNOTSUPP.
>>>
>>>
>>> Moreover, this function returns an error code which is good. Although
>>> everywhere you call this function you do not consider the error. Please
>>> look
>>> into changing this.
>>
>> Also will consider the return value from the places it is called to
>> display the info "card doesnt support po notify"
>>>
>>>
>>>
>>>>> +
>>>>> + 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)&&
>>>
>>>
>>> Some white spaces..
>>
>> will rum cleanpatch
>>>
>>>
>>>
>>>>> + (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)
>>>
>>>
>>> Some white spaces..
>>>
>>>
>>>>> + 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,20 @@ static int mmc_suspend(struct mmc_host *host)
>>>>> BUG_ON(!host);
>>>>> BUG_ON(!host->card);
>>>>>
>>>>> - mmc_claim_host(host);
>>>
>>>
>>> nested claims is OK, so you can keep this if the code becomes easier to
>>> read!? The corresponding mmc_release_host then as well of course...
>>>
>>>>> - 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->caps2& MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) {
>>>
>>>
>>> Some white spaces..
>>>
>>> This will not work. If the card does not support "poweroff_notify", but
>>> sleep, then the card will not be put to sleep. The cap should not be used
>>> to
>>> determine what the card supports, only the host.
>>
>> I will remove th host caps option. and directly call the power off
>> notify function. This function will execute the switch command only if
>> the card supports the power off notify.
>> so comparison with host caps can be omitted.
>
>
> I don't think removing the host cap i a good option. If the host only
> supports removing VCC and not VCCQ, we would like to issue the sleep cmd in
> favor of poweroff_notify cmd. Both conditions must thus be checked.
>
> Thus I think a "mmc_card_can_poweroff_notify" could be feasible to use here
> for simplifying things.
>
If host does not remove VCCQ, then it will not enable
MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND. I agree that we need to have
an additional check just to ensure that we wrongly do not issue power
Off notify on 4.41 eMMC. We can do something like,
static int mmc_suspend(struct mmc_host *host)
{
...
card = host->card;
if (card && mmc_card_mmc(card) &&
(card->poweroff_notify_state == MMC_POWERED_ON) &&
(host->caps2 & MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) {
err = mmc_poweroff_notify(host);
} else {
....
}
....
}
Girish, I still like to have this host cap.
>
>
>>>
>>> Maybe an new function "mmc_card_can_poweroff_notify" can help out in
>>> taking
>>> the right decision here. The you also might be able to get rid of the
>>> host->poweroff_notify_state variable, don't know.
>>
>> not necessary to add a new function as said above the notify function
>> takes care of card support
>>>
>>>
>>>
>>>>> + 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 +1417,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 +1479,7 @@ static const struct mmc_bus_ops mmc_ops = {
>>>>> .resume = NULL,
>>>>> .power_restore = mmc_power_restore,
>>>>> .alive = mmc_alive,
>>>>> + .poweroff_notify = NULL,
>>>
>>>
>>> Is there any problems with adding the function here? Even if an eMMC
>>> always
>>> should be considered non-removable, and thus the unsafe bus_ops should be
>>> used. I don't know the reason to why sleep/awake has been added to this
>>> bus_ops, but I think poweroff_notify should be handled similarly...
>>
>> No problem in adding. The only reason for not adding is that none of
>> the removable cards support poweroff notify. this feature is supported
>> above 4.5 version. If you have come across any 4.5 removable device,
>> it would more convinient to add here.
>
>
> For testing I believe 4.5 eMMC in an SD-card adapter can be used. But just
> for testing...
>
>
>>>
>>>
>>>>> };
>>>>>
>>>>> static const struct mmc_bus_ops mmc_ops_unsafe = {
>>>>> @@ -1441,6 +1491,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 cbde4b7..cf44a6f 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)
>>>
>>>
>>> Don't know if we would like to use the old caps(1) bit 7, since
>>> MMC_CAP_DISABLE has been removed and thus this bit is now available...
>>
>> I am planning to remove this caps.
>>>
>>>
>>>
>>>>>
>>>>> mmc_pm_flag_t pm_caps; /* supported pm features
>>>>> */
>>>>> unsigned int power_notify_type;
>>>>
>>>>
>>>>
>>>> Hi Ulf,
>>>> need your comment on the above.
>>>>>
>>>>>
>>>>> --
>>>>> 1.7.1
>>>>>
>>>
>>> Overall, I like this patch, we are getting close to a final version I
>>> think.
>>>
>>> Kind regards
>>> Ulf Hansson
>
>
> Kind regards
> Ulf Hansson
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] MMC-4.5 Power OFF Notify rework
2012-04-27 8:42 ` Saugata Das
@ 2012-04-27 8:53 ` Girish K S
0 siblings, 0 replies; 12+ messages in thread
From: Girish K S @ 2012-04-27 8:53 UTC (permalink / raw)
To: Saugata Das; +Cc: Ulf Hansson, linux-mmc, patches
On 27 April 2012 14:12, Saugata Das <saugata.das@linaro.org> wrote:
> On 27 April 2012 13:10, Ulf Hansson <ulf.hansson@stericsson.com> wrote:
>> On 04/27/2012 06:40 AM, Girish K S wrote:
>>>
>>> On 23 April 2012 19:41, Ulf Hansson<ulf.hansson@stericsson.com> wrote:
>>>>
>>>> Hi Girish,
>>>>
>>>> Please see some comments below...
>>>>
>>>>
>>>> On 04/20/2012 01:33 PM, Girish K S wrote:
>>>>>
>>>>>
>>>>> On 19 April 2012 18:11, Girish K S<girish.shivananjappa@linaro.org>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> This is a rework of the existing POWER OFF NOTIFY patch. The current
>>>>>> problem
>>>>>> with the patch comes from the ambiguity on the usage of POWER OFF
>>>>>> NOTIFY
>>>>>> together with SLEEP and misunderstanding on the usage of MMC_POWER_OFF
>>>>>> power_mode from mmc_set_ios in different host controller drivers.
>>>>>>
>>>>>> This new patch works around this problem by adding a new host CAP,
>>>>>> MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND, which when set sends a
>>>>>> POWER OFF NOTIFY from mmc_suspend instead of SLEEP. It is expected that
>>>>>> host
>>>>>> controller drivers will set this CAP, if they switch off both Vcc and
>>>>>> Vccq
>>>>>> from MMC_POWER_OFF condition within mmc_set_ios. However, note that
>>>>>> there
>>>>>> is no harm in sending MMC_POWER_NOTIFY even if Vccq is not switched
>>>>>> off.
>>>>>>
>>>>>> This patch also sends POWER OFF NOTIFY from power management routines
>>>>>> (e.g.
>>>>>> mmc_power_save_host, mmc_pm_notify/PM_SUSPEND_PREPARE, mmc_stop_host),
>>>>>> which
>>>>>> does reinitialization of the eMMC on the return path of the power
>>>>>> management
>>>>>> routines (e.g. mmc_power_restore_host, mmc_pm_notify/PM_POST_RESTORE,
>>>>>> mmc_start_host).
>>>>>>
>>>>>> 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.
>>>>
>>>>
>>>> Great, I like this! :-)
>>>>
>>>>>>
>>>>>> Signed-off-by: Saugata Das<saugata.das@linaro.org>
>>>>>> Signed-off-by: Girish K S<girish.shivananjappa@linaro.org>
>>>>>> ---
>>>>>> drivers/mmc/core/core.c | 83
>>>>>> ++++++++++-----------------------------------
>>>>>> drivers/mmc/core/core.h | 1 +
>>>>>> drivers/mmc/core/mmc.c | 75
>>>>>> +++++++++++++++++++++++++++++++++++-------
>>>>>> include/linux/mmc/host.h | 1 +
>>>>>> 4 files changed, 84 insertions(+), 76 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>>> index e541efb..4b8b2c1 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,9 @@ 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)
>>>>>> + host->bus_ops->poweroff_notify(host);
>>>>>> +
>>>>>> /* Calling bus_ops->remove() with a claimed host can
>>>>>> deadlock */
>>>>>> if (host->bus_ops->remove)
>>>>>> host->bus_ops->remove(host);
>>>>>> @@ -2093,6 +2036,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,7 +2063,9 @@ 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)
>>>>>> + host->bus_ops->poweroff_notify(host);
>>>>>> mmc_bus_put(host);
>>>>>>
>>>>>> mmc_power_off(host);
>>>>>> @@ -2142,7 +2088,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 +2107,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 +2128,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);
>>>>>>
>>>>>> @@ -2394,6 +2346,8 @@ int mmc_pm_notify(struct notifier_block
>>>>>> *notify_block,
>>>>>>
>>>>>> if (!host->bus_ops || host->bus_ops->suspend)
>>>>>> break;
>>>>>> + if (host->bus_ops->poweroff_notify)
>>>>>> + host->bus_ops->poweroff_notify(host);
>>>>>>
>>>>>> /* Calling bus_ops->remove() with a claimed host can
>>>>>> deadlock */
>>>>>> if (host->bus_ops->remove)
>>>>>> @@ -2403,6 +2357,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..62bdee6 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 = -EINVAL;
>>>>
>>>>
>>>> Should we consider it an error if the card does not support poweroff
>>>> notification?
>>>
>>> To make it more meaningful I will change the error value to -EOPNOTSUPP.
>>>>
>>>>
>>>> Moreover, this function returns an error code which is good. Although
>>>> everywhere you call this function you do not consider the error. Please
>>>> look
>>>> into changing this.
>>>
>>> Also will consider the return value from the places it is called to
>>> display the info "card doesnt support po notify"
>>>>
>>>>
>>>>
>>>>>> +
>>>>>> + 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)&&
>>>>
>>>>
>>>> Some white spaces..
>>>
>>> will rum cleanpatch
>>>>
>>>>
>>>>
>>>>>> + (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)
>>>>
>>>>
>>>> Some white spaces..
>>>>
>>>>
>>>>>> + 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,20 @@ static int mmc_suspend(struct mmc_host *host)
>>>>>> BUG_ON(!host);
>>>>>> BUG_ON(!host->card);
>>>>>>
>>>>>> - mmc_claim_host(host);
>>>>
>>>>
>>>> nested claims is OK, so you can keep this if the code becomes easier to
>>>> read!? The corresponding mmc_release_host then as well of course...
>>>>
>>>>>> - 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->caps2& MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) {
>>>>
>>>>
>>>> Some white spaces..
>>>>
>>>> This will not work. If the card does not support "poweroff_notify", but
>>>> sleep, then the card will not be put to sleep. The cap should not be used
>>>> to
>>>> determine what the card supports, only the host.
>>>
>>> I will remove th host caps option. and directly call the power off
>>> notify function. This function will execute the switch command only if
>>> the card supports the power off notify.
>>> so comparison with host caps can be omitted.
>>
>>
>> I don't think removing the host cap i a good option. If the host only
>> supports removing VCC and not VCCQ, we would like to issue the sleep cmd in
>> favor of poweroff_notify cmd. Both conditions must thus be checked.
>>
>> Thus I think a "mmc_card_can_poweroff_notify" could be feasible to use here
>> for simplifying things.
>>
>
> If host does not remove VCCQ, then it will not enable
> MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND. I agree that we need to have
> an additional check just to ensure that we wrongly do not issue power
> Off notify on 4.41 eMMC. We can do something like,
>
> static int mmc_suspend(struct mmc_host *host)
> {
> ...
> card = host->card;
>
> if (card && mmc_card_mmc(card) &&
> (card->poweroff_notify_state == MMC_POWERED_ON) &&
These above checks are already done inside mmc_poweroff_notify so you
can add the below comparison there itself
> (host->caps2 & MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) {
> err = mmc_poweroff_notify(host);
> } else {
> ....
> }
> ....
> }
>
> Girish, I still like to have this host cap.
>
>>
>>
>>>>
>>>> Maybe an new function "mmc_card_can_poweroff_notify" can help out in
>>>> taking
>>>> the right decision here. The you also might be able to get rid of the
>>>> host->poweroff_notify_state variable, don't know.
>>>
>>> not necessary to add a new function as said above the notify function
>>> takes care of card support
>>>>
>>>>
>>>>
>>>>>> + 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 +1417,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 +1479,7 @@ static const struct mmc_bus_ops mmc_ops = {
>>>>>> .resume = NULL,
>>>>>> .power_restore = mmc_power_restore,
>>>>>> .alive = mmc_alive,
>>>>>> + .poweroff_notify = NULL,
>>>>
>>>>
>>>> Is there any problems with adding the function here? Even if an eMMC
>>>> always
>>>> should be considered non-removable, and thus the unsafe bus_ops should be
>>>> used. I don't know the reason to why sleep/awake has been added to this
>>>> bus_ops, but I think poweroff_notify should be handled similarly...
>>>
>>> No problem in adding. The only reason for not adding is that none of
>>> the removable cards support poweroff notify. this feature is supported
>>> above 4.5 version. If you have come across any 4.5 removable device,
>>> it would more convinient to add here.
>>
>>
>> For testing I believe 4.5 eMMC in an SD-card adapter can be used. But just
>> for testing...
>>
>>
>>>>
>>>>
>>>>>> };
>>>>>>
>>>>>> static const struct mmc_bus_ops mmc_ops_unsafe = {
>>>>>> @@ -1441,6 +1491,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 cbde4b7..cf44a6f 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)
>>>>
>>>>
>>>> Don't know if we would like to use the old caps(1) bit 7, since
>>>> MMC_CAP_DISABLE has been removed and thus this bit is now available...
>>>
>>> I am planning to remove this caps.
>>>>
>>>>
>>>>
>>>>>>
>>>>>> mmc_pm_flag_t pm_caps; /* supported pm features
>>>>>> */
>>>>>> unsigned int power_notify_type;
>>>>>
>>>>>
>>>>>
>>>>> Hi Ulf,
>>>>> need your comment on the above.
>>>>>>
>>>>>>
>>>>>> --
>>>>>> 1.7.1
>>>>>>
>>>>
>>>> Overall, I like this patch, we are getting close to a final version I
>>>> think.
>>>>
>>>> Kind regards
>>>> Ulf Hansson
>>
>>
>> Kind regards
>> Ulf Hansson
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] MMC-4.5 Power OFF Notify Rework
@ 2012-06-15 4:08 Saugata Das
2012-06-15 12:57 ` S, Venkatraman
0 siblings, 1 reply; 12+ messages in thread
From: Saugata Das @ 2012-06-15 4:08 UTC (permalink / raw)
To: linux-mmc; +Cc: patches, saugata.das, girish.shivananjappa, asutoshd, subhashj
From: Saugata Das <saugata.das@linaro.org>
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.
For the cards that cannot cut vccq can sleep during suspend. But the after
suspend->sleep->poweroff the ios values are modified. which results in
malfunction of card after resume. This patch fixes that issue by saving the
ios before sleep and restoring the saved values before resume.
Signed-off-by: Saugata Das <saugata.das@linaro.org>
Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
changes in v6:
-save/restore ios during suspend/resume
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 | 127 ++++++++++++++++++++++-----------------------
drivers/mmc/core/core.h | 3 +
drivers/mmc/core/mmc.c | 58 ++++++++++++++++++---
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 | 6 +--
include/linux/mmc/mmc.h | 7 +++
9 files changed, 128 insertions(+), 93 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 0b6141d..6f68aad 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -734,6 +734,25 @@ void mmc_set_chip_select(struct mmc_host *host, int mode)
}
/*
+ * used to save the ios before suspend.
+ */
+void mmc_save_ios(struct mmc_host *host, struct mmc_ios *ios)
+{
+ mmc_host_clk_hold(host);
+ memcpy(&host->saved_ios, ios, sizeof(struct mmc_ios));
+ mmc_host_clk_release(host);
+}
+
+/*
+ * Restore the saved ios.
+ */
+void mmc_restore_ios(struct mmc_host *host, struct mmc_ios *saved_ios)
+{
+ memcpy(&host->ios, saved_ios, sizeof(struct mmc_ios));
+ mmc_set_ios(host);
+}
+
+/*
* Sets the host clock to the highest possible frequency that
* is below "hz".
*/
@@ -1101,48 +1120,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 +1179,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 +1187,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 +1685,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 +2064,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 +2108,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 +2159,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 +2180,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 +2377,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 +2409,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..6952abe 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);
@@ -33,6 +34,8 @@ void mmc_detach_bus(struct mmc_host *host);
void mmc_init_erase(struct mmc_card *card);
void mmc_set_chip_select(struct mmc_host *host, int mode);
+void mmc_save_ios(struct mmc_host *host, struct mmc_ios *ios);
+void mmc_restore_ios(struct mmc_host *host, struct mmc_ios *saved_ios);
void mmc_set_clock(struct mmc_host *host, unsigned int hz);
void mmc_gate_clock(struct mmc_host *host);
void mmc_ungate_clock(struct mmc_host *host);
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 2f0e11c..c809025 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,19 @@ 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)) {
+ mmc_save_ios(host, &host->ios);
+ 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;
@@ -1346,8 +1386,8 @@ static int mmc_resume(struct mmc_host *host)
mmc_claim_host(host);
if (mmc_card_is_sleep(host->card)) {
+ mmc_restore_ios(host, &host->saved_ios);
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 +1447,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 +1459,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..b089968 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 */
@@ -270,6 +267,7 @@ struct mmc_host {
spinlock_t lock; /* lock for claim and bus ops */
struct mmc_ios ios; /* current io bus settings */
+ struct mmc_ios saved_ios; /* saved io bus settings */
u32 ocr; /* the current OCR setting */
/* group bitfields together to minimize padding */
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.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] MMC-4.5 Power OFF Notify Rework
2012-06-15 4:08 [PATCH] MMC-4.5 Power OFF Notify Rework Saugata Das
@ 2012-06-15 12:57 ` S, Venkatraman
0 siblings, 0 replies; 12+ messages in thread
From: S, Venkatraman @ 2012-06-15 12:57 UTC (permalink / raw)
To: Saugata Das
Cc: linux-mmc, patches, saugata.das, girish.shivananjappa, asutoshd,
subhashj
On Fri, Jun 15, 2012 at 9:38 AM, Saugata Das <saugata.das@stericsson.com> wrote:
> From: Saugata Das <saugata.das@linaro.org>
>
> 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.
>
> For the cards that cannot cut vccq can sleep during suspend. But the after
> suspend->sleep->poweroff the ios values are modified. which results in
> malfunction of card after resume. This patch fixes that issue by saving the
> ios before sleep and restoring the saved values before resume.
>
> Signed-off-by: Saugata Das <saugata.das@linaro.org>
> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
>
I just had a chance to test this on OMAP. I only have v4.41 card on
this one, so it doesn't do
Power Off Notify, but suspend resume works with just CMD5, and hasn't caused a
regression. FWIW,
Partially Tested-by: Venkatraman S<svenkatr@ti.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] MMC-4.5 Power OFF Notify Rework
@ 2012-06-29 9:03 Saugata Das
2012-08-20 14:13 ` Ulf Hansson
0 siblings, 1 reply; 12+ messages in thread
From: Saugata Das @ 2012-06-29 9:03 UTC (permalink / raw)
To: linux-mmc
Cc: patches, ulf.hansson, venkat, Saugata Das, Girish K S, Asutosh Das
From: Saugata Das <saugata.das@linaro.org>
This is a rework of the existing POWER OFF NOTIFY patch. The CMD0 based
reinitialization of the eMMC during mmc_resume is introduced back. Function
poweroff_notify has been added as a bus_ops (as desired by the reviewers on
a previous version of the patch). Removed the configuration of notify type
("power_notify_type") from the host drivers. This is now passed as parameter
to poweroff_notify.
Signed-off-by: Saugata Das <saugata.das@linaro.org>
Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
drivers/mmc/core/core.c | 108 ++++++++++++++++++--------------------------
drivers/mmc/core/core.h | 1 +
drivers/mmc/core/mmc.c | 44 +++++++++++++++---
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 | 4 --
include/linux/mmc/mmc.h | 7 +++
9 files changed, 92 insertions(+), 92 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..8684577 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_err("%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.
*/
@@ -1321,8 +1355,6 @@ static int mmc_suspend(struct mmc_host *host)
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);
@@ -1345,11 +1377,7 @@ static int mmc_resume(struct mmc_host *host)
BUG_ON(!host->card);
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);
+ err = mmc_init_card(host, host->ocr, host->card);
mmc_release_host(host);
return err;
@@ -1407,6 +1435,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 +1447,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..b685dff 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -240,10 +240,6 @@ struct mmc_host {
#define MMC_CAP2_HC_ERASE_SZ (1 << 9) /* High-capacity erase size */
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.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] MMC-4.5 Power OFF Notify Rework
2012-06-29 9:03 Saugata Das
@ 2012-08-20 14:13 ` Ulf Hansson
0 siblings, 0 replies; 12+ messages in thread
From: Ulf Hansson @ 2012-08-20 14:13 UTC (permalink / raw)
To: Saugata Das
Cc: linux-mmc, patches, ulf.hansson, venkat, Saugata Das, Girish K S,
Asutosh Das
Hi Saugata,
Sorry for a very late reply, I just came back from vacation.
Anyway, this patch looks good to me. You have my Ack.
Kind regards
Ulf Hansson
On 29 June 2012 11:03, Saugata Das <saugata.das@stericsson.com> wrote:
> From: Saugata Das <saugata.das@linaro.org>
>
> This is a rework of the existing POWER OFF NOTIFY patch. The CMD0 based
> reinitialization of the eMMC during mmc_resume is introduced back. Function
> poweroff_notify has been added as a bus_ops (as desired by the reviewers on
> a previous version of the patch). Removed the configuration of notify type
> ("power_notify_type") from the host drivers. This is now passed as parameter
> to poweroff_notify.
>
> Signed-off-by: Saugata Das <saugata.das@linaro.org>
> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> ---
> drivers/mmc/core/core.c | 108 ++++++++++++++++++--------------------------
> drivers/mmc/core/core.h | 1 +
> drivers/mmc/core/mmc.c | 44 +++++++++++++++---
> 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 | 4 --
> include/linux/mmc/mmc.h | 7 +++
> 9 files changed, 92 insertions(+), 92 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..8684577 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_err("%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.
> */
> @@ -1321,8 +1355,6 @@ static int mmc_suspend(struct mmc_host *host)
> 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);
> @@ -1345,11 +1377,7 @@ static int mmc_resume(struct mmc_host *host)
> BUG_ON(!host->card);
>
> 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);
> + err = mmc_init_card(host, host->ocr, host->card);
> mmc_release_host(host);
>
> return err;
> @@ -1407,6 +1435,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 +1447,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..b685dff 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -240,10 +240,6 @@ struct mmc_host {
> #define MMC_CAP2_HC_ERASE_SZ (1 << 9) /* High-capacity erase size */
>
> 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.3
>
> --
> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-08-20 14:13 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-19 12:41 [PATCH] MMC-4.5 Power OFF Notify rework Girish K S
2012-04-20 11:33 ` Girish K S
2012-04-20 12:55 ` Ulf Hansson
2012-04-23 14:11 ` Ulf Hansson
2012-04-27 4:40 ` Girish K S
2012-04-27 7:40 ` Ulf Hansson
2012-04-27 8:42 ` Saugata Das
2012-04-27 8:53 ` Girish K S
2012-06-15 4:08 [PATCH] MMC-4.5 Power OFF Notify Rework Saugata Das
2012-06-15 12:57 ` S, Venkatraman
2012-06-29 9:03 Saugata Das
2012-08-20 14:13 ` Ulf Hansson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.