All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ulf Hansson <ulf.hansson@linaro.org>
To: Srinivasarao Pathipati <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 <quic_kamasali@quicinc.com>
Subject: Re: [PATCH V1] mmc: core: Add partial initialization support
Date: Wed, 27 Apr 2022 09:35:47 +0200	[thread overview]
Message-ID: <CAPDyKFo7H=fOiX49gttrKO3gwe=ddYMjdgrO_xSnnZjZh7P7ZA@mail.gmail.com> (raw)
In-Reply-To: <1650963852-4173-1-git-send-email-quic_spathi@quicinc.com>

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
>

  parent reply	other threads:[~2022-04-27  7:36 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-26  9:04 [PATCH V1] mmc: core: Add partial initialization support Srinivasarao Pathipati
2022-04-26 18:33 ` Avri Altman
2022-04-26 20:07 ` kernel test robot
2022-04-27  7:35 ` Ulf Hansson [this message]
2022-05-24 11:58   ` Kamasali Satyanarayan (Consultant) (QUIC)
2022-05-24 14:37     ` Sarthak Garg (QUIC)
2022-05-31  9:23       ` Sarthak Garg (QUIC)
2022-06-15 22:48       ` Ulf Hansson
2023-09-26  6:39         ` Sarthak Garg
2023-09-28  3:20           ` Wenchao Chen
2023-10-04 10:45             ` Sarthak Garg
2022-06-02  7:56     ` Ben Chuang
2023-09-26  6:31       ` Sarthak Garg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPDyKFo7H=fOiX49gttrKO3gwe=ddYMjdgrO_xSnnZjZh7P7ZA@mail.gmail.com' \
    --to=ulf.hansson@linaro.org \
    --cc=avri.altman@wdc.com \
    --cc=briannorris@chromium.org \
    --cc=digetx@gmail.com \
    --cc=huijin.park@samsung.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=merez@codeaurora.org \
    --cc=quic_kamasali@quicinc.com \
    --cc=quic_spathi@quicinc.com \
    --cc=s.shtylyov@omp.ru \
    --cc=sbhanu@codeaurora.org \
    --cc=shawn.lin@rock-chips.com \
    --cc=vbadigan@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.