| Message ID | 20200401195722.208157-3-marex@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] mmc: Prepare all code for mmc_set_signal_voltage() returning > 0 | expand |
On Wed, 1 Apr 2020 at 21:57, Marek Vasut <marex@denx.de> 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@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 | 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
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) + 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; }
Instead of reimplementing the logic in mmc_set_signal_voltage(), use the mmc code function directly. 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@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 | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-)