* [RFC PATCH v3] mmc: sleep notification
@ 2015-06-08 6:46 Avi Shchislowski
2015-06-08 9:03 ` Ulf Hansson
0 siblings, 1 reply; 8+ messages in thread
From: Avi Shchislowski @ 2015-06-08 6:46 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Alex Lemberg, linux-mmc
Hi Ulf,
Here is a V3 per your request.
Please review.
This patch is implements the new additional state of
Power_Off_Notification - SLEEP_NOTIFICATION.
Until now, the implementation of Power_Off_Notification
supported only three modes - POWERED_ON (0x01),
POWER_OFF_SHORT (0x02) and POWER_OFF_LONG (0x03).
As part of eMMC5.0 before moving to Sleep state hosts may set the
POWER_OFF_NOTIFICATION byte to SLEEP_NOTIFICATION (0x04).
After setting SLEEP_NOTIFICATION, host should wait for
the busy line to be de-asserted.
Signed-off-by: Alex Lemberg <alex.lemberg@sandisk.com>
Signed-off-by: Avi Shchislowski <avi.shchislowski@sandisk.com>
---------
V3:
- remove macros
- remove hpi_en dependency
- add helper function for sleep notification
v2:
- remove calling mmc_interrupt_hpi in case the device is doing
sleep notification from block.c
- remove call of "mmc_card_doing_sleep_notify" from core.c
- mmc_sleep_notify changed to static function
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index a802863..56dae18 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -60,6 +60,14 @@ static const unsigned int tacc_mant[] = {
})
/*
+ * Indicates the maximum allowed timeout - 83.88s
+ * as defined in eMMC Spec for the Sleep Notification
+ * SWITCH command when notifying the device that it is
+ * about to be moved to sleep state.
+ */
+#define MMC_SLEEP_NOTIFY_MAX_TIME 0x17
+
+/*
* Given the decoded CSD structure, decode the raw CID to our CID structure.
*/
static int mmc_decode_cid(struct mmc_card *card)
@@ -582,6 +590,8 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd)
card->ext_csd.ffu_capable =
(ext_csd[EXT_CSD_SUPPORTED_MODE] & 0x1) &&
!(ext_csd[EXT_CSD_FW_CONFIG] & 0x1);
+ card->ext_csd.sleep_notify_time =
+ ext_csd[EXT_CSD_SLEEP_NOTIFICATION_TIME];
}
out:
return err;
@@ -1642,6 +1652,44 @@ out_release:
return err;
}
+/*
+ * check if device is in program state (busy)
+ */
+static bool mmc_device_prg_state(struct mmc_card *card)
+{
+ int rc;
+ u32 status;
+ bool state;
+
+ mmc_get_card(card);
+ rc = mmc_send_status(card, &status);
+ if (rc) {
+ pr_err("%s: Get card status fail. rc=%d\n",
+ mmc_hostname(card->host), rc);
+ state = false;
+ goto out;
+ }
+
+ if (R1_CURRENT_STATE(status) == R1_STATE_PRG)
+ state = true;
+ else
+ state = false;
+out:
+ mmc_put_card(card);
+ return state;
+}
+
+static int can_sleep_notify(struct mmc_card *card)
+{
+ /* sleep notify enable/disable for eMMC 5.0 and above */
+ return (card && card->ext_csd.rev >= 7 &&
+ (card->host->caps2 & MMC_CAP2_SLEEP_NOTIFY) &&
+ card->ext_csd.sleep_notify_time > 0 &&
+ card->ext_csd.sleep_notify_time <=
+ MMC_SLEEP_NOTIFY_MAX_TIME &&
+ card->ext_csd.power_off_notification == EXT_CSD_POWER_ON);
+}
+
static int mmc_can_poweroff_notify(const struct mmc_card *card)
{
return card &&
@@ -1653,17 +1701,32 @@ static int mmc_poweroff_notify(struct mmc_card *card, unsigned int notify_type)
{
unsigned int timeout = card->ext_csd.generic_cmd6_time;
int err;
+ bool use_busy_signal;
/* Use EXT_CSD_POWER_OFF_SHORT as default notification type. */
if (notify_type == EXT_CSD_POWER_OFF_LONG)
timeout = card->ext_csd.power_off_longtime;
+ else if (notify_type == EXT_CSD_SLEEP_NOTIFICATION) {
+ /* calculate the maximum timeout for the
+ * switch command when notifying the device
+ * that it is about to move to sleep */
+ timeout = DIV_ROUND_UP((10 *
+ (1 << (unsigned int)(card->ext_csd.sleep_notify_time))),1000);
+ }
+
+ use_busy_signal = (notify_type == EXT_CSD_SLEEP_NOTIFICATION) ?
+ false : true;
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_POWER_OFF_NOTIFICATION,
- notify_type, timeout, true, false, false);
- if (err)
+ notify_type, timeout,
+ use_busy_signal, false, false);
+ if (!err) {
+ card->ext_csd.power_off_notification = notify_type;
+ } else {
pr_err("%s: Power Off Notification timed out, %u\n",
- mmc_hostname(card->host), timeout);
+ mmc_hostname(card->host), timeout);
+ }
/* Disable the power off notification after the switch operation. */
card->ext_csd.power_off_notification = EXT_CSD_NO_POWER_NOTIFICATION;
@@ -1671,6 +1734,70 @@ static int mmc_poweroff_notify(struct mmc_card *card, unsigned int notify_type)
return err;
}
+static int mmc_sleep_notify(struct mmc_card *card)
+{
+ int err = 0;
+ bool is_busy = 0;
+ unsigned long prg_wait = 0;
+ unsigned int timeout;
+
+ if (!can_sleep_notify(card) || !mmc_can_poweroff_notify(card))
+ return 0;
+
+ if (!mmc_card_doing_sleep_notify(card)) {
+ mmc_get_card(card);
+ mmc_card_set_sleep_notify(card);
+ err = mmc_poweroff_notify(card,
+ EXT_CSD_SLEEP_NOTIFICATION);
+ mmc_put_card(card);
+ if (err) {
+ pr_err("%s: mmc_poweroff_notify failed with %d\n",
+ __func__, err);
+ goto out;
+ }
+ timeout = DIV_ROUND_UP((10 *
+ (1 << (unsigned int)(card->ext_csd.sleep_notify_time)))
+ ,1000);
+ prg_wait = jiffies + msecs_to_jiffies(timeout);
+ }
+ /*
+ * Loop will run until Device is no
+ * more in Busy state
+ */
+
+ do {
+ /* added some delay to avoid sending card status too often */
+ msleep(20);
+ err = 0;
+ is_busy = mmc_device_prg_state(card);
+ if (is_busy && time_after(jiffies, prg_wait)) {
+ /*
+ * making sure we are still in busy before
+ * sending HPI due to timeout error
+ */
+ is_busy = mmc_device_prg_state(card);
+ if (is_busy) {
+ if (mmc_card_doing_sleep_notify(card)) {
+ card->ext_csd.power_off_notification =
+ EXT_CSD_POWER_ON;
+ mmc_interrupt_hpi(card);
+ }
+ err = -ETIMEDOUT;
+ break;
+ }
+ }
+ } while (is_busy);
+
+out:
+ mmc_card_clr_sleep_notify(card);
+ if (err) {
+ pr_err("%s: mmc_poweroff_notify for sleep failed with %d\n",
+ mmc_hostname(card->host), err);
+ }
+
+ return err;
+}
+
/*
* Host is being removed. Free up the current card.
*/
@@ -1744,14 +1871,19 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
if (err)
goto out;
- if (mmc_can_poweroff_notify(host->card) &&
- ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
- err = mmc_poweroff_notify(host->card, notify_type);
- else if (mmc_can_sleep(host->card))
+ if (mmc_can_poweroff_notify(host->card)) {
+ if (!can_sleep_notify(host->card)) {
+ err = mmc_poweroff_notify(host->card, notify_type);
+ } else if (is_suspend) {
+ err = mmc_sleep_notify(host->card);
+ if (err != -ETIMEDOUT)
+ err = mmc_sleep(host);
+ }
+ } else if (mmc_can_sleep(host->card)) {
err = mmc_sleep(host);
- else if (!mmc_host_is_spi(host))
+ } else if (!mmc_host_is_spi(host)) {
err = mmc_deselect_cards(host);
-
+ }
if (!err) {
mmc_power_off(host);
mmc_card_set_suspended(host->card);
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 19f0175..3189e73 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -62,6 +62,7 @@ struct mmc_ext_csd {
unsigned int sa_timeout; /* Units: 100ns */
unsigned int generic_cmd6_time; /* Units: 10ms */
unsigned int power_off_longtime; /* Units: ms */
+ unsigned int sleep_notify_time; /* Units: us */
u8 power_off_notification; /* state */
unsigned int hs_max_dtr;
unsigned int hs200_max_dtr;
@@ -262,6 +263,7 @@ struct mmc_card {
#define MMC_CARD_REMOVED (1<<4) /* card has been removed */
#define MMC_STATE_DOING_BKOPS (1<<5) /* card is doing BKOPS */
#define MMC_STATE_SUSPENDED (1<<6) /* card is suspended */
+#define MMC_STATE_SLEEP_NOTIFY (1<<7) /* card in sleep notify */
unsigned int quirks; /* card quirks */
#define MMC_QUIRK_LENIENT_FN0 (1<<0) /* allow SDIO FN0 writes outside of the VS CCCR range */
#define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1) /* use func->cur_blksize */
@@ -427,6 +429,7 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
#define mmc_card_removed(c) ((c) && ((c)->state & MMC_CARD_REMOVED))
#define mmc_card_doing_bkops(c) ((c)->state & MMC_STATE_DOING_BKOPS)
#define mmc_card_suspended(c) ((c)->state & MMC_STATE_SUSPENDED)
+#define mmc_card_doing_sleep_notify(c) ((c)->state & MMC_STATE_SLEEP_NOTIFY)
#define mmc_card_set_present(c) ((c)->state |= MMC_STATE_PRESENT)
#define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
@@ -437,6 +440,8 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
#define mmc_card_clr_doing_bkops(c) ((c)->state &= ~MMC_STATE_DOING_BKOPS)
#define mmc_card_set_suspended(c) ((c)->state |= MMC_STATE_SUSPENDED)
#define mmc_card_clr_suspended(c) ((c)->state &= ~MMC_STATE_SUSPENDED)
+#define mmc_card_set_sleep_notify(c) ((c)->state |= MMC_STATE_SLEEP_NOTIFY)
+#define mmc_card_clr_sleep_notify(c) ((c)->state &= ~MMC_STATE_SLEEP_NOTIFY)
/*
* Quirk add/remove for MMC products.
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index f471193..111e05d 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -286,6 +286,7 @@ struct mmc_host {
MMC_CAP2_HS400_1_2V)
#define MMC_CAP2_HSX00_1_2V (MMC_CAP2_HS200_1_2V_SDR | MMC_CAP2_HS400_1_2V)
#define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
+#define MMC_CAP2_SLEEP_NOTIFY (1 << 18) /* sleep notify supported */
mmc_pm_flag_t pm_caps; /* supported pm features */
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 124f562..bbb71ae 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -309,6 +309,7 @@ struct _mmc_csd {
#define EXT_CSD_PWR_CL_52_360 202 /* RO */
#define EXT_CSD_PWR_CL_26_360 203 /* RO */
#define EXT_CSD_SEC_CNT 212 /* RO, 4 bytes */
+#define EXT_CSD_SLEEP_NOTIFICATION_TIME 216 /* RO */
#define EXT_CSD_S_A_TIMEOUT 217 /* RO */
#define EXT_CSD_REL_WR_SEC_C 222 /* RO */
#define EXT_CSD_HC_WP_GRP_SIZE 221 /* RO */
@@ -403,6 +404,7 @@ struct _mmc_csd {
#define EXT_CSD_POWER_ON 1
#define EXT_CSD_POWER_OFF_SHORT 2
#define EXT_CSD_POWER_OFF_LONG 3
+#define EXT_CSD_SLEEP_NOTIFICATION 4
#define EXT_CSD_PWR_CL_8BIT_MASK 0xF0 /* 8 bit PWR CLS */
#define EXT_CSD_PWR_CL_4BIT_MASK 0x0F /* 8 bit PWR CLS */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v3] mmc: sleep notification
2015-06-08 6:46 [RFC PATCH v3] mmc: sleep notification Avi Shchislowski
@ 2015-06-08 9:03 ` Ulf Hansson
2015-06-08 13:17 ` Alex Lemberg
0 siblings, 1 reply; 8+ messages in thread
From: Ulf Hansson @ 2015-06-08 9:03 UTC (permalink / raw)
To: Avi Shchislowski; +Cc: Alex Lemberg, linux-mmc
On 8 June 2015 at 08:46, Avi Shchislowski <Avi.Shchislowski@sandisk.com> wrote:
> Hi Ulf,
>
> Here is a V3 per your request.
> Please review.
>
> This patch is implements the new additional state of
> Power_Off_Notification - SLEEP_NOTIFICATION.
> Until now, the implementation of Power_Off_Notification
> supported only three modes - POWERED_ON (0x01),
> POWER_OFF_SHORT (0x02) and POWER_OFF_LONG (0x03).
>
> As part of eMMC5.0 before moving to Sleep state hosts may set the
> POWER_OFF_NOTIFICATION byte to SLEEP_NOTIFICATION (0x04).
> After setting SLEEP_NOTIFICATION, host should wait for
> the busy line to be de-asserted.
>
> Signed-off-by: Alex Lemberg <alex.lemberg@sandisk.com>
> Signed-off-by: Avi Shchislowski <avi.shchislowski@sandisk.com>
You need to trim your change-log here. I can't apply patches which
comes in this format and run checkpatch, please.
Moreover, try answer the *what* and *why* question in the change log.
>
> ---------
It's nice that you provide a history, but don't include that into the
commit message.
Change the above nine "-" to three "-", to make the history below be
posted as a appended message for the patch.
> V3:
> - remove macros
> - remove hpi_en dependency
> - add helper function for sleep notification
>
> v2:
> - remove calling mmc_interrupt_hpi in case the device is doing
> sleep notification from block.c
> - remove call of "mmc_card_doing_sleep_notify" from core.c
> - mmc_sleep_notify changed to static function
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index a802863..56dae18 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -60,6 +60,14 @@ static const unsigned int tacc_mant[] = {
> })
>
> /*
> + * Indicates the maximum allowed timeout - 83.88s
> + * as defined in eMMC Spec for the Sleep Notification
> + * SWITCH command when notifying the device that it is
> + * about to be moved to sleep state.
> + */
> +#define MMC_SLEEP_NOTIFY_MAX_TIME 0x17
> +
> +/*
> * Given the decoded CSD structure, decode the raw CID to our CID structure.
> */
> static int mmc_decode_cid(struct mmc_card *card)
> @@ -582,6 +590,8 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd)
> card->ext_csd.ffu_capable =
> (ext_csd[EXT_CSD_SUPPORTED_MODE] & 0x1) &&
> !(ext_csd[EXT_CSD_FW_CONFIG] & 0x1);
> + card->ext_csd.sleep_notify_time =
> + ext_csd[EXT_CSD_SLEEP_NOTIFICATION_TIME];
> }
> out:
> return err;
> @@ -1642,6 +1652,44 @@ out_release:
> return err;
> }
>
> +/*
> + * check if device is in program state (busy)
> + */
> +static bool mmc_device_prg_state(struct mmc_card *card)
> +{
> + int rc;
> + u32 status;
> + bool state;
> +
> + mmc_get_card(card);
> + rc = mmc_send_status(card, &status);
> + if (rc) {
> + pr_err("%s: Get card status fail. rc=%d\n",
> + mmc_hostname(card->host), rc);
> + state = false;
> + goto out;
> + }
> +
> + if (R1_CURRENT_STATE(status) == R1_STATE_PRG)
> + state = true;
> + else
> + state = false;
> +out:
> + mmc_put_card(card);
> + return state;
> +}
> +
> +static int can_sleep_notify(struct mmc_card *card)
> +{
> + /* sleep notify enable/disable for eMMC 5.0 and above */
> + return (card && card->ext_csd.rev >= 7 &&
> + (card->host->caps2 & MMC_CAP2_SLEEP_NOTIFY) &&
> + card->ext_csd.sleep_notify_time > 0 &&
> + card->ext_csd.sleep_notify_time <=
> + MMC_SLEEP_NOTIFY_MAX_TIME &&
> + card->ext_csd.power_off_notification == EXT_CSD_POWER_ON);
> +}
> +
> static int mmc_can_poweroff_notify(const struct mmc_card *card)
> {
> return card &&
> @@ -1653,17 +1701,32 @@ static int mmc_poweroff_notify(struct mmc_card *card, unsigned int notify_type)
> {
> unsigned int timeout = card->ext_csd.generic_cmd6_time;
> int err;
> + bool use_busy_signal;
>
> /* Use EXT_CSD_POWER_OFF_SHORT as default notification type. */
> if (notify_type == EXT_CSD_POWER_OFF_LONG)
> timeout = card->ext_csd.power_off_longtime;
> + else if (notify_type == EXT_CSD_SLEEP_NOTIFICATION) {
> + /* calculate the maximum timeout for the
> + * switch command when notifying the device
> + * that it is about to move to sleep */
>
> + timeout = DIV_ROUND_UP((10 *
> + (1 << (unsigned int)(card->ext_csd.sleep_notify_time))),1000);
> + }
> +
> + use_busy_signal = (notify_type == EXT_CSD_SLEEP_NOTIFICATION) ?
> + false : true;
> err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> EXT_CSD_POWER_OFF_NOTIFICATION,
> - notify_type, timeout, true, false, false);
> - if (err)
> + notify_type, timeout,
> + use_busy_signal, false, false);
> + if (!err) {
> + card->ext_csd.power_off_notification = notify_type;
> + } else {
> pr_err("%s: Power Off Notification timed out, %u\n",
> - mmc_hostname(card->host), timeout);
> + mmc_hostname(card->host), timeout);
> + }
>
> /* Disable the power off notification after the switch operation. */
> card->ext_csd.power_off_notification = EXT_CSD_NO_POWER_NOTIFICATION;
> @@ -1671,6 +1734,70 @@ static int mmc_poweroff_notify(struct mmc_card *card, unsigned int notify_type)
> return err;
> }
>
> +static int mmc_sleep_notify(struct mmc_card *card)
> +{
> + int err = 0;
> + bool is_busy = 0;
> + unsigned long prg_wait = 0;
> + unsigned int timeout;
> +
> + if (!can_sleep_notify(card) || !mmc_can_poweroff_notify(card))
> + return 0;
> +
> + if (!mmc_card_doing_sleep_notify(card)) {
> + mmc_get_card(card);
> + mmc_card_set_sleep_notify(card);
> + err = mmc_poweroff_notify(card,
> + EXT_CSD_SLEEP_NOTIFICATION);
> + mmc_put_card(card);
> + if (err) {
> + pr_err("%s: mmc_poweroff_notify failed with %d\n",
> + __func__, err);
> + goto out;
> + }
> + timeout = DIV_ROUND_UP((10 *
> + (1 << (unsigned int)(card->ext_csd.sleep_notify_time)))
> + ,1000);
> + prg_wait = jiffies + msecs_to_jiffies(timeout);
> + }
> + /*
> + * Loop will run until Device is no
> + * more in Busy state
> + */
> +
> + do {
> + /* added some delay to avoid sending card status too often */
> + msleep(20);
> + err = 0;
> + is_busy = mmc_device_prg_state(card);
> + if (is_busy && time_after(jiffies, prg_wait)) {
> + /*
> + * making sure we are still in busy before
> + * sending HPI due to timeout error
> + */
> + is_busy = mmc_device_prg_state(card);
> + if (is_busy) {
> + if (mmc_card_doing_sleep_notify(card)) {
> + card->ext_csd.power_off_notification =
> + EXT_CSD_POWER_ON;
> + mmc_interrupt_hpi(card);
> + }
> + err = -ETIMEDOUT;
> + break;
> + }
> + }
> + } while (is_busy);
> +
> +out:
> + mmc_card_clr_sleep_notify(card);
> + if (err) {
> + pr_err("%s: mmc_poweroff_notify for sleep failed with %d\n",
> + mmc_hostname(card->host), err);
> + }
> +
> + return err;
> +}
> +
> /*
> * Host is being removed. Free up the current card.
> */
> @@ -1744,14 +1871,19 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
> if (err)
> goto out;
>
> - if (mmc_can_poweroff_notify(host->card) &&
> - ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
> - err = mmc_poweroff_notify(host->card, notify_type);
> - else if (mmc_can_sleep(host->card))
> + if (mmc_can_poweroff_notify(host->card)) {
> + if (!can_sleep_notify(host->card)) {
> + err = mmc_poweroff_notify(host->card, notify_type);
> + } else if (is_suspend) {
> + err = mmc_sleep_notify(host->card);
> + if (err != -ETIMEDOUT)
> + err = mmc_sleep(host);
> + }
> + } else if (mmc_can_sleep(host->card)) {
> err = mmc_sleep(host);
> - else if (!mmc_host_is_spi(host))
> + } else if (!mmc_host_is_spi(host)) {
> err = mmc_deselect_cards(host);
> -
> + }
> if (!err) {
> mmc_power_off(host);
> mmc_card_set_suspended(host->card);
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 19f0175..3189e73 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -62,6 +62,7 @@ struct mmc_ext_csd {
> unsigned int sa_timeout; /* Units: 100ns */
> unsigned int generic_cmd6_time; /* Units: 10ms */
> unsigned int power_off_longtime; /* Units: ms */
> + unsigned int sleep_notify_time; /* Units: us */
> u8 power_off_notification; /* state */
> unsigned int hs_max_dtr;
> unsigned int hs200_max_dtr;
> @@ -262,6 +263,7 @@ struct mmc_card {
> #define MMC_CARD_REMOVED (1<<4) /* card has been removed */
> #define MMC_STATE_DOING_BKOPS (1<<5) /* card is doing BKOPS */
> #define MMC_STATE_SUSPENDED (1<<6) /* card is suspended */
> +#define MMC_STATE_SLEEP_NOTIFY (1<<7) /* card in sleep notify */
> unsigned int quirks; /* card quirks */
> #define MMC_QUIRK_LENIENT_FN0 (1<<0) /* allow SDIO FN0 writes outside of the VS CCCR range */
> #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1) /* use func->cur_blksize */
> @@ -427,6 +429,7 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
> #define mmc_card_removed(c) ((c) && ((c)->state & MMC_CARD_REMOVED))
> #define mmc_card_doing_bkops(c) ((c)->state & MMC_STATE_DOING_BKOPS)
> #define mmc_card_suspended(c) ((c)->state & MMC_STATE_SUSPENDED)
> +#define mmc_card_doing_sleep_notify(c) ((c)->state & MMC_STATE_SLEEP_NOTIFY)
>
> #define mmc_card_set_present(c) ((c)->state |= MMC_STATE_PRESENT)
> #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
> @@ -437,6 +440,8 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
> #define mmc_card_clr_doing_bkops(c) ((c)->state &= ~MMC_STATE_DOING_BKOPS)
> #define mmc_card_set_suspended(c) ((c)->state |= MMC_STATE_SUSPENDED)
> #define mmc_card_clr_suspended(c) ((c)->state &= ~MMC_STATE_SUSPENDED)
> +#define mmc_card_set_sleep_notify(c) ((c)->state |= MMC_STATE_SLEEP_NOTIFY)
> +#define mmc_card_clr_sleep_notify(c) ((c)->state &= ~MMC_STATE_SLEEP_NOTIFY)
>
> /*
> * Quirk add/remove for MMC products.
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index f471193..111e05d 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -286,6 +286,7 @@ struct mmc_host {
> MMC_CAP2_HS400_1_2V)
> #define MMC_CAP2_HSX00_1_2V (MMC_CAP2_HS200_1_2V_SDR | MMC_CAP2_HS400_1_2V)
> #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
> +#define MMC_CAP2_SLEEP_NOTIFY (1 << 18) /* sleep notify supported */
>
> mmc_pm_flag_t pm_caps; /* supported pm features */
>
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index 124f562..bbb71ae 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -309,6 +309,7 @@ struct _mmc_csd {
> #define EXT_CSD_PWR_CL_52_360 202 /* RO */
> #define EXT_CSD_PWR_CL_26_360 203 /* RO */
> #define EXT_CSD_SEC_CNT 212 /* RO, 4 bytes */
> +#define EXT_CSD_SLEEP_NOTIFICATION_TIME 216 /* RO */
> #define EXT_CSD_S_A_TIMEOUT 217 /* RO */
> #define EXT_CSD_REL_WR_SEC_C 222 /* RO */
> #define EXT_CSD_HC_WP_GRP_SIZE 221 /* RO */
> @@ -403,6 +404,7 @@ struct _mmc_csd {
> #define EXT_CSD_POWER_ON 1
> #define EXT_CSD_POWER_OFF_SHORT 2
> #define EXT_CSD_POWER_OFF_LONG 3
> +#define EXT_CSD_SLEEP_NOTIFICATION 4
>
> #define EXT_CSD_PWR_CL_8BIT_MASK 0xF0 /* 8 bit PWR CLS */
> #define EXT_CSD_PWR_CL_4BIT_MASK 0x0F /* 8 bit PWR CLS */
> --
> 1.7.9.5
>
>
>
One of my comments for v2, was that I think you should remove all code
which was related to HPI to interrupt sleep notification from the
runtime PM resume path. Instead I wanted you to add that functionality
as separate patch based on top of this patch.
You haven't done that in v3, why?
Kind regards
Uffe
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [RFC PATCH v3] mmc: sleep notification
2015-06-08 9:03 ` Ulf Hansson
@ 2015-06-08 13:17 ` Alex Lemberg
2015-06-15 8:23 ` Ulf Hansson
0 siblings, 1 reply; 8+ messages in thread
From: Alex Lemberg @ 2015-06-08 13:17 UTC (permalink / raw)
To: Ulf Hansson, Avi Shchislowski; +Cc: linux-mmc
Hi Ulf,
[...]
>
> One of my comments for v2, was that I think you should remove all code which
> was related to HPI to interrupt sleep notification from the runtime PM resume
> path. Instead I wanted you to add that functionality as separate patch based on
> top of this patch.
>
> You haven't done that in v3, why?
The sleep_notify call was moved to suspend() per your recommendation.
As far as I understand, no new requests should be sent during mmc_suspend() process,
thus HPI support is not needed anymore.
Is this the correct assumption?
The only case where HPI is used in this patch - is during sleep_notify timeout error.
Thanks,
Alex
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v3] mmc: sleep notification
2015-06-08 13:17 ` Alex Lemberg
@ 2015-06-15 8:23 ` Ulf Hansson
2015-06-18 14:57 ` Alex Lemberg
0 siblings, 1 reply; 8+ messages in thread
From: Ulf Hansson @ 2015-06-15 8:23 UTC (permalink / raw)
To: Alex Lemberg; +Cc: Avi Shchislowski, linux-mmc
On 8 June 2015 at 15:17, Alex Lemberg <Alex.Lemberg@sandisk.com> wrote:
> Hi Ulf,
>
> [...]
>
>>
>> One of my comments for v2, was that I think you should remove all code which
>> was related to HPI to interrupt sleep notification from the runtime PM resume
>> path. Instead I wanted you to add that functionality as separate patch based on
>> top of this patch.
>>
>> You haven't done that in v3, why?
>
> The sleep_notify call was moved to suspend() per your recommendation.
> As far as I understand, no new requests should be sent during mmc_suspend() process,
> thus HPI support is not needed anymore.
> Is this the correct assumption?
Yes.
I don't think you need mmc_card_set_sleep_notify() and the
corresponding new MMC_STATE_SLEEP_NOTIFY , mmc_device_prg_state(),
etc.
Overall, I think this patch could be simplified yet another step.
> The only case where HPI is used in this patch - is during sleep_notify timeout error.
Why?
One final question, I noticed that you have removed the check for
MMC_CAP2_FULL_PWR_CYCLE in the _mmc_suspend() function, why?
Kind regards
Uffe
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [RFC PATCH v3] mmc: sleep notification
2015-06-15 8:23 ` Ulf Hansson
@ 2015-06-18 14:57 ` Alex Lemberg
2015-07-22 14:40 ` Ulf Hansson
0 siblings, 1 reply; 8+ messages in thread
From: Alex Lemberg @ 2015-06-18 14:57 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Avi Shchislowski, linux-mmc
Hi Ulf,
> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> Sent: Monday, June 15, 2015 11:24 AM
> To: Alex Lemberg
> Cc: Avi Shchislowski; linux-mmc
> Subject: Re: [RFC PATCH v3] mmc: sleep notification
>
> On 8 June 2015 at 15:17, Alex Lemberg <Alex.Lemberg@sandisk.com>
> wrote:
> > Hi Ulf,
> >
> > [...]
> >
> >>
> >> One of my comments for v2, was that I think you should remove all
> >> code which was related to HPI to interrupt sleep notification from
> >> the runtime PM resume path. Instead I wanted you to add that
> >> functionality as separate patch based on top of this patch.
> >>
> >> You haven't done that in v3, why?
> >
> > The sleep_notify call was moved to suspend() per your recommendation.
> > As far as I understand, no new requests should be sent during
> > mmc_suspend() process, thus HPI support is not needed anymore.
> > Is this the correct assumption?
>
> Yes.
>
> I don't think you need mmc_card_set_sleep_notify() and the corresponding
> new MMC_STATE_SLEEP_NOTIFY , mmc_device_prg_state(), etc.
mmc_card_set_sleep_notify(), MMC_STATE_SLEEP_NOTIFY and the corresponding - We are removing it from the current patch. Probably will add them as a separate patch in future.
mmc_device_prg_state() - is required for waiting for Sleep_Notification completion/timeout, although we would like to leave this function.
>
> Overall, I think this patch could be simplified yet another step.
>
> > The only case where HPI is used in this patch - is during sleep_notify
> timeout error.
>
> Why?
In case of timeout error, we would like to handle it by sending HPI - to let device interrupt/stop the prg state.
>
> One final question, I noticed that you have removed the check for
> MMC_CAP2_FULL_PWR_CYCLE in the _mmc_suspend() function, why?
As far as we understand, this MMC_CAP2_FULL_PWR_CYCLE was used to distinguish between PON (when the power can be cut)
and Sleep (when the power have to stay ON).
Now we have Sleep_Notification PON, which allows to cut the power also in case of Sleep.
Hope the interpretation above is correct.
>
> Kind regards
> Uffe
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v3] mmc: sleep notification
2015-06-18 14:57 ` Alex Lemberg
@ 2015-07-22 14:40 ` Ulf Hansson
2015-08-03 12:44 ` Alex Lemberg
0 siblings, 1 reply; 8+ messages in thread
From: Ulf Hansson @ 2015-07-22 14:40 UTC (permalink / raw)
To: Alex Lemberg; +Cc: Avi Shchislowski, linux-mmc
On 18 June 2015 at 16:57, Alex Lemberg <Alex.Lemberg@sandisk.com> wrote:
> Hi Ulf,
>
>> -----Original Message-----
>> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
>> Sent: Monday, June 15, 2015 11:24 AM
>> To: Alex Lemberg
>> Cc: Avi Shchislowski; linux-mmc
>> Subject: Re: [RFC PATCH v3] mmc: sleep notification
>>
>> On 8 June 2015 at 15:17, Alex Lemberg <Alex.Lemberg@sandisk.com>
>> wrote:
>> > Hi Ulf,
>> >
>> > [...]
>> >
>> >>
>> >> One of my comments for v2, was that I think you should remove all
>> >> code which was related to HPI to interrupt sleep notification from
>> >> the runtime PM resume path. Instead I wanted you to add that
>> >> functionality as separate patch based on top of this patch.
>> >>
>> >> You haven't done that in v3, why?
>> >
>> > The sleep_notify call was moved to suspend() per your recommendation.
>> > As far as I understand, no new requests should be sent during
>> > mmc_suspend() process, thus HPI support is not needed anymore.
>> > Is this the correct assumption?
>>
>> Yes.
>>
>> I don't think you need mmc_card_set_sleep_notify() and the corresponding
>> new MMC_STATE_SLEEP_NOTIFY , mmc_device_prg_state(), etc.
>
> mmc_card_set_sleep_notify(), MMC_STATE_SLEEP_NOTIFY and the corresponding - We are removing it from the current patch. Probably will add them as a separate patch in future.
Great!
> mmc_device_prg_state() - is required for waiting for Sleep_Notification completion/timeout, although we would like to leave this function.
Doesn't __mmc_switch() already deal with this? No, I don't think it's needed.
>
>>
>> Overall, I think this patch could be simplified yet another step.
>>
>> > The only case where HPI is used in this patch - is during sleep_notify
>> timeout error.
>>
>> Why?
>
> In case of timeout error, we would like to handle it by sending HPI - to let device interrupt/stop the prg state.
Is that according to the spec? I think it would be better to try a
reset sequence to recover, by using CMD0 and so forth, what do you
think?
>
>>
>> One final question, I noticed that you have removed the check for
>> MMC_CAP2_FULL_PWR_CYCLE in the _mmc_suspend() function, why?
>
> As far as we understand, this MMC_CAP2_FULL_PWR_CYCLE was used to distinguish between PON (when the power can be cut)
> and Sleep (when the power have to stay ON).
> Now we have Sleep_Notification PON, which allows to cut the power also in case of Sleep.
> Hope the interpretation above is correct.
Unfortunate your interpretation is wrong.
MMC_CAP2_FULL_PWR_CYCLE, indicates whether the mmc host can cut *both*
VCC and VCCQ.
For the suspend case, the default behaviour is to send CMD5 (sleep),
as we don't trust hosts to set MMC_CAP2_FULL_PWR_CYCLE even if it
potentially could.
Keeping this policy is important, as we don't want to issue PON unless
we know that we will be cutting both VCC and VCCQ.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [RFC PATCH v3] mmc: sleep notification
2015-07-22 14:40 ` Ulf Hansson
@ 2015-08-03 12:44 ` Alex Lemberg
2015-08-25 12:04 ` Ulf Hansson
0 siblings, 1 reply; 8+ messages in thread
From: Alex Lemberg @ 2015-08-03 12:44 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Avi Shchislowski, linux-mmc
Hi Ulf,
> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> Sent: Wednesday, July 22, 2015 5:41 PM
> To: Alex Lemberg
> Cc: Avi Shchislowski; linux-mmc
> Subject: Re: [RFC PATCH v3] mmc: sleep notification
>
> On 18 June 2015 at 16:57, Alex Lemberg <Alex.Lemberg@sandisk.com>
> wrote:
> > Hi Ulf,
> >
> >> -----Original Message-----
> >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> >> Sent: Monday, June 15, 2015 11:24 AM
> >> To: Alex Lemberg
> >> Cc: Avi Shchislowski; linux-mmc
> >> Subject: Re: [RFC PATCH v3] mmc: sleep notification
> >>
> >> On 8 June 2015 at 15:17, Alex Lemberg <Alex.Lemberg@sandisk.com>
> >> wrote:
> >> > Hi Ulf,
> >> >
> >> > [...]
> >> >
> >> >>
> >> >> One of my comments for v2, was that I think you should remove all
> >> >> code which was related to HPI to interrupt sleep notification from
> >> >> the runtime PM resume path. Instead I wanted you to add that
> >> >> functionality as separate patch based on top of this patch.
> >> >>
> >> >> You haven't done that in v3, why?
> >> >
> >> > The sleep_notify call was moved to suspend() per your
> recommendation.
> >> > As far as I understand, no new requests should be sent during
> >> > mmc_suspend() process, thus HPI support is not needed anymore.
> >> > Is this the correct assumption?
> >>
> >> Yes.
> >>
> >> I don't think you need mmc_card_set_sleep_notify() and the
> >> corresponding new MMC_STATE_SLEEP_NOTIFY ,
> mmc_device_prg_state(), etc.
> >
> > mmc_card_set_sleep_notify(), MMC_STATE_SLEEP_NOTIFY and the
> corresponding - We are removing it from the current patch. Probably will add
> them as a separate patch in future.
>
> Great!
>
> > mmc_device_prg_state() - is required for waiting for Sleep_Notification
> completion/timeout, although we would like to leave this function.
>
> Doesn't __mmc_switch() already deal with this? No, I don't think it's needed.
You are right, __mmc_switch() wait loop is enough, if HPI is not going to be in use during sleep_notification.
>
> >
> >>
> >> Overall, I think this patch could be simplified yet another step.
> >>
> >> > The only case where HPI is used in this patch - is during
> >> > sleep_notify
> >> timeout error.
> >>
> >> Why?
> >
> > In case of timeout error, we would like to handle it by sending HPI - to let
> device interrupt/stop the prg state.
>
> Is that according to the spec? I think it would be better to try a reset
> sequence to recover, by using CMD0 and so forth, what do you think?
Not must by the spec, but there are several options how to handle timeout error.
Checking the existing driver, the '-ETIMEDOUT' error is handled by mmc_interrupt_hpi ()
in case of mmc_wait_for_req_done() function.
Can we use the same approach?
>
> >
> >>
> >> One final question, I noticed that you have removed the check for
> >> MMC_CAP2_FULL_PWR_CYCLE in the _mmc_suspend() function, why?
> >
> > As far as we understand, this MMC_CAP2_FULL_PWR_CYCLE was used to
> > distinguish between PON (when the power can be cut) and Sleep (when the
> power have to stay ON).
> > Now we have Sleep_Notification PON, which allows to cut the power also
> in case of Sleep.
> > Hope the interpretation above is correct.
>
> Unfortunate your interpretation is wrong.
>
> MMC_CAP2_FULL_PWR_CYCLE, indicates whether the mmc host can cut
> *both* VCC and VCCQ.
>
> For the suspend case, the default behaviour is to send CMD5 (sleep), as we
> don't trust hosts to set MMC_CAP2_FULL_PWR_CYCLE even if it potentially
> could.
> Keeping this policy is important, as we don't want to issue PON unless we
> know that we will be cutting both VCC and VCCQ.
You are right, spec doesn’t talk about vccq in case of Sleep_Notification, but
only about vcc.
We will stay with existing logic and add sleep_notification call before sending sleep command.
Thanks,
Alex
>
> Kind regards
> Uffe
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v3] mmc: sleep notification
2015-08-03 12:44 ` Alex Lemberg
@ 2015-08-25 12:04 ` Ulf Hansson
0 siblings, 0 replies; 8+ messages in thread
From: Ulf Hansson @ 2015-08-25 12:04 UTC (permalink / raw)
To: Alex Lemberg; +Cc: Avi Shchislowski, linux-mmc
[...]
>> >> > The only case where HPI is used in this patch - is during
>> >> > sleep_notify
>> >> timeout error.
>> >>
>> >> Why?
>> >
>> > In case of timeout error, we would like to handle it by sending HPI - to let
>> device interrupt/stop the prg state.
>>
>> Is that according to the spec? I think it would be better to try a reset
>> sequence to recover, by using CMD0 and so forth, what do you think?
>
> Not must by the spec, but there are several options how to handle timeout error.
> Checking the existing driver, the '-ETIMEDOUT' error is handled by mmc_interrupt_hpi ()
> in case of mmc_wait_for_req_done() function.
> Can we use the same approach?
Currently the logic to recover from errors during system PM suspend,
relies on the request-retry mechanism which is executed far later.
>From the ->suspend() callback we will only propagate the error code
towards the PM core. That means the PM core will bail out and reverse
the actions already done for suspended devices.
In this initial patch, I suggest we stick to that behaviour.
Although, I am open to discuss a more sophisticated error handling,
but let's do that from a separate patch.
[...]
Kind regards
Uffe
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-08-25 12:05 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-08 6:46 [RFC PATCH v3] mmc: sleep notification Avi Shchislowski
2015-06-08 9:03 ` Ulf Hansson
2015-06-08 13:17 ` Alex Lemberg
2015-06-15 8:23 ` Ulf Hansson
2015-06-18 14:57 ` Alex Lemberg
2015-07-22 14:40 ` Ulf Hansson
2015-08-03 12:44 ` Alex Lemberg
2015-08-25 12:04 ` 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.