On Wed, 1 Apr 2020 at 21:57, Marek Vasut <marex@xxxxxxx> wrote:
>
> Instead of reimplementing the logic in mmc_set_signal_voltage(),
> use the mmc code function directly.
Again, this isn't about mmc_set_signal_voltage() but about
mmc_regulator_set_vqmmc(). Please update the changelog to reflect
that.
>
> 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. But since mmc_set_signal_voltage() would return 0, then the
> condition for calling .post_sig_volt_switch() is not satisfied if the
> switch did not happen.
>
> Signed-off-by: Marek Vasut <marex@xxxxxxx>
> Cc: Alexandre Torgue <alexandre.torgue@xxxxxx>
> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Cc: Ludovic Barre <ludovic.barre@xxxxxx>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> Cc: Maxime Coquelin <mcoquelin.stm32@xxxxxxxxx>
> Cc: Patrice Chotard <patrice.chotard@xxxxxx>
> Cc: Patrick Delaunay <patrick.delaunay@xxxxxx>
> Cc: Russell King <linux@xxxxxxxxxxxxxxx>
> Cc: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> Cc: linux-stm32@xxxxxxxxxxxxxxxxxxxxxxxxxxxx
> To: linux-mmc@xxxxxxxxxxxxxxx
> ---
> drivers/mmc/host/mmci.c | 28 ++++++----------------------
> 1 file changed, 6 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 647567def612..b8c8f0e623de 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -1861,31 +1861,15 @@ 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;
> -
> - if (!IS_ERR(mmc->supply.vqmmc)) {
> + int ret;
>
> - switch (ios->signal_voltage) {
> - case MMC_SIGNAL_VOLTAGE_330:
> - ret = regulator_set_voltage(mmc->supply.vqmmc,
> - 2700000, 3600000);
> - break;
> - case MMC_SIGNAL_VOLTAGE_180:
> - ret = regulator_set_voltage(mmc->supply.vqmmc,
> - 1700000, 1950000);
> - break;
> - case MMC_SIGNAL_VOLTAGE_120:
> - ret = regulator_set_voltage(mmc->supply.vqmmc,
> - 1100000, 1300000);
> - break;
> - }
> + ret = mmc_regulator_set_vqmmc(mmc, ios);
>
> - if (!ret && host->ops && host->ops->post_sig_volt_switch)
> - ret = host->ops->post_sig_volt_switch(host, ios);
> + if (!ret && host->ops && host->ops->post_sig_volt_switch)
I would suggest to add a comment here somewhere, that you explicitly
want to avoid calling the ->post_sig_volt_switch() unless the voltage
really changed. Just to make this clear.
> + ret = host->ops->post_sig_volt_switch(host, ios);
>
> - if (ret)
> - dev_warn(mmc_dev(mmc), "Voltage switch failed\n");
> - }
> + if (ret < 0)
> + dev_warn(mmc_dev(mmc), "Voltage switch failed\n");
>
> return ret;
> }
> --
> 2.25.1
>
Kind regards
Uffe
