Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755539Ab0FXPMu (ORCPT ); Thu, 24 Jun 2010 11:12:50 -0400 Received: from eu1sys200aog113.obsmtp.com ([207.126.144.135]:47110 "EHLO eu1sys200aog113.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754063Ab0FXPMs (ORCPT ); Thu, 24 Jun 2010 11:12:48 -0400 From: Linus Walleij To: , , Cc: , Linus Walleij , Liam Girdwood , Mark Brown , Tony Lindgren , Adrian Hunter , Robert Jarzmik , Sundar Iyer , Bengt Jonsson Subject: [PATCH] MMC: remove regulator refcount fiddling in mmc core Date: Thu, 24 Jun 2010 17:12:17 +0200 Message-ID: <1277392337-27627-1-git-send-email-linus.walleij@stericsson.com> X-Mailer: git-send-email 1.6.3.3 MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8323 Lines: 261 Currently the mmc_regulator_set_ocr() fiddles with the regulator refcount by selectively calling regulator_[enable|disable] depending on the state of the regulator. This will confuse the reference count if case the regulator is for example shared with other MMC slots or user for other stuff than the MMC card. Push regulator_[enable|disable] out into the MMC host drivers and remove this from the MMC core so the reference count can be trusted. Cc: Andrew Morton Cc: Liam Girdwood Cc: Mark Brown Cc: Tony Lindgren Cc: Adrian Hunter Cc: Robert Jarzmik Cc: Sundar Iyer Cc: Bengt Jonsson Signed-off-by: Linus Walleij --- This has been regression compiled for U300, OMAP3, PXA3XX defconfigs, tested on U300. We're facing problems with our regulator reference counters if this is not fixed. --- drivers/mmc/core/core.c | 66 ++++++++++++++++++----------------------- drivers/mmc/host/mmci.c | 10 ++++-- drivers/mmc/host/omap_hsmmc.c | 40 ++++++++++++++++++------ drivers/mmc/host/pxamci.c | 17 ++++++++-- 4 files changed, 78 insertions(+), 55 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 569e94d..904f245 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -784,47 +784,39 @@ int mmc_regulator_set_ocr(struct regulator *supply, unsigned short vdd_bit) { int result = 0; int min_uV, max_uV; - int enabled; + int tmp; + int voltage; - enabled = regulator_is_enabled(supply); - if (enabled < 0) - return enabled; - - if (vdd_bit) { - int tmp; - int voltage; - - /* REVISIT mmc_vddrange_to_ocrmask() may have set some - * bits this regulator doesn't quite support ... don't - * be too picky, most cards and regulators are OK with - * a 0.1V range goof (it's a small error percentage). - */ - tmp = vdd_bit - ilog2(MMC_VDD_165_195); - if (tmp == 0) { - min_uV = 1650 * 1000; - max_uV = 1950 * 1000; - } else { - min_uV = 1900 * 1000 + tmp * 100 * 1000; - max_uV = min_uV + 100 * 1000; - } - - /* avoid needless changes to this voltage; the regulator - * might not allow this operation - */ - voltage = regulator_get_voltage(supply); - if (voltage < 0) - result = voltage; - else if (voltage < min_uV || voltage > max_uV) - result = regulator_set_voltage(supply, min_uV, max_uV); - else - result = 0; + if (!vdd_bit) + return 0; - if (result == 0 && !enabled) - result = regulator_enable(supply); - } else if (enabled) { - result = regulator_disable(supply); + /* + * REVISIT mmc_vddrange_to_ocrmask() may have set some + * bits this regulator doesn't quite support ... don't + * be too picky, most cards and regulators are OK with + * a 0.1V range goof (it's a small error percentage). + */ + tmp = vdd_bit - ilog2(MMC_VDD_165_195); + if (tmp == 0) { + min_uV = 1650 * 1000; + max_uV = 1950 * 1000; + } else { + min_uV = 1900 * 1000 + tmp * 100 * 1000; + max_uV = min_uV + 100 * 1000; } + /* + * Avoid needless changes to this voltage; the regulator + * might not allow this operation + */ + voltage = regulator_get_voltage(supply); + if (voltage < 0) + result = voltage; + else if (voltage < min_uV || voltage > max_uV) + result = regulator_set_voltage(supply, min_uV, max_uV); + else + result = 0; + return result; } EXPORT_SYMBOL(mmc_regulator_set_ocr); diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 4917af9..5f530b1 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -467,15 +467,17 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) switch (ios->power_mode) { case MMC_POWER_OFF: - if(host->vcc && - regulator_is_enabled(host->vcc)) + if (host->vcc) regulator_disable(host->vcc); break; case MMC_POWER_UP: #ifdef CONFIG_REGULATOR - if (host->vcc) - /* This implicitly enables the regulator */ + if (host->vcc) { + if (regulator_enable(host->vcc)) + dev_err(mmc_dev(mmc), + "could not enable regulator\n"); mmc_regulator_set_ocr(host->vcc, ios->vdd); + } #endif /* * The translate_vdd function is not used if you have diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index b032828..d8d07e1 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -247,10 +247,17 @@ static int omap_hsmmc_1_set_power(struct device *dev, int slot, int power_on, if (mmc_slot(host).before_set_reg) mmc_slot(host).before_set_reg(dev, slot, power_on, vdd); - if (power_on) - ret = mmc_regulator_set_ocr(host->vcc, vdd); - else - ret = mmc_regulator_set_ocr(host->vcc, 0); + if (power_on) { + ret = regulator_enable(host->vcc); + if (ret) + dev_err(dev, "could not enable regulator\n"); + else + ret = mmc_regulator_set_ocr(host->vcc, vdd); + } else { + ret = regulator_disable(host->vcc); + if (ret) + dev_err(dev, "could not disable regulator\n"); + } if (mmc_slot(host).after_set_reg) mmc_slot(host).after_set_reg(dev, slot, power_on, vdd); @@ -289,7 +296,11 @@ static int omap_hsmmc_23_set_power(struct device *dev, int slot, int power_on, * chips/cards need an interface voltage rail too. */ if (power_on) { - ret = mmc_regulator_set_ocr(host->vcc, vdd); + ret = regulator_enable(host->vcc); + if (ret < 0) + dev_err(dev, "could not enable regulator\n"); + else + ret = mmc_regulator_set_ocr(host->vcc, vdd); /* Enable interface voltage rail, if needed */ if (ret == 0 && host->vcc_aux) { ret = regulator_enable(host->vcc_aux); @@ -297,10 +308,15 @@ static int omap_hsmmc_23_set_power(struct device *dev, int slot, int power_on, ret = mmc_regulator_set_ocr(host->vcc, 0); } } else { + /* Shut down the rail */ if (host->vcc_aux) ret = regulator_disable(host->vcc_aux); - if (ret == 0) - ret = mmc_regulator_set_ocr(host->vcc, 0); + if (ret == 0) { + /* Then proeeed to shut down the local regulator */ + ret = regulator_disable(host->vcc); + if (ret == 0) + ret = mmc_regulator_set_ocr(host->vcc, 0); + } } if (mmc_slot(host).after_set_reg) @@ -340,10 +356,14 @@ static int omap_hsmmc_23_set_sleep(struct device *dev, int slot, int sleep, if (cardsleep) { /* VCC can be turned off if card is asleep */ - if (sleep) - err = mmc_regulator_set_ocr(host->vcc, 0); - else + if (sleep) { + err = regulator_disable(host->vcc); + } else { + err = regulator_enable(host->vcc); + if (err) + return err; err = mmc_regulator_set_ocr(host->vcc, vdd); + } } else err = regulator_set_mode(host->vcc, mode); if (err) diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c index 0a4e43f..b7e0216 100644 --- a/drivers/mmc/host/pxamci.c +++ b/drivers/mmc/host/pxamci.c @@ -99,13 +99,22 @@ static inline void pxamci_init_ocr(struct pxamci_host *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); + if (host->vcc) { + if (power_mode == MMC_POWER_UP) { + if (regulator_enable(host->vcc)) + dev_err(mmc_dev(host->mmc), + "could not enable regulator\n"); + mmc_regulator_set_ocr(host->vcc, vdd); + } else if (power_mode == MMC_POWER_OFF) + regulator_disable(host->vcc); + } #endif if (!host->vcc && host->pdata && gpio_is_valid(host->pdata->gpio_power)) { @@ -492,7 +501,7 @@ static void pxamci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) if (host->power_mode != ios->power_mode) { host->power_mode = ios->power_mode; - pxamci_set_power(host, ios->vdd); + pxamci_set_power(host, ios->power_mode, ios->vdd); if (ios->power_mode == MMC_POWER_ON) host->cmdat |= CMDAT_INIT; -- 1.6.3.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/