Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
MIME-Version: 1.0
References: <20231201030547.11553-1-mengqi.zhang@mediatek.com>
In-Reply-To: <20231201030547.11553-1-mengqi.zhang@mediatek.com>
From:   Ulf Hansson <ulf.hansson@linaro.org>
Date:   Thu, 7 Dec 2023 14:37:57 +0100
Message-ID: <CAPDyKFqhGULWp3P2EpxUMLvdiijqu7qxPrY40GU2m3FM7XVCVQ@mail.gmail.com>
Subject: Re: [v1,1/1] mmc: core: Add HS400es support in retune and
 initialization flow
To:     Mengqi Zhang <mengqi.zhang@mediatek.com>
Cc:     chaotian.jing@mediatek.com, matthias.bgg@gmail.com,
        wenbin.mei@mediatek.com, yangyingliang@huawei.com,
        adrian.hunter@intel.com, avri.altman@wdc.com,
        vincent.whitchurch@axis.com, linux-mmc@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, 1 Dec 2023 at 04:06, Mengqi Zhang <mengqi.zhang@mediatek.com> wrote:
>
> In HS400es mode, when we enter mmc_retune(), we will be forced to
> the HS400 mode instead of HS400es mode. So we decided to add support
> for HS400es in the mmc_retune() process.

Hmm, I was under the impression that we really don't need to do
re-tuning at all, when using HS400es mode.

Are we calling mmc_retune_enable() from a place that we shouldn't?

>
> Additionally, during the initialization to the HS400es stage, add
> hs400 tuning as an optional process.
>
> Signed-off-by: Mengqi Zhang <mengqi.zhang@mediatek.com>

Kind regards
Uffe

> ---
>  drivers/mmc/core/host.c | 11 ++++++++++-
>  drivers/mmc/core/mmc.c  | 10 +++++++++-
>  2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index 096093f7be00..4bddbbb3d539 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -179,7 +179,7 @@ EXPORT_SYMBOL(mmc_retune_release);
>  int mmc_retune(struct mmc_host *host)
>  {
>         bool return_to_hs400 = false;
> -       int err;
> +       int err = 0;
>
>         if (host->retune_now)
>                 host->retune_now = 0;
> @@ -194,6 +194,15 @@ int mmc_retune(struct mmc_host *host)
>         host->doing_retune = 1;
>
>         if (host->ios.timing == MMC_TIMING_MMC_HS400) {
> +               if (host->ios.enhanced_strobe) {
> +                       if (host->ops->execute_hs400_tuning) {
> +                               mmc_retune_disable(host);
> +                               err = host->ops->execute_hs400_tuning(host, host->card);
> +                               mmc_retune_enable(host);
> +                       }
> +                       goto out;
> +               }
> +
>                 err = mmc_hs400_to_hs200(host->card);
>                 if (err)
>                         goto out;
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 705942edacc6..9760eea2c104 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1823,7 +1823,15 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>                 if (err)
>                         goto free_card;
>
> -       } else if (!mmc_card_hs400es(card)) {
> +       } else if (mmc_card_hs400es(card)) {
> +               if (host->ops->execute_hs400_tuning) {
> +                       mmc_retune_disable(host);
> +                       err = host->ops->execute_hs400_tuning(host, card);
> +                       mmc_retune_enable(host);
> +                       if (err)
> +                               goto free_card;
> +               }
> +       } else {
>                 /* Select the desired bus width optionally */
>                 err = mmc_select_bus_width(card);
>                 if (err > 0 && mmc_card_hs(card)) {
> --
> 2.25.1
>