Re: [PATCH v2] Revert "mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage_switch"

[Ulf Hansson]

On Mon, 27 Jan 2025 at 21:12, Josua Mayer <josua@xxxxxxxxxxxxx> wrote:
>
> This reverts commit 941a7abd4666912b84ab209396fdb54b0dae685d.
>
> This commit uses presence of device-tree properties vmmc-supply and
> vqmmc-supply for deciding whether to enable a quirk affecting timing of
> clock and data.
> The intention was to address issues observed with eMMC and SD on AM62
> platforms.
>
> This new quirk is however also enabled for AM64 breaking microSD access
> on the SolidRun HimmingBoard-T which is supported in-tree since v6.11,
> causing a regression. During boot microSD initialization now fails with
> the error below:
>
> [    2.008520] mmc1: SDHCI controller on fa00000.mmc [fa00000.mmc] using ADMA 64-bit
> [    2.115348] mmc1: error -110 whilst initialising SD card
>
> The heuristics for enabling the quirk are clearly not correct as they
> break at least one but potentially many existing boards.
>
> Revert the change and restore original behaviour until a more
> appropriate method of selecting the quirk is derived.
>
> Fixes: 941a7abd4666 ("mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage_switch")
> Closes: https://lore.kernel.org/linux-mmc/a70fc9fc-186f-4165-a652-3de50733763a@xxxxxxxxxxxxx/
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Josua Mayer <josua@xxxxxxxxxxxxx>
> Acked-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>

Applied this for fixes, thanks!

Kind regards
Uffe



> ---
> Changes in v2:
> - Fixed "Fixes:" tag invalid commit description copied from history
>   (Reported-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>)
>   (Reported-by: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>)
> - Link to v1: https://lore.kernel.org/r/20250127-am654-mmc-regression-v1-1-d831f9a13ae9@xxxxxxxxxxxxx
> ---
>  drivers/mmc/host/sdhci_am654.c | 30 ------------------------------
>  1 file changed, 30 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> index b73f673db92bbc042392995e715815e15ace6005..f75c31815ab00d17b5757063521f56ba5643babe 100644
> --- a/drivers/mmc/host/sdhci_am654.c
> +++ b/drivers/mmc/host/sdhci_am654.c
> @@ -155,7 +155,6 @@ struct sdhci_am654_data {
>         u32 tuning_loop;
>
>  #define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0)
> -#define SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA BIT(1)
>  };
>
>  struct window {
> @@ -357,29 +356,6 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,
>         sdhci_set_clock(host, clock);
>  }
>
> -static int sdhci_am654_start_signal_voltage_switch(struct mmc_host *mmc, struct mmc_ios *ios)
> -{
> -       struct sdhci_host *host = mmc_priv(mmc);
> -       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> -       struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> -       int ret;
> -
> -       if ((sdhci_am654->quirks & SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA) &&
> -           ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
> -               if (!IS_ERR(mmc->supply.vqmmc)) {
> -                       ret = mmc_regulator_set_vqmmc(mmc, ios);
> -                       if (ret < 0) {
> -                               pr_err("%s: Switching to 1.8V signalling voltage failed,\n",
> -                                      mmc_hostname(mmc));
> -                               return -EIO;
> -                       }
> -               }
> -               return 0;
> -       }
> -
> -       return sdhci_start_signal_voltage_switch(mmc, ios);
> -}
> -
>  static u8 sdhci_am654_write_power_on(struct sdhci_host *host, u8 val, int reg)
>  {
>         writeb(val, host->ioaddr + reg);
> @@ -868,11 +844,6 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,
>         if (device_property_read_bool(dev, "ti,fails-without-test-cd"))
>                 sdhci_am654->quirks |= SDHCI_AM654_QUIRK_FORCE_CDTEST;
>
> -       /* Suppress v1p8 ena for eMMC and SD with vqmmc supply */
> -       if (!!of_parse_phandle(dev->of_node, "vmmc-supply", 0) ==
> -           !!of_parse_phandle(dev->of_node, "vqmmc-supply", 0))
> -               sdhci_am654->quirks |= SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA;
> -
>         sdhci_get_of_property(pdev);
>
>         return 0;
> @@ -969,7 +940,6 @@ static int sdhci_am654_probe(struct platform_device *pdev)
>                 goto err_pltfm_free;
>         }
>
> -       host->mmc_host_ops.start_signal_voltage_switch = sdhci_am654_start_signal_voltage_switch;
>         host->mmc_host_ops.execute_tuning = sdhci_am654_execute_tuning;
>
>         pm_runtime_get_noresume(dev);
>
> ---
> base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
> change-id: 20250127-am654-mmc-regression-ed289f8967c2
>
> Best regards,
> --
> Josua Mayer <josua@xxxxxxxxxxxxx>
>