[PATCH 00/29] mmc: mmcif and tmio regulator and OF support

[PATCH 00/29] mmc: mmcif and tmio regulator and OF support

[Guennadi Liakhovetski]

Hi

I was not directly sure, what was the better way to send these out - as 
one seriess or as several. I would actually prefer several, but there are 
many dependencies. So, maybe best would be to try to review and address 
any issues on the MLs, only posting new versions of affected patches, and 
eventually provide a branch to pull from.

Now to contents:

1. prerequisites: goes on top of

[PATCH 4/6] mmc: sdhi: don't use connection IDs to obtain a clock reference
[PATCH 5/6] mmc: mmcif: don't use connection IDs to obtain a clock reference

http://thread.gmane.org/gmane.linux.ports.sh.devel/14425

[PATCH 0/3] mmc: sh-mmcif: clock management updates         
[PATCH 1/3] mmc: sh_mmcif: simplify and use meaningful label names in error-handling
[PATCH 2/3] mmc: sh_mmcif: fix clock management             
[PATCH 3/3] mmc: sh_mmcif: re-read the clock frequency upon re-enabling

http://thread.gmane.org/gmane.linux.ports.sh.devel/14448

(the following two patches should already be in the MMC tree)

[PATCH] mmc: cd-gpio.c: Include header to pickup exported symbol prototypes

http://www.spinics.net/lists/linux-mmc/msg13823.html

[PATCH] mmc: cd-gpio: allow NULL context in mmc_cd_gpio_free()

http://thread.gmane.org/gmane.linux.kernel.mmc/14103

2. supersede: this series supersedes my earlier patches:

[PATCH/RFC 0/7] mmc: tmio: regulator support, clock management, convert ecovec
[PATCH 1/7] mmc: tmio: stop interface clock before runtime PM suspending
[PATCH 2/7] mmc: tmio: don't needlessly enable interrupts during probing
[PATCH 3/7] mmc: tmio: add callbacks to enable-update and disable the interface clock
[PATCH 4/7] mmc: sdhi: implement tmio-mmc clock enable-update and disable callbacks
[PATCH 5/7] mmc: tmio: add regulator support
[PATCH 6/7] mmc: sdhi: do not install dummy callbacks
[PATCH 7/7] sh: ecovec: switch MMC power control to regulators

http://thread.gmane.org/gmane.linux.ports.sh.devel/14539

[PATCH 0/5] mmc: sh_mmcif: add regulator support
[PATCH 1/5] mmc: sh_mmcif: remove redundant .down_pwr() callback
[PATCH 2/5] sh: ecovec: remove unused .down_pwr() MMCIF callback
[PATCH 3/5] mmc: sh_mmcif: remove unused .down_pwr() callback
[PATCH 4/5] mmc: sh_mmcif: add regulator support
[PATCH/RFC 5/5] sh: ecovec: switch MMC power control to regulator

http://thread.gmane.org/gmane.linux.ports.sh.devel/14469

3. regulators: this series includes a bug-fix:

[PATCH 04/31] regulator: fix devm_regulator_put() to call regulator_put() explicitly

a cosmetic improvement:

[PATCH 05/31] regulator: use IS_ERR_OR_NULL() instead of open-coding

and a feature addition to the regulators framework.

4. mmc: this is where most of the patches fall into. This series includes 
additions and enhancements to the cd-gpio helper, which from now on is 
going to be called "slot-gpio;" a new file drivers/mmc/core/of.c with only 
two functions, so, if preferred, it can be merged into core.c; and 
numerous features, enhancements and improvements to the MMCIF and 
SDHI/TMIO drivers.

The of.c addition is actually pretty important, because it defines an 
external interface: it defines mmc properties, that all mmc host drivers 
will now be able to use, instead of adding new proprietary ones. Drivers, 
that already have OF support cannot stop supporting their interfaces. 
Since we do not want to rewrite .dts filesand break binary compatibility 
with existing platforms, these properties have to be carefully designed.

mmc regulator support: thanks to Mark Brown for comments. This version 
adds a generic mmc function to obtain a "vmmc" regulator for a specific 
mmc interface and now uses the device-managed version of regulator_get().

5. sh patches: 

[PATCH 02/29] sh: ecovec: remove unused .down_pwr() MMCIF callback

If we get an ack for this one, we can pull it together with other 
mmc-related patches via the mmc tree. If not, the following patch

[PATCH 03/29] mmc: sh_mmcif: remove unused .down_pwr() callback

will have to wait until [PATCH 02/29] makes it in.

One more

[PATCH 29/29] sh: ecovec: switch MMC power control to regulators

is optional and mostly is offered as an example of how regulators 
can be used with sh_mmcif and sh_mobile_sdhi.

Thanks
Guennadi

Guennadi Liakhovetski (29):
  mmc: sh_mmcif: remove redundant .down_pwr() callback
  sh: ecovec: remove unused .down_pwr() MMCIF callback
  mmc: sh_mmcif: remove unused .down_pwr() callback
  regulator: fix devm_regulator_put() to call regulator_put()
    explicitly
  regulator: use IS_ERR_OR_NULL() instead of open-coding
  regulator: export a function to check if regulator can change status
  mmc: add a function to get a regulator, supplying card's Vdd
  mmc: sh_mmcif: add regulator support
  mmc: tmio: stop interface clock before runtime PM suspending
  mmc: tmio: don't needlessly enable interrupts during probing
  mmc: tmio: add callbacks to enable-update and disable the interface
    clock
  mmc: sdhi: implement tmio-mmc clock enable-update and disable
    callbacks
  mmc: tmio: add regulator support
  mmc: sdhi: do not install dummy callbacks
  mmc: extend and rename cd-gpio helpers to handle more slot GPIO
    functions
  mmc: tmio: use MMC opcode defines instead of numbers
  mmc: use a more generic name for slot function types and fields
  mmc: tmio: remove a duplicated comment line
  mmc: add two capability flags for CD and WP signal polarity
  mmc: add CD GPIO polling support to slot functions
  mmc: convert slot functions to managed allocation
  mmc: add WP pin handler to slot functions
  mmc: tmio: use generic GPIO CD and WP handlers
  mmc: add a simple generic OF parser
  mmc: tmio: add OF support
  mmc: sdhi: add OF support, make platform data optional
  mmc: mmcif: add OF support
  mmc: mmcif: add support for generic GPIO CD polling
  sh: ecovec: switch MMC power control to regulators

 arch/sh/boards/mach-ecovec24/setup.c |   91 ++++++++++++------
 drivers/mmc/core/Makefile            |    4 +-
 drivers/mmc/core/cd-gpio.c           |   83 ----------------
 drivers/mmc/core/core.c              |   27 +++++
 drivers/mmc/core/host.c              |    4 +
 drivers/mmc/core/of.c                |   91 ++++++++++++++++++
 drivers/mmc/core/slot-gpio.c         |  175 ++++++++++++++++++++++++++++++++++
 drivers/mmc/host/sh_mmcif.c          |   83 ++++++++++++++---
 drivers/mmc/host/sh_mobile_sdhi.c    |   56 ++++++++---
 drivers/mmc/host/tmio_mmc.h          |    2 +
 drivers/mmc/host/tmio_mmc_pio.c      |  159 +++++++++++++++++++++++++------
 drivers/regulator/core.c             |   20 ++++-
 include/linux/mfd/tmio.h             |    3 +
 include/linux/mmc/cd-gpio.h          |   18 ----
 include/linux/mmc/host.h             |   39 +++++++-
 include/linux/mmc/sh_mmcif.h         |    1 -
 include/linux/mmc/slot-gpio.h        |   24 +++++
 include/linux/regulator/consumer.h   |    7 ++
 18 files changed, 694 insertions(+), 193 deletions(-)
 delete mode 100644 drivers/mmc/core/cd-gpio.c
 create mode 100644 drivers/mmc/core/of.c
 create mode 100644 drivers/mmc/core/slot-gpio.c
 delete mode 100644 include/linux/mmc/cd-gpio.h
 create mode 100644 include/linux/mmc/slot-gpio.h

-- 
1.7.2.5

[PATCH 01/29] mmc: sh_mmcif: remove redundant .down_pwr() callback

[Guennadi Liakhovetski]

From the original version of sh_mmcif the .set_pwr() callback has only been
used to turn the card's power on, and the .down_pwr() callback has been
used to turn it off. .set_pwr() can be used for both these tasks, which is
also how it is implemented by the only user of this API: the SH7724 ecovec
board.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index ed59392..19c7576 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -957,8 +957,8 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			pm_runtime_put(&host->pd->dev);
 			clk_disable(host->hclk);
 			host->power = false;
-			if (p->down_pwr && ios->power_mode = MMC_POWER_OFF)
-				p->down_pwr(host->pd);
+			if (p->set_pwr && ios->power_mode = MMC_POWER_OFF)
+				p->set_pwr(host->pd, 0);
 		}
 		host->state = STATE_IDLE;
 		return;
-- 
1.7.2.5

[PATCH 02/29] sh: ecovec: remove unused .down_pwr() MMCIF callback

[Guennadi Liakhovetski]

.down_pwr() on ecovec was equivalent to .set_pwr(0). Now that .down_pwr()
is no longer used by the driver, it can be removed.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Paul Mundt <lethal@linux-sh.org>
---
 arch/sh/boards/mach-ecovec24/setup.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
index d12fe9d..a2f70e6 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -877,11 +877,6 @@ static void mmcif_set_pwr(struct platform_device *pdev, int state)
 	gpio_set_value(GPIO_PTB7, state);
 }
 
-static void mmcif_down_pwr(struct platform_device *pdev)
-{
-	gpio_set_value(GPIO_PTB7, 0);
-}
-
 static struct resource sh_mmcif_resources[] = {
 	[0] = {
 		.name	= "SH_MMCIF",
@@ -903,7 +898,6 @@ static struct resource sh_mmcif_resources[] = {
 
 static struct sh_mmcif_plat_data sh_mmcif_plat = {
 	.set_pwr	= mmcif_set_pwr,
-	.down_pwr	= mmcif_down_pwr,
 	.sup_pclk	= 0, /* SH7724: Max Pclk/2 */
 	.caps		= MMC_CAP_4_BIT_DATA |
 			  MMC_CAP_8_BIT_DATA |
-- 
1.7.2.5

[PATCH 03/29] mmc: sh_mmcif: remove unused .down_pwr() callback

[Guennadi Liakhovetski]

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

Please, hold this patch back, until

[PATCH 02/29] sh: ecovec: remove unused .down_pwr() MMCIF callback

is committed.

 include/linux/mmc/sh_mmcif.h |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/include/linux/mmc/sh_mmcif.h b/include/linux/mmc/sh_mmcif.h
index 05f0e3d..cb84340 100644
--- a/include/linux/mmc/sh_mmcif.h
+++ b/include/linux/mmc/sh_mmcif.h
@@ -39,7 +39,6 @@ struct sh_mmcif_dma {
 
 struct sh_mmcif_plat_data {
 	void (*set_pwr)(struct platform_device *pdev, int state);
-	void (*down_pwr)(struct platform_device *pdev);
 	int (*get_cd)(struct platform_device *pdef);
 	struct sh_mmcif_dma	*dma;		/* Deprecated. Instead */
 	unsigned int		slave_id_tx;	/* use embedded slave_id_[tr]x */
-- 
1.7.2.5

[PATCH 04/29] regulator: fix devm_regulator_put() to call regulator_put() explicitly

[Guennadi Liakhovetski]

devres_destroy() doesn't call the release() method, it only destroys the
resource. The caller should take care to release the associated object
itself.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/regulator/core.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e70dd38..fdeabba 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1429,6 +1429,7 @@ void devm_regulator_put(struct regulator *regulator)
 {
 	int rc;
 
+	regulator_put(regulator);
 	rc = devres_destroy(regulator->dev, devm_regulator_release,
 			    devm_regulator_match, regulator);
 	WARN_ON(rc);
-- 
1.7.2.5

Re: [PATCH 04/29] regulator: fix devm_regulator_put() to call regulator_put() explicitly

[Mark Brown]

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

On Thu, May 03, 2012 at 05:05:33PM +0200, Guennadi Liakhovetski wrote:
> devres_destroy() doesn't call the release() method, it only destroys the
> resource. The caller should take care to release the associated object
> itself.

> +	regulator_put(regulator);
>  	rc = devres_destroy(regulator->dev, devm_regulator_release,
>  			    devm_regulator_match, regulator);

Oh dear.  This seems like pretty peculiar behaviour on the part of
devres_destroy() - everything about the function and its documentation
would suggest that it'd actually free the resource not just free the
devm_ internal bits of the resource, and especially given that we have
to pass the release function in.

This also seems like it's the wrong way round - it probably shouldn't
matter that much but we should really only do the regulator_put() after
the destroy succeeded since there's a small chance that hotplug or
something might cause this to run while the device is being removed
elsewhere so we could end up trying to double free.

Also, this patch is totally unrelated to the rest of the series and
should really have been sent separately.

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

Re: [PATCH 04/29] regulator: fix devm_regulator_put() to call regulator_put() explicitly

[Guennadi Liakhovetski]

Hi Mark

On Thu, 3 May 2012, Mark Brown wrote:

> On Thu, May 03, 2012 at 05:05:33PM +0200, Guennadi Liakhovetski wrote:
> > devres_destroy() doesn't call the release() method, it only destroys the
> > resource. The caller should take care to release the associated object
> > itself.
> 
> > +	regulator_put(regulator);
> >  	rc = devres_destroy(regulator->dev, devm_regulator_release,
> >  			    devm_regulator_match, regulator);

So, shall we rather use your new devres_release() here?

Thanks
Guennadi

> Oh dear.  This seems like pretty peculiar behaviour on the part of
> devres_destroy() - everything about the function and its documentation
> would suggest that it'd actually free the resource not just free the
> devm_ internal bits of the resource, and especially given that we have
> to pass the release function in.
> 
> This also seems like it's the wrong way round - it probably shouldn't
> matter that much but we should really only do the regulator_put() after
> the destroy succeeded since there's a small chance that hotplug or
> something might cause this to run while the device is being removed
> elsewhere so we could end up trying to double free.
> 
> Also, this patch is totally unrelated to the rest of the series and
> should really have been sent separately.

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 04/29] regulator: fix devm_regulator_put() to call regulator_put() explicitly

[Mark Brown]

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

On Fri, May 18, 2012 at 10:54:26AM +0200, Guennadi Liakhovetski wrote:

> > > +	regulator_put(regulator);
> > >  	rc = devres_destroy(regulator->dev, devm_regulator_release,
> > >  			    devm_regulator_match, regulator);

> So, shall we rather use your new devres_release() here?

Yes, I already have a patch queued for 3.6 after things propagate
through in the merge window and have a fix based on the current API in
the regulator tree for 3.5 that doesn't have the double free issue.

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

[PATCH 05/29] regulator: use IS_ERR_OR_NULL() instead of open-coding

[Guennadi Liakhovetski]

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/regulator/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index fdeabba..a57defb 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1381,7 +1381,7 @@ void regulator_put(struct regulator *regulator)
 {
 	struct regulator_dev *rdev;
 
-	if (regulator = NULL || IS_ERR(regulator))
+	if (IS_ERR_OR_NULL(regulator))
 		return;
 
 	mutex_lock(&regulator_list_mutex);
-- 
1.7.2.5

Re: [PATCH 05/29] regulator: use IS_ERR_OR_NULL() instead of open-coding

[Mark Brown]

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

On Thu, May 03, 2012 at 05:05:34PM +0200, Guennadi Liakhovetski wrote:

> -	if (regulator == NULL || IS_ERR(regulator))
> +	if (IS_ERR_OR_NULL(regulator))

The bigger question here is why we're accepting NULL in the first place.

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

Re: [PATCH 05/29] regulator: use IS_ERR_OR_NULL() instead of open-coding

[Guennadi Liakhovetski]

On Thu, 3 May 2012, Mark Brown wrote:

> On Thu, May 03, 2012 at 05:05:34PM +0200, Guennadi Liakhovetski wrote:
> 
> > -	if (regulator = NULL || IS_ERR(regulator))
> > +	if (IS_ERR_OR_NULL(regulator))
> 
> The bigger question here is why we're accepting NULL in the first place.

No idea, you seem to be right - regulator_get() never returns NULL, so, it 
shouldn't be needed here. We could either remove this check altogether or 
change it to a BUG_OG().

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 05/29] regulator: use IS_ERR_OR_NULL() instead of open-coding

[Paul Mundt]

On Thu, May 03, 2012 at 04:32:39PM +0100, Mark Brown wrote:
> On Thu, May 03, 2012 at 05:05:34PM +0200, Guennadi Liakhovetski wrote:
> 
> > -	if (regulator = NULL || IS_ERR(regulator))
> > +	if (IS_ERR_OR_NULL(regulator))
> 
> The bigger question here is why we're accepting NULL in the first place.

I'm not sure about the regulator case, but it's been useful to support
passing NULL around in the clock framework case. There are plenty of
cases where a struct clk is optional and if we fail to find the clock we
just set clk to NULL and continue on without having to constantly check
the value of the clk pointer, which helps considerably when you consider
the number of clk_enable/disable() pairs some drivers have.

Presumably the same applies for regulators?

Re: [PATCH 05/29] regulator: use IS_ERR_OR_NULL() instead of open-coding

[Guennadi Liakhovetski]

On Fri, 18 May 2012, Paul Mundt wrote:

> On Thu, May 03, 2012 at 04:32:39PM +0100, Mark Brown wrote:
> > On Thu, May 03, 2012 at 05:05:34PM +0200, Guennadi Liakhovetski wrote:
> > 
> > > -	if (regulator = NULL || IS_ERR(regulator))
> > > +	if (IS_ERR_OR_NULL(regulator))
> > 
> > The bigger question here is why we're accepting NULL in the first place.
> 
> I'm not sure about the regulator case, but it's been useful to support
> passing NULL around in the clock framework case. There are plenty of
> cases where a struct clk is optional and if we fail to find the clock we
> just set clk to NULL and continue on without having to constantly check
> the value of the clk pointer, which helps considerably when you consider
> the number of clk_enable/disable() pairs some drivers have.
> 
> Presumably the same applies for regulators?

Right, but you normally do something like

	priv->regulator = regulator_get();
	...
	regulator_put(priv->regulator);

Since regulator_get() only returns either a valid pointer or an ERR_PTR() 
value, priv->regulator won't be NULL. Doing something like

	priv->regulator = regulator_get();
	if (IS_ERR(priv->regulator))
		priv->regulator = NULL;

would seem too weird to me.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 05/29] regulator: use IS_ERR_OR_NULL() instead of open-coding

[Paul Mundt]

On Fri, May 18, 2012 at 11:18:21AM +0200, Guennadi Liakhovetski wrote:
> On Fri, 18 May 2012, Paul Mundt wrote:
> 
> > On Thu, May 03, 2012 at 04:32:39PM +0100, Mark Brown wrote:
> > > On Thu, May 03, 2012 at 05:05:34PM +0200, Guennadi Liakhovetski wrote:
> > > 
> > > > -	if (regulator = NULL || IS_ERR(regulator))
> > > > +	if (IS_ERR_OR_NULL(regulator))
> > > 
> > > The bigger question here is why we're accepting NULL in the first place.
> > 
> > I'm not sure about the regulator case, but it's been useful to support
> > passing NULL around in the clock framework case. There are plenty of
> > cases where a struct clk is optional and if we fail to find the clock we
> > just set clk to NULL and continue on without having to constantly check
> > the value of the clk pointer, which helps considerably when you consider
> > the number of clk_enable/disable() pairs some drivers have.
> > 
> > Presumably the same applies for regulators?
> 
> Right, but you normally do something like
> 
> 	priv->regulator = regulator_get();
> 	...
> 	regulator_put(priv->regulator);
> 
> Since regulator_get() only returns either a valid pointer or an ERR_PTR() 
> value, priv->regulator won't be NULL. Doing something like
> 
> 	priv->regulator = regulator_get();
> 	if (IS_ERR(priv->regulator))
> 		priv->regulator = NULL;
> 
> would seem too weird to me.
> 
Perhaps, but that's precisely what we do for struct clk in the case where
it not existing is non-fatal.

Re: [PATCH 05/29] regulator: use IS_ERR_OR_NULL() instead of open-coding

[Mark Brown]

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

On Fri, May 18, 2012 at 05:57:50PM +0900, Paul Mundt wrote:
> On Thu, May 03, 2012 at 04:32:39PM +0100, Mark Brown wrote:
> > On Thu, May 03, 2012 at 05:05:34PM +0200, Guennadi Liakhovetski wrote:

> > > -	if (regulator == NULL || IS_ERR(regulator))
> > > +	if (IS_ERR_OR_NULL(regulator))

> > The bigger question here is why we're accepting NULL in the first place.

> I'm not sure about the regulator case, but it's been useful to support
> passing NULL around in the clock framework case. There are plenty of
> cases where a struct clk is optional and if we fail to find the clock we
> just set clk to NULL and continue on without having to constantly check
> the value of the clk pointer, which helps considerably when you consider
> the number of clk_enable/disable() pairs some drivers have.

> Presumably the same applies for regulators?

This is checking the value of regulator_get(), NULL is a perfectly valid
regulator value to get passed back.

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

Re: [PATCH 05/29] regulator: use IS_ERR_OR_NULL() instead of open-coding

[Guennadi Liakhovetski]

Hi Mark

On Fri, 18 May 2012, Mark Brown wrote:

> On Fri, May 18, 2012 at 05:57:50PM +0900, Paul Mundt wrote:
> > On Thu, May 03, 2012 at 04:32:39PM +0100, Mark Brown wrote:
> > > On Thu, May 03, 2012 at 05:05:34PM +0200, Guennadi Liakhovetski wrote:
> 
> > > > -	if (regulator = NULL || IS_ERR(regulator))
> > > > +	if (IS_ERR_OR_NULL(regulator))
> 
> > > The bigger question here is why we're accepting NULL in the first place.
> 
> > I'm not sure about the regulator case, but it's been useful to support
> > passing NULL around in the clock framework case. There are plenty of
> > cases where a struct clk is optional and if we fail to find the clock we
> > just set clk to NULL and continue on without having to constantly check
> > the value of the clk pointer, which helps considerably when you consider
> > the number of clk_enable/disable() pairs some drivers have.
> 
> > Presumably the same applies for regulators?
> 
> This is checking the value of regulator_get(), NULL is a perfectly valid
> regulator value to get passed back.

Sorry, could you clarify, how it is valid? No, I'm not proposing to revive 
this patch, just curious, what exactly you meant by this. AFAICS NULL is 
never returned from regulator_get(). The value, that's returned by it is 
then directly dereferenced in other regulator API calls, for which you 
don't normally want a NULL. So, having a regulator pointer = NULL seems 
to be as valid to me as causing a BUG() is? :-)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 05/29] regulator: use IS_ERR_OR_NULL() instead of open-coding

[Mark Brown]

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

On Mon, Jun 11, 2012 at 01:38:13PM +0200, Guennadi Liakhovetski wrote:
> On Fri, 18 May 2012, Mark Brown wrote:

> > This is checking the value of regulator_get(), NULL is a perfectly valid
> > regulator value to get passed back.

> Sorry, could you clarify, how it is valid? No, I'm not proposing to revive 
> this patch, just curious, what exactly you meant by this. AFAICS NULL is 
> never returned from regulator_get(). The value, that's returned by it is 
> then directly dereferenced in other regulator API calls, for which you 
> don't normally want a NULL. So, having a regulator pointer == NULL seems 
> to be as valid to me as causing a BUG() is? :-)

The stubs return NULL and we could decide in future to use NULL for
something else (though that seems unlikely).

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

Re: [PATCH 05/29] regulator: use IS_ERR_OR_NULL() instead of open-coding

[Guennadi Liakhovetski]

On Mon, 11 Jun 2012, Mark Brown wrote:

> On Mon, Jun 11, 2012 at 01:38:13PM +0200, Guennadi Liakhovetski wrote:
> > On Fri, 18 May 2012, Mark Brown wrote:
> 
> > > This is checking the value of regulator_get(), NULL is a perfectly valid
> > > regulator value to get passed back.
> 
> > Sorry, could you clarify, how it is valid? No, I'm not proposing to revive 
> > this patch, just curious, what exactly you meant by this. AFAICS NULL is 
> > never returned from regulator_get(). The value, that's returned by it is 
> > then directly dereferenced in other regulator API calls, for which you 
> > don't normally want a NULL. So, having a regulator pointer = NULL seems 
> > to be as valid to me as causing a BUG() is? :-)
> 
> The stubs return NULL and we could decide in future to use NULL for
> something else (though that seems unlikely).

Ah, sure, forgot about that, sorry. In fact, I was wondering, whether it's 
indeed a good idea to return success from functions like 
regulator_set_voltage() or regulator_get_voltage() when regulator is even 
not configured? Isn't it confusing for drivers to get a success from 
set_voltage(voltage > 0) and then get 0V back from get_voltage()?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 05/29] regulator: use IS_ERR_OR_NULL() instead of open-coding

[Mark Brown]

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

On Mon, Jun 11, 2012 at 02:06:29PM +0200, Guennadi Liakhovetski wrote:
> On Mon, 11 Jun 2012, Mark Brown wrote:

> > The stubs return NULL and we could decide in future to use NULL for
> > something else (though that seems unlikely).

> Ah, sure, forgot about that, sorry. In fact, I was wondering, whether it's 
> indeed a good idea to return success from functions like 
> regulator_set_voltage() or regulator_get_voltage() when regulator is even 
> not configured? Isn't it confusing for drivers to get a success from 

Anything that really cares about set_voltage() should have a hard
dependency on the API, it's mainly done so that cpufreq type drivers
can work easily (since they don't *really* care about the voltages
normally, generally if nothing happens in the regulator they're just
less effective).

> set_voltage(voltage > 0) and then get 0V back from get_voltage()?

There's not a use case I can think of for this, though.

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

[PATCH 06/29] regulator: export a function to check if regulator can change status

[Guennadi Liakhovetski]

Some consumers, e.g., the mmc subsystem, can benefit from an ability to
check, whether a regulator can switch power on and off.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/regulator/core.c           |   17 +++++++++++++++++
 include/linux/regulator/consumer.h |    7 +++++++
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index a57defb..ec4cc0b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1447,6 +1447,23 @@ static int _regulator_can_change_status(struct regulator_dev *rdev)
 		return 0;
 }
 
+bool regulator_can_change_status(struct regulator *regulator)
+{
+	struct regulator_dev *rdev;
+	int ret;
+
+	if (IS_ERR_OR_NULL(regulator))
+		return false;
+
+	rdev = regulator->rdev;
+	mutex_lock(&rdev->mutex);
+	ret = _regulator_can_change_status(rdev);
+	mutex_unlock(&rdev->mutex);
+
+	return ret && !rdev->constraints->always_on;
+}
+EXPORT_SYMBOL_GPL(regulator_can_change_status);
+
 /* locks held by regulator_enable() */
 static int _regulator_enable(struct regulator_dev *rdev)
 {
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 4ed1b30..a15a476 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -177,6 +177,8 @@ int regulator_set_mode(struct regulator *regulator, unsigned int mode);
 unsigned int regulator_get_mode(struct regulator *regulator);
 int regulator_set_optimum_mode(struct regulator *regulator, int load_uA);
 
+bool regulator_can_change_status(struct regulator *regulator);
+
 /* regulator notifier block */
 int regulator_register_notifier(struct regulator *regulator,
 			      struct notifier_block *nb);
@@ -344,6 +346,11 @@ static inline void regulator_set_drvdata(struct regulator *regulator,
 {
 }
 
+static inline bool regulator_can_change_status(struct regulator *regulator)
+{
+	return false;
+}
+
 #endif
 
 #endif
-- 
1.7.2.5

Re: [PATCH 06/29] regulator: export a function to check if regulator can change status

[Mark Brown]

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

On Thu, May 03, 2012 at 05:05:35PM +0200, Guennadi Liakhovetski wrote:
> Some consumers, e.g., the mmc subsystem, can benefit from an ability to
> check, whether a regulator can switch power on and off.

You're going to have to expand a lot more on why this is a good idea...
Bear in mind that the regulator may be shared by other users so even if
the regulator is flagged as potentially being able to change status that
doesn't mean that it actually can change status.

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

Re: [PATCH 06/29] regulator: export a function to check if regulator can change status

[Guennadi Liakhovetski]

Hi Mark

On Thu, 3 May 2012, Mark Brown wrote:

> On Thu, May 03, 2012 at 05:05:35PM +0200, Guennadi Liakhovetski wrote:
> > Some consumers, e.g., the mmc subsystem, can benefit from an ability to
> > check, whether a regulator can switch power on and off.
> 
> You're going to have to expand a lot more on why this is a good idea...

I want to use this call in mmc to decide whether or not to set the 
MMC_CAP_POWER_OFF_CARD mmc capability flag. This flag is used with SDIO 
cards to decide whether or not to enable runtime PM for them.

> Bear in mind that the regulator may be shared by other users so even if
> the regulator is flagged as potentially being able to change status that
> doesn't mean that it actually can change status.

Yes, I think, that's ok. We just need to know, that the card can in 
principle be powered off. We don't need a guarantee, that we can power it 
down at any time, when we think we need it.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 06/29] regulator: export a function to check if regulator can change status

[Mark Brown]

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

On Fri, May 18, 2012 at 10:33:42AM +0200, Guennadi Liakhovetski wrote:
> On Thu, 3 May 2012, Mark Brown wrote:

> > Bear in mind that the regulator may be shared by other users so even if
> > the regulator is flagged as potentially being able to change status that
> > doesn't mean that it actually can change status.

> Yes, I think, that's ok. We just need to know, that the card can in 
> principle be powered off. We don't need a guarantee, that we can power it 
> down at any time, when we think we need it.

Well, in that case what's the benefit in not just unconditionally
claiming that you can power off and doing runtime PM on the card?

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

Re: [PATCH 06/29] regulator: export a function to check if regulator can change status

[Guennadi Liakhovetski]

On Fri, 18 May 2012, Mark Brown wrote:

> On Fri, May 18, 2012 at 10:33:42AM +0200, Guennadi Liakhovetski wrote:
> > On Thu, 3 May 2012, Mark Brown wrote:
> 
> > > Bear in mind that the regulator may be shared by other users so even if
> > > the regulator is flagged as potentially being able to change status that
> > > doesn't mean that it actually can change status.
> 
> > Yes, I think, that's ok. We just need to know, that the card can in 
> > principle be powered off. We don't need a guarantee, that we can power it 
> > down at any time, when we think we need it.
> 
> Well, in that case what's the benefit in not just unconditionally
> claiming that you can power off and doing runtime PM on the card?

I'm using the ability of the regulator to change status as a signal from 
the user: "yes, this card may be powered off," or "no, keep it on." 
However, now that I think about it, it is not necessarily a good 
criterion. It can be, that the regulator can power off, but the card 
actually shouldn't be. Hm...

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 07/29] mmc: add a function to get a regulator, supplying card's Vdd

[Guennadi Liakhovetski]

Add a function to get a regulator, supplying card's Vdd on a specific host.
If such a regulator is found, the function checks, whether a valid OCR mask
can be obtained from this regulator, and whether it can switch card's power
on and off.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/mmc/core/core.c  |   27 +++++++++++++++++++++++++++
 include/linux/mmc/host.h |    6 ++++++
 2 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 7474c47..ccb5c8a 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1012,6 +1012,33 @@ int mmc_regulator_set_ocr(struct mmc_host *mmc,
 }
 EXPORT_SYMBOL(mmc_regulator_set_ocr);
 
+struct regulator *mmc_regulator_get_vmmc(struct mmc_host *mmc)
+{
+	struct device *dev = mmc_dev(mmc);
+	struct regulator *supply = devm_regulator_get(dev, "vmmc");
+	int ret;
+
+	if (IS_ERR(supply))
+		return NULL;
+
+	ret = mmc_regulator_get_ocrmask(supply);
+	if (ret > 0)
+		mmc->ocr_avail = ret;
+
+	if (regulator_can_change_status(supply)) {
+		mmc->caps |= MMC_CAP_POWER_OFF_CARD;
+		return supply;
+	}
+
+	devm_regulator_put(supply);
+
+	if (ret <= 0)
+		dev_warn(dev, "Ignoring useless (dummy?) regulator\n");
+
+	return NULL;
+}
+EXPORT_SYMBOL(mmc_regulator_get_vmmc);
+
 #endif /* CONFIG_REGULATOR */
 
 /*
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index cbde4b7..bcf5562 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -362,6 +362,7 @@ int mmc_regulator_get_ocrmask(struct regulator *supply);
 int mmc_regulator_set_ocr(struct mmc_host *mmc,
 			struct regulator *supply,
 			unsigned short vdd_bit);
+struct regulator *mmc_regulator_get_vmmc(struct mmc_host *mmc);
 #else
 static inline int mmc_regulator_get_ocrmask(struct regulator *supply)
 {
@@ -374,6 +375,11 @@ static inline int mmc_regulator_set_ocr(struct mmc_host *mmc,
 {
 	return 0;
 }
+
+static inline struct regulator *mmc_regulator_get_vmmc(struct mmc_host *mmc)
+{
+	return NULL;
+}
 #endif
 
 int mmc_card_awake(struct mmc_host *host);
-- 
1.7.2.5

Re: [PATCH 07/29] mmc: add a function to get a regulator, supplying card's Vdd

[Mark Brown]

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

On Thu, May 03, 2012 at 05:05:36PM +0200, Guennadi Liakhovetski wrote:

> +	ret = mmc_regulator_get_ocrmask(supply);
> +	if (ret > 0)
> +		mmc->ocr_avail = ret;
> +
> +	if (regulator_can_change_status(supply)) {
> +		mmc->caps |= MMC_CAP_POWER_OFF_CARD;
> +		return supply;
> +	}
> + 
> +	devm_regulator_put(supply);
> +
> +	if (ret <= 0)
> +		dev_warn(dev, "Ignoring useless (dummy?) regulator\n");

This code seems very tricky and a bit confusing - you're checking that
either you can change the voltage or change the status but you're doing
it in a fairly abstruse way and it'll come out as saying that a fixed
voltage regulator is a useful regulator to have which probably isn't what
you want if it's worth ignoring a dummy regulator.

You may also run into trouble on boards that use the ability to disable
unused regulators at the end of boot - they'll power things off even
without the ability to change status at runtime.

It'd at least be nice to write things more directly, or add some
comments.

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

Re: [PATCH 07/29] mmc: add a function to get a regulator, supplying card's Vdd

[Guennadi Liakhovetski]

On Thu, 3 May 2012, Mark Brown wrote:

> On Thu, May 03, 2012 at 05:05:36PM +0200, Guennadi Liakhovetski wrote:
> 
> > +	ret = mmc_regulator_get_ocrmask(supply);
> > +	if (ret > 0)
> > +		mmc->ocr_avail = ret;
> > +
> > +	if (regulator_can_change_status(supply)) {
> > +		mmc->caps |= MMC_CAP_POWER_OFF_CARD;
> > +		return supply;
> > +	}
> > + 
> > +	devm_regulator_put(supply);
> > +
> > +	if (ret <= 0)
> > +		dev_warn(dev, "Ignoring useless (dummy?) regulator\n");
> 
> This code seems very tricky and a bit confusing - you're checking that
> either you can change the voltage or change the status but you're doing
> it in a fairly abstruse way and it'll come out as saying that a fixed
> voltage regulator is a useful regulator to have which probably isn't what
> you want if it's worth ignoring a dummy regulator.

This function handles 3 cases:

1. A regulator that can (in principle) change state, i.e., switch on and 
off. Such a regulator is good to keep for the runtime to power up and down 
the card.
2. A regulator, that cannot switch, but at least can tell its supplied 
voltage is used ones to read out supported voltages and released again.
3. A regulator that can do none of the above is defined as "useless"

So, a fixed voltage regulator is indeed a useful one - it can tell us the 
voltage and maybe even switch power. A dummy regulator can do none of the 
above.

> You may also run into trouble on boards that use the ability to disable
> unused regulators at the end of boot - they'll power things off even
> without the ability to change status at runtime.

Aha, you mean, I shouldn't put() the regulator, even if it cannot change 
status itself? Can this also happen with a dummy regulator?

> It'd at least be nice to write things more directly, or add some
> comments.

Hm, sure, I can add some comments:-)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 07/29] mmc: add a function to get a regulator, supplying card's Vdd

[Mark Brown]

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

On Fri, May 18, 2012 at 11:28:31AM +0200, Guennadi Liakhovetski wrote:

> 1. A regulator that can (in principle) change state, i.e., switch on and 
> off. Such a regulator is good to keep for the runtime to power up and down 
> the card.

> 2. A regulator, that cannot switch, but at least can tell its supplied 
> voltage is used ones to read out supported voltages and released again.

This doesn't make sense, you can only change the voltages if you hold a
reference to the regulator.

> 3. A regulator that can do none of the above is defined as "useless"

> > You may also run into trouble on boards that use the ability to disable
> > unused regulators at the end of boot - they'll power things off even
> > without the ability to change status at runtime.

> Aha, you mean, I shouldn't put() the regulator, even if it cannot change 
> status itself?

Yes, you ought to to be safe and like I say if you want to manage the
voltage then you need to keep a reference to the regulator.

> Can this also happen with a dummy regulator?

Obviously not, though if you've got explicit code in to handle dummy
regulators you're rather missing the point.  Their entire purpose is to
provide a crutch to keep the system going if bits are missing from the
bindings, they're not intended to be used in production.

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

Re: [PATCH 07/29] mmc: add a function to get a regulator, supplying card's Vdd

[Guennadi Liakhovetski]

On Fri, 18 May 2012, Mark Brown wrote:

> On Fri, May 18, 2012 at 11:28:31AM +0200, Guennadi Liakhovetski wrote:
> 
> > 1. A regulator that can (in principle) change state, i.e., switch on and 
> > off. Such a regulator is good to keep for the runtime to power up and down 
> > the card.
> 
> > 2. A regulator, that cannot switch, but at least can tell its supplied 
> > voltage is used ones to read out supported voltages and released again.
> 
> This doesn't make sense, you can only change the voltages if you hold a
> reference to the regulator.

Yes, that's obviously a thinko. I forgot, that mmc also switches between 
different voltages, not only powers on and off.

> > 3. A regulator that can do none of the above is defined as "useless"
> 
> > > You may also run into trouble on boards that use the ability to disable
> > > unused regulators at the end of boot - they'll power things off even
> > > without the ability to change status at runtime.
> 
> > Aha, you mean, I shouldn't put() the regulator, even if it cannot change 
> > status itself?
> 
> Yes, you ought to to be safe and like I say if you want to manage the
> voltage then you need to keep a reference to the regulator.
> 
> > Can this also happen with a dummy regulator?
> 
> Obviously not,

Well, I certainly understand, that a dummy regulator cannot switch power 
by itself:-) Well, theoretically it can happen, that the board code / OF 
is missing the required entry and a shared regulator will be powered off, 
that also powers mmc, but that's obviously a bug.

> though if you've got explicit code in to handle dummy
> regulators you're rather missing the point.  Their entire purpose is to
> provide a crutch to keep the system going if bits are missing from the
> bindings, they're not intended to be used in production.

I was thinking about what to use to power off the card if both a platform 
callback (legacy) and a regulator are available (see, e.g., "[PATCH 
08/29] mmc: sh_mmcif: add regulator support"). But perhaps giving 
preference to the regulator is bogus. Maybe I should just switch both, or 
even give preference to the call-back. What do you think? Switching both 
is, perhaps, the safest bet.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 07/29] mmc: add a function to get a regulator, supplying card's Vdd

[Mark Brown]

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

On Fri, May 18, 2012 at 05:39:39PM +0200, Guennadi Liakhovetski wrote:

> 08/29] mmc: sh_mmcif: add regulator support"). But perhaps giving 
> preference to the regulator is bogus. Maybe I should just switch both, or 
> even give preference to the call-back. What do you think? Switching both 
> is, perhaps, the safest bet.

I think most things either give preference to the callback or complain
if you try to set both.

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

[PATCH 08/29] mmc: sh_mmcif: add regulator support

[Guennadi Liakhovetski]

Add regulator support to the sh_mmcif driver, but also preserve the current
power-callback.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/mmc/host/sh_mmcif.c |   48 ++++++++++++++++++++++++++++++++++++------
 1 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 19c7576..f6d0b11 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -58,6 +58,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_qos.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 #include <linux/spinlock.h>
 #include <linux/module.h>
 
@@ -231,6 +232,8 @@ struct sh_mmcif_host {
 	bool power;
 	bool card_present;
 
+	struct regulator	*vdd;
+
 	/* DMA support */
 	struct dma_chan		*chan_rx;
 	struct dma_chan		*chan_tx;
@@ -923,10 +926,24 @@ static void sh_mmcif_clk_update(struct sh_mmcif_host *host)
 	host->mmc->f_min = host->clk / 512;
 }
 
+static void sh_mmcif_set_power(struct sh_mmcif_host *host, struct mmc_ios *ios)
+{
+	struct sh_mmcif_plat_data *pd = host->pd->dev.platform_data;
+
+	if (!(host->mmc->caps & MMC_CAP_POWER_OFF_CARD))
+		return;
+
+	if (host->vdd)
+		/* Errors ignored... */
+		mmc_regulator_set_ocr(host->mmc, host->vdd,
+				      ios->power_mode ? ios->vdd : 0);
+	else if (pd->set_pwr)
+		pd->set_pwr(host->pd, ios->power_mode != MMC_POWER_OFF);
+}
+
 static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct sh_mmcif_host *host = mmc_priv(mmc);
-	struct sh_mmcif_plat_data *p = host->pd->dev.platform_data;
 	unsigned long flags;
 
 	spin_lock_irqsave(&host->lock, flags);
@@ -944,6 +961,7 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			sh_mmcif_request_dma(host, host->pd->dev.platform_data);
 			host->card_present = true;
 		}
+		sh_mmcif_set_power(host, ios);
 	} else if (ios->power_mode = MMC_POWER_OFF || !ios->clock) {
 		/* clock stop */
 		sh_mmcif_clock_control(host, 0);
@@ -957,8 +975,8 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			pm_runtime_put(&host->pd->dev);
 			clk_disable(host->hclk);
 			host->power = false;
-			if (p->set_pwr && ios->power_mode = MMC_POWER_OFF)
-				p->set_pwr(host->pd, 0);
+			if (ios->power_mode = MMC_POWER_OFF)
+				sh_mmcif_set_power(host, ios);
 		}
 		host->state = STATE_IDLE;
 		return;
@@ -966,8 +984,6 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 	if (ios->clock) {
 		if (!host->power) {
-			if (p->set_pwr)
-				p->set_pwr(host->pd, ios->power_mode);
 			clk_enable(host->hclk);
 			sh_mmcif_clk_update(host);
 			pm_runtime_get_sync(&host->pd->dev);
@@ -1252,6 +1268,24 @@ static void mmcif_timeout_work(struct work_struct *work)
 	mmc_request_done(host->mmc, mrq);
 }
 
+static void sh_mmcif_init_ocr(struct sh_mmcif_host *host)
+{
+	struct sh_mmcif_plat_data *pd = host->pd->dev.platform_data;
+	struct mmc_host *mmc = host->mmc;
+
+	host->vdd = mmc_regulator_get_vmmc(mmc);
+
+	if ((mmc->ocr_avail && pd->ocr) || (host->vdd && pd->set_pwr))
+		dev_warn(mmc_dev(mmc),
+			 "Platform OCR mask / .set_pwr() are ignored\n");
+
+	if (pd->set_pwr)
+		mmc->caps |= MMC_CAP_POWER_OFF_CARD;
+
+	if (!mmc->ocr_avail)
+		mmc->ocr_avail = pd->ocr;
+}
+
 static int __devinit sh_mmcif_probe(struct platform_device *pdev)
 {
 	int ret = 0, irq[2];
@@ -1298,8 +1332,8 @@ static int __devinit sh_mmcif_probe(struct platform_device *pdev)
 	spin_lock_init(&host->lock);
 
 	mmc->ops = &sh_mmcif_ops;
-	if (pd->ocr)
-		mmc->ocr_avail = pd->ocr;
+	sh_mmcif_init_ocr(host);
+
 	mmc->caps = MMC_CAP_MMC_HIGHSPEED;
 	if (pd->caps)
 		mmc->caps |= pd->caps;
-- 
1.7.2.5

Re: [PATCH 08/29] mmc: sh_mmcif: add regulator support

[Mark Brown]

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

On Thu, May 03, 2012 at 05:05:37PM +0200, Guennadi Liakhovetski wrote:

> +#include <linux/regulator/consumer.h>

Where's this used?

> +static void sh_mmcif_set_power(struct sh_mmcif_host *host, struct mmc_ios *ios)
> +{
> +	struct sh_mmcif_plat_data *pd = host->pd->dev.platform_data;
> +
> +	if (!(host->mmc->caps & MMC_CAP_POWER_OFF_CARD))
> +		return;

Surely the core should be implementing this?

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

Re: [PATCH 08/29] mmc: sh_mmcif: add regulator support

[Guennadi Liakhovetski]

On Thu, 3 May 2012, Mark Brown wrote:

> On Thu, May 03, 2012 at 05:05:37PM +0200, Guennadi Liakhovetski wrote:
> 
> > +#include <linux/regulator/consumer.h>
> 
> Where's this used?

You're right, it's not. A forward declaration of struct regulator would 
suffice.

> > +static void sh_mmcif_set_power(struct sh_mmcif_host *host, struct mmc_ios *ios)
> > +{
> > +	struct sh_mmcif_plat_data *pd = host->pd->dev.platform_data;
> > +
> > +	if (!(host->mmc->caps & MMC_CAP_POWER_OFF_CARD))
> > +		return;
> 
> Surely the core should be implementing this?

Hm, actually, I don't think so. sh_mmcif_set_power() is called as a part 
of the driver's .set_ios() method. The core has to call that method 
regardless whether MMC_CAP_POWER_OFF_CARD is set or not. Only inside that 
method we can decide, that a part of its functionality (power on / off) is 
unavailable.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 09/29] mmc: tmio: stop interface clock before runtime PM suspending

[Guennadi Liakhovetski]

The call to pm_runtime_put() in tmio-mmc's .set_ios() method can switch
off the supplying clock or even power down the whole unit. Therefore,
calling tmio_mmc_clk_stop() after that can be redundant, put it before.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/tmio_mmc_pio.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 9a7996a..0d2643b 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -809,11 +809,11 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	} else if (ios->power_mode != MMC_POWER_UP) {
 		if (host->set_pwr && ios->power_mode = MMC_POWER_OFF)
 			host->set_pwr(host->pdev, 0);
+		tmio_mmc_clk_stop(host);
 		if (host->power) {
 			host->power = false;
 			pm_runtime_put(dev);
 		}
-		tmio_mmc_clk_stop(host);
 	}
 
 	switch (ios->bus_width) {
-- 
1.7.2.5

[PATCH 10/29] mmc: tmio: don't needlessly enable interrupts during probing

[Guennadi Liakhovetski]

We don't have to force-enable MMC interrupts in our .probe() method,
mmc_add_host() will trigger card detection, that will enable all the
necessary interrupts.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/tmio_mmc_pio.c |   21 +++++++++++----------
 1 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 0d2643b..8cd3637 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -948,6 +948,17 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 
 	_host->sdcard_irq_mask = sd_ctrl_read32(_host, CTL_IRQ_MASK);
 	tmio_mmc_disable_mmc_irqs(_host, TMIO_MASK_ALL);
+
+	/* Unmask the IRQs we want to know about */
+	if (!_host->chan_rx)
+		irq_mask |= TMIO_MASK_READOP;
+	if (!_host->chan_tx)
+		irq_mask |= TMIO_MASK_WRITEOP;
+	if (!_host->native_hotplug)
+		irq_mask &= ~(TMIO_STAT_CARD_REMOVE | TMIO_STAT_CARD_INSERT);
+
+	_host->sdcard_irq_mask &= ~irq_mask;
+
 	if (pdata->flags & TMIO_MMC_SDIO_IRQ)
 		tmio_mmc_enable_sdio_irq(mmc, 0);
 
@@ -965,16 +976,6 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 
 	dev_pm_qos_expose_latency_limit(&pdev->dev, 100);
 
-	/* Unmask the IRQs we want to know about */
-	if (!_host->chan_rx)
-		irq_mask |= TMIO_MASK_READOP;
-	if (!_host->chan_tx)
-		irq_mask |= TMIO_MASK_WRITEOP;
-	if (!_host->native_hotplug)
-		irq_mask &= ~(TMIO_STAT_CARD_REMOVE | TMIO_STAT_CARD_INSERT);
-
-	tmio_mmc_enable_mmc_irqs(_host, irq_mask);
-
 	if (pdata->flags & TMIO_MMC_USE_GPIO_CD) {
 		ret = mmc_cd_gpio_request(mmc, pdata->cd_gpio);
 		if (ret < 0) {
-- 
1.7.2.5

[PATCH 11/29] mmc: tmio: add callbacks to enable-update and disable the interface clock

[Guennadi Liakhovetski]

Every time the clockc is enabled after possibly being disabled, we have
to re-read its frequency and update our configuration.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/tmio_mmc_pio.c |   35 ++++++++++++++++++++++++++++++++---
 include/linux/mfd/tmio.h        |    3 +++
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 8cd3637..579f990 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -751,6 +751,22 @@ fail:
 	mmc_request_done(mmc, mrq);
 }
 
+static int tmio_mmc_clk_update(struct mmc_host *mmc)
+{
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+	struct tmio_mmc_data *pdata = host->pdata;
+	int ret;
+
+	if (!pdata->clk_enable)
+		return -ENOTSUPP;
+
+	ret = pdata->clk_enable(host->pdev, &mmc->f_max);
+	if (!ret)
+		mmc->f_min = mmc->f_max / 512;
+
+	return ret;
+}
+
 /* Set MMC clock / power.
  * Note: This controller uses a simple divider scheme therefore it cannot
  * run a MMC card at full speed (20MHz). The max clock is 24MHz on SD, but as
@@ -797,6 +813,7 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	 */
 	if (ios->power_mode = MMC_POWER_ON && ios->clock) {
 		if (!host->power) {
+			tmio_mmc_clk_update(mmc);
 			pm_runtime_get_sync(dev);
 			host->power = true;
 		}
@@ -811,8 +828,11 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			host->set_pwr(host->pdev, 0);
 		tmio_mmc_clk_stop(host);
 		if (host->power) {
+			struct tmio_mmc_data *pdata = host->pdata;
 			host->power = false;
 			pm_runtime_put(dev);
+			if (pdata->clk_disable)
+				pdata->clk_disable(host->pdev);
 		}
 	}
 
@@ -904,8 +924,6 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 
 	mmc->ops = &tmio_mmc_ops;
 	mmc->caps = MMC_CAP_4_BIT_DATA | pdata->capabilities;
-	mmc->f_max = pdata->hclk;
-	mmc->f_min = mmc->f_max / 512;
 	mmc->max_segs = 32;
 	mmc->max_blk_size = 512;
 	mmc->max_blk_count = (PAGE_CACHE_SIZE / mmc->max_blk_size) *
@@ -927,6 +945,11 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	if (ret < 0)
 		goto pm_disable;
 
+	if (tmio_mmc_clk_update(mmc) < 0) {
+		mmc->f_max = pdata->hclk;
+		mmc->f_min = mmc->f_max / 512;
+	}
+
 	/*
 	 * There are 4 different scenarios for the card detection:
 	 *  1) an external gpio irq handles the cd (best for power savings)
@@ -972,7 +995,13 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	/* See if we also get DMA */
 	tmio_mmc_request_dma(_host, pdata);
 
-	mmc_add_host(mmc);
+	ret = mmc_add_host(mmc);
+	if (pdata->clk_disable)
+		pdata->clk_disable(pdev);
+	if (ret < 0) {
+		tmio_mmc_host_remove(_host);
+		return ret;
+	}
 
 	dev_pm_qos_expose_latency_limit(&pdev->dev, 100);
 
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index f5171db..b332c4c 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -110,6 +110,9 @@ struct tmio_mmc_data {
 	void (*set_clk_div)(struct platform_device *host, int state);
 	int (*get_cd)(struct platform_device *host);
 	int (*write16_hook)(struct tmio_mmc_host *host, int addr);
+	/* clock management callbacks */
+	int (*clk_enable)(struct platform_device *pdev, unsigned int *f);
+	void (*clk_disable)(struct platform_device *pdev);
 };
 
 /*
-- 
1.7.2.5

[PATCH 12/29] mmc: sdhi: implement tmio-mmc clock enable-update and disable callbacks

[Guennadi Liakhovetski]

Instead of delivering one static clock frequency value, read from the
hardware during probing, enable the tmio-mmc driver to re-read the
frequency dynamically.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mobile_sdhi.c |   24 +++++++++++++++++++++++-
 1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 5ac656e..a746ae8 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -39,6 +39,27 @@ struct sh_mobile_sdhi {
 	struct tmio_mmc_dma dma_priv;
 };
 
+static int sh_mobile_sdhi_clk_enable(struct platform_device *pdev, unsigned int *f)
+{
+	struct mmc_host *mmc = dev_get_drvdata(&pdev->dev);
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+	struct sh_mobile_sdhi *priv = container_of(host->pdata, struct sh_mobile_sdhi, mmc_data);
+	int ret = clk_enable(priv->clk);
+	if (ret < 0)
+		return ret;
+
+	*f = clk_get_rate(priv->clk);
+	return 0;
+}
+
+static void sh_mobile_sdhi_clk_disable(struct platform_device *pdev)
+{
+	struct mmc_host *mmc = dev_get_drvdata(&pdev->dev);
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+	struct sh_mobile_sdhi *priv = container_of(host->pdata, struct sh_mobile_sdhi, mmc_data);
+	clk_disable(priv->clk);
+}
+
 static void sh_mobile_sdhi_set_pwr(struct platform_device *pdev, int state)
 {
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
@@ -130,9 +151,10 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 		goto eclkget;
 	}
 
-	mmc_data->hclk = clk_get_rate(priv->clk);
 	mmc_data->set_pwr = sh_mobile_sdhi_set_pwr;
 	mmc_data->get_cd = sh_mobile_sdhi_get_cd;
+	mmc_data->clk_enable = sh_mobile_sdhi_clk_enable;
+	mmc_data->clk_disable = sh_mobile_sdhi_clk_disable;
 	mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
 	if (p) {
 		mmc_data->flags = p->tmio_flags;
-- 
1.7.2.5

[PATCH 13/29] mmc: tmio: add regulator support

[Guennadi Liakhovetski]

Currently the TMIO MMC driver derives the OCR mask from the platform data
and uses a platform callback to turn card power on and off. This patch adds
regulator support to the driver.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/mmc/host/tmio_mmc.h     |    2 +
 drivers/mmc/host/tmio_mmc_pio.c |   44 +++++++++++++++++++++++++++++++-------
 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index d857f5c..12c0108 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -55,6 +55,8 @@ struct tmio_mmc_host {
 	void (*set_pwr)(struct platform_device *host, int state);
 	void (*set_clk_div)(struct platform_device *host, int state);
 
+	struct regulator	*vdd;
+
 	/* pio related stuff */
 	struct scatterlist      *sg_ptr;
 	struct scatterlist      *sg_orig;
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 579f990..175cc71 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -42,6 +42,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_qos.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 #include <linux/scatterlist.h>
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
@@ -767,6 +768,19 @@ static int tmio_mmc_clk_update(struct mmc_host *mmc)
 	return ret;
 }
 
+static void tmio_mmc_set_power(struct tmio_mmc_host *host, struct mmc_ios *ios)
+{
+	if (!(host->mmc->caps & MMC_CAP_POWER_OFF_CARD))
+		return;
+
+	if (host->vdd)
+		/* Errors ignored... */
+		mmc_regulator_set_ocr(host->mmc, host->vdd,
+				      ios->power_mode ? ios->vdd : 0);
+	else if (host->set_pwr)
+		host->set_pwr(host->pdev, ios->power_mode != MMC_POWER_OFF);
+}
+
 /* Set MMC clock / power.
  * Note: This controller uses a simple divider scheme therefore it cannot
  * run a MMC card at full speed (20MHz). The max clock is 24MHz on SD, but as
@@ -819,13 +833,12 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		}
 		tmio_mmc_set_clock(host, ios->clock);
 		/* power up SD bus */
-		if (host->set_pwr)
-			host->set_pwr(host->pdev, 1);
+		tmio_mmc_set_power(host, ios);
 		/* start bus clock */
 		tmio_mmc_clk_start(host);
 	} else if (ios->power_mode != MMC_POWER_UP) {
-		if (host->set_pwr && ios->power_mode = MMC_POWER_OFF)
-			host->set_pwr(host->pdev, 0);
+		if (ios->power_mode = MMC_POWER_OFF)
+			tmio_mmc_set_power(host, ios);
 		tmio_mmc_clk_stop(host);
 		if (host->power) {
 			struct tmio_mmc_data *pdata = host->pdata;
@@ -885,6 +898,24 @@ static const struct mmc_host_ops tmio_mmc_ops = {
 	.enable_sdio_irq = tmio_mmc_enable_sdio_irq,
 };
 
+static void tmio_mmc_init_ocr(struct tmio_mmc_host *host)
+{
+	struct tmio_mmc_data *pdata = host->pdata;
+	struct mmc_host *mmc = host->mmc;
+
+	host->vdd = mmc_regulator_get_vmmc(mmc);
+
+	if ((mmc->ocr_avail && pdata->ocr_mask) || (host->vdd && host->set_pwr))
+		dev_warn(mmc_dev(mmc),
+			 "Platform OCR mask / .set_pwr() are ignored\n");
+
+	if (host->set_pwr)
+		mmc->caps |= MMC_CAP_POWER_OFF_CARD;
+
+	if (!mmc->ocr_avail)
+		mmc->ocr_avail = pdata->ocr_mask ? : MMC_VDD_32_33 | MMC_VDD_33_34;
+}
+
 int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 				  struct platform_device *pdev,
 				  struct tmio_mmc_data *pdata)
@@ -930,10 +961,7 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 		mmc->max_segs;
 	mmc->max_req_size = mmc->max_blk_size * mmc->max_blk_count;
 	mmc->max_seg_size = mmc->max_req_size;
-	if (pdata->ocr_mask)
-		mmc->ocr_avail = pdata->ocr_mask;
-	else
-		mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+	tmio_mmc_init_ocr(_host);
 
 	_host->native_hotplug = !(pdata->flags & TMIO_MMC_USE_GPIO_CD ||
 				  mmc->caps & MMC_CAP_NEEDS_POLL ||
-- 
1.7.2.5

Re: [PATCH 13/29] mmc: tmio: add regulator support

[Mark Brown]

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

On Thu, May 03, 2012 at 05:05:42PM +0200, Guennadi Liakhovetski wrote:

> +static void tmio_mmc_set_power(struct tmio_mmc_host *host, struct mmc_ios *ios)
> +{
> +	if (!(host->mmc->caps & MMC_CAP_POWER_OFF_CARD))
> +		return;
> +
> +	if (host->vdd)
> +		/* Errors ignored... */
> +		mmc_regulator_set_ocr(host->mmc, host->vdd,
> +				      ios->power_mode ? ios->vdd : 0);
> +	else if (host->set_pwr)
> +		host->set_pwr(host->pdev, ios->power_mode != MMC_POWER_OFF);
> +}

This looks pretty much identical to the previous code, should it be
being factored out into the core?  Or put the regulator bit in the core
and provide an option to override with manual callbacks if that's
required.

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

Re: [PATCH 13/29] mmc: tmio: add regulator support

[Guennadi Liakhovetski]

On Thu, 3 May 2012, Mark Brown wrote:

> On Thu, May 03, 2012 at 05:05:42PM +0200, Guennadi Liakhovetski wrote:
> 
> > +static void tmio_mmc_set_power(struct tmio_mmc_host *host, struct mmc_ios *ios)
> > +{
> > +	if (!(host->mmc->caps & MMC_CAP_POWER_OFF_CARD))
> > +		return;
> > +
> > +	if (host->vdd)
> > +		/* Errors ignored... */
> > +		mmc_regulator_set_ocr(host->mmc, host->vdd,
> > +				      ios->power_mode ? ios->vdd : 0);
> > +	else if (host->set_pwr)
> > +		host->set_pwr(host->pdev, ios->power_mode != MMC_POWER_OFF);
> > +}
> 
> This looks pretty much identical to the previous code,

Maybe it's because it's been written and is maintained by the same group 
of people? :-)

> should it be being factored out into the core?

Weeeell, it's not a lot of code. We could make this a library function, 
provided by the core, but I don't think it would be very straight forward 
to also automate its calling, thus freeing individual drivers from this 
task. As you can see in these two drivers, one of them turns the card on 
already upon MMC_POWER_UP, the other one only in MMC_POWER_ON. I'm not 
sure off the top of my head why this difference is there, but in any case 
different drivers might have different ideas about this. So, I'm not sure 
whether this micro-optimisation is worth it.

> Or put the regulator bit in the core
> and provide an option to override with manual callbacks if that's
> required.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 13/29] mmc: tmio: add regulator support

[Mark Brown]

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

On Fri, May 18, 2012 at 11:55:53AM +0200, Guennadi Liakhovetski wrote:
> On Thu, 3 May 2012, Mark Brown wrote:

> > should it be being factored out into the core?

> Weeeell, it's not a lot of code. We could make this a library function, 
> provided by the core, but I don't think it would be very straight forward 
> to also automate its calling, thus freeing individual drivers from this 
> task. As you can see in these two drivers, one of them turns the card on 
> already upon MMC_POWER_UP, the other one only in MMC_POWER_ON. I'm not 
> sure off the top of my head why this difference is there, but in any case 
> different drivers might have different ideas about this. So, I'm not sure 
> whether this micro-optimisation is worth it.

My take on this would be that we shouldn't even have to think about what
the differences are and that if there are genuine tradeoffs here it's
likely that we should be offering them at runtime or to boards or
something rather than picking something in an individual driver.  If
there are meaningful differences I'd at least expect to be able to tell
what they are.

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

[PATCH 14/29] mmc: sdhi: do not install dummy callbacks

[Guennadi Liakhovetski]

Currently the SDHI glue for the TMIO MMC driver installs dummy .get_cd() and
.set_pwr() callbacks even if the platform didn't supply them. This is not
necessary, since the TMIO MMC driver itself checks for NULL callbacks. This
is also dubious if the platform provides a regulator for SD-card power
switching. It is better to only install those callbacks, if they are really
provided by the platform.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mobile_sdhi.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index a746ae8..ae2e3da 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -64,18 +64,14 @@ static void sh_mobile_sdhi_set_pwr(struct platform_device *pdev, int state)
 {
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
 
-	if (p && p->set_pwr)
-		p->set_pwr(pdev, state);
+	p->set_pwr(pdev, state);
 }
 
 static int sh_mobile_sdhi_get_cd(struct platform_device *pdev)
 {
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
 
-	if (p && p->get_cd)
-		return p->get_cd(pdev);
-	else
-		return -ENOSYS;
+	return p->get_cd(pdev);
 }
 
 static int sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
@@ -151,8 +147,10 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 		goto eclkget;
 	}
 
-	mmc_data->set_pwr = sh_mobile_sdhi_set_pwr;
-	mmc_data->get_cd = sh_mobile_sdhi_get_cd;
+	if (p->set_pwr)
+		mmc_data->set_pwr = sh_mobile_sdhi_set_pwr;
+	if (p->get_cd)
+		mmc_data->get_cd = sh_mobile_sdhi_get_cd;
 	mmc_data->clk_enable = sh_mobile_sdhi_clk_enable;
 	mmc_data->clk_disable = sh_mobile_sdhi_clk_disable;
 	mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
-- 
1.7.2.5

[PATCH 15/29] mmc: extend and rename cd-gpio helpers to handle more slot GPIO functions

[Guennadi Liakhovetski]

GPIOs can be used in MMC/SD-card slots not only for hotplug detection, but
also to implement the write-protection pin. Rename cd-gpio helpers to
slot-gpio to make addition of further slot GPIO functions possible.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/core/Makefile       |    2 +-
 drivers/mmc/core/cd-gpio.c      |   83 ---------------------------------------
 drivers/mmc/core/slot-gpio.c    |   83 +++++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/tmio_mmc_pio.c |    6 +-
 include/linux/mmc/cd-gpio.h     |   18 --------
 include/linux/mmc/slot-gpio.h   |   18 ++++++++
 6 files changed, 105 insertions(+), 105 deletions(-)
 delete mode 100644 drivers/mmc/core/cd-gpio.c
 create mode 100644 drivers/mmc/core/slot-gpio.c
 delete mode 100644 include/linux/mmc/cd-gpio.h
 create mode 100644 include/linux/mmc/slot-gpio.h

diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index dca4428..38ed210 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -7,6 +7,6 @@ mmc_core-y			:= core.o bus.o host.o \
 				   mmc.o mmc_ops.o sd.o sd_ops.o \
 				   sdio.o sdio_ops.o sdio_bus.o \
 				   sdio_cis.o sdio_io.o sdio_irq.o \
-				   quirks.o cd-gpio.o
+				   quirks.o slot-gpio.o
 
 mmc_core-$(CONFIG_DEBUG_FS)	+= debugfs.o
diff --git a/drivers/mmc/core/cd-gpio.c b/drivers/mmc/core/cd-gpio.c
deleted file mode 100644
index f13e38d..0000000
--- a/drivers/mmc/core/cd-gpio.c
+++ /dev/null
@@ -1,83 +0,0 @@
-/*
- * Generic GPIO card-detect helper
- *
- * Copyright (C) 2011, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#include <linux/err.h>
-#include <linux/gpio.h>
-#include <linux/interrupt.h>
-#include <linux/jiffies.h>
-#include <linux/mmc/cd-gpio.h>
-#include <linux/mmc/host.h>
-#include <linux/module.h>
-#include <linux/slab.h>
-
-struct mmc_cd_gpio {
-	unsigned int gpio;
-	char label[0];
-};
-
-static irqreturn_t mmc_cd_gpio_irqt(int irq, void *dev_id)
-{
-	/* Schedule a card detection after a debounce timeout */
-	mmc_detect_change(dev_id, msecs_to_jiffies(100));
-	return IRQ_HANDLED;
-}
-
-int mmc_cd_gpio_request(struct mmc_host *host, unsigned int gpio)
-{
-	size_t len = strlen(dev_name(host->parent)) + 4;
-	struct mmc_cd_gpio *cd;
-	int irq = gpio_to_irq(gpio);
-	int ret;
-
-	if (irq < 0)
-		return irq;
-
-	cd = kmalloc(sizeof(*cd) + len, GFP_KERNEL);
-	if (!cd)
-		return -ENOMEM;
-
-	snprintf(cd->label, len, "%s cd", dev_name(host->parent));
-
-	ret = gpio_request_one(gpio, GPIOF_DIR_IN, cd->label);
-	if (ret < 0)
-		goto egpioreq;
-
-	ret = request_threaded_irq(irq, NULL, mmc_cd_gpio_irqt,
-				   IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
-				   cd->label, host);
-	if (ret < 0)
-		goto eirqreq;
-
-	cd->gpio = gpio;
-	host->hotplug.irq = irq;
-	host->hotplug.handler_priv = cd;
-
-	return 0;
-
-eirqreq:
-	gpio_free(gpio);
-egpioreq:
-	kfree(cd);
-	return ret;
-}
-EXPORT_SYMBOL(mmc_cd_gpio_request);
-
-void mmc_cd_gpio_free(struct mmc_host *host)
-{
-	struct mmc_cd_gpio *cd = host->hotplug.handler_priv;
-
-	if (!cd)
-		return;
-
-	free_irq(host->hotplug.irq, host);
-	gpio_free(cd->gpio);
-	kfree(cd);
-}
-EXPORT_SYMBOL(mmc_cd_gpio_free);
diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
new file mode 100644
index 0000000..17d705f
--- /dev/null
+++ b/drivers/mmc/core/slot-gpio.c
@@ -0,0 +1,83 @@
+/*
+ * Generic GPIO card-detect helper
+ *
+ * Copyright (C) 2011, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/slot-gpio.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+struct mmc_gpio {
+	unsigned int cd_gpio;
+	char cd_label[0];
+};
+
+static irqreturn_t mmc_gpio_cd_irqt(int irq, void *dev_id)
+{
+	/* Schedule a card detection after a debounce timeout */
+	mmc_detect_change(dev_id, msecs_to_jiffies(100));
+	return IRQ_HANDLED;
+}
+
+int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio)
+{
+	size_t len = strlen(dev_name(host->parent)) + 4;
+	struct mmc_gpio *ctx;
+	int irq = gpio_to_irq(gpio);
+	int ret;
+
+	if (irq < 0)
+		return irq;
+
+	ctx = kmalloc(sizeof(*ctx) + len, GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent));
+
+	ret = gpio_request_one(gpio, GPIOF_DIR_IN, ctx->cd_label);
+	if (ret < 0)
+		goto egpioreq;
+
+	ret = request_threaded_irq(irq, NULL, mmc_gpio_cd_irqt,
+				   IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				   ctx->cd_label, host);
+	if (ret < 0)
+		goto eirqreq;
+
+	ctx->cd_gpio = gpio;
+	host->hotplug.irq = irq;
+	host->hotplug.handler_priv = ctx;
+
+	return 0;
+
+eirqreq:
+	gpio_free(gpio);
+egpioreq:
+	kfree(ctx);
+	return ret;
+}
+EXPORT_SYMBOL(mmc_gpio_request_cd);
+
+void mmc_gpio_free_cd(struct mmc_host *host)
+{
+	struct mmc_gpio *ctx = host->hotplug.handler_priv;
+
+	if (!ctx)
+		return;
+
+	free_irq(host->hotplug.irq, host);
+	gpio_free(ctx->cd_gpio);
+	kfree(ctx);
+}
+EXPORT_SYMBOL(mmc_gpio_free_cd);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 175cc71..7da63b5 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -34,8 +34,8 @@
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/mfd/tmio.h>
-#include <linux/mmc/cd-gpio.h>
 #include <linux/mmc/host.h>
+#include <linux/mmc/slot-gpio.h>
 #include <linux/mmc/tmio.h>
 #include <linux/module.h>
 #include <linux/pagemap.h>
@@ -1034,7 +1034,7 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	dev_pm_qos_expose_latency_limit(&pdev->dev, 100);
 
 	if (pdata->flags & TMIO_MMC_USE_GPIO_CD) {
-		ret = mmc_cd_gpio_request(mmc, pdata->cd_gpio);
+		ret = mmc_gpio_request_cd(mmc, pdata->cd_gpio);
 		if (ret < 0) {
 			tmio_mmc_host_remove(_host);
 			return ret;
@@ -1066,7 +1066,7 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host)
 		 * This means we can miss a card-eject, but this is anyway
 		 * possible, because of delayed processing of hotplug events.
 		 */
-		mmc_cd_gpio_free(mmc);
+		mmc_gpio_free_cd(mmc);
 
 	if (!host->native_hotplug)
 		pm_runtime_get_sync(&pdev->dev);
diff --git a/include/linux/mmc/cd-gpio.h b/include/linux/mmc/cd-gpio.h
deleted file mode 100644
index cefaba0..0000000
--- a/include/linux/mmc/cd-gpio.h
+++ /dev/null
@@ -1,18 +0,0 @@
-/*
- * Generic GPIO card-detect helper header
- *
- * Copyright (C) 2011, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#ifndef MMC_CD_GPIO_H
-#define MMC_CD_GPIO_H
-
-struct mmc_host;
-int mmc_cd_gpio_request(struct mmc_host *host, unsigned int gpio);
-void mmc_cd_gpio_free(struct mmc_host *host);
-
-#endif
diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
new file mode 100644
index 0000000..edfaa32
--- /dev/null
+++ b/include/linux/mmc/slot-gpio.h
@@ -0,0 +1,18 @@
+/*
+ * Generic GPIO card-detect helper header
+ *
+ * Copyright (C) 2011, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef MMC_SLOT_GPIO_H
+#define MMC_SLOT_GPIO_H
+
+struct mmc_host;
+int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio);
+void mmc_gpio_free_cd(struct mmc_host *host);
+
+#endif
-- 
1.7.2.5

Re: [PATCH 15/29] mmc: extend and rename cd-gpio helpers to handle more slot GPIO functions

[S, Venkatraman]

On Thu, May 3, 2012 at 8:35 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> GPIOs can be used in MMC/SD-card slots not only for hotplug detection, but
> also to implement the write-protection pin. Rename cd-gpio helpers to
> slot-gpio to make addition of further slot GPIO functions possible.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  drivers/mmc/core/Makefile       |    2 +-
>  drivers/mmc/core/cd-gpio.c      |   83 ---------------------------------------
>  drivers/mmc/core/slot-gpio.c    |   83 +++++++++++++++++++++++++++++++++++++++
>  drivers/mmc/host/tmio_mmc_pio.c |    6 +-
>  include/linux/mmc/cd-gpio.h     |   18 --------
>  include/linux/mmc/slot-gpio.h   |   18 ++++++++
>  6 files changed, 105 insertions(+), 105 deletions(-)
>  delete mode 100644 drivers/mmc/core/cd-gpio.c
>  create mode 100644 drivers/mmc/core/slot-gpio.c
>  delete mode 100644 include/linux/mmc/cd-gpio.h
>  create mode 100644 include/linux/mmc/slot-gpio.h
>
> diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
> index dca4428..38ed210 100644
> --- a/drivers/mmc/core/Makefile
> +++ b/drivers/mmc/core/Makefile
> @@ -7,6 +7,6 @@ mmc_core-y                      := core.o bus.o host.o \
>                                   mmc.o mmc_ops.o sd.o sd_ops.o \
>                                   sdio.o sdio_ops.o sdio_bus.o \
>                                   sdio_cis.o sdio_io.o sdio_irq.o \
> -                                  quirks.o cd-gpio.o
> +                                  quirks.o slot-gpio.o
>
>  mmc_core-$(CONFIG_DEBUG_FS)    += debugfs.o
> diff --git a/drivers/mmc/core/cd-gpio.c b/drivers/mmc/core/cd-gpio.c
> deleted file mode 100644
> index f13e38d..0000000
> --- a/drivers/mmc/core/cd-gpio.c
> +++ /dev/null
> @@ -1,83 +0,0 @@
> -/*
> - * Generic GPIO card-detect helper
> - *
> - * Copyright (C) 2011, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - */
> -
> -#include <linux/err.h>
> -#include <linux/gpio.h>
> -#include <linux/interrupt.h>
> -#include <linux/jiffies.h>
> -#include <linux/mmc/cd-gpio.h>
> -#include <linux/mmc/host.h>
> -#include <linux/module.h>
> -#include <linux/slab.h>
> -
> -struct mmc_cd_gpio {
> -       unsigned int gpio;
> -       char label[0];
> -};
> -
> -static irqreturn_t mmc_cd_gpio_irqt(int irq, void *dev_id)
> -{
> -       /* Schedule a card detection after a debounce timeout */
> -       mmc_detect_change(dev_id, msecs_to_jiffies(100));
> -       return IRQ_HANDLED;
> -}
> -
> -int mmc_cd_gpio_request(struct mmc_host *host, unsigned int gpio)
> -{
> -       size_t len = strlen(dev_name(host->parent)) + 4;
> -       struct mmc_cd_gpio *cd;
> -       int irq = gpio_to_irq(gpio);
> -       int ret;
> -
> -       if (irq < 0)
> -               return irq;
> -
> -       cd = kmalloc(sizeof(*cd) + len, GFP_KERNEL);
> -       if (!cd)
> -               return -ENOMEM;
> -
> -       snprintf(cd->label, len, "%s cd", dev_name(host->parent));
> -
> -       ret = gpio_request_one(gpio, GPIOF_DIR_IN, cd->label);
> -       if (ret < 0)
> -               goto egpioreq;
> -
> -       ret = request_threaded_irq(irq, NULL, mmc_cd_gpio_irqt,
> -                                  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> -                                  cd->label, host);
> -       if (ret < 0)
> -               goto eirqreq;
> -
> -       cd->gpio = gpio;
> -       host->hotplug.irq = irq;
> -       host->hotplug.handler_priv = cd;
> -
> -       return 0;
> -
> -eirqreq:
> -       gpio_free(gpio);
> -egpioreq:
> -       kfree(cd);
> -       return ret;
> -}
> -EXPORT_SYMBOL(mmc_cd_gpio_request);
> -
> -void mmc_cd_gpio_free(struct mmc_host *host)
> -{
> -       struct mmc_cd_gpio *cd = host->hotplug.handler_priv;
> -
> -       if (!cd)
> -               return;
> -
> -       free_irq(host->hotplug.irq, host);
> -       gpio_free(cd->gpio);
> -       kfree(cd);
> -}
> -EXPORT_SYMBOL(mmc_cd_gpio_free);
> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
> new file mode 100644
> index 0000000..17d705f
> --- /dev/null
> +++ b/drivers/mmc/core/slot-gpio.c
> @@ -0,0 +1,83 @@
> +/*
> + * Generic GPIO card-detect helper
> + *
> + * Copyright (C) 2011, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/gpio.h>
> +#include <linux/interrupt.h>
> +#include <linux/jiffies.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/slot-gpio.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +
> +struct mmc_gpio {
> +       unsigned int cd_gpio;
> +       char cd_label[0];
> +};
> +
> +static irqreturn_t mmc_gpio_cd_irqt(int irq, void *dev_id)
> +{
> +       /* Schedule a card detection after a debounce timeout */
> +       mmc_detect_change(dev_id, msecs_to_jiffies(100));
> +       return IRQ_HANDLED;
> +}
> +
> +int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio)
> +{
> +       size_t len = strlen(dev_name(host->parent)) + 4;
> +       struct mmc_gpio *ctx;
> +       int irq = gpio_to_irq(gpio);
> +       int ret;
> +
> +       if (irq < 0)
> +               return irq;
> +
> +       ctx = kmalloc(sizeof(*ctx) + len, GFP_KERNEL);
> +       if (!ctx)
> +               return -ENOMEM;
> +
> +       snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent));
> +
> +       ret = gpio_request_one(gpio, GPIOF_DIR_IN, ctx->cd_label);
> +       if (ret < 0)
> +               goto egpioreq;
> +
> +       ret = request_threaded_irq(irq, NULL, mmc_gpio_cd_irqt,
> +                                  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +                                  ctx->cd_label, host);

Learnt about IRQF_ONESHOT recently - you need to add that as a flag when
your handler function is NULL. See
http://www.spinics.net/lists/linux-tip-commits/msg14974.html

> +       if (ret < 0)
> +               goto eirqreq;
> +
> +       ctx->cd_gpio = gpio;
> +       host->hotplug.irq = irq;
> +       host->hotplug.handler_priv = ctx;
> +
> +       return 0;
> +
> +eirqreq:
> +       gpio_free(gpio);
> +egpioreq:
> +       kfree(ctx);
> +       return ret;
> +}
> +EXPORT_SYMBOL(mmc_gpio_request_cd);
> +
> +void mmc_gpio_free_cd(struct mmc_host *host)
> +{
> +       struct mmc_gpio *ctx = host->hotplug.handler_priv;
> +
> +       if (!ctx)
> +               return;
> +
> +       free_irq(host->hotplug.irq, host);
> +       gpio_free(ctx->cd_gpio);
> +       kfree(ctx);
> +}
> +EXPORT_SYMBOL(mmc_gpio_free_cd);
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index 175cc71..7da63b5 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -34,8 +34,8 @@
>  #include <linux/io.h>
>  #include <linux/irq.h>
>  #include <linux/mfd/tmio.h>
> -#include <linux/mmc/cd-gpio.h>
>  #include <linux/mmc/host.h>
> +#include <linux/mmc/slot-gpio.h>
>  #include <linux/mmc/tmio.h>
>  #include <linux/module.h>
>  #include <linux/pagemap.h>
> @@ -1034,7 +1034,7 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
>        dev_pm_qos_expose_latency_limit(&pdev->dev, 100);
>
>        if (pdata->flags & TMIO_MMC_USE_GPIO_CD) {
> -               ret = mmc_cd_gpio_request(mmc, pdata->cd_gpio);
> +               ret = mmc_gpio_request_cd(mmc, pdata->cd_gpio);
>                if (ret < 0) {
>                        tmio_mmc_host_remove(_host);
>                        return ret;
> @@ -1066,7 +1066,7 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host)
>                 * This means we can miss a card-eject, but this is anyway
>                 * possible, because of delayed processing of hotplug events.
>                 */
> -               mmc_cd_gpio_free(mmc);
> +               mmc_gpio_free_cd(mmc);
>
>        if (!host->native_hotplug)
>                pm_runtime_get_sync(&pdev->dev);
> diff --git a/include/linux/mmc/cd-gpio.h b/include/linux/mmc/cd-gpio.h
> deleted file mode 100644
> index cefaba0..0000000
> --- a/include/linux/mmc/cd-gpio.h
> +++ /dev/null
> @@ -1,18 +0,0 @@
> -/*
> - * Generic GPIO card-detect helper header
> - *
> - * Copyright (C) 2011, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - */
> -
> -#ifndef MMC_CD_GPIO_H
> -#define MMC_CD_GPIO_H
> -
> -struct mmc_host;
> -int mmc_cd_gpio_request(struct mmc_host *host, unsigned int gpio);
> -void mmc_cd_gpio_free(struct mmc_host *host);
> -
> -#endif
> diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
> new file mode 100644
> index 0000000..edfaa32
> --- /dev/null
> +++ b/include/linux/mmc/slot-gpio.h
> @@ -0,0 +1,18 @@
> +/*
> + * Generic GPIO card-detect helper header
> + *
> + * Copyright (C) 2011, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef MMC_SLOT_GPIO_H
> +#define MMC_SLOT_GPIO_H
> +
> +struct mmc_host;
> +int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio);
> +void mmc_gpio_free_cd(struct mmc_host *host);
> +
> +#endif
> --
> 1.7.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 16/29] mmc: tmio: use MMC opcode defines instead of numbers

[Guennadi Liakhovetski]

mmc.h defines macros for most frequently used MMC opcodes. Use them instead
of hard-coded numbers.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/tmio_mmc_pio.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 7da63b5..e843da0 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -35,6 +35,7 @@
 #include <linux/irq.h>
 #include <linux/mfd/tmio.h>
 #include <linux/mmc/host.h>
+#include <linux/mmc/mmc.h>
 #include <linux/mmc/slot-gpio.h>
 #include <linux/mmc/tmio.h>
 #include <linux/module.h>
@@ -306,8 +307,8 @@ static int tmio_mmc_start_command(struct tmio_mmc_host *host, struct mmc_command
 	int c = cmd->opcode;
 	u32 irq_mask = TMIO_MASK_CMD;
 
-	/* Command 12 is handled by hardware */
-	if (cmd->opcode = 12 && !cmd->arg) {
+	/* CMD12 is handled by hardware */
+	if (cmd->opcode = MMC_STOP_TRANSMISSION && !cmd->arg) {
 		sd_ctrl_write16(host, CTL_STOP_INTERNAL_ACTION, 0x001);
 		return 0;
 	}
@@ -450,7 +451,7 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host)
 	}
 
 	if (stop) {
-		if (stop->opcode = 12 && !stop->arg)
+		if (stop->opcode = MMC_STOP_TRANSMISSION && !stop->arg)
 			sd_ctrl_write16(host, CTL_STOP_INTERNAL_ACTION, 0x000);
 		else
 			BUG();
-- 
1.7.2.5

[PATCH 17/29] mmc: use a more generic name for slot function types and fields

[Guennadi Liakhovetski]

struct mmc_host::hotplug is becoming a generic hook for slot functions.
Rename it accordingly.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/core/host.c      |    2 ++
 drivers/mmc/core/slot-gpio.c |    8 ++++----
 include/linux/mmc/host.h     |   17 ++++++++++++++---
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 91c84c7..b8c5290 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -327,6 +327,8 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
 
 	mmc_host_clk_init(host);
 
+	host->slot.cd_irq = -EINVAL;
+
 	spin_lock_init(&host->lock);
 	init_waitqueue_head(&host->wq);
 	INIT_DELAYED_WORK(&host->detect, mmc_rescan);
diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 17d705f..11a8ef1 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -56,8 +56,8 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio)
 		goto eirqreq;
 
 	ctx->cd_gpio = gpio;
-	host->hotplug.irq = irq;
-	host->hotplug.handler_priv = ctx;
+	host->slot.cd_irq = irq;
+	host->slot.handler_priv = ctx;
 
 	return 0;
 
@@ -71,12 +71,12 @@ EXPORT_SYMBOL(mmc_gpio_request_cd);
 
 void mmc_gpio_free_cd(struct mmc_host *host)
 {
-	struct mmc_gpio *ctx = host->hotplug.handler_priv;
+	struct mmc_gpio *ctx = host->slot.handler_priv;
 
 	if (!ctx)
 		return;
 
-	free_irq(host->hotplug.irq, host);
+	free_irq(host->slot.cd_irq, host);
 	gpio_free(ctx->cd_gpio);
 	kfree(ctx);
 }
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index bcf5562..3b4d611 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -150,8 +150,19 @@ struct mmc_async_req {
 	int (*err_check) (struct mmc_card *, struct mmc_async_req *);
 };
 
-struct mmc_hotplug {
-	unsigned int irq;
+/**
+ * struct mmc_slot - MMC slot functions
+ *
+ * @cd_irq:		MMC/SD-card slot hotplug detection IRQ or -EINVAL
+ * @handler_priv:	MMC/SD-card slot context
+ *
+ * Some MMC/SD host controllers implement slot-functions like card and
+ * write-protect detection natively. However, a large number of controllers
+ * leave these functions to the CPU. This struct provides a hook to attach
+ * such slot-function drivers.
+ */
+struct mmc_slot {
+	unsigned int cd_irq;
 	void *handler_priv;
 };
 
@@ -290,7 +301,7 @@ struct mmc_host {
 
 	struct delayed_work	detect;
 	int			detect_change;	/* card detect flag */
-	struct mmc_hotplug	hotplug;
+	struct mmc_slot		slot;
 
 	const struct mmc_bus_ops *bus_ops;	/* current bus driver */
 	unsigned int		bus_refs;	/* reference counter */
-- 
1.7.2.5

[PATCH 18/29] mmc: tmio: remove a duplicated comment line

[Guennadi Liakhovetski]

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/tmio_mmc_pio.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index e843da0..d1eb611 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -989,7 +989,6 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	 *  While we increment the runtime PM counter for all scenarios when
 	 *  the mmc core activates us by calling an appropriate set_ios(), we
 	 *  must additionally ensure that in case 2) the tmio mmc hardware stays
-	 *  additionally ensure that in case 2) the tmio mmc hardware stays
 	 *  powered on during runtime for the card detection to work.
 	 */
 	if (_host->native_hotplug)
-- 
1.7.2.5

[PATCH 19/29] mmc: add two capability flags for CD and WP signal polarity

[Guennadi Liakhovetski]

To handle CD and WP SD/MMC slot pins we need generic flags to specify their
polarity.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 include/linux/mmc/host.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 3b4d611..f1ca61b 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -249,6 +249,8 @@ struct mmc_host {
 #define MMC_CAP2_BROKEN_VOLTAGE	(1 << 7)	/* Use the broken voltage */
 #define MMC_CAP2_DETECT_ON_ERR	(1 << 8)	/* On I/O err check card removal */
 #define MMC_CAP2_HC_ERASE_SZ	(1 << 9)	/* High-capacity erase size */
+#define MMC_CAP2_INVERTED_CD	(1 << 10)	/* Card-detect signal active low */
+#define MMC_CAP2_INVERTED_RO	(1 << 11)	/* Write-protect signal active low */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 	unsigned int        power_notify_type;
-- 
1.7.2.5

[PATCH 20/29] mmc: add CD GPIO polling support to slot functions

[Guennadi Liakhovetski]

A simple extension of mmc slot functions add support for CD GPIO polling
for cases, where the GPIO cannot produce interrupts or this is for some
reason not desired.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/core/slot-gpio.c  |   42 +++++++++++++++++++++++++++++-----------
 include/linux/mmc/slot-gpio.h |    2 +
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 11a8ef1..1491fd8 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -29,6 +29,18 @@ static irqreturn_t mmc_gpio_cd_irqt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+int mmc_gpio_get_cd(struct mmc_host *host)
+{
+	struct mmc_gpio *ctx = host->slot.handler_priv;
+
+	if (!ctx || !gpio_is_valid(ctx->cd_gpio))
+		return -ENOSYS;
+
+	return !gpio_get_value_cansleep(ctx->cd_gpio) ^
+		!(host->caps2 & MMC_CAP2_INVERTED_CD);
+}
+EXPORT_SYMBOL(mmc_gpio_get_cd);
+
 int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio)
 {
 	size_t len = strlen(dev_name(host->parent)) + 4;
@@ -36,9 +48,6 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio)
 	int irq = gpio_to_irq(gpio);
 	int ret;
 
-	if (irq < 0)
-		return irq;
-
 	ctx = kmalloc(sizeof(*ctx) + len, GFP_KERNEL);
 	if (!ctx)
 		return -ENOMEM;
@@ -49,20 +58,25 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio)
 	if (ret < 0)
 		goto egpioreq;
 
-	ret = request_threaded_irq(irq, NULL, mmc_gpio_cd_irqt,
-				   IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
-				   ctx->cd_label, host);
-	if (ret < 0)
-		goto eirqreq;
+	/*
+	 * Even if gpio_to_irq() returns a valid IRQ number, the platform might
+	 * still prefer to poll, e.g., because that IRQ number is already used
+	 * by another unit and cannot be shared.
+	 */
+	if (host->caps & MMC_CAP_NEEDS_POLL || irq < 0) {
+		host->caps |= MMC_CAP_NEEDS_POLL;
+		host->slot.cd_irq = -EINVAL;
+	} else if (!request_threaded_irq(irq, NULL, mmc_gpio_cd_irqt,
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				ctx->cd_label, host)) {
+		host->slot.cd_irq = irq;
+	}
 
 	ctx->cd_gpio = gpio;
-	host->slot.cd_irq = irq;
 	host->slot.handler_priv = ctx;
 
 	return 0;
 
-eirqreq:
-	gpio_free(gpio);
 egpioreq:
 	kfree(ctx);
 	return ret;
@@ -76,8 +90,12 @@ void mmc_gpio_free_cd(struct mmc_host *host)
 	if (!ctx)
 		return;
 
-	free_irq(host->slot.cd_irq, host);
+	if (host->slot.cd_irq >= 0) {
+		free_irq(host->slot.cd_irq, host);
+		host->slot.cd_irq = -EINVAL;
+	}
 	gpio_free(ctx->cd_gpio);
+	host->slot.handler_priv = NULL;
 	kfree(ctx);
 }
 EXPORT_SYMBOL(mmc_gpio_free_cd);
diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
index edfaa32..1a977d7 100644
--- a/include/linux/mmc/slot-gpio.h
+++ b/include/linux/mmc/slot-gpio.h
@@ -12,6 +12,8 @@
 #define MMC_SLOT_GPIO_H
 
 struct mmc_host;
+
+int mmc_gpio_get_cd(struct mmc_host *host);
 int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio);
 void mmc_gpio_free_cd(struct mmc_host *host);
 
-- 
1.7.2.5

[PATCH 21/29] mmc: convert slot functions to managed allocation

[Guennadi Liakhovetski]

This prepares for the addition of further slot functions.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/core/host.c      |    2 +
 drivers/mmc/core/slot-gpio.c |   49 +++++++++++++++++++++++++++++++----------
 include/linux/mmc/host.h     |    2 +
 3 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index b8c5290..74cf29a5 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -32,6 +32,7 @@
 static void mmc_host_classdev_release(struct device *dev)
 {
 	struct mmc_host *host = cls_dev_to_mmc_host(dev);
+	mutex_destroy(&host->slot.lock);
 	kfree(host);
 }
 
@@ -327,6 +328,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
 
 	mmc_host_clk_init(host);
 
+	mutex_init(&host->slot.lock);
 	host->slot.cd_irq = -EINVAL;
 
 	spin_lock_init(&host->lock);
diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 1491fd8..7e16a50 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -29,6 +29,33 @@ static irqreturn_t mmc_gpio_cd_irqt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static int mmc_gpio_alloc(struct mmc_host *host)
+{
+	size_t len = strlen(dev_name(host->parent)) + 4;
+	struct mmc_gpio *ctx;
+
+	mutex_lock(&host->slot.lock);
+
+	ctx = host->slot.handler_priv;
+	if (!ctx) {
+		/*
+		 * devm_kzalloc() can be called after device_initialize(), even
+		 * before device_add(), i.e., between mmc_alloc_host() and
+		 * mmc_add_host()
+		 */
+		ctx = devm_kzalloc(&host->class_dev, sizeof(*ctx) + len,
+				   GFP_KERNEL);
+		if (ctx) {
+			snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent));
+			host->slot.handler_priv = ctx;
+		}
+	}
+
+	mutex_unlock(&host->slot.lock);
+
+	return ctx ? 0 : -ENOMEM;
+}
+
 int mmc_gpio_get_cd(struct mmc_host *host)
 {
 	struct mmc_gpio *ctx = host->slot.handler_priv;
@@ -43,20 +70,24 @@ EXPORT_SYMBOL(mmc_gpio_get_cd);
 
 int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio)
 {
-	size_t len = strlen(dev_name(host->parent)) + 4;
 	struct mmc_gpio *ctx;
 	int irq = gpio_to_irq(gpio);
 	int ret;
 
-	ctx = kmalloc(sizeof(*ctx) + len, GFP_KERNEL);
-	if (!ctx)
-		return -ENOMEM;
+	ret = mmc_gpio_alloc(host);
+	if (ret < 0)
+		return ret;
 
-	snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent));
+	ctx = host->slot.handler_priv;
 
 	ret = gpio_request_one(gpio, GPIOF_DIR_IN, ctx->cd_label);
 	if (ret < 0)
-		goto egpioreq;
+		/*
+		 * don't bother freeing gpio. It might still get used by other
+		 * slot functions, in any case it will be freed, when the device
+		 * is destroyed.
+		 */
+		return ret;
 
 	/*
 	 * Even if gpio_to_irq() returns a valid IRQ number, the platform might
@@ -76,10 +107,6 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio)
 	host->slot.handler_priv = ctx;
 
 	return 0;
-
-egpioreq:
-	kfree(ctx);
-	return ret;
 }
 EXPORT_SYMBOL(mmc_gpio_request_cd);
 
@@ -95,7 +122,5 @@ void mmc_gpio_free_cd(struct mmc_host *host)
 		host->slot.cd_irq = -EINVAL;
 	}
 	gpio_free(ctx->cd_gpio);
-	host->slot.handler_priv = NULL;
-	kfree(ctx);
 }
 EXPORT_SYMBOL(mmc_gpio_free_cd);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index f1ca61b..f8157a9 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -154,6 +154,7 @@ struct mmc_async_req {
  * struct mmc_slot - MMC slot functions
  *
  * @cd_irq:		MMC/SD-card slot hotplug detection IRQ or -EINVAL
+ * @lock:		protect the @handler_priv pointer
  * @handler_priv:	MMC/SD-card slot context
  *
  * Some MMC/SD host controllers implement slot-functions like card and
@@ -163,6 +164,7 @@ struct mmc_async_req {
  */
 struct mmc_slot {
 	unsigned int cd_irq;
+	struct mutex lock;
 	void *handler_priv;
 };
 
-- 
1.7.2.5

[PATCH 22/29] mmc: add WP pin handler to slot functions

[Guennadi Liakhovetski]

Card Write-Protect pin is often implemented, using a GPIO, which makes it
simple to provide a generic handler for it.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/core/slot-gpio.c  |   51 ++++++++++++++++++++++++++++++++++++++++-
 include/linux/mmc/slot-gpio.h |    4 +++
 2 files changed, 54 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 7e16a50..eb04c06 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -18,7 +18,9 @@
 #include <linux/slab.h>
 
 struct mmc_gpio {
+	unsigned int ro_gpio;
 	unsigned int cd_gpio;
+	char *ro_label;
 	char cd_label[0];
 };
 
@@ -43,10 +45,12 @@ static int mmc_gpio_alloc(struct mmc_host *host)
 		 * before device_add(), i.e., between mmc_alloc_host() and
 		 * mmc_add_host()
 		 */
-		ctx = devm_kzalloc(&host->class_dev, sizeof(*ctx) + len,
+		ctx = devm_kzalloc(&host->class_dev, sizeof(*ctx) + 2 * len,
 				   GFP_KERNEL);
 		if (ctx) {
+			ctx->ro_label = ctx->cd_label + len;
 			snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent));
+			snprintf(ctx->ro_label, len, "%s ro", dev_name(host->parent));
 			host->slot.handler_priv = ctx;
 		}
 	}
@@ -56,6 +60,18 @@ static int mmc_gpio_alloc(struct mmc_host *host)
 	return ctx ? 0 : -ENOMEM;
 }
 
+int mmc_gpio_get_ro(struct mmc_host *host)
+{
+	struct mmc_gpio *ctx = host->slot.handler_priv;
+
+	if (!ctx || !gpio_is_valid(ctx->ro_gpio))
+		return -ENOSYS;
+
+	return !gpio_get_value_cansleep(ctx->ro_gpio) ^
+		!(host->caps2 & MMC_CAP2_INVERTED_RO);
+}
+EXPORT_SYMBOL(mmc_gpio_get_ro);
+
 int mmc_gpio_get_cd(struct mmc_host *host)
 {
 	struct mmc_gpio *ctx = host->slot.handler_priv;
@@ -68,6 +84,28 @@ int mmc_gpio_get_cd(struct mmc_host *host)
 }
 EXPORT_SYMBOL(mmc_gpio_get_cd);
 
+int mmc_gpio_request_ro(struct mmc_host *host, unsigned int gpio)
+{
+	struct mmc_gpio *ctx;
+	int ret;
+
+	if (!gpio_is_valid(gpio))
+		return -EINVAL;
+
+	ret = mmc_gpio_alloc(host);
+	if (ret < 0)
+		return ret;
+
+	ctx = host->slot.handler_priv;
+
+	ret = gpio_request_one(gpio, GPIOF_DIR_IN, ctx->ro_label);
+	if (ret < 0)
+		return ret;;
+
+	return 0;
+}
+EXPORT_SYMBOL(mmc_gpio_request_ro);
+
 int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio)
 {
 	struct mmc_gpio *ctx;
@@ -110,6 +148,17 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio)
 }
 EXPORT_SYMBOL(mmc_gpio_request_cd);
 
+void mmc_gpio_free_ro(struct mmc_host *host)
+{
+	struct mmc_gpio *ctx = host->slot.handler_priv;
+
+	if (!ctx || !gpio_is_valid(ctx->ro_gpio))
+		return;
+
+	gpio_free(ctx->ro_gpio);
+}
+EXPORT_SYMBOL(mmc_gpio_free_ro);
+
 void mmc_gpio_free_cd(struct mmc_host *host)
 {
 	struct mmc_gpio *ctx = host->slot.handler_priv;
diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
index 1a977d7..7d88d27 100644
--- a/include/linux/mmc/slot-gpio.h
+++ b/include/linux/mmc/slot-gpio.h
@@ -13,6 +13,10 @@
 
 struct mmc_host;
 
+int mmc_gpio_get_ro(struct mmc_host *host);
+int mmc_gpio_request_ro(struct mmc_host *host, unsigned int gpio);
+void mmc_gpio_free_ro(struct mmc_host *host);
+
 int mmc_gpio_get_cd(struct mmc_host *host);
 int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio);
 void mmc_gpio_free_cd(struct mmc_host *host);
-- 
1.7.2.5

[PATCH 23/29] mmc: tmio: use generic GPIO CD and WP handlers

[Guennadi Liakhovetski]

The tmio-mmc driver is already using the generic GPIO CD handler in IRQ
mode. This patch adds support for CD polling mode and also checks for
availability of a WP GPIO.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/tmio_mmc_pio.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index d1eb611..85daa53 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -875,6 +875,9 @@ static int tmio_mmc_get_ro(struct mmc_host *mmc)
 {
 	struct tmio_mmc_host *host = mmc_priv(mmc);
 	struct tmio_mmc_data *pdata = host->pdata;
+	int ret = mmc_gpio_get_ro(mmc);
+	if (ret >= 0)
+		return ret;
 
 	return !((pdata->flags & TMIO_MMC_WRPROTECT_DISABLE) ||
 		 (sd_ctrl_read32(host, CTL_STATUS) & TMIO_STAT_WRPROTECT));
@@ -884,6 +887,9 @@ static int tmio_mmc_get_cd(struct mmc_host *mmc)
 {
 	struct tmio_mmc_host *host = mmc_priv(mmc);
 	struct tmio_mmc_data *pdata = host->pdata;
+	int ret = mmc_gpio_get_cd(mmc);
+	if (ret >= 0)
+		return ret;
 
 	if (!pdata->get_cd)
 		return -ENOSYS;
-- 
1.7.2.5

[PATCH 24/29] mmc: add a simple generic OF parser

[Guennadi Liakhovetski]

Many MMC host drivers already use OF data to obtain their platform-specific
configuration. Each of them is doing this in its special way, whereas many
parameters are identical and can easily be generalised. This patch adds
such a generic parser. So far it only adds and handles very few basic
properties. New ones can be added in the future as required.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/mmc/core/Makefile |    2 +
 drivers/mmc/core/of.c     |   91 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/host.h  |   12 ++++++
 3 files changed, 105 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mmc/core/of.c

diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index 38ed210..aa7727e 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -9,4 +9,6 @@ mmc_core-y			:= core.o bus.o host.o \
 				   sdio_cis.o sdio_io.o sdio_irq.o \
 				   quirks.o slot-gpio.o
 
+mmc_core-$(CONFIG_OF)		+= of.o
+
 mmc_core-$(CONFIG_DEBUG_FS)	+= debugfs.o
diff --git a/drivers/mmc/core/of.c b/drivers/mmc/core/of.c
new file mode 100644
index 0000000..2fba115
--- /dev/null
+++ b/drivers/mmc/core/of.c
@@ -0,0 +1,91 @@
+/*
+ * Generic MMC OF parser
+ *
+ * Copyright (C) 2012, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/slot-gpio.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+
+/*
+ * Implementation-specific capabilities, that the controller driver usually
+ * cannot autodetect and has to receive from an external source, e.g., from
+ * platform data or from OF are:
+ *
+ * MMC_CAP_4_BIT_DATA: depends on the board layout: how many data lines are
+ *			connected
+ * MMC_CAP_8_BIT_DATA: ditto
+ * MMC_CAP2_INVERTED_CD: card-detect active-low (0 = card present)
+ * MMC_CAP2_INVERTED_RO: write-protect active-low (0 = read-only)
+ *	Some features might be supplied as GPIO bindings:
+ * MMC_CAP_SDIO_IRQ: depends, whether an SDIO IRQ line is connected
+ * MMC_CAP_NEEDS_POLL: is a card-detect pin connected?
+ * MMC_CAP_NONREMOVABLE: can also be a board feature, also, some media-types
+ *			(eMMC?) are intrinsicly non-removable
+ *	Others can be retrieved from regulators:
+ * MMC_CAP_POWER_OFF_CARD
+ */
+
+void mmc_of_get(struct mmc_host *mmc)
+{
+	struct device *dev = mmc_dev(mmc);
+	struct device_node *node = dev->of_node;
+	enum of_gpio_flags flags;
+	unsigned int gpio;
+	int ret;
+
+	if (!node || !of_device_is_available(node))
+		return;
+
+	if (of_get_property(node, "mmc,4_bit_data", NULL))
+		mmc->caps |= MMC_CAP_4_BIT_DATA;
+	if (of_get_property(node, "mmc,8_bit_data", NULL))
+		mmc->caps |= MMC_CAP_8_BIT_DATA;
+	if (of_get_property(node, "mmc,sdio_irq", NULL))
+		mmc->caps |= MMC_CAP_SDIO_IRQ;
+	if (of_get_property(node, "mmc,nonremovable", NULL))
+		mmc->caps |= MMC_CAP_NONREMOVABLE;
+	if (of_get_property(node, "mmc,needs_poll", NULL))
+		mmc->caps |= MMC_CAP_NEEDS_POLL;
+	if (of_get_property(node, "mmc,inverted_cd", NULL))
+		mmc->caps2 |= MMC_CAP2_INVERTED_CD;
+	if (of_get_property(node, "mmc,inverted_ro", NULL))
+		mmc->caps2 |= MMC_CAP2_INVERTED_RO;
+
+	/*
+	 * Both mmc_gpio_request_cd() and mmc_gpio_request_ro() check for
+	 * gpio_valid(), so, no need to check for error
+	 */
+
+	gpio = of_get_named_gpio_flags(node, "cd-gpios", 0, &flags);
+	ret = mmc_gpio_request_cd(mmc, gpio);
+	if (ret < 0)
+		dev_warn(mmc->parent, "Failed to obtain CD GPIO: %d\n", ret);
+
+	gpio = of_get_named_gpio_flags(node, "ro-gpios", 0, &flags);
+	ret = mmc_gpio_request_ro(mmc, gpio);
+	if (ret < 0)
+		dev_warn(mmc->parent, "Failed to obtain RO GPIO: %d\n", ret);
+}
+EXPORT_SYMBOL(mmc_of_get);
+
+void mmc_of_put(struct mmc_host *mmc)
+{
+	struct device_node *node = mmc_dev(mmc)->of_node;
+
+	if (!node || !of_device_is_available(node))
+		return;
+
+	mmc_gpio_free_ro(mmc);
+	mmc_gpio_free_cd(mmc);
+}
+EXPORT_SYMBOL(mmc_of_put);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index f8157a9..94b3805 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -450,4 +450,16 @@ static inline unsigned int mmc_host_clk_rate(struct mmc_host *host)
 	return host->ios.clock;
 }
 #endif
+
+#ifdef CONFIG_OF
+void mmc_of_get(struct mmc_host *mmc);
+void mmc_of_put(struct mmc_host *mmc);
+#else
+static inline void mmc_of_get(struct mmc_host *mmc)
+{
+}
+static inline void mmc_of_put(struct mmc_host *mmc)
+{
+}
+#endif
 #endif /* LINUX_MMC_HOST_H */
-- 
1.7.2.5

Re: [PATCH 24/29] mmc: add a simple generic OF parser

[Olof Johansson]

On Thu, May 3, 2012 at 8:05 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Many MMC host drivers already use OF data to obtain their platform-specific
> configuration. Each of them is doing this in its special way, whereas many
> parameters are identical and can easily be generalised. This patch adds
> such a generic parser. So far it only adds and handles very few basic
> properties. New ones can be added in the future as required.

The bindings need to be documented under Documentation/devicetree/mmc

It's common to use dashes (-) instead of underscores (_) in properties

Also, no need to prefix with mmc. But also:


> +       if (of_get_property(node, "mmc,4_bit_data", NULL))
> +               mmc->caps |= MMC_CAP_4_BIT_DATA;
> +       if (of_get_property(node, "mmc,8_bit_data", NULL))
> +               mmc->caps |= MMC_CAP_8_BIT_DATA;

In the device tree, do these encode the capability of the controller,
or how the device under the controller is wired up? It's not uncommon
to have an 4-bit device on a controller that could possibly support
8-bit devices. It needs to be clear from the property name which is
which to not confuse everyone all the time.

> +       if (of_get_property(node, "mmc,sdio_irq", NULL))
> +               mmc->caps |= MMC_CAP_SDIO_IRQ;

"supports-sdio-interrupts"

> +       if (of_get_property(node, "mmc,nonremovable", NULL))
> +               mmc->caps |= MMC_CAP_NONREMOVABLE;

This is a property of the card (well, how the card is connected), not
the controller, right? Again, see above regarding confusion.

> +       if (of_get_property(node, "mmc,needs_poll", NULL))
> +               mmc->caps |= MMC_CAP_NEEDS_POLL;

This should be implicit from lack of interrupt property in the device node.

> +       if (of_get_property(node, "mmc,inverted_cd", NULL))
> +               mmc->caps2 |= MMC_CAP2_INVERTED_CD;
> +       if (of_get_property(node, "mmc,inverted_ro", NULL))
> +               mmc->caps2 |= MMC_CAP2_INVERTED_RO;

The GPIO specifiers should be able to encode polarity. Is this needed
for other cases as well?


-Olof

Re: [PATCH 24/29] mmc: add a simple generic OF parser

[Guennadi Liakhovetski]

Hi Olof

On Fri, 4 May 2012, Olof Johansson wrote:

> On Thu, May 3, 2012 at 8:05 AM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > Many MMC host drivers already use OF data to obtain their platform-specific
> > configuration. Each of them is doing this in its special way, whereas many
> > parameters are identical and can easily be generalised. This patch adds
> > such a generic parser. So far it only adds and handles very few basic
> > properties. New ones can be added in the future as required.
> 
> The bindings need to be documented under Documentation/devicetree/mmc

Right, I would do that, but I think, I'll drop this patch altogether for 
now. There is an alternative patch being developed at the moment, that 
standardises mmc bindings too. It doesn't provide a parser, but that can 
be added later if needed.

Thanks
Guennadi

> It's common to use dashes (-) instead of underscores (_) in properties
> 
> Also, no need to prefix with mmc. But also:
> 
> 
> > +       if (of_get_property(node, "mmc,4_bit_data", NULL))
> > +               mmc->caps |= MMC_CAP_4_BIT_DATA;
> > +       if (of_get_property(node, "mmc,8_bit_data", NULL))
> > +               mmc->caps |= MMC_CAP_8_BIT_DATA;
> 
> In the device tree, do these encode the capability of the controller,
> or how the device under the controller is wired up? It's not uncommon
> to have an 4-bit device on a controller that could possibly support
> 8-bit devices. It needs to be clear from the property name which is
> which to not confuse everyone all the time.
> 
> > +       if (of_get_property(node, "mmc,sdio_irq", NULL))
> > +               mmc->caps |= MMC_CAP_SDIO_IRQ;
> 
> "supports-sdio-interrupts"
> 
> > +       if (of_get_property(node, "mmc,nonremovable", NULL))
> > +               mmc->caps |= MMC_CAP_NONREMOVABLE;
> 
> This is a property of the card (well, how the card is connected), not
> the controller, right? Again, see above regarding confusion.
> 
> > +       if (of_get_property(node, "mmc,needs_poll", NULL))
> > +               mmc->caps |= MMC_CAP_NEEDS_POLL;
> 
> This should be implicit from lack of interrupt property in the device node.
> 
> > +       if (of_get_property(node, "mmc,inverted_cd", NULL))
> > +               mmc->caps2 |= MMC_CAP2_INVERTED_CD;
> > +       if (of_get_property(node, "mmc,inverted_ro", NULL))
> > +               mmc->caps2 |= MMC_CAP2_INVERTED_RO;
> 
> The GPIO specifiers should be able to encode polarity. Is this needed
> for other cases as well?
> 
> 
> -Olof
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 24/29] mmc: add a simple generic OF parser

[Magnus Damm]

On Fri, May 4, 2012 at 12:05 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Many MMC host drivers already use OF data to obtain their platform-specific
> configuration. Each of them is doing this in its special way, whereas many
> parameters are identical and can easily be generalised. This patch adds
> such a generic parser. So far it only adds and handles very few basic
> properties. New ones can be added in the future as required.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>

I usually don't say things this loud, but here it is: NAK!

To begin with: this is not what we agreed on earlier. I asked you to
only implement a small subset to begin with. This so we don't end up
supporting a bunch of DT flags forever.

In general it's a good idea to make something generic, but I think now
is too early for our drivers on SH and SH-Mobile ARM. We need to
experiment more with PINCTRL as well as GPIO bindings via DT. When we
have done that I think we should revisit.

Please see below for my reasons:

> +       if (of_get_property(node, "mmc,4_bit_data", NULL))
> +               mmc->caps |= MMC_CAP_4_BIT_DATA;
> +       if (of_get_property(node, "mmc,8_bit_data", NULL))
> +               mmc->caps |= MMC_CAP_8_BIT_DATA;

I suspect that these overlap with PINCTRL. I don't want to add support
for potentially duplicated functionality.

> +       gpio = of_get_named_gpio_flags(node, "cd-gpios", 0, &flags);
> +       ret = mmc_gpio_request_cd(mmc, gpio);
> +       if (ret < 0)
> +               dev_warn(mmc->parent, "Failed to obtain CD GPIO: %d\n", ret);
> +
> +       gpio = of_get_named_gpio_flags(node, "ro-gpios", 0, &flags);
> +       ret = mmc_gpio_request_ro(mmc, gpio);
> +       if (ret < 0)
> +               dev_warn(mmc->parent, "Failed to obtain RO GPIO: %d\n", ret);
> +}

We want to have stable DT bindings for GPIO and PINCTRL before we let
this loose with DT.

So please ignore this patch for now. Thanks for your efforts anyway.

/ magnus

[PATCH 25/29] mmc: tmio: add OF support

[Guennadi Liakhovetski]

Add OF support to the TMIO MMC to parse both standard and driver-specific
properties.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/mmc/host/tmio_mmc_pio.c |   39 ++++++++++++++++++++++++++++++++++++---
 1 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 85daa53..efa7bde 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -39,6 +39,7 @@
 #include <linux/mmc/slot-gpio.h>
 #include <linux/mmc/tmio.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/pagemap.h>
 #include <linux/platform_device.h>
 #include <linux/pm_qos.h>
@@ -905,7 +906,7 @@ static const struct mmc_host_ops tmio_mmc_ops = {
 	.enable_sdio_irq = tmio_mmc_enable_sdio_irq,
 };
 
-static void tmio_mmc_init_ocr(struct tmio_mmc_host *host)
+static void __devinit tmio_mmc_init_ocr(struct tmio_mmc_host *host)
 {
 	struct tmio_mmc_data *pdata = host->pdata;
 	struct mmc_host *mmc = host->mmc;
@@ -923,6 +924,22 @@ static void tmio_mmc_init_ocr(struct tmio_mmc_host *host)
 		mmc->ocr_avail = pdata->ocr_mask ? : MMC_VDD_32_33 | MMC_VDD_33_34;
 }
 
+static void __devinit tmio_mmc_parse_of(struct tmio_mmc_host *host)
+{
+	struct device_node *node = host->pdev->dev.of_node;
+	struct tmio_mmc_data *pdata = host->pdata;
+
+	if (!node || !of_device_is_available(node))
+		return;
+
+	if (of_get_property(node, "tmio,high_speed", NULL))
+		pdata->capabilities |= MMC_CAP_SD_HIGHSPEED;
+	if (of_get_property(node, "tmio,wrprotect_disable", NULL))
+		pdata->flags |= TMIO_MMC_WRPROTECT_DISABLE;
+	if (of_get_property(node, "tmio,idle_wait", NULL))
+		pdata->flags |= TMIO_MMC_HAS_IDLE_WAIT;
+}
+
 int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 				  struct platform_device *pdev,
 				  struct tmio_mmc_data *pdata)
@@ -960,8 +977,19 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 		goto host_free;
 	}
 
+	tmio_mmc_parse_of(_host);
+	/*
+	 * After OF parsing we have 3 possibilities:
+	 * 1. mmc->slot.cd_irq >= 0	- an IRQ-capable GPIO has been found and
+	 *				  an ISR has been installed
+	 * 2. MMC_CAP_NEEDS_POLL	- a GPIO has been found, polling will be
+	 *				  used
+	 * 3. none of the above		- no usable GPIO CD has been detected
+	 */
+	mmc_of_get(mmc);
+
 	mmc->ops = &tmio_mmc_ops;
-	mmc->caps = MMC_CAP_4_BIT_DATA | pdata->capabilities;
+	mmc->caps |= MMC_CAP_4_BIT_DATA | pdata->capabilities;
 	mmc->max_segs = 32;
 	mmc->max_blk_size = 512;
 	mmc->max_blk_count = (PAGE_CACHE_SIZE / mmc->max_blk_size) *
@@ -972,7 +1000,8 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 
 	_host->native_hotplug = !(pdata->flags & TMIO_MMC_USE_GPIO_CD ||
 				  mmc->caps & MMC_CAP_NEEDS_POLL ||
-				  mmc->caps & MMC_CAP_NONREMOVABLE);
+				  mmc->caps & MMC_CAP_NONREMOVABLE ||
+				  mmc->slot.cd_irq >= 0);
 
 	_host->power = false;
 	pm_runtime_enable(&pdev->dev);
@@ -1053,6 +1082,7 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 
 pm_disable:
 	pm_runtime_disable(&pdev->dev);
+	mmc_of_put(mmc);
 	iounmap(_host->ctl);
 host_free:
 	mmc_free_host(mmc);
@@ -1074,6 +1104,9 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host)
 		 */
 		mmc_gpio_free_cd(mmc);
 
+	/* This will disable OF-configured card-detection, do early */
+	mmc_of_put(mmc);
+
 	if (!host->native_hotplug)
 		pm_runtime_get_sync(&pdev->dev);
 
-- 
1.7.2.5

[PATCH 26/29] mmc: sdhi: add OF support, make platform data optional

[Guennadi Liakhovetski]

Now, that most parameters, required to configure the TMIO MMC driver can be
obtained from OF, we can also add support for it to the SDHI glue, which
also makes it necessary to be able to run without platform data.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/mmc/host/sh_mobile_sdhi.c |   26 +++++++++++++++++---------
 1 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index ae2e3da..b20c637 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -22,6 +22,7 @@
 #include <linux/clk.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/sh_mobile_sdhi.h>
@@ -132,9 +133,8 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	}
 
 	mmc_data = &priv->mmc_data;
-	p->pdata = mmc_data;
 
-	if (p->init) {
+	if (p && p->init) {
 		ret = p->init(pdev, &sdhi_ops);
 		if (ret)
 			goto einit;
@@ -147,10 +147,6 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 		goto eclkget;
 	}
 
-	if (p->set_pwr)
-		mmc_data->set_pwr = sh_mobile_sdhi_set_pwr;
-	if (p->get_cd)
-		mmc_data->get_cd = sh_mobile_sdhi_get_cd;
 	mmc_data->clk_enable = sh_mobile_sdhi_clk_enable;
 	mmc_data->clk_disable = sh_mobile_sdhi_clk_disable;
 	mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
@@ -161,6 +157,10 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 		mmc_data->ocr_mask = p->tmio_ocr_mask;
 		mmc_data->capabilities |= p->tmio_caps;
 		mmc_data->cd_gpio = p->cd_gpio;
+		if (p->set_pwr)
+			mmc_data->set_pwr = sh_mobile_sdhi_set_pwr;
+		if (p->get_cd)
+			mmc_data->get_cd = sh_mobile_sdhi_get_cd;
 
 		if (p->dma_slave_tx > 0 && p->dma_slave_rx > 0) {
 			priv->param_tx.slave_id = p->dma_slave_tx;
@@ -266,7 +266,7 @@ eirq_card_detect:
 eprobe:
 	clk_put(priv->clk);
 eclkget:
-	if (p->cleanup)
+	if (p && p->cleanup)
 		p->cleanup(pdev);
 einit:
 	kfree(priv);
@@ -281,7 +281,8 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
 	int i = 0, irq;
 
-	p->pdata = NULL;
+	if (p)
+		p->pdata = NULL;
 
 	tmio_mmc_host_remove(host);
 
@@ -294,7 +295,7 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 
 	clk_put(priv->clk);
 
-	if (p->cleanup)
+	if (p && p->cleanup)
 		p->cleanup(pdev);
 
 	kfree(priv);
@@ -309,11 +310,18 @@ static const struct dev_pm_ops tmio_mmc_dev_pm_ops = {
 	.runtime_resume = tmio_mmc_host_runtime_resume,
 };
 
+static const struct of_device_id sh_mobile_sdhi_of_match[] = {
+	{ .compatible = "renesas,shmobile-sdhi" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match);
+
 static struct platform_driver sh_mobile_sdhi_driver = {
 	.driver		= {
 		.name	= "sh_mobile_sdhi",
 		.owner	= THIS_MODULE,
 		.pm	= &tmio_mmc_dev_pm_ops,
+		.of_match_table = sh_mobile_sdhi_of_match,
 	},
 	.probe		= sh_mobile_sdhi_probe,
 	.remove		= __devexit_p(sh_mobile_sdhi_remove),
-- 
1.7.2.5

[PATCH 27/29] mmc: mmcif: add OF support

[Guennadi Liakhovetski]

Add generic MMC OF parsing support to the sh-mmcif driver.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/mmc/host/sh_mmcif.c |   33 +++++++++++++++++++++++++++------
 1 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index f6d0b11..4fdedcf 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -54,6 +54,7 @@
 #include <linux/mmc/mmc.h>
 #include <linux/mmc/sdio.h>
 #include <linux/mmc/sh_mmcif.h>
+#include <linux/of.h>
 #include <linux/pagemap.h>
 #include <linux/platform_device.h>
 #include <linux/pm_qos.h>
@@ -387,6 +388,9 @@ static void sh_mmcif_request_dma(struct sh_mmcif_host *host,
 	struct sh_dmae_slave *tx, *rx;
 	host->dma_active = false;
 
+	if (!pdata)
+		return;
+
 	/* We can only either use DMA for both Tx and Rx or not use it at all */
 	if (pdata->dma) {
 		dev_warn(&host->pd->dev,
@@ -447,13 +451,14 @@ static void sh_mmcif_release_dma(struct sh_mmcif_host *host)
 static void sh_mmcif_clock_control(struct sh_mmcif_host *host, unsigned int clk)
 {
 	struct sh_mmcif_plat_data *p = host->pd->dev.platform_data;
+	bool sup_pclk = p ? p->sup_pclk : false;
 
 	sh_mmcif_bitclr(host, MMCIF_CE_CLK_CTRL, CLK_ENABLE);
 	sh_mmcif_bitclr(host, MMCIF_CE_CLK_CTRL, CLK_CLEAR);
 
 	if (!clk)
 		return;
-	if (p->sup_pclk && clk = host->clk)
+	if (sup_pclk && clk = host->clk)
 		sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_SUP_PCLK);
 	else
 		sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_CLEAR &
@@ -937,7 +942,7 @@ static void sh_mmcif_set_power(struct sh_mmcif_host *host, struct mmc_ios *ios)
 		/* Errors ignored... */
 		mmc_regulator_set_ocr(host->mmc, host->vdd,
 				      ios->power_mode ? ios->vdd : 0);
-	else if (pd->set_pwr)
+	else if (pd && pd->set_pwr)
 		pd->set_pwr(host->pd, ios->power_mode != MMC_POWER_OFF);
 }
 
@@ -1002,7 +1007,7 @@ static int sh_mmcif_get_cd(struct mmc_host *mmc)
 	struct sh_mmcif_host *host = mmc_priv(mmc);
 	struct sh_mmcif_plat_data *p = host->pd->dev.platform_data;
 
-	if (!p->get_cd)
+	if (!p || !p->get_cd)
 		return -ENOSYS;
 	else
 		return p->get_cd(host->pd);
@@ -1275,6 +1280,9 @@ static void sh_mmcif_init_ocr(struct sh_mmcif_host *host)
 
 	host->vdd = mmc_regulator_get_vmmc(mmc);
 
+	if (!pd)
+		return;
+
 	if ((mmc->ocr_avail && pd->ocr) || (host->vdd && pd->set_pwr))
 		dev_warn(mmc_dev(mmc),
 			 "Platform OCR mask / .set_pwr() are ignored\n");
@@ -1288,6 +1296,7 @@ static void sh_mmcif_init_ocr(struct sh_mmcif_host *host)
 
 static int __devinit sh_mmcif_probe(struct platform_device *pdev)
 {
+	struct device_node *node = pdev->dev.of_node;
 	int ret = 0, irq[2];
 	struct mmc_host *mmc;
 	struct sh_mmcif_host *host;
@@ -1295,7 +1304,7 @@ static int __devinit sh_mmcif_probe(struct platform_device *pdev)
 	struct resource *res;
 	void __iomem *reg;
 
-	if (!pd) {
+	if (!pd && !of_device_is_available(node)) {
 		dev_err(&pdev->dev, "sh_mmcif plat data error.\n");
 		return -ENXIO;
 	}
@@ -1331,11 +1340,13 @@ static int __devinit sh_mmcif_probe(struct platform_device *pdev)
 
 	spin_lock_init(&host->lock);
 
+	mmc_of_get(mmc);
+
 	mmc->ops = &sh_mmcif_ops;
 	sh_mmcif_init_ocr(host);
 
-	mmc->caps = MMC_CAP_MMC_HIGHSPEED;
-	if (pd->caps)
+	mmc->caps |= MMC_CAP_MMC_HIGHSPEED;
+	if (pd && pd->caps)
 		mmc->caps |= pd->caps;
 	mmc->max_segs = 32;
 	mmc->max_blk_size = 512;
@@ -1400,6 +1411,7 @@ eresume:
 	clk_put(host->hclk);
 eclkget:
 	pm_runtime_disable(&pdev->dev);
+	mmc_of_put(mmc);
 	mmc_free_host(mmc);
 ealloch:
 	iounmap(reg);
@@ -1412,6 +1424,7 @@ static int __devexit sh_mmcif_remove(struct platform_device *pdev)
 	int irq[2];
 
 	host->dying = true;
+	mmc_of_put(host->mmc);
 	clk_enable(host->hclk);
 	pm_runtime_get_sync(&pdev->dev);
 
@@ -1469,6 +1482,12 @@ static int sh_mmcif_resume(struct device *dev)
 #define sh_mmcif_resume		NULL
 #endif	/* CONFIG_PM */
 
+static const struct of_device_id mmcif_of_match[] = {
+	{ .compatible = "renesas,sh-mmcif" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mmcif_of_match);
+
 static const struct dev_pm_ops sh_mmcif_dev_pm_ops = {
 	.suspend = sh_mmcif_suspend,
 	.resume = sh_mmcif_resume,
@@ -1480,6 +1499,8 @@ static struct platform_driver sh_mmcif_driver = {
 	.driver		= {
 		.name	= DRIVER_NAME,
 		.pm	= &sh_mmcif_dev_pm_ops,
+		.owner	= THIS_MODULE,
+		.of_match_table = mmcif_of_match,
 	},
 };
 
-- 
1.7.2.5

[PATCH 28/29] mmc: mmcif: add support for generic GPIO CD polling

[Guennadi Liakhovetski]

The generic MMC OF support can be used to configure GPIO card-detection, use
this ability.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 4fdedcf..295a078 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -54,6 +54,7 @@
 #include <linux/mmc/mmc.h>
 #include <linux/mmc/sdio.h>
 #include <linux/mmc/sh_mmcif.h>
+#include <linux/mmc/slot-gpio.h>
 #include <linux/of.h>
 #include <linux/pagemap.h>
 #include <linux/platform_device.h>
@@ -1006,6 +1007,9 @@ static int sh_mmcif_get_cd(struct mmc_host *mmc)
 {
 	struct sh_mmcif_host *host = mmc_priv(mmc);
 	struct sh_mmcif_plat_data *p = host->pd->dev.platform_data;
+	int ret = mmc_gpio_get_cd(mmc);
+	if (ret >= 0)
+		return ret;
 
 	if (!p || !p->get_cd)
 		return -ENOSYS;
-- 
1.7.2.5

[PATCH 29/29] sh: ecovec: switch MMC power control to regulators

[Guennadi Liakhovetski]

Power on the CN11 and CN12 SD/MMC slots on ecovec is controlled by GPIOs,
which makes it possible to use the fixed voltage regulator driver to switch
card power on and off.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Paul Mundt <lethal@linux-sh.org>
---

Optional, mainly FYI

 arch/sh/boards/mach-ecovec24/setup.c |   85 ++++++++++++++++++++++++---------
 1 files changed, 62 insertions(+), 23 deletions(-)

diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
index a2f70e6..0822931 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -19,6 +19,9 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/delay.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/fixed.h>
+#include <linux/regulator/machine.h>
 #include <linux/usb/r8a66597.h>
 #include <linux/usb/renesas_usbhs.h>
 #include <linux/i2c.h>
@@ -515,12 +518,66 @@ static struct i2c_board_info ts_i2c_clients = {
 	.irq		= IRQ0,
 };
 
+static struct regulator_consumer_supply cn12_power_consumers[] +{
+	REGULATOR_SUPPLY("vmmc", "sh_mmcif.0"),
+	REGULATOR_SUPPLY("vmmc", "sh_mobile_sdhi.1"),
+};
+
+static struct regulator_init_data cn12_power_init_data = {
+	.constraints = {
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies  = ARRAY_SIZE(cn12_power_consumers),
+	.consumer_supplies      = cn12_power_consumers,
+};
+
+static struct fixed_voltage_config cn12_power_info = {
+	.supply_name = "CN12 SD/MMC Vdd",
+	.microvolts = 3300000,
+	.gpio = GPIO_PTB7,
+	.enable_high = 1,
+	.init_data = &cn12_power_init_data,
+};
+
+static struct platform_device cn12_power = {
+	.name = "reg-fixed-voltage",
+	.id   = 0,
+	.dev  = {
+		.platform_data = &cn12_power_info,
+	},
+};
+
 #if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
 /* SDHI0 */
-static void sdhi0_set_pwr(struct platform_device *pdev, int state)
+static struct regulator_consumer_supply sdhi0_power_consumers[]  {
-	gpio_set_value(GPIO_PTB6, state);
-}
+	REGULATOR_SUPPLY("vmmc", "sh_mobile_sdhi.0"),
+};
+
+static struct regulator_init_data sdhi0_power_init_data = {
+	.constraints = {
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies  = ARRAY_SIZE(sdhi0_power_consumers),
+	.consumer_supplies      = sdhi0_power_consumers,
+};
+
+static struct fixed_voltage_config sdhi0_power_info = {
+	.supply_name = "CN11 SD/MMC Vdd",
+	.microvolts = 3300000,
+	.gpio = GPIO_PTB6,
+	.enable_high = 1,
+	.init_data = &sdhi0_power_init_data,
+};
+
+static struct platform_device sdhi0_power = {
+	.name = "reg-fixed-voltage",
+	.id   = 1,
+	.dev  = {
+		.platform_data = &sdhi0_power_info,
+	},
+};
 
 static int sdhi0_get_cd(struct platform_device *pdev)
 {
@@ -530,7 +587,6 @@ static int sdhi0_get_cd(struct platform_device *pdev)
 static struct sh_mobile_sdhi_info sdhi0_info = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
-	.set_pwr	= sdhi0_set_pwr,
 	.tmio_caps      = MMC_CAP_SDIO_IRQ | MMC_CAP_POWER_OFF_CARD |
 			  MMC_CAP_NEEDS_POLL,
 	.get_cd		= sdhi0_get_cd,
@@ -561,11 +617,6 @@ static struct platform_device sdhi0_device = {
 
 #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
 /* SDHI1 */
-static void sdhi1_set_pwr(struct platform_device *pdev, int state)
-{
-	gpio_set_value(GPIO_PTB7, state);
-}
-
 static int sdhi1_get_cd(struct platform_device *pdev)
 {
 	return !gpio_get_value(GPIO_PTW7);
@@ -576,7 +627,6 @@ static struct sh_mobile_sdhi_info sdhi1_info = {
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
 	.tmio_caps      = MMC_CAP_SDIO_IRQ | MMC_CAP_POWER_OFF_CARD |
 			  MMC_CAP_NEEDS_POLL,
-	.set_pwr	= sdhi1_set_pwr,
 	.get_cd		= sdhi1_get_cd,
 };
 
@@ -872,11 +922,6 @@ static struct platform_device vou_device = {
 
 #if defined(CONFIG_MMC_SH_MMCIF) || defined(CONFIG_MMC_SH_MMCIF_MODULE)
 /* SH_MMCIF */
-static void mmcif_set_pwr(struct platform_device *pdev, int state)
-{
-	gpio_set_value(GPIO_PTB7, state);
-}
-
 static struct resource sh_mmcif_resources[] = {
 	[0] = {
 		.name	= "SH_MMCIF",
@@ -897,12 +942,10 @@ static struct resource sh_mmcif_resources[] = {
 };
 
 static struct sh_mmcif_plat_data sh_mmcif_plat = {
-	.set_pwr	= mmcif_set_pwr,
 	.sup_pclk	= 0, /* SH7724: Max Pclk/2 */
 	.caps		= MMC_CAP_4_BIT_DATA |
 			  MMC_CAP_8_BIT_DATA |
 			  MMC_CAP_NEEDS_POLL,
-	.ocr		= MMC_VDD_32_33 | MMC_VDD_33_34,
 };
 
 static struct platform_device sh_mmcif_device = {
@@ -927,7 +970,9 @@ static struct platform_device *ecovec_devices[] __initdata = {
 	&ceu0_device,
 	&ceu1_device,
 	&keysc_device,
+	&cn12_power,
 #if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
+	&sdhi0_power,
 	&sdhi0_device,
 #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
 	&sdhi1_device,
@@ -1224,8 +1269,6 @@ static int __init arch_setup(void)
 	gpio_request(GPIO_FN_SDHI0D2,  NULL);
 	gpio_request(GPIO_FN_SDHI0D1,  NULL);
 	gpio_request(GPIO_FN_SDHI0D0,  NULL);
-	gpio_request(GPIO_PTB6, NULL);
-	gpio_direction_output(GPIO_PTB6, 0);
 #else
 	/* enable MSIOF0 on CN11 (needs DS2.4 set to OFF) */
 	gpio_request(GPIO_FN_MSIOF0_TXD, NULL);
@@ -1254,8 +1297,6 @@ static int __init arch_setup(void)
 	gpio_request(GPIO_FN_MMC_D0, NULL);
 	gpio_request(GPIO_FN_MMC_CLK, NULL);
 	gpio_request(GPIO_FN_MMC_CMD, NULL);
-	gpio_request(GPIO_PTB7, NULL);
-	gpio_direction_output(GPIO_PTB7, 0);
 
 	cn12_enabled = true;
 #elif defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
@@ -1267,8 +1308,6 @@ static int __init arch_setup(void)
 	gpio_request(GPIO_FN_SDHI1D2,  NULL);
 	gpio_request(GPIO_FN_SDHI1D1,  NULL);
 	gpio_request(GPIO_FN_SDHI1D0,  NULL);
-	gpio_request(GPIO_PTB7, NULL);
-	gpio_direction_output(GPIO_PTB7, 0);
 
 	/* Card-detect, used on CN12 with SDHI1 */
 	gpio_request(GPIO_PTW7, NULL);
-- 
1.7.2.5