[PATCH 2/2] mmc: only support voltage (vdd) that regulator agree's with

[PATCH 2/2] mmc: only support voltage (vdd) that regulator agree's with

[philipspatches]

From: Philip Rakity <prakity@marvell.com>

If we are using a regulator the SD Host Controller and the
regulator should agree about the voltages supported.  Use
the common subset that is supported.

Signed-off-by: Philip Rakity <prakity@marvell.com>
---
 drivers/mmc/host/sdhci.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 248f68b..78be427 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2894,6 +2894,21 @@ int sdhci_add_host(struct sdhci_host *host)
 	 * register is set. The actual current value is 4 times the register
 	 * value.
 	 */
+	if (host->vmmc) {
+		ret = regulator_is_supported_voltage(host->vmmc, 3300000,
+			3300000);
+		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))
+			caps[0] &= ~SDHCI_CAN_VDD_330;
+		ret = regulator_is_supported_voltage(host->vmmc, 3000000,
+			3000000);
+		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_300)))
+			caps[0] &= ~SDHCI_CAN_VDD_300;
+		ret = regulator_is_supported_voltage(host->vmmc, 1800000,
+			1800000);
+		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_180)))
+			caps[0] &= ~SDHCI_CAN_VDD_180;
+	}
+
 	max_current_caps = sdhci_readl(host, SDHCI_MAX_CURRENT);
 	if (!max_current_caps && host->vmmc) {
 		u32 curr = regulator_get_current_limit(host->vmmc);
-- 
1.7.0.4

Re: [PATCH 2/2] mmc: only support voltage (vdd) that regulator agree's with

[Chris Ball]

Hi,

On Fri, Jun 08 2012, philipspatches@gmail.com wrote:
> From: Philip Rakity <prakity@marvell.com>
>
> If we are using a regulator the SD Host Controller and the
> regulator should agree about the voltages supported.  Use
> the common subset that is supported.
>
> Signed-off-by: Philip Rakity <prakity@marvell.com>
> ---
>  drivers/mmc/host/sdhci.c |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 248f68b..78be427 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2894,6 +2894,21 @@ int sdhci_add_host(struct sdhci_host *host)
>  	 * register is set. The actual current value is 4 times the register
>  	 * value.
>  	 */
> +	if (host->vmmc) {
> +		ret = regulator_is_supported_voltage(host->vmmc, 3300000,
> +			3300000);
> +		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))
> +			caps[0] &= ~SDHCI_CAN_VDD_330;
> +		ret = regulator_is_supported_voltage(host->vmmc, 3000000,
> +			3000000);
> +		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_300)))
> +			caps[0] &= ~SDHCI_CAN_VDD_300;
> +		ret = regulator_is_supported_voltage(host->vmmc, 1800000,
> +			1800000);
> +		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_180)))
> +			caps[0] &= ~SDHCI_CAN_VDD_180;
> +	}
> +
>  	max_current_caps = sdhci_readl(host, SDHCI_MAX_CURRENT);
>  	if (!max_current_caps && host->vmmc) {
>  		u32 curr = regulator_get_current_limit(host->vmmc);

Thanks, pushed to mmc-next for 3.6.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 2/2] mmc: only support voltage (vdd) that regulator agree's with

[Chris Ball]

Hi,

On Fri, Jun 08 2012, philipspatches@gmail.com wrote:
> From: Philip Rakity <prakity@marvell.com>
>
> If we are using a regulator the SD Host Controller and the
> regulator should agree about the voltages supported.  Use
> the common subset that is supported.
>
> Signed-off-by: Philip Rakity <prakity@marvell.com>

This breaks the build when CONFIG_REGULATOR=n -- I've applied what looks
like the correct fix to me:

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index eae7c3c..caba999 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2832,51 +2832,53 @@ int sdhci_add_host(struct sdhci_host *host)
+#ifdef CONFIG_REGULATOR
 	/*
 	 * According to SD Host Controller spec v3.00, if the Host System
 	 * can afford more than 150mA, Host Driver should set XPC to 1. Also
 	 * the value is meaningful only if Voltage Support in the Capabilities
 	 * register is set. The actual current value is 4 times the register
 	 * value.
 	 */
 	if (host->vmmc) {
 		ret = regulator_is_supported_voltage(host->vmmc, 3300000,
 			3300000);
 		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))
 			caps[0] &= ~SDHCI_CAN_VDD_330;
 		ret = regulator_is_supported_voltage(host->vmmc, 3000000,
 			3000000);
 		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_300)))
 			caps[0] &= ~SDHCI_CAN_VDD_300;
 		ret = regulator_is_supported_voltage(host->vmmc, 1800000,
 			1800000);
 		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_180)))
 			caps[0] &= ~SDHCI_CAN_VDD_180;
 	}
+#endif /* CONFIG_REGULATOR */

-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 2/2] mmc: only support voltage (vdd) that regulator agree's with

[Aaron Lu]

Hi Chris,

On Fri, Jun 29, 2012 at 07:19:19PM -0400, Chris Ball wrote:
> Hi,
> 
> On Fri, Jun 08 2012, philipspatches@gmail.com wrote:
> > From: Philip Rakity <prakity@marvell.com>
> >
> > If we are using a regulator the SD Host Controller and the
> > regulator should agree about the voltages supported.  Use
> > the common subset that is supported.
> >
> > Signed-off-by: Philip Rakity <prakity@marvell.com>
> 
> This breaks the build when CONFIG_REGULATOR=n -- I've applied what looks
> like the correct fix to me:
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index eae7c3c..caba999 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2832,51 +2832,53 @@ int sdhci_add_host(struct sdhci_host *host)
> +#ifdef CONFIG_REGULATOR
>  	/*
>  	 * According to SD Host Controller spec v3.00, if the Host System
>  	 * can afford more than 150mA, Host Driver should set XPC to 1. Also
>  	 * the value is meaningful only if Voltage Support in the Capabilities
>  	 * register is set. The actual current value is 4 times the register
>  	 * value.
>  	 */
The above comments does not belong to regulator code, it should be
placed on top of the max_current_caps variable like this:

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index caba999..f76736b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2845,13 +2845,6 @@ int sdhci_add_host(struct sdhci_host *host)
 	}
 
 #ifdef CONFIG_REGULATOR
-	/*
-	 * According to SD Host Controller spec v3.00, if the Host System
-	 * can afford more than 150mA, Host Driver should set XPC to 1. Also
-	 * the value is meaningful only if Voltage Support in the Capabilities
-	 * register is set. The actual current value is 4 times the register
-	 * value.
-	 */
 	if (host->vmmc) {
 		ret = regulator_is_supported_voltage(host->vmmc, 3300000,
 			3300000);
@@ -2868,6 +2861,13 @@ int sdhci_add_host(struct sdhci_host *host)
 	}
 #endif /* CONFIG_REGULATOR */
 
+	/*
+	 * According to SD Host Controller spec v3.00, if the Host System
+	 * can afford more than 150mA, Host Driver should set XPC to 1. Also
+	 * the value is meaningful only if Voltage Support in the Capabilities
+	 * register is set. The actual current value is 4 times the register
+	 * value.
+	 */
 	max_current_caps = sdhci_readl(host, SDHCI_MAX_CURRENT);
 	if (!max_current_caps && host->vmmc) {
 		u32 curr = regulator_get_current_limit(host->vmmc);

Thanks,
Aaron

>  	if (host->vmmc) {
>  		ret = regulator_is_supported_voltage(host->vmmc, 3300000,
>  			3300000);
>  		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))
>  			caps[0] &= ~SDHCI_CAN_VDD_330;
>  		ret = regulator_is_supported_voltage(host->vmmc, 3000000,
>  			3000000);
>  		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_300)))
>  			caps[0] &= ~SDHCI_CAN_VDD_300;
>  		ret = regulator_is_supported_voltage(host->vmmc, 1800000,
>  			1800000);
>  		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_180)))
>  			caps[0] &= ~SDHCI_CAN_VDD_180;
>  	}
> +#endif /* CONFIG_REGULATOR */
> 
> -- 
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child
> 

Re: [PATCH 2/2] mmc: only support voltage (vdd) that regulator agree's with

[Chris Ball]

Hi Aaron,

On Tue, Jul 03 2012, Aaron Lu wrote:
> On Fri, Jun 29, 2012 at 07:19:19PM -0400, Chris Ball wrote:
>> Hi,
>> 
>> On Fri, Jun 08 2012, philipspatches@gmail.com wrote:
>> > From: Philip Rakity <prakity@marvell.com>
>> >
>> > If we are using a regulator the SD Host Controller and the
>> > regulator should agree about the voltages supported.  Use
>> > the common subset that is supported.
>> >
>> > Signed-off-by: Philip Rakity <prakity@marvell.com>
>> 
>> This breaks the build when CONFIG_REGULATOR=n -- I've applied what looks
>> like the correct fix to me:
>> 
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index eae7c3c..caba999 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -2832,51 +2832,53 @@ int sdhci_add_host(struct sdhci_host *host)
>> +#ifdef CONFIG_REGULATOR
>>  	/*
>>  	 * According to SD Host Controller spec v3.00, if the Host System
>>  	 * can afford more than 150mA, Host Driver should set XPC to 1. Also
>>  	 * the value is meaningful only if Voltage Support in the Capabilities
>>  	 * register is set. The actual current value is 4 times the register
>>  	 * value.
>>  	 */
> The above comments does not belong to regulator code, it should be
> placed on top of the max_current_caps variable like this:

Thanks for catching that!  I've rebased your change into the original patch.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 2/2] mmc: only support voltage (vdd) that regulator agree's with

[Tushar Behera]

On 06/09/2012 12:56 AM, philipspatches@gmail.com wrote:
> From: Philip Rakity <prakity@marvell.com>
> 
> If we are using a regulator the SD Host Controller and the
> regulator should agree about the voltages supported.  Use
> the common subset that is supported.
> 
> Signed-off-by: Philip Rakity <prakity@marvell.com>
> ---
>  drivers/mmc/host/sdhci.c |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 248f68b..78be427 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2894,6 +2894,21 @@ int sdhci_add_host(struct sdhci_host *host)
>  	 * register is set. The actual current value is 4 times the register
>  	 * value.
>  	 */
> +	if (host->vmmc) {
> +		ret = regulator_is_supported_voltage(host->vmmc, 3300000,
> +			3300000);
> +		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))
> +			caps[0] &= ~SDHCI_CAN_VDD_330;
> +		ret = regulator_is_supported_voltage(host->vmmc, 3000000,
> +			3000000);
> +		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_300)))
> +			caps[0] &= ~SDHCI_CAN_VDD_300;
> +		ret = regulator_is_supported_voltage(host->vmmc, 1800000,
> +			1800000);
> +		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_180)))
> +			caps[0] &= ~SDHCI_CAN_VDD_180;
> +	}
> +
>  	max_current_caps = sdhci_readl(host, SDHCI_MAX_CURRENT);
>  	if (!max_current_caps && host->vmmc) {
>  		u32 curr = regulator_get_current_limit(host->vmmc);
> 

Does the above code mean that 2.8V for vmmc regulator is not a valid value?

Currently I am working on a board for which vmmc regulator voltage is
set as 2.8V and the SD/MMC card is working fine (by not defining vmmc in
platform code).

-- 
Tushar Behera