diff mbox

[06/23] mmc: sdhci: using common mmc_regulator_set_vqmmc()

Message ID 1460741387-23815-7-git-send-email-aisheng.dong@nxp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Aisheng Dong April 15, 2016, 5:29 p.m. UTC
Instead of using private VCCQ regulator signal voltage switch code,
we switch to use the more robust common function mmc_regulator_set_vqmmc()
in MMC core which set the target voltage as close as possible to target
voltage.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
Don't have a board to test mmc_regulator_set_vqmmc() switch way,
need others to help verify.
---
 drivers/mmc/host/sdhci.c | 39 ++-------------------------------------
 1 file changed, 2 insertions(+), 37 deletions(-)

Comments

Adrian Hunter April 22, 2016, 11:48 a.m. UTC | #1
On 15/04/16 20:29, Dong Aisheng wrote:
> Instead of using private VCCQ regulator signal voltage switch code,
> we switch to use the more robust common function mmc_regulator_set_vqmmc()
> in MMC core which set the target voltage as close as possible to target
> voltage.
> 
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
> Don't have a board to test mmc_regulator_set_vqmmc() switch way,
> need others to help verify.

So was there a reason you wanted to change it?

I agree mmc_regulator_set_vqmmc() looks superior, but we really need
feedback from people using this code..

> ---
>  drivers/mmc/host/sdhci.c | 39 ++-------------------------------------
>  1 file changed, 2 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 7f63f5d..2338aab 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1726,43 +1726,8 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
>  	if (ret)
>  		return ret;
>  
> -	if (IS_ERR(mmc->supply.vqmmc))
> -		return 0;
> -
> -	switch (ios->signal_voltage) {
> -	case MMC_SIGNAL_VOLTAGE_330:
> -		ret = regulator_set_voltage(mmc->supply.vqmmc, 2700000,
> -					    3600000);
> -		if (ret) {
> -			pr_warn("%s: Switching to 3.3V signalling voltage failed\n",
> -				mmc_hostname(mmc));
> -			return -EIO;
> -		}
> -
> -		return 0;
> -	case MMC_SIGNAL_VOLTAGE_180:
> -		ret = regulator_set_voltage(mmc->supply.vqmmc,
> -				1700000, 1950000);
> -		if (ret) {
> -			pr_warn("%s: Switching to 1.8V signalling voltage failed\n",
> -				mmc_hostname(mmc));
> -			return -EIO;
> -		}
> -
> -		return 0;
> -	case MMC_SIGNAL_VOLTAGE_120:
> -		ret = regulator_set_voltage(mmc->supply.vqmmc, 1100000,
> -					    1300000);
> -		if (ret) {
> -			pr_warn("%s: Switching to 1.2V signalling voltage failed\n",
> -				mmc_hostname(mmc));
> -			return -EIO;
> -		}
> -		return 0;
> -	default:
> -		/* No signal voltage switch required */
> -		return 0;
> -	}
> +	/* do regulator signal voltage switch if exist */
> +	return mmc_regulator_set_vqmmc(mmc, ios);
>  }
>  
>  static int sdhci_card_busy(struct mmc_host *mmc)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dong Aisheng April 24, 2016, 9:25 a.m. UTC | #2
On Fri, Apr 22, 2016 at 7:48 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 15/04/16 20:29, Dong Aisheng wrote:
>> Instead of using private VCCQ regulator signal voltage switch code,
>> we switch to use the more robust common function mmc_regulator_set_vqmmc()
>> in MMC core which set the target voltage as close as possible to target
>> voltage.
>>
>> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
>> ---
>> Don't have a board to test mmc_regulator_set_vqmmc() switch way,
>> need others to help verify.
>
> So was there a reason you wanted to change it?
>

Yes, main reason is we already have common mmc_regulator_set_vqmmc(),
we don't want to maintain two copies of it. And the common one is more robust.

The original one is more error prone since it may set a margine IO voltage
which may more easy to lead an potential IO error.
e.g. if vqmmc is capable of 1.7V, then our host will set to 1.7v
rather than 1.8v
which is a mininum allowed IO voltage range(1.7v ~ 1.95v) defined by spec.

> I agree mmc_regulator_set_vqmmc() looks superior, but we really need
> feedback from people using this code..
>

Yes, although from code inspection, there's no functionality change,
but tests from others using it will be appreciated.

Regards
Dong Aisheng

>> ---
>>  drivers/mmc/host/sdhci.c | 39 ++-------------------------------------
>>  1 file changed, 2 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 7f63f5d..2338aab 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1726,43 +1726,8 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
>>       if (ret)
>>               return ret;
>>
>> -     if (IS_ERR(mmc->supply.vqmmc))
>> -             return 0;
>> -
>> -     switch (ios->signal_voltage) {
>> -     case MMC_SIGNAL_VOLTAGE_330:
>> -             ret = regulator_set_voltage(mmc->supply.vqmmc, 2700000,
>> -                                         3600000);
>> -             if (ret) {
>> -                     pr_warn("%s: Switching to 3.3V signalling voltage failed\n",
>> -                             mmc_hostname(mmc));
>> -                     return -EIO;
>> -             }
>> -
>> -             return 0;
>> -     case MMC_SIGNAL_VOLTAGE_180:
>> -             ret = regulator_set_voltage(mmc->supply.vqmmc,
>> -                             1700000, 1950000);
>> -             if (ret) {
>> -                     pr_warn("%s: Switching to 1.8V signalling voltage failed\n",
>> -                             mmc_hostname(mmc));
>> -                     return -EIO;
>> -             }
>> -
>> -             return 0;
>> -     case MMC_SIGNAL_VOLTAGE_120:
>> -             ret = regulator_set_voltage(mmc->supply.vqmmc, 1100000,
>> -                                         1300000);
>> -             if (ret) {
>> -                     pr_warn("%s: Switching to 1.2V signalling voltage failed\n",
>> -                             mmc_hostname(mmc));
>> -                     return -EIO;
>> -             }
>> -             return 0;
>> -     default:
>> -             /* No signal voltage switch required */
>> -             return 0;
>> -     }
>> +     /* do regulator signal voltage switch if exist */
>> +     return mmc_regulator_set_vqmmc(mmc, ios);
>>  }
>>
>>  static int sdhci_card_busy(struct mmc_host *mmc)
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 7f63f5d..2338aab 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1726,43 +1726,8 @@  static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
 	if (ret)
 		return ret;
 
-	if (IS_ERR(mmc->supply.vqmmc))
-		return 0;
-
-	switch (ios->signal_voltage) {
-	case MMC_SIGNAL_VOLTAGE_330:
-		ret = regulator_set_voltage(mmc->supply.vqmmc, 2700000,
-					    3600000);
-		if (ret) {
-			pr_warn("%s: Switching to 3.3V signalling voltage failed\n",
-				mmc_hostname(mmc));
-			return -EIO;
-		}
-
-		return 0;
-	case MMC_SIGNAL_VOLTAGE_180:
-		ret = regulator_set_voltage(mmc->supply.vqmmc,
-				1700000, 1950000);
-		if (ret) {
-			pr_warn("%s: Switching to 1.8V signalling voltage failed\n",
-				mmc_hostname(mmc));
-			return -EIO;
-		}
-
-		return 0;
-	case MMC_SIGNAL_VOLTAGE_120:
-		ret = regulator_set_voltage(mmc->supply.vqmmc, 1100000,
-					    1300000);
-		if (ret) {
-			pr_warn("%s: Switching to 1.2V signalling voltage failed\n",
-				mmc_hostname(mmc));
-			return -EIO;
-		}
-		return 0;
-	default:
-		/* No signal voltage switch required */
-		return 0;
-	}
+	/* do regulator signal voltage switch if exist */
+	return mmc_regulator_set_vqmmc(mmc, ios);
 }
 
 static int sdhci_card_busy(struct mmc_host *mmc)