[PATCH 00/11] mmc: tmio/sdhi: hotplug & PM

[PATCH 00/11] mmc: tmio/sdhi: hotplug & PM

[Guennadi Liakhovetski]

This patch series uses a previously proposed generic GPIO card hotplug 
handler, fixes multiple PM issues, adds some cosmetic clean up. Not all of 
these patches really depend on each other, but it's easier to preserve the 
original order. Some patches, e.g., #9, are an RFC so far and might have to 
be reworked a bit.

Guennadi Liakhovetski (11):
  mmc: tmio: calculate the native hotplug condition only once
  mmc: tmio_mmc: support the generic MMC GPIO card hotplug helper
  mmc: sh_mobile_sdhi: pass card hotplug GPIO number and flags to TMIO
    MMC
  ARM: mach-shmobile: convert mackerel to use the generic MMC GPIO
    hotplug helper
  ARM: mach-shmobile: convert ag5evm to use the generic MMC GPIO
    hotplug helper
  mfd: tmio: remove now unneeded tmio_mmc_cd_wakeup()
  mmc: tmio_mmc: power status flag doesn't have to be exposed in
    platform data
  mmc: tmio_mmc: remove unused sdio_irq_enabled flag
  mmc: sh_mobile_sdhi: do not manage PM clocks manually on ARM
  mmc: tmio: reconfigure the controller on runtime resume
  mmc: tmio: cosmetic: prettify the tmio_mmc_set_ios() function

 arch/arm/mach-shmobile/board-ag5evm.c   |   24 +------
 arch/arm/mach-shmobile/board-mackerel.c |   24 +------
 drivers/mmc/host/sh_mobile_sdhi.c       |   11 +++-
 drivers/mmc/host/tmio_mmc.h             |    9 +--
 drivers/mmc/host/tmio_mmc_pio.c         |  104 ++++++++++++++-----------------
 include/linux/mfd/tmio.h                |   17 ++---
 include/linux/mmc/sh_mobile_sdhi.h      |    3 +
 7 files changed, 79 insertions(+), 113 deletions(-)

-- 
1.7.2.5

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

[PATCH 01/11] mmc: tmio: calculate the native hotplug condition only once

[Guennadi Liakhovetski]

The condition, whether we have to use the native TMIO card hotplug
detection interrupt, is rather complex, it is better to only calculate it
once and store in the private data.

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

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index a95e6d9..9aa6a18 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -86,6 +86,7 @@ struct tmio_mmc_host {
 	spinlock_t		lock;		/* protect host private data */
 	unsigned long		last_req_ts;
 	struct mutex		ios_lock;	/* protect set_ios() context */
+	bool			native_hotplug;
 };
 
 int tmio_mmc_host_probe(struct tmio_mmc_host **host,
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index abad01b..1a9e975 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -908,6 +908,9 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	else
 		mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
 
+	_host->native_hotplug = !(pdata->flags & TMIO_MMC_HAS_COLD_CD ||
+				  mmc->caps & MMC_CAP_NEEDS_POLL ||
+				  mmc->caps & MMC_CAP_NONREMOVABLE);
 	pdata->power = false;
 	pm_runtime_enable(&pdev->dev);
 	ret = pm_runtime_resume(&pdev->dev);
@@ -926,9 +929,7 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	 *  additionally ensure that in case 2) the tmio mmc hardware stays
 	 *  powered on during runtime for the card detection to work.
 	 */
-	if (!(pdata->flags & TMIO_MMC_HAS_COLD_CD
-		|| mmc->caps & MMC_CAP_NEEDS_POLL
-		|| mmc->caps & MMC_CAP_NONREMOVABLE))
+	if (_host->native_hotplug)
 		pm_runtime_get_noresume(&pdev->dev);
 
 	tmio_mmc_clk_stop(_host);
@@ -984,9 +985,7 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host)
 	 * the controller, the runtime PM is suspended and pdata->power = false,
 	 * so, our .runtime_resume() will not try to detect a card in the slot.
 	 */
-	if (host->pdata->flags & TMIO_MMC_HAS_COLD_CD
-		|| host->mmc->caps & MMC_CAP_NEEDS_POLL
-		|| host->mmc->caps & MMC_CAP_NONREMOVABLE)
+	if (!host->native_hotplug)
 		pm_runtime_get_sync(&pdev->dev);
 
 	mmc_remove_host(host->mmc);
-- 
1.7.2.5

[PATCH 02/11] mmc: tmio_mmc: support the generic MMC GPIO card hotplug helper

[Guennadi Liakhovetski]

If the platform specified a GPIO number and IRQ trigger polarity flags, use
the generic MMC GPIO card hotplug helper.

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

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 9aa6a18..9fb6e6b 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -53,10 +53,6 @@ struct tmio_mmc_host {
 	void (*set_pwr)(struct platform_device *host, int state);
 	void (*set_clk_div)(struct platform_device *host, int state);
 
-	int			pm_error;
-	/* recognise system-wide suspend in runtime PM methods */
-	bool			pm_global;
-
 	/* 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 1a9e975..41b7eff 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -34,6 +34,7 @@
 #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/tmio.h>
 #include <linux/module.h>
@@ -783,8 +784,10 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	spin_unlock_irqrestore(&host->lock, flags);
 
 	/*
-	 * pdata->power = false only if COLD_CD is available, otherwise only
-	 * in short time intervals during probing or resuming
+	 * pdata->power toggles between false and true in both cases - either
+	 * or not the controller can be runtime-suspended during inactivity. But
+	 * if the controller has to be kept on, the runtime-pm usage_count is
+	 * kept positive, so no suspending actually takes place.
 	 */
 	if (ios->power_mode = MMC_POWER_ON && ios->clock) {
 		if (!pdata->power) {
@@ -924,9 +927,9 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	 *  3) a worker thread polls the sdhi - indicated by MMC_CAP_NEEDS_POLL
 	 *  4) the medium is non-removable - indicated by MMC_CAP_NONREMOVABLE
 	 *
-	 *  While we increment the rtpm 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
+	 *  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
 	 *  powered on during runtime for the card detection to work.
 	 */
 	if (_host->native_hotplug)
@@ -960,6 +963,16 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 
 	tmio_mmc_enable_mmc_irqs(_host, irq_mask);
 
+	if (pdata->flags & TMIO_MMC_HAS_COLD_CD &&
+	    pdata->cd_flags & IRQF_TRIGGER_MASK) {
+		ret = mmc_cd_gpio_request(mmc, pdata->cd_gpio, pdata->cd_irq,
+					  pdata->cd_flags);
+		if (ret < 0) {
+			tmio_mmc_host_remove(_host);
+			return ret;
+		}
+	}
+
 	*host = _host;
 
 	return 0;
@@ -977,18 +990,21 @@ EXPORT_SYMBOL(tmio_mmc_host_probe);
 void tmio_mmc_host_remove(struct tmio_mmc_host *host)
 {
 	struct platform_device *pdev = host->pdev;
+	struct tmio_mmc_data *pdata = host->pdata;
+	struct mmc_host *mmc = host->mmc;
+
+	if (pdata->flags & TMIO_MMC_HAS_COLD_CD &&
+	    pdata->cd_flags & IRQF_TRIGGER_MASK)
+		/*
+		 * 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);
 
-	/*
-	 * We don't have to manipulate pdata->power here: if there is a card in
-	 * the slot, the runtime PM is active and our .runtime_resume() will not
-	 * be run. If there is no card in the slot and the platform can suspend
-	 * the controller, the runtime PM is suspended and pdata->power = false,
-	 * so, our .runtime_resume() will not try to detect a card in the slot.
-	 */
 	if (!host->native_hotplug)
 		pm_runtime_get_sync(&pdev->dev);
 
-	mmc_remove_host(host->mmc);
+	mmc_remove_host(mmc);
 	cancel_work_sync(&host->done);
 	cancel_delayed_work_sync(&host->delayed_reset_work);
 	tmio_mmc_release_dma(host);
@@ -997,7 +1013,7 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host)
 	pm_runtime_disable(&pdev->dev);
 
 	iounmap(host->ctl);
-	mmc_free_host(host->mmc);
+	mmc_free_host(mmc);
 }
 EXPORT_SYMBOL(tmio_mmc_host_remove);
 
@@ -1011,8 +1027,6 @@ int tmio_mmc_host_suspend(struct device *dev)
 	if (!ret)
 		tmio_mmc_disable_mmc_irqs(host, TMIO_MASK_ALL);
 
-	host->pm_error = pm_runtime_put_sync(dev);
-
 	return ret;
 }
 EXPORT_SYMBOL(tmio_mmc_host_suspend);
@@ -1020,22 +1034,8 @@ EXPORT_SYMBOL(tmio_mmc_host_suspend);
 int tmio_mmc_host_resume(struct device *dev)
 {
 	struct mmc_host *mmc = dev_get_drvdata(dev);
-	struct tmio_mmc_host *host = mmc_priv(mmc);
 
 	/* The MMC core will perform the complete set up */
-	host->pdata->power = false;
-
-	host->pm_global = true;
-	if (!host->pm_error)
-		pm_runtime_get_sync(dev);
-
-	if (host->pm_global) {
-		/* Runtime PM resume callback didn't run */
-		tmio_mmc_reset(host);
-		tmio_mmc_enable_dma(host, true);
-		host->pm_global = false;
-	}
-
 	return mmc_resume_host(mmc);
 }
 EXPORT_SYMBOL(tmio_mmc_host_resume);
@@ -1052,19 +1052,10 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
 {
 	struct mmc_host *mmc = dev_get_drvdata(dev);
 	struct tmio_mmc_host *host = mmc_priv(mmc);
-	struct tmio_mmc_data *pdata = host->pdata;
 
 	tmio_mmc_reset(host);
 	tmio_mmc_enable_dma(host, true);
 
-	if (pdata->power) {
-		/* Only entered after a card-insert interrupt */
-		if (!mmc->card)
-			tmio_mmc_set_ios(mmc, &mmc->ios);
-		mmc_detect_change(mmc, msecs_to_jiffies(100));
-	}
-	host->pm_global = false;
-
 	return 0;
 }
 EXPORT_SYMBOL(tmio_mmc_host_runtime_resume);
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 0dc9804..4ef9307 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -1,8 +1,10 @@
 #ifndef MFD_TMIO_H
 #define MFD_TMIO_H
 
+#include <linux/device.h>
 #include <linux/fb.h>
 #include <linux/io.h>
+#include <linux/jiffies.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 
@@ -64,8 +66,8 @@
 #define TMIO_MMC_SDIO_IRQ		(1 << 2)
 /*
  * Some platforms can detect card insertion events with controller powered
- * down, in which case they have to call tmio_mmc_cd_wakeup() to power up the
- * controller and report the event to the driver.
+ * down, using a GPIO IRQ, in which case they have to fill in cd_irq, cd_gpio,
+ * and cd_flags fields of struct tmio_mmc_data.
  */
 #define TMIO_MMC_HAS_COLD_CD		(1 << 3)
 /*
@@ -98,18 +100,23 @@ struct tmio_mmc_data {
 	struct tmio_mmc_dma		*dma;
 	struct device			*dev;
 	bool				power;
+	unsigned int			cd_gpio;
+	unsigned int			cd_irq;
+	unsigned long			cd_flags;
 	void (*set_pwr)(struct platform_device *host, int state);
 	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);
 };
 
+/*
+ * This function is deprecated and will be removed soon. Please, convert your
+ * platform to use drivers/mmc/core/cd-gpio.c
+ */
 static inline void tmio_mmc_cd_wakeup(struct tmio_mmc_data *pdata)
 {
-	if (pdata && !pdata->power) {
-		pdata->power = true;
-		pm_runtime_get(pdata->dev);
-	}
+	if (pdata)
+		mmc_detect_change(dev_get_drvdata(dev), msecs_to_jiffies(100));
 }
 
 /*
-- 
1.7.2.5

Re: [PATCH 02/11] mmc: tmio_mmc: support the generic MMC GPIO card

[Magnus Damm]

On Wed, Jan 4, 2012 at 11:17 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> If the platform specified a GPIO number and IRQ trigger polarity flags, use
> the generic MMC GPIO card hotplug helper.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  drivers/mmc/host/tmio_mmc.h     |    4 --
>  drivers/mmc/host/tmio_mmc_pio.c |   69 +++++++++++++++++----------------------
>  include/linux/mfd/tmio.h        |   19 +++++++---
>  3 files changed, 43 insertions(+), 49 deletions(-)
> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> index 0dc9804..4ef9307 100644
> --- a/include/linux/mfd/tmio.h
> +++ b/include/linux/mfd/tmio.h
> @@ -1,8 +1,10 @@
>  #ifndef MFD_TMIO_H
>  #define MFD_TMIO_H
>
> +#include <linux/device.h>
>  #include <linux/fb.h>
>  #include <linux/io.h>
> +#include <linux/jiffies.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>
> @@ -64,8 +66,8 @@
>  #define TMIO_MMC_SDIO_IRQ              (1 << 2)
>  /*
>  * Some platforms can detect card insertion events with controller powered
> - * down, in which case they have to call tmio_mmc_cd_wakeup() to power up the
> - * controller and report the event to the driver.
> + * down, using a GPIO IRQ, in which case they have to fill in cd_irq, cd_gpio,
> + * and cd_flags fields of struct tmio_mmc_data.
>  */
>  #define TMIO_MMC_HAS_COLD_CD           (1 << 3)
>  /*
> @@ -98,18 +100,23 @@ struct tmio_mmc_data {
>        struct tmio_mmc_dma             *dma;
>        struct device                   *dev;
>        bool                            power;
> +       unsigned int                    cd_gpio;
> +       unsigned int                    cd_irq;
> +       unsigned long                   cd_flags;
>        void (*set_pwr)(struct platform_device *host, int state);
>        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);
>  };

This looks much easier than callbacks and notifiers. Thanks!

But why do you need to pass three values here? I expected it to be
sufficient with GPIO and perhaps also with trigger flags. Shouldn't
gpio_to_irq() give you the IRQ from the GPIO? And why on earth would
we want to trigger on something else than both edges?

The SoC code may me rather limited in terms of supporting
gpio_to_irq() but that needs to be fixed then. On SH-Mobile the PFC
code has been updated to allow this, so it's only SoC-specific code
missing. Anyone with access to hardware and data sheet can do this,
and if there are some issues with this then I'd be happy to help
myself.

On the cosmetic side, wouldn't it make sense to have a structure of
these values and just pass that along? Otherwise people will mix and
match short and ints for the GPIO and it may become rather messy over
time.

Thanks,

/ magnus

Re: [PATCH 02/11] mmc: tmio_mmc: support the generic MMC GPIO card

[Guennadi Liakhovetski]

Hi Magnus

Thanks for your review(s)!

On Fri, 6 Jan 2012, Magnus Damm wrote:

> On Wed, Jan 4, 2012 at 11:17 PM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > If the platform specified a GPIO number and IRQ trigger polarity flags, use
> > the generic MMC GPIO card hotplug helper.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >  drivers/mmc/host/tmio_mmc.h     |    4 --
> >  drivers/mmc/host/tmio_mmc_pio.c |   69 +++++++++++++++++----------------------
> >  include/linux/mfd/tmio.h        |   19 +++++++---
> >  3 files changed, 43 insertions(+), 49 deletions(-)
> > diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> > index 0dc9804..4ef9307 100644
> > --- a/include/linux/mfd/tmio.h
> > +++ b/include/linux/mfd/tmio.h
> > @@ -1,8 +1,10 @@
> >  #ifndef MFD_TMIO_H
> >  #define MFD_TMIO_H
> >
> > +#include <linux/device.h>
> >  #include <linux/fb.h>
> >  #include <linux/io.h>
> > +#include <linux/jiffies.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_runtime.h>
> >
> > @@ -64,8 +66,8 @@
> >  #define TMIO_MMC_SDIO_IRQ              (1 << 2)
> >  /*
> >  * Some platforms can detect card insertion events with controller powered
> > - * down, in which case they have to call tmio_mmc_cd_wakeup() to power up the
> > - * controller and report the event to the driver.
> > + * down, using a GPIO IRQ, in which case they have to fill in cd_irq, cd_gpio,
> > + * and cd_flags fields of struct tmio_mmc_data.
> >  */
> >  #define TMIO_MMC_HAS_COLD_CD           (1 << 3)
> >  /*
> > @@ -98,18 +100,23 @@ struct tmio_mmc_data {
> >        struct tmio_mmc_dma             *dma;
> >        struct device                   *dev;
> >        bool                            power;
> > +       unsigned int                    cd_gpio;
> > +       unsigned int                    cd_irq;
> > +       unsigned long                   cd_flags;
> >        void (*set_pwr)(struct platform_device *host, int state);
> >        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);
> >  };
> 
> This looks much easier than callbacks and notifiers. Thanks!
> 
> But why do you need to pass three values here? I expected it to be
> sufficient with GPIO and perhaps also with trigger flags. Shouldn't
> gpio_to_irq() give you the IRQ from the GPIO?

Well, it could:-) What about sh7372? sh73a0 does implement it, whereas 
sh7372 only returns an error. And - yes, we can fix that one. But this 
tells me - there can be other platforms like that, and they might not get 
fixed quickly enough. Should we really refuse to work with them? We could 
make this parameter optional: set to to an error code and the cd-gpio will 
try to use gpio_to_irq(). If it fails - no luck.

> And why on earth would
> we want to trigger on something else than both edges?

Both edges seems like a logical choice to me too. But - remember the time, 
when one of sh-mobile platforms didn't implement triggering on both edges? 
Until we've implemented a software workaround, whereby we switch to the 
opposite edge after each interrupt? Are you sure there are no such 
platforms around left?

> The SoC code may me rather limited in terms of supporting
> gpio_to_irq() but that needs to be fixed then. On SH-Mobile the PFC
> code has been updated to allow this, so it's only SoC-specific code
> missing. Anyone with access to hardware and data sheet can do this,
> and if there are some issues with this then I'd be happy to help
> myself.
> 
> On the cosmetic side, wouldn't it make sense to have a structure of
> these values and just pass that along? Otherwise people will mix and
> match short and ints for the GPIO and it may become rather messy over
> time.

Short for GPIO? Why would anyone use that and how can that become 
messy?:-) In principle - I'm basically fine either way. If you really 
prefer a struct here - we can do that too, yes.

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

[PATCH 03/11] mmc: sh_mobile_sdhi: pass card hotplug GPIO number and flags to TMIO MMC

[Guennadi Liakhovetski]

To use TMIO MMC driver ability to interface to the generic MMC GPIO card
hotplug detection helper, the SDHI driver has to pass the GPIO number and
IRQ flags down from its own platform data.

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

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 58da3c4..a8d353e 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -129,6 +129,9 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 			mmc_data->write16_hook = sh_mobile_sdhi_write16_hook;
 		mmc_data->ocr_mask = p->tmio_ocr_mask;
 		mmc_data->capabilities |= p->tmio_caps;
+		mmc_data->cd_gpio = p->cd_gpio;
+		mmc_data->cd_irq = p->cd_irq;
+		mmc_data->cd_flags = p->cd_flags;
 
 		if (p->dma_slave_tx > 0 && p->dma_slave_rx > 0) {
 			priv->param_tx.slave_id = p->dma_slave_tx;
@@ -211,7 +214,7 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 
 	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
 		 mmc_hostname(host->mmc), (unsigned long)
-		 (platform_get_resource(pdev,IORESOURCE_MEM, 0)->start),
+		 (platform_get_resource(pdev, IORESOURCE_MEM, 0)->start),
 		 mmc_data->hclk / 1000000);
 
 	return ret;
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index 71b8054..86347f6 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -16,6 +16,9 @@ struct sh_mobile_sdhi_info {
 	unsigned long tmio_flags;
 	unsigned long tmio_caps;
 	u32 tmio_ocr_mask;	/* available MMC voltages */
+	unsigned int cd_gpio;
+	unsigned int cd_irq;
+	unsigned long cd_flags;
 	struct tmio_mmc_data *pdata;
 	void (*set_pwr)(struct platform_device *pdev, int state);
 	int (*get_cd)(struct platform_device *pdev);
-- 
1.7.2.5

[PATCH 04/11] ARM: mach-shmobile: convert mackerel to use the generic MMC GPIO hotplug helper

[Guennadi Liakhovetski]

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 arch/arm/mach-shmobile/board-mackerel.c |   24 ++++--------------------
 1 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 9c5e598..b35343a 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1004,21 +1004,14 @@ static int slot_cn7_get_cd(struct platform_device *pdev)
 }
 
 /* SDHI0 */
-static irqreturn_t mackerel_sdhi0_gpio_cd(int irq, void *arg)
-{
-	struct device *dev = arg;
-	struct sh_mobile_sdhi_info *info = dev->platform_data;
-	struct tmio_mmc_data *pdata = info->pdata;
-
-	tmio_mmc_cd_wakeup(pdata);
-
-	return IRQ_HANDLED;
-}
-
 static struct sh_mobile_sdhi_info sdhi0_info = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.tmio_flags	= TMIO_MMC_HAS_COLD_CD,
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
+	.cd_gpio	= GPIO_PORT172,
+	.cd_irq		= evt2irq(0x3340),
+	.cd_flags	= IRQF_TRIGGER_FALLING,
 };
 
 static struct resource sdhi0_resources[] = {
@@ -1408,7 +1401,6 @@ static void __init mackerel_init(void)
 {
 	u32 srcr4;
 	struct clk *clk;
-	int ret;
 
 	sh7372_pinmux_init();
 
@@ -1505,7 +1497,6 @@ static void __init mackerel_init(void)
 	irq_set_irq_type(IRQ21, IRQ_TYPE_LEVEL_HIGH);
 
 	/* enable SDHI0 */
-	gpio_request(GPIO_FN_SDHICD0, NULL);
 	gpio_request(GPIO_FN_SDHIWP0, NULL);
 	gpio_request(GPIO_FN_SDHICMD0, NULL);
 	gpio_request(GPIO_FN_SDHICLK0, NULL);
@@ -1514,13 +1505,6 @@ static void __init mackerel_init(void)
 	gpio_request(GPIO_FN_SDHID0_1, NULL);
 	gpio_request(GPIO_FN_SDHID0_0, NULL);
 
-	ret = request_irq(evt2irq(0x3340), mackerel_sdhi0_gpio_cd,
-			  IRQF_TRIGGER_FALLING, "sdhi0 cd", &sdhi0_device.dev);
-	if (!ret)
-		sdhi0_info.tmio_flags |= TMIO_MMC_HAS_COLD_CD;
-	else
-		pr_err("Cannot get IRQ #%d: %d\n", evt2irq(0x3340), ret);
-
 #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
 	/* enable SDHI1 */
 	gpio_request(GPIO_FN_SDHICMD1, NULL);
-- 
1.7.2.5

[PATCH 05/11] ARM: mach-shmobile: convert ag5evm to use the generic MMC GPIO hotplug helper

[Guennadi Liakhovetski]

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

Only compile-tested.

 arch/arm/mach-shmobile/board-ag5evm.c |   24 ++++--------------------
 1 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index b862e9f..b1db188 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -340,23 +340,15 @@ static struct platform_device mipidsi0_device = {
 };
 
 /* SDHI0 */
-static irqreturn_t ag5evm_sdhi0_gpio_cd(int irq, void *arg)
-{
-	struct device *dev = arg;
-	struct sh_mobile_sdhi_info *info = dev->platform_data;
-	struct tmio_mmc_data *pdata = info->pdata;
-
-	tmio_mmc_cd_wakeup(pdata);
-
-	return IRQ_HANDLED;
-}
-
 static struct sh_mobile_sdhi_info sdhi0_info = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
-	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT,
+	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_HAS_COLD_CD,
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED,
 	.tmio_ocr_mask	= MMC_VDD_27_28 | MMC_VDD_28_29,
+	.cd_gpio	= GPIO_PORT251,
+	.cd_irq		= intcs_evt2irq(0x3c0),
+	.cd_flags	= IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
 };
 
 static struct resource sdhi0_resources[] = {
@@ -560,7 +552,6 @@ static void __init ag5evm_init(void)
 	__raw_writel(0x2a809010, DSI0PHYCR);
 
 	/* enable SDHI0 on CN15 [SD I/F] */
-	gpio_request(GPIO_FN_SDHICD0, NULL);
 	gpio_request(GPIO_FN_SDHIWP0, NULL);
 	gpio_request(GPIO_FN_SDHICMD0, NULL);
 	gpio_request(GPIO_FN_SDHICLK0, NULL);
@@ -569,13 +560,6 @@ static void __init ag5evm_init(void)
 	gpio_request(GPIO_FN_SDHID0_1, NULL);
 	gpio_request(GPIO_FN_SDHID0_0, NULL);
 
-	if (!request_irq(intcs_evt2irq(0x3c0), ag5evm_sdhi0_gpio_cd,
-			 IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
-			 "sdhi0 cd", &sdhi0_device.dev))
-		sdhi0_info.tmio_flags |= TMIO_MMC_HAS_COLD_CD;
-	else
-		pr_warn("Unable to setup SDHI0 GPIO IRQ\n");
-
 	/* enable SDHI1 on CN4 [WLAN I/F] */
 	gpio_request(GPIO_FN_SDHICLK1, NULL);
 	gpio_request(GPIO_FN_SDHICMD1_PU, NULL);
-- 
1.7.2.5

[PATCH 06/11] mfd: tmio: remove now unneeded tmio_mmc_cd_wakeup()

[Guennadi Liakhovetski]

Now, that all users of tmio_mmc_cd_wakeup() have been converted over to
drivers/mmc/core/cd-gpio.c, that function can be removed.

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

diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 4ef9307..08fb7d2 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -4,7 +4,6 @@
 #include <linux/device.h>
 #include <linux/fb.h>
 #include <linux/io.h>
-#include <linux/jiffies.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 
@@ -110,16 +109,6 @@ struct tmio_mmc_data {
 };
 
 /*
- * This function is deprecated and will be removed soon. Please, convert your
- * platform to use drivers/mmc/core/cd-gpio.c
- */
-static inline void tmio_mmc_cd_wakeup(struct tmio_mmc_data *pdata)
-{
-	if (pdata)
-		mmc_detect_change(dev_get_drvdata(dev), msecs_to_jiffies(100));
-}
-
-/*
  * data for the NAND controller
  */
 struct tmio_nand_data {
-- 
1.7.2.5

Re: [PATCH 06/11] mfd: tmio: remove now unneeded tmio_mmc_cd_wakeup()

[Samuel Ortiz]

Hi Guennadi,

On Wed, Jan 04, 2012 at 03:17:11PM +0100, Guennadi Liakhovetski wrote:
> Now, that all users of tmio_mmc_cd_wakeup() have been converted over to
> drivers/mmc/core/cd-gpio.c, that function can be removed.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Acked-by: Samuel Ortiz <sameo@linux.intel.com>

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 06/11] mfd: tmio: remove now unneeded tmio_mmc_cd_wakeup()

[Magnus Damm]

On Wed, Jan 4, 2012 at 11:17 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Now, that all users of tmio_mmc_cd_wakeup() have been converted over to
> drivers/mmc/core/cd-gpio.c, that function can be removed.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  include/linux/mfd/tmio.h |   11 -----------
>  1 files changed, 0 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> index 4ef9307..08fb7d2 100644
> --- a/include/linux/mfd/tmio.h
> +++ b/include/linux/mfd/tmio.h
> @@ -4,7 +4,6 @@
>  #include <linux/device.h>
>  #include <linux/fb.h>
>  #include <linux/io.h>
> -#include <linux/jiffies.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>
> @@ -110,16 +109,6 @@ struct tmio_mmc_data {
>  };
>
>  /*
> - * This function is deprecated and will be removed soon. Please, convert your
> - * platform to use drivers/mmc/core/cd-gpio.c
> - */
> -static inline void tmio_mmc_cd_wakeup(struct tmio_mmc_data *pdata)
> -{
> -       if (pdata)
> -               mmc_detect_change(dev_get_drvdata(dev), msecs_to_jiffies(100));
> -}
> -
> -/*

Ok, I can clearly see the merit of removing unused code. And I do
appreciate that you clean up the code. Thank you!

The only issue is that we do however have tons of users of the SDHI
driver (just grep in arch/arm/mach-shmobile and in arch/sh) and many
of them can use other pins than the SDHI CD function for hotplug
detection. To support such configurations we need to have some way to
notify the driver instance about the hotplug change. This patch is
removing just that.

I do realize that you recently posted some generic MMC GPIO hotplug
patches, and I am happy that you agreed with me that it is better to
put it in the generic MMC core than pushing it in the SDHI or MMCIF
driver. So this development seems like a perfect fit for both the SDHI
driver and the MMCIF driver.

I'd like to see platforms move over to your MMC GPIO hotplug helpers,
but at the same time I want to keep the existing notification method
to make a smooth transition. I personally added GPIO IRQ support
rather recently to SH Mobile ARM, but it is still rather early and we
do not have many users yet. I have seen customer-specific hardware
designs using non-GPIO pins for CD handling too, and to support such
hardware we want to keep flexible notification methods. So with that
in mind I'd like to keep the options open and allow fully developed
platforms to use your MMC GPIO hotplug code, but also allow custom
platforms and boards/SoCs with more limited software support handle
hotplug notification the old way.

So please don't remove this function at this point. Give us time to move over.

Thanks,

/ magnus

[PATCH 07/11] mmc: tmio_mmc: power status flag doesn't have to be exposed in platform data

[Guennadi Liakhovetski]

The controller power status flag does not have to be accessed from the
hot-plug detection code any more, it can now be removed from the platform
data and put in the controller private struct.

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

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 9fb6e6b..0c39002 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -49,6 +49,9 @@ struct tmio_mmc_host {
 	struct mmc_host         *mmc;
 	unsigned int		sdio_irq_enabled;
 
+	/* Controller power state */
+	bool			power;
+
 	/* Callbacks for clock / power control */
 	void (*set_pwr)(struct platform_device *host, int state);
 	void (*set_clk_div)(struct platform_device *host, int state);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 41b7eff..478e57b 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -754,7 +754,6 @@ fail:
 static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct tmio_mmc_host *host = mmc_priv(mmc);
-	struct tmio_mmc_data *pdata = host->pdata;
 	unsigned long flags;
 
 	mutex_lock(&host->ios_lock);
@@ -784,15 +783,15 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	spin_unlock_irqrestore(&host->lock, flags);
 
 	/*
-	 * pdata->power toggles between false and true in both cases - either
+	 * host->power toggles between false and true in both cases - either
 	 * or not the controller can be runtime-suspended during inactivity. But
 	 * if the controller has to be kept on, the runtime-pm usage_count is
 	 * kept positive, so no suspending actually takes place.
 	 */
 	if (ios->power_mode = MMC_POWER_ON && ios->clock) {
-		if (!pdata->power) {
+		if (!host->power) {
 			pm_runtime_get_sync(&host->pdev->dev);
-			pdata->power = true;
+			host->power = true;
 		}
 		tmio_mmc_set_clock(host, ios->clock);
 		/* power up SD bus */
@@ -803,8 +802,8 @@ 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);
-		if (pdata->power) {
-			pdata->power = false;
+		if (host->power) {
+			host->power = false;
 			pm_runtime_put(&host->pdev->dev);
 		}
 		tmio_mmc_clk_stop(host);
@@ -914,7 +913,7 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	_host->native_hotplug = !(pdata->flags & TMIO_MMC_HAS_COLD_CD ||
 				  mmc->caps & MMC_CAP_NEEDS_POLL ||
 				  mmc->caps & MMC_CAP_NONREMOVABLE);
-	pdata->power = false;
+	_host->power = false;
 	pm_runtime_enable(&pdev->dev);
 	ret = pm_runtime_resume(&pdev->dev);
 	if (ret < 0)
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 08fb7d2..367c65c6 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -98,7 +98,6 @@ struct tmio_mmc_data {
 	u32				ocr_mask;	/* available voltages */
 	struct tmio_mmc_dma		*dma;
 	struct device			*dev;
-	bool				power;
 	unsigned int			cd_gpio;
 	unsigned int			cd_irq;
 	unsigned long			cd_flags;
-- 
1.7.2.5

[PATCH 08/11] mmc: tmio_mmc: remove unused sdio_irq_enabled flag

[Guennadi Liakhovetski]

The sdio_irq_enabled member of struct tmio_mmc_host is a left-over from the
previously removed SDIO IRQ workaround. It is no longer needed and can now
be removed too.

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

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 0c39002..62d0d14 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -47,7 +47,6 @@ struct tmio_mmc_host {
 	struct mmc_request      *mrq;
 	struct mmc_data         *data;
 	struct mmc_host         *mmc;
-	unsigned int		sdio_irq_enabled;
 
 	/* Controller power state */
 	bool			power;
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 478e57b..1811006 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -127,7 +127,6 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
 	struct tmio_mmc_host *host = mmc_priv(mmc);
 
 	if (enable) {
-		host->sdio_irq_enabled = 1;
 		host->sdio_irq_mask = TMIO_SDIO_MASK_ALL &
 					~TMIO_SDIO_STAT_IOIRQ;
 		sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001);
@@ -136,7 +135,6 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
 		host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
 		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
 		sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000);
-		host->sdio_irq_enabled = 0;
 	}
 }
 
-- 
1.7.2.5

[PATCH 09/11] mmc: sh_mobile_sdhi: do not manage PM clocks manually on ARM

[Guennadi Liakhovetski]

On ARM the same clock is used by the PM subsystem and by the driver
directly. This leads to the clock staying permanently on, independent of
the runtime PM state. This patch makes clock enable and disable calls in
the driver SuperH-specific.

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

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index a8d353e..0df69ab 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -117,7 +117,9 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 		goto eclkget;
 	}
 
+#if defined(CONFIG_SUPERH)
 	clk_enable(priv->clk);
+#endif
 
 	mmc_data->hclk = clk_get_rate(priv->clk);
 	mmc_data->set_pwr = sh_mobile_sdhi_set_pwr;
@@ -235,7 +237,9 @@ eirq_sdio:
 eirq_card_detect:
 	tmio_mmc_host_remove(host);
 eprobe:
+#if defined(CONFIG_SUPERH)
 	clk_disable(priv->clk);
+#endif
 	clk_put(priv->clk);
 eclkget:
 	kfree(priv);
@@ -261,7 +265,9 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 		free_irq(irq, host);
 	}
 
+#if defined(CONFIG_SUPERH)
 	clk_disable(priv->clk);
+#endif
 	clk_put(priv->clk);
 	kfree(priv);
 
-- 
1.7.2.5

Re: [PATCH 09/11] mmc: sh_mobile_sdhi: do not manage PM clocks

[Magnus Damm]

Hi Guennadi,

On Wed, Jan 4, 2012 at 11:17 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> On ARM the same clock is used by the PM subsystem and by the driver
> directly. This leads to the clock staying permanently on, independent of
> the runtime PM state. This patch makes clock enable and disable calls in
> the driver SuperH-specific.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)

Thanks for your patch.

From my point of view there is very little difference between SH and
SH-Mobile ARM, so I don't understand the special treatment. Also, is
this change really needed for SH with your recent Runtime PM patches?

I believe we need to make use of the clock framework to get the clock
rate configuration in the SDHI driver. At the same time we want to
stop the clock as frequently as possible. I believe we should disable
and enable the clock together with the Runtime PM get/put operations -
pretty much exactly like our I2C master driver does. The only problem
is, while the clock is disabled the clock rate may be changed. So we
need to be able to notify the MMC subsystem about the new clock rate.

Any ideas on how to make this happen?

Thanks,

/ magnus

Re: [PATCH 09/11] mmc: sh_mobile_sdhi: do not manage PM clocks

[Guennadi Liakhovetski]

On Fri, 6 Jan 2012, Magnus Damm wrote:

> Hi Guennadi,
> 
> On Wed, Jan 4, 2012 at 11:17 PM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > On ARM the same clock is used by the PM subsystem and by the driver
> > directly. This leads to the clock staying permanently on, independent of
> > the runtime PM state. This patch makes clock enable and disable calls in
> > the driver SuperH-specific.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >  drivers/mmc/host/sh_mobile_sdhi.c |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> Thanks for your patch.
> 
> >From my point of view there is very little difference between SH and
> SH-Mobile ARM, so I don't understand the special treatment. Also, is
> this change really needed for SH with your recent Runtime PM patches?

I didn't try SuperH. I seem to remember, there were 2 clocks per SDHI 
controller on some platforms, of which one was handled by runtime PM and 
the other one directly. However, a quick look through SDHI users didn't 
reveal any such cases, so, maybe I'm confusing SDHI with some other 
hardware block. In that case - yes, I'd gladly remove that handling 
completely!

> I believe we need to make use of the clock framework to get the clock
> rate configuration in the SDHI driver. At the same time we want to
> stop the clock as frequently as possible. I believe we should disable
> and enable the clock together with the Runtime PM get/put operations -
> pretty much exactly like our I2C master driver does. The only problem
> is, while the clock is disabled the clock rate may be changed. So we
> need to be able to notify the MMC subsystem about the new clock rate.
> 
> Any ideas on how to make this happen?

Huh, if that's really possible... I'll have a look at that.

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

Re: [PATCH 09/11] mmc: sh_mobile_sdhi: do not manage PM clocks

[Guennadi Liakhovetski]

Hi Magnus

On Fri, 6 Jan 2012, Magnus Damm wrote:

> Hi Guennadi,
> 
> On Wed, Jan 4, 2012 at 11:17 PM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > On ARM the same clock is used by the PM subsystem and by the driver
> > directly. This leads to the clock staying permanently on, independent of
> > the runtime PM state. This patch makes clock enable and disable calls in
> > the driver SuperH-specific.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >  drivers/mmc/host/sh_mobile_sdhi.c |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> Thanks for your patch.
> 
> >From my point of view there is very little difference between SH and
> SH-Mobile ARM, so I don't understand the special treatment. Also, is
> this change really needed for SH with your recent Runtime PM patches?
> 
> I believe we need to make use of the clock framework to get the clock
> rate configuration in the SDHI driver. At the same time we want to
> stop the clock as frequently as possible. I believe we should disable
> and enable the clock together with the Runtime PM get/put operations -
> pretty much exactly like our I2C master driver does. The only problem
> is, while the clock is disabled the clock rate may be changed. So we
> need to be able to notify the MMC subsystem about the new clock rate.

It sounds a bit strange to me, that the clock frequency can change - I 
don't think we take it into account in other drivers? But if this is 
really the case, would something like this help in this case:

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

> Any ideas on how to make this happen?

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

[PATCH 09/11 v2] mmc: sh_mobile_sdhi: do not manage PM clocks manually

[Guennadi Liakhovetski]

On sh-mobile platforms the MMC clock frequency for the TMIO MMC unit is
obtained from the same clock, as the one, that runtime power-manages the
controller. The SDHI glue code has to access that clock directly,
bypassing the runtime PM framework, to get its frequency, but it
shouldn't enable or disable it.

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

Hi Magnus

On Fri, 6 Jan 2012, Magnus Damm wrote:

> >From my point of view there is very little difference between SH and
> SH-Mobile ARM, so I don't understand the special treatment. Also, is
> this change really needed for SH with your recent Runtime PM patches?

I tested on ecovec, using polled hotplug and debug prints from the PM 
clock enable / disable routines - seems to also work properly with this 
patch. So, looks like you were right, cannot say anything about other 
platforms, obviously.

 drivers/mmc/host/sh_mobile_sdhi.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index a8d353e..95a2405 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -117,8 +117,6 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 		goto eclkget;
 	}
 
-	clk_enable(priv->clk);
-
 	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;
@@ -235,7 +233,6 @@ eirq_sdio:
 eirq_card_detect:
 	tmio_mmc_host_remove(host);
 eprobe:
-	clk_disable(priv->clk);
 	clk_put(priv->clk);
 eclkget:
 	kfree(priv);
@@ -261,7 +258,6 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 		free_irq(irq, host);
 	}
 
-	clk_disable(priv->clk);
 	clk_put(priv->clk);
 	kfree(priv);
 
-- 
1.7.2.5

[PATCH 10/11] mmc: tmio: reconfigure the controller on runtime resume

[Guennadi Liakhovetski]

After the controller has been runtime suspended it has to be reset and DMA
has to be re-enabled, if it is used.

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

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 1811006..9106b3d 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -1031,6 +1031,10 @@ EXPORT_SYMBOL(tmio_mmc_host_suspend);
 int tmio_mmc_host_resume(struct device *dev)
 {
 	struct mmc_host *mmc = dev_get_drvdata(dev);
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+
+	tmio_mmc_reset(host);
+	tmio_mmc_enable_dma(host, true);
 
 	/* The MMC core will perform the complete set up */
 	return mmc_resume_host(mmc);
-- 
1.7.2.5

[PATCH 11/11] mmc: tmio: cosmetic: prettify the tmio_mmc_set_ios() function

[Guennadi Liakhovetski]

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

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 9106b3d..f68e6f3 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -752,6 +752,7 @@ fail:
 static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct tmio_mmc_host *host = mmc_priv(mmc);
+	struct device *dev = &host->pdev->dev;
 	unsigned long flags;
 
 	mutex_lock(&host->ios_lock);
@@ -759,13 +760,13 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	spin_lock_irqsave(&host->lock, flags);
 	if (host->mrq) {
 		if (IS_ERR(host->mrq)) {
-			dev_dbg(&host->pdev->dev,
+			dev_dbg(dev,
 				"%s.%d: concurrent .set_ios(), clk %u, mode %u\n",
 				current->comm, task_pid_nr(current),
 				ios->clock, ios->power_mode);
 			host->mrq = ERR_PTR(-EINTR);
 		} else {
-			dev_dbg(&host->pdev->dev,
+			dev_dbg(dev,
 				"%s.%d: CMD%u active since %lu, now %lu!\n",
 				current->comm, task_pid_nr(current),
 				host->mrq->cmd->opcode, host->last_req_ts, jiffies);
@@ -788,7 +789,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) {
-			pm_runtime_get_sync(&host->pdev->dev);
+			pm_runtime_get_sync(dev);
 			host->power = true;
 		}
 		tmio_mmc_set_clock(host, ios->clock);
@@ -802,7 +803,7 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			host->set_pwr(host->pdev, 0);
 		if (host->power) {
 			host->power = false;
-			pm_runtime_put(&host->pdev->dev);
+			pm_runtime_put(dev);
 		}
 		tmio_mmc_clk_stop(host);
 	}
-- 
1.7.2.5