| Message ID | 1425980197-823-1-git-send-email-avi.shchislowski@sandisk.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10 March 2015 at 10:36, Avi Shchislowski <avi.shchislowski@sandisk.com> wrote: > 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. > The max timeout allowed for busy line de-assertion defined > in SLEEP_NOTIFICATION_TIME byte in EXT_CSD [216]. > HPI may interrupt the SLEEP_NOTIFICATION operation. > In that case POWER_OFF_NOTIFICATION byte will restore to POWERED_ON. Oh, just great! JEDEC has invented yet another way of how to send a device to sleep state. I wonder what was wrong with the CMD5 option. Anyway, thanks for looking into this. > > Signed-off-by: Alex Lemberg <alex.lemberg@sandisk.com> > Signed-off-by: Avi Shchislowski <avi.shchislowski@sandisk.com> > --- > drivers/mmc/card/block.c | 19 +++++- > drivers/mmc/core/core.c | 16 +++++- > drivers/mmc/core/core.h | 2 + > drivers/mmc/core/mmc.c | 143 +++++++++++++++++++++++++++++++++++++++++++--- > include/linux/mmc/card.h | 6 ++ > include/linux/mmc/host.h | 1 + > include/linux/mmc/mmc.h | 2 + > 7 files changed, 180 insertions(+), 9 deletions(-) > > diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c > index 4409d79..f511ecc3 100644 > --- a/drivers/mmc/card/block.c > +++ b/drivers/mmc/card/block.c > @@ -2010,9 +2010,26 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req) > unsigned long flags; > unsigned int cmd_flags = req ? req->cmd_flags : 0; > > - if (req && !mq->mqrq_prev->req) > + if (unlikely(req && mmc_card_mmc(card) && > + (card->ext_csd.power_off_notification == > + EXT_CSD_SLEEP_NOTIFICATION))) { > + /* restoring the power_off_notification > + * field's state to as it was before so > + * that the sleep notification will be > + * able to resume later > + */ > + card->ext_csd.power_off_notification = EXT_CSD_POWER_ON; > + } > + > + if (req && !mq->mqrq_prev->req) { > /* claim host only for the first request */ > mmc_get_card(card); > + if (unlikely(req && > + mmc_card_doing_sleep_notify(card))) { > + mmc_interrupt_hpi(card); > + mmc_card_clr_sleep_notify(card); > + } > + } Both above new added code blocks makes no sense to me. Why does the mmc block layer need to care about this? > > ret = mmc_blk_part_switch(card, md); > if (ret) { > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 5bda29b..a090593 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -271,7 +271,8 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception) > > BUG_ON(!card); > > - if (!card->ext_csd.bkops_en || mmc_card_doing_bkops(card)) > + if (!card->ext_csd.bkops_en || mmc_card_doing_bkops(card) || > + mmc_card_doing_sleep_notify(card)) We can't do BKOPS when the card is suspended (which is when the card may be put in sleep state), so no need to protect for this. > return; > > err = mmc_read_bkops_status(card); > @@ -2630,6 +2631,19 @@ int mmc_pm_notify(struct notifier_block *notify_block, > err = host->bus_ops->pre_suspend(host); > if (!err) > break; > + if (host->card && host->bus_ops->suspend) { Nope, this is the wrong place for this code. You should look into mmc.c for adding this. > + err = mmc_sleep_notify(host->card); > + /* We assume that HPI was sent > + * in case of -ETIMEDOUT error, > + * so suspend flow can be continued > + */ > + if (err && err != -ETIMEDOUT) { > + pr_err("%s:sleep notify err=%d\n", > + __func__, err); > + return -EBUSY; > + } > + break; > + } > > /* Calling bus_ops->remove() with a claimed host can deadlock */ > host->bus_ops->remove(host); > diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h > index d76597c..b6b4431 100644 > --- a/drivers/mmc/core/core.h > +++ b/drivers/mmc/core/core.h > @@ -14,6 +14,7 @@ > #include <linux/delay.h> > > #define MMC_CMD_RETRIES 3 > +#define MMC_SLEEP_NOTIFY_MAX_TIME 0x17 > > struct mmc_bus_ops { > void (*remove)(struct mmc_host *); > @@ -33,6 +34,7 @@ void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops); > void mmc_detach_bus(struct mmc_host *host); > > void mmc_init_erase(struct mmc_card *card); > +int mmc_sleep_notify(struct mmc_card *card); If we need a separate function to deal with this, it should be static function within mmc.c. > > void mmc_set_chip_select(struct mmc_host *host, int mode); > void mmc_set_clock(struct mmc_host *host, unsigned int hz); > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index 02ad792..1d97d24 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -57,6 +57,11 @@ static const unsigned int tacc_mant[] = { > __res & __mask; \ > }) > > +#define GET_SLEEP_NOTIFY_TIME(value) \ > + (10 * (1 << (unsigned int)(value))) > +#define GET_SLEEP_NOTIFY_TIME_MSEC(value) \ > + (DIV_ROUND_UP(GET_SLEEP_NOTIFY_TIME(value), 1000)) > + > /* > * Given the decoded CSD structure, decode the raw CID to our CID structure. > */ > @@ -571,6 +576,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; > @@ -1468,6 +1475,15 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, > card->ext_csd.hpi_en = 1; > } > > + /* sleep notify enable/disable for eMMC 5.0 and above */ > + if ((card->ext_csd.rev >= 7) && card->ext_csd.hpi_en && Why depend on ext_csd.hpi_en? > + (card->host->caps2 & MMC_CAP2_SLEEP_NOTIFY) && Please don't add a new CAP for this. MMC_CAP2_FULL_PWR_CYCLE tells us if the card can be fully power gated, I think we can use that instead!? > + card->ext_csd.sleep_notify_time > 0 && > + card->ext_csd.sleep_notify_time <= > + MMC_SLEEP_NOTIFY_MAX_TIME) { > + card->can_sleep_notify = 1; Instead of adding "can_sleep_notify", let's just encapsulate similar code as above in a helper function which tells us if card supports "sleep notification". Compare how mmc_can_sleep() is implemented and used. > + } > + > /* > * If cache size is higher than 0, this indicates > * the existence of cache and it can be turned on. > @@ -1576,6 +1592,33 @@ static int mmc_sleep(struct mmc_host *host) > 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 mmc_can_poweroff_notify(const struct mmc_card *card) > { > return card && > @@ -1585,22 +1628,108 @@ static int mmc_can_poweroff_notify(const struct mmc_card *card) > > static int mmc_poweroff_notify(struct mmc_card *card, unsigned int notify_type) > { > - unsigned int timeout = card->ext_csd.generic_cmd6_time; > + unsigned int timeout_ms = card->ext_csd.generic_cmd6_time; What's reason to do a rename in this patch? If you want that as a clarification of the code, please do that in a separate patch. > 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; > + timeout_ms = 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_ms = GET_SLEEP_NOTIFY_TIME_MSEC( > + card->ext_csd.sleep_notify_time); > + } > > + /* do not wait on busy signal in case of > + * Sleep Notification - to let host get > + * another requests At this point, there should be no other request. Or what am I missing? > + */ > + 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_ms, > + 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_ms); > + } > + > + return err; > +} > > - /* Disable the power off notification after the switch operation. */ > - card->ext_csd.power_off_notification = EXT_CSD_NO_POWER_NOTIFICATION; Why remove this? > +int mmc_sleep_notify(struct mmc_card *card) I don't understand the need for this function. From what scenarios do we want to invoke it? > +{ > + int err = 0; > + bool is_busy = 0; > + unsigned long prg_wait = 0; > + > + if (!card->can_sleep_notify || !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; > + } > + > + prg_wait = jiffies + > + msecs_to_jiffies(GET_SLEEP_NOTIFY_TIME_MSEC( > + card->ext_csd.sleep_notify_time)); > + } > + > + /* > + * Loop will run until: > + * 1. Device is no more in Busy state > + * 2. Sleep notification is not interrupted by HPI & IO request > + */ > + do { > + /* added some delay to avoid sending card status too often */ > + msleep(20); > + err = 0; > + /* Stop polling in case sleep notification was HPIed already */ > + if (!mmc_card_doing_sleep_notify(card)) { > + is_busy = mmc_device_prg_state(card); > + if (is_busy) > + err = -EBUSY; > + break; > + } > + 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; > } > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h > index 4d69c00..6998344 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 */ I think MMC_STATE_SUSPENDED should be enough, we don't need another one specific for 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 */ > @@ -309,6 +311,7 @@ struct mmc_card { > struct dentry *debugfs_root; > struct mmc_part part[MMC_NUM_PHY_PARTITION]; /* physical partitions */ > unsigned int nr_parts; > + u8 can_sleep_notify; /* sleep_notify on/off */ As stated, please remove. > }; > > /* > @@ -427,6 +430,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 +441,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 9f32270..4c0542a 100644 > --- a/include/linux/mmc/host.h > +++ b/include/linux/mmc/host.h > @@ -291,6 +291,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 49ad7a9..69bda9a 100644 > --- a/include/linux/mmc/mmc.h > +++ b/include/linux/mmc/mmc.h > @@ -314,6 +314,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 */ > @@ -408,6 +409,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 > Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/03/15 11:36, Avi Shchislowski wrote: > 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. > The max timeout allowed for busy line de-assertion defined > in SLEEP_NOTIFICATION_TIME byte in EXT_CSD [216]. > HPI may interrupt the SLEEP_NOTIFICATION operation. > In that case POWER_OFF_NOTIFICATION byte will restore to POWERED_ON. Is it so that the point of SLEEP_NOTIFICATION is to notify that VCC will be powered off after the device is put to sleep by CMD5? At the moment, the card does not get put to sleep if there is support for power-off-notification? -- 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
Hi Ulf, Thanks for your review and comments! > -----Original Message----- > From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > Sent: Tuesday, March 10, 2015 5:08 AM > To: Avi Shchislowski > Cc: linux-mmc; Chris Ball; Alex Lemberg > Subject: Re: [PATCH] mmc: sleep notification > > On 10 March 2015 at 10:36, Avi Shchislowski <avi.shchislowski@sandisk.com> > wrote: > > 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. > > The max timeout allowed for busy line de-assertion defined in > > SLEEP_NOTIFICATION_TIME byte in EXT_CSD [216]. > > HPI may interrupt the SLEEP_NOTIFICATION operation. > > In that case POWER_OFF_NOTIFICATION byte will restore to POWERED_ON. > > Oh, just great! JEDEC has invented yet another way of how to send a device to > sleep state. I wonder what was wrong with the CMD5 option. I think before adding sleep_notification, host has no way to notify device before sending sleep command. Now device can better prepare itself for moving into the sleep state. Also, I think we need to clarify one more point for this patch: As was mentioned in commit message - Sleep_Notification can be interrupted by HPI. This allows not blocking the host during the Sleep_Notification busy time and allows accepting requests coming during this stage. Thus, without having HPI supported, suspend/resume process might be influenced by Sleep_Notification busy time, and this should not happen - suspend/resume should be done in very fast and not blocking manner. Thanks, Alex
Hi Adrian, > -----Original Message----- > From: Adrian Hunter [mailto:adrian.hunter@intel.com] > Sent: Tuesday, March 10, 2015 6:36 AM > To: Avi Shchislowski; ulf.hansson@linaro.org > Cc: linux-mmc@vger.kernel.org; chris@printf.net; Alex Lemberg > Subject: Re: [PATCH] mmc: sleep notification > > On 10/03/15 11:36, Avi Shchislowski wrote: > > 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. > > The max timeout allowed for busy line de-assertion defined in > > SLEEP_NOTIFICATION_TIME byte in EXT_CSD [216]. > > HPI may interrupt the SLEEP_NOTIFICATION operation. > > In that case POWER_OFF_NOTIFICATION byte will restore to POWERED_ON. > > Is it so that the point of SLEEP_NOTIFICATION is to notify that VCC will be > powered off after the device is put to sleep by CMD5? From the spec: the host should issue a power off notification (POWER_OFF_LONG, POWER_OFF_SHORT ) if it intends to turn off both VCC and VCCQ power supplies or it may use to a power off notification (SLEEP_NOTIFICATION ) if it intends to turn-off VCC after moving the device to Sleep state. > > At the moment, the card does not get put to sleep if there is support for power- > off-notification? Driver will send Sleep only if PON is not supported... Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10 March 2015 at 17:32, Alex Lemberg <Alex.Lemberg@sandisk.com> wrote: > Hi Adrian, > >> -----Original Message----- >> From: Adrian Hunter [mailto:adrian.hunter@intel.com] >> Sent: Tuesday, March 10, 2015 6:36 AM >> To: Avi Shchislowski; ulf.hansson@linaro.org >> Cc: linux-mmc@vger.kernel.org; chris@printf.net; Alex Lemberg >> Subject: Re: [PATCH] mmc: sleep notification >> >> On 10/03/15 11:36, Avi Shchislowski wrote: >> > 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. >> > The max timeout allowed for busy line de-assertion defined in >> > SLEEP_NOTIFICATION_TIME byte in EXT_CSD [216]. >> > HPI may interrupt the SLEEP_NOTIFICATION operation. >> > In that case POWER_OFF_NOTIFICATION byte will restore to POWERED_ON. >> >> Is it so that the point of SLEEP_NOTIFICATION is to notify that VCC will be >> powered off after the device is put to sleep by CMD5? > > From the spec: > > the host should issue a power off notification (POWER_OFF_LONG, POWER_OFF_SHORT ) > if it intends to turn off both VCC and VCCQ power supplies or it may use > to a power off notification (SLEEP_NOTIFICATION ) if it intends to turn-off VCC > after moving the device to Sleep state. Ah, so CMD5 will still need to be issued. Thanks for clarifying. > >> >> At the moment, the card does not get put to sleep if there is support for power- >> off-notification? > > Driver will send Sleep only if PON is not supported... Nope, that's not entirely correct. The MMC core requires PON to be supported by the card and the host to have MMC_CAP2_FULL_PWR_CYCLE set. Else it will send sleep (CMD5) instead. Kind regards Uffe -- 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
> -----Original Message----- > From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > Sent: Tuesday, March 10, 2015 10:22 AM > To: Alex Lemberg > Cc: Adrian Hunter; Avi Shchislowski; linux-mmc@vger.kernel.org; > chris@printf.net > Subject: Re: [PATCH] mmc: sleep notification > > On 10 March 2015 at 17:32, Alex Lemberg <Alex.Lemberg@sandisk.com> > wrote: > > Hi Adrian, > > > >> -----Original Message----- > >> From: Adrian Hunter [mailto:adrian.hunter@intel.com] > >> Sent: Tuesday, March 10, 2015 6:36 AM > >> To: Avi Shchislowski; ulf.hansson@linaro.org > >> Cc: linux-mmc@vger.kernel.org; chris@printf.net; Alex Lemberg > >> Subject: Re: [PATCH] mmc: sleep notification > >> > >> On 10/03/15 11:36, Avi Shchislowski wrote: > >> > 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. > >> > The max timeout allowed for busy line de-assertion defined in > >> > SLEEP_NOTIFICATION_TIME byte in EXT_CSD [216]. > >> > HPI may interrupt the SLEEP_NOTIFICATION operation. > >> > In that case POWER_OFF_NOTIFICATION byte will restore to > POWERED_ON. > >> > >> Is it so that the point of SLEEP_NOTIFICATION is to notify that VCC > >> will be powered off after the device is put to sleep by CMD5? > > > > From the spec: > > > > the host should issue a power off notification (POWER_OFF_LONG, > > POWER_OFF_SHORT ) if it intends to turn off both VCC and VCCQ power > > supplies or it may use to a power off notification (SLEEP_NOTIFICATION > > ) if it intends to turn-off VCC after moving the device to Sleep state. > > Ah, so CMD5 will still need to be issued. Thanks for clarifying. > > > > >> > >> At the moment, the card does not get put to sleep if there is support > >> for power- off-notification? > > > > Driver will send Sleep only if PON is not supported... > > Nope, that's not entirely correct. > > The MMC core requires PON to be supported by the card and the host to have > MMC_CAP2_FULL_PWR_CYCLE set. Else it will send sleep (CMD5) instead. > You are right - it requires both PON and MMC_CAP2_FULL_PWR_CYCLE to be set... Thanks for mentioning this. > Kind regards > Uffe
On 10 March 2015 at 15:32, Alex Lemberg <Alex.Lemberg@sandisk.com> wrote: > Hi Ulf, > > Thanks for your review and comments! > >> -----Original Message----- >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org] >> Sent: Tuesday, March 10, 2015 5:08 AM >> To: Avi Shchislowski >> Cc: linux-mmc; Chris Ball; Alex Lemberg >> Subject: Re: [PATCH] mmc: sleep notification >> >> On 10 March 2015 at 10:36, Avi Shchislowski <avi.shchislowski@sandisk.com> >> wrote: >> > 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. >> > The max timeout allowed for busy line de-assertion defined in >> > SLEEP_NOTIFICATION_TIME byte in EXT_CSD [216]. >> > HPI may interrupt the SLEEP_NOTIFICATION operation. >> > In that case POWER_OFF_NOTIFICATION byte will restore to POWERED_ON. >> >> Oh, just great! JEDEC has invented yet another way of how to send a device to >> sleep state. I wonder what was wrong with the CMD5 option. > > I think before adding sleep_notification, host has no way to notify device before sending > sleep command. Now device can better prepare itself for moving into the sleep state. > > Also, I think we need to clarify one more point for this patch: > As was mentioned in commit message - Sleep_Notification can be interrupted by HPI. > This allows not blocking the host during the Sleep_Notification busy time and allows accepting > requests coming during this stage. Thus, without having HPI supported, suspend/resume process > might be influenced by Sleep_Notification busy time, and this should not happen - suspend/resume > should be done in very fast and not blocking manner. I fail to understand your comment here. Please tell me at what point(s) your think it make sense to issue the SLEEP_NOTIFICATION? If that is during the suspend phase, then a HPI request can't be triggered. Kind regards Uffe -- 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
SGkgVWxmLA0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IFVsZiBIYW5z c29uIFttYWlsdG86dWxmLmhhbnNzb25AbGluYXJvLm9yZ10NCj4gU2VudDogV2VkbmVzZGF5LCBN YXJjaCAxMSwgMjAxNSA0OjUwIEFNDQo+IFRvOiBBbGV4IExlbWJlcmcNCj4gQ2M6IEF2aSBTaGNo aXNsb3dza2k7IGxpbnV4LW1tYzsgQ2hyaXMgQmFsbA0KPiBTdWJqZWN0OiBSZTogW1BBVENIXSBt bWM6IHNsZWVwIG5vdGlmaWNhdGlvbg0KPiANCj4gT24gMTAgTWFyY2ggMjAxNSBhdCAxNTozMiwg QWxleCBMZW1iZXJnIDxBbGV4LkxlbWJlcmdAc2FuZGlzay5jb20+DQo+IHdyb3RlOg0KPiA+IEhp IFVsZiwNCj4gPg0KPiA+IFRoYW5rcyBmb3IgeW91ciByZXZpZXcgYW5kIGNvbW1lbnRzIQ0KPiA+ DQo+ID4+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4+IEZyb206IFVsZiBIYW5zc29u IFttYWlsdG86dWxmLmhhbnNzb25AbGluYXJvLm9yZ10NCj4gPj4gU2VudDogVHVlc2RheSwgTWFy Y2ggMTAsIDIwMTUgNTowOCBBTQ0KPiA+PiBUbzogQXZpIFNoY2hpc2xvd3NraQ0KPiA+PiBDYzog bGludXgtbW1jOyBDaHJpcyBCYWxsOyBBbGV4IExlbWJlcmcNCj4gPj4gU3ViamVjdDogUmU6IFtQ QVRDSF0gbW1jOiBzbGVlcCBub3RpZmljYXRpb24NCj4gPj4NCj4gPj4gT24gMTAgTWFyY2ggMjAx NSBhdCAxMDozNiwgQXZpIFNoY2hpc2xvd3NraQ0KPiA+PiA8YXZpLnNoY2hpc2xvd3NraUBzYW5k aXNrLmNvbT4NCj4gPj4gd3JvdGU6DQo+ID4+ID4gVGhpcyBwYXRjaCBpcyBpbXBsZW1lbnRzIHRo ZSBuZXcgYWRkaXRpb25hbCBzdGF0ZSBvZg0KPiA+PiA+IFBvd2VyX09mZl9Ob3RpZmljYXRpb24g 4oCTIFNMRUVQX05PVElGSUNBVElPTi4NCj4gPj4gPiBVbnRpbCBub3csIHRoZSBpbXBsZW1lbnRh dGlvbiBvZiBQb3dlcl9PZmZfTm90aWZpY2F0aW9uIHN1cHBvcnRlZA0KPiA+PiA+IG9ubHkgdGhy ZWUgbW9kZXMg4oCTIFBPV0VSRURfT04gKDB4MDEpLCBQT1dFUl9PRkZfU0hPUlQgKDB4MDIpDQo+ IGFuZA0KPiA+PiA+IFBPV0VSX09GRl9MT05HICgweDAzKS4NCj4gPj4gPg0KPiA+PiA+IEFzIHBh cnQgb2YgZU1NQzUuMCBiZWZvcmUgbW92aW5nIHRvIFNsZWVwIHN0YXRlIGhvc3RzIG1heSBzZXQg dGhlDQo+ID4+ID4gUE9XRVJfT0ZGX05PVElGSUNBVElPTiBieXRlIHRvIFNMRUVQX05PVElGSUNB VElPTiAoMHgwNCkuDQo+ID4+ID4gQWZ0ZXIgc2V0dGluZyBTTEVFUF9OT1RJRklDQVRJT04sIGhv c3Qgc2hvdWxkIHdhaXQgZm9yIHRoZSBidXN5DQo+ID4+ID4gbGluZSB0byBiZSBkZS1hc3NlcnRl ZC4NCj4gPj4gPiBUaGUgbWF4IHRpbWVvdXQgYWxsb3dlZCBmb3IgYnVzeSBsaW5lIGRlLWFzc2Vy dGlvbiBkZWZpbmVkIGluDQo+ID4+ID4gU0xFRVBfTk9USUZJQ0FUSU9OX1RJTUUgYnl0ZSBpbiBF WFRfQ1NEIFsyMTZdLg0KPiA+PiA+IEhQSSBtYXkgaW50ZXJydXB0IHRoZSBTTEVFUF9OT1RJRklD QVRJT04gb3BlcmF0aW9uLg0KPiA+PiA+IEluIHRoYXQgY2FzZSBQT1dFUl9PRkZfTk9USUZJQ0FU SU9OIGJ5dGUgd2lsbCByZXN0b3JlIHRvDQo+IFBPV0VSRURfT04uDQo+ID4+DQo+ID4+IE9oLCBq dXN0IGdyZWF0ISBKRURFQyBoYXMgaW52ZW50ZWQgeWV0IGFub3RoZXIgd2F5IG9mIGhvdyB0byBz ZW5kIGENCj4gPj4gZGV2aWNlIHRvIHNsZWVwIHN0YXRlLiBJIHdvbmRlciB3aGF0IHdhcyB3cm9u ZyB3aXRoIHRoZSBDTUQ1IG9wdGlvbi4NCj4gPg0KPiA+IEkgdGhpbmsgYmVmb3JlIGFkZGluZyBz bGVlcF9ub3RpZmljYXRpb24sIGhvc3QgaGFzIG5vIHdheSB0byBub3RpZnkNCj4gPiBkZXZpY2Ug YmVmb3JlIHNlbmRpbmcgc2xlZXAgY29tbWFuZC4gTm93IGRldmljZSBjYW4gYmV0dGVyIHByZXBh cmUgaXRzZWxmDQo+IGZvciBtb3ZpbmcgaW50byB0aGUgc2xlZXAgc3RhdGUuDQo+ID4NCj4gPiBB bHNvLCBJIHRoaW5rIHdlIG5lZWQgdG8gY2xhcmlmeSBvbmUgbW9yZSBwb2ludCBmb3IgdGhpcyBw YXRjaDoNCj4gPiBBcyB3YXMgbWVudGlvbmVkIGluIGNvbW1pdCBtZXNzYWdlIC0gU2xlZXBfTm90 aWZpY2F0aW9uIGNhbiBiZSBpbnRlcnJ1cHRlZA0KPiBieSBIUEkuDQo+ID4gVGhpcyBhbGxvd3Mg bm90IGJsb2NraW5nIHRoZSBob3N0IGR1cmluZyB0aGUgU2xlZXBfTm90aWZpY2F0aW9uIGJ1c3kN Cj4gPiB0aW1lIGFuZCBhbGxvd3MgYWNjZXB0aW5nIHJlcXVlc3RzIGNvbWluZyBkdXJpbmcgdGhp cyBzdGFnZS4gVGh1cywNCj4gPiB3aXRob3V0IGhhdmluZyBIUEkgc3VwcG9ydGVkLCBzdXNwZW5k L3Jlc3VtZSBwcm9jZXNzIG1pZ2h0IGJlDQo+ID4gaW5mbHVlbmNlZCBieSBTbGVlcF9Ob3RpZmlj YXRpb24gYnVzeSB0aW1lLCBhbmQgdGhpcyBzaG91bGQgbm90IGhhcHBlbiAtDQo+IHN1c3BlbmQv cmVzdW1lIHNob3VsZCBiZSBkb25lIGluIHZlcnkgZmFzdCBhbmQgbm90IGJsb2NraW5nIG1hbm5l ci4NCj4gDQo+IEkgZmFpbCB0byB1bmRlcnN0YW5kIHlvdXIgY29tbWVudCBoZXJlLg0KPiANCj4g UGxlYXNlIHRlbGwgbWUgYXQgd2hhdCBwb2ludChzKSB5b3VyIHRoaW5rIGl0IG1ha2Ugc2Vuc2Ug dG8gaXNzdWUgdGhlDQo+IFNMRUVQX05PVElGSUNBVElPTj8gSWYgdGhhdCBpcyBkdXJpbmcgdGhl IHN1c3BlbmQgcGhhc2UsIHRoZW4gYSBIUEkgcmVxdWVzdA0KPiBjYW4ndCBiZSB0cmlnZ2VyZWQu DQoNCkkgdGhpbmsgU0xFRVBfTk9USUZJQ0FUSU9OIHNob3VsZCBiZSBpc3N1ZWQgb24gbW1jX3Bt X25vdGlmeSgpIGNhbGwsDQpvbiBQTV9TVVNQRU5EX1BSRVBBUkUgY2FzZS4NCiANCg0KPiBLaW5k IHJlZ2FyZHMNCj4gVWZmZQ0K -- 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
[...] >> > Also, I think we need to clarify one more point for this patch: >> > As was mentioned in commit message - Sleep_Notification can be interrupted >> by HPI. >> > This allows not blocking the host during the Sleep_Notification busy >> > time and allows accepting requests coming during this stage. Thus, >> > without having HPI supported, suspend/resume process might be >> > influenced by Sleep_Notification busy time, and this should not happen - >> suspend/resume should be done in very fast and not blocking manner. >> >> I fail to understand your comment here. >> >> Please tell me at what point(s) your think it make sense to issue the >> SLEEP_NOTIFICATION? If that is during the suspend phase, then a HPI request >> can't be triggered. > > I think SLEEP_NOTIFICATION should be issued on mmc_pm_notify() call, > on PM_SUSPEND_PREPARE case. So, exactly why is that to prefer, comparing doing it in system PM ->suspend() callback? Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/12/2015 06:09 PM, Ulf Hansson wrote: > [...] > >>>> Also, I think we need to clarify one more point for this patch: >>>> As was mentioned in commit message - Sleep_Notification can be interrupted >>> by HPI. >>>> This allows not blocking the host during the Sleep_Notification busy >>>> time and allows accepting requests coming during this stage. Thus, >>>> without having HPI supported, suspend/resume process might be >>>> influenced by Sleep_Notification busy time, and this should not happen - >>> suspend/resume should be done in very fast and not blocking manner. >>> >>> I fail to understand your comment here. >>> >>> Please tell me at what point(s) your think it make sense to issue the >>> SLEEP_NOTIFICATION? If that is during the suspend phase, then a HPI request >>> can't be triggered. >> >> I think SLEEP_NOTIFICATION should be issued on mmc_pm_notify() call, >> on PM_SUSPEND_PREPARE case. > > So, exactly why is that to prefer, comparing doing it in system PM > ->suspend() callback? Actually, i didn't know the benefit of this feature. Best Regards, Jaehoon Chung > > Kind regards > Uffe > -- > 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 > -- 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
>-----Original Message----- >From: Adrian Hunter [mailto:adrian.hunter@intel.com] >Sent: Tuesday, March 10, 2015 3:36 PM >To: Avi Shchislowski; ulf.hansson@linaro.org >Cc: linux-mmc@vger.kernel.org; chris@printf.net; Alex Lemberg >Subject: Re: [PATCH] mmc: sleep notification > >On 10/03/15 11:36, Avi Shchislowski wrote: >> 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. >> The max timeout allowed for busy line de-assertion defined in >> SLEEP_NOTIFICATION_TIME byte in EXT_CSD [216]. >> HPI may interrupt the SLEEP_NOTIFICATION operation. >> In that case POWER_OFF_NOTIFICATION byte will restore to POWERED_ON. > >Is it so that the point of SLEEP_NOTIFICATION is to notify that VCC will be >powered off after the device is put to sleep by CMD5? The purpose of SLEEP_NOTIFICATION is to give the eMMC enough time to prepare itself before powered off. Before the SLEEP_NOTIFICATION fetcher the eMMC did not know he will get into sleep but only when the sleep was issued With this solution the device start to prepare itself during the sleep preparation > >At the moment, the card does not get put to sleep if there is support for >power-off-notification? No, it will enter to sleep after the device will finish its "internal work" -- 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
SGkgVWxmLA0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IFVsZiBIYW5z c29uIFttYWlsdG86dWxmLmhhbnNzb25AbGluYXJvLm9yZ10NCj4gU2VudDogVGh1cnNkYXksIE1h cmNoIDEyLCAyMDE1IDExOjEwIEFNDQo+IFRvOiBBbGV4IExlbWJlcmcNCj4gQ2M6IEF2aSBTaGNo aXNsb3dza2k7IGxpbnV4LW1tYzsgQ2hyaXMgQmFsbA0KPiBTdWJqZWN0OiBSZTogW1BBVENIXSBt bWM6IHNsZWVwIG5vdGlmaWNhdGlvbg0KPiANCj4gWy4uLl0NCj4gDQo+ID4+ID4gQWxzbywgSSB0 aGluayB3ZSBuZWVkIHRvIGNsYXJpZnkgb25lIG1vcmUgcG9pbnQgZm9yIHRoaXMgcGF0Y2g6DQo+ ID4+ID4gQXMgd2FzIG1lbnRpb25lZCBpbiBjb21taXQgbWVzc2FnZSAtIFNsZWVwX05vdGlmaWNh dGlvbiBjYW4gYmUNCj4gPj4gPiBpbnRlcnJ1cHRlZA0KPiA+PiBieSBIUEkuDQo+ID4+ID4gVGhp cyBhbGxvd3Mgbm90IGJsb2NraW5nIHRoZSBob3N0IGR1cmluZyB0aGUgU2xlZXBfTm90aWZpY2F0 aW9uDQo+ID4+ID4gYnVzeSB0aW1lIGFuZCBhbGxvd3MgYWNjZXB0aW5nIHJlcXVlc3RzIGNvbWlu ZyBkdXJpbmcgdGhpcyBzdGFnZS4NCj4gPj4gPiBUaHVzLCB3aXRob3V0IGhhdmluZyBIUEkgc3Vw cG9ydGVkLCBzdXNwZW5kL3Jlc3VtZSBwcm9jZXNzIG1pZ2h0IGJlDQo+ID4+ID4gaW5mbHVlbmNl ZCBieSBTbGVlcF9Ob3RpZmljYXRpb24gYnVzeSB0aW1lLCBhbmQgdGhpcyBzaG91bGQgbm90DQo+ ID4+ID4gaGFwcGVuIC0NCj4gPj4gc3VzcGVuZC9yZXN1bWUgc2hvdWxkIGJlIGRvbmUgaW4gdmVy eSBmYXN0IGFuZCBub3QgYmxvY2tpbmcgbWFubmVyLg0KPiA+Pg0KPiA+PiBJIGZhaWwgdG8gdW5k ZXJzdGFuZCB5b3VyIGNvbW1lbnQgaGVyZS4NCj4gPj4NCj4gPj4gUGxlYXNlIHRlbGwgbWUgYXQg d2hhdCBwb2ludChzKSB5b3VyIHRoaW5rIGl0IG1ha2Ugc2Vuc2UgdG8gaXNzdWUgdGhlDQo+ID4+ IFNMRUVQX05PVElGSUNBVElPTj8gSWYgdGhhdCBpcyBkdXJpbmcgdGhlIHN1c3BlbmQgcGhhc2Us IHRoZW4gYSBIUEkNCj4gPj4gcmVxdWVzdCBjYW4ndCBiZSB0cmlnZ2VyZWQuDQo+ID4NCj4gPiBJ IHRoaW5rIFNMRUVQX05PVElGSUNBVElPTiBzaG91bGQgYmUgaXNzdWVkIG9uIG1tY19wbV9ub3Rp ZnkoKSBjYWxsLA0KPiA+IG9uIFBNX1NVU1BFTkRfUFJFUEFSRSBjYXNlLg0KPiANCj4gU28sIGV4 YWN0bHkgd2h5IGlzIHRoYXQgdG8gcHJlZmVyLCBjb21wYXJpbmcgZG9pbmcgaXQgaW4gc3lzdGVt IFBNDQo+IC0+c3VzcGVuZCgpIGNhbGxiYWNrPw0KDQpBc3N1bWluZyB0aGF0IFNMRUVQX05PVElG SUNBVElPTiBtYXkgdGFrZSB0aW1lDQooZGVmaW5lZCBpbiBTTEVFUF9OT1RJRklDQVRJT05fVElN RSBieXRlIGluIEVYVF9DU0QgWzIxNl0pLA0KSSB0aGluayBpdCBpcyBiZXR0ZXIgdG8gc2VuZCBp dCBmcm9tIHBtIG5vdGlmaWVyIC0gbW1jX3BtX25vdGlmeSgpLg0KDQo+IA0KPiBLaW5kIHJlZ2Fy ZHMNCj4gVWZmZQ0K -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 16 March 2015 at 12:37, Alex Lemberg <Alex.Lemberg@sandisk.com> wrote: > Hi Ulf, > >> -----Original Message----- >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org] >> Sent: Thursday, March 12, 2015 11:10 AM >> To: Alex Lemberg >> Cc: Avi Shchislowski; linux-mmc; Chris Ball >> Subject: Re: [PATCH] mmc: sleep notification >> >> [...] >> >> >> > Also, I think we need to clarify one more point for this patch: >> >> > As was mentioned in commit message - Sleep_Notification can be >> >> > interrupted >> >> by HPI. >> >> > This allows not blocking the host during the Sleep_Notification >> >> > busy time and allows accepting requests coming during this stage. >> >> > Thus, without having HPI supported, suspend/resume process might be >> >> > influenced by Sleep_Notification busy time, and this should not >> >> > happen - >> >> suspend/resume should be done in very fast and not blocking manner. >> >> >> >> I fail to understand your comment here. >> >> >> >> Please tell me at what point(s) your think it make sense to issue the >> >> SLEEP_NOTIFICATION? If that is during the suspend phase, then a HPI >> >> request can't be triggered. >> > >> > I think SLEEP_NOTIFICATION should be issued on mmc_pm_notify() call, >> > on PM_SUSPEND_PREPARE case. >> >> So, exactly why is that to prefer, comparing doing it in system PM >> ->suspend() callback? > > Assuming that SLEEP_NOTIFICATION may take time > (defined in SLEEP_NOTIFICATION_TIME byte in EXT_CSD [216]), > I think it is better to send it from pm notifier - mmc_pm_notify(). I assume you think that you will "earn" some milliseconds doing it in this phase, but I am not so sure. After the PM_SUSPEND_PREPARE notifier, we still have the mmc block queue open and are ready to serve requests. Therefore I would expect the SLEEP_NOTIFICATION to potentially be interrupted by using HPI. Then we end up with the following sequence during the system PM sleep phase. 1. Issue SLEEP_NOTIFICATION. 2. Interrupt SLEEP_NOTIFICATION, using HPI. 3. Serve blk request 4. Issue SLEEP_NOTIFICATION. 5. Issue SLEEP (CMD5). That seems like a bad idea to me. Kind regards Uffe -- 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
> -----Original Message----- > From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > Sent: Monday, March 16, 2015 3:35 PM > To: Alex Lemberg > Cc: Avi Shchislowski; linux-mmc; Chris Ball > Subject: Re: [PATCH] mmc: sleep notification > > On 16 March 2015 at 12:37, Alex Lemberg <Alex.Lemberg@sandisk.com> > wrote: > > Hi Ulf, > > > >> -----Original Message----- > >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > >> Sent: Thursday, March 12, 2015 11:10 AM > >> To: Alex Lemberg > >> Cc: Avi Shchislowski; linux-mmc; Chris Ball > >> Subject: Re: [PATCH] mmc: sleep notification > >> > >> [...] > >> > >> >> > Also, I think we need to clarify one more point for this patch: > >> >> > As was mentioned in commit message - Sleep_Notification can be > >> >> > interrupted > >> >> by HPI. > >> >> > This allows not blocking the host during the Sleep_Notification > >> >> > busy time and allows accepting requests coming during this stage. > >> >> > Thus, without having HPI supported, suspend/resume process might > >> >> > be influenced by Sleep_Notification busy time, and this should > >> >> > not happen - > >> >> suspend/resume should be done in very fast and not blocking manner. > >> >> > >> >> I fail to understand your comment here. > >> >> > >> >> Please tell me at what point(s) your think it make sense to issue > >> >> the SLEEP_NOTIFICATION? If that is during the suspend phase, then > >> >> a HPI request can't be triggered. > >> > > >> > I think SLEEP_NOTIFICATION should be issued on mmc_pm_notify() > >> > call, on PM_SUSPEND_PREPARE case. > >> > >> So, exactly why is that to prefer, comparing doing it in system PM > >> ->suspend() callback? > > > > Assuming that SLEEP_NOTIFICATION may take time (defined in > > SLEEP_NOTIFICATION_TIME byte in EXT_CSD [216]), I think it is better > > to send it from pm notifier - mmc_pm_notify(). > > I assume you think that you will "earn" some milliseconds doing it in this phase, > but I am not so sure. I was thinking mainly about how to prevent "blocking" during Sleep_Notification process. > > After the PM_SUSPEND_PREPARE notifier, we still have the mmc block queue > open and are ready to serve requests. Therefore I would expect the > SLEEP_NOTIFICATION to potentially be interrupted by using HPI. Right, and this was a reason for adding HPI support during the Sleep_Notification. > > Then we end up with the following sequence during the system PM sleep phase. > 1. Issue SLEEP_NOTIFICATION. > 2. Interrupt SLEEP_NOTIFICATION, using HPI. > 3. Serve blk request > 4. Issue SLEEP_NOTIFICATION. > 5. Issue SLEEP (CMD5). Correct, this is a sequence that we see after adding the patch. > That seems like a bad idea to me. Could you please explain why? Would you suggest to call Sleep_Notification from Suspend only? What if Sleep_Notification will take several seconds? > > Kind regards > Uffe
On 16 March 2015 at 17:58, Alex Lemberg <Alex.Lemberg@sandisk.com> wrote: > > >> -----Original Message----- >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org] >> Sent: Monday, March 16, 2015 3:35 PM >> To: Alex Lemberg >> Cc: Avi Shchislowski; linux-mmc; Chris Ball >> Subject: Re: [PATCH] mmc: sleep notification >> >> On 16 March 2015 at 12:37, Alex Lemberg <Alex.Lemberg@sandisk.com> >> wrote: >> > Hi Ulf, >> > >> >> -----Original Message----- >> >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org] >> >> Sent: Thursday, March 12, 2015 11:10 AM >> >> To: Alex Lemberg >> >> Cc: Avi Shchislowski; linux-mmc; Chris Ball >> >> Subject: Re: [PATCH] mmc: sleep notification >> >> >> >> [...] >> >> >> >> >> > Also, I think we need to clarify one more point for this patch: >> >> >> > As was mentioned in commit message - Sleep_Notification can be >> >> >> > interrupted >> >> >> by HPI. >> >> >> > This allows not blocking the host during the Sleep_Notification >> >> >> > busy time and allows accepting requests coming during this stage. >> >> >> > Thus, without having HPI supported, suspend/resume process might >> >> >> > be influenced by Sleep_Notification busy time, and this should >> >> >> > not happen - >> >> >> suspend/resume should be done in very fast and not blocking manner. >> >> >> >> >> >> I fail to understand your comment here. >> >> >> >> >> >> Please tell me at what point(s) your think it make sense to issue >> >> >> the SLEEP_NOTIFICATION? If that is during the suspend phase, then >> >> >> a HPI request can't be triggered. >> >> > >> >> > I think SLEEP_NOTIFICATION should be issued on mmc_pm_notify() >> >> > call, on PM_SUSPEND_PREPARE case. >> >> >> >> So, exactly why is that to prefer, comparing doing it in system PM >> >> ->suspend() callback? >> > >> > Assuming that SLEEP_NOTIFICATION may take time (defined in >> > SLEEP_NOTIFICATION_TIME byte in EXT_CSD [216]), I think it is better >> > to send it from pm notifier - mmc_pm_notify(). >> >> I assume you think that you will "earn" some milliseconds doing it in this phase, >> but I am not so sure. > > I was thinking mainly about how to prevent "blocking" during Sleep_Notification process. > >> >> After the PM_SUSPEND_PREPARE notifier, we still have the mmc block queue >> open and are ready to serve requests. Therefore I would expect the >> SLEEP_NOTIFICATION to potentially be interrupted by using HPI. > > Right, and this was a reason for adding HPI support during the Sleep_Notification. > >> >> Then we end up with the following sequence during the system PM sleep phase. >> 1. Issue SLEEP_NOTIFICATION. >> 2. Interrupt SLEEP_NOTIFICATION, using HPI. >> 3. Serve blk request >> 4. Issue SLEEP_NOTIFICATION. >> 5. Issue SLEEP (CMD5). > > Correct, this is a sequence that we see after adding the patch. > >> That seems like a bad idea to me. > > Could you please explain why? > Would you suggest to call Sleep_Notification from Suspend only? Since otherwise we may end up issuing SLEEP_NOTIFICATION not only once, but twice during a system PM sleep sequence. So then we actually get an increased system PM suspend time. > What if Sleep_Notification will take several seconds? Yes, that horrible! You should tell your colleagues designing FTLs to make sure that _never_ happens. Also, this makes me wonder how this kind of feature ever could make it into the JEDEC specification. What we had earlier with "CMD5 only" should never have been changed. Kind regards Uffe -- 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
Hi Ulf, Sorry for the delay in response... > -----Original Message----- > From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > Sent: Tuesday, March 17, 2015 12:44 PM > To: Alex Lemberg > Cc: Avi Shchislowski; linux-mmc; Chris Ball > Subject: Re: [PATCH] mmc: sleep notification > > On 16 March 2015 at 17:58, Alex Lemberg <Alex.Lemberg@sandisk.com> > wrote: > > > > > >> -----Original Message----- > >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > >> Sent: Monday, March 16, 2015 3:35 PM > >> To: Alex Lemberg > >> Cc: Avi Shchislowski; linux-mmc; Chris Ball > >> Subject: Re: [PATCH] mmc: sleep notification > >> > >> On 16 March 2015 at 12:37, Alex Lemberg <Alex.Lemberg@sandisk.com> > >> wrote: > >> > Hi Ulf, > >> > > >> >> -----Original Message----- > >> >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > >> >> Sent: Thursday, March 12, 2015 11:10 AM > >> >> To: Alex Lemberg > >> >> Cc: Avi Shchislowski; linux-mmc; Chris Ball > >> >> Subject: Re: [PATCH] mmc: sleep notification > >> >> > >> >> [...] > >> >> > >> >> >> > Also, I think we need to clarify one more point for this patch: > >> >> >> > As was mentioned in commit message - Sleep_Notification can > >> >> >> > be interrupted > >> >> >> by HPI. > >> >> >> > This allows not blocking the host during the > >> >> >> > Sleep_Notification busy time and allows accepting requests coming > during this stage. > >> >> >> > Thus, without having HPI supported, suspend/resume process > >> >> >> > might be influenced by Sleep_Notification busy time, and this > >> >> >> > should not happen - > >> >> >> suspend/resume should be done in very fast and not blocking manner. > >> >> >> > >> >> >> I fail to understand your comment here. > >> >> >> > >> >> >> Please tell me at what point(s) your think it make sense to > >> >> >> issue the SLEEP_NOTIFICATION? If that is during the suspend > >> >> >> phase, then a HPI request can't be triggered. > >> >> > > >> >> > I think SLEEP_NOTIFICATION should be issued on mmc_pm_notify() > >> >> > call, on PM_SUSPEND_PREPARE case. > >> >> > >> >> So, exactly why is that to prefer, comparing doing it in system PM > >> >> ->suspend() callback? > >> > > >> > Assuming that SLEEP_NOTIFICATION may take time (defined in > >> > SLEEP_NOTIFICATION_TIME byte in EXT_CSD [216]), I think it is > >> > better to send it from pm notifier - mmc_pm_notify(). > >> > >> I assume you think that you will "earn" some milliseconds doing it in > >> this phase, but I am not so sure. > > > > I was thinking mainly about how to prevent "blocking" during > Sleep_Notification process. > > > >> > >> After the PM_SUSPEND_PREPARE notifier, we still have the mmc block > >> queue open and are ready to serve requests. Therefore I would expect > >> the SLEEP_NOTIFICATION to potentially be interrupted by using HPI. > > > > Right, and this was a reason for adding HPI support during the > Sleep_Notification. > > > >> > >> Then we end up with the following sequence during the system PM sleep > phase. > >> 1. Issue SLEEP_NOTIFICATION. > >> 2. Interrupt SLEEP_NOTIFICATION, using HPI. > >> 3. Serve blk request > >> 4. Issue SLEEP_NOTIFICATION. > >> 5. Issue SLEEP (CMD5). > > > > Correct, this is a sequence that we see after adding the patch. > > > >> That seems like a bad idea to me. > > > > Could you please explain why? > > Would you suggest to call Sleep_Notification from Suspend only? > > Since otherwise we may end up issuing SLEEP_NOTIFICATION not only once, but > twice during a system PM sleep sequence. So then we actually get an increased > system PM suspend time. > > > What if Sleep_Notification will take several seconds? > > Yes, that horrible! You should tell your colleagues designing FTLs to make sure > that _never_ happens. Agree, but we need to consider and take care of all such cases in the driver side. Device might be busy with its internal garbage collection, and as spec allows, it will complete it in a great manner after host sends PON command. > > Also, this makes me wonder how this kind of feature ever could make it into the > JEDEC specification. > > What we had earlier with "CMD5 only" should never have been changed. > > Kind regards > Uffe
[...] >> > What if Sleep_Notification will take several seconds? >> >> Yes, that horrible! You should tell your colleagues designing FTLs to make sure >> that _never_ happens. > > Agree, but we need to consider and take care of all such cases in the driver side. > Device might be busy with its internal garbage collection, and as spec allows, it > will complete it in a great manner after host sends PON command. > Okay, so to me, I think the SLEEP notification needs to be sent as a part of the system PM suspend phase. And more exactly from the bus_ops->suspend() callback. In addition to that, we should be able improve the situation by also sending the SLEEP notification as part of an "idle operation". With "idle operation" I mean the mmc block layer hasn’t received any new request for a while (the framework for this is already implemented by using runtime PM). So if the SLEEP notification was sent during the "idle operation" and no new request was needed during the system PM suspend phase we wouldn't have to wake up the card again using HPI, but instead just leave it "sleeping" and send CMD5. Still, there would be no guarantees that "idle operations" is executed before a system PM suspend phase starts. So there are no way we can improve the worst case scenario. Kind regards Uffe -- 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
Hi Ulf, > -----Original Message----- > From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > Sent: Friday, March 27, 2015 3:57 PM > To: Alex Lemberg > Cc: Avi Shchislowski; linux-mmc; Chris Ball > Subject: Re: [PATCH] mmc: sleep notification > > [...] > > >> > What if Sleep_Notification will take several seconds? > >> > >> Yes, that horrible! You should tell your colleagues designing FTLs to > >> make sure that _never_ happens. > > > > Agree, but we need to consider and take care of all such cases in the driver > side. > > Device might be busy with its internal garbage collection, and as spec > > allows, it will complete it in a great manner after host sends PON command. > > > > Okay, so to me, I think the SLEEP notification needs to be sent as a part of the > system PM suspend phase. And more exactly from the > bus_ops->suspend() callback. > PM suspend phase is a right place for PON Sleep_Notification, but it also the critical one and need to be completed as fast as possible. If I'm not mistaken, during the PM Suspend, the user space is freezing. Is it OK to let PON Sleep_Notification to be called at that stage? Calling from the notifier_call (mmc_pm_notify()) will allow to complete Sleep_Notification process before entering to the critical part - the Suspend. Isn't it? > In addition to that, we should be able improve the situation by also sending the > SLEEP notification as part of an "idle operation". With "idle operation" I mean > the mmc block layer hasn’t received any new request for a while (the > framework for this is already implemented by using runtime PM). OK, sounds like a good approach. In case runtime PM is supported, we can call PON Sleep_Notification from mmc_runtime_suspend() during the "idle operation", and then call it again during PM Suspend. > > So if the SLEEP notification was sent during the "idle operation" and no new > request was needed during the system PM suspend phase we wouldn't have to > wake up the card again using HPI, but instead just leave it "sleeping" and send > CMD5. > > Still, there would be no guarantees that "idle operations" is executed before a > system PM suspend phase starts. So there are no way we can improve the > worst case scenario. > > Kind regards > Uffe
[...] >> Okay, so to me, I think the SLEEP notification needs to be sent as a part of the >> system PM suspend phase. And more exactly from the >> bus_ops->suspend() callback. >> > > PM suspend phase is a right place for PON Sleep_Notification, > but it also the critical one and need to be completed as fast as possible. > > If I'm not mistaken, during the PM Suspend, the user space is freezing. > Is it OK to let PON Sleep_Notification to be called at that stage? Of course we shall strive in minimizing the system PM suspend time. Though to me, I think the system PM _resume_ time is of more importance. In this case we don't have any option. We need to issue the sleep notification as part of the the system PM suspend phase and that will unfortunate and likely increase the total system PM suspend time. > > Calling from the notifier_call (mmc_pm_notify()) will allow to complete > Sleep_Notification process before entering to the critical part - the Suspend. > Isn't it? Why is mmc_pm_notify() a less critical part? More importantly as stated several times, we can still get I/O requests after the mmc_pm_notify() phase and then the sleep notification needs to be interrupted and re-issued anyway. > >> In addition to that, we should be able improve the situation by also sending the >> SLEEP notification as part of an "idle operation". With "idle operation" I mean >> the mmc block layer hasn’t received any new request for a while (the >> framework for this is already implemented by using runtime PM). > > OK, sounds like a good approach. > In case runtime PM is supported, we can call PON Sleep_Notification from > mmc_runtime_suspend() during the "idle operation", > and then call it again during PM Suspend. Yes. If you plan to send a new version of $subject patch, please split into at least two patches. One which adds the system PM part and one which adds the "idle operation" part. Kind regards Uffe -- 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
Hi Ulf, > -----Original Message----- > From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > Sent: Wednesday, April 01, 2015 2:46 PM > To: Alex Lemberg > Cc: Avi Shchislowski; linux-mmc; Chris Ball > Subject: Re: [PATCH] mmc: sleep notification > > [...] > > >> Okay, so to me, I think the SLEEP notification needs to be sent as a > >> part of the system PM suspend phase. And more exactly from the > >> bus_ops->suspend() callback. > >> > > > > PM suspend phase is a right place for PON Sleep_Notification, but it > > also the critical one and need to be completed as fast as possible. > > > > If I'm not mistaken, during the PM Suspend, the user space is freezing. > > Is it OK to let PON Sleep_Notification to be called at that stage? > > Of course we shall strive in minimizing the system PM suspend time. > Though to me, I think the system PM _resume_ time is of more importance. > > In this case we don't have any option. We need to issue the sleep notification as > part of the the system PM suspend phase and that will unfortunate and likely > increase the total system PM suspend time. > > > > > Calling from the notifier_call (mmc_pm_notify()) will allow to > > complete Sleep_Notification process before entering to the critical part - the > Suspend. > > Isn't it? > > Why is mmc_pm_notify() a less critical part? > > More importantly as stated several times, we can still get I/O requests after > the mmc_pm_notify() phase and then the sleep notification needs to be > interrupted and re-issued anyway. You are right - the sleep_notification might be re-issued several times, but in case of mmc_pm_notify() the user space is not frozen yet, and system will not stuck until sleep_notification completion, like in case of PM suspend. > > > > >> In addition to that, we should be able improve the situation by also > >> sending the SLEEP notification as part of an "idle operation". With > >> "idle operation" I mean the mmc block layer hasn’t received any new > >> request for a while (the framework for this is already implemented by using > runtime PM). > > > > OK, sounds like a good approach. > > In case runtime PM is supported, we can call PON Sleep_Notification > > from > > mmc_runtime_suspend() during the "idle operation", and then call it > > again during PM Suspend. > > Yes. > > If you plan to send a new version of $subject patch, please split into at least two > patches. One which adds the system PM part and one which adds the "idle > operation" part. As far as I see, the same _mmc_suspend() function is used in both cases - PM Suspend and "idle operation". The _mmc_suspend() is called from both - mmc_suspend() and mmc_runtime_suspend(). In case we decide to go with PM Suspend and "idle operation", should we just add the sleep_notification support to _mmc_suspend() for covering both cases? > > Kind regards > Uffe
On 6 April 2015 at 15:44, Alex Lemberg <Alex.Lemberg@sandisk.com> wrote: > Hi Ulf, > >> -----Original Message----- >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org] >> Sent: Wednesday, April 01, 2015 2:46 PM >> To: Alex Lemberg >> Cc: Avi Shchislowski; linux-mmc; Chris Ball >> Subject: Re: [PATCH] mmc: sleep notification >> >> [...] >> >> >> Okay, so to me, I think the SLEEP notification needs to be sent as a >> >> part of the system PM suspend phase. And more exactly from the >> >> bus_ops->suspend() callback. >> >> >> > >> > PM suspend phase is a right place for PON Sleep_Notification, but it >> > also the critical one and need to be completed as fast as possible. >> > >> > If I'm not mistaken, during the PM Suspend, the user space is freezing. >> > Is it OK to let PON Sleep_Notification to be called at that stage? >> >> Of course we shall strive in minimizing the system PM suspend time. >> Though to me, I think the system PM _resume_ time is of more importance. >> >> In this case we don't have any option. We need to issue the sleep notification as >> part of the the system PM suspend phase and that will unfortunate and likely >> increase the total system PM suspend time. >> >> > >> > Calling from the notifier_call (mmc_pm_notify()) will allow to >> > complete Sleep_Notification process before entering to the critical part - the >> Suspend. >> > Isn't it? >> >> Why is mmc_pm_notify() a less critical part? >> >> More importantly as stated several times, we can still get I/O requests after >> the mmc_pm_notify() phase and then the sleep notification needs to be >> interrupted and re-issued anyway. > > You are right - the sleep_notification might be re-issued several times, > but in case of mmc_pm_notify() the user space is not frozen yet, and > system will not stuck until sleep_notification completion, like in case of > PM suspend. > >> >> > >> >> In addition to that, we should be able improve the situation by also >> >> sending the SLEEP notification as part of an "idle operation". With >> >> "idle operation" I mean the mmc block layer hasn’t received any new >> >> request for a while (the framework for this is already implemented by using >> runtime PM). >> > >> > OK, sounds like a good approach. >> > In case runtime PM is supported, we can call PON Sleep_Notification >> > from >> > mmc_runtime_suspend() during the "idle operation", and then call it >> > again during PM Suspend. >> >> Yes. >> >> If you plan to send a new version of $subject patch, please split into at least two >> patches. One which adds the system PM part and one which adds the "idle >> operation" part. > > As far as I see, the same _mmc_suspend() function is used in both cases - > PM Suspend and "idle operation". > The _mmc_suspend() is called from both - mmc_suspend() and > mmc_runtime_suspend(). > In case we decide to go with PM Suspend and "idle operation", > should we just add the sleep_notification support to _mmc_suspend() for > covering both cases? That works as a start. Though, it would mean MMC_CAP_AGGRESSIVE_PM is needed to enable "idle operations" for this case. It might be good enough. If not, we need to send the sleep notification from mmc_runtime_suspend() as well. Kind regards Uffe -- 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
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c index 4409d79..f511ecc3 100644 --- a/drivers/mmc/card/block.c +++ b/drivers/mmc/card/block.c @@ -2010,9 +2010,26 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req) unsigned long flags; unsigned int cmd_flags = req ? req->cmd_flags : 0; - if (req && !mq->mqrq_prev->req) + if (unlikely(req && mmc_card_mmc(card) && + (card->ext_csd.power_off_notification == + EXT_CSD_SLEEP_NOTIFICATION))) { + /* restoring the power_off_notification + * field's state to as it was before so + * that the sleep notification will be + * able to resume later + */ + card->ext_csd.power_off_notification = EXT_CSD_POWER_ON; + } + + if (req && !mq->mqrq_prev->req) { /* claim host only for the first request */ mmc_get_card(card); + if (unlikely(req && + mmc_card_doing_sleep_notify(card))) { + mmc_interrupt_hpi(card); + mmc_card_clr_sleep_notify(card); + } + } ret = mmc_blk_part_switch(card, md); if (ret) { diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 5bda29b..a090593 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -271,7 +271,8 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception) BUG_ON(!card); - if (!card->ext_csd.bkops_en || mmc_card_doing_bkops(card)) + if (!card->ext_csd.bkops_en || mmc_card_doing_bkops(card) || + mmc_card_doing_sleep_notify(card)) return; err = mmc_read_bkops_status(card); @@ -2630,6 +2631,19 @@ int mmc_pm_notify(struct notifier_block *notify_block, err = host->bus_ops->pre_suspend(host); if (!err) break; + if (host->card && host->bus_ops->suspend) { + err = mmc_sleep_notify(host->card); + /* We assume that HPI was sent + * in case of -ETIMEDOUT error, + * so suspend flow can be continued + */ + if (err && err != -ETIMEDOUT) { + pr_err("%s:sleep notify err=%d\n", + __func__, err); + return -EBUSY; + } + break; + } /* Calling bus_ops->remove() with a claimed host can deadlock */ host->bus_ops->remove(host); diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index d76597c..b6b4431 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -14,6 +14,7 @@ #include <linux/delay.h> #define MMC_CMD_RETRIES 3 +#define MMC_SLEEP_NOTIFY_MAX_TIME 0x17 struct mmc_bus_ops { void (*remove)(struct mmc_host *); @@ -33,6 +34,7 @@ void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops); void mmc_detach_bus(struct mmc_host *host); void mmc_init_erase(struct mmc_card *card); +int mmc_sleep_notify(struct mmc_card *card); void mmc_set_chip_select(struct mmc_host *host, int mode); void mmc_set_clock(struct mmc_host *host, unsigned int hz); diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 02ad792..1d97d24 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -57,6 +57,11 @@ static const unsigned int tacc_mant[] = { __res & __mask; \ }) +#define GET_SLEEP_NOTIFY_TIME(value) \ + (10 * (1 << (unsigned int)(value))) +#define GET_SLEEP_NOTIFY_TIME_MSEC(value) \ + (DIV_ROUND_UP(GET_SLEEP_NOTIFY_TIME(value), 1000)) + /* * Given the decoded CSD structure, decode the raw CID to our CID structure. */ @@ -571,6 +576,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; @@ -1468,6 +1475,15 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, card->ext_csd.hpi_en = 1; } + /* sleep notify enable/disable for eMMC 5.0 and above */ + if ((card->ext_csd.rev >= 7) && card->ext_csd.hpi_en && + (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->can_sleep_notify = 1; + } + /* * If cache size is higher than 0, this indicates * the existence of cache and it can be turned on. @@ -1576,6 +1592,33 @@ static int mmc_sleep(struct mmc_host *host) 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 mmc_can_poweroff_notify(const struct mmc_card *card) { return card && @@ -1585,22 +1628,108 @@ static int mmc_can_poweroff_notify(const struct mmc_card *card) static int mmc_poweroff_notify(struct mmc_card *card, unsigned int notify_type) { - unsigned int timeout = card->ext_csd.generic_cmd6_time; + unsigned int timeout_ms = 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; + timeout_ms = 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_ms = GET_SLEEP_NOTIFY_TIME_MSEC( + card->ext_csd.sleep_notify_time); + } + /* do not wait on busy signal in case of + * Sleep Notification - to let host get + * another requests + */ + 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_ms, + 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_ms); + } + + return err; +} - /* Disable the power off notification after the switch operation. */ - card->ext_csd.power_off_notification = EXT_CSD_NO_POWER_NOTIFICATION; +int mmc_sleep_notify(struct mmc_card *card) +{ + int err = 0; + bool is_busy = 0; + unsigned long prg_wait = 0; + + if (!card->can_sleep_notify || !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; + } + + prg_wait = jiffies + + msecs_to_jiffies(GET_SLEEP_NOTIFY_TIME_MSEC( + card->ext_csd.sleep_notify_time)); + } + + /* + * Loop will run until: + * 1. Device is no more in Busy state + * 2. Sleep notification is not interrupted by HPI & IO request + */ + do { + /* added some delay to avoid sending card status too often */ + msleep(20); + err = 0; + /* Stop polling in case sleep notification was HPIed already */ + if (!mmc_card_doing_sleep_notify(card)) { + is_busy = mmc_device_prg_state(card); + if (is_busy) + err = -EBUSY; + break; + } + 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; } diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index 4d69c00..6998344 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 */ @@ -309,6 +311,7 @@ struct mmc_card { struct dentry *debugfs_root; struct mmc_part part[MMC_NUM_PHY_PARTITION]; /* physical partitions */ unsigned int nr_parts; + u8 can_sleep_notify; /* sleep_notify on/off */ }; /* @@ -427,6 +430,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 +441,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 9f32270..4c0542a 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -291,6 +291,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 49ad7a9..69bda9a 100644 --- a/include/linux/mmc/mmc.h +++ b/include/linux/mmc/mmc.h @@ -314,6 +314,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 */ @@ -408,6 +409,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 */