[PATCH 0/4] mmc_spi: Add support for regulator framework

[PATCH 0/4] mmc_spi: Add support for regulator framework

[Antonio Ospite]

Hi,

this patchset has the purpose of adding support for the regulator framework to 
the mmc_spi driver. The first three patches are preparatory cleanups to make 
the forth one more straightforward.

Maybe the fourth patch can be improved, I am open to any suggestions about it.

These changes take strong inspiration from the pxamci driver; they have been 
tested on a Motorola A910, which uses a regulator to powerup the MMC card 
connected to the SPI bus, a test from a current user of the mmc_spi driver 
would not hurt just to be sure no regressions have been introduced.

Thanks,
   Antonio

Antonio Ospite (4):
  mmc_spi.c: factor out the check for power capability
  mmc_spi.c: factor out the SD card shutdown sequence
  mmc_spi.c: factor out a mmc_spi_setpower() function
  mmc_spi.c: add support for the regulator framework

 drivers/mmc/host/mmc_spi.c |  194 +++++++++++++++++++++++++++++---------------
 1 files changed, 129 insertions(+), 65 deletions(-)

-- 
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?

[PATCH 1/4] mmc_spi.c: factor out the check for power capability

[Antonio Ospite]

Factor out the 'canpower' condition into a dedicated function in order
to avoid repetition and to make changing the condition easier.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 drivers/mmc/host/mmc_spi.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index fd877f6..1db18ce 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -152,6 +152,10 @@ struct mmc_spi_host {
 	dma_addr_t		ones_dma;
 };
 
+static inline int mmc_spi_canpower(struct mmc_spi_host *host)
+{
+	return host->pdata && host->pdata->setpower;
+}
 
 /****************************************************************************/
 
@@ -1187,19 +1191,16 @@ static void mmc_spi_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	struct mmc_spi_host *host = mmc_priv(mmc);
 
 	if (host->power_mode != ios->power_mode) {
-		int		canpower;
-
-		canpower = host->pdata && host->pdata->setpower;
 
 		dev_dbg(&host->spi->dev, "mmc_spi: power %s (%d)%s\n",
 				mmc_powerstring(ios->power_mode),
 				ios->vdd,
-				canpower ? ", can switch" : "");
+				mmc_spi_canpower(host) ? ", can switch" : "");
 
 		/* switch power on/off if possible, accounting for
 		 * max 250msec powerup time if needed.
 		 */
-		if (canpower) {
+		if (mmc_spi_canpower(host)) {
 			switch (ios->power_mode) {
 			case MMC_POWER_OFF:
 			case MMC_POWER_UP:
@@ -1223,7 +1224,8 @@ static void mmc_spi_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		 *   - MOSI low comes from writing zero
 		 *   - Chipselect is usually active low...
 		 */
-		if (canpower && ios->power_mode == MMC_POWER_OFF) {
+		if (mmc_spi_canpower(host) &&
+		    ios->power_mode == MMC_POWER_OFF) {
 			int mres;
 			u8 nullbyte = 0;
 
@@ -1399,7 +1401,7 @@ static int mmc_spi_probe(struct spi_device *spi)
 		dev_warn(&spi->dev, "ASSUMING 3.2-3.4 V slot power\n");
 		mmc->ocr_avail = MMC_VDD_32_33|MMC_VDD_33_34;
 	}
-	if (host->pdata && host->pdata->setpower) {
+	if (mmc_spi_canpower(host)) {
 		host->powerup_msecs = host->pdata->powerup_msecs;
 		if (!host->powerup_msecs || host->powerup_msecs > 250)
 			host->powerup_msecs = 250;
@@ -1459,7 +1461,7 @@ static int mmc_spi_probe(struct spi_device *spi)
 			host->dma_dev ? "" : ", no DMA",
 			(host->pdata && host->pdata->get_ro)
 				? "" : ", no WP",
-			(host->pdata && host->pdata->setpower)
+			mmc_spi_canpower(host)
 				? "" : ", no poweroff",
 			(mmc->caps & MMC_CAP_NEEDS_POLL)
 				? ", cd polling" : "");
-- 
1.7.4.1

[PATCH 2/4] mmc_spi.c: factor out the SD card shutdown sequence

[Antonio Ospite]

Factor out the SD card shutdown sequence to a dedicated
mmc_spi_shutdownsequence() function in order to make mmc_spi_set_ios()
more readable, and also for symmetry with mmc_spi_initsequence() which
is already a dedicated function.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 drivers/mmc/host/mmc_spi.c |   90 +++++++++++++++++++++++--------------------
 1 files changed, 48 insertions(+), 42 deletions(-)

diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 1db18ce..fe0fdc4 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -1176,6 +1176,51 @@ static void mmc_spi_initsequence(struct mmc_spi_host *host)
 	}
 }
 
+/* See Section 6.4.2, in SD "Simplified Physical Layer Specification 2.0"
+ *
+ * If powering down, ground all card inputs to avoid power delivery from data
+ * lines!  On a shared SPI bus, this will probably be temporary; 6.4.2 of
+ * the simplified SD spec says this must last at least 1msec.
+ *
+ *   - Clock low means CPOL 0, e.g. mode 0
+ *   - MOSI low comes from writing zero
+ *   - Chipselect is usually active low...
+ */
+static void mmc_spi_shutdownsequence(struct mmc_spi_host *host)
+{
+	int mres;
+	u8 nullbyte = 0;
+
+	host->spi->mode &= ~(SPI_CPOL|SPI_CPHA);
+	mres = spi_setup(host->spi);
+	if (mres < 0)
+		dev_dbg(&host->spi->dev,
+			"switch to SPI mode 0 failed\n");
+
+	if (spi_write(host->spi, &nullbyte, 1) < 0)
+		dev_dbg(&host->spi->dev,
+			"put spi signals to low failed\n");
+
+	/*
+	 * Now clock should be low due to spi mode 0;
+	 * MOSI should be low because of written 0x00;
+	 * chipselect should be low (it is active low)
+	 * power supply is off, so now MMC is off too!
+	 *
+	 * FIXME no, chipselect can be high since the
+	 * device is inactive and SPI_CS_HIGH is clear...
+	 */
+	msleep(10);
+	if (mres == 0) {
+		host->spi->mode |= (SPI_CPOL|SPI_CPHA);
+		mres = spi_setup(host->spi);
+		if (mres < 0)
+			dev_dbg(&host->spi->dev,
+				"switch back to SPI mode 3"
+				" failed\n");
+	}
+}
+
 static char *mmc_powerstring(u8 power_mode)
 {
 	switch (power_mode) {
@@ -1215,49 +1260,10 @@ static void mmc_spi_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		if (ios->power_mode == MMC_POWER_ON)
 			mmc_spi_initsequence(host);
 
-		/* If powering down, ground all card inputs to avoid power
-		 * delivery from data lines!  On a shared SPI bus, this
-		 * will probably be temporary; 6.4.2 of the simplified SD
-		 * spec says this must last at least 1msec.
-		 *
-		 *   - Clock low means CPOL 0, e.g. mode 0
-		 *   - MOSI low comes from writing zero
-		 *   - Chipselect is usually active low...
-		 */
+		/* See 6.4.2 in the simplified SD card physical spec 2.0 */
 		if (mmc_spi_canpower(host) &&
-		    ios->power_mode == MMC_POWER_OFF) {
-			int mres;
-			u8 nullbyte = 0;
-
-			host->spi->mode &= ~(SPI_CPOL|SPI_CPHA);
-			mres = spi_setup(host->spi);
-			if (mres < 0)
-				dev_dbg(&host->spi->dev,
-					"switch to SPI mode 0 failed\n");
-
-			if (spi_write(host->spi, &nullbyte, 1) < 0)
-				dev_dbg(&host->spi->dev,
-					"put spi signals to low failed\n");
-
-			/*
-			 * Now clock should be low due to spi mode 0;
-			 * MOSI should be low because of written 0x00;
-			 * chipselect should be low (it is active low)
-			 * power supply is off, so now MMC is off too!
-			 *
-			 * FIXME no, chipselect can be high since the
-			 * device is inactive and SPI_CS_HIGH is clear...
-			 */
-			msleep(10);
-			if (mres == 0) {
-				host->spi->mode |= (SPI_CPOL|SPI_CPHA);
-				mres = spi_setup(host->spi);
-				if (mres < 0)
-					dev_dbg(&host->spi->dev,
-						"switch back to SPI mode 3"
-						" failed\n");
-			}
-		}
+		    ios->power_mode == MMC_POWER_OFF)
+			mmc_spi_shutdownsequence(host);
 
 		host->power_mode = ios->power_mode;
 	}
-- 
1.7.4.1

[PATCH 3/4] mmc_spi.c: factor out a mmc_spi_setpower() function

[Antonio Ospite]

Factor out a mmc_spi_setpower() function so to make changing it more
elegant without adding too much stuff to mmc_spi_set_ios().

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 drivers/mmc/host/mmc_spi.c |   43 +++++++++++++++++++++++++++++++------------
 1 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index fe0fdc4..5e4b2c7 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -1221,6 +1221,26 @@ static void mmc_spi_shutdownsequence(struct mmc_spi_host *host)
 	}
 }
 
+static inline int mmc_spi_setpower(struct mmc_spi_host *host,
+					unsigned char power_mode,
+					unsigned int vdd)
+{
+	/* switch power on/off if possible, accounting for
+	 * max 250msec powerup time if needed.
+	 */
+	if (mmc_spi_canpower(host)) {
+		switch (power_mode) {
+		case MMC_POWER_OFF:
+		case MMC_POWER_UP:
+			host->pdata->setpower(&host->spi->dev, vdd);
+			if (power_mode == MMC_POWER_UP)
+				msleep(host->powerup_msecs);
+		}
+	}
+
+	return 0;
+}
+
 static char *mmc_powerstring(u8 power_mode)
 {
 	switch (power_mode) {
@@ -1236,24 +1256,23 @@ static void mmc_spi_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	struct mmc_spi_host *host = mmc_priv(mmc);
 
 	if (host->power_mode != ios->power_mode) {
+		int ret;
 
 		dev_dbg(&host->spi->dev, "mmc_spi: power %s (%d)%s\n",
 				mmc_powerstring(ios->power_mode),
 				ios->vdd,
 				mmc_spi_canpower(host) ? ", can switch" : "");
 
-		/* switch power on/off if possible, accounting for
-		 * max 250msec powerup time if needed.
-		 */
-		if (mmc_spi_canpower(host)) {
-			switch (ios->power_mode) {
-			case MMC_POWER_OFF:
-			case MMC_POWER_UP:
-				host->pdata->setpower(&host->spi->dev,
-						ios->vdd);
-				if (ios->power_mode == MMC_POWER_UP)
-					msleep(host->powerup_msecs);
-			}
+		ret = mmc_spi_setpower(host, ios->power_mode, ios->vdd);
+		if (ret) {
+			dev_err(mmc_dev(mmc), "unable to set power\n");
+			/*
+			 * The .set_ios() function in the mmc_host_ops
+			 * struct return void, and failing to set the
+			 * power should be rare so we print an error and
+			 * return here.
+			 */
+			return;
 		}
 
 		/* See 6.4.1 in the simplified SD card physical spec 2.0 */
-- 
1.7.4.1

[PATCH 4/4] mmc_spi.c: add support for the regulator framework

[Antonio Ospite]

Add support for powering up SD cards driven by regulators.
This makes the mmc_spi driver work also with the Motorola A910 phone.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 drivers/mmc/host/mmc_spi.c |   61 +++++++++++++++++++++++++++++++++++--------
 1 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 5e4b2c7..d5fe841 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -31,6 +31,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/crc7.h>
 #include <linux/crc-itu-t.h>
+#include <linux/regulator/consumer.h>
 #include <linux/scatterlist.h>
 
 #include <linux/mmc/host.h>
@@ -150,11 +151,13 @@ struct mmc_spi_host {
 	 */
 	void			*ones;
 	dma_addr_t		ones_dma;
+
+	struct regulator	*vcc;
 };
 
 static inline int mmc_spi_canpower(struct mmc_spi_host *host)
 {
-	return host->pdata && host->pdata->setpower;
+	return (host->pdata && host->pdata->setpower) || host->vcc;
 }
 
 /****************************************************************************/
@@ -1225,17 +1228,35 @@ static inline int mmc_spi_setpower(struct mmc_spi_host *host,
 					unsigned char power_mode,
 					unsigned int vdd)
 {
+	if (!mmc_spi_canpower(host))
+		return -ENOSYS;
+
 	/* switch power on/off if possible, accounting for
 	 * max 250msec powerup time if needed.
 	 */
-	if (mmc_spi_canpower(host)) {
-		switch (power_mode) {
-		case MMC_POWER_OFF:
-		case MMC_POWER_UP:
+	switch (power_mode) {
+	case MMC_POWER_OFF:
+		if (host->vcc) {
+			int ret = mmc_regulator_set_ocr(host->mmc,
+							host->vcc, 0);
+			if (ret)
+				return ret;
+		} else {
+			host->pdata->setpower(&host->spi->dev, vdd);
+		}
+		break;
+
+	case MMC_POWER_UP:
+		if (host->vcc) {
+			int 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);
 		}
+		msleep(host->powerup_msecs);
+		break;
 	}
 
 	return 0;
@@ -1420,12 +1441,28 @@ static int mmc_spi_probe(struct spi_device *spi)
 	 * and power switching gpios.
 	 */
 	host->pdata = mmc_spi_get_pdata(spi);
-	if (host->pdata)
-		mmc->ocr_avail = host->pdata->ocr_mask;
-	if (!mmc->ocr_avail) {
-		dev_warn(&spi->dev, "ASSUMING 3.2-3.4 V slot power\n");
-		mmc->ocr_avail = MMC_VDD_32_33|MMC_VDD_33_34;
+#ifdef CONFIG_REGULATOR
+	host->vcc = regulator_get(mmc_dev(host->mmc), "vmmc");
+
+	if (IS_ERR(host->vcc)) {
+		host->vcc = NULL;
+	} else {
+		host->mmc->ocr_avail = mmc_regulator_get_ocrmask(host->vcc);
+		if (host->pdata && host->pdata->ocr_mask)
+			dev_warn(mmc_dev(host->mmc),
+				"ocr_mask/setpower will not be used\n");
 	}
+#endif
+	if (host->vcc == NULL) {
+		/* fall-back to platform data */
+		if (host->pdata)
+			mmc->ocr_avail = host->pdata->ocr_mask;
+		if (!mmc->ocr_avail) {
+			dev_warn(&spi->dev, "ASSUMING 3.2-3.4 V slot power\n");
+			mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+		}
+	}
+
 	if (mmc_spi_canpower(host)) {
 		host->powerup_msecs = host->pdata->powerup_msecs;
 		if (!host->powerup_msecs || host->powerup_msecs > 250)
-- 
1.7.4.1

Re: [PATCH 0/4] mmc_spi: Add support for regulator framework

[Antonio Ospite]

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

On Mon, 21 Mar 2011 19:46:38 +0100
Antonio Ospite <ospite@studenti.unina.it> wrote:

> Hi,
> 
> this patchset has the purpose of adding support for the regulator framework to 
> the mmc_spi driver. The first three patches are preparatory cleanups to make 
> the fourth one more straightforward.
> 
> Maybe the fourth patch can be improved, I am open to any suggestions about it.
>

Ping. I forgot to Cc spi-devel-general on this series, should I resend
it?

> These changes take strong inspiration from the pxamci driver; they have been 
> tested on a Motorola A910, which uses a regulator to powerup the MMC card 
> connected to the SPI bus, a test from a current user of the mmc_spi driver 
> would not hurt just to be sure no regressions have been introduced.
> 
> Thanks,
>    Antonio
> 
> Antonio Ospite (4):
>   mmc_spi.c: factor out the check for power capability
>   mmc_spi.c: factor out the SD card shutdown sequence
>   mmc_spi.c: factor out a mmc_spi_setpower() function
>   mmc_spi.c: add support for the regulator framework
> 
>  drivers/mmc/host/mmc_spi.c |  194 +++++++++++++++++++++++++++++---------------
>  1 files changed, 129 insertions(+), 65 deletions(-)
> 

-- 
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 --]

Re: [PATCH 0/4] mmc_spi: Add support for regulator framework

[Grant Likely]

On Mon, Apr 04, 2011 at 11:56:31AM +0200, Antonio Ospite wrote:
> On Mon, 21 Mar 2011 19:46:38 +0100
> Antonio Ospite <ospite@studenti.unina.it> wrote:
> 
> > Hi,
> > 
> > this patchset has the purpose of adding support for the regulator framework to 
> > the mmc_spi driver. The first three patches are preparatory cleanups to make 
> > the fourth one more straightforward.
> > 
> > Maybe the fourth patch can be improved, I am open to any suggestions about it.
> >
> 
> Ping. I forgot to Cc spi-devel-general on this series, should I resend
> it?

Not a bad idea.  It doesn't go via my tree since it is an mmc patch,
not an spi one, but I don't mind taking a look at the spi bits.

g.

> 
> > These changes take strong inspiration from the pxamci driver; they have been 
> > tested on a Motorola A910, which uses a regulator to powerup the MMC card 
> > connected to the SPI bus, a test from a current user of the mmc_spi driver 
> > would not hurt just to be sure no regressions have been introduced.
> > 
> > Thanks,
> >    Antonio
> > 
> > Antonio Ospite (4):
> >   mmc_spi.c: factor out the check for power capability
> >   mmc_spi.c: factor out the SD card shutdown sequence
> >   mmc_spi.c: factor out a mmc_spi_setpower() function
> >   mmc_spi.c: add support for the regulator framework
> > 
> >  drivers/mmc/host/mmc_spi.c |  194 +++++++++++++++++++++++++++++---------------
> >  1 files changed, 129 insertions(+), 65 deletions(-)
> > 
> 
> -- 
> 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?

Re: [PATCH 0/4] mmc_spi: Add support for regulator framework

[Antonio Ospite]

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

On Mon, 4 Apr 2011 21:05:43 -0600
Grant Likely <grant.likely@secretlab.ca> wrote:

> On Mon, Apr 04, 2011 at 11:56:31AM +0200, Antonio Ospite wrote:
> > On Mon, 21 Mar 2011 19:46:38 +0100
> > Antonio Ospite <ospite@studenti.unina.it> wrote:
> > 
> > > Hi,
> > > 
> > > this patchset has the purpose of adding support for the regulator framework to 
> > > the mmc_spi driver. The first three patches are preparatory cleanups to make 
> > > the fourth one more straightforward.
> > > 
> > > Maybe the fourth patch can be improved, I am open to any suggestions about it.
> > >
> > 
> > Ping. I forgot to Cc spi-devel-general on this series, should I resend
> > it?
> 
> Not a bad idea.  It doesn't go via my tree since it is an mmc patch,
> not an spi one, but I don't mind taking a look at the spi bits.
> 

Grant you were on Cc from the start so you should have the patches
somewhere; please tell me if you don't.
I'd avoid resending if not strictly necessary.

Regards,
   Antonio

-- 
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 --]

Re: [PATCH 0/4] mmc_spi: Add support for regulator framework

[Grant Likely]

On Tue, Apr 05, 2011 at 10:43:47AM +0200, Antonio Ospite wrote:
> On Mon, 4 Apr 2011 21:05:43 -0600
> Grant Likely <grant.likely@secretlab.ca> wrote:
> 
> > On Mon, Apr 04, 2011 at 11:56:31AM +0200, Antonio Ospite wrote:
> > > On Mon, 21 Mar 2011 19:46:38 +0100
> > > Antonio Ospite <ospite@studenti.unina.it> wrote:
> > > 
> > > > Hi,
> > > > 
> > > > this patchset has the purpose of adding support for the regulator framework to 
> > > > the mmc_spi driver. The first three patches are preparatory cleanups to make 
> > > > the fourth one more straightforward.
> > > > 
> > > > Maybe the fourth patch can be improved, I am open to any suggestions about it.
> > > >
> > > 
> > > Ping. I forgot to Cc spi-devel-general on this series, should I resend
> > > it?
> > 
> > Not a bad idea.  It doesn't go via my tree since it is an mmc patch,
> > not an spi one, but I don't mind taking a look at the spi bits.
> > 
> 
> Grant you were on Cc from the start so you should have the patches
> somewhere; please tell me if you don't.
> I'd avoid resending if not strictly necessary.

Ah, then I probably scanned it briefly and decided I didn't need to
respond to it.  Don't worry about reposting.

g.