2011-03-21 18:47:43

by Antonio Ospite

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

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?


2011-03-21 18:47:22

by Antonio Ospite

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

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 <[email protected]>
---
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

2011-03-21 18:47:17

by Antonio Ospite

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

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 <[email protected]>
---
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

2011-03-21 18:47:37

by Antonio Ospite

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

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 <[email protected]>
---
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

2011-03-21 18:47:41

by Antonio Ospite

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

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 <[email protected]>
---
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

2011-04-04 09:56:47

by Antonio Ospite

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

On Mon, 21 Mar 2011 19:46:38 +0100
Antonio Ospite <[email protected]> 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?


Attachments:
(No filename) (1.37 kB)
(No filename) (198.00 B)
Download all attachments

2011-04-05 03:06:01

by Grant Likely

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

On Mon, Apr 04, 2011 at 11:56:31AM +0200, Antonio Ospite wrote:
> On Mon, 21 Mar 2011 19:46:38 +0100
> Antonio Ospite <[email protected]> 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?

2011-04-05 08:44:06

by Antonio Ospite

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

On Mon, 4 Apr 2011 21:05:43 -0600
Grant Likely <[email protected]> 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 <[email protected]> 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?


Attachments:
(No filename) (1.18 kB)
(No filename) (198.00 B)
Download all attachments

2011-04-05 13:46:41

by Grant Likely

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

On Tue, Apr 05, 2011 at 10:43:47AM +0200, Antonio Ospite wrote:
> On Mon, 4 Apr 2011 21:05:43 -0600
> Grant Likely <[email protected]> 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 <[email protected]> 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.