Mark Brown wrote:
On 5 Aug 2010, at 22:32, [email protected] wrote:

Subject: mmc: use regulator framework correctly
From: Adrian Hunter <[email protected]>

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to