diff mbox

[3/3] mmc: sdhci: apply voltage range check only for non-fixed regulators

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

Commit Message

Marek Szyprowski Nov. 13, 2012, 8:48 a.m. UTC
Fixed regulators cannot change their voltage, so disable all voltage range
checking for them, otherwise the driver fails to operate with fixed
regulators.

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

Comments

Chris Ball Nov. 13, 2012, 12:45 p.m. UTC | #1
Hi Marek,

On Tue, Nov 13 2012, Marek Szyprowski wrote:
> Fixed regulators cannot change their voltage, so disable all voltage range
> checking for them, otherwise the driver fails to operate with fixed
> regulators.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

Perhaps it's a good idea to mention that the regulator API is changing
(being fixed) at the same time, and that's why this patch is necessary.

> ---
>  drivers/mmc/host/sdhci.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index c7851c0..6f6534e 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
>  		regulator_enable(host->vmmc);
>  
>  #ifdef CONFIG_REGULATOR
> -	if (host->vmmc) {
> +	if (host->vmmc && regulator_count_voltages(host->vmmc) > 1) {
>  		ret = regulator_is_supported_voltage(host->vmmc, 3300000,
>  			3300000);
>  		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))

What about other occasions where is_supported_voltage() is used, like
this one -- is it necessary here too?

        else if (regulator_is_supported_voltage(host->vqmmc, 1800000, 1800000))
                regulator_enable(host->vqmmc);

Thanks,                

- Chris.
Mark Brown Nov. 14, 2012, 1:10 a.m. UTC | #2
On Tue, Nov 13, 2012 at 07:45:07AM -0500, Chris Ball wrote:
> On Tue, Nov 13 2012, Marek Szyprowski wrote:
> > Fixed regulators cannot change their voltage, so disable all voltage range
> > checking for them, otherwise the driver fails to operate with fixed
> > regulators.

> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

> Perhaps it's a good idea to mention that the regulator API is changing
> (being fixed) at the same time, and that's why this patch is necessary.

This should be totally unrelated to any externally visible change in the
regulator API, if anything I would expect fixes in the API to *improve*
the ability of clients to work with fixed voltage regulators.  What's
missing here is some explanation as to what the problem is and how it's
being fixed.

As I understand it this should really be a workaround for hardware which
runs cards at out of spec voltages; we have a fixed voltage regulator
which claims to not support any of the voltages that are in spec (due to
this being what the hardware does) so if we try to use the voltage
management stuff in the MMC API it gets upset.  As a workaround if we
can't change the voltage we just ignore the voltage aspects of the API.
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@  int sdhci_add_host(struct sdhci_host *host)
 		regulator_enable(host->vmmc);
 
 #ifdef CONFIG_REGULATOR
-	if (host->vmmc) {
+	if (host->vmmc && regulator_count_voltages(host->vmmc) > 1) {
 		ret = regulator_is_supported_voltage(host->vmmc, 3300000,
 			3300000);
 		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))