diff mbox series

[V1] mmc: core: Add partial initialization support

Message ID 1650963852-4173-1-git-send-email-quic_spathi@quicinc.com (mailing list archive)
State New, archived
Headers show
Series [V1] mmc: core: Add partial initialization support | expand

Commit Message

Srinivasarao Pathipati April 26, 2022, 9:04 a.m. UTC
From: Maya Erez <merez@codeaurora.org>

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 <merez@codeaurora.org>
Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
Signed-off-by: kamasali <quic_kamasali@quicinc.com>
Signed-off-by: Srinivasarao Pathipati <quic_spathi@quicinc.com>
---
 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(-)

Comments

Avri Altman April 26, 2022, 6:33 p.m. UTC | #1
> From: Maya Erez <merez@codeaurora.org>
> 
> 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 <merez@codeaurora.org>
> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
> Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
> Signed-off-by: kamasali <quic_kamasali@quicinc.com>
> Signed-off-by: Srinivasarao Pathipati <quic_spathi@quicinc.com>
> ---
>  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
kernel test robot April 26, 2022, 8:07 p.m. UTC | #2
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/202204270458.l6hjhnD4-lkp@intel.com/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 <lkp@intel.com>

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
Ulf Hansson April 27, 2022, 7:35 a.m. UTC | #3
On Tue, 26 Apr 2022 at 11:04, Srinivasarao Pathipati
<quic_spathi@quicinc.com> wrote:
>
> From: Maya Erez <merez@codeaurora.org>
>
> 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 <merez@codeaurora.org>
> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
> Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
> Signed-off-by: kamasali <quic_kamasali@quicinc.com>
> Signed-off-by: Srinivasarao Pathipati <quic_spathi@quicinc.com>

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-git-send-email-vbadigan@codeaurora.org/

> ---
>  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
>
Kamasali Satyanarayan (Consultant) (QUIC) May 24, 2022, 11:58 a.m. UTC | #4
Hi,

These patches will be further taken by Sarthak.

Thanks,
Satya

-----Original Message-----
From: Ulf Hansson <ulf.hansson@linaro.org> 
Sent: Wednesday, April 27, 2022 1:06 PM
To: quic_spathi <quic_spathi@quicinc.com>
Cc: avri.altman@wdc.com; linus.walleij@linaro.org; shawn.lin@rock-chips.com; merez@codeaurora.org; s.shtylyov@omp.ru; huijin.park@samsung.com; briannorris@chromium.org; digetx@gmail.com; linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Veerabhadrarao Badiganti <vbadigan@codeaurora.org>; Shaik Sajida Bhanu <sbhanu@codeaurora.org>; Kamasali Satyanarayan (Consultant) (QUIC) <quic_kamasali@quicinc.com>
Subject: Re: [PATCH V1] mmc: core: Add partial initialization support

On Tue, 26 Apr 2022 at 11:04, Srinivasarao Pathipati <quic_spathi@quicinc.com> wrote:
>
> From: Maya Erez <merez@codeaurora.org>
>
> 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 <merez@codeaurora.org>
> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
> Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
> Signed-off-by: kamasali <quic_kamasali@quicinc.com>
> Signed-off-by: Srinivasarao Pathipati <quic_spathi@quicinc.com>

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-git-send-email-vbadigan@codeaurora.org/

> ---
>  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
>
Sarthak Garg May 24, 2022, 2:37 p.m. UTC | #5
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)
> <quic_kamasali@quicinc.com>
> Sent: Tuesday, May 24, 2022 5:29 PM
> To: 'Ulf Hansson' <ulf.hansson@linaro.org>; Sarthak Garg (QUIC)
> <quic_sartgarg@quicinc.com>
> Cc: avri.altman@wdc.com; linus.walleij@linaro.org; shawn.lin@rock-chips.com;
> merez@codeaurora.org; s.shtylyov@omp.ru; huijin.park@samsung.com;
> briannorris@chromium.org; digetx@gmail.com; linux-mmc@vger.kernel.org;
> linux-kernel@vger.kernel.org; Veerabhadrarao Badiganti
> <vbadigan@codeaurora.org>; Shaik Sajida Bhanu <sbhanu@codeaurora.org>;
> 'quic_spathi@quicinc.com' <quic_spathi@quicinc.com>
> 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 <ulf.hansson@linaro.org>
> Sent: Wednesday, April 27, 2022 1:06 PM
> To: quic_spathi <quic_spathi@quicinc.com>
> Cc: avri.altman@wdc.com; linus.walleij@linaro.org; shawn.lin@rock-chips.com;
> merez@codeaurora.org; s.shtylyov@omp.ru; huijin.park@samsung.com;
> briannorris@chromium.org; digetx@gmail.com; linux-mmc@vger.kernel.org;
> linux-kernel@vger.kernel.org; Veerabhadrarao Badiganti
> <vbadigan@codeaurora.org>; Shaik Sajida Bhanu <sbhanu@codeaurora.org>;
> Kamasali Satyanarayan (Consultant) (QUIC) <quic_kamasali@quicinc.com>
> Subject: Re: [PATCH V1] mmc: core: Add partial initialization support
> 
> On Tue, 26 Apr 2022 at 11:04, Srinivasarao Pathipati <quic_spathi@quicinc.com>
> wrote:
> >
> > From: Maya Erez <merez@codeaurora.org>
> >
> > 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 <merez@codeaurora.org>
> > Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
> > Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
> > Signed-off-by: kamasali <quic_kamasali@quicinc.com>
> > Signed-off-by: Srinivasarao Pathipati <quic_spathi@quicinc.com>
> 
> 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-
> git-send-email-vbadigan@codeaurora.org/
> 
> > ---
> >  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
> >
Sarthak Garg May 31, 2022, 9:23 a.m. UTC | #6
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) <quic_kamasali@quicinc.com>;
> 'Ulf Hansson' <ulf.hansson@linaro.org>
> Cc: avri.altman@wdc.com; linus.walleij@linaro.org; shawn.lin@rock-chips.com;
> merez@codeaurora.org; s.shtylyov@omp.ru; huijin.park@samsung.com;
> briannorris@chromium.org; digetx@gmail.com; linux-mmc@vger.kernel.org;
> linux-kernel@vger.kernel.org; Veerabhadrarao Badiganti
> <vbadigan@codeaurora.org>; Shaik Sajida Bhanu <sbhanu@codeaurora.org>;
> 'quic_spathi@quicinc.com' <quic_spathi@quicinc.com>
> 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)
> > <quic_kamasali@quicinc.com>
> > Sent: Tuesday, May 24, 2022 5:29 PM
> > To: 'Ulf Hansson' <ulf.hansson@linaro.org>; Sarthak Garg (QUIC)
> > <quic_sartgarg@quicinc.com>
> > Cc: avri.altman@wdc.com; linus.walleij@linaro.org;
> > shawn.lin@rock-chips.com; merez@codeaurora.org; s.shtylyov@omp.ru;
> > huijin.park@samsung.com; briannorris@chromium.org; digetx@gmail.com;
> > linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org;
> > Veerabhadrarao Badiganti <vbadigan@codeaurora.org>; Shaik Sajida Bhanu
> > <sbhanu@codeaurora.org>; 'quic_spathi@quicinc.com'
> > <quic_spathi@quicinc.com>
> > 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 <ulf.hansson@linaro.org>
> > Sent: Wednesday, April 27, 2022 1:06 PM
> > To: quic_spathi <quic_spathi@quicinc.com>
> > Cc: avri.altman@wdc.com; linus.walleij@linaro.org;
> > shawn.lin@rock-chips.com; merez@codeaurora.org; s.shtylyov@omp.ru;
> > huijin.park@samsung.com; briannorris@chromium.org; digetx@gmail.com;
> > linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org;
> > Veerabhadrarao Badiganti <vbadigan@codeaurora.org>; Shaik Sajida Bhanu
> > <sbhanu@codeaurora.org>; Kamasali Satyanarayan (Consultant) (QUIC)
> > <quic_kamasali@quicinc.com>
> > Subject: Re: [PATCH V1] mmc: core: Add partial initialization support
> >
> > On Tue, 26 Apr 2022 at 11:04, Srinivasarao Pathipati
> > <quic_spathi@quicinc.com>
> > wrote:
> > >
> > > From: Maya Erez <merez@codeaurora.org>
> > >
> > > 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 <merez@codeaurora.org>
> > > Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
> > > Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
> > > Signed-off-by: kamasali <quic_kamasali@quicinc.com>
> > > Signed-off-by: Srinivasarao Pathipati <quic_spathi@quicinc.com>
> >
> > 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
> > - git-send-email-vbadigan@codeaurora.org/
> >
> > > ---
> > >  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
> > >
Ben Chuang June 2, 2022, 7:56 a.m. UTC | #7
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) <quic_kamasali@quicinc.com> wrote:
>
> Hi,
>
> These patches will be further taken by Sarthak.
>
> Thanks,
> Satya
>
> -----Original Message-----
> From: Ulf Hansson <ulf.hansson@linaro.org>
> Sent: Wednesday, April 27, 2022 1:06 PM
> To: quic_spathi <quic_spathi@quicinc.com>
> Cc: avri.altman@wdc.com; linus.walleij@linaro.org; shawn.lin@rock-chips.com; merez@codeaurora.org; s.shtylyov@omp.ru; huijin.park@samsung.com; briannorris@chromium.org; digetx@gmail.com; linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Veerabhadrarao Badiganti <vbadigan@codeaurora.org>; Shaik Sajida Bhanu <sbhanu@codeaurora.org>; Kamasali Satyanarayan (Consultant) (QUIC) <quic_kamasali@quicinc.com>
> Subject: Re: [PATCH V1] mmc: core: Add partial initialization support
>
> On Tue, 26 Apr 2022 at 11:04, Srinivasarao Pathipati <quic_spathi@quicinc.com> wrote:
> >
> > From: Maya Erez <merez@codeaurora.org>
> >
> > 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 <merez@codeaurora.org>
> > Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
> > Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
> > Signed-off-by: kamasali <quic_kamasali@quicinc.com>
> > Signed-off-by: Srinivasarao Pathipati <quic_spathi@quicinc.com>
>
> 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-git-send-email-vbadigan@codeaurora.org/
>
> > ---
> >  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
> >
Ulf Hansson June 15, 2022, 10:48 p.m. UTC | #8
On Tue, 24 May 2022 at 07:37, Sarthak Garg (QUIC)
<quic_sartgarg@quicinc.com> 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
Sarthak Garg Sept. 26, 2023, 6:31 a.m. UTC | #9
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) <quic_kamasali@quicinc.com> wrote:
>> Hi,
>>
>> These patches will be further taken by Sarthak.
>>
>> Thanks,
>> Satya
>>
>> -----Original Message-----
>> From: Ulf Hansson <ulf.hansson@linaro.org>
>> Sent: Wednesday, April 27, 2022 1:06 PM
>> To: quic_spathi <quic_spathi@quicinc.com>
>> Cc: avri.altman@wdc.com; linus.walleij@linaro.org; shawn.lin@rock-chips.com; merez@codeaurora.org; s.shtylyov@omp.ru; huijin.park@samsung.com; briannorris@chromium.org; digetx@gmail.com; linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Veerabhadrarao Badiganti <vbadigan@codeaurora.org>; Shaik Sajida Bhanu <sbhanu@codeaurora.org>; Kamasali Satyanarayan (Consultant) (QUIC) <quic_kamasali@quicinc.com>
>> Subject: Re: [PATCH V1] mmc: core: Add partial initialization support
>>
>> On Tue, 26 Apr 2022 at 11:04, Srinivasarao Pathipati <quic_spathi@quicinc.com> wrote:
>>> From: Maya Erez <merez@codeaurora.org>
>>>
>>> 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 <merez@codeaurora.org>
>>> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
>>> Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
>>> Signed-off-by: kamasali <quic_kamasali@quicinc.com>
>>> Signed-off-by: Srinivasarao Pathipati <quic_spathi@quicinc.com>
>> 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-git-send-email-vbadigan@codeaurora.org/
>>
>>> ---
>>>   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
>>>
Sarthak Garg Sept. 26, 2023, 6:39 a.m. UTC | #10
On 6/16/2022 4:18 AM, Ulf Hansson wrote:
> On Tue, 24 May 2022 at 07:37, Sarthak Garg (QUIC)
> <quic_sartgarg@quicinc.com> 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.
Wenchao Chen Sept. 28, 2023, 3:20 a.m. UTC | #11
On Tue, 26 Sept 2023 at 18:41, Sarthak Garg <quic_sartgarg@quicinc.com> wrote:
>
>
> On 6/16/2022 4:18 AM, Ulf Hansson wrote:
> > On Tue, 24 May 2022 at 07:37, Sarthak Garg (QUIC)
> > <quic_sartgarg@quicinc.com> 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.
>
>
Sarthak Garg Oct. 4, 2023, 10:45 a.m. UTC | #12
On 9/28/2023 8:50 AM, Wenchao Chen wrote:
> On Tue, 26 Sept 2023 at 18:41, Sarthak Garg <quic_sartgarg@quicinc.com> wrote:
>>
>> On 6/16/2022 4:18 AM, Ulf Hansson wrote:
>>> On Tue, 24 May 2022 at 07:37, Sarthak Garg (QUIC)
>>> <quic_sartgarg@quicinc.com> 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.
>>
>>
diff mbox series

Patch

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;