Re: [PATCH V4 2/2] mmc: sdhci-msm: Use internal voltage control
From: Veerabhadrarao Badiganti
Date: Thu Jun 18 2020 - 02:30:26 EST
On 6/17/2020 7:46 PM, Ulf Hansson wrote:
On Wed, 17 Jun 2020 at 14:46, Veerabhadrarao Badiganti
<vbadigan@xxxxxxxxxxxxxx> wrote:
Thanks for comments Uffe.
On 6/17/2020 3:04 PM, Ulf Hansson wrote:
On Tue, 16 Jun 2020 at 17:38, Veerabhadrarao Badiganti
<vbadigan@xxxxxxxxxxxxxx> wrote:
On qcom SD host controllers voltage switching be done after the HW
is ready for it. The HW informs its readiness through power irq.
The voltage switching should happen only then.
Use the internal voltage switching and then control the voltage
switching using power irq.
IO-bus supply of eMMC would be kept always-on. So set the load
for this supply to configure it in LPM when eMMC is suspend state
and in HPM when eMMC is active.
Co-developed-by: Asutosh Das <asutoshd@xxxxxxxxxxxxxx>
Signed-off-by: Asutosh Das <asutoshd@xxxxxxxxxxxxxx>
Co-developed-by: Vijay Viswanath <vviswana@xxxxxxxxxxxxxx>
Signed-off-by: Vijay Viswanath <vviswana@xxxxxxxxxxxxxx>
Co-developed-by: Veerabhadrarao Badiganti <vbadigan@xxxxxxxxxxxxxx>
Signed-off-by: Veerabhadrarao Badiganti <vbadigan@xxxxxxxxxxxxxx>
---
drivers/mmc/host/sdhci-msm.c | 208 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 199 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 15c42b059240..8297b2142748 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -37,7 +37,9 @@
#define CORE_PWRCTL_IO_LOW BIT(2)
#define CORE_PWRCTL_IO_HIGH BIT(3)
#define CORE_PWRCTL_BUS_SUCCESS BIT(0)
+#define CORE_PWRCTL_BUS_FAIL BIT(1)
#define CORE_PWRCTL_IO_SUCCESS BIT(2)
+#define CORE_PWRCTL_IO_FAIL BIT(3)
#define REQ_BUS_OFF BIT(0)
#define REQ_BUS_ON BIT(1)
#define REQ_IO_LOW BIT(2)
@@ -127,6 +129,9 @@
/* Timeout value to avoid infinite waiting for pwr_irq */
#define MSM_PWR_IRQ_TIMEOUT_MS 5000
+/* Max load for eMMC Vdd-io supply */
+#define MMC_VQMMC_MAX_LOAD_UA 325000
+
#define msm_host_readl(msm_host, host, offset) \
msm_host->var_ops->msm_readl_relaxed(host, offset)
@@ -278,6 +283,7 @@ struct sdhci_msm_host {
bool uses_tassadar_dll;
u32 dll_config;
u32 ddr_config;
+ bool vqmmc_enabled;
};
static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
@@ -1346,6 +1352,88 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
sdhci_msm_hs400(host, &mmc->ios);
}
+static int sdhci_msm_set_vmmc(struct mmc_host *mmc)
+{
+ if (IS_ERR(mmc->supply.vmmc))
+ return 0;
+
+ return mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, mmc->ios.vdd);
+}
+
+static int msm_toggle_vqmmc(struct sdhci_msm_host *msm_host,
+ struct mmc_host *mmc, bool level)
+{
+ int ret;
+ struct mmc_ios ios;
+
+ if (msm_host->vqmmc_enabled == level)
+ return 0;
+
+ if (level) {
+ /* Set the IO voltage regulator to default voltage level */
+ if (msm_host->caps_0 & CORE_3_0V_SUPPORT)
+ ios.signal_voltage = MMC_SIGNAL_VOLTAGE_330;
+ else if (msm_host->caps_0 & CORE_1_8V_SUPPORT)
+ ios.signal_voltage = MMC_SIGNAL_VOLTAGE_180;
+
+ if (msm_host->caps_0 & CORE_VOLT_SUPPORT) {
+ ret = mmc_regulator_set_vqmmc(mmc, &ios);
+ if (ret < 0) {
+ dev_err(mmc_dev(mmc), "%s: vqmmc set volgate failed: %d\n",
+ mmc_hostname(mmc), ret);
+ goto out;
+ }
+ }
+ ret = regulator_enable(mmc->supply.vqmmc);
+ } else {
+ ret = regulator_disable(mmc->supply.vqmmc);
+ }
+
+ if (ret)
+ dev_err(mmc_dev(mmc), "%s: vqmm %sable failed: %d\n",
+ mmc_hostname(mmc), level ? "en":"dis", ret);
+ else
+ msm_host->vqmmc_enabled = level;
+out:
+ return ret;
+}
+
+static int msm_config_vqmmc_mode(struct sdhci_msm_host *msm_host,
+ struct mmc_host *mmc, bool hpm)
+{
+ int load, ret;
+
+ load = hpm ? MMC_VQMMC_MAX_LOAD_UA : 0;
+ ret = regulator_set_load(mmc->supply.vqmmc, load);
+ if (ret)
+ dev_err(mmc_dev(mmc), "%s: vqmmc set load failed: %d\n",
+ mmc_hostname(mmc), ret);
+ return ret;
+}
+
+static int sdhci_msm_set_vqmmc(struct sdhci_msm_host *msm_host,
+ struct mmc_host *mmc, bool level)
+{
+ int ret;
+ bool always_on;
+
+ if (IS_ERR(mmc->supply.vqmmc) ||
White space.
+ (mmc->ios.power_mode == MMC_POWER_UNDEFINED))
+ return 0;
+ /*
+ * For eMMC don't turn off Vqmmc, Instead just configure it in LPM
+ * and HPM modes by setting the right amonut of load.
/s/right amonut of load/corresponding load
+ */
+ always_on = mmc->card && mmc_card_mmc(mmc->card);
+
+ if (always_on)
+ ret = msm_config_vqmmc_mode(msm_host, mmc, level);
+ else
+ ret = msm_toggle_vqmmc(msm_host, mmc, level);
I am worried that this isn't really doing what you think it does.
always_on may not always be set for an eMMC.
This is because the mmc->card doesn't get assigned until the
initialization of the eMMC has been completed. In other words, way
after VCC and VCCQ have been turned on and changed voltage levels.
For the very first time, i have to enable the vqmmc.
And second time on-wards I have to set load instead of turning vqmmc off
/ on.
This condition helps me in doing exactly the same.
This gets invoked first time when mmc_power_up() is called.
Yes, by that time card won't be initialized. So this check fails and
allows to turn on the regulator instead of setting load.
After that, this gets invoked only next time mmc_power_up/off() is called
which is during mmc_suspend/resume.
I see.
That assumes the initialization always works at the first attempt, so
no re-try or power cycle gets done in some error/reset path.
By this time card is initialized, it allows to set load.
Okay, I get it. Thanks for clarifying.
Perhaps you can add a small comment about this in the code, so we
don't forget the reasons why the code looks like this.
Sure. Will update comment with this info.
Moreover, mmc_card_mmc() is also about legacy MMC cards, not solely for eMMCs.
Okay. But on qcom platform, we have support for only eMMC/SD/SDIO.
So this mmc_card_mmc() meets my requirements!!! Allows only eMMC not SD
& SDIO.
Well, eMMC is fairly similar to MMC, from a basic functionality point
of view. So, I am quite sure you can get it to work with your
controller, even if it's not officially supported.
If you want special treatment of an eMMC, I think it's better to use
the DT bindings Documentation/devicetree/bindings/mmc/mmc-card.txt. If
you have such a subnode, that indicates that there is an eMMC card
attached.
That said, if you don't like the pure DT based solution, as I
suggested above - at least extend mmc_card_mmc() with checking the
MMC_CAP_NONREMOVABLE flag.
sure. Will update it. Thank you.
[...]
Kind regards
Uffe