Re: [PATCH V3 1/2] mmc: core: Add host specific tuning support for SD HS mode

[Ulf Hansson]

On Fri, 25 Aug 2023 at 11:18, Wenchao Chen <wenchao.chen@xxxxxxxxxx> wrote:
>
> Added .prepare_hs_tuning and .execute_hs_tuning host callbacks to
> support host-specific tuning for SD high speed mode. Howerver, these
> callbacks are optional, host specific - and that there is nothing
> in the SD spec that mentions this.
>
> Signed-off-by: Wenchao Chen <wenchao.chen@xxxxxxxxxx>
> ---
>  drivers/mmc/core/sd.c     | 14 ++++++++++++++
>  drivers/mmc/core/sd_ops.c |  1 +
>  include/linux/mmc/host.h  |  8 ++++++++
>  3 files changed, 23 insertions(+)
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 246ce027ae0a..c3e554344c99 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -1518,6 +1518,13 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
>                  */
>                 mmc_set_clock(host, mmc_sd_get_max_clock(card));
>
> +               if (host->ios.timing == MMC_TIMING_SD_HS &&
> +                       host->ops->prepare_sd_hs_tuning) {
> +                       err = host->ops->prepare_sd_hs_tuning(host, card);
> +                       if (err)
> +                               goto free_card;
> +               }
> +
>                 /*
>                  * Switch to wider bus (if supported).
>                  */
> @@ -1529,6 +1536,13 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
>
>                         mmc_set_bus_width(host, MMC_BUS_WIDTH_4);
>                 }
> +
> +               if (host->ios.timing == MMC_TIMING_SD_HS &&
> +                       host->ops->execute_sd_hs_tuning) {
> +                       err = host->ops->execute_sd_hs_tuning(host, card);
> +                       if (err)
> +                               goto free_card;
> +               }
>         }
>  cont:
>         if (!oldcard) {
> diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
> index ef8d1dce5af1..a59cd592f06e 100644
> --- a/drivers/mmc/core/sd_ops.c
> +++ b/drivers/mmc/core/sd_ops.c
> @@ -323,6 +323,7 @@ int mmc_sd_switch(struct mmc_card *card, int mode, int group,
>         return mmc_send_adtc_data(card, card->host, SD_SWITCH, cmd_args, resp,
>                                   64);
>  }
> +EXPORT_SYMBOL_GPL(mmc_sd_switch);
>
>  int mmc_app_sd_status(struct mmc_card *card, void *ssr)
>  {
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 461d1543893b..62a6847a3b6f 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -184,6 +184,12 @@ struct mmc_host_ops {
>         /* Execute HS400 tuning depending host driver */
>         int     (*execute_hs400_tuning)(struct mmc_host *host, struct mmc_card *card);
>
> +       /* Optional callback to prepare for SD high-speed tuning */
> +       int     (*prepare_sd_hs_tuning)(struct mmc_host *host, struct mmc_card *card);
> +
> +       /* Optional callback to execute SD high-speed tuning */
> +       int     (*execute_sd_hs_tuning)(struct mmc_host *host, struct mmc_card *card);
> +
>         /* Prepare switch to DDR during the HS400 init sequence */
>         int     (*hs400_prepare_ddr)(struct mmc_host *host);
>
> @@ -665,6 +671,8 @@ static inline void mmc_debugfs_err_stats_inc(struct mmc_host *host,
>         host->err_stats[stat] += 1;
>  }
>
> +int mmc_sd_switch(struct mmc_card *card, int mode, int group, u8 value, u8 *resp);

We need to drop the corresponding declaration in drivers/mmc/core/sd_ops.h.

> +int mmc_send_status(struct mmc_card *card, u32 *status);

We need to drop the corresponding declaration in drivers/mmc/core/mmc_ops.h

>  int mmc_send_tuning(struct mmc_host *host, u32 opcode, int *cmd_error);
>  int mmc_send_abort_tuning(struct mmc_host *host, u32 opcode);
>  int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd);
> --
> 2.17.1
>

Other than the small thing above, this looks good to me!

Although, no need to resend, I will amend the patch when applying.

Kind regards
Uffe