+ mmc-use-regulator-framework-correctly.patch added to -mm tree

+ mmc-use-regulator-framework-correctly.patch added to -mm tree

[akpm]

The patch titled
     mmc: use regulator framework correctly
has been added to the -mm tree.  Its filename is
     mmc-use-regulator-framework-correctly.patch

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/SubmitChecklist when testing your code ***

See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find
out what to do about this

The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/

------------------------------------------------------
Subject: mmc: use regulator framework correctly
From: Adrian Hunter <adrian.hunter@nokia.com>

Issues with the regulator framework no longer exist, so regulator_enable()
/ regulator_disable() should be used correctly.

Signed-off-by: Adrian Hunter <adrian.hunter@nokia.com>
Cc: Liam Girdwood <lrg@slimlogic.co.uk>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: <linux-mmc@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/mmc/core/core.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff -puN drivers/mmc/core/core.c~mmc-use-regulator-framework-correctly drivers/mmc/core/core.c
--- a/drivers/mmc/core/core.c~mmc-use-regulator-framework-correctly
+++ a/drivers/mmc/core/core.c
@@ -784,11 +784,6 @@ int mmc_regulator_set_ocr(struct regulat
 {
 	int			result = 0;
 	int			min_uV, max_uV;
-	int			enabled;
-
-	enabled = regulator_is_enabled(supply);
-	if (enabled < 0)
-		return enabled;
 
 	if (vdd_bit) {
 		int		tmp;
@@ -819,9 +814,9 @@ int mmc_regulator_set_ocr(struct regulat
 		else
 			result = 0;
 
-		if (result == 0 && !enabled)
+		if (result == 0)
 			result = regulator_enable(supply);
-	} else if (enabled) {
+	} else {
 		result = regulator_disable(supply);
 	}
 
_

Patches currently in -mm which might be from adrian.hunter@nokia.com are

origin.patch
linux-next.patch
mmc-recognize-csd-structure.patch
mmc-recognize-csd-structure-fix.patch
mmc-split-mmc_sd_init_card.patch
mmc-implement-sd-combo-iomem-support.patch
mmc-omap-fix-for-bus-width-which-improves-sd-cards-peformance.patch
sdio-allow-non-standard-sdio-cards.patch
omap_hsmmc-add-init_card-pass-through-callback.patch
omap-pandora-pass-wl1251-information-to-sdio-core.patch
mmc-add-erase-secure-erase-trim-and-secure-trim-operations.patch
mmc_block-add-discard-support.patch
omap_hsmmc-add-erase-capability.patch
block-add-secure-discard.patch
mmc_block-add-support-for-secure-discard.patch
mmc_test-add-performance-tests.patch
mmc_test-fix-large-memory-allocation.patch
mmc-use-regulator-framework-correctly.patch

Re: + mmc-use-regulator-framework-correctly.patch added to -mm tree

[Mark Brown]

On 5 Aug 2010, at 22:32, akpm@linux-foundation.org wrote:

> Subject: mmc: use regulator framework correctly
> From: Adrian Hunter <adrian.hunter@nokia.com>

> Issues with the regulator framework no longer exist, so regulator_enable()
> / regulator_disable() should be used correctly.

There have been no changes in the regulator framework here. David had some strong ideas about what he wanted the regulator API to do which caused him to make a number of forceful statements but that's not the same thing.

> -	int			enabled;
> -
> -	enabled = regulator_is_enabled(supply);
> -	if (enabled < 0)
> -		return enabled;
> 
> 	if (vdd_bit) {
> 		int		tmp;
> @@ -819,9 +814,9 @@ int mmc_regulator_set_ocr(struct regulat
> 		else
> 			result = 0;
> 
> -		if (result == 0 && !enabled)
> +		if (result == 0)
> 			result = regulator_enable(supply);
> -	} else if (enabled) {
> +	} else {
> 		result = regulator_disable(supply);
> 	}

This is only going to do the right thing if we can rely on the MMC framework matching the number of enables it does with the number of disables, including handling of startup with an already enabled regulator causing the refcount to start at one if regulator_get_exclusive() is used (which the current code does rely on). I'd rather see the analysis in the changelog explaining why the new code is safe.

Re: + mmc-use-regulator-framework-correctly.patch added to -mm tree

[Adrian Hunter]

Mark Brown wrote:
> On 5 Aug 2010, at 22:32, akpm@linux-foundation.org wrote:
> 
>> Subject: mmc: use regulator framework correctly
>> From: Adrian Hunter <adrian.hunter@nokia.com>
> 
>> Issues with the regulator framework no longer exist, so regulator_enable()
>> / regulator_disable() should be used correctly.
> 
> There have been no changes in the regulator framework here. David had some strong ideas about what he wanted the regulator API to do which caused him to make a number of forceful statements but that's not the same thing.
> 
>> -	int			enabled;
>> -
>> -	enabled = regulator_is_enabled(supply);
>> -	if (enabled < 0)
>> -		return enabled;
>>
>> 	if (vdd_bit) {
>> 		int		tmp;
>> @@ -819,9 +814,9 @@ int mmc_regulator_set_ocr(struct regulat
>> 		else
>> 			result = 0;
>>
>> -		if (result == 0 && !enabled)
>> +		if (result == 0)
>> 			result = regulator_enable(supply);
>> -	} else if (enabled) {
>> +	} else {
>> 		result = regulator_disable(supply);
>> 	}
> 
> This is only going to do the right thing if we can rely on the MMC framework matching the number of enables it does with the number of disables, including handling of startup with an already enabled regulator causing the refcount to start at one if regulator_get_exclusive() is used (which the current code does rely on). I'd rather see the analysis in the changelog explaining why the new code is safe.

Yes, the patch is no good.  If I had tried to make a decent commit message,
as you pointed out, I would not have sent the patch.  Sorry all.

Andrew please remove this patch.