[PATCH V3 1/3] mmc: sdhci: Allow platform controlled voltage switching

[PATCH V3 1/3] mmc: sdhci: Allow platform controlled voltage switching

[Veerabhadrarao Badiganti]

From: Vijay Viswanath <vviswana@codeaurora.org>

Some controllers can have internal mechanism to inform the SW that it
is ready for voltage switching. For such controllers, changing voltage
before the HW is ready can result in various issues.

During setup/cleanup of host, check whether regulator enable/disable
was already done by platform driver.

Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
---
 drivers/mmc/host/sdhci.c | 32 +++++++++++++++++++-------------
 drivers/mmc/host/sdhci.h |  1 +
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 99bdae5..ea7ce1d 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -3616,6 +3616,7 @@ int sdhci_setup_host(struct sdhci_host *host)
 	unsigned int override_timeout_clk;
 	u32 max_clk;
 	int ret;
+	bool enable_vqmmc = false;
 
 	WARN_ON(host == NULL);
 	if (host == NULL)
@@ -3629,9 +3630,12 @@ int sdhci_setup_host(struct sdhci_host *host)
 	 * the host can take the appropriate action if regulators are not
 	 * available.
 	 */
-	ret = mmc_regulator_get_supply(mmc);
-	if (ret)
-		return ret;
+	if (!mmc->supply.vqmmc) {
+		ret = mmc_regulator_get_supply(mmc);
+		if (ret)
+			return ret;
+		enable_vqmmc  = true;
+	}
 
 	DBG("Version:   0x%08x | Present:  0x%08x\n",
 	    sdhci_readw(host, SDHCI_HOST_VERSION),
@@ -3880,7 +3884,15 @@ int sdhci_setup_host(struct sdhci_host *host)
 		mmc->caps |= MMC_CAP_NEEDS_POLL;
 
 	if (!IS_ERR(mmc->supply.vqmmc)) {
-		ret = regulator_enable(mmc->supply.vqmmc);
+		if (enable_vqmmc) {
+			ret = regulator_enable(mmc->supply.vqmmc);
+			if (ret) {
+				pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
+					mmc_hostname(mmc), ret);
+				mmc->supply.vqmmc = ERR_PTR(-EINVAL);
+			}
+			host->vqmmc_enabled = !ret;
+		}
 
 		/* If vqmmc provides no 1.8V signalling, then there's no UHS */
 		if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 1700000,
@@ -3893,12 +3905,6 @@ int sdhci_setup_host(struct sdhci_host *host)
 		if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 2700000,
 						    3600000))
 			host->flags &= ~SDHCI_SIGNALING_330;
-
-		if (ret) {
-			pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
-				mmc_hostname(mmc), ret);
-			mmc->supply.vqmmc = ERR_PTR(-EINVAL);
-		}
 	}
 
 	if (host->quirks2 & SDHCI_QUIRK2_NO_1_8_V) {
@@ -4136,7 +4142,7 @@ int sdhci_setup_host(struct sdhci_host *host)
 	return 0;
 
 unreg:
-	if (!IS_ERR(mmc->supply.vqmmc))
+	if (host->vqmmc_enabled)
 		regulator_disable(mmc->supply.vqmmc);
 undma:
 	if (host->align_buffer)
@@ -4154,7 +4160,7 @@ void sdhci_cleanup_host(struct sdhci_host *host)
 {
 	struct mmc_host *mmc = host->mmc;
 
-	if (!IS_ERR(mmc->supply.vqmmc))
+	if (host->vqmmc_enabled)
 		regulator_disable(mmc->supply.vqmmc);
 
 	if (host->align_buffer)
@@ -4287,7 +4293,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
 
 	tasklet_kill(&host->finish_tasklet);
 
-	if (!IS_ERR(mmc->supply.vqmmc))
+	if (host->vqmmc_enabled)
 		regulator_disable(mmc->supply.vqmmc);
 
 	if (host->align_buffer)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index b001cf4..3c28152 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -524,6 +524,7 @@ struct sdhci_host {
 	bool pending_reset;	/* Cmd/data reset is pending */
 	bool irq_wake_enabled;	/* IRQ wakeup is enabled */
 	bool v4_mode;		/* Host Version 4 Enable */
+	bool vqmmc_enabled;	/* Vqmmc is enabled */
 
 	struct mmc_request *mrqs_done[SDHCI_MAX_MRQS];	/* Requests done */
 	struct mmc_command *cmd;	/* Current command */
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH V3 1/3] mmc: sdhci: Allow platform controlled voltage switching

[Evan Green]

On Mon, Oct 8, 2018 at 6:22 AM Veerabhadrarao Badiganti
<vbadigan@codeaurora.org> wrote:
>
> From: Vijay Viswanath <vviswana@codeaurora.org>
>
> Some controllers can have internal mechanism to inform the SW that it
> is ready for voltage switching. For such controllers, changing voltage
> before the HW is ready can result in various issues.
>
> During setup/cleanup of host, check whether regulator enable/disable
> was already done by platform driver.
>
> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci.c | 32 +++++++++++++++++++-------------
>  drivers/mmc/host/sdhci.h |  1 +
>  2 files changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 99bdae5..ea7ce1d 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -3616,6 +3616,7 @@ int sdhci_setup_host(struct sdhci_host *host)
>         unsigned int override_timeout_clk;
>         u32 max_clk;
>         int ret;
> +       bool enable_vqmmc = false;
>
>         WARN_ON(host == NULL);
>         if (host == NULL)
> @@ -3629,9 +3630,12 @@ int sdhci_setup_host(struct sdhci_host *host)
>          * the host can take the appropriate action if regulators are not
>          * available.
>          */
> -       ret = mmc_regulator_get_supply(mmc);
> -       if (ret)
> -               return ret;
> +       if (!mmc->supply.vqmmc) {
> +               ret = mmc_regulator_get_supply(mmc);
> +               if (ret)
> +                       return ret;
> +               enable_vqmmc  = true;
> +       }

Did you not like my suggestion in the previous patch of changing
mmc_regulator_get_supply to only get supplies that were not set
previously? What you have here is almost the same, except you've got
this weird coupling where if vqmmc is already present, you implicitly
skip looking at vmmc entirely (since mmc_regulator_get_supply does
more than just get vqmmc).

>
>         DBG("Version:   0x%08x | Present:  0x%08x\n",
>             sdhci_readw(host, SDHCI_HOST_VERSION),
> @@ -3880,7 +3884,15 @@ int sdhci_setup_host(struct sdhci_host *host)
>                 mmc->caps |= MMC_CAP_NEEDS_POLL;
>
>         if (!IS_ERR(mmc->supply.vqmmc)) {
> -               ret = regulator_enable(mmc->supply.vqmmc);
> +               if (enable_vqmmc) {
> +                       ret = regulator_enable(mmc->supply.vqmmc);
> +                       if (ret) {
> +                               pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
> +                                       mmc_hostname(mmc), ret);
> +                               mmc->supply.vqmmc = ERR_PTR(-EINVAL);
> +                       }
> +                       host->vqmmc_enabled = !ret;
> +               }
>
>                 /* If vqmmc provides no 1.8V signalling, then there's no UHS */
>                 if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 1700000,
> @@ -3893,12 +3905,6 @@ int sdhci_setup_host(struct sdhci_host *host)
>                 if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 2700000,
>                                                     3600000))
>                         host->flags &= ~SDHCI_SIGNALING_330;
> -
> -               if (ret) {
> -                       pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
> -                               mmc_hostname(mmc), ret);
> -                       mmc->supply.vqmmc = ERR_PTR(-EINVAL);
> -               }
>         }
>
>         if (host->quirks2 & SDHCI_QUIRK2_NO_1_8_V) {
> @@ -4136,7 +4142,7 @@ int sdhci_setup_host(struct sdhci_host *host)
>         return 0;
>
>  unreg:
> -       if (!IS_ERR(mmc->supply.vqmmc))
> +       if (host->vqmmc_enabled)
>                 regulator_disable(mmc->supply.vqmmc);
>  undma:
>         if (host->align_buffer)
> @@ -4154,7 +4160,7 @@ void sdhci_cleanup_host(struct sdhci_host *host)
>  {
>         struct mmc_host *mmc = host->mmc;
>
> -       if (!IS_ERR(mmc->supply.vqmmc))
> +       if (host->vqmmc_enabled)
>                 regulator_disable(mmc->supply.vqmmc);
>
>         if (host->align_buffer)
> @@ -4287,7 +4293,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
>
>         tasklet_kill(&host->finish_tasklet);
>
> -       if (!IS_ERR(mmc->supply.vqmmc))
> +       if (host->vqmmc_enabled)
>                 regulator_disable(mmc->supply.vqmmc);
>
>         if (host->align_buffer)
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index b001cf4..3c28152 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -524,6 +524,7 @@ struct sdhci_host {
>         bool pending_reset;     /* Cmd/data reset is pending */
>         bool irq_wake_enabled;  /* IRQ wakeup is enabled */
>         bool v4_mode;           /* Host Version 4 Enable */
> +       bool vqmmc_enabled;     /* Vqmmc is enabled */

I still don't love this, since it doesn't mean what it says. Everyone
else that has a vqmmc_enabled member uses it to actually mean "vqmmc
is enabled", but this doesn't mean that. For example, you don't clear
this when you disable the regulator in patch 3, so this would be set
even if the regulator is disabled, and you don't set it when sdhci
enables the regulator, so the regulator is on when this flag is not
set.

At the very least, I think a rename is in order. I'm struggling with
the rename, since
"vqmmc_pltfrm_enable_but_feel_free_to_call_set_voltage_anywhere" is
too long. I guess vqmmc_pltfrm_enabled is the best I've got for now.

>
>         struct mmc_request *mrqs_done[SDHCI_MAX_MRQS];  /* Requests done */
>         struct mmc_command *cmd;        /* Current command */
> --
> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
>

Re: [PATCH V3 1/3] mmc: sdhci: Allow platform controlled voltage switching

[Veerabhadrarao Badiganti]

On 10/17/2018 3:39 AM, Evan Green wrote:
> On Mon, Oct 8, 2018 at 6:22 AM Veerabhadrarao Badiganti
> <vbadigan@codeaurora.org> wrote:
>> From: Vijay Viswanath <vviswana@codeaurora.org>
>>
>> Some controllers can have internal mechanism to inform the SW that it
>> is ready for voltage switching. For such controllers, changing voltage
>> before the HW is ready can result in various issues.
>>
>> During setup/cleanup of host, check whether regulator enable/disable
>> was already done by platform driver.
>>
>> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
>> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
>> ---
>>   drivers/mmc/host/sdhci.c | 32 +++++++++++++++++++-------------
>>   drivers/mmc/host/sdhci.h |  1 +
>>   2 files changed, 20 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 99bdae5..ea7ce1d 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -3616,6 +3616,7 @@ int sdhci_setup_host(struct sdhci_host *host)
>>          unsigned int override_timeout_clk;
>>          u32 max_clk;
>>          int ret;
>> +       bool enable_vqmmc = false;
>>
>>          WARN_ON(host == NULL);
>>          if (host == NULL)
>> @@ -3629,9 +3630,12 @@ int sdhci_setup_host(struct sdhci_host *host)
>>           * the host can take the appropriate action if regulators are not
>>           * available.
>>           */
>> -       ret = mmc_regulator_get_supply(mmc);
>> -       if (ret)
>> -               return ret;
>> +       if (!mmc->supply.vqmmc) {
>> +               ret = mmc_regulator_get_supply(mmc);
>> +               if (ret)
>> +                       return ret;
>> +               enable_vqmmc  = true;
>> +       }
> Did you not like my suggestion in the previous patch of changing
> mmc_regulator_get_supply to only get supplies that were not set
> previously? What you have here is almost the same, except you've got
> this weird coupling where if vqmmc is already present, you implicitly
> skip looking at vmmc entirely (since mmc_regulator_get_supply does
> more than just get vqmmc).

sure. Will update mmc_regulator_get_supply() as you suggested.

>>          DBG("Version:   0x%08x | Present:  0x%08x\n",
>>              sdhci_readw(host, SDHCI_HOST_VERSION),
>> @@ -3880,7 +3884,15 @@ int sdhci_setup_host(struct sdhci_host *host)
>>                  mmc->caps |= MMC_CAP_NEEDS_POLL;
>>
>>          if (!IS_ERR(mmc->supply.vqmmc)) {
>> -               ret = regulator_enable(mmc->supply.vqmmc);
>> +               if (enable_vqmmc) {
>> +                       ret = regulator_enable(mmc->supply.vqmmc);
>> +                       if (ret) {
>> +                               pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
>> +                                       mmc_hostname(mmc), ret);
>> +                               mmc->supply.vqmmc = ERR_PTR(-EINVAL);
>> +                       }
>> +                       host->vqmmc_enabled = !ret;
>> +               }
>>
>>                  /* If vqmmc provides no 1.8V signalling, then there's no UHS */
>>                  if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 1700000,
>> @@ -3893,12 +3905,6 @@ int sdhci_setup_host(struct sdhci_host *host)
>>                  if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 2700000,
>>                                                      3600000))
>>                          host->flags &= ~SDHCI_SIGNALING_330;
>> -
>> -               if (ret) {
>> -                       pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
>> -                               mmc_hostname(mmc), ret);
>> -                       mmc->supply.vqmmc = ERR_PTR(-EINVAL);
>> -               }
>>          }
>>
>>          if (host->quirks2 & SDHCI_QUIRK2_NO_1_8_V) {
>> @@ -4136,7 +4142,7 @@ int sdhci_setup_host(struct sdhci_host *host)
>>          return 0;
>>
>>   unreg:
>> -       if (!IS_ERR(mmc->supply.vqmmc))
>> +       if (host->vqmmc_enabled)
>>                  regulator_disable(mmc->supply.vqmmc);
>>   undma:
>>          if (host->align_buffer)
>> @@ -4154,7 +4160,7 @@ void sdhci_cleanup_host(struct sdhci_host *host)
>>   {
>>          struct mmc_host *mmc = host->mmc;
>>
>> -       if (!IS_ERR(mmc->supply.vqmmc))
>> +       if (host->vqmmc_enabled)
>>                  regulator_disable(mmc->supply.vqmmc);
>>
>>          if (host->align_buffer)
>> @@ -4287,7 +4293,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
>>
>>          tasklet_kill(&host->finish_tasklet);
>>
>> -       if (!IS_ERR(mmc->supply.vqmmc))
>> +       if (host->vqmmc_enabled)
>>                  regulator_disable(mmc->supply.vqmmc);
>>
>>          if (host->align_buffer)
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index b001cf4..3c28152 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -524,6 +524,7 @@ struct sdhci_host {
>>          bool pending_reset;     /* Cmd/data reset is pending */
>>          bool irq_wake_enabled;  /* IRQ wakeup is enabled */
>>          bool v4_mode;           /* Host Version 4 Enable */
>> +       bool vqmmc_enabled;     /* Vqmmc is enabled */
> I still don't love this, since it doesn't mean what it says. Everyone
> else that has a vqmmc_enabled member uses it to actually mean "vqmmc
> is enabled", but this doesn't mean that. For example, you don't clear
> this when you disable the regulator in patch 3, so this would be set
> even if the regulator is disabled, and you don't set it when sdhci
> enables the regulator, so the regulator is on when this flag is not
> set.
>
> At the very least, I think a rename is in order. I'm struggling with
> the rename, since
> "vqmmc_pltfrm_enable_but_feel_free_to_call_set_voltage_anywhere" is
> too long. I guess vqmmc_pltfrm_enabled is the best I've got for now.

Sure. Will rename it.

>>          struct mmc_request *mrqs_done[SDHCI_MAX_MRQS];  /* Requests done */
>>          struct mmc_command *cmd;        /* Current command */
>> --
>> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
>>

Re: [PATCH V3 1/3] mmc: sdhci: Allow platform controlled voltage switching

[Veerabhadrarao Badiganti]

On 11/12/2018 10:49 PM, Veerabhadrarao Badiganti wrote:
>
> On 10/17/2018 3:39 AM, Evan Green wrote:
>> On Mon, Oct 8, 2018 at 6:22 AM Veerabhadrarao Badiganti
>> <vbadigan@codeaurora.org> wrote:
>>> From: Vijay Viswanath <vviswana@codeaurora.org>
>>>
>>> Some controllers can have internal mechanism to inform the SW that it
>>> is ready for voltage switching. For such controllers, changing voltage
>>> before the HW is ready can result in various issues.
>>>
>>> During setup/cleanup of host, check whether regulator enable/disable
>>> was already done by platform driver.
>>>
>>> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
>>> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
>>> ---
>>>   drivers/mmc/host/sdhci.c | 32 +++++++++++++++++++-------------
>>>   drivers/mmc/host/sdhci.h |  1 +
>>>   2 files changed, 20 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 99bdae5..ea7ce1d 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -3616,6 +3616,7 @@ int sdhci_setup_host(struct sdhci_host *host)
>>>          unsigned int override_timeout_clk;
>>>          u32 max_clk;
>>>          int ret;
>>> +       bool enable_vqmmc = false;
>>>
>>>          WARN_ON(host == NULL);
>>>          if (host == NULL)
>>> @@ -3629,9 +3630,12 @@ int sdhci_setup_host(struct sdhci_host *host)
>>>           * the host can take the appropriate action if regulators 
>>> are not
>>>           * available.
>>>           */
>>> -       ret = mmc_regulator_get_supply(mmc);
>>> -       if (ret)
>>> -               return ret;
>>> +       if (!mmc->supply.vqmmc) {
>>> +               ret = mmc_regulator_get_supply(mmc);
>>> +               if (ret)
>>> +                       return ret;
>>> +               enable_vqmmc  = true;
>>> +       }
>> Did you not like my suggestion in the previous patch of changing
>> mmc_regulator_get_supply to only get supplies that were not set
>> previously? What you have here is almost the same, except you've got
>> this weird coupling where if vqmmc is already present, you implicitly
>> skip looking at vmmc entirely (since mmc_regulator_get_supply does
>> more than just get vqmmc).
>
> sure. Will update mmc_regulator_get_supply() as you suggested.
>
>>>          DBG("Version:   0x%08x | Present:  0x%08x\n",
>>>              sdhci_readw(host, SDHCI_HOST_VERSION),
>>> @@ -3880,7 +3884,15 @@ int sdhci_setup_host(struct sdhci_host *host)
>>>                  mmc->caps |= MMC_CAP_NEEDS_POLL;
>>>
>>>          if (!IS_ERR(mmc->supply.vqmmc)) {
>>> -               ret = regulator_enable(mmc->supply.vqmmc);
>>> +               if (enable_vqmmc) {
>>> +                       ret = regulator_enable(mmc->supply.vqmmc);
>>> +                       if (ret) {
>>> +                               pr_warn("%s: Failed to enable vqmmc 
>>> regulator: %d\n",
>>> +                                       mmc_hostname(mmc), ret);
>>> +                               mmc->supply.vqmmc = ERR_PTR(-EINVAL);
>>> +                       }
>>> +                       host->vqmmc_enabled = !ret;
>>> +               }
>>>
>>>                  /* If vqmmc provides no 1.8V signalling, then 
>>> there's no UHS */
>>>                  if 
>>> (!regulator_is_supported_voltage(mmc->supply.vqmmc, 1700000,
>>> @@ -3893,12 +3905,6 @@ int sdhci_setup_host(struct sdhci_host *host)
>>>                  if 
>>> (!regulator_is_supported_voltage(mmc->supply.vqmmc, 2700000,
>>>                                                      3600000))
>>>                          host->flags &= ~SDHCI_SIGNALING_330;
>>> -
>>> -               if (ret) {
>>> -                       pr_warn("%s: Failed to enable vqmmc 
>>> regulator: %d\n",
>>> -                               mmc_hostname(mmc), ret);
>>> -                       mmc->supply.vqmmc = ERR_PTR(-EINVAL);
>>> -               }
>>>          }
>>>
>>>          if (host->quirks2 & SDHCI_QUIRK2_NO_1_8_V) {
>>> @@ -4136,7 +4142,7 @@ int sdhci_setup_host(struct sdhci_host *host)
>>>          return 0;
>>>
>>>   unreg:
>>> -       if (!IS_ERR(mmc->supply.vqmmc))
>>> +       if (host->vqmmc_enabled)
>>>                  regulator_disable(mmc->supply.vqmmc);
>>>   undma:
>>>          if (host->align_buffer)
>>> @@ -4154,7 +4160,7 @@ void sdhci_cleanup_host(struct sdhci_host *host)
>>>   {
>>>          struct mmc_host *mmc = host->mmc;
>>>
>>> -       if (!IS_ERR(mmc->supply.vqmmc))
>>> +       if (host->vqmmc_enabled)
>>>                  regulator_disable(mmc->supply.vqmmc);
>>>
>>>          if (host->align_buffer)
>>> @@ -4287,7 +4293,7 @@ void sdhci_remove_host(struct sdhci_host 
>>> *host, int dead)
>>>
>>>          tasklet_kill(&host->finish_tasklet);
>>>
>>> -       if (!IS_ERR(mmc->supply.vqmmc))
>>> +       if (host->vqmmc_enabled)
>>>                  regulator_disable(mmc->supply.vqmmc);
>>>
>>>          if (host->align_buffer)
>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>> index b001cf4..3c28152 100644
>>> --- a/drivers/mmc/host/sdhci.h
>>> +++ b/drivers/mmc/host/sdhci.h
>>> @@ -524,6 +524,7 @@ struct sdhci_host {
>>>          bool pending_reset;     /* Cmd/data reset is pending */
>>>          bool irq_wake_enabled;  /* IRQ wakeup is enabled */
>>>          bool v4_mode;           /* Host Version 4 Enable */
>>> +       bool vqmmc_enabled;     /* Vqmmc is enabled */
>> I still don't love this, since it doesn't mean what it says. Everyone
>> else that has a vqmmc_enabled member uses it to actually mean "vqmmc
>> is enabled", but this doesn't mean that. For example, you don't clear
>> this when you disable the regulator in patch 3, so this would be set
>> even if the regulator is disabled, and you don't set it when sdhci
>> enables the regulator, so the regulator is on when this flag is not
>> set.
>>
Hi Evan

This flag is meant to say "disable vqmmc *only* if it is enabled by host 
driver (sdhci_host)".
If host driver doesn't enable vqmmc (enabled by platfrm driver) or if it 
fails to enable it, then don't call disable vqmmc.

Agree with you, the present name is not conveying its purpose.
It must be something like "vqmmc_enabled_by_host".

Please let me know if you have any suggestions on this name.


>> At the very least, I think a rename is in order. I'm struggling with
>> the rename, since
>> "vqmmc_pltfrm_enable_but_feel_free_to_call_set_voltage_anywhere" is
>> too long. I guess vqmmc_pltfrm_enabled is the best I've got for now.
>
> Sure. Will rename it.
>
>>>          struct mmc_request *mrqs_done[SDHCI_MAX_MRQS];  /* Requests 
>>> done */
>>>          struct mmc_command *cmd;        /* Current command */
>>> -- 
>>> Qualcomm India Private Limited, on behalf of Qualcomm Innovation 
>>> Center, Inc.
>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
>>> Linux Foundation Collaborative Project.
>>>
>

Re: [PATCH V3 1/3] mmc: sdhci: Allow platform controlled voltage switching

[Evan Green]

On Wed, Nov 14, 2018 at 6:36 AM Veerabhadrarao Badiganti
<vbadigan@codeaurora.org> wrote:
>
> >>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> >>> index b001cf4..3c28152 100644
> >>> --- a/drivers/mmc/host/sdhci.h
> >>> +++ b/drivers/mmc/host/sdhci.h
> >>> @@ -524,6 +524,7 @@ struct sdhci_host {
> >>>          bool pending_reset;     /* Cmd/data reset is pending */
> >>>          bool irq_wake_enabled;  /* IRQ wakeup is enabled */
> >>>          bool v4_mode;           /* Host Version 4 Enable */
> >>> +       bool vqmmc_enabled;     /* Vqmmc is enabled */
> >> I still don't love this, since it doesn't mean what it says. Everyone
> >> else that has a vqmmc_enabled member uses it to actually mean "vqmmc
> >> is enabled", but this doesn't mean that. For example, you don't clear
> >> this when you disable the regulator in patch 3, so this would be set
> >> even if the regulator is disabled, and you don't set it when sdhci
> >> enables the regulator, so the regulator is on when this flag is not
> >> set.
> >>
> Hi Evan
>
> This flag is meant to say "disable vqmmc *only* if it is enabled by host
> driver (sdhci_host)".
> If host driver doesn't enable vqmmc (enabled by platfrm driver) or if it
> fails to enable it, then don't call disable vqmmc.
>
> Agree with you, the present name is not conveying its purpose.
> It must be something like "vqmmc_enabled_by_host".
>
> Please let me know if you have any suggestions on this name.

Yeah. Maybe vqmmc_pltfrm_controlled? Or vqmmc_enabled_by_platfrm as
you suggested?
-Evan

Re: [PATCH V3 1/3] mmc: sdhci: Allow platform controlled voltage switching

[Adrian Hunter]

On 16/11/18 1:17 AM, Evan Green wrote:
> On Wed, Nov 14, 2018 at 6:36 AM Veerabhadrarao Badiganti
> <vbadigan@codeaurora.org> wrote:
>>
>>>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>>>> index b001cf4..3c28152 100644
>>>>> --- a/drivers/mmc/host/sdhci.h
>>>>> +++ b/drivers/mmc/host/sdhci.h
>>>>> @@ -524,6 +524,7 @@ struct sdhci_host {
>>>>>          bool pending_reset;     /* Cmd/data reset is pending */
>>>>>          bool irq_wake_enabled;  /* IRQ wakeup is enabled */
>>>>>          bool v4_mode;           /* Host Version 4 Enable */
>>>>> +       bool vqmmc_enabled;     /* Vqmmc is enabled */
>>>> I still don't love this, since it doesn't mean what it says. Everyone
>>>> else that has a vqmmc_enabled member uses it to actually mean "vqmmc
>>>> is enabled", but this doesn't mean that. For example, you don't clear
>>>> this when you disable the regulator in patch 3, so this would be set
>>>> even if the regulator is disabled, and you don't set it when sdhci
>>>> enables the regulator, so the regulator is on when this flag is not
>>>> set.
>>>>
>> Hi Evan
>>
>> This flag is meant to say "disable vqmmc *only* if it is enabled by host
>> driver (sdhci_host)".
>> If host driver doesn't enable vqmmc (enabled by platfrm driver) or if it
>> fails to enable it, then don't call disable vqmmc.
>>
>> Agree with you, the present name is not conveying its purpose.
>> It must be something like "vqmmc_enabled_by_host".
>>
>> Please let me know if you have any suggestions on this name.
> 
> Yeah. Maybe vqmmc_pltfrm_controlled? Or vqmmc_enabled_by_platfrm as
> you suggested?

"pltfrm" doesn't mean anything here.  Just change the comment "vqmmc enabled
in sdhci.c"

[PATCH V3 2/3] dt-bindings: mmc: sdhci-msm: Add entries for passing load values

[Veerabhadrarao Badiganti]

From: Vijay Viswanath <vviswana@codeaurora.org>

The load a particular sdhc controller should request from a regulator
is device specific and hence each device should individually vote for
the required load.

Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
---
 Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
index 502b3b8..cb22178 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
@@ -25,6 +25,10 @@ Required properties:
 	"xo"	- TCXO clock (optional)
 	"cal"	- reference clock for RCLK delay calibration (optional)
 	"sleep"	- sleep clock for RCLK delay calibration (optional)
+- qcom,<supply>-current-level-microamp - specifies load levels for supply during BUS_ON and
+					BUS_OFF states in power irq. Should be specified in
+					pairs (lpm, hpm), for BUS_OFF and BUS_ON respectively.
+					Units uA.
 
 Example:
 
@@ -36,7 +40,9 @@ Example:
 		non-removable;
 
 		vmmc-supply = <&pm8941_l20>;
+		qcom,vmmc-current-level-microamp = <200 570000>;
 		vqmmc-supply = <&pm8941_s3>;
+		qcom,vqmmc-current-level-microamp = <200 325000>;
 
 		pinctrl-names = "default";
 		pinctrl-0 = <&sdc1_clk &sdc1_cmd &sdc1_data>;
@@ -53,7 +59,9 @@ Example:
 		cd-gpios = <&msmgpio 62 0x1>;
 
 		vmmc-supply = <&pm8941_l21>;
+		qcom,vmmc-current-level-microamp = <200 800000>;
 		vqmmc-supply = <&pm8941_l13>;
+		qcom,vqmmc-current-level-microamp = <200 22000>;
 
 		pinctrl-names = "default";
 		pinctrl-0 = <&sdc2_clk &sdc2_cmd &sdc2_data>;
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH V3 2/3] dt-bindings: mmc: sdhci-msm: Add entries for passing load values

[Rob Herring]

On Mon, Oct 08, 2018 at 06:48:58PM +0530, Veerabhadrarao Badiganti wrote:
> From: Vijay Viswanath <vviswana@codeaurora.org>
> 
> The load a particular sdhc controller should request from a regulator
> is device specific and hence each device should individually vote for
> the required load.
> 
> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> index 502b3b8..cb22178 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> @@ -25,6 +25,10 @@ Required properties:
>  	"xo"	- TCXO clock (optional)
>  	"cal"	- reference clock for RCLK delay calibration (optional)
>  	"sleep"	- sleep clock for RCLK delay calibration (optional)
> +- qcom,<supply>-current-level-microamp - specifies load levels for supply during BUS_ON and
> +					BUS_OFF states in power irq. Should be specified in
> +					pairs (lpm, hpm), for BUS_OFF and BUS_ON respectively.
> +					Units uA.

Seems like something that should be common?

Re: [PATCH V3 2/3] dt-bindings: mmc: sdhci-msm: Add entries for passing load values

[Veerabhadrarao Badiganti]

On 10/12/2018 8:25 PM, Rob Herring wrote:
> On Mon, Oct 08, 2018 at 06:48:58PM +0530, Veerabhadrarao Badiganti wrote:
>> From: Vijay Viswanath <vviswana@codeaurora.org>
>>
>> The load a particular sdhc controller should request from a regulator
>> is device specific and hence each device should individually vote for
>> the required load.
>>
>> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
>> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
>> ---
>>   Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> index 502b3b8..cb22178 100644
>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> @@ -25,6 +25,10 @@ Required properties:
>>   	"xo"	- TCXO clock (optional)
>>   	"cal"	- reference clock for RCLK delay calibration (optional)
>>   	"sleep"	- sleep clock for RCLK delay calibration (optional)
>> +- qcom,<supply>-current-level-microamp - specifies load levels for supply during BUS_ON and
>> +					BUS_OFF states in power irq. Should be specified in
>> +					pairs (lpm, hpm), for BUS_OFF and BUS_ON respectively.
>> +					Units uA.
> Seems like something that should be common?

Hi Rob,
Can you please little elaborate your comment?
Do mean this should be common for all vendors, not specific to qcom-mmc?


Thanks,
Veera

[PATCH V3 3/3] mmc: sdhci-msm: Use internal voltage control

[Veerabhadrarao Badiganti]

From: Vijay Viswanath <vviswana@codeaurora.org>

Some sdhci-msm controllers require that 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 quirk for internal voltage switching and then control the
voltage switching using power irq.

Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 212 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 204 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 3cc8bfe..6918e70 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -43,7 +43,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)
@@ -258,6 +260,10 @@ struct sdhci_msm_host {
 	bool mci_removed;
 	const struct sdhci_msm_variant_ops *var_ops;
 	const struct sdhci_msm_offset *offset;
+	bool vmmc_load;
+	u32 vmmc_level[2];
+	bool vqmmc_load;
+	u32 vqmmc_level[2];
 };
 
 static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
@@ -1211,6 +1217,74 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
 		sdhci_msm_hs400(host, &mmc->ios);
 }
 
+static int sdhci_msm_set_vmmc(struct sdhci_msm_host *msm_host,
+			      struct mmc_host *mmc, int level)
+{
+	int load = msm_host->vmmc_level[level];
+	int ret;
+
+	if (IS_ERR(mmc->supply.vmmc))
+		return 0;
+
+	if (msm_host->vmmc_load) {
+		ret = regulator_set_load(mmc->supply.vmmc, load);
+		if (ret)
+			goto out;
+	}
+
+	ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, mmc->ios.vdd);
+out:
+	if (ret)
+		pr_err("%s: vmmc set load/ocr 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, int level)
+{
+	int load = msm_host->vqmmc_level[level];
+	int ret;
+	struct mmc_ios ios;
+
+	if (IS_ERR(mmc->supply.vqmmc))
+		return 0;
+
+	if (msm_host->vqmmc_load) {
+		ret = regulator_set_load(mmc->supply.vqmmc, load);
+		if (ret)
+			goto out;
+	}
+
+	/*
+	 * The IO voltage regulator may not always support a voltage close to
+	 * vdd. Set IO voltage based on capability of the regulator.
+	 */
+	if (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) {
+			pr_debug("%s: %s: setting signal voltage: %d\n",
+					mmc_hostname(mmc), __func__,
+					ios.signal_voltage);
+			ret = mmc_regulator_set_vqmmc(mmc, &ios);
+			if (ret)
+				goto out;
+		}
+		ret = regulator_enable(mmc->supply.vqmmc);
+	} else {
+		ret = regulator_disable(mmc->supply.vqmmc);
+	}
+out:
+	if (ret)
+		pr_err("%s: vqmmc failed: %d\n", mmc_hostname(mmc), ret);
+
+	return ret;
+}
+
 static inline void sdhci_msm_init_pwr_irq_wait(struct sdhci_msm_host *msm_host)
 {
 	init_waitqueue_head(&msm_host->pwr_irq_wait);
@@ -1314,8 +1388,9 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	struct mmc_host *mmc = host->mmc;
 	u32 irq_status, irq_ack = 0;
-	int retry = 10;
+	int retry = 10, ret = 0;
 	u32 pwr_state = 0, io_level = 0;
 	u32 config;
 	const struct sdhci_msm_offset *msm_offset = msm_host->offset;
@@ -1351,14 +1426,35 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 
 	/* Handle BUS ON/OFF*/
 	if (irq_status & CORE_PWRCTL_BUS_ON) {
-		pwr_state = REQ_BUS_ON;
-		io_level = REQ_IO_HIGH;
-		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
+		ret = sdhci_msm_set_vmmc(msm_host, mmc, 1);
+		if (!ret)
+			ret = sdhci_msm_set_vqmmc(msm_host, mmc, 1);
+
+		if (!ret) {
+			pwr_state = REQ_BUS_ON;
+			io_level = REQ_IO_HIGH;
+			irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
+		} else {
+			pr_err("%s: BUS_ON req failed(%d). irq_status: 0x%08x\n",
+					mmc_hostname(mmc), ret, irq_status);
+			irq_ack |= CORE_PWRCTL_BUS_FAIL;
+			sdhci_msm_set_vmmc(msm_host, mmc, 0);
+		}
 	}
 	if (irq_status & CORE_PWRCTL_BUS_OFF) {
-		pwr_state = REQ_BUS_OFF;
-		io_level = REQ_IO_LOW;
-		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
+		ret = sdhci_msm_set_vmmc(msm_host, mmc, 0);
+		if (!ret)
+			ret = sdhci_msm_set_vqmmc(msm_host, mmc, 0);
+
+		if (!ret) {
+			pwr_state = REQ_BUS_OFF;
+			io_level = REQ_IO_LOW;
+			irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
+		} else {
+			pr_err("%s: BUS_ON req failed(%d). irq_status: 0x%08x\n",
+					mmc_hostname(mmc), ret, irq_status);
+			irq_ack |= CORE_PWRCTL_BUS_FAIL;
+		}
 	}
 	/* Handle IO LOW/HIGH */
 	if (irq_status & CORE_PWRCTL_IO_LOW) {
@@ -1370,6 +1466,15 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 		irq_ack |= CORE_PWRCTL_IO_SUCCESS;
 	}
 
+	if (io_level && !IS_ERR(mmc->supply.vqmmc) && !pwr_state) {
+		ret = mmc_regulator_set_vqmmc(mmc, &mmc->ios);
+		if (ret)
+			pr_err("%s: IO_level setting failed(%d). signal_voltage: %d, vdd: %d irq_status: 0x%08x\n",
+					mmc_hostname(mmc), ret,
+					mmc->ios.signal_voltage, mmc->ios.vdd,
+					irq_status);
+	}
+
 	/*
 	 * The driver has to acknowledge the interrupt, switch voltages and
 	 * report back if it succeded or not to this register. The voltage
@@ -1605,6 +1710,91 @@ static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host)
 	pr_debug("%s: supported caps: 0x%08x\n", mmc_hostname(mmc), caps);
 }
 
+static int sdhci_msm_register_vreg(struct sdhci_msm_host *msm_host)
+{
+	int ret = 0;
+
+	ret = mmc_regulator_get_supply(msm_host->mmc);
+	if (ret)
+		return ret;
+	if (device_property_read_u32_array(&msm_host->pdev->dev,
+			"qcom,vmmc-current-level-microamp",
+			msm_host->vmmc_level, 2) >= 0)
+		msm_host->vmmc_load = true;
+	if (device_property_read_u32_array(&msm_host->pdev->dev,
+			"qcom,vqmmc-current-level-microamp",
+			msm_host->vqmmc_level, 2) >= 0)
+		msm_host->vqmmc_load = true;
+
+	sdhci_msm_set_regulator_caps(msm_host);
+
+	return 0;
+
+}
+
+static int sdhci_msm_start_signal_voltage_switch(struct mmc_host *mmc,
+				      struct mmc_ios *ios)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	u16 ctrl;
+
+	/*
+	 * Signal Voltage Switching is only applicable for Host Controllers
+	 * v3.00 and above.
+	 */
+	if (host->version < SDHCI_SPEC_300)
+		return 0;
+
+	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+
+	switch (ios->signal_voltage) {
+	case MMC_SIGNAL_VOLTAGE_330:
+		if (!(host->flags & SDHCI_SIGNALING_330))
+			return -EINVAL;
+		/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
+		ctrl &= ~SDHCI_CTRL_VDD_180;
+		sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+
+		/* 3.3V regulator output should be stable within 5 ms */
+		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+		if (!(ctrl & SDHCI_CTRL_VDD_180))
+			return 0;
+
+		pr_warn("%s: 3.3V regulator output did not became stable\n",
+			mmc_hostname(mmc));
+
+		return -EAGAIN;
+	case MMC_SIGNAL_VOLTAGE_180:
+		if (!(host->flags & SDHCI_SIGNALING_180))
+			return -EINVAL;
+
+		/*
+		 * Enable 1.8V Signal Enable in the Host Control2
+		 * register
+		 */
+		ctrl |= SDHCI_CTRL_VDD_180;
+		sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+
+		/* 1.8V regulator output should be stable within 5 ms */
+		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+		if (ctrl & SDHCI_CTRL_VDD_180)
+			return 0;
+
+		pr_warn("%s: 1.8V regulator output did not became stable\n",
+			mmc_hostname(mmc));
+
+		return -EAGAIN;
+	case MMC_SIGNAL_VOLTAGE_120:
+		if (!(host->flags & SDHCI_SIGNALING_120))
+			return -EINVAL;
+		return 0;
+	default:
+		/* No signal voltage switch required */
+		return 0;
+	}
+
+}
+
 static const struct sdhci_msm_variant_ops mci_var_ops = {
 	.msm_readl_relaxed = sdhci_msm_mci_variant_readl_relaxed,
 	.msm_writel_relaxed = sdhci_msm_mci_variant_writel_relaxed,
@@ -1644,6 +1834,7 @@ static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host)
 	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
 	.write_w = sdhci_msm_writew,
 	.write_b = sdhci_msm_writeb,
+	.set_power = sdhci_set_power_noreg,
 };
 
 static const struct sdhci_pltfm_data sdhci_msm_pdata = {
@@ -1818,6 +2009,10 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 				msm_offset->core_vendor_spec_capabilities0);
 	}
 
+	ret = sdhci_msm_register_vreg(msm_host);
+	if (ret)
+		goto clk_disable;
+
 	/*
 	 * Power on reset state may trigger power irq if previous status of
 	 * PWRCTL was either BUS_ON or IO_HIGH_V. So before enabling pwr irq
@@ -1862,11 +2057,12 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 					 MSM_MMC_AUTOSUSPEND_DELAY_MS);
 	pm_runtime_use_autosuspend(&pdev->dev);
 
+	host->mmc_host_ops.start_signal_voltage_switch =
+		sdhci_msm_start_signal_voltage_switch;
 	host->mmc_host_ops.execute_tuning = sdhci_msm_execute_tuning;
 	ret = sdhci_add_host(host);
 	if (ret)
 		goto pm_runtime_disable;
-	sdhci_msm_set_regulator_caps(msm_host);
 
 	pm_runtime_mark_last_busy(&pdev->dev);
 	pm_runtime_put_autosuspend(&pdev->dev);
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH V1 0/3] Internal voltage control for qcom SDHC

[Veerabhadrarao Badiganti]

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.

So added support to register voltage regulators from the msm driver
and use them.

This patchset was posted long back but not actively pursued
https://lore.kernel.org/linux-arm-msm/1539004739-32060-1-git-send-email-vbadigan@codeaurora.org/
So posting it as fresh patchset.  

Veerabhadrarao Badiganti (1):
  dt-bindings: mmc: Supply max load for mmc supplies

Vijay Viswanath (2):
  mmc: sdhci-msm: Use internal voltage control
  mmc: sdhci: Allow platform controlled voltage switching

 .../devicetree/bindings/mmc/mmc-controller.yaml    |  16 ++
 drivers/mmc/host/sdhci-msm.c                       | 215 ++++++++++++++++++++-
 drivers/mmc/host/sdhci.c                           |  32 +--
 drivers/mmc/host/sdhci.h                           |   1 +
 4 files changed, 243 insertions(+), 21 deletions(-)

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCH V3 0/3] Internal voltage control for qcom SDHC

[Veerabhadrarao Badiganti]

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.

So added support to register voltage regulators from the msm driver
and use them.

This patchset was posted long back but not actively pursued
https://lore.kernel.org/linux-arm-msm/1539004739-32060-1-git-send-email-vbadigan@codeaurora.org/
So posting it as fresh patchset.  

Changes since V2:
	- Removed redundant log from sdhci_msm_set_vmmc.
	- Added the condition for skiping disabling of vqmmc for eMMC.
	- Updated logic such that, setting load for vqmmc only if
	  it is kept ON. 
	- Retained ack by Adrian for second patch.
	- Updated dt properties names as per Robs comments.

Changes since V1:
	- Removed setting load for Vmmc regulator while turning it on/off.
	  Instead setting the active load once during probe.
	- Simplified handlng of supplies for BUS_ON/OFF cases in shci_msm_handle_pwr_irq().
	- Moved common code out of switch case in sdhci_msm_start_signal_voltage_switch().
	- Updated variable name to sdhci_core_to_disable_vqmmc.
	- Updated pr_err logs to dev_err logs.

Veerabhadrarao Badiganti (2):
  dt-bindings: mmc: Supply max load for mmc supplies
  mmc: sdhci-msm: Use internal voltage control

Vijay Viswanath (1):
  mmc: sdhci: Allow platform controlled voltage switching

 .../devicetree/bindings/mmc/mmc-controller.yaml    |   6 +
 drivers/mmc/host/sdhci-msm.c                       | 235 ++++++++++++++++++++-
 drivers/mmc/host/sdhci.c                           |  32 +--
 drivers/mmc/host/sdhci.h                           |   1 +
 4 files changed, 252 insertions(+), 22 deletions(-)

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCH V3 3/3] mmc: sdhci-msm: Use internal voltage control

[Veerabhadrarao Badiganti]

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.

Set the regulator load as well so that regulator can be configured
in LPM mode when in is not being used.

Co-developed-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Co-developed-by: Vijay Viswanath <vviswana@codeaurora.org>
Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
Co-developed-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 235 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 226 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 95cd9735e9a3..20ef90fc7dd7 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -36,7 +36,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)
@@ -277,6 +279,8 @@ struct sdhci_msm_host {
 	bool uses_tassadar_dll;
 	u32 dll_config;
 	u32 ddr_config;
+	u32 vqmmc_load;
+	bool vqmmc_enabled;
 };
 
 static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
@@ -1339,6 +1343,91 @@ 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;
+
+	if (!msm_host->vqmmc_load)
+		return 0;
+
+	load = hpm ? msm_host->vqmmc_load : 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)		||
+	    (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.
+	 */
+	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);
+
+	return ret;
+}
+
 static inline void sdhci_msm_init_pwr_irq_wait(struct sdhci_msm_host *msm_host)
 {
 	init_waitqueue_head(&msm_host->pwr_irq_wait);
@@ -1442,8 +1531,9 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	struct mmc_host *mmc = host->mmc;
 	u32 irq_status, irq_ack = 0;
-	int retry = 10;
+	int retry = 10, ret;
 	u32 pwr_state = 0, io_level = 0;
 	u32 config;
 	const struct sdhci_msm_offset *msm_offset = msm_host->offset;
@@ -1481,21 +1571,42 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 	if (irq_status & CORE_PWRCTL_BUS_ON) {
 		pwr_state = REQ_BUS_ON;
 		io_level = REQ_IO_HIGH;
-		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
 	}
 	if (irq_status & CORE_PWRCTL_BUS_OFF) {
 		pwr_state = REQ_BUS_OFF;
 		io_level = REQ_IO_LOW;
-		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
 	}
+
+	if (pwr_state) {
+		ret = sdhci_msm_set_vmmc(mmc);
+		if (!ret)
+			ret = sdhci_msm_set_vqmmc(msm_host, mmc,
+					pwr_state & REQ_BUS_ON);
+		if (!ret)
+			irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
+		else
+			irq_ack |= CORE_PWRCTL_BUS_FAIL;
+	}
+
 	/* Handle IO LOW/HIGH */
-	if (irq_status & CORE_PWRCTL_IO_LOW) {
+	if (irq_status & CORE_PWRCTL_IO_LOW)
 		io_level = REQ_IO_LOW;
-		irq_ack |= CORE_PWRCTL_IO_SUCCESS;
-	}
-	if (irq_status & CORE_PWRCTL_IO_HIGH) {
+
+	if (irq_status & CORE_PWRCTL_IO_HIGH)
 		io_level = REQ_IO_HIGH;
+
+	if (io_level)
 		irq_ack |= CORE_PWRCTL_IO_SUCCESS;
+
+	if (io_level && !IS_ERR(mmc->supply.vqmmc) && !pwr_state) {
+		ret = mmc_regulator_set_vqmmc(mmc, &mmc->ios);
+		if (ret < 0) {
+			dev_err(mmc_dev(mmc), "%s: IO_level setting failed(%d). signal_voltage: %d, vdd: %d irq_status: 0x%08x\n",
+					mmc_hostname(mmc), ret,
+					mmc->ios.signal_voltage, mmc->ios.vdd,
+					irq_status);
+			irq_ack |= CORE_PWRCTL_IO_FAIL;
+		}
 	}
 
 	/*
@@ -1544,7 +1655,7 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 	if (io_level)
 		msm_host->curr_io_level = io_level;
 
-	pr_debug("%s: %s: Handled IRQ(%d), irq_status=0x%x, ack=0x%x\n",
+	dev_dbg(mmc_dev(mmc), "%s: %s: Handled IRQ(%d), irq_status=0x%x, ack=0x%x\n",
 		mmc_hostname(msm_host->mmc), __func__, irq, irq_status,
 		irq_ack);
 }
@@ -1874,6 +1985,106 @@ static void sdhci_msm_reset(struct sdhci_host *host, u8 mask)
 	sdhci_reset(host, mask);
 }
 
+static int sdhci_msm_register_vreg(struct sdhci_msm_host *msm_host)
+{
+	int ret;
+	u32 vmmc_load;
+	struct mmc_host *mmc = msm_host->mmc;
+
+	ret = mmc_regulator_get_supply(msm_host->mmc);
+	if (ret)
+		return ret;
+	device_property_read_u32(&msm_host->pdev->dev,
+			"vmmc-supply-max-microamp",
+			&vmmc_load);
+	device_property_read_u32(&msm_host->pdev->dev,
+			"vqmmc-supply-max-microamp",
+			&msm_host->vqmmc_load);
+
+	/* Set active load */
+	if (!IS_ERR(mmc->supply.vmmc) && vmmc_load) {
+		ret = regulator_set_load(mmc->supply.vmmc, vmmc_load);
+		if (ret) {
+			dev_err(mmc_dev(mmc), "%s: vmmc set active load failed: %d\n",
+				mmc_hostname(mmc), ret);
+			return ret;
+		}
+	}
+	if (!IS_ERR(mmc->supply.vqmmc) && msm_host->vqmmc_load) {
+		ret = regulator_set_load(mmc->supply.vmmc,
+					msm_host->vqmmc_load);
+		if (ret) {
+			dev_err(mmc_dev(mmc), "%s: vqmmc set active load failed: %d\n",
+				mmc_hostname(mmc), ret);
+			return ret;
+		}
+	}
+
+	sdhci_msm_set_regulator_caps(msm_host);
+	mmc->ios.power_mode = MMC_POWER_UNDEFINED;
+
+	return 0;
+}
+
+static int sdhci_msm_start_signal_voltage_switch(struct mmc_host *mmc,
+				      struct mmc_ios *ios)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	u16 ctrl, status;
+
+	/*
+	 * Signal Voltage Switching is only applicable for Host Controllers
+	 * v3.00 and above.
+	 */
+	if (host->version < SDHCI_SPEC_300)
+		return 0;
+
+	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+
+	switch (ios->signal_voltage) {
+	case MMC_SIGNAL_VOLTAGE_330:
+		if (!(host->flags & SDHCI_SIGNALING_330))
+			return -EINVAL;
+
+		/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
+		ctrl &= ~SDHCI_CTRL_VDD_180;
+		break;
+	case MMC_SIGNAL_VOLTAGE_180:
+		if (!(host->flags & SDHCI_SIGNALING_180))
+			return -EINVAL;
+
+		/*
+		 * Enable 1.8V Signal Enable in the Host Control2
+		 * register
+		 */
+		ctrl |= SDHCI_CTRL_VDD_180;
+		break;
+	case MMC_SIGNAL_VOLTAGE_120:
+		if (!(host->flags & SDHCI_SIGNALING_120))
+			return -EINVAL;
+		return 0;
+	default:
+		/* No signal voltage switch required */
+		return 0;
+	}
+
+	sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+
+	/* Wait for 5ms */
+	usleep_range(5000, 5500);
+
+	/* regulator output should be stable within 5 ms */
+	status = ctrl & SDHCI_CTRL_VDD_180;
+	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+	if ((ctrl & SDHCI_CTRL_VDD_180) == status)
+		return 0;
+
+	dev_warn(mmc_dev(mmc), "%s: Regulator output did not became stable\n",
+		mmc_hostname(mmc));
+
+	return -EAGAIN;
+}
+
 #define DRIVER_NAME "sdhci_msm"
 #define SDHCI_MSM_DUMP(f, x...) \
 	pr_err("%s: " DRIVER_NAME ": " f, mmc_hostname(host->mmc), ## x)
@@ -1960,6 +2171,7 @@ void sdhci_msm_dump_vendor_regs(struct sdhci_host *host)
 	.write_b = sdhci_msm_writeb,
 	.irq	= sdhci_msm_cqe_irq,
 	.dump_vendor_regs = sdhci_msm_dump_vendor_regs,
+	.set_power = sdhci_set_power_noreg,
 };
 
 static const struct sdhci_pltfm_data sdhci_msm_pdata = {
@@ -2169,6 +2381,10 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	if (core_major == 1 && core_minor >= 0x49)
 		msm_host->updated_ddr_cfg = true;
 
+	ret = sdhci_msm_register_vreg(msm_host);
+	if (ret)
+		goto clk_disable;
+
 	/*
 	 * Power on reset state may trigger power irq if previous status of
 	 * PWRCTL was either BUS_ON or IO_HIGH_V. So before enabling pwr irq
@@ -2213,6 +2429,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 					 MSM_MMC_AUTOSUSPEND_DELAY_MS);
 	pm_runtime_use_autosuspend(&pdev->dev);
 
+	host->mmc_host_ops.start_signal_voltage_switch =
+		sdhci_msm_start_signal_voltage_switch;
 	host->mmc_host_ops.execute_tuning = sdhci_msm_execute_tuning;
 	if (of_property_read_bool(node, "supports-cqe"))
 		ret = sdhci_msm_cqe_add_host(host, pdev);
@@ -2220,7 +2438,6 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 		ret = sdhci_add_host(host);
 	if (ret)
 		goto pm_runtime_disable;
-	sdhci_msm_set_regulator_caps(msm_host);
 
 	pm_runtime_mark_last_busy(&pdev->dev);
 	pm_runtime_put_autosuspend(&pdev->dev);
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

Re: [PATCH V3 3/3] mmc: sdhci-msm: Use internal voltage control

[Veerabhadrarao Badiganti]

Hi Bjorn,

Do you have any comments on V3 patchset?

Thanks
Veera

On 6/2/2020 4:17 PM, Veerabhadrarao Badiganti 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.
>
> Set the regulator load as well so that regulator can be configured
> in LPM mode when in is not being used.
>
> Co-developed-by: Asutosh Das <asutoshd@codeaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> Co-developed-by: Vijay Viswanath <vviswana@codeaurora.org>
> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
> Co-developed-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
> ---
>   drivers/mmc/host/sdhci-msm.c | 235 +++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 226 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 95cd9735e9a3..20ef90fc7dd7 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -36,7 +36,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)
> @@ -277,6 +279,8 @@ struct sdhci_msm_host {
>   	bool uses_tassadar_dll;
>   	u32 dll_config;
>   	u32 ddr_config;
> +	u32 vqmmc_load;
> +	bool vqmmc_enabled;
>   };
>   
>   static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
> @@ -1339,6 +1343,91 @@ 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;
> +
> +	if (!msm_host->vqmmc_load)
> +		return 0;
> +
> +	load = hpm ? msm_host->vqmmc_load : 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)		||
> +	    (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.
> +	 */
> +	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);
> +
> +	return ret;
> +}
> +
>   static inline void sdhci_msm_init_pwr_irq_wait(struct sdhci_msm_host *msm_host)
>   {
>   	init_waitqueue_head(&msm_host->pwr_irq_wait);
> @@ -1442,8 +1531,9 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>   {
>   	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>   	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	struct mmc_host *mmc = host->mmc;
>   	u32 irq_status, irq_ack = 0;
> -	int retry = 10;
> +	int retry = 10, ret;
>   	u32 pwr_state = 0, io_level = 0;
>   	u32 config;
>   	const struct sdhci_msm_offset *msm_offset = msm_host->offset;
> @@ -1481,21 +1571,42 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>   	if (irq_status & CORE_PWRCTL_BUS_ON) {
>   		pwr_state = REQ_BUS_ON;
>   		io_level = REQ_IO_HIGH;
> -		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
>   	}
>   	if (irq_status & CORE_PWRCTL_BUS_OFF) {
>   		pwr_state = REQ_BUS_OFF;
>   		io_level = REQ_IO_LOW;
> -		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
>   	}
> +
> +	if (pwr_state) {
> +		ret = sdhci_msm_set_vmmc(mmc);
> +		if (!ret)
> +			ret = sdhci_msm_set_vqmmc(msm_host, mmc,
> +					pwr_state & REQ_BUS_ON);
> +		if (!ret)
> +			irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
> +		else
> +			irq_ack |= CORE_PWRCTL_BUS_FAIL;
> +	}
> +
>   	/* Handle IO LOW/HIGH */
> -	if (irq_status & CORE_PWRCTL_IO_LOW) {
> +	if (irq_status & CORE_PWRCTL_IO_LOW)
>   		io_level = REQ_IO_LOW;
> -		irq_ack |= CORE_PWRCTL_IO_SUCCESS;
> -	}
> -	if (irq_status & CORE_PWRCTL_IO_HIGH) {
> +
> +	if (irq_status & CORE_PWRCTL_IO_HIGH)
>   		io_level = REQ_IO_HIGH;
> +
> +	if (io_level)
>   		irq_ack |= CORE_PWRCTL_IO_SUCCESS;
> +
> +	if (io_level && !IS_ERR(mmc->supply.vqmmc) && !pwr_state) {
> +		ret = mmc_regulator_set_vqmmc(mmc, &mmc->ios);
> +		if (ret < 0) {
> +			dev_err(mmc_dev(mmc), "%s: IO_level setting failed(%d). signal_voltage: %d, vdd: %d irq_status: 0x%08x\n",
> +					mmc_hostname(mmc), ret,
> +					mmc->ios.signal_voltage, mmc->ios.vdd,
> +					irq_status);
> +			irq_ack |= CORE_PWRCTL_IO_FAIL;
> +		}
>   	}
>   
>   	/*
> @@ -1544,7 +1655,7 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>   	if (io_level)
>   		msm_host->curr_io_level = io_level;
>   
> -	pr_debug("%s: %s: Handled IRQ(%d), irq_status=0x%x, ack=0x%x\n",
> +	dev_dbg(mmc_dev(mmc), "%s: %s: Handled IRQ(%d), irq_status=0x%x, ack=0x%x\n",
>   		mmc_hostname(msm_host->mmc), __func__, irq, irq_status,
>   		irq_ack);
>   }
> @@ -1874,6 +1985,106 @@ static void sdhci_msm_reset(struct sdhci_host *host, u8 mask)
>   	sdhci_reset(host, mask);
>   }
>   
> +static int sdhci_msm_register_vreg(struct sdhci_msm_host *msm_host)
> +{
> +	int ret;
> +	u32 vmmc_load;
> +	struct mmc_host *mmc = msm_host->mmc;
> +
> +	ret = mmc_regulator_get_supply(msm_host->mmc);
> +	if (ret)
> +		return ret;
> +	device_property_read_u32(&msm_host->pdev->dev,
> +			"vmmc-supply-max-microamp",
> +			&vmmc_load);
> +	device_property_read_u32(&msm_host->pdev->dev,
> +			"vqmmc-supply-max-microamp",
> +			&msm_host->vqmmc_load);
> +
> +	/* Set active load */
> +	if (!IS_ERR(mmc->supply.vmmc) && vmmc_load) {
> +		ret = regulator_set_load(mmc->supply.vmmc, vmmc_load);
> +		if (ret) {
> +			dev_err(mmc_dev(mmc), "%s: vmmc set active load failed: %d\n",
> +				mmc_hostname(mmc), ret);
> +			return ret;
> +		}
> +	}
> +	if (!IS_ERR(mmc->supply.vqmmc) && msm_host->vqmmc_load) {
> +		ret = regulator_set_load(mmc->supply.vmmc,
> +					msm_host->vqmmc_load);
> +		if (ret) {
> +			dev_err(mmc_dev(mmc), "%s: vqmmc set active load failed: %d\n",
> +				mmc_hostname(mmc), ret);
> +			return ret;
> +		}
> +	}
> +
> +	sdhci_msm_set_regulator_caps(msm_host);
> +	mmc->ios.power_mode = MMC_POWER_UNDEFINED;
> +
> +	return 0;
> +}
> +
> +static int sdhci_msm_start_signal_voltage_switch(struct mmc_host *mmc,
> +				      struct mmc_ios *ios)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +	u16 ctrl, status;
> +
> +	/*
> +	 * Signal Voltage Switching is only applicable for Host Controllers
> +	 * v3.00 and above.
> +	 */
> +	if (host->version < SDHCI_SPEC_300)
> +		return 0;
> +
> +	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +
> +	switch (ios->signal_voltage) {
> +	case MMC_SIGNAL_VOLTAGE_330:
> +		if (!(host->flags & SDHCI_SIGNALING_330))
> +			return -EINVAL;
> +
> +		/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
> +		ctrl &= ~SDHCI_CTRL_VDD_180;
> +		break;
> +	case MMC_SIGNAL_VOLTAGE_180:
> +		if (!(host->flags & SDHCI_SIGNALING_180))
> +			return -EINVAL;
> +
> +		/*
> +		 * Enable 1.8V Signal Enable in the Host Control2
> +		 * register
> +		 */
> +		ctrl |= SDHCI_CTRL_VDD_180;
> +		break;
> +	case MMC_SIGNAL_VOLTAGE_120:
> +		if (!(host->flags & SDHCI_SIGNALING_120))
> +			return -EINVAL;
> +		return 0;
> +	default:
> +		/* No signal voltage switch required */
> +		return 0;
> +	}
> +
> +	sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> +
> +	/* Wait for 5ms */
> +	usleep_range(5000, 5500);
> +
> +	/* regulator output should be stable within 5 ms */
> +	status = ctrl & SDHCI_CTRL_VDD_180;
> +	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +	if ((ctrl & SDHCI_CTRL_VDD_180) == status)
> +		return 0;
> +
> +	dev_warn(mmc_dev(mmc), "%s: Regulator output did not became stable\n",
> +		mmc_hostname(mmc));
> +
> +	return -EAGAIN;
> +}
> +
>   #define DRIVER_NAME "sdhci_msm"
>   #define SDHCI_MSM_DUMP(f, x...) \
>   	pr_err("%s: " DRIVER_NAME ": " f, mmc_hostname(host->mmc), ## x)
> @@ -1960,6 +2171,7 @@ void sdhci_msm_dump_vendor_regs(struct sdhci_host *host)
>   	.write_b = sdhci_msm_writeb,
>   	.irq	= sdhci_msm_cqe_irq,
>   	.dump_vendor_regs = sdhci_msm_dump_vendor_regs,
> +	.set_power = sdhci_set_power_noreg,
>   };
>   
>   static const struct sdhci_pltfm_data sdhci_msm_pdata = {
> @@ -2169,6 +2381,10 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>   	if (core_major == 1 && core_minor >= 0x49)
>   		msm_host->updated_ddr_cfg = true;
>   
> +	ret = sdhci_msm_register_vreg(msm_host);
> +	if (ret)
> +		goto clk_disable;
> +
>   	/*
>   	 * Power on reset state may trigger power irq if previous status of
>   	 * PWRCTL was either BUS_ON or IO_HIGH_V. So before enabling pwr irq
> @@ -2213,6 +2429,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>   					 MSM_MMC_AUTOSUSPEND_DELAY_MS);
>   	pm_runtime_use_autosuspend(&pdev->dev);
>   
> +	host->mmc_host_ops.start_signal_voltage_switch =
> +		sdhci_msm_start_signal_voltage_switch;
>   	host->mmc_host_ops.execute_tuning = sdhci_msm_execute_tuning;
>   	if (of_property_read_bool(node, "supports-cqe"))
>   		ret = sdhci_msm_cqe_add_host(host, pdev);
> @@ -2220,7 +2438,6 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>   		ret = sdhci_add_host(host);
>   	if (ret)
>   		goto pm_runtime_disable;
> -	sdhci_msm_set_regulator_caps(msm_host);
>   
>   	pm_runtime_mark_last_busy(&pdev->dev);
>   	pm_runtime_put_autosuspend(&pdev->dev);