Re: [PATCH v3] mmc_spi.c: add support for the regulator framework

[Antonio Ospite <ospite@studenti.unina.it>]

[-- Attachment #1: Type: text/plain, Size: 3975 bytes --]

On Wed, 18 May 2011 12:42:12 -0700
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> On Wed, May 18, 2011 at 07:23:20PM +0200, Antonio Ospite wrote:
> 
[...]
> >   2. Add a proper function with all the needed parameters to abstract
> >      the actual var names, this would be something like:
> >      mmc_regulator_set_power(mmc, power_mode, vdd, vcc, pdata)
> >      and yet checking for pdata->setpower can be tricky as 'setpower' 
> >      is an arbitrary name.
> 
> This would be good.

A WIP patch, I don't have a lot of time to spend on this, I tried to
handle the driver specific 'setpower' callback with a macro, let me
know if you have a better idea:

diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index bcb793e..dbbe535 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -298,6 +298,32 @@ static inline int mmc_regulator_set_ocr(struct mmc_host *mmc,
 }
 #endif
 
+#define STRUCT_FIELD(s, f) ((s) && (s)->f ? (s)->f : NULL )
+
+static inline int mmc_regulator_set_power(struct mmc_host *mmc,
+				 unsigned char	power_mode,
+				 struct regulator *supply,
+				 unsigned short vdd_bit,
+				 struct device *device,
+				 void (*setpower_cb)(struct device *, unsigned int))
+{
+	if (supply) {
+		int ret;
+
+		if (power_mode == MMC_POWER_OFF)
+			vdd_bit = 0;
+
+		ret = mmc_regulator_set_ocr(mmc, supply, vdd_bit);
+		if (ret)
+			return ret;
+	} else {
+		if (setpower_cb)
+			setpower_cb(device, vdd_bit);
+	}
+
+	return 0;
+}
+
 int mmc_card_awake(struct mmc_host *host);
 int mmc_card_sleep(struct mmc_host *host);
 int mmc_card_can_sleep(struct mmc_host *host);
diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 2e8db20..c9a229f 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -1231,21 +1231,18 @@ static inline int mmc_spi_setpower(struct mmc_spi_host *host,
 					unsigned char power_mode,
 					unsigned int vdd)
 {
+	int ret;
+
 	if (!mmc_spi_canpower(host))
 		return -ENOSYS;
 
-	if (host->vcc) {
-		int ret;
 
-		if (power_mode == MMC_POWER_OFF)
-			vdd = 0;
+	ret = mmc_regulator_set_power(host->mmc, power_mode, host->vcc, vdd,
+				      &host->spi->dev,
+				      STRUCT_FIELD(host->pdata, setpower));
+	if (ret)
+		return ret;
 
-		ret = mmc_regulator_set_ocr(host->mmc, host->vcc, vdd);
-		if (ret)
-			return ret;
-	} else {
-		host->pdata->setpower(&host->spi->dev, vdd);
-	}
 
 	if (power_mode == MMC_POWER_UP)
 		msleep(host->powerup_msecs);
diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
index fbdb21e..f5d9529 100644
--- a/drivers/mmc/host/pxamci.c
+++ b/drivers/mmc/host/pxamci.c
@@ -103,29 +103,20 @@ static inline int pxamci_set_power(struct pxamci_host *host,
 				    unsigned int vdd)
 {
 	int on;
+	int ret;
 
-	if (host->vcc) {
-		int ret;
+	ret = mmc_regulator_set_power(host->mmc, power_mode, host->vcc, vdd,
+				      mmc_dev(host->mmc),
+				      STRUCT_FIELD(host->pdata, setpower));
+	if (ret)
+		return ret;
 
-		if (power_mode == MMC_POWER_UP) {
-			ret = mmc_regulator_set_ocr(host->mmc, host->vcc, vdd);
-			if (ret)
-				return ret;
-		} else if (power_mode == MMC_POWER_OFF) {
-			ret = mmc_regulator_set_ocr(host->mmc, host->vcc, 0);
-			if (ret)
-				return ret;
-		}
-	}
 	if (!host->vcc && host->pdata &&
 	    gpio_is_valid(host->pdata->gpio_power)) {
 		on = ((1 << vdd) & host->pdata->ocr_mask);
 		gpio_set_value(host->pdata->gpio_power,
 			       !!on ^ host->pdata->gpio_power_invert);
 	}
-	if (!host->vcc && host->pdata && host->pdata->setpower)
-		host->pdata->setpower(mmc_dev(host->mmc), vdd);
-
 	return 0;
 }
 


-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]