2022-04-26 20:05:23

by Srinivasarao Pathipati

[permalink] [raw]
Subject: [PATCH V1] mmc: core: Add partial initialization support

From: Maya Erez <[email protected]>

This change adds the ability to partially initialize
the MMC card by using card Sleep/Awake sequence (CMD5).
Card will be sent to Sleep state during runtime/system suspend
and will be woken up during runtime/system resume.
By using this sequence the card doesn't need full initialization
which gives time reduction in system/runtime resume path.

Signed-off-by: Maya Erez <[email protected]>
Signed-off-by: Veerabhadrarao Badiganti <[email protected]>
Signed-off-by: Shaik Sajida Bhanu <[email protected]>
Signed-off-by: kamasali <[email protected]>
Signed-off-by: Srinivasarao Pathipati <[email protected]>
---
drivers/mmc/core/mmc.c | 149 ++++++++++++++++++++++++++++++++++++++++++++---
include/linux/mmc/card.h | 4 ++
include/linux/mmc/host.h | 2 +
3 files changed, 146 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 9ab915b..8691c00 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1942,7 +1942,14 @@ static int mmc_sleep_busy_cb(void *cb_data, bool *busy)
return 0;
}

-static int mmc_sleep(struct mmc_host *host)
+static int mmc_can_sleepawake(struct mmc_host *host)
+{
+ return host && (host->caps2 & MMC_CAP2_SLEEP_AWAKE) && host->card &&
+ (host->card->ext_csd.rev >= 3);
+
+}
+
+static int mmc_sleepawake(struct mmc_host *host, bool sleep)
{
struct mmc_command cmd = {};
struct mmc_card *card = host->card;
@@ -1953,14 +1960,17 @@ static int mmc_sleep(struct mmc_host *host)
/* Re-tuning can't be done once the card is deselected */
mmc_retune_hold(host);

- err = mmc_deselect_cards(host);
- if (err)
- goto out_release;
+ if (sleep) {
+ err = mmc_deselect_cards(host);
+ if (err)
+ goto out_release;
+ }

cmd.opcode = MMC_SLEEP_AWAKE;
cmd.arg = card->rca << 16;
- cmd.arg |= 1 << 15;
use_r1b_resp = mmc_prepare_busy_cmd(host, &cmd, timeout_ms);
+ if (sleep)
+ cmd.arg |= 1 << 15;

err = mmc_wait_for_cmd(host, &cmd, 0);
if (err)
@@ -1982,6 +1992,9 @@ static int mmc_sleep(struct mmc_host *host)

err = __mmc_poll_for_busy(host, 0, timeout_ms, &mmc_sleep_busy_cb, host);

+ if (!sleep)
+ err = mmc_select_card(card);
+
out_release:
mmc_retune_release(host);
return err;
@@ -2080,6 +2093,66 @@ static int _mmc_flush_cache(struct mmc_host *host)
pr_err("%s: cache flush error %d\n",
mmc_hostname(host), err);
}
+ return err;
+}
+
+static int mmc_cache_card_ext_csd(struct mmc_host *host)
+{
+ int err;
+ u8 *ext_csd;
+ struct mmc_card *card = host->card;
+
+ err = mmc_get_ext_csd(card, &ext_csd);
+ if (err || !ext_csd) {
+ pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
+ mmc_hostname(host), __func__, err);
+ return err;
+ }
+ /* only cache read/write fields that the sw changes */
+ card->ext_csd.raw_ext_csd_cmdq = ext_csd[EXT_CSD_CMDQ_MODE_EN];
+ card->ext_csd.raw_ext_csd_cache_ctrl = ext_csd[EXT_CSD_CACHE_CTRL];
+ card->ext_csd.raw_ext_csd_bus_width = ext_csd[EXT_CSD_BUS_WIDTH];
+ card->ext_csd.raw_ext_csd_hs_timing = ext_csd[EXT_CSD_HS_TIMING];
+
+ kfree(ext_csd);
+
+ return 0;
+}
+
+static int mmc_test_awake_ext_csd(struct mmc_host *host)
+{
+ int err;
+ u8 *ext_csd;
+ struct mmc_card *card = host->card;
+
+ err = mmc_get_ext_csd(card, &ext_csd);
+ if (err) {
+ pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
+ mmc_hostname(host), __func__, err);
+ return err;
+ }
+
+ /* only compare read/write fields that the sw changes */
+ pr_debug("%s: %s: type(cached:current) cmdq(%d:%d) cache_ctrl(%d:%d) bus_width (%d:%d) timing(%d:%d)\n",
+ mmc_hostname(host), __func__,
+ card->ext_csd.raw_ext_csd_cmdq,
+ ext_csd[EXT_CSD_CMDQ_MODE_EN],
+ card->ext_csd.raw_ext_csd_cache_ctrl,
+ ext_csd[EXT_CSD_CACHE_CTRL],
+ card->ext_csd.raw_ext_csd_bus_width,
+ ext_csd[EXT_CSD_BUS_WIDTH],
+ card->ext_csd.raw_ext_csd_hs_timing,
+ ext_csd[EXT_CSD_HS_TIMING]);
+ err = !((card->ext_csd.raw_ext_csd_cmdq ==
+ ext_csd[EXT_CSD_CMDQ_MODE_EN]) &&
+ (card->ext_csd.raw_ext_csd_cache_ctrl ==
+ ext_csd[EXT_CSD_CACHE_CTRL]) &&
+ (card->ext_csd.raw_ext_csd_bus_width ==
+ ext_csd[EXT_CSD_BUS_WIDTH]) &&
+ (card->ext_csd.raw_ext_csd_hs_timing ==
+ ext_csd[EXT_CSD_HS_TIMING]));
+
+ kfree(ext_csd);

return err;
}
@@ -2103,8 +2176,12 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend ||
(host->caps2 & MMC_CAP2_FULL_PWR_CYCLE_IN_SUSPEND)))
err = mmc_poweroff_notify(host->card, notify_type);
- else if (mmc_can_sleep(host->card))
- err = mmc_sleep(host);
+ if (mmc_can_sleepawake(host)) {
+ memcpy(&host->cached_ios, &host->ios, sizeof(host->cached_ios));
+ mmc_cache_card_ext_csd(host);
+ }
+ if (mmc_can_sleep(host->card))
+ err = mmc_sleepawake(host, true);
else if (!mmc_host_is_spi(host))
err = mmc_deselect_cards(host);

@@ -2117,6 +2194,48 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
return err;
}

+static int mmc_partial_init(struct mmc_host *host)
+{
+ int err = 0;
+ struct mmc_card *card = host->card;
+
+ mmc_set_bus_width(host, host->cached_ios.bus_width);
+ mmc_set_timing(host, host->cached_ios.timing);
+ if (host->cached_ios.enhanced_strobe) {
+ host->ios.enhanced_strobe = true;
+ if (host->ops->hs400_enhanced_strobe)
+ host->ops->hs400_enhanced_strobe(host, &host->ios);
+ }
+ mmc_set_clock(host, host->cached_ios.clock);
+ mmc_set_bus_mode(host, host->cached_ios.bus_mode);
+
+ if (!mmc_card_hs400es(card) &&
+ (mmc_card_hs200(card) || mmc_card_hs400(card))) {
+ err = mmc_execute_tuning(card);
+ if (err) {
+ pr_err("%s: %s: Tuning failed (%d)\n",
+ mmc_hostname(host), __func__, err);
+ goto out;
+ }
+ }
+ /*
+ * The ext_csd is read to make sure the card did not went through
+ * Power-failure during sleep period.
+ * A subset of the W/E_P, W/C_P register will be tested. In case
+ * these registers values are different from the values that were
+ * cached during suspend, we will conclude that a Power-failure occurred
+ * and will do full initialization sequence.
+ */
+ err = mmc_test_awake_ext_csd(host);
+ if (err) {
+ pr_debug("%s: %s: fail on ext_csd read (%d)\n",
+ mmc_hostname(host), __func__, err);
+ goto out;
+ }
+out:
+ return err;
+}
+
/*
* Suspend callback
*/
@@ -2139,7 +2258,7 @@ static int mmc_suspend(struct mmc_host *host)
*/
static int _mmc_resume(struct mmc_host *host)
{
- int err = 0;
+ int err = -EINVAL;

mmc_claim_host(host);

@@ -2147,7 +2266,19 @@ static int _mmc_resume(struct mmc_host *host)
goto out;

mmc_power_up(host, host->card->ocr);
- err = mmc_init_card(host, host->card->ocr, host->card);
+
+ if (mmc_can_sleepawake(host)) {
+ err = mmc_sleepawake(host, false);
+ if (!err)
+ err = mmc_partial_init(host);
+ else
+ pr_err("%s: %s: awake failed (%d), fallback to full init\n",
+ mmc_hostname(host), __func__, err);
+ }
+
+ if (err)
+ err = mmc_init_card(host, host->card->ocr, host->card);
+
mmc_card_clr_suspended(host->card);

out:
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 37f9758..ed7f6f7 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -86,6 +86,8 @@ struct mmc_ext_csd {
unsigned int data_tag_unit_size; /* DATA TAG UNIT size */
unsigned int boot_ro_lock; /* ro lock support */
bool boot_ro_lockable;
+ u8 raw_ext_csd_cmdq; /* 15 */
+ u8 raw_ext_csd_cache_ctrl; /* 33 */
bool ffu_capable; /* Firmware upgrade support */
bool cmdq_en; /* Command Queue enabled */
bool cmdq_support; /* Command Queue supported */
@@ -96,7 +98,9 @@ struct mmc_ext_csd {
u8 raw_partition_support; /* 160 */
u8 raw_rpmb_size_mult; /* 168 */
u8 raw_erased_mem_count; /* 181 */
+ u8 raw_ext_csd_bus_width; /* 183 */
u8 strobe_support; /* 184 */
+ u8 raw_ext_csd_hs_timing; /* 185 */
u8 raw_ext_csd_structure; /* 194 */
u8 raw_card_type; /* 196 */
u8 raw_driver_strength; /* 197 */
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index c38072e..a9ddf7a 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -422,6 +422,7 @@ struct mmc_host {
#else
#define MMC_CAP2_CRYPTO 0
#endif
+#define MMC_CAP2_SLEEP_AWAKE (1 << 29) /* Use Sleep/Awake (CMD5) */
#define MMC_CAP2_ALT_GPT_TEGRA (1 << 28) /* Host with eMMC that has GPT entry at a non-standard location */

int fixed_drv_type; /* fixed driver type for non-removable media */
@@ -441,6 +442,7 @@ struct mmc_host {
spinlock_t lock; /* lock for claim and bus ops */

struct mmc_ios ios; /* current io bus settings */
+ struct mmc_ios cached_ios;

/* group bitfields together to minimize padding */
unsigned int use_spi_crc:1;
--
2.7.4


2022-04-27 09:03:43

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH V1] mmc: core: Add partial initialization support

> From: Maya Erez <[email protected]>
>
> This change adds the ability to partially initialize
> the MMC card by using card Sleep/Awake sequence (CMD5).
> Card will be sent to Sleep state during runtime/system suspend
> and will be woken up during runtime/system resume.
> By using this sequence the card doesn't need full initialization
> which gives time reduction in system/runtime resume path.
>
> Signed-off-by: Maya Erez <[email protected]>
> Signed-off-by: Veerabhadrarao Badiganti <[email protected]>
> Signed-off-by: Shaik Sajida Bhanu <[email protected]>
> Signed-off-by: kamasali <[email protected]>
> Signed-off-by: Srinivasarao Pathipati <[email protected]>
> ---
> drivers/mmc/core/mmc.c | 149
> ++++++++++++++++++++++++++++++++++++++++++++---
> include/linux/mmc/card.h | 4 ++
> include/linux/mmc/host.h | 2 +
> 3 files changed, 146 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 9ab915b..8691c00 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1942,7 +1942,14 @@ static int mmc_sleep_busy_cb(void *cb_data, bool
> *busy)
> return 0;
> }
>
> -static int mmc_sleep(struct mmc_host *host)
> +static int mmc_can_sleepawake(struct mmc_host *host)
> +{
> + return host && (host->caps2 & MMC_CAP2_SLEEP_AWAKE) && host-
> >card &&
> + (host->card->ext_csd.rev >= 3);
Maybe just:
return mmc_can_sleep(host->card) && (host->caps2 & MMC_CAP2_SLEEP_AWAKE);

> +
> +}
> +
> +static int mmc_sleepawake(struct mmc_host *host, bool sleep)
> {
> struct mmc_command cmd = {};
> struct mmc_card *card = host->card;
> @@ -1953,14 +1960,17 @@ static int mmc_sleep(struct mmc_host *host)
> /* Re-tuning can't be done once the card is deselected */
> mmc_retune_hold(host);
>
> - err = mmc_deselect_cards(host);
> - if (err)
> - goto out_release;
> + if (sleep) {
> + err = mmc_deselect_cards(host);
> + if (err)
> + goto out_release;
> + }
>
> cmd.opcode = MMC_SLEEP_AWAKE;
> cmd.arg = card->rca << 16;
> - cmd.arg |= 1 << 15;
> use_r1b_resp = mmc_prepare_busy_cmd(host, &cmd, timeout_ms);
> + if (sleep)
> + cmd.arg |= 1 << 15;
>
> err = mmc_wait_for_cmd(host, &cmd, 0);
> if (err)
> @@ -1982,6 +1992,9 @@ static int mmc_sleep(struct mmc_host *host)
>
> err = __mmc_poll_for_busy(host, 0, timeout_ms, &mmc_sleep_busy_cb,
> host);
>
> + if (!sleep)
> + err = mmc_select_card(card);
> +
> out_release:
> mmc_retune_release(host);
> return err;
> @@ -2080,6 +2093,66 @@ static int _mmc_flush_cache(struct mmc_host
> *host)
> pr_err("%s: cache flush error %d\n",
> mmc_hostname(host), err);
> }
> + return err;
> +}
> +
> +static int mmc_cache_card_ext_csd(struct mmc_host *host)
> +{
> + int err;
> + u8 *ext_csd;
> + struct mmc_card *card = host->card;
> +
> + err = mmc_get_ext_csd(card, &ext_csd);
> + if (err || !ext_csd) {
> + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
> + mmc_hostname(host), __func__, err);
> + return err;
> + }
> + /* only cache read/write fields that the sw changes */
> + card->ext_csd.raw_ext_csd_cmdq = ext_csd[EXT_CSD_CMDQ_MODE_EN];
> + card->ext_csd.raw_ext_csd_cache_ctrl = ext_csd[EXT_CSD_CACHE_CTRL];
> + card->ext_csd.raw_ext_csd_bus_width = ext_csd[EXT_CSD_BUS_WIDTH];
> + card->ext_csd.raw_ext_csd_hs_timing = ext_csd[EXT_CSD_HS_TIMING];
> +
> + kfree(ext_csd);
> +
> + return 0;
> +}
> +
> +static int mmc_test_awake_ext_csd(struct mmc_host *host)
> +{
> + int err;
> + u8 *ext_csd;
> + struct mmc_card *card = host->card;
> +
> + err = mmc_get_ext_csd(card, &ext_csd);
> + if (err) {
> + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
> + mmc_hostname(host), __func__, err);
> + return err;
> + }
> +
> + /* only compare read/write fields that the sw changes */
> + pr_debug("%s: %s: type(cached:current) cmdq(%d:%d) cache_ctrl(%d:%d)
> bus_width (%d:%d) timing(%d:%d)\n",
> + mmc_hostname(host), __func__,
> + card->ext_csd.raw_ext_csd_cmdq,
> + ext_csd[EXT_CSD_CMDQ_MODE_EN],
> + card->ext_csd.raw_ext_csd_cache_ctrl,
> + ext_csd[EXT_CSD_CACHE_CTRL],
> + card->ext_csd.raw_ext_csd_bus_width,
> + ext_csd[EXT_CSD_BUS_WIDTH],
> + card->ext_csd.raw_ext_csd_hs_timing,
> + ext_csd[EXT_CSD_HS_TIMING]);
> + err = !((card->ext_csd.raw_ext_csd_cmdq ==
> + ext_csd[EXT_CSD_CMDQ_MODE_EN]) &&
> + (card->ext_csd.raw_ext_csd_cache_ctrl ==
> + ext_csd[EXT_CSD_CACHE_CTRL]) &&
> + (card->ext_csd.raw_ext_csd_bus_width ==
> + ext_csd[EXT_CSD_BUS_WIDTH]) &&
> + (card->ext_csd.raw_ext_csd_hs_timing ==
> + ext_csd[EXT_CSD_HS_TIMING]));
> +
> + kfree(ext_csd);
>
> return err;
> }
> @@ -2103,8 +2176,12 @@ static int _mmc_suspend(struct mmc_host *host,
> bool is_suspend)
> ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend ||
> (host->caps2 & MMC_CAP2_FULL_PWR_CYCLE_IN_SUSPEND)))
> err = mmc_poweroff_notify(host->card, notify_type);
> - else if (mmc_can_sleep(host->card))
> - err = mmc_sleep(host);
> + if (mmc_can_sleepawake(host)) {
> + memcpy(&host->cached_ios, &host->ios, sizeof(host->cached_ios));
> + mmc_cache_card_ext_csd(host);
> + }
> + if (mmc_can_sleep(host->card))
> + err = mmc_sleepawake(host, true);
> else if (!mmc_host_is_spi(host))
> err = mmc_deselect_cards(host);
>
> @@ -2117,6 +2194,48 @@ static int _mmc_suspend(struct mmc_host *host,
> bool is_suspend)
> return err;
> }
>
> +static int mmc_partial_init(struct mmc_host *host)
> +{
> + int err = 0;
> + struct mmc_card *card = host->card;
> +
> + mmc_set_bus_width(host, host->cached_ios.bus_width);
> + mmc_set_timing(host, host->cached_ios.timing);
> + if (host->cached_ios.enhanced_strobe) {
> + host->ios.enhanced_strobe = true;
> + if (host->ops->hs400_enhanced_strobe)
> + host->ops->hs400_enhanced_strobe(host, &host->ios);
> + }
> + mmc_set_clock(host, host->cached_ios.clock);
> + mmc_set_bus_mode(host, host->cached_ios.bus_mode);
> +
> + if (!mmc_card_hs400es(card) &&
> + (mmc_card_hs200(card) || mmc_card_hs400(card))) {
> + err = mmc_execute_tuning(card);
> + if (err) {
> + pr_err("%s: %s: Tuning failed (%d)\n",
> + mmc_hostname(host), __func__, err);
> + goto out;
> + }
> + }
> + /*
> + * The ext_csd is read to make sure the card did not went through
> + * Power-failure during sleep period.
> + * A subset of the W/E_P, W/C_P register will be tested. In case
> + * these registers values are different from the values that were
> + * cached during suspend, we will conclude that a Power-failure occurred
> + * and will do full initialization sequence.
> + */
This seems like a strange enough hack.
Power-failure during sleep period? really?

> + err = mmc_test_awake_ext_csd(host);
> + if (err) {
> + pr_debug("%s: %s: fail on ext_csd read (%d)\n",
> + mmc_hostname(host), __func__, err);
> + goto out;
Redundant goto

> + }
> +out:
> + return err;
> +}

Thanks,
Avri

> +
> /*
> * Suspend callback
> */
> @@ -2139,7 +2258,7 @@ static int mmc_suspend(struct mmc_host *host)
> */
> static int _mmc_resume(struct mmc_host *host)
> {
> - int err = 0;
> + int err = -EINVAL;
>
> mmc_claim_host(host);
>
> @@ -2147,7 +2266,19 @@ static int _mmc_resume(struct mmc_host *host)
> goto out;
>
> mmc_power_up(host, host->card->ocr);
> - err = mmc_init_card(host, host->card->ocr, host->card);
> +
> + if (mmc_can_sleepawake(host)) {
> + err = mmc_sleepawake(host, false);
> + if (!err)
> + err = mmc_partial_init(host);
> + else
> + pr_err("%s: %s: awake failed (%d), fallback to full init\n",
> + mmc_hostname(host), __func__, err);
> + }
> +
> + if (err)
> + err = mmc_init_card(host, host->card->ocr, host->card);
> +
> mmc_card_clr_suspended(host->card);
>
> out:
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 37f9758..ed7f6f7 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -86,6 +86,8 @@ struct mmc_ext_csd {
> unsigned int data_tag_unit_size; /* DATA TAG UNIT size */
> unsigned int boot_ro_lock; /* ro lock support */
> bool boot_ro_lockable;
> + u8 raw_ext_csd_cmdq; /* 15 */
> + u8 raw_ext_csd_cache_ctrl; /* 33 */
> bool ffu_capable; /* Firmware upgrade support */
> bool cmdq_en; /* Command Queue enabled */
> bool cmdq_support; /* Command Queue supported */
> @@ -96,7 +98,9 @@ struct mmc_ext_csd {
> u8 raw_partition_support; /* 160 */
> u8 raw_rpmb_size_mult; /* 168 */
> u8 raw_erased_mem_count; /* 181 */
> + u8 raw_ext_csd_bus_width; /* 183 */
> u8 strobe_support; /* 184 */
> + u8 raw_ext_csd_hs_timing; /* 185 */
> u8 raw_ext_csd_structure; /* 194 */
> u8 raw_card_type; /* 196 */
> u8 raw_driver_strength; /* 197 */
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index c38072e..a9ddf7a 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -422,6 +422,7 @@ struct mmc_host {
> #else
> #define MMC_CAP2_CRYPTO 0
> #endif
> +#define MMC_CAP2_SLEEP_AWAKE (1 << 29) /* Use Sleep/Awake
> (CMD5) */
> #define MMC_CAP2_ALT_GPT_TEGRA (1 << 28) /* Host with eMMC that
> has GPT entry at a non-standard location */
>
> int fixed_drv_type; /* fixed driver type for non-removable
> media */
> @@ -441,6 +442,7 @@ struct mmc_host {
> spinlock_t lock; /* lock for claim and bus ops */
>
> struct mmc_ios ios; /* current io bus settings */
> + struct mmc_ios cached_ios;
>
> /* group bitfields together to minimize padding */
> unsigned int use_spi_crc:1;
> --
> 2.7.4

2022-04-27 09:51:25

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH V1] mmc: core: Add partial initialization support

On Tue, 26 Apr 2022 at 11:04, Srinivasarao Pathipati
<[email protected]> wrote:
>
> From: Maya Erez <[email protected]>
>
> This change adds the ability to partially initialize
> the MMC card by using card Sleep/Awake sequence (CMD5).
> Card will be sent to Sleep state during runtime/system suspend
> and will be woken up during runtime/system resume.
> By using this sequence the card doesn't need full initialization
> which gives time reduction in system/runtime resume path.
>
> Signed-off-by: Maya Erez <[email protected]>
> Signed-off-by: Veerabhadrarao Badiganti <[email protected]>
> Signed-off-by: Shaik Sajida Bhanu <[email protected]>
> Signed-off-by: kamasali <[email protected]>
> Signed-off-by: Srinivasarao Pathipati <[email protected]>

It seems like this patch has been posted before [1]. Let me repeat my
question sent back then.

It would be nice if you could provide some more exact numbers of what
the gain is for a couple of different eMMCs, to justify the change.
Can you please do that?

Kind regards
Uffe

[1]
https://patchwork.kernel.org/project/linux-mmc/patch/[email protected]/

> ---
> drivers/mmc/core/mmc.c | 149 ++++++++++++++++++++++++++++++++++++++++++++---
> include/linux/mmc/card.h | 4 ++
> include/linux/mmc/host.h | 2 +
> 3 files changed, 146 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 9ab915b..8691c00 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1942,7 +1942,14 @@ static int mmc_sleep_busy_cb(void *cb_data, bool *busy)
> return 0;
> }
>
> -static int mmc_sleep(struct mmc_host *host)
> +static int mmc_can_sleepawake(struct mmc_host *host)
> +{
> + return host && (host->caps2 & MMC_CAP2_SLEEP_AWAKE) && host->card &&
> + (host->card->ext_csd.rev >= 3);
> +
> +}
> +
> +static int mmc_sleepawake(struct mmc_host *host, bool sleep)
> {
> struct mmc_command cmd = {};
> struct mmc_card *card = host->card;
> @@ -1953,14 +1960,17 @@ static int mmc_sleep(struct mmc_host *host)
> /* Re-tuning can't be done once the card is deselected */
> mmc_retune_hold(host);
>
> - err = mmc_deselect_cards(host);
> - if (err)
> - goto out_release;
> + if (sleep) {
> + err = mmc_deselect_cards(host);
> + if (err)
> + goto out_release;
> + }
>
> cmd.opcode = MMC_SLEEP_AWAKE;
> cmd.arg = card->rca << 16;
> - cmd.arg |= 1 << 15;
> use_r1b_resp = mmc_prepare_busy_cmd(host, &cmd, timeout_ms);
> + if (sleep)
> + cmd.arg |= 1 << 15;
>
> err = mmc_wait_for_cmd(host, &cmd, 0);
> if (err)
> @@ -1982,6 +1992,9 @@ static int mmc_sleep(struct mmc_host *host)
>
> err = __mmc_poll_for_busy(host, 0, timeout_ms, &mmc_sleep_busy_cb, host);
>
> + if (!sleep)
> + err = mmc_select_card(card);
> +
> out_release:
> mmc_retune_release(host);
> return err;
> @@ -2080,6 +2093,66 @@ static int _mmc_flush_cache(struct mmc_host *host)
> pr_err("%s: cache flush error %d\n",
> mmc_hostname(host), err);
> }
> + return err;
> +}
> +
> +static int mmc_cache_card_ext_csd(struct mmc_host *host)
> +{
> + int err;
> + u8 *ext_csd;
> + struct mmc_card *card = host->card;
> +
> + err = mmc_get_ext_csd(card, &ext_csd);
> + if (err || !ext_csd) {
> + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
> + mmc_hostname(host), __func__, err);
> + return err;
> + }
> + /* only cache read/write fields that the sw changes */
> + card->ext_csd.raw_ext_csd_cmdq = ext_csd[EXT_CSD_CMDQ_MODE_EN];
> + card->ext_csd.raw_ext_csd_cache_ctrl = ext_csd[EXT_CSD_CACHE_CTRL];
> + card->ext_csd.raw_ext_csd_bus_width = ext_csd[EXT_CSD_BUS_WIDTH];
> + card->ext_csd.raw_ext_csd_hs_timing = ext_csd[EXT_CSD_HS_TIMING];
> +
> + kfree(ext_csd);
> +
> + return 0;
> +}
> +
> +static int mmc_test_awake_ext_csd(struct mmc_host *host)
> +{
> + int err;
> + u8 *ext_csd;
> + struct mmc_card *card = host->card;
> +
> + err = mmc_get_ext_csd(card, &ext_csd);
> + if (err) {
> + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
> + mmc_hostname(host), __func__, err);
> + return err;
> + }
> +
> + /* only compare read/write fields that the sw changes */
> + pr_debug("%s: %s: type(cached:current) cmdq(%d:%d) cache_ctrl(%d:%d) bus_width (%d:%d) timing(%d:%d)\n",
> + mmc_hostname(host), __func__,
> + card->ext_csd.raw_ext_csd_cmdq,
> + ext_csd[EXT_CSD_CMDQ_MODE_EN],
> + card->ext_csd.raw_ext_csd_cache_ctrl,
> + ext_csd[EXT_CSD_CACHE_CTRL],
> + card->ext_csd.raw_ext_csd_bus_width,
> + ext_csd[EXT_CSD_BUS_WIDTH],
> + card->ext_csd.raw_ext_csd_hs_timing,
> + ext_csd[EXT_CSD_HS_TIMING]);
> + err = !((card->ext_csd.raw_ext_csd_cmdq ==
> + ext_csd[EXT_CSD_CMDQ_MODE_EN]) &&
> + (card->ext_csd.raw_ext_csd_cache_ctrl ==
> + ext_csd[EXT_CSD_CACHE_CTRL]) &&
> + (card->ext_csd.raw_ext_csd_bus_width ==
> + ext_csd[EXT_CSD_BUS_WIDTH]) &&
> + (card->ext_csd.raw_ext_csd_hs_timing ==
> + ext_csd[EXT_CSD_HS_TIMING]));
> +
> + kfree(ext_csd);
>
> return err;
> }
> @@ -2103,8 +2176,12 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
> ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend ||
> (host->caps2 & MMC_CAP2_FULL_PWR_CYCLE_IN_SUSPEND)))
> err = mmc_poweroff_notify(host->card, notify_type);
> - else if (mmc_can_sleep(host->card))
> - err = mmc_sleep(host);
> + if (mmc_can_sleepawake(host)) {
> + memcpy(&host->cached_ios, &host->ios, sizeof(host->cached_ios));
> + mmc_cache_card_ext_csd(host);
> + }
> + if (mmc_can_sleep(host->card))
> + err = mmc_sleepawake(host, true);
> else if (!mmc_host_is_spi(host))
> err = mmc_deselect_cards(host);
>
> @@ -2117,6 +2194,48 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
> return err;
> }
>
> +static int mmc_partial_init(struct mmc_host *host)
> +{
> + int err = 0;
> + struct mmc_card *card = host->card;
> +
> + mmc_set_bus_width(host, host->cached_ios.bus_width);
> + mmc_set_timing(host, host->cached_ios.timing);
> + if (host->cached_ios.enhanced_strobe) {
> + host->ios.enhanced_strobe = true;
> + if (host->ops->hs400_enhanced_strobe)
> + host->ops->hs400_enhanced_strobe(host, &host->ios);
> + }
> + mmc_set_clock(host, host->cached_ios.clock);
> + mmc_set_bus_mode(host, host->cached_ios.bus_mode);
> +
> + if (!mmc_card_hs400es(card) &&
> + (mmc_card_hs200(card) || mmc_card_hs400(card))) {
> + err = mmc_execute_tuning(card);
> + if (err) {
> + pr_err("%s: %s: Tuning failed (%d)\n",
> + mmc_hostname(host), __func__, err);
> + goto out;
> + }
> + }
> + /*
> + * The ext_csd is read to make sure the card did not went through
> + * Power-failure during sleep period.
> + * A subset of the W/E_P, W/C_P register will be tested. In case
> + * these registers values are different from the values that were
> + * cached during suspend, we will conclude that a Power-failure occurred
> + * and will do full initialization sequence.
> + */
> + err = mmc_test_awake_ext_csd(host);
> + if (err) {
> + pr_debug("%s: %s: fail on ext_csd read (%d)\n",
> + mmc_hostname(host), __func__, err);
> + goto out;
> + }
> +out:
> + return err;
> +}
> +
> /*
> * Suspend callback
> */
> @@ -2139,7 +2258,7 @@ static int mmc_suspend(struct mmc_host *host)
> */
> static int _mmc_resume(struct mmc_host *host)
> {
> - int err = 0;
> + int err = -EINVAL;
>
> mmc_claim_host(host);
>
> @@ -2147,7 +2266,19 @@ static int _mmc_resume(struct mmc_host *host)
> goto out;
>
> mmc_power_up(host, host->card->ocr);
> - err = mmc_init_card(host, host->card->ocr, host->card);
> +
> + if (mmc_can_sleepawake(host)) {
> + err = mmc_sleepawake(host, false);
> + if (!err)
> + err = mmc_partial_init(host);
> + else
> + pr_err("%s: %s: awake failed (%d), fallback to full init\n",
> + mmc_hostname(host), __func__, err);
> + }
> +
> + if (err)
> + err = mmc_init_card(host, host->card->ocr, host->card);
> +
> mmc_card_clr_suspended(host->card);
>
> out:
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 37f9758..ed7f6f7 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -86,6 +86,8 @@ struct mmc_ext_csd {
> unsigned int data_tag_unit_size; /* DATA TAG UNIT size */
> unsigned int boot_ro_lock; /* ro lock support */
> bool boot_ro_lockable;
> + u8 raw_ext_csd_cmdq; /* 15 */
> + u8 raw_ext_csd_cache_ctrl; /* 33 */
> bool ffu_capable; /* Firmware upgrade support */
> bool cmdq_en; /* Command Queue enabled */
> bool cmdq_support; /* Command Queue supported */
> @@ -96,7 +98,9 @@ struct mmc_ext_csd {
> u8 raw_partition_support; /* 160 */
> u8 raw_rpmb_size_mult; /* 168 */
> u8 raw_erased_mem_count; /* 181 */
> + u8 raw_ext_csd_bus_width; /* 183 */
> u8 strobe_support; /* 184 */
> + u8 raw_ext_csd_hs_timing; /* 185 */
> u8 raw_ext_csd_structure; /* 194 */
> u8 raw_card_type; /* 196 */
> u8 raw_driver_strength; /* 197 */
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index c38072e..a9ddf7a 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -422,6 +422,7 @@ struct mmc_host {
> #else
> #define MMC_CAP2_CRYPTO 0
> #endif
> +#define MMC_CAP2_SLEEP_AWAKE (1 << 29) /* Use Sleep/Awake (CMD5) */
> #define MMC_CAP2_ALT_GPT_TEGRA (1 << 28) /* Host with eMMC that has GPT entry at a non-standard location */
>
> int fixed_drv_type; /* fixed driver type for non-removable media */
> @@ -441,6 +442,7 @@ struct mmc_host {
> spinlock_t lock; /* lock for claim and bus ops */
>
> struct mmc_ios ios; /* current io bus settings */
> + struct mmc_ios cached_ios;
>
> /* group bitfields together to minimize padding */
> unsigned int use_spi_crc:1;
> --
> 2.7.4
>

2022-04-27 11:30:22

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH V1] mmc: core: Add partial initialization support

Hi Srinivasarao,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on ulf-hansson-mmc-mirror/next v5.18-rc4 next-20220426]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/intel-lab-lkp/linux/commits/Srinivasarao-Pathipati/mmc-core-Add-partial-initialization-support/20220426-175124
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git d615b5416f8a1afeb82d13b238f8152c572d59c0
config: nios2-randconfig-m031-20220425 (https://download.01.org/0day-ci/archive/20220427/[email protected]/config)
compiler: nios2-linux-gcc (GCC) 11.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

smatch warnings:
drivers/mmc/core/mmc.c:2168 _mmc_suspend() warn: inconsistent indenting

vim +2168 drivers/mmc/core/mmc.c

2146
2147 static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
2148 {
2149 int err = 0;
2150 unsigned int notify_type = is_suspend ? EXT_CSD_POWER_OFF_SHORT :
2151 EXT_CSD_POWER_OFF_LONG;
2152
2153 mmc_claim_host(host);
2154
2155 if (mmc_card_suspended(host->card))
2156 goto out;
2157
2158 err = _mmc_flush_cache(host);
2159 if (err)
2160 goto out;
2161
2162 if (mmc_can_poweroff_notify(host->card) &&
2163 ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend ||
2164 (host->caps2 & MMC_CAP2_FULL_PWR_CYCLE_IN_SUSPEND)))
2165 err = mmc_poweroff_notify(host->card, notify_type);
2166 if (mmc_can_sleepawake(host)) {
2167 memcpy(&host->cached_ios, &host->ios, sizeof(host->cached_ios));
> 2168 mmc_cache_card_ext_csd(host);
2169 }
2170 if (mmc_can_sleep(host->card))
2171 err = mmc_sleepawake(host, true);
2172 else if (!mmc_host_is_spi(host))
2173 err = mmc_deselect_cards(host);
2174
2175 if (!err) {
2176 mmc_power_off(host);
2177 mmc_card_set_suspended(host->card);
2178 }
2179 out:
2180 mmc_release_host(host);
2181 return err;
2182 }
2183

--
0-DAY CI Kernel Test Service
https://01.org/lkp

Subject: RE: [PATCH V1] mmc: core: Add partial initialization support

Hi,

These patches will be further taken by Sarthak.

Thanks,
Satya

-----Original Message-----
From: Ulf Hansson <[email protected]>
Sent: Wednesday, April 27, 2022 1:06 PM
To: quic_spathi <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Veerabhadrarao Badiganti <[email protected]>; Shaik Sajida Bhanu <[email protected]>; Kamasali Satyanarayan (Consultant) (QUIC) <[email protected]>
Subject: Re: [PATCH V1] mmc: core: Add partial initialization support

On Tue, 26 Apr 2022 at 11:04, Srinivasarao Pathipati <[email protected]> wrote:
>
> From: Maya Erez <[email protected]>
>
> This change adds the ability to partially initialize the MMC card by
> using card Sleep/Awake sequence (CMD5).
> Card will be sent to Sleep state during runtime/system suspend and
> will be woken up during runtime/system resume.
> By using this sequence the card doesn't need full initialization which
> gives time reduction in system/runtime resume path.
>
> Signed-off-by: Maya Erez <[email protected]>
> Signed-off-by: Veerabhadrarao Badiganti <[email protected]>
> Signed-off-by: Shaik Sajida Bhanu <[email protected]>
> Signed-off-by: kamasali <[email protected]>
> Signed-off-by: Srinivasarao Pathipati <[email protected]>

It seems like this patch has been posted before [1]. Let me repeat my question sent back then.

It would be nice if you could provide some more exact numbers of what the gain is for a couple of different eMMCs, to justify the change.
Can you please do that?

Kind regards
Uffe

[1]
https://patchwork.kernel.org/project/linux-mmc/patch/[email protected]/

> ---
> drivers/mmc/core/mmc.c | 149 ++++++++++++++++++++++++++++++++++++++++++++---
> include/linux/mmc/card.h | 4 ++
> include/linux/mmc/host.h | 2 +
> 3 files changed, 146 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
> 9ab915b..8691c00 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1942,7 +1942,14 @@ static int mmc_sleep_busy_cb(void *cb_data, bool *busy)
> return 0;
> }
>
> -static int mmc_sleep(struct mmc_host *host)
> +static int mmc_can_sleepawake(struct mmc_host *host) {
> + return host && (host->caps2 & MMC_CAP2_SLEEP_AWAKE) && host->card &&
> + (host->card->ext_csd.rev >= 3);
> +
> +}
> +
> +static int mmc_sleepawake(struct mmc_host *host, bool sleep)
> {
> struct mmc_command cmd = {};
> struct mmc_card *card = host->card; @@ -1953,14 +1960,17 @@
> static int mmc_sleep(struct mmc_host *host)
> /* Re-tuning can't be done once the card is deselected */
> mmc_retune_hold(host);
>
> - err = mmc_deselect_cards(host);
> - if (err)
> - goto out_release;
> + if (sleep) {
> + err = mmc_deselect_cards(host);
> + if (err)
> + goto out_release;
> + }
>
> cmd.opcode = MMC_SLEEP_AWAKE;
> cmd.arg = card->rca << 16;
> - cmd.arg |= 1 << 15;
> use_r1b_resp = mmc_prepare_busy_cmd(host, &cmd, timeout_ms);
> + if (sleep)
> + cmd.arg |= 1 << 15;
>
> err = mmc_wait_for_cmd(host, &cmd, 0);
> if (err)
> @@ -1982,6 +1992,9 @@ static int mmc_sleep(struct mmc_host *host)
>
> err = __mmc_poll_for_busy(host, 0, timeout_ms,
> &mmc_sleep_busy_cb, host);
>
> + if (!sleep)
> + err = mmc_select_card(card);
> +
> out_release:
> mmc_retune_release(host);
> return err;
> @@ -2080,6 +2093,66 @@ static int _mmc_flush_cache(struct mmc_host *host)
> pr_err("%s: cache flush error %d\n",
> mmc_hostname(host), err);
> }
> + return err;
> +}
> +
> +static int mmc_cache_card_ext_csd(struct mmc_host *host) {
> + int err;
> + u8 *ext_csd;
> + struct mmc_card *card = host->card;
> +
> + err = mmc_get_ext_csd(card, &ext_csd);
> + if (err || !ext_csd) {
> + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
> + mmc_hostname(host), __func__, err);
> + return err;
> + }
> + /* only cache read/write fields that the sw changes */
> + card->ext_csd.raw_ext_csd_cmdq = ext_csd[EXT_CSD_CMDQ_MODE_EN];
> + card->ext_csd.raw_ext_csd_cache_ctrl = ext_csd[EXT_CSD_CACHE_CTRL];
> + card->ext_csd.raw_ext_csd_bus_width = ext_csd[EXT_CSD_BUS_WIDTH];
> + card->ext_csd.raw_ext_csd_hs_timing =
> + ext_csd[EXT_CSD_HS_TIMING];
> +
> + kfree(ext_csd);
> +
> + return 0;
> +}
> +
> +static int mmc_test_awake_ext_csd(struct mmc_host *host) {
> + int err;
> + u8 *ext_csd;
> + struct mmc_card *card = host->card;
> +
> + err = mmc_get_ext_csd(card, &ext_csd);
> + if (err) {
> + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
> + mmc_hostname(host), __func__, err);
> + return err;
> + }
> +
> + /* only compare read/write fields that the sw changes */
> + pr_debug("%s: %s: type(cached:current) cmdq(%d:%d) cache_ctrl(%d:%d) bus_width (%d:%d) timing(%d:%d)\n",
> + mmc_hostname(host), __func__,
> + card->ext_csd.raw_ext_csd_cmdq,
> + ext_csd[EXT_CSD_CMDQ_MODE_EN],
> + card->ext_csd.raw_ext_csd_cache_ctrl,
> + ext_csd[EXT_CSD_CACHE_CTRL],
> + card->ext_csd.raw_ext_csd_bus_width,
> + ext_csd[EXT_CSD_BUS_WIDTH],
> + card->ext_csd.raw_ext_csd_hs_timing,
> + ext_csd[EXT_CSD_HS_TIMING]);
> + err = !((card->ext_csd.raw_ext_csd_cmdq ==
> + ext_csd[EXT_CSD_CMDQ_MODE_EN]) &&
> + (card->ext_csd.raw_ext_csd_cache_ctrl ==
> + ext_csd[EXT_CSD_CACHE_CTRL]) &&
> + (card->ext_csd.raw_ext_csd_bus_width ==
> + ext_csd[EXT_CSD_BUS_WIDTH]) &&
> + (card->ext_csd.raw_ext_csd_hs_timing ==
> + ext_csd[EXT_CSD_HS_TIMING]));
> +
> + kfree(ext_csd);
>
> return err;
> }
> @@ -2103,8 +2176,12 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
> ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend ||
> (host->caps2 & MMC_CAP2_FULL_PWR_CYCLE_IN_SUSPEND)))
> err = mmc_poweroff_notify(host->card, notify_type);
> - else if (mmc_can_sleep(host->card))
> - err = mmc_sleep(host);
> + if (mmc_can_sleepawake(host)) {
> + memcpy(&host->cached_ios, &host->ios, sizeof(host->cached_ios));
> + mmc_cache_card_ext_csd(host);
> + }
> + if (mmc_can_sleep(host->card))
> + err = mmc_sleepawake(host, true);
> else if (!mmc_host_is_spi(host))
> err = mmc_deselect_cards(host);
>
> @@ -2117,6 +2194,48 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
> return err;
> }
>
> +static int mmc_partial_init(struct mmc_host *host) {
> + int err = 0;
> + struct mmc_card *card = host->card;
> +
> + mmc_set_bus_width(host, host->cached_ios.bus_width);
> + mmc_set_timing(host, host->cached_ios.timing);
> + if (host->cached_ios.enhanced_strobe) {
> + host->ios.enhanced_strobe = true;
> + if (host->ops->hs400_enhanced_strobe)
> + host->ops->hs400_enhanced_strobe(host, &host->ios);
> + }
> + mmc_set_clock(host, host->cached_ios.clock);
> + mmc_set_bus_mode(host, host->cached_ios.bus_mode);
> +
> + if (!mmc_card_hs400es(card) &&
> + (mmc_card_hs200(card) || mmc_card_hs400(card))) {
> + err = mmc_execute_tuning(card);
> + if (err) {
> + pr_err("%s: %s: Tuning failed (%d)\n",
> + mmc_hostname(host), __func__, err);
> + goto out;
> + }
> + }
> + /*
> + * The ext_csd is read to make sure the card did not went through
> + * Power-failure during sleep period.
> + * A subset of the W/E_P, W/C_P register will be tested. In case
> + * these registers values are different from the values that were
> + * cached during suspend, we will conclude that a Power-failure occurred
> + * and will do full initialization sequence.
> + */
> + err = mmc_test_awake_ext_csd(host);
> + if (err) {
> + pr_debug("%s: %s: fail on ext_csd read (%d)\n",
> + mmc_hostname(host), __func__, err);
> + goto out;
> + }
> +out:
> + return err;
> +}
> +
> /*
> * Suspend callback
> */
> @@ -2139,7 +2258,7 @@ static int mmc_suspend(struct mmc_host *host)
> */
> static int _mmc_resume(struct mmc_host *host) {
> - int err = 0;
> + int err = -EINVAL;
>
> mmc_claim_host(host);
>
> @@ -2147,7 +2266,19 @@ static int _mmc_resume(struct mmc_host *host)
> goto out;
>
> mmc_power_up(host, host->card->ocr);
> - err = mmc_init_card(host, host->card->ocr, host->card);
> +
> + if (mmc_can_sleepawake(host)) {
> + err = mmc_sleepawake(host, false);
> + if (!err)
> + err = mmc_partial_init(host);
> + else
> + pr_err("%s: %s: awake failed (%d), fallback to full init\n",
> + mmc_hostname(host), __func__, err);
> + }
> +
> + if (err)
> + err = mmc_init_card(host, host->card->ocr, host->card);
> +
> mmc_card_clr_suspended(host->card);
>
> out:
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 37f9758..ed7f6f7 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -86,6 +86,8 @@ struct mmc_ext_csd {
> unsigned int data_tag_unit_size; /* DATA TAG UNIT size */
> unsigned int boot_ro_lock; /* ro lock support */
> bool boot_ro_lockable;
> + u8 raw_ext_csd_cmdq; /* 15 */
> + u8 raw_ext_csd_cache_ctrl; /* 33 */
> bool ffu_capable; /* Firmware upgrade support */
> bool cmdq_en; /* Command Queue enabled */
> bool cmdq_support; /* Command Queue supported */
> @@ -96,7 +98,9 @@ struct mmc_ext_csd {
> u8 raw_partition_support; /* 160 */
> u8 raw_rpmb_size_mult; /* 168 */
> u8 raw_erased_mem_count; /* 181 */
> + u8 raw_ext_csd_bus_width; /* 183 */
> u8 strobe_support; /* 184 */
> + u8 raw_ext_csd_hs_timing; /* 185 */
> u8 raw_ext_csd_structure; /* 194 */
> u8 raw_card_type; /* 196 */
> u8 raw_driver_strength; /* 197 */
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index c38072e..a9ddf7a 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -422,6 +422,7 @@ struct mmc_host {
> #else
> #define MMC_CAP2_CRYPTO 0
> #endif
> +#define MMC_CAP2_SLEEP_AWAKE (1 << 29) /* Use Sleep/Awake (CMD5) */
> #define MMC_CAP2_ALT_GPT_TEGRA (1 << 28) /* Host with eMMC that has GPT entry at a non-standard location */
>
> int fixed_drv_type; /* fixed driver type for non-removable media */
> @@ -441,6 +442,7 @@ struct mmc_host {
> spinlock_t lock; /* lock for claim and bus ops */
>
> struct mmc_ios ios; /* current io bus settings */
> + struct mmc_ios cached_ios;
>
> /* group bitfields together to minimize padding */
> unsigned int use_spi_crc:1;
> --
> 2.7.4
>

2022-05-25 06:45:22

by Sarthak Garg

[permalink] [raw]
Subject: RE: [PATCH V1] mmc: core: Add partial initialization support

Hi Ulf,

Please find the gains seen on micron and kingston eMMC parts below with partial initialization feature (These are the averaged numbers) :

1) Micron eMMC (ManfID 0x13)

Partial init Full Init

a) _mmc_resume: _mmc_resume :

Total time : 62ms Total time : 84ms
(Decrease % from full init = ~26%)

Breakup :
mmc_claim_host_time: 0.2ms mmc_claim_host_time: 0.1ms
mmc_power_up_time: 33ms mmc_power_up_time: 33ms
mmc_sleepawake_time: 28ms mmc_init_card_time: 50ms
mmc_partial_init_time: 1ms

b) _mmc_suspend: _mmc_suspend:

Total time: 5ms Total time: 7.5ms
mmc_claim_host_time: 0.5ms mmc_claim_host_time: 1ms
mmc_flush_cache_time : 1.5 ms mmc_flush_cache_time : 2.5 ms
mmc_sleep_time: 1.5ms mmc_sleep_time: 2ms
mmc_power_off_time: 1.5ms mmc_power_off_time: 1.5ms


2) Kingston eMMC (ManfID 0x70)

Partial init Full Init

a) _mmc_resume: _mmc_resume :
Total time : 46ms Total time : 62ms
(Decrease % from full init = ~25%)

Breakup :
mmc_claim_host_time: 0.2ms mmc_claim_host_time: 0.2ms
mmc_power_up_time: 30ms mmc_power_up_time: 30ms
mmc_sleepawake_time: 14ms mmc_init_card_time: 31ms
mmc_partial_init_time: 2ms


b) _mmc_suspend: _mmc_suspend:
Total time : 5ms Total: 5ms

Breakup :
mmc_claim_host_time: 0.5ms mmc_claim_host_time: 0.5ms
mmc_flush_cache_time : 1.5 ms mmc_flush_cache_time : 1.5 ms
mmc_sleep_time: 1.5ms mmc_sleep_time: 1ms
mmc_power_off_time: 1.5ms mmc_power_off_time: 1.5ms

Did some minor modifications as well to this patchset as per avri's comment which I'll post as V2.
Please let me know your inputs about these numbers.

Thanks,
Sarthak

> -----Original Message-----
> From: Kamasali Satyanarayan (Consultant) (QUIC)
> <[email protected]>
> Sent: Tuesday, May 24, 2022 5:29 PM
> To: 'Ulf Hansson' <[email protected]>; Sarthak Garg (QUIC)
> <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Veerabhadrarao Badiganti
> <[email protected]>; Shaik Sajida Bhanu <[email protected]>;
> '[email protected]' <[email protected]>
> Subject: RE: [PATCH V1] mmc: core: Add partial initialization support
>
> Hi,
>
> These patches will be further taken by Sarthak.
>
> Thanks,
> Satya
>
> -----Original Message-----
> From: Ulf Hansson <[email protected]>
> Sent: Wednesday, April 27, 2022 1:06 PM
> To: quic_spathi <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Veerabhadrarao Badiganti
> <[email protected]>; Shaik Sajida Bhanu <[email protected]>;
> Kamasali Satyanarayan (Consultant) (QUIC) <[email protected]>
> Subject: Re: [PATCH V1] mmc: core: Add partial initialization support
>
> On Tue, 26 Apr 2022 at 11:04, Srinivasarao Pathipati <[email protected]>
> wrote:
> >
> > From: Maya Erez <[email protected]>
> >
> > This change adds the ability to partially initialize the MMC card by
> > using card Sleep/Awake sequence (CMD5).
> > Card will be sent to Sleep state during runtime/system suspend and
> > will be woken up during runtime/system resume.
> > By using this sequence the card doesn't need full initialization which
> > gives time reduction in system/runtime resume path.
> >
> > Signed-off-by: Maya Erez <[email protected]>
> > Signed-off-by: Veerabhadrarao Badiganti <[email protected]>
> > Signed-off-by: Shaik Sajida Bhanu <[email protected]>
> > Signed-off-by: kamasali <[email protected]>
> > Signed-off-by: Srinivasarao Pathipati <[email protected]>
>
> It seems like this patch has been posted before [1]. Let me repeat my question
> sent back then.
>
> It would be nice if you could provide some more exact numbers of what the gain
> is for a couple of different eMMCs, to justify the change.
> Can you please do that?
>
> Kind regards
> Uffe
>
> [1]
> https://patchwork.kernel.org/project/linux-mmc/patch/1591277381-7734-1-
> [email protected]/
>
> > ---
> > drivers/mmc/core/mmc.c | 149
> ++++++++++++++++++++++++++++++++++++++++++++---
> > include/linux/mmc/card.h | 4 ++
> > include/linux/mmc/host.h | 2 +
> > 3 files changed, 146 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
> > 9ab915b..8691c00 100644
> > --- a/drivers/mmc/core/mmc.c
> > +++ b/drivers/mmc/core/mmc.c
> > @@ -1942,7 +1942,14 @@ static int mmc_sleep_busy_cb(void *cb_data, bool
> *busy)
> > return 0;
> > }
> >
> > -static int mmc_sleep(struct mmc_host *host)
> > +static int mmc_can_sleepawake(struct mmc_host *host) {
> > + return host && (host->caps2 & MMC_CAP2_SLEEP_AWAKE) && host-
> >card &&
> > + (host->card->ext_csd.rev >= 3);
> > +
> > +}
> > +
> > +static int mmc_sleepawake(struct mmc_host *host, bool sleep)
> > {
> > struct mmc_command cmd = {};
> > struct mmc_card *card = host->card; @@ -1953,14 +1960,17 @@
> > static int mmc_sleep(struct mmc_host *host)
> > /* Re-tuning can't be done once the card is deselected */
> > mmc_retune_hold(host);
> >
> > - err = mmc_deselect_cards(host);
> > - if (err)
> > - goto out_release;
> > + if (sleep) {
> > + err = mmc_deselect_cards(host);
> > + if (err)
> > + goto out_release;
> > + }
> >
> > cmd.opcode = MMC_SLEEP_AWAKE;
> > cmd.arg = card->rca << 16;
> > - cmd.arg |= 1 << 15;
> > use_r1b_resp = mmc_prepare_busy_cmd(host, &cmd, timeout_ms);
> > + if (sleep)
> > + cmd.arg |= 1 << 15;
> >
> > err = mmc_wait_for_cmd(host, &cmd, 0);
> > if (err)
> > @@ -1982,6 +1992,9 @@ static int mmc_sleep(struct mmc_host *host)
> >
> > err = __mmc_poll_for_busy(host, 0, timeout_ms,
> > &mmc_sleep_busy_cb, host);
> >
> > + if (!sleep)
> > + err = mmc_select_card(card);
> > +
> > out_release:
> > mmc_retune_release(host);
> > return err;
> > @@ -2080,6 +2093,66 @@ static int _mmc_flush_cache(struct mmc_host
> *host)
> > pr_err("%s: cache flush error %d\n",
> > mmc_hostname(host), err);
> > }
> > + return err;
> > +}
> > +
> > +static int mmc_cache_card_ext_csd(struct mmc_host *host) {
> > + int err;
> > + u8 *ext_csd;
> > + struct mmc_card *card = host->card;
> > +
> > + err = mmc_get_ext_csd(card, &ext_csd);
> > + if (err || !ext_csd) {
> > + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
> > + mmc_hostname(host), __func__, err);
> > + return err;
> > + }
> > + /* only cache read/write fields that the sw changes */
> > + card->ext_csd.raw_ext_csd_cmdq =
> ext_csd[EXT_CSD_CMDQ_MODE_EN];
> > + card->ext_csd.raw_ext_csd_cache_ctrl =
> ext_csd[EXT_CSD_CACHE_CTRL];
> > + card->ext_csd.raw_ext_csd_bus_width =
> ext_csd[EXT_CSD_BUS_WIDTH];
> > + card->ext_csd.raw_ext_csd_hs_timing =
> > + ext_csd[EXT_CSD_HS_TIMING];
> > +
> > + kfree(ext_csd);
> > +
> > + return 0;
> > +}
> > +
> > +static int mmc_test_awake_ext_csd(struct mmc_host *host) {
> > + int err;
> > + u8 *ext_csd;
> > + struct mmc_card *card = host->card;
> > +
> > + err = mmc_get_ext_csd(card, &ext_csd);
> > + if (err) {
> > + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
> > + mmc_hostname(host), __func__, err);
> > + return err;
> > + }
> > +
> > + /* only compare read/write fields that the sw changes */
> > + pr_debug("%s: %s: type(cached:current) cmdq(%d:%d) cache_ctrl(%d:%d)
> bus_width (%d:%d) timing(%d:%d)\n",
> > + mmc_hostname(host), __func__,
> > + card->ext_csd.raw_ext_csd_cmdq,
> > + ext_csd[EXT_CSD_CMDQ_MODE_EN],
> > + card->ext_csd.raw_ext_csd_cache_ctrl,
> > + ext_csd[EXT_CSD_CACHE_CTRL],
> > + card->ext_csd.raw_ext_csd_bus_width,
> > + ext_csd[EXT_CSD_BUS_WIDTH],
> > + card->ext_csd.raw_ext_csd_hs_timing,
> > + ext_csd[EXT_CSD_HS_TIMING]);
> > + err = !((card->ext_csd.raw_ext_csd_cmdq ==
> > + ext_csd[EXT_CSD_CMDQ_MODE_EN]) &&
> > + (card->ext_csd.raw_ext_csd_cache_ctrl ==
> > + ext_csd[EXT_CSD_CACHE_CTRL]) &&
> > + (card->ext_csd.raw_ext_csd_bus_width ==
> > + ext_csd[EXT_CSD_BUS_WIDTH]) &&
> > + (card->ext_csd.raw_ext_csd_hs_timing ==
> > + ext_csd[EXT_CSD_HS_TIMING]));
> > +
> > + kfree(ext_csd);
> >
> > return err;
> > }
> > @@ -2103,8 +2176,12 @@ static int _mmc_suspend(struct mmc_host *host,
> bool is_suspend)
> > ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend ||
> > (host->caps2 & MMC_CAP2_FULL_PWR_CYCLE_IN_SUSPEND)))
> > err = mmc_poweroff_notify(host->card, notify_type);
> > - else if (mmc_can_sleep(host->card))
> > - err = mmc_sleep(host);
> > + if (mmc_can_sleepawake(host)) {
> > + memcpy(&host->cached_ios, &host->ios, sizeof(host->cached_ios));
> > + mmc_cache_card_ext_csd(host);
> > + }
> > + if (mmc_can_sleep(host->card))
> > + err = mmc_sleepawake(host, true);
> > else if (!mmc_host_is_spi(host))
> > err = mmc_deselect_cards(host);
> >
> > @@ -2117,6 +2194,48 @@ static int _mmc_suspend(struct mmc_host *host,
> bool is_suspend)
> > return err;
> > }
> >
> > +static int mmc_partial_init(struct mmc_host *host) {
> > + int err = 0;
> > + struct mmc_card *card = host->card;
> > +
> > + mmc_set_bus_width(host, host->cached_ios.bus_width);
> > + mmc_set_timing(host, host->cached_ios.timing);
> > + if (host->cached_ios.enhanced_strobe) {
> > + host->ios.enhanced_strobe = true;
> > + if (host->ops->hs400_enhanced_strobe)
> > + host->ops->hs400_enhanced_strobe(host, &host->ios);
> > + }
> > + mmc_set_clock(host, host->cached_ios.clock);
> > + mmc_set_bus_mode(host, host->cached_ios.bus_mode);
> > +
> > + if (!mmc_card_hs400es(card) &&
> > + (mmc_card_hs200(card) || mmc_card_hs400(card))) {
> > + err = mmc_execute_tuning(card);
> > + if (err) {
> > + pr_err("%s: %s: Tuning failed (%d)\n",
> > + mmc_hostname(host), __func__, err);
> > + goto out;
> > + }
> > + }
> > + /*
> > + * The ext_csd is read to make sure the card did not went through
> > + * Power-failure during sleep period.
> > + * A subset of the W/E_P, W/C_P register will be tested. In case
> > + * these registers values are different from the values that were
> > + * cached during suspend, we will conclude that a Power-failure occurred
> > + * and will do full initialization sequence.
> > + */
> > + err = mmc_test_awake_ext_csd(host);
> > + if (err) {
> > + pr_debug("%s: %s: fail on ext_csd read (%d)\n",
> > + mmc_hostname(host), __func__, err);
> > + goto out;
> > + }
> > +out:
> > + return err;
> > +}
> > +
> > /*
> > * Suspend callback
> > */
> > @@ -2139,7 +2258,7 @@ static int mmc_suspend(struct mmc_host *host)
> > */
> > static int _mmc_resume(struct mmc_host *host) {
> > - int err = 0;
> > + int err = -EINVAL;
> >
> > mmc_claim_host(host);
> >
> > @@ -2147,7 +2266,19 @@ static int _mmc_resume(struct mmc_host *host)
> > goto out;
> >
> > mmc_power_up(host, host->card->ocr);
> > - err = mmc_init_card(host, host->card->ocr, host->card);
> > +
> > + if (mmc_can_sleepawake(host)) {
> > + err = mmc_sleepawake(host, false);
> > + if (!err)
> > + err = mmc_partial_init(host);
> > + else
> > + pr_err("%s: %s: awake failed (%d), fallback to full init\n",
> > + mmc_hostname(host), __func__, err);
> > + }
> > +
> > + if (err)
> > + err = mmc_init_card(host, host->card->ocr,
> > + host->card);
> > +
> > mmc_card_clr_suspended(host->card);
> >
> > out:
> > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index
> > 37f9758..ed7f6f7 100644
> > --- a/include/linux/mmc/card.h
> > +++ b/include/linux/mmc/card.h
> > @@ -86,6 +86,8 @@ struct mmc_ext_csd {
> > unsigned int data_tag_unit_size; /* DATA TAG UNIT size */
> > unsigned int boot_ro_lock; /* ro lock support */
> > bool boot_ro_lockable;
> > + u8 raw_ext_csd_cmdq; /* 15 */
> > + u8 raw_ext_csd_cache_ctrl; /* 33 */
> > bool ffu_capable; /* Firmware upgrade support */
> > bool cmdq_en; /* Command Queue enabled */
> > bool cmdq_support; /* Command Queue supported */
> > @@ -96,7 +98,9 @@ struct mmc_ext_csd {
> > u8 raw_partition_support; /* 160 */
> > u8 raw_rpmb_size_mult; /* 168 */
> > u8 raw_erased_mem_count; /* 181 */
> > + u8 raw_ext_csd_bus_width; /* 183 */
> > u8 strobe_support; /* 184 */
> > + u8 raw_ext_csd_hs_timing; /* 185 */
> > u8 raw_ext_csd_structure; /* 194 */
> > u8 raw_card_type; /* 196 */
> > u8 raw_driver_strength; /* 197 */
> > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index
> > c38072e..a9ddf7a 100644
> > --- a/include/linux/mmc/host.h
> > +++ b/include/linux/mmc/host.h
> > @@ -422,6 +422,7 @@ struct mmc_host {
> > #else
> > #define MMC_CAP2_CRYPTO 0
> > #endif
> > +#define MMC_CAP2_SLEEP_AWAKE (1 << 29) /* Use Sleep/Awake
> (CMD5) */
> > #define MMC_CAP2_ALT_GPT_TEGRA (1 << 28) /* Host with eMMC that
> has GPT entry at a non-standard location */
> >
> > int fixed_drv_type; /* fixed driver type for non-removable
> media */
> > @@ -441,6 +442,7 @@ struct mmc_host {
> > spinlock_t lock; /* lock for claim and bus ops */
> >
> > struct mmc_ios ios; /* current io bus settings */
> > + struct mmc_ios cached_ios;
> >
> > /* group bitfields together to minimize padding */
> > unsigned int use_spi_crc:1;
> > --
> > 2.7.4
> >

2022-06-01 21:33:09

by Sarthak Garg

[permalink] [raw]
Subject: RE: [PATCH V1] mmc: core: Add partial initialization support

Hi ulf,

Did you get a chance to check the numbers ?

Thanks,
Sarthak

> -----Original Message-----
> From: Sarthak Garg (QUIC)
> Sent: Tuesday, May 24, 2022 8:07 PM
> To: Kamasali Satyanarayan (Consultant) (QUIC) <[email protected]>;
> 'Ulf Hansson' <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Veerabhadrarao Badiganti
> <[email protected]>; Shaik Sajida Bhanu <[email protected]>;
> '[email protected]' <[email protected]>
> Subject: RE: [PATCH V1] mmc: core: Add partial initialization support
>
> Hi Ulf,
>
> Please find the gains seen on micron and kingston eMMC parts below with
> partial initialization feature (These are the averaged numbers) :
>
> 1) Micron eMMC (ManfID 0x13)
>
> Partial init Full Init
>
> a) _mmc_resume: _mmc_resume :
>
> Total time : 62ms Total time : 84ms
> (Decrease % from full init = ~26%)
>
> Breakup :
> mmc_claim_host_time: 0.2ms mmc_claim_host_time: 0.1ms
> mmc_power_up_time: 33ms mmc_power_up_time: 33ms
> mmc_sleepawake_time: 28ms mmc_init_card_time: 50ms
> mmc_partial_init_time: 1ms
>
> b) _mmc_suspend: _mmc_suspend:
>
> Total time: 5ms Total time: 7.5ms
> mmc_claim_host_time: 0.5ms mmc_claim_host_time: 1ms
> mmc_flush_cache_time : 1.5 ms mmc_flush_cache_time : 2.5 ms
> mmc_sleep_time: 1.5ms mmc_sleep_time: 2ms
> mmc_power_off_time: 1.5ms mmc_power_off_time: 1.5ms
>
>
> 2) Kingston eMMC (ManfID 0x70)
>
> Partial init Full Init
>
> a) _mmc_resume: _mmc_resume :
> Total time : 46ms Total time : 62ms
> (Decrease % from full init = ~25%)
>
> Breakup :
> mmc_claim_host_time: 0.2ms mmc_claim_host_time: 0.2ms
> mmc_power_up_time: 30ms mmc_power_up_time: 30ms
> mmc_sleepawake_time: 14ms mmc_init_card_time: 31ms
> mmc_partial_init_time: 2ms
>
>
> b) _mmc_suspend: _mmc_suspend:
> Total time : 5ms Total: 5ms
>
> Breakup :
> mmc_claim_host_time: 0.5ms mmc_claim_host_time: 0.5ms
> mmc_flush_cache_time : 1.5 ms mmc_flush_cache_time : 1.5 ms
> mmc_sleep_time: 1.5ms mmc_sleep_time: 1ms
> mmc_power_off_time: 1.5ms mmc_power_off_time: 1.5ms
>
> Did some minor modifications as well to this patchset as per avri's comment
> which I'll post as V2.
> Please let me know your inputs about these numbers.
>
> Thanks,
> Sarthak
>
> > -----Original Message-----
> > From: Kamasali Satyanarayan (Consultant) (QUIC)
> > <[email protected]>
> > Sent: Tuesday, May 24, 2022 5:29 PM
> > To: 'Ulf Hansson' <[email protected]>; Sarthak Garg (QUIC)
> > <[email protected]>
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected];
> > Veerabhadrarao Badiganti <[email protected]>; Shaik Sajida Bhanu
> > <[email protected]>; '[email protected]'
> > <[email protected]>
> > Subject: RE: [PATCH V1] mmc: core: Add partial initialization support
> >
> > Hi,
> >
> > These patches will be further taken by Sarthak.
> >
> > Thanks,
> > Satya
> >
> > -----Original Message-----
> > From: Ulf Hansson <[email protected]>
> > Sent: Wednesday, April 27, 2022 1:06 PM
> > To: quic_spathi <[email protected]>
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected];
> > Veerabhadrarao Badiganti <[email protected]>; Shaik Sajida Bhanu
> > <[email protected]>; Kamasali Satyanarayan (Consultant) (QUIC)
> > <[email protected]>
> > Subject: Re: [PATCH V1] mmc: core: Add partial initialization support
> >
> > On Tue, 26 Apr 2022 at 11:04, Srinivasarao Pathipati
> > <[email protected]>
> > wrote:
> > >
> > > From: Maya Erez <[email protected]>
> > >
> > > This change adds the ability to partially initialize the MMC card by
> > > using card Sleep/Awake sequence (CMD5).
> > > Card will be sent to Sleep state during runtime/system suspend and
> > > will be woken up during runtime/system resume.
> > > By using this sequence the card doesn't need full initialization
> > > which gives time reduction in system/runtime resume path.
> > >
> > > Signed-off-by: Maya Erez <[email protected]>
> > > Signed-off-by: Veerabhadrarao Badiganti <[email protected]>
> > > Signed-off-by: Shaik Sajida Bhanu <[email protected]>
> > > Signed-off-by: kamasali <[email protected]>
> > > Signed-off-by: Srinivasarao Pathipati <[email protected]>
> >
> > It seems like this patch has been posted before [1]. Let me repeat my
> > question sent back then.
> >
> > It would be nice if you could provide some more exact numbers of what
> > the gain is for a couple of different eMMCs, to justify the change.
> > Can you please do that?
> >
> > Kind regards
> > Uffe
> >
> > [1]
> > https://patchwork.kernel.org/project/linux-mmc/patch/1591277381-7734-1
> > - [email protected]/
> >
> > > ---
> > > drivers/mmc/core/mmc.c | 149
> > ++++++++++++++++++++++++++++++++++++++++++++---
> > > include/linux/mmc/card.h | 4 ++
> > > include/linux/mmc/host.h | 2 +
> > > 3 files changed, 146 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
> > > 9ab915b..8691c00 100644
> > > --- a/drivers/mmc/core/mmc.c
> > > +++ b/drivers/mmc/core/mmc.c
> > > @@ -1942,7 +1942,14 @@ static int mmc_sleep_busy_cb(void *cb_data,
> > > bool
> > *busy)
> > > return 0;
> > > }
> > >
> > > -static int mmc_sleep(struct mmc_host *host)
> > > +static int mmc_can_sleepawake(struct mmc_host *host) {
> > > + return host && (host->caps2 & MMC_CAP2_SLEEP_AWAKE) && host-
> > >card &&
> > > + (host->card->ext_csd.rev >= 3);
> > > +
> > > +}
> > > +
> > > +static int mmc_sleepawake(struct mmc_host *host, bool sleep)
> > > {
> > > struct mmc_command cmd = {};
> > > struct mmc_card *card = host->card; @@ -1953,14 +1960,17 @@
> > > static int mmc_sleep(struct mmc_host *host)
> > > /* Re-tuning can't be done once the card is deselected */
> > > mmc_retune_hold(host);
> > >
> > > - err = mmc_deselect_cards(host);
> > > - if (err)
> > > - goto out_release;
> > > + if (sleep) {
> > > + err = mmc_deselect_cards(host);
> > > + if (err)
> > > + goto out_release;
> > > + }
> > >
> > > cmd.opcode = MMC_SLEEP_AWAKE;
> > > cmd.arg = card->rca << 16;
> > > - cmd.arg |= 1 << 15;
> > > use_r1b_resp = mmc_prepare_busy_cmd(host, &cmd, timeout_ms);
> > > + if (sleep)
> > > + cmd.arg |= 1 << 15;
> > >
> > > err = mmc_wait_for_cmd(host, &cmd, 0);
> > > if (err)
> > > @@ -1982,6 +1992,9 @@ static int mmc_sleep(struct mmc_host *host)
> > >
> > > err = __mmc_poll_for_busy(host, 0, timeout_ms,
> > > &mmc_sleep_busy_cb, host);
> > >
> > > + if (!sleep)
> > > + err = mmc_select_card(card);
> > > +
> > > out_release:
> > > mmc_retune_release(host);
> > > return err;
> > > @@ -2080,6 +2093,66 @@ static int _mmc_flush_cache(struct mmc_host
> > *host)
> > > pr_err("%s: cache flush error %d\n",
> > > mmc_hostname(host), err);
> > > }
> > > + return err;
> > > +}
> > > +
> > > +static int mmc_cache_card_ext_csd(struct mmc_host *host) {
> > > + int err;
> > > + u8 *ext_csd;
> > > + struct mmc_card *card = host->card;
> > > +
> > > + err = mmc_get_ext_csd(card, &ext_csd);
> > > + if (err || !ext_csd) {
> > > + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
> > > + mmc_hostname(host), __func__, err);
> > > + return err;
> > > + }
> > > + /* only cache read/write fields that the sw changes */
> > > + card->ext_csd.raw_ext_csd_cmdq =
> > ext_csd[EXT_CSD_CMDQ_MODE_EN];
> > > + card->ext_csd.raw_ext_csd_cache_ctrl =
> > ext_csd[EXT_CSD_CACHE_CTRL];
> > > + card->ext_csd.raw_ext_csd_bus_width =
> > ext_csd[EXT_CSD_BUS_WIDTH];
> > > + card->ext_csd.raw_ext_csd_hs_timing =
> > > + ext_csd[EXT_CSD_HS_TIMING];
> > > +
> > > + kfree(ext_csd);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int mmc_test_awake_ext_csd(struct mmc_host *host) {
> > > + int err;
> > > + u8 *ext_csd;
> > > + struct mmc_card *card = host->card;
> > > +
> > > + err = mmc_get_ext_csd(card, &ext_csd);
> > > + if (err) {
> > > + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
> > > + mmc_hostname(host), __func__, err);
> > > + return err;
> > > + }
> > > +
> > > + /* only compare read/write fields that the sw changes */
> > > + pr_debug("%s: %s: type(cached:current) cmdq(%d:%d)
> > > + cache_ctrl(%d:%d)
> > bus_width (%d:%d) timing(%d:%d)\n",
> > > + mmc_hostname(host), __func__,
> > > + card->ext_csd.raw_ext_csd_cmdq,
> > > + ext_csd[EXT_CSD_CMDQ_MODE_EN],
> > > + card->ext_csd.raw_ext_csd_cache_ctrl,
> > > + ext_csd[EXT_CSD_CACHE_CTRL],
> > > + card->ext_csd.raw_ext_csd_bus_width,
> > > + ext_csd[EXT_CSD_BUS_WIDTH],
> > > + card->ext_csd.raw_ext_csd_hs_timing,
> > > + ext_csd[EXT_CSD_HS_TIMING]);
> > > + err = !((card->ext_csd.raw_ext_csd_cmdq ==
> > > + ext_csd[EXT_CSD_CMDQ_MODE_EN]) &&
> > > + (card->ext_csd.raw_ext_csd_cache_ctrl ==
> > > + ext_csd[EXT_CSD_CACHE_CTRL]) &&
> > > + (card->ext_csd.raw_ext_csd_bus_width ==
> > > + ext_csd[EXT_CSD_BUS_WIDTH]) &&
> > > + (card->ext_csd.raw_ext_csd_hs_timing ==
> > > + ext_csd[EXT_CSD_HS_TIMING]));
> > > +
> > > + kfree(ext_csd);
> > >
> > > return err;
> > > }
> > > @@ -2103,8 +2176,12 @@ static int _mmc_suspend(struct mmc_host
> > > *host,
> > bool is_suspend)
> > > ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend ||
> > > (host->caps2 & MMC_CAP2_FULL_PWR_CYCLE_IN_SUSPEND)))
> > > err = mmc_poweroff_notify(host->card, notify_type);
> > > - else if (mmc_can_sleep(host->card))
> > > - err = mmc_sleep(host);
> > > + if (mmc_can_sleepawake(host)) {
> > > + memcpy(&host->cached_ios, &host->ios, sizeof(host-
> >cached_ios));
> > > + mmc_cache_card_ext_csd(host);
> > > + }
> > > + if (mmc_can_sleep(host->card))
> > > + err = mmc_sleepawake(host, true);
> > > else if (!mmc_host_is_spi(host))
> > > err = mmc_deselect_cards(host);
> > >
> > > @@ -2117,6 +2194,48 @@ static int _mmc_suspend(struct mmc_host
> > > *host,
> > bool is_suspend)
> > > return err;
> > > }
> > >
> > > +static int mmc_partial_init(struct mmc_host *host) {
> > > + int err = 0;
> > > + struct mmc_card *card = host->card;
> > > +
> > > + mmc_set_bus_width(host, host->cached_ios.bus_width);
> > > + mmc_set_timing(host, host->cached_ios.timing);
> > > + if (host->cached_ios.enhanced_strobe) {
> > > + host->ios.enhanced_strobe = true;
> > > + if (host->ops->hs400_enhanced_strobe)
> > > + host->ops->hs400_enhanced_strobe(host, &host->ios);
> > > + }
> > > + mmc_set_clock(host, host->cached_ios.clock);
> > > + mmc_set_bus_mode(host, host->cached_ios.bus_mode);
> > > +
> > > + if (!mmc_card_hs400es(card) &&
> > > + (mmc_card_hs200(card) || mmc_card_hs400(card))) {
> > > + err = mmc_execute_tuning(card);
> > > + if (err) {
> > > + pr_err("%s: %s: Tuning failed (%d)\n",
> > > + mmc_hostname(host), __func__, err);
> > > + goto out;
> > > + }
> > > + }
> > > + /*
> > > + * The ext_csd is read to make sure the card did not went through
> > > + * Power-failure during sleep period.
> > > + * A subset of the W/E_P, W/C_P register will be tested. In case
> > > + * these registers values are different from the values that were
> > > + * cached during suspend, we will conclude that a Power-failure
> occurred
> > > + * and will do full initialization sequence.
> > > + */
> > > + err = mmc_test_awake_ext_csd(host);
> > > + if (err) {
> > > + pr_debug("%s: %s: fail on ext_csd read (%d)\n",
> > > + mmc_hostname(host), __func__, err);
> > > + goto out;
> > > + }
> > > +out:
> > > + return err;
> > > +}
> > > +
> > > /*
> > > * Suspend callback
> > > */
> > > @@ -2139,7 +2258,7 @@ static int mmc_suspend(struct mmc_host *host)
> > > */
> > > static int _mmc_resume(struct mmc_host *host) {
> > > - int err = 0;
> > > + int err = -EINVAL;
> > >
> > > mmc_claim_host(host);
> > >
> > > @@ -2147,7 +2266,19 @@ static int _mmc_resume(struct mmc_host *host)
> > > goto out;
> > >
> > > mmc_power_up(host, host->card->ocr);
> > > - err = mmc_init_card(host, host->card->ocr, host->card);
> > > +
> > > + if (mmc_can_sleepawake(host)) {
> > > + err = mmc_sleepawake(host, false);
> > > + if (!err)
> > > + err = mmc_partial_init(host);
> > > + else
> > > + pr_err("%s: %s: awake failed (%d), fallback to full init\n",
> > > + mmc_hostname(host), __func__, err);
> > > + }
> > > +
> > > + if (err)
> > > + err = mmc_init_card(host, host->card->ocr,
> > > + host->card);
> > > +
> > > mmc_card_clr_suspended(host->card);
> > >
> > > out:
> > > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> > > index
> > > 37f9758..ed7f6f7 100644
> > > --- a/include/linux/mmc/card.h
> > > +++ b/include/linux/mmc/card.h
> > > @@ -86,6 +86,8 @@ struct mmc_ext_csd {
> > > unsigned int data_tag_unit_size; /* DATA TAG UNIT size */
> > > unsigned int boot_ro_lock; /* ro lock support */
> > > bool boot_ro_lockable;
> > > + u8 raw_ext_csd_cmdq; /* 15 */
> > > + u8 raw_ext_csd_cache_ctrl; /* 33 */
> > > bool ffu_capable; /* Firmware upgrade support */
> > > bool cmdq_en; /* Command Queue enabled */
> > > bool cmdq_support; /* Command Queue supported */
> > > @@ -96,7 +98,9 @@ struct mmc_ext_csd {
> > > u8 raw_partition_support; /* 160 */
> > > u8 raw_rpmb_size_mult; /* 168 */
> > > u8 raw_erased_mem_count; /* 181 */
> > > + u8 raw_ext_csd_bus_width; /* 183 */
> > > u8 strobe_support; /* 184 */
> > > + u8 raw_ext_csd_hs_timing; /* 185 */
> > > u8 raw_ext_csd_structure; /* 194 */
> > > u8 raw_card_type; /* 196 */
> > > u8 raw_driver_strength; /* 197 */
> > > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> > > index c38072e..a9ddf7a 100644
> > > --- a/include/linux/mmc/host.h
> > > +++ b/include/linux/mmc/host.h
> > > @@ -422,6 +422,7 @@ struct mmc_host { #else
> > > #define MMC_CAP2_CRYPTO 0
> > > #endif
> > > +#define MMC_CAP2_SLEEP_AWAKE (1 << 29) /* Use Sleep/Awake
> > (CMD5) */
> > > #define MMC_CAP2_ALT_GPT_TEGRA (1 << 28) /* Host with eMMC that
> > has GPT entry at a non-standard location */
> > >
> > > int fixed_drv_type; /* fixed driver type for non-removable
> > media */
> > > @@ -441,6 +442,7 @@ struct mmc_host {
> > > spinlock_t lock; /* lock for claim and bus ops */
> > >
> > > struct mmc_ios ios; /* current io bus settings */
> > > + struct mmc_ios cached_ios;
> > >
> > > /* group bitfields together to minimize padding */
> > > unsigned int use_spi_crc:1;
> > > --
> > > 2.7.4
> > >

2022-06-02 08:25:48

by Ben Chuang

[permalink] [raw]
Subject: Re: [PATCH V1] mmc: core: Add partial initialization support

Hi Sarthak and others,

I like your patch, I'm not familiar with the whole sleep/wake flow.
But I have a question.

Regarding mmc_poweroff_notify() , there is a parameter notify_type.
In the spec., it has a value 0x04: SLEEP_NOTIFICATION (host is going
to put the device in sleep mode.)
Is there anything that needs to be changed in _mmc_suspend() before
calling mmc_sleepawake(host, true)/mmc_power_off(host)?
Thanks in advance.

Best regards,
Ben

On Wed, May 25, 2022 at 12:31 AM Kamasali Satyanarayan (Consultant)
(QUIC) <[email protected]> wrote:
>
> Hi,
>
> These patches will be further taken by Sarthak.
>
> Thanks,
> Satya
>
> -----Original Message-----
> From: Ulf Hansson <[email protected]>
> Sent: Wednesday, April 27, 2022 1:06 PM
> To: quic_spathi <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Veerabhadrarao Badiganti <[email protected]>; Shaik Sajida Bhanu <[email protected]>; Kamasali Satyanarayan (Consultant) (QUIC) <[email protected]>
> Subject: Re: [PATCH V1] mmc: core: Add partial initialization support
>
> On Tue, 26 Apr 2022 at 11:04, Srinivasarao Pathipati <[email protected]> wrote:
> >
> > From: Maya Erez <[email protected]>
> >
> > This change adds the ability to partially initialize the MMC card by
> > using card Sleep/Awake sequence (CMD5).
> > Card will be sent to Sleep state during runtime/system suspend and
> > will be woken up during runtime/system resume.
> > By using this sequence the card doesn't need full initialization which
> > gives time reduction in system/runtime resume path.
> >
> > Signed-off-by: Maya Erez <[email protected]>
> > Signed-off-by: Veerabhadrarao Badiganti <[email protected]>
> > Signed-off-by: Shaik Sajida Bhanu <[email protected]>
> > Signed-off-by: kamasali <[email protected]>
> > Signed-off-by: Srinivasarao Pathipati <[email protected]>
>
> It seems like this patch has been posted before [1]. Let me repeat my question sent back then.
>
> It would be nice if you could provide some more exact numbers of what the gain is for a couple of different eMMCs, to justify the change.
> Can you please do that?
>
> Kind regards
> Uffe
>
> [1]
> https://patchwork.kernel.org/project/linux-mmc/patch/[email protected]/
>
> > ---
> > drivers/mmc/core/mmc.c | 149 ++++++++++++++++++++++++++++++++++++++++++++---
> > include/linux/mmc/card.h | 4 ++
> > include/linux/mmc/host.h | 2 +
> > 3 files changed, 146 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
> > 9ab915b..8691c00 100644
> > --- a/drivers/mmc/core/mmc.c
> > +++ b/drivers/mmc/core/mmc.c
> > @@ -1942,7 +1942,14 @@ static int mmc_sleep_busy_cb(void *cb_data, bool *busy)
> > return 0;
> > }
> >
> > -static int mmc_sleep(struct mmc_host *host)
> > +static int mmc_can_sleepawake(struct mmc_host *host) {
> > + return host && (host->caps2 & MMC_CAP2_SLEEP_AWAKE) && host->card &&
> > + (host->card->ext_csd.rev >= 3);
> > +
> > +}
> > +
> > +static int mmc_sleepawake(struct mmc_host *host, bool sleep)
> > {
> > struct mmc_command cmd = {};
> > struct mmc_card *card = host->card; @@ -1953,14 +1960,17 @@
> > static int mmc_sleep(struct mmc_host *host)
> > /* Re-tuning can't be done once the card is deselected */
> > mmc_retune_hold(host);
> >
> > - err = mmc_deselect_cards(host);
> > - if (err)
> > - goto out_release;
> > + if (sleep) {
> > + err = mmc_deselect_cards(host);
> > + if (err)
> > + goto out_release;
> > + }
> >
> > cmd.opcode = MMC_SLEEP_AWAKE;
> > cmd.arg = card->rca << 16;
> > - cmd.arg |= 1 << 15;
> > use_r1b_resp = mmc_prepare_busy_cmd(host, &cmd, timeout_ms);
> > + if (sleep)
> > + cmd.arg |= 1 << 15;
> >
> > err = mmc_wait_for_cmd(host, &cmd, 0);
> > if (err)
> > @@ -1982,6 +1992,9 @@ static int mmc_sleep(struct mmc_host *host)
> >
> > err = __mmc_poll_for_busy(host, 0, timeout_ms,
> > &mmc_sleep_busy_cb, host);
> >
> > + if (!sleep)
> > + err = mmc_select_card(card);
> > +
> > out_release:
> > mmc_retune_release(host);
> > return err;
> > @@ -2080,6 +2093,66 @@ static int _mmc_flush_cache(struct mmc_host *host)
> > pr_err("%s: cache flush error %d\n",
> > mmc_hostname(host), err);
> > }
> > + return err;
> > +}
> > +
> > +static int mmc_cache_card_ext_csd(struct mmc_host *host) {
> > + int err;
> > + u8 *ext_csd;
> > + struct mmc_card *card = host->card;
> > +
> > + err = mmc_get_ext_csd(card, &ext_csd);
> > + if (err || !ext_csd) {
> > + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
> > + mmc_hostname(host), __func__, err);
> > + return err;
> > + }
> > + /* only cache read/write fields that the sw changes */
> > + card->ext_csd.raw_ext_csd_cmdq = ext_csd[EXT_CSD_CMDQ_MODE_EN];
> > + card->ext_csd.raw_ext_csd_cache_ctrl = ext_csd[EXT_CSD_CACHE_CTRL];
> > + card->ext_csd.raw_ext_csd_bus_width = ext_csd[EXT_CSD_BUS_WIDTH];
> > + card->ext_csd.raw_ext_csd_hs_timing =
> > + ext_csd[EXT_CSD_HS_TIMING];
> > +
> > + kfree(ext_csd);
> > +
> > + return 0;
> > +}
> > +
> > +static int mmc_test_awake_ext_csd(struct mmc_host *host) {
> > + int err;
> > + u8 *ext_csd;
> > + struct mmc_card *card = host->card;
> > +
> > + err = mmc_get_ext_csd(card, &ext_csd);
> > + if (err) {
> > + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
> > + mmc_hostname(host), __func__, err);
> > + return err;
> > + }
> > +
> > + /* only compare read/write fields that the sw changes */
> > + pr_debug("%s: %s: type(cached:current) cmdq(%d:%d) cache_ctrl(%d:%d) bus_width (%d:%d) timing(%d:%d)\n",
> > + mmc_hostname(host), __func__,
> > + card->ext_csd.raw_ext_csd_cmdq,
> > + ext_csd[EXT_CSD_CMDQ_MODE_EN],
> > + card->ext_csd.raw_ext_csd_cache_ctrl,
> > + ext_csd[EXT_CSD_CACHE_CTRL],
> > + card->ext_csd.raw_ext_csd_bus_width,
> > + ext_csd[EXT_CSD_BUS_WIDTH],
> > + card->ext_csd.raw_ext_csd_hs_timing,
> > + ext_csd[EXT_CSD_HS_TIMING]);
> > + err = !((card->ext_csd.raw_ext_csd_cmdq ==
> > + ext_csd[EXT_CSD_CMDQ_MODE_EN]) &&
> > + (card->ext_csd.raw_ext_csd_cache_ctrl ==
> > + ext_csd[EXT_CSD_CACHE_CTRL]) &&
> > + (card->ext_csd.raw_ext_csd_bus_width ==
> > + ext_csd[EXT_CSD_BUS_WIDTH]) &&
> > + (card->ext_csd.raw_ext_csd_hs_timing ==
> > + ext_csd[EXT_CSD_HS_TIMING]));
> > +
> > + kfree(ext_csd);
> >
> > return err;
> > }
> > @@ -2103,8 +2176,12 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
> > ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend ||
> > (host->caps2 & MMC_CAP2_FULL_PWR_CYCLE_IN_SUSPEND)))
> > err = mmc_poweroff_notify(host->card, notify_type);
> > - else if (mmc_can_sleep(host->card))
> > - err = mmc_sleep(host);
> > + if (mmc_can_sleepawake(host)) {
> > + memcpy(&host->cached_ios, &host->ios, sizeof(host->cached_ios));
> > + mmc_cache_card_ext_csd(host);
> > + }
> > + if (mmc_can_sleep(host->card))
> > + err = mmc_sleepawake(host, true);
> > else if (!mmc_host_is_spi(host))
> > err = mmc_deselect_cards(host);
> >
> > @@ -2117,6 +2194,48 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
> > return err;
> > }
> >
> > +static int mmc_partial_init(struct mmc_host *host) {
> > + int err = 0;
> > + struct mmc_card *card = host->card;
> > +
> > + mmc_set_bus_width(host, host->cached_ios.bus_width);
> > + mmc_set_timing(host, host->cached_ios.timing);
> > + if (host->cached_ios.enhanced_strobe) {
> > + host->ios.enhanced_strobe = true;
> > + if (host->ops->hs400_enhanced_strobe)
> > + host->ops->hs400_enhanced_strobe(host, &host->ios);
> > + }
> > + mmc_set_clock(host, host->cached_ios.clock);
> > + mmc_set_bus_mode(host, host->cached_ios.bus_mode);
> > +
> > + if (!mmc_card_hs400es(card) &&
> > + (mmc_card_hs200(card) || mmc_card_hs400(card))) {
> > + err = mmc_execute_tuning(card);
> > + if (err) {
> > + pr_err("%s: %s: Tuning failed (%d)\n",
> > + mmc_hostname(host), __func__, err);
> > + goto out;
> > + }
> > + }
> > + /*
> > + * The ext_csd is read to make sure the card did not went through
> > + * Power-failure during sleep period.
> > + * A subset of the W/E_P, W/C_P register will be tested. In case
> > + * these registers values are different from the values that were
> > + * cached during suspend, we will conclude that a Power-failure occurred
> > + * and will do full initialization sequence.
> > + */
> > + err = mmc_test_awake_ext_csd(host);
> > + if (err) {
> > + pr_debug("%s: %s: fail on ext_csd read (%d)\n",
> > + mmc_hostname(host), __func__, err);
> > + goto out;
> > + }
> > +out:
> > + return err;
> > +}
> > +
> > /*
> > * Suspend callback
> > */
> > @@ -2139,7 +2258,7 @@ static int mmc_suspend(struct mmc_host *host)
> > */
> > static int _mmc_resume(struct mmc_host *host) {
> > - int err = 0;
> > + int err = -EINVAL;
> >
> > mmc_claim_host(host);
> >
> > @@ -2147,7 +2266,19 @@ static int _mmc_resume(struct mmc_host *host)
> > goto out;
> >
> > mmc_power_up(host, host->card->ocr);
> > - err = mmc_init_card(host, host->card->ocr, host->card);
> > +
> > + if (mmc_can_sleepawake(host)) {
> > + err = mmc_sleepawake(host, false);
> > + if (!err)
> > + err = mmc_partial_init(host);
> > + else
> > + pr_err("%s: %s: awake failed (%d), fallback to full init\n",
> > + mmc_hostname(host), __func__, err);
> > + }
> > +
> > + if (err)
> > + err = mmc_init_card(host, host->card->ocr, host->card);
> > +
> > mmc_card_clr_suspended(host->card);
> >
> > out:
> > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> > index 37f9758..ed7f6f7 100644
> > --- a/include/linux/mmc/card.h
> > +++ b/include/linux/mmc/card.h
> > @@ -86,6 +86,8 @@ struct mmc_ext_csd {
> > unsigned int data_tag_unit_size; /* DATA TAG UNIT size */
> > unsigned int boot_ro_lock; /* ro lock support */
> > bool boot_ro_lockable;
> > + u8 raw_ext_csd_cmdq; /* 15 */
> > + u8 raw_ext_csd_cache_ctrl; /* 33 */
> > bool ffu_capable; /* Firmware upgrade support */
> > bool cmdq_en; /* Command Queue enabled */
> > bool cmdq_support; /* Command Queue supported */
> > @@ -96,7 +98,9 @@ struct mmc_ext_csd {
> > u8 raw_partition_support; /* 160 */
> > u8 raw_rpmb_size_mult; /* 168 */
> > u8 raw_erased_mem_count; /* 181 */
> > + u8 raw_ext_csd_bus_width; /* 183 */
> > u8 strobe_support; /* 184 */
> > + u8 raw_ext_csd_hs_timing; /* 185 */
> > u8 raw_ext_csd_structure; /* 194 */
> > u8 raw_card_type; /* 196 */
> > u8 raw_driver_strength; /* 197 */
> > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> > index c38072e..a9ddf7a 100644
> > --- a/include/linux/mmc/host.h
> > +++ b/include/linux/mmc/host.h
> > @@ -422,6 +422,7 @@ struct mmc_host {
> > #else
> > #define MMC_CAP2_CRYPTO 0
> > #endif
> > +#define MMC_CAP2_SLEEP_AWAKE (1 << 29) /* Use Sleep/Awake (CMD5) */
> > #define MMC_CAP2_ALT_GPT_TEGRA (1 << 28) /* Host with eMMC that has GPT entry at a non-standard location */
> >
> > int fixed_drv_type; /* fixed driver type for non-removable media */
> > @@ -441,6 +442,7 @@ struct mmc_host {
> > spinlock_t lock; /* lock for claim and bus ops */
> >
> > struct mmc_ios ios; /* current io bus settings */
> > + struct mmc_ios cached_ios;
> >
> > /* group bitfields together to minimize padding */
> > unsigned int use_spi_crc:1;
> > --
> > 2.7.4
> >

2022-06-15 23:25:47

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH V1] mmc: core: Add partial initialization support

On Tue, 24 May 2022 at 07:37, Sarthak Garg (QUIC)
<[email protected]> wrote:
>
> Hi Ulf,
>
> Please find the gains seen on micron and kingston eMMC parts below with partial initialization feature (These are the averaged numbers) :
>
> 1) Micron eMMC (ManfID 0x13)
>
> Partial init Full Init
>
> a) _mmc_resume: _mmc_resume :
>
> Total time : 62ms Total time : 84ms
> (Decrease % from full init = ~26%)

Alright, so we gained around 22ms. Not too bad.

>
> Breakup :
> mmc_claim_host_time: 0.2ms mmc_claim_host_time: 0.1ms
> mmc_power_up_time: 33ms mmc_power_up_time: 33ms
> mmc_sleepawake_time: 28ms mmc_init_card_time: 50ms
> mmc_partial_init_time: 1ms
>
> b) _mmc_suspend: _mmc_suspend:
>
> Total time: 5ms Total time: 7.5ms
> mmc_claim_host_time: 0.5ms mmc_claim_host_time: 1ms
> mmc_flush_cache_time : 1.5 ms mmc_flush_cache_time : 2.5 ms
> mmc_sleep_time: 1.5ms mmc_sleep_time: 2ms
> mmc_power_off_time: 1.5ms mmc_power_off_time: 1.5ms

The suspend time shouldn't really differ. Or is there a reason for this?

>
>
> 2) Kingston eMMC (ManfID 0x70)
>
> Partial init Full Init
>
> a) _mmc_resume: _mmc_resume :
> Total time : 46ms Total time : 62ms
> (Decrease % from full init = ~25%)
>
> Breakup :
> mmc_claim_host_time: 0.2ms mmc_claim_host_time: 0.2ms
> mmc_power_up_time: 30ms mmc_power_up_time: 30ms
> mmc_sleepawake_time: 14ms mmc_init_card_time: 31ms
> mmc_partial_init_time: 2ms
>
>
> b) _mmc_suspend: _mmc_suspend:
> Total time : 5ms Total: 5ms
>
> Breakup :
> mmc_claim_host_time: 0.5ms mmc_claim_host_time: 0.5ms
> mmc_flush_cache_time : 1.5 ms mmc_flush_cache_time : 1.5 ms
> mmc_sleep_time: 1.5ms mmc_sleep_time: 1ms
> mmc_power_off_time: 1.5ms mmc_power_off_time: 1.5ms
>
> Did some minor modifications as well to this patchset as per avri's comment which I'll post as V2.
> Please let me know your inputs about these numbers.

Thanks for posting these numbers, much appreciated! Please try to
include some of the data as part of the commit message as I think it's
valuable information.

When it comes to reviewing the code, I am awaiting your v2 then.

[...]

Kind regards
Uffe

2023-09-26 06:40:21

by Sarthak Garg

[permalink] [raw]
Subject: Re: [PATCH V1] mmc: core: Add partial initialization support


On 6/16/2022 4:18 AM, Ulf Hansson wrote:
> On Tue, 24 May 2022 at 07:37, Sarthak Garg (QUIC)
> <[email protected]> wrote:
>> Hi Ulf,
>>
>> Please find the gains seen on micron and kingston eMMC parts below with partial initialization feature (These are the averaged numbers) :
>>
>> 1) Micron eMMC (ManfID 0x13)
>>
>> Partial init Full Init
>>
>> a) _mmc_resume: _mmc_resume :
>>
>> Total time : 62ms Total time : 84ms
>> (Decrease % from full init = ~26%)
> Alright, so we gained around 22ms. Not too bad.
>
>> Breakup :
>> mmc_claim_host_time: 0.2ms mmc_claim_host_time: 0.1ms
>> mmc_power_up_time: 33ms mmc_power_up_time: 33ms
>> mmc_sleepawake_time: 28ms mmc_init_card_time: 50ms
>> mmc_partial_init_time: 1ms
>>
>> b) _mmc_suspend: _mmc_suspend:
>>
>> Total time: 5ms Total time: 7.5ms
>> mmc_claim_host_time: 0.5ms mmc_claim_host_time: 1ms
>> mmc_flush_cache_time : 1.5 ms mmc_flush_cache_time : 2.5 ms
>> mmc_sleep_time: 1.5ms mmc_sleep_time: 2ms
>> mmc_power_off_time: 1.5ms mmc_power_off_time: 1.5ms
> The suspend time shouldn't really differ. Or is there a reason for this?


I think this could be due to run to run variation as we can see
mmc_claim_host and mmc_flush_cache itself taking some extra 1ms.


>
>>
>> 2) Kingston eMMC (ManfID 0x70)
>>
>> Partial init Full Init
>>
>> a) _mmc_resume: _mmc_resume :
>> Total time : 46ms Total time : 62ms
>> (Decrease % from full init = ~25%)
>>
>> Breakup :
>> mmc_claim_host_time: 0.2ms mmc_claim_host_time: 0.2ms
>> mmc_power_up_time: 30ms mmc_power_up_time: 30ms
>> mmc_sleepawake_time: 14ms mmc_init_card_time: 31ms
>> mmc_partial_init_time: 2ms
>>
>>
>> b) _mmc_suspend: _mmc_suspend:
>> Total time : 5ms Total: 5ms
>>
>> Breakup :
>> mmc_claim_host_time: 0.5ms mmc_claim_host_time: 0.5ms
>> mmc_flush_cache_time : 1.5 ms mmc_flush_cache_time : 1.5 ms
>> mmc_sleep_time: 1.5ms mmc_sleep_time: 1ms
>> mmc_power_off_time: 1.5ms mmc_power_off_time: 1.5ms
>>
>> Did some minor modifications as well to this patchset as per avri's comment which I'll post as V2.
>> Please let me know your inputs about these numbers.
> Thanks for posting these numbers, much appreciated! Please try to
> include some of the data as part of the commit message as I think it's
> valuable information.
>
> When it comes to reviewing the code, I am awaiting your v2 then.
>
> [...]
>
> Kind regards
> Uffe

Sure will add this data to the commit text in V2.


2023-09-26 06:43:41

by Sarthak Garg

[permalink] [raw]
Subject: Re: [PATCH V1] mmc: core: Add partial initialization support


On 6/2/2022 1:26 PM, Ben Chuang wrote:
> Hi Sarthak and others,
>
> I like your patch, I'm not familiar with the whole sleep/wake flow.
> But I have a question.
>
> Regarding mmc_poweroff_notify() , there is a parameter notify_type.
> In the spec., it has a value 0x04: SLEEP_NOTIFICATION (host is going
> to put the device in sleep mode.)
> Is there anything that needs to be changed in _mmc_suspend() before
> calling mmc_sleepawake(host, true)/mmc_power_off(host)?
> Thanks in advance.
>
> Best regards,
> Ben


Hi Ben,

I went through the sleep notification in the eMMC spec. This sleep
notification seems optional as per this statement in spec: "It *may* use
to a power notification (SLEEP_NOTIFICATION) if it intends to turn-off
VCC after moving the device to sleep state". So it doesn't seem to be a
good club this with my change. We can revisit and take this separately.

Thanks,
Sarthak

>
> On Wed, May 25, 2022 at 12:31 AM Kamasali Satyanarayan (Consultant)
> (QUIC) <[email protected]> wrote:
>> Hi,
>>
>> These patches will be further taken by Sarthak.
>>
>> Thanks,
>> Satya
>>
>> -----Original Message-----
>> From: Ulf Hansson <[email protected]>
>> Sent: Wednesday, April 27, 2022 1:06 PM
>> To: quic_spathi <[email protected]>
>> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Veerabhadrarao Badiganti <[email protected]>; Shaik Sajida Bhanu <[email protected]>; Kamasali Satyanarayan (Consultant) (QUIC) <[email protected]>
>> Subject: Re: [PATCH V1] mmc: core: Add partial initialization support
>>
>> On Tue, 26 Apr 2022 at 11:04, Srinivasarao Pathipati <[email protected]> wrote:
>>> From: Maya Erez <[email protected]>
>>>
>>> This change adds the ability to partially initialize the MMC card by
>>> using card Sleep/Awake sequence (CMD5).
>>> Card will be sent to Sleep state during runtime/system suspend and
>>> will be woken up during runtime/system resume.
>>> By using this sequence the card doesn't need full initialization which
>>> gives time reduction in system/runtime resume path.
>>>
>>> Signed-off-by: Maya Erez <[email protected]>
>>> Signed-off-by: Veerabhadrarao Badiganti <[email protected]>
>>> Signed-off-by: Shaik Sajida Bhanu <[email protected]>
>>> Signed-off-by: kamasali <[email protected]>
>>> Signed-off-by: Srinivasarao Pathipati <[email protected]>
>> It seems like this patch has been posted before [1]. Let me repeat my question sent back then.
>>
>> It would be nice if you could provide some more exact numbers of what the gain is for a couple of different eMMCs, to justify the change.
>> Can you please do that?
>>
>> Kind regards
>> Uffe
>>
>> [1]
>> https://patchwork.kernel.org/project/linux-mmc/patch/[email protected]/
>>
>>> ---
>>> drivers/mmc/core/mmc.c | 149 ++++++++++++++++++++++++++++++++++++++++++++---
>>> include/linux/mmc/card.h | 4 ++
>>> include/linux/mmc/host.h | 2 +
>>> 3 files changed, 146 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
>>> 9ab915b..8691c00 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -1942,7 +1942,14 @@ static int mmc_sleep_busy_cb(void *cb_data, bool *busy)
>>> return 0;
>>> }
>>>
>>> -static int mmc_sleep(struct mmc_host *host)
>>> +static int mmc_can_sleepawake(struct mmc_host *host) {
>>> + return host && (host->caps2 & MMC_CAP2_SLEEP_AWAKE) && host->card &&
>>> + (host->card->ext_csd.rev >= 3);
>>> +
>>> +}
>>> +
>>> +static int mmc_sleepawake(struct mmc_host *host, bool sleep)
>>> {
>>> struct mmc_command cmd = {};
>>> struct mmc_card *card = host->card; @@ -1953,14 +1960,17 @@
>>> static int mmc_sleep(struct mmc_host *host)
>>> /* Re-tuning can't be done once the card is deselected */
>>> mmc_retune_hold(host);
>>>
>>> - err = mmc_deselect_cards(host);
>>> - if (err)
>>> - goto out_release;
>>> + if (sleep) {
>>> + err = mmc_deselect_cards(host);
>>> + if (err)
>>> + goto out_release;
>>> + }
>>>
>>> cmd.opcode = MMC_SLEEP_AWAKE;
>>> cmd.arg = card->rca << 16;
>>> - cmd.arg |= 1 << 15;
>>> use_r1b_resp = mmc_prepare_busy_cmd(host, &cmd, timeout_ms);
>>> + if (sleep)
>>> + cmd.arg |= 1 << 15;
>>>
>>> err = mmc_wait_for_cmd(host, &cmd, 0);
>>> if (err)
>>> @@ -1982,6 +1992,9 @@ static int mmc_sleep(struct mmc_host *host)
>>>
>>> err = __mmc_poll_for_busy(host, 0, timeout_ms,
>>> &mmc_sleep_busy_cb, host);
>>>
>>> + if (!sleep)
>>> + err = mmc_select_card(card);
>>> +
>>> out_release:
>>> mmc_retune_release(host);
>>> return err;
>>> @@ -2080,6 +2093,66 @@ static int _mmc_flush_cache(struct mmc_host *host)
>>> pr_err("%s: cache flush error %d\n",
>>> mmc_hostname(host), err);
>>> }
>>> + return err;
>>> +}
>>> +
>>> +static int mmc_cache_card_ext_csd(struct mmc_host *host) {
>>> + int err;
>>> + u8 *ext_csd;
>>> + struct mmc_card *card = host->card;
>>> +
>>> + err = mmc_get_ext_csd(card, &ext_csd);
>>> + if (err || !ext_csd) {
>>> + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
>>> + mmc_hostname(host), __func__, err);
>>> + return err;
>>> + }
>>> + /* only cache read/write fields that the sw changes */
>>> + card->ext_csd.raw_ext_csd_cmdq = ext_csd[EXT_CSD_CMDQ_MODE_EN];
>>> + card->ext_csd.raw_ext_csd_cache_ctrl = ext_csd[EXT_CSD_CACHE_CTRL];
>>> + card->ext_csd.raw_ext_csd_bus_width = ext_csd[EXT_CSD_BUS_WIDTH];
>>> + card->ext_csd.raw_ext_csd_hs_timing =
>>> + ext_csd[EXT_CSD_HS_TIMING];
>>> +
>>> + kfree(ext_csd);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int mmc_test_awake_ext_csd(struct mmc_host *host) {
>>> + int err;
>>> + u8 *ext_csd;
>>> + struct mmc_card *card = host->card;
>>> +
>>> + err = mmc_get_ext_csd(card, &ext_csd);
>>> + if (err) {
>>> + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
>>> + mmc_hostname(host), __func__, err);
>>> + return err;
>>> + }
>>> +
>>> + /* only compare read/write fields that the sw changes */
>>> + pr_debug("%s: %s: type(cached:current) cmdq(%d:%d) cache_ctrl(%d:%d) bus_width (%d:%d) timing(%d:%d)\n",
>>> + mmc_hostname(host), __func__,
>>> + card->ext_csd.raw_ext_csd_cmdq,
>>> + ext_csd[EXT_CSD_CMDQ_MODE_EN],
>>> + card->ext_csd.raw_ext_csd_cache_ctrl,
>>> + ext_csd[EXT_CSD_CACHE_CTRL],
>>> + card->ext_csd.raw_ext_csd_bus_width,
>>> + ext_csd[EXT_CSD_BUS_WIDTH],
>>> + card->ext_csd.raw_ext_csd_hs_timing,
>>> + ext_csd[EXT_CSD_HS_TIMING]);
>>> + err = !((card->ext_csd.raw_ext_csd_cmdq ==
>>> + ext_csd[EXT_CSD_CMDQ_MODE_EN]) &&
>>> + (card->ext_csd.raw_ext_csd_cache_ctrl ==
>>> + ext_csd[EXT_CSD_CACHE_CTRL]) &&
>>> + (card->ext_csd.raw_ext_csd_bus_width ==
>>> + ext_csd[EXT_CSD_BUS_WIDTH]) &&
>>> + (card->ext_csd.raw_ext_csd_hs_timing ==
>>> + ext_csd[EXT_CSD_HS_TIMING]));
>>> +
>>> + kfree(ext_csd);
>>>
>>> return err;
>>> }
>>> @@ -2103,8 +2176,12 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
>>> ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend ||
>>> (host->caps2 & MMC_CAP2_FULL_PWR_CYCLE_IN_SUSPEND)))
>>> err = mmc_poweroff_notify(host->card, notify_type);
>>> - else if (mmc_can_sleep(host->card))
>>> - err = mmc_sleep(host);
>>> + if (mmc_can_sleepawake(host)) {
>>> + memcpy(&host->cached_ios, &host->ios, sizeof(host->cached_ios));
>>> + mmc_cache_card_ext_csd(host);
>>> + }
>>> + if (mmc_can_sleep(host->card))
>>> + err = mmc_sleepawake(host, true);
>>> else if (!mmc_host_is_spi(host))
>>> err = mmc_deselect_cards(host);
>>>
>>> @@ -2117,6 +2194,48 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
>>> return err;
>>> }
>>>
>>> +static int mmc_partial_init(struct mmc_host *host) {
>>> + int err = 0;
>>> + struct mmc_card *card = host->card;
>>> +
>>> + mmc_set_bus_width(host, host->cached_ios.bus_width);
>>> + mmc_set_timing(host, host->cached_ios.timing);
>>> + if (host->cached_ios.enhanced_strobe) {
>>> + host->ios.enhanced_strobe = true;
>>> + if (host->ops->hs400_enhanced_strobe)
>>> + host->ops->hs400_enhanced_strobe(host, &host->ios);
>>> + }
>>> + mmc_set_clock(host, host->cached_ios.clock);
>>> + mmc_set_bus_mode(host, host->cached_ios.bus_mode);
>>> +
>>> + if (!mmc_card_hs400es(card) &&
>>> + (mmc_card_hs200(card) || mmc_card_hs400(card))) {
>>> + err = mmc_execute_tuning(card);
>>> + if (err) {
>>> + pr_err("%s: %s: Tuning failed (%d)\n",
>>> + mmc_hostname(host), __func__, err);
>>> + goto out;
>>> + }
>>> + }
>>> + /*
>>> + * The ext_csd is read to make sure the card did not went through
>>> + * Power-failure during sleep period.
>>> + * A subset of the W/E_P, W/C_P register will be tested. In case
>>> + * these registers values are different from the values that were
>>> + * cached during suspend, we will conclude that a Power-failure occurred
>>> + * and will do full initialization sequence.
>>> + */
>>> + err = mmc_test_awake_ext_csd(host);
>>> + if (err) {
>>> + pr_debug("%s: %s: fail on ext_csd read (%d)\n",
>>> + mmc_hostname(host), __func__, err);
>>> + goto out;
>>> + }
>>> +out:
>>> + return err;
>>> +}
>>> +
>>> /*
>>> * Suspend callback
>>> */
>>> @@ -2139,7 +2258,7 @@ static int mmc_suspend(struct mmc_host *host)
>>> */
>>> static int _mmc_resume(struct mmc_host *host) {
>>> - int err = 0;
>>> + int err = -EINVAL;
>>>
>>> mmc_claim_host(host);
>>>
>>> @@ -2147,7 +2266,19 @@ static int _mmc_resume(struct mmc_host *host)
>>> goto out;
>>>
>>> mmc_power_up(host, host->card->ocr);
>>> - err = mmc_init_card(host, host->card->ocr, host->card);
>>> +
>>> + if (mmc_can_sleepawake(host)) {
>>> + err = mmc_sleepawake(host, false);
>>> + if (!err)
>>> + err = mmc_partial_init(host);
>>> + else
>>> + pr_err("%s: %s: awake failed (%d), fallback to full init\n",
>>> + mmc_hostname(host), __func__, err);
>>> + }
>>> +
>>> + if (err)
>>> + err = mmc_init_card(host, host->card->ocr, host->card);
>>> +
>>> mmc_card_clr_suspended(host->card);
>>>
>>> out:
>>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>>> index 37f9758..ed7f6f7 100644
>>> --- a/include/linux/mmc/card.h
>>> +++ b/include/linux/mmc/card.h
>>> @@ -86,6 +86,8 @@ struct mmc_ext_csd {
>>> unsigned int data_tag_unit_size; /* DATA TAG UNIT size */
>>> unsigned int boot_ro_lock; /* ro lock support */
>>> bool boot_ro_lockable;
>>> + u8 raw_ext_csd_cmdq; /* 15 */
>>> + u8 raw_ext_csd_cache_ctrl; /* 33 */
>>> bool ffu_capable; /* Firmware upgrade support */
>>> bool cmdq_en; /* Command Queue enabled */
>>> bool cmdq_support; /* Command Queue supported */
>>> @@ -96,7 +98,9 @@ struct mmc_ext_csd {
>>> u8 raw_partition_support; /* 160 */
>>> u8 raw_rpmb_size_mult; /* 168 */
>>> u8 raw_erased_mem_count; /* 181 */
>>> + u8 raw_ext_csd_bus_width; /* 183 */
>>> u8 strobe_support; /* 184 */
>>> + u8 raw_ext_csd_hs_timing; /* 185 */
>>> u8 raw_ext_csd_structure; /* 194 */
>>> u8 raw_card_type; /* 196 */
>>> u8 raw_driver_strength; /* 197 */
>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>> index c38072e..a9ddf7a 100644
>>> --- a/include/linux/mmc/host.h
>>> +++ b/include/linux/mmc/host.h
>>> @@ -422,6 +422,7 @@ struct mmc_host {
>>> #else
>>> #define MMC_CAP2_CRYPTO 0
>>> #endif
>>> +#define MMC_CAP2_SLEEP_AWAKE (1 << 29) /* Use Sleep/Awake (CMD5) */
>>> #define MMC_CAP2_ALT_GPT_TEGRA (1 << 28) /* Host with eMMC that has GPT entry at a non-standard location */
>>>
>>> int fixed_drv_type; /* fixed driver type for non-removable media */
>>> @@ -441,6 +442,7 @@ struct mmc_host {
>>> spinlock_t lock; /* lock for claim and bus ops */
>>>
>>> struct mmc_ios ios; /* current io bus settings */
>>> + struct mmc_ios cached_ios;
>>>
>>> /* group bitfields together to minimize padding */
>>> unsigned int use_spi_crc:1;
>>> --
>>> 2.7.4
>>>

2023-09-28 07:52:27

by Wenchao Chen

[permalink] [raw]
Subject: Re: [PATCH V1] mmc: core: Add partial initialization support

On Tue, 26 Sept 2023 at 18:41, Sarthak Garg <[email protected]> wrote:
>
>
> On 6/16/2022 4:18 AM, Ulf Hansson wrote:
> > On Tue, 24 May 2022 at 07:37, Sarthak Garg (QUIC)
> > <[email protected]> wrote:
> >> Hi Ulf,
> >>
> >> Please find the gains seen on micron and kingston eMMC parts below with partial initialization feature (These are the averaged numbers) :
> >>
> >> 1) Micron eMMC (ManfID 0x13)
> >>
> >> Partial init Full Init
> >>
> >> a) _mmc_resume: _mmc_resume :
> >>
> >> Total time : 62ms Total time : 84ms
> >> (Decrease % from full init = ~26%)
> > Alright, so we gained around 22ms. Not too bad.
> >
> >> Breakup :
> >> mmc_claim_host_time: 0.2ms mmc_claim_host_time: 0.1ms
> >> mmc_power_up_time: 33ms mmc_power_up_time: 33ms
> >> mmc_sleepawake_time: 28ms mmc_init_card_time: 50ms
> >> mmc_partial_init_time: 1ms
> >>
> >> b) _mmc_suspend: _mmc_suspend:
> >>
> >> Total time: 5ms Total time: 7.5ms
> >> mmc_claim_host_time: 0.5ms mmc_claim_host_time: 1ms
> >> mmc_flush_cache_time : 1.5 ms mmc_flush_cache_time : 2.5 ms
> >> mmc_sleep_time: 1.5ms mmc_sleep_time: 2ms
> >> mmc_power_off_time: 1.5ms mmc_power_off_time: 1.5ms
> > The suspend time shouldn't really differ. Or is there a reason for this?
>
>
> I think this could be due to run to run variation as we can see
> mmc_claim_host and mmc_flush_cache itself taking some extra 1ms.
>
>

Hi Sarthak

I have a question.
1.What is the difference between Partial init and Full Init on SOC
power consumption?
2.Partial init and Full init improve IO performance?
3.Could you share the test methods if you want?

> >
> >>
> >> 2) Kingston eMMC (ManfID 0x70)
> >>
> >> Partial init Full Init
> >>
> >> a) _mmc_resume: _mmc_resume :
> >> Total time : 46ms Total time : 62ms
> >> (Decrease % from full init = ~25%)
> >>
> >> Breakup :
> >> mmc_claim_host_time: 0.2ms mmc_claim_host_time: 0.2ms
> >> mmc_power_up_time: 30ms mmc_power_up_time: 30ms
> >> mmc_sleepawake_time: 14ms mmc_init_card_time: 31ms
> >> mmc_partial_init_time: 2ms
> >>
> >>
> >> b) _mmc_suspend: _mmc_suspend:
> >> Total time : 5ms Total: 5ms
> >>
> >> Breakup :
> >> mmc_claim_host_time: 0.5ms mmc_claim_host_time: 0.5ms
> >> mmc_flush_cache_time : 1.5 ms mmc_flush_cache_time : 1.5 ms
> >> mmc_sleep_time: 1.5ms mmc_sleep_time: 1ms
> >> mmc_power_off_time: 1.5ms mmc_power_off_time: 1.5ms
> >>
> >> Did some minor modifications as well to this patchset as per avri's comment which I'll post as V2.
> >> Please let me know your inputs about these numbers.
> > Thanks for posting these numbers, much appreciated! Please try to
> > include some of the data as part of the commit message as I think it's
> > valuable information.
> >
> > When it comes to reviewing the code, I am awaiting your v2 then.
> >
> > [...]
> >
> > Kind regards
> > Uffe
>
> Sure will add this data to the commit text in V2.
>
>

2023-10-04 10:47:12

by Sarthak Garg

[permalink] [raw]
Subject: Re: [PATCH V1] mmc: core: Add partial initialization support


On 9/28/2023 8:50 AM, Wenchao Chen wrote:
> On Tue, 26 Sept 2023 at 18:41, Sarthak Garg <[email protected]> wrote:
>>
>> On 6/16/2022 4:18 AM, Ulf Hansson wrote:
>>> On Tue, 24 May 2022 at 07:37, Sarthak Garg (QUIC)
>>> <[email protected]> wrote:
>>>> Hi Ulf,
>>>>
>>>> Please find the gains seen on micron and kingston eMMC parts below with partial initialization feature (These are the averaged numbers) :
>>>>
>>>> 1) Micron eMMC (ManfID 0x13)
>>>>
>>>> Partial init Full Init
>>>>
>>>> a) _mmc_resume: _mmc_resume :
>>>>
>>>> Total time : 62ms Total time : 84ms
>>>> (Decrease % from full init = ~26%)
>>> Alright, so we gained around 22ms. Not too bad.
>>>
>>>> Breakup :
>>>> mmc_claim_host_time: 0.2ms mmc_claim_host_time: 0.1ms
>>>> mmc_power_up_time: 33ms mmc_power_up_time: 33ms
>>>> mmc_sleepawake_time: 28ms mmc_init_card_time: 50ms
>>>> mmc_partial_init_time: 1ms
>>>>
>>>> b) _mmc_suspend: _mmc_suspend:
>>>>
>>>> Total time: 5ms Total time: 7.5ms
>>>> mmc_claim_host_time: 0.5ms mmc_claim_host_time: 1ms
>>>> mmc_flush_cache_time : 1.5 ms mmc_flush_cache_time : 2.5 ms
>>>> mmc_sleep_time: 1.5ms mmc_sleep_time: 2ms
>>>> mmc_power_off_time: 1.5ms mmc_power_off_time: 1.5ms
>>> The suspend time shouldn't really differ. Or is there a reason for this?
>>
>> I think this could be due to run to run variation as we can see
>> mmc_claim_host and mmc_flush_cache itself taking some extra 1ms.
>>
>>
> Hi Sarthak
>
> I have a question.
> 1.What is the difference between Partial init and Full Init on SOC
> power consumption?
> 2.Partial init and Full init improve IO performance?
> 3.Could you share the test methods if you want?


No difference in SOC power consumption between the two .. it just
improves the resume latency as mentioned in commit text.
Partial init and Full init only improves the resume latency and no IO
performance impact.
We don't have any such test methods instead just evaluated the resume
delays using ftraces.

Thanks,
Sarthak

>
>>>> 2) Kingston eMMC (ManfID 0x70)
>>>>
>>>> Partial init Full Init
>>>>
>>>> a) _mmc_resume: _mmc_resume :
>>>> Total time : 46ms Total time : 62ms
>>>> (Decrease % from full init = ~25%)
>>>>
>>>> Breakup :
>>>> mmc_claim_host_time: 0.2ms mmc_claim_host_time: 0.2ms
>>>> mmc_power_up_time: 30ms mmc_power_up_time: 30ms
>>>> mmc_sleepawake_time: 14ms mmc_init_card_time: 31ms
>>>> mmc_partial_init_time: 2ms
>>>>
>>>>
>>>> b) _mmc_suspend: _mmc_suspend:
>>>> Total time : 5ms Total: 5ms
>>>>
>>>> Breakup :
>>>> mmc_claim_host_time: 0.5ms mmc_claim_host_time: 0.5ms
>>>> mmc_flush_cache_time : 1.5 ms mmc_flush_cache_time : 1.5 ms
>>>> mmc_sleep_time: 1.5ms mmc_sleep_time: 1ms
>>>> mmc_power_off_time: 1.5ms mmc_power_off_time: 1.5ms
>>>>
>>>> Did some minor modifications as well to this patchset as per avri's comment which I'll post as V2.
>>>> Please let me know your inputs about these numbers.
>>> Thanks for posting these numbers, much appreciated! Please try to
>>> include some of the data as part of the commit message as I think it's
>>> valuable information.
>>>
>>> When it comes to reviewing the code, I am awaiting your v2 then.
>>>
>>> [...]
>>>
>>> Kind regards
>>> Uffe
>> Sure will add this data to the commit text in V2.
>>
>>