Re: [PATCH] mmc: mmci: Only call .post_sig_volt_switch if voltage switch happened

[Ulf Hansson <ulf.hansson@linaro.org>]

On Tue, 31 Mar 2020 at 17:53, Marek Vasut <marex@denx.de> wrote:
>
> Call the post voltage switch handler only if the voltage switch actually
> happened. It is well possible that the regulator is already set correctly
> and no voltage switch happened, so there is no need to take any further
> action.
>
> This fixes a real issue on STM32MP1 where, if the eMMC is supplied with
> VccQ=1.8 V, the post voltage switch code will spin indefinitelly waiting
> for the voltage switch to complete, even though no voltage switch really
> happened.

Whether this is a common problem or not, I think in a first step we
should manage this in the common mmc_regulator_set_vqmmc().

Then on top of that, convert mmci into using the mmc_regulator_set_vqmmc() API.

Can please try this approach instead?

Kind regards
Uffe

>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Ludovic Barre <ludovic.barre@st.com>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Patrice Chotard <patrice.chotard@st.com>
> Cc: Patrick Delaunay <patrick.delaunay@st.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: linux-stm32@st-md-mailman.stormreply.com
> To: linux-mmc@vger.kernel.org
> ---
>  drivers/mmc/host/mmci.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 647567def612..11c2f417cbe8 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -1861,10 +1861,12 @@ static int mmci_get_cd(struct mmc_host *mmc)
>  static int mmci_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
>  {
>         struct mmci_host *host = mmc_priv(mmc);
> -       int ret = 0;
> +       int old_voltage, new_voltage, ret = 0;
>
>         if (!IS_ERR(mmc->supply.vqmmc)) {
>
> +               old_voltage = regulator_get_voltage(mmc->supply.vqmmc);
> +
>                 switch (ios->signal_voltage) {
>                 case MMC_SIGNAL_VOLTAGE_330:
>                         ret = regulator_set_voltage(mmc->supply.vqmmc,
> @@ -1880,7 +1882,10 @@ static int mmci_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
>                         break;
>                 }
>
> -               if (!ret && host->ops && host->ops->post_sig_volt_switch)
> +               new_voltage = regulator_get_voltage(mmc->supply.vqmmc);
> +
> +               if (!ret && old_voltage != new_voltage &&
> +                   host->ops && host->ops->post_sig_volt_switch)
>                         ret = host->ops->post_sig_volt_switch(host, ios);
>
>                 if (ret)
> --
> 2.25.1
>