Re: [PATCH] mmc: sdhci-cadence: do not use hardware tuning for SD mode

[Ulf Hansson]

On Mon, 20 Jul 2020 at 08:12, Masahiro Yamada
<yamada.masahiro@xxxxxxxxxxxxx> wrote:
>
> As commit ef6b75671b5f ("mmc: sdhci-cadence: send tune request twice to
> work around errata") stated, this IP has an errata. This commit applies
> the second workaround for the SD mode.
>
> Due to the errata, it is not possible to use the hardware tuning provided
> by SDHCI_HOST_CONTROL2.
>
> Use the software-controlled tuning like the eMMC mode.
>
> Set sdhci_host_ops::platform_execute_tuning instead of overriding
> mmc_host_ops::execute_tuning.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>

Applied for next (a while ago), thanks!
Kind regards
Uffe


> ---
>
>  drivers/mmc/host/sdhci-cadence.c | 123 ++++++++++++++++---------------
>  1 file changed, 62 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index 4a6c9ba82538..4d9f7681817c 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -202,57 +202,6 @@ static u32 sdhci_cdns_get_emmc_mode(struct sdhci_cdns_priv *priv)
>         return FIELD_GET(SDHCI_CDNS_HRS06_MODE, tmp);
>  }
>
> -static void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host,
> -                                        unsigned int timing)
> -{
> -       struct sdhci_cdns_priv *priv = sdhci_cdns_priv(host);
> -       u32 mode;
> -
> -       switch (timing) {
> -       case MMC_TIMING_MMC_HS:
> -               mode = SDHCI_CDNS_HRS06_MODE_MMC_SDR;
> -               break;
> -       case MMC_TIMING_MMC_DDR52:
> -               mode = SDHCI_CDNS_HRS06_MODE_MMC_DDR;
> -               break;
> -       case MMC_TIMING_MMC_HS200:
> -               mode = SDHCI_CDNS_HRS06_MODE_MMC_HS200;
> -               break;
> -       case MMC_TIMING_MMC_HS400:
> -               if (priv->enhanced_strobe)
> -                       mode = SDHCI_CDNS_HRS06_MODE_MMC_HS400ES;
> -               else
> -                       mode = SDHCI_CDNS_HRS06_MODE_MMC_HS400;
> -               break;
> -       default:
> -               mode = SDHCI_CDNS_HRS06_MODE_SD;
> -               break;
> -       }
> -
> -       sdhci_cdns_set_emmc_mode(priv, mode);
> -
> -       /* For SD, fall back to the default handler */
> -       if (mode == SDHCI_CDNS_HRS06_MODE_SD)
> -               sdhci_set_uhs_signaling(host, timing);
> -}
> -
> -static const struct sdhci_ops sdhci_cdns_ops = {
> -       .set_clock = sdhci_set_clock,
> -       .get_timeout_clock = sdhci_cdns_get_timeout_clock,
> -       .set_bus_width = sdhci_set_bus_width,
> -       .reset = sdhci_reset,
> -       .set_uhs_signaling = sdhci_cdns_set_uhs_signaling,
> -};
> -
> -static const struct sdhci_pltfm_data sdhci_cdns_uniphier_pltfm_data = {
> -       .ops = &sdhci_cdns_ops,
> -       .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> -};
> -
> -static const struct sdhci_pltfm_data sdhci_cdns_pltfm_data = {
> -       .ops = &sdhci_cdns_ops,
> -};
> -
>  static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val)
>  {
>         struct sdhci_cdns_priv *priv = sdhci_cdns_priv(host);
> @@ -286,23 +235,24 @@ static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val)
>         return 0;
>  }
>
> -static int sdhci_cdns_execute_tuning(struct mmc_host *mmc, u32 opcode)
> +/*
> + * In SD mode, software must not use the hardware tuning and instead perform
> + * an almost identical procedure to eMMC.
> + */
> +static int sdhci_cdns_execute_tuning(struct sdhci_host *host, u32 opcode)
>  {
> -       struct sdhci_host *host = mmc_priv(mmc);
>         int cur_streak = 0;
>         int max_streak = 0;
>         int end_of_streak = 0;
>         int i;
>
>         /*
> -        * This handler only implements the eMMC tuning that is specific to
> -        * this controller.  Fall back to the standard method for SD timing.
> +        * Do not execute tuning for UHS_SDR50 or UHS_DDR50.
> +        * The delay is set by probe, based on the DT properties.
>          */
> -       if (host->timing != MMC_TIMING_MMC_HS200)
> -               return sdhci_execute_tuning(mmc, opcode);
> -
> -       if (WARN_ON(opcode != MMC_SEND_TUNING_BLOCK_HS200))
> -               return -EINVAL;
> +       if (host->timing != MMC_TIMING_MMC_HS200 &&
> +           host->timing != MMC_TIMING_UHS_SDR104)
> +               return 0;
>
>         for (i = 0; i < SDHCI_CDNS_MAX_TUNING_LOOP; i++) {
>                 if (sdhci_cdns_set_tune_val(host, i) ||
> @@ -325,6 +275,58 @@ static int sdhci_cdns_execute_tuning(struct mmc_host *mmc, u32 opcode)
>         return sdhci_cdns_set_tune_val(host, end_of_streak - max_streak / 2);
>  }
>
> +static void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host,
> +                                        unsigned int timing)
> +{
> +       struct sdhci_cdns_priv *priv = sdhci_cdns_priv(host);
> +       u32 mode;
> +
> +       switch (timing) {
> +       case MMC_TIMING_MMC_HS:
> +               mode = SDHCI_CDNS_HRS06_MODE_MMC_SDR;
> +               break;
> +       case MMC_TIMING_MMC_DDR52:
> +               mode = SDHCI_CDNS_HRS06_MODE_MMC_DDR;
> +               break;
> +       case MMC_TIMING_MMC_HS200:
> +               mode = SDHCI_CDNS_HRS06_MODE_MMC_HS200;
> +               break;
> +       case MMC_TIMING_MMC_HS400:
> +               if (priv->enhanced_strobe)
> +                       mode = SDHCI_CDNS_HRS06_MODE_MMC_HS400ES;
> +               else
> +                       mode = SDHCI_CDNS_HRS06_MODE_MMC_HS400;
> +               break;
> +       default:
> +               mode = SDHCI_CDNS_HRS06_MODE_SD;
> +               break;
> +       }
> +
> +       sdhci_cdns_set_emmc_mode(priv, mode);
> +
> +       /* For SD, fall back to the default handler */
> +       if (mode == SDHCI_CDNS_HRS06_MODE_SD)
> +               sdhci_set_uhs_signaling(host, timing);
> +}
> +
> +static const struct sdhci_ops sdhci_cdns_ops = {
> +       .set_clock = sdhci_set_clock,
> +       .get_timeout_clock = sdhci_cdns_get_timeout_clock,
> +       .set_bus_width = sdhci_set_bus_width,
> +       .reset = sdhci_reset,
> +       .platform_execute_tuning = sdhci_cdns_execute_tuning,
> +       .set_uhs_signaling = sdhci_cdns_set_uhs_signaling,
> +};
> +
> +static const struct sdhci_pltfm_data sdhci_cdns_uniphier_pltfm_data = {
> +       .ops = &sdhci_cdns_ops,
> +       .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +};
> +
> +static const struct sdhci_pltfm_data sdhci_cdns_pltfm_data = {
> +       .ops = &sdhci_cdns_ops,
> +};
> +
>  static void sdhci_cdns_hs400_enhanced_strobe(struct mmc_host *mmc,
>                                              struct mmc_ios *ios)
>  {
> @@ -385,7 +387,6 @@ static int sdhci_cdns_probe(struct platform_device *pdev)
>         priv->hrs_addr = host->ioaddr;
>         priv->enhanced_strobe = false;
>         host->ioaddr += SDHCI_CDNS_SRS_BASE;
> -       host->mmc_host_ops.execute_tuning = sdhci_cdns_execute_tuning;
>         host->mmc_host_ops.hs400_enhanced_strobe =
>                                 sdhci_cdns_hs400_enhanced_strobe;
>         sdhci_enable_v4_mode(host);
> --
> 2.25.1
>