diff mbox

[3/3] mmc: sdhci: check voltage range only on regulators aware of voltage value

Message ID 1354629663-29091-4-git-send-email-m.szyprowski@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Marek Szyprowski Dec. 4, 2012, 2:01 p.m. UTC
Some regulators don't report any voltage values, so checking supported
voltage range results in disabling all SDHCI_CAN_VDD_* flags and
registration failure. This patch finally provides a correct fix for the
registration of SDHCI driver with all possible voltage regulators:
dummy, fixed and regulated without using regulator_count_voltages()
hacks.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/mmc/host/sdhci.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Kevin Liu Dec. 4, 2012, 2:50 p.m. UTC | #1
2012/12/4 Marek Szyprowski <m.szyprowski@samsung.com>:
> Some regulators don't report any voltage values, so checking supported
> voltage range results in disabling all SDHCI_CAN_VDD_* flags and
> registration failure. This patch finally provides a correct fix for the
> registration of SDHCI driver with all possible voltage regulators:
> dummy, fixed and regulated without using regulator_count_voltages()
> hacks.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/mmc/host/sdhci.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index a9ad2cd..d244dc0 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2930,7 +2930,11 @@ int sdhci_add_host(struct sdhci_host *host)
>         }
>
>  #ifdef CONFIG_REGULATOR
> -       if (host->vmmc && regulator_count_voltages(host->vmmc) > 1) {
> +       /*
> +        * Voltage range check makes sense only if regulator reports
> +        * any voltage value.
> +        */
> +       if (host->vmmc && regulator_get_voltage(host->vmmc) > 0) {
>                 ret = regulator_is_supported_voltage(host->vmmc, 2700000,
>                         3600000);
>                 if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))

Good idea. But how about the regulator is disabled at this point? So I
suggest to change to
     if (host->vmmc && regulator_get_voltage(host->vmmc) >= 0)

Thanks
Kevin
--
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
Mark Brown Dec. 5, 2012, 1:48 a.m. UTC | #2
On Tue, Dec 04, 2012 at 10:50:03PM +0800, Kevin Liu wrote:
> 2012/12/4 Marek Szyprowski <m.szyprowski@samsung.com>:

> > +       if (host->vmmc && regulator_get_voltage(host->vmmc) > 0) {
> >                 ret = regulator_is_supported_voltage(host->vmmc, 2700000,
> >                         3600000);
> >                 if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))

> Good idea. But how about the regulator is disabled at this point? So I
> suggest to change to
>      if (host->vmmc && regulator_get_voltage(host->vmmc) >= 0)

I'd not expect regulator_get_voltage() to return 0 for disabled
regulators, I'd expect it to return the voltage the regulator will have
when enabled.
--
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
Kevin Liu Dec. 5, 2012, 2:12 a.m. UTC | #3
2012/12/5 Mark Brown <broonie@opensource.wolfsonmicro.com>
>
> On Tue, Dec 04, 2012 at 10:50:03PM +0800, Kevin Liu wrote:
> > 2012/12/4 Marek Szyprowski <m.szyprowski@samsung.com>:
>
> > > +       if (host->vmmc && regulator_get_voltage(host->vmmc) > 0) {
> > >                 ret = regulator_is_supported_voltage(host->vmmc,
> > > 2700000,
> > >                         3600000);
> > >                 if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))
>
> > Good idea. But how about the regulator is disabled at this point? So I
> > suggest to change to
> >      if (host->vmmc && regulator_get_voltage(host->vmmc) >= 0)
>
> I'd not expect regulator_get_voltage() to return 0 for disabled
> regulators, I'd expect it to return the voltage the regulator will have
> when enabled.
Understand.

Thanks
Kevin
--
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
Marek Szyprowski Feb. 1, 2013, 2:40 p.m. UTC | #4
Hello,

On 12/4/2012 3:01 PM, Marek Szyprowski wrote:
> Some regulators don't report any voltage values, so checking supported
> voltage range results in disabling all SDHCI_CAN_VDD_* flags and
> registration failure. This patch finally provides a correct fix for the
> registration of SDHCI driver with all possible voltage regulators:
> dummy, fixed and regulated without using regulator_count_voltages()
> hacks.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

Any chance to get this patch scheduled for v3.9?

> ---
>   drivers/mmc/host/sdhci.c |    6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index a9ad2cd..d244dc0 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2930,7 +2930,11 @@ int sdhci_add_host(struct sdhci_host *host)
>   	}
>   
>   #ifdef CONFIG_REGULATOR
> -	if (host->vmmc && regulator_count_voltages(host->vmmc) > 1) {
> +	/*
> +	 * Voltage range check makes sense only if regulator reports
> +	 * any voltage value.
> +	 */
> +	if (host->vmmc && regulator_get_voltage(host->vmmc) > 0) {
>   		ret = regulator_is_supported_voltage(host->vmmc, 2700000,
>   			3600000);
>   		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))

Best regards
Chris Ball Feb. 11, 2013, 5:27 p.m. UTC | #5
Hi Marek,

On Fri, Feb 01 2013, Marek Szyprowski wrote:
> On 12/4/2012 3:01 PM, Marek Szyprowski wrote:
>> Some regulators don't report any voltage values, so checking supported
>> voltage range results in disabling all SDHCI_CAN_VDD_* flags and
>> registration failure. This patch finally provides a correct fix for the
>> registration of SDHCI driver with all possible voltage regulators:
>> dummy, fixed and regulated without using regulator_count_voltages()
>> hacks.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>
> Any chance to get this patch scheduled for v3.9?

This one doesn't apply -- please could you rebase/resend?

Thanks,

- Chris.
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index a9ad2cd..d244dc0 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2930,7 +2930,11 @@  int sdhci_add_host(struct sdhci_host *host)
 	}
 
 #ifdef CONFIG_REGULATOR
-	if (host->vmmc && regulator_count_voltages(host->vmmc) > 1) {
+	/*
+	 * Voltage range check makes sense only if regulator reports
+	 * any voltage value.
+	 */
+	if (host->vmmc && regulator_get_voltage(host->vmmc) > 0) {
 		ret = regulator_is_supported_voltage(host->vmmc, 2700000,
 			3600000);
 		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))