On Sun, 5 Sep 2010 11:05:38 +0200
Linus Walleij <linus.walleij@xxxxxxxxxxxxxx> wrote:
After discovering a problem in regulator reference counting I
took Mark Brown's advice to move the reference count into the
MMC core by making the regulator status a member of
struct mmc_host.
...
-static inline void pxamci_set_power(struct pxamci_host *host, unsigned int vdd)
+static inline void pxamci_set_power(struct pxamci_host *host,
+ unsigned char power_mode,
+ unsigned int vdd)
{
int on;
-#ifdef CONFIG_REGULATOR
- if (host->vcc)
- mmc_regulator_set_ocr(host->vcc, vdd);
-#endif
+ if (host->vcc) {
+ int ret;
+
+ if (power_mode == MMC_POWER_UP)
+ ret = mmc_regulator_set_ocr(host->mmc, host->vcc, vdd);
+ else if (power_mode == MMC_POWER_OFF)
+ ret = mmc_regulator_set_ocr(host->mmc, host->vcc, 0);
+ }
There's no point in copying the return value into a local then ignoring
it. mmc_regulator_set_ocr() can return a negative errno so we should
test for that, clean up and propagate the error.
If we really do deliberately ignore the error then there should be a
code comment which excuses this behaviour and perhaps a warning printk.
The same comments apply to mmci_set_ios().
omap_hsmmc_1_set_power() gets it right.
Why doesn't omap_hsmmc_23_set_sleep() run .before_set_reg() and
.after_set_reg()?
...