[PATCH v2 0/3] mmc: omap_hsmmc: make more use of mmc library functionality

[PATCH v2 0/3] mmc: omap_hsmmc: make more use of mmc library functionality

[NeilBrown]

This is resend, rebased on
  git://git.linaro.org/people/ulf.hansson/mmc.git  next

and so should apply to 3.19-rc1.

omap_hsmmc currently duplicates some work that can be done for
it by common code, and consequently does not benefit from extra
functionality in that common code.

In particular, mmc_of_parse and the slot-gpio library are not used.

This set of patches allows omap_hsmmc to use that common
functionality, and benefit from any extra devicetree parsing
that it performs.

The one awkward part of this change is that omap_hsmmc has an
interrupt handler for 'card detect' which does more than the
common code.
I see three options:
 1 - move that functionality into common code
 2 - discard that functionality
 3 - allow the common code to be configured to use a device-specific
     card detect interrupt.

This series implements '3'.  I suspect a mix of '1' and '2' would
be a better choice but I know none of the history or justification
for those differences.

My preference would be for this series to be applied (if there are
no other issues) and if there are opinions about effecting '1' or '2',
they can be done with subsequent patches.

Thanks,
NeilBrown


---

NeilBrown (3):
      mmc: omap_hsmmc: remove prepare/complete system suspend support.
      mmc: omap_hsmmc: use slot-gpio library for gpio support.
      mmc: omap_hsmmc: use mmc_of_parse to parse common mmc configuration.


 drivers/mmc/core/slot-gpio.c           |   24 +++++
 drivers/mmc/host/omap_hsmmc.c          |  150 +++++---------------------------
 include/linux/mmc/slot-gpio.h          |    2 
 include/linux/platform_data/mmc-omap.h |    4 -
 4 files changed, 47 insertions(+), 133 deletions(-)

--
Signature

[PATCH 1/3] mmc: omap_hsmmc: remove prepare/complete system suspend support.

[NeilBrown]

The only function of these 'prepare' and 'complete' is to
disable the 'card detect' irq during suspend.

The commit which added this,
commit a48ce884d5819d5df2cf1139ab3c43f8e9e419b3
    mmc: omap_hsmmc: Introduce omap_hsmmc_prepare/complete

justified it by the need to avoid the registration of new devices
during suspend.
However mmc_pm_notify will set ->rescan_disable in the 'prepare'
stage and clear it in the 'complete' stage, so no card detection
will actually happen.
Also the interrupt will be disabled before final suspend as part
of common suspend processing.

So this disabling of the interrupt is unnecessary, and interferes
with a transition to using common code for card-detect management.

Cc: Felipe Balbi <balbi@ti.com>
Cc: Venkatraman S <svenkatr@ti.com>
Cc: Chris Ball <cjb@laptop.org>
Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/mmc/host/omap_hsmmc.c          |   50 --------------------------------
 include/linux/platform_data/mmc-omap.h |    4 ---
 2 files changed, 54 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 7c71dcdcba8b..537cba8f1de1 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -275,31 +275,6 @@ static int omap_hsmmc_get_cover_state(struct device *dev)
 	return !gpio_get_value_cansleep(mmc->switch_pin);
 }
 
-#ifdef CONFIG_PM
-
-static int omap_hsmmc_suspend_cdirq(struct device *dev)
-{
-	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
-
-	disable_irq(host->card_detect_irq);
-	return 0;
-}
-
-static int omap_hsmmc_resume_cdirq(struct device *dev)
-{
-	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
-
-	enable_irq(host->card_detect_irq);
-	return 0;
-}
-
-#else
-
-#define omap_hsmmc_suspend_cdirq	NULL
-#define omap_hsmmc_resume_cdirq		NULL
-
-#endif
-
 #ifdef CONFIG_REGULATOR
 
 static int omap_hsmmc_set_power(struct device *dev, int power_on, int vdd)
@@ -2234,8 +2209,6 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
 				"Unable to grab MMC CD IRQ\n");
 			goto err_irq_cd;
 		}
-		host->suspend = omap_hsmmc_suspend_cdirq;
-		host->resume = omap_hsmmc_resume_cdirq;
 	}
 
 	omap_hsmmc_disable_irq(host);
@@ -2322,25 +2295,6 @@ static int omap_hsmmc_remove(struct platform_device *pdev)
 }
 
 #ifdef CONFIG_PM
-static int omap_hsmmc_prepare(struct device *dev)
-{
-	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
-
-	if (host->suspend)
-		return host->suspend(dev);
-
-	return 0;
-}
-
-static void omap_hsmmc_complete(struct device *dev)
-{
-	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
-
-	if (host->resume)
-		host->resume(dev);
-
-}
-
 static int omap_hsmmc_suspend(struct device *dev)
 {
 	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
@@ -2398,8 +2352,6 @@ static int omap_hsmmc_resume(struct device *dev)
 }
 
 #else
-#define omap_hsmmc_prepare	NULL
-#define omap_hsmmc_complete	NULL
 #define omap_hsmmc_suspend	NULL
 #define omap_hsmmc_resume	NULL
 #endif
@@ -2484,8 +2436,6 @@ static int omap_hsmmc_runtime_resume(struct device *dev)
 static struct dev_pm_ops omap_hsmmc_dev_pm_ops = {
 	.suspend	= omap_hsmmc_suspend,
 	.resume		= omap_hsmmc_resume,
-	.prepare	= omap_hsmmc_prepare,
-	.complete	= omap_hsmmc_complete,
 	.runtime_suspend = omap_hsmmc_runtime_suspend,
 	.runtime_resume = omap_hsmmc_runtime_resume,
 };
diff --git a/include/linux/platform_data/mmc-omap.h b/include/linux/platform_data/mmc-omap.h
index 5c188f4e9bec..929469291406 100644
--- a/include/linux/platform_data/mmc-omap.h
+++ b/include/linux/platform_data/mmc-omap.h
@@ -31,10 +31,6 @@ struct omap_mmc_platform_data {
 	void (*cleanup)(struct device *dev);
 	void (*shutdown)(struct device *dev);
 
-	/* To handle board related suspend/resume functionality for MMC */
-	int (*suspend)(struct device *dev, int slot);
-	int (*resume)(struct device *dev, int slot);
-
 	/* Return context loss count due to PM states changing */
 	int (*get_context_loss_count)(struct device *dev);
 

[PATCH 2/3] mmc: omap_hsmmc: use slot-gpio library for gpio support.

[NeilBrown]

Using the common code removes some code duplication, and
makes it easier to switch to using mmc_of_parse() which
will remove more duplication.

As hsmmc has a slightly different interrupt service routine
for card-detect, enhance slot-gpio to allow an alternate
routine to be provided.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/mmc/core/slot-gpio.c  |   24 ++++++++++++++-
 drivers/mmc/host/omap_hsmmc.c |   67 +++++++++--------------------------------
 include/linux/mmc/slot-gpio.h |    2 +
 3 files changed, 39 insertions(+), 54 deletions(-)

diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 69bbf2adb329..f56323f5a996 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -23,6 +23,7 @@ struct mmc_gpio {
 	struct gpio_desc *cd_gpio;
 	bool override_ro_active_level;
 	bool override_cd_active_level;
+	irqreturn_t (*cd_gpio_isr)(int irq, void *dev_id);
 	char *ro_label;
 	char cd_label[0];
 };
@@ -156,8 +157,10 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
 		irq = -EINVAL;
 
 	if (irq >= 0) {
+		if (ctx->cd_gpio_isr == NULL)
+			ctx->cd_gpio_isr = mmc_gpio_cd_irqt;
 		ret = devm_request_threaded_irq(&host->class_dev, irq,
-			NULL, mmc_gpio_cd_irqt,
+			NULL, ctx->cd_gpio_isr,
 			IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
 			ctx->cd_label, host);
 		if (ret < 0)
@@ -171,6 +174,25 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
 }
 EXPORT_SYMBOL(mmc_gpiod_request_cd_irq);
 
+/* Register an alternate interrupt service routine for
+ * the card-detect GPIO.
+ */
+int mmc_gpio_request_cd_isr(struct mmc_host *host,
+			    irqreturn_t (*isr)(int irq, void *dev_id))
+{
+	struct mmc_gpio *ctx;
+	int ret;
+
+	ret = mmc_gpio_alloc(host);
+	if (ret < 0)
+		return ret;
+	ctx = host->slot.handler_priv;
+	if (ctx->cd_gpio_isr)
+		return -EBUSY;
+	ctx->cd_gpio_isr = isr;
+	return 0;
+}
+
 /**
  * mmc_gpio_request_cd - request a gpio for card-detection
  * @host: mmc host
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 537cba8f1de1..32514b648e3c 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -36,6 +36,7 @@
 #include <linux/mmc/host.h>
 #include <linux/mmc/core.h>
 #include <linux/mmc/mmc.h>
+#include <linux/mmc/slot-gpio.h>
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/gpio.h>
@@ -251,28 +252,22 @@ static void omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host);
 static int omap_hsmmc_card_detect(struct device *dev)
 {
 	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
-	struct omap_hsmmc_platform_data *mmc = host->pdata;
 
-	/* NOTE: assumes card detect signal is active-low */
-	return !gpio_get_value_cansleep(mmc->switch_pin);
+	return mmc_gpio_get_cd(host->mmc);
 }
 
 static int omap_hsmmc_get_wp(struct device *dev)
 {
 	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
-	struct omap_hsmmc_platform_data *mmc = host->pdata;
 
-	/* NOTE: assumes write protect signal is active-high */
-	return gpio_get_value_cansleep(mmc->gpio_wp);
+	return mmc_gpio_get_ro(host->mmc);
 }
 
 static int omap_hsmmc_get_cover_state(struct device *dev)
 {
 	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
-	struct omap_hsmmc_platform_data *mmc = host->pdata;
 
-	/* NOTE: assumes card detect signal is active-low */
-	return !gpio_get_value_cansleep(mmc->switch_pin);
+	return mmc_gpio_get_cd(host->mmc);
 }
 
 #ifdef CONFIG_REGULATOR
@@ -439,7 +434,10 @@ static inline int omap_hsmmc_have_reg(void)
 
 #endif
 
-static int omap_hsmmc_gpio_init(struct omap_hsmmc_host *host,
+static irqreturn_t omap_hsmmc_detect(int irq, void *dev_id);
+
+static int omap_hsmmc_gpio_init(struct mmc_host *mmc,
+				struct omap_hsmmc_host *host,
 				struct omap_hsmmc_platform_data *pdata)
 {
 	int ret;
@@ -452,46 +450,26 @@ static int omap_hsmmc_gpio_init(struct omap_hsmmc_host *host,
 			host->card_detect = omap_hsmmc_card_detect;
 		host->card_detect_irq =
 				gpio_to_irq(pdata->switch_pin);
-		ret = gpio_request(pdata->switch_pin, "mmc_cd");
+		ret = mmc_gpio_request_cd_isr(mmc, omap_hsmmc_detect);
 		if (ret)
 			return ret;
-		ret = gpio_direction_input(pdata->switch_pin);
+		ret = mmc_gpio_request_cd(mmc, pdata->switch_pin, 0);
 		if (ret)
-			goto err_free_sp;
+			return ret;
 	} else {
 		pdata->switch_pin = -EINVAL;
 	}
 
 	if (gpio_is_valid(pdata->gpio_wp)) {
 		host->get_ro = omap_hsmmc_get_wp;
-		ret = gpio_request(pdata->gpio_wp, "mmc_wp");
-		if (ret)
-			goto err_free_cd;
-		ret = gpio_direction_input(pdata->gpio_wp);
+		ret = mmc_gpio_request_ro(mmc, pdata->gpio_wp);
 		if (ret)
-			goto err_free_wp;
+			return ret;
 	} else {
 		pdata->gpio_wp = -EINVAL;
 	}
 
 	return 0;
-
-err_free_wp:
-	gpio_free(pdata->gpio_wp);
-err_free_cd:
-	if (gpio_is_valid(pdata->switch_pin))
-err_free_sp:
-		gpio_free(pdata->switch_pin);
-	return ret;
-}
-
-static void omap_hsmmc_gpio_free(struct omap_hsmmc_host *host,
-				 struct omap_hsmmc_platform_data *pdata)
-{
-	if (gpio_is_valid(pdata->gpio_wp))
-		gpio_free(pdata->gpio_wp);
-	if (gpio_is_valid(pdata->switch_pin))
-		gpio_free(pdata->switch_pin);
 }
 
 /*
@@ -2066,7 +2044,7 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
 	host->next_data.cookie = 1;
 	host->pbias_enabled = 0;
 
-	ret = omap_hsmmc_gpio_init(host, pdata);
+	ret = omap_hsmmc_gpio_init(mmc, host, pdata);
 	if (ret)
 		goto err_gpio;
 
@@ -2197,20 +2175,6 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
 
 	mmc->ocr_avail = mmc_pdata(host)->ocr_mask;
 
-	/* Request IRQ for card detect */
-	if (host->card_detect_irq) {
-		ret = devm_request_threaded_irq(&pdev->dev,
-						host->card_detect_irq,
-						NULL, omap_hsmmc_detect,
-					   IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-					   mmc_hostname(mmc), host);
-		if (ret) {
-			dev_err(mmc_dev(host->mmc),
-				"Unable to grab MMC CD IRQ\n");
-			goto err_irq_cd;
-		}
-	}
-
 	omap_hsmmc_disable_irq(host);
 
 	/*
@@ -2249,7 +2213,6 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
 
 err_slot_name:
 	mmc_remove_host(mmc);
-err_irq_cd:
 	if (host->use_reg)
 		omap_hsmmc_reg_put(host);
 err_irq:
@@ -2262,7 +2225,6 @@ err_irq:
 	if (host->dbclk)
 		clk_disable_unprepare(host->dbclk);
 err1:
-	omap_hsmmc_gpio_free(host, pdata);
 err_gpio:
 	mmc_free_host(mmc);
 err:
@@ -2288,7 +2250,6 @@ static int omap_hsmmc_remove(struct platform_device *pdev)
 	if (host->dbclk)
 		clk_disable_unprepare(host->dbclk);
 
-	omap_hsmmc_gpio_free(host, host->pdata);
 	mmc_free_host(host->mmc);
 
 	return 0;
diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
index e56fa24c9322..9e55db60deb0 100644
--- a/include/linux/mmc/slot-gpio.h
+++ b/include/linux/mmc/slot-gpio.h
@@ -28,6 +28,8 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
 int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
 			 unsigned int idx, bool override_active_level,
 			 unsigned int debounce, bool *gpio_invert);
+int mmc_gpio_request_cd_isr(struct mmc_host *host,
+			    irqreturn_t (*isr)(int irq, void *dev_id));
 void mmc_gpiod_free_cd(struct mmc_host *host);
 void mmc_gpiod_request_cd_irq(struct mmc_host *host);
 

[PATCH 3/3] mmc: omap_hsmmc: use mmc_of_parse to parse common mmc configuration.

[NeilBrown]

This ensures that all standard options are available to hsmmc,
In particular, I need cap-power-off-card.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/mmc/host/omap_hsmmc.c |   33 ++++++++-------------------------
 1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 32514b648e3c..1bfacc59e3bf 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1931,13 +1931,6 @@ static struct omap_hsmmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
 {
 	struct omap_hsmmc_platform_data *pdata;
 	struct device_node *np = dev->of_node;
-	u32 bus_width, max_freq;
-	int cd_gpio, wp_gpio;
-
-	cd_gpio = of_get_named_gpio(np, "cd-gpios", 0);
-	wp_gpio = of_get_named_gpio(np, "wp-gpios", 0);
-	if (cd_gpio == -EPROBE_DEFER || wp_gpio == -EPROBE_DEFER)
-		return ERR_PTR(-EPROBE_DEFER);
 
 	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
@@ -1946,34 +1939,20 @@ static struct omap_hsmmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
 	if (of_find_property(np, "ti,dual-volt", NULL))
 		pdata->controller_flags |= OMAP_HSMMC_SUPPORTS_DUAL_VOLT;
 
-	pdata->switch_pin = cd_gpio;
-	pdata->gpio_wp = wp_gpio;
+	pdata->switch_pin = -EINVAL;
+	pdata->gpio_wp = -EINVAL;
 
 	if (of_find_property(np, "ti,non-removable", NULL)) {
 		pdata->nonremovable = true;
 		pdata->no_regulator_off_init = true;
 	}
-	of_property_read_u32(np, "bus-width", &bus_width);
-	if (bus_width == 4)
-		pdata->caps |= MMC_CAP_4_BIT_DATA;
-	else if (bus_width == 8)
-		pdata->caps |= MMC_CAP_8_BIT_DATA;
 
 	if (of_find_property(np, "ti,needs-special-reset", NULL))
 		pdata->features |= HSMMC_HAS_UPDATED_RESET;
 
-	if (!of_property_read_u32(np, "max-frequency", &max_freq))
-		pdata->max_freq = max_freq;
-
 	if (of_find_property(np, "ti,needs-special-hs-handling", NULL))
 		pdata->features |= HSMMC_HAS_HSPE_SUPPORT;
 
-	if (of_find_property(np, "keep-power-in-suspend", NULL))
-		pdata->pm_caps |= MMC_PM_KEEP_POWER;
-
-	if (of_find_property(np, "enable-sdio-wakeup", NULL))
-		pdata->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
-
 	return pdata;
 }
 #else
@@ -2031,6 +2010,10 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
 		goto err;
 	}
 
+	ret = mmc_of_parse(mmc);
+	if (ret)
+		goto err1;
+
 	host		= mmc_priv(mmc);
 	host->mmc	= mmc;
 	host->pdata	= pdata;
@@ -2059,7 +2042,7 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
 
 	if (pdata->max_freq > 0)
 		mmc->f_max = pdata->max_freq;
-	else
+	else if (mmc->f_max == 0)
 		mmc->f_max = OMAP_MMC_MAX_CLOCK;
 
 	spin_lock_init(&host->irq_lock);
@@ -2113,7 +2096,7 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
 	if (mmc_pdata(host)->nonremovable)
 		mmc->caps |= MMC_CAP_NONREMOVABLE;
 
-	mmc->pm_caps = mmc_pdata(host)->pm_caps;
+	mmc->pm_caps |= mmc_pdata(host)->pm_caps;
 
 	omap_hsmmc_conf_bus_power(host);
 

Re: [PATCH 2/3] mmc: omap_hsmmc: use slot-gpio library for gpio support.

[Ulf Hansson]

On 11 December 2014 at 22:43, NeilBrown <neilb@suse.de> wrote:
> Using the common code removes some code duplication, and
> makes it easier to switch to using mmc_of_parse() which
> will remove more duplication.
>
> As hsmmc has a slightly different interrupt service routine
> for card-detect, enhance slot-gpio to allow an alternate
> routine to be provided.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  drivers/mmc/core/slot-gpio.c  |   24 ++++++++++++++-
>  drivers/mmc/host/omap_hsmmc.c |   67 +++++++++--------------------------------
>  include/linux/mmc/slot-gpio.h |    2 +
>  3 files changed, 39 insertions(+), 54 deletions(-)

I like the looks of this patch, still I want the mmc core parts to be
separated into it's own patch. Can you please split this up.

Kind regards
Uffe

>
> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
> index 69bbf2adb329..f56323f5a996 100644
> --- a/drivers/mmc/core/slot-gpio.c
> +++ b/drivers/mmc/core/slot-gpio.c
> @@ -23,6 +23,7 @@ struct mmc_gpio {
>         struct gpio_desc *cd_gpio;
>         bool override_ro_active_level;
>         bool override_cd_active_level;
> +       irqreturn_t (*cd_gpio_isr)(int irq, void *dev_id);
>         char *ro_label;
>         char cd_label[0];
>  };
> @@ -156,8 +157,10 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
>                 irq = -EINVAL;
>
>         if (irq >= 0) {
> +               if (ctx->cd_gpio_isr == NULL)
> +                       ctx->cd_gpio_isr = mmc_gpio_cd_irqt;
>                 ret = devm_request_threaded_irq(&host->class_dev, irq,
> -                       NULL, mmc_gpio_cd_irqt,
> +                       NULL, ctx->cd_gpio_isr,
>                         IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>                         ctx->cd_label, host);
>                 if (ret < 0)
> @@ -171,6 +174,25 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
>  }
>  EXPORT_SYMBOL(mmc_gpiod_request_cd_irq);
>
> +/* Register an alternate interrupt service routine for
> + * the card-detect GPIO.
> + */
> +int mmc_gpio_request_cd_isr(struct mmc_host *host,
> +                           irqreturn_t (*isr)(int irq, void *dev_id))
> +{
> +       struct mmc_gpio *ctx;
> +       int ret;
> +
> +       ret = mmc_gpio_alloc(host);
> +       if (ret < 0)
> +               return ret;
> +       ctx = host->slot.handler_priv;
> +       if (ctx->cd_gpio_isr)
> +               return -EBUSY;
> +       ctx->cd_gpio_isr = isr;
> +       return 0;
> +}
> +
>  /**
>   * mmc_gpio_request_cd - request a gpio for card-detection
>   * @host: mmc host
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 537cba8f1de1..32514b648e3c 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -36,6 +36,7 @@
>  #include <linux/mmc/host.h>
>  #include <linux/mmc/core.h>
>  #include <linux/mmc/mmc.h>
> +#include <linux/mmc/slot-gpio.h>
>  #include <linux/io.h>
>  #include <linux/irq.h>
>  #include <linux/gpio.h>
> @@ -251,28 +252,22 @@ static void omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host);
>  static int omap_hsmmc_card_detect(struct device *dev)
>  {
>         struct omap_hsmmc_host *host = dev_get_drvdata(dev);
> -       struct omap_hsmmc_platform_data *mmc = host->pdata;
>
> -       /* NOTE: assumes card detect signal is active-low */
> -       return !gpio_get_value_cansleep(mmc->switch_pin);
> +       return mmc_gpio_get_cd(host->mmc);
>  }
>
>  static int omap_hsmmc_get_wp(struct device *dev)
>  {
>         struct omap_hsmmc_host *host = dev_get_drvdata(dev);
> -       struct omap_hsmmc_platform_data *mmc = host->pdata;
>
> -       /* NOTE: assumes write protect signal is active-high */
> -       return gpio_get_value_cansleep(mmc->gpio_wp);
> +       return mmc_gpio_get_ro(host->mmc);
>  }
>
>  static int omap_hsmmc_get_cover_state(struct device *dev)
>  {
>         struct omap_hsmmc_host *host = dev_get_drvdata(dev);
> -       struct omap_hsmmc_platform_data *mmc = host->pdata;
>
> -       /* NOTE: assumes card detect signal is active-low */
> -       return !gpio_get_value_cansleep(mmc->switch_pin);
> +       return mmc_gpio_get_cd(host->mmc);
>  }
>
>  #ifdef CONFIG_REGULATOR
> @@ -439,7 +434,10 @@ static inline int omap_hsmmc_have_reg(void)
>
>  #endif
>
> -static int omap_hsmmc_gpio_init(struct omap_hsmmc_host *host,
> +static irqreturn_t omap_hsmmc_detect(int irq, void *dev_id);
> +
> +static int omap_hsmmc_gpio_init(struct mmc_host *mmc,
> +                               struct omap_hsmmc_host *host,
>                                 struct omap_hsmmc_platform_data *pdata)
>  {
>         int ret;
> @@ -452,46 +450,26 @@ static int omap_hsmmc_gpio_init(struct omap_hsmmc_host *host,
>                         host->card_detect = omap_hsmmc_card_detect;
>                 host->card_detect_irq =
>                                 gpio_to_irq(pdata->switch_pin);
> -               ret = gpio_request(pdata->switch_pin, "mmc_cd");
> +               ret = mmc_gpio_request_cd_isr(mmc, omap_hsmmc_detect);
>                 if (ret)
>                         return ret;
> -               ret = gpio_direction_input(pdata->switch_pin);
> +               ret = mmc_gpio_request_cd(mmc, pdata->switch_pin, 0);
>                 if (ret)
> -                       goto err_free_sp;
> +                       return ret;
>         } else {
>                 pdata->switch_pin = -EINVAL;
>         }
>
>         if (gpio_is_valid(pdata->gpio_wp)) {
>                 host->get_ro = omap_hsmmc_get_wp;
> -               ret = gpio_request(pdata->gpio_wp, "mmc_wp");
> -               if (ret)
> -                       goto err_free_cd;
> -               ret = gpio_direction_input(pdata->gpio_wp);
> +               ret = mmc_gpio_request_ro(mmc, pdata->gpio_wp);
>                 if (ret)
> -                       goto err_free_wp;
> +                       return ret;
>         } else {
>                 pdata->gpio_wp = -EINVAL;
>         }
>
>         return 0;
> -
> -err_free_wp:
> -       gpio_free(pdata->gpio_wp);
> -err_free_cd:
> -       if (gpio_is_valid(pdata->switch_pin))
> -err_free_sp:
> -               gpio_free(pdata->switch_pin);
> -       return ret;
> -}
> -
> -static void omap_hsmmc_gpio_free(struct omap_hsmmc_host *host,
> -                                struct omap_hsmmc_platform_data *pdata)
> -{
> -       if (gpio_is_valid(pdata->gpio_wp))
> -               gpio_free(pdata->gpio_wp);
> -       if (gpio_is_valid(pdata->switch_pin))
> -               gpio_free(pdata->switch_pin);
>  }
>
>  /*
> @@ -2066,7 +2044,7 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
>         host->next_data.cookie = 1;
>         host->pbias_enabled = 0;
>
> -       ret = omap_hsmmc_gpio_init(host, pdata);
> +       ret = omap_hsmmc_gpio_init(mmc, host, pdata);
>         if (ret)
>                 goto err_gpio;
>
> @@ -2197,20 +2175,6 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
>
>         mmc->ocr_avail = mmc_pdata(host)->ocr_mask;
>
> -       /* Request IRQ for card detect */
> -       if (host->card_detect_irq) {
> -               ret = devm_request_threaded_irq(&pdev->dev,
> -                                               host->card_detect_irq,
> -                                               NULL, omap_hsmmc_detect,
> -                                          IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -                                          mmc_hostname(mmc), host);
> -               if (ret) {
> -                       dev_err(mmc_dev(host->mmc),
> -                               "Unable to grab MMC CD IRQ\n");
> -                       goto err_irq_cd;
> -               }
> -       }
> -
>         omap_hsmmc_disable_irq(host);
>
>         /*
> @@ -2249,7 +2213,6 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
>
>  err_slot_name:
>         mmc_remove_host(mmc);
> -err_irq_cd:
>         if (host->use_reg)
>                 omap_hsmmc_reg_put(host);
>  err_irq:
> @@ -2262,7 +2225,6 @@ err_irq:
>         if (host->dbclk)
>                 clk_disable_unprepare(host->dbclk);
>  err1:
> -       omap_hsmmc_gpio_free(host, pdata);
>  err_gpio:
>         mmc_free_host(mmc);
>  err:
> @@ -2288,7 +2250,6 @@ static int omap_hsmmc_remove(struct platform_device *pdev)
>         if (host->dbclk)
>                 clk_disable_unprepare(host->dbclk);
>
> -       omap_hsmmc_gpio_free(host, host->pdata);
>         mmc_free_host(host->mmc);
>
>         return 0;
> diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
> index e56fa24c9322..9e55db60deb0 100644
> --- a/include/linux/mmc/slot-gpio.h
> +++ b/include/linux/mmc/slot-gpio.h
> @@ -28,6 +28,8 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
>  int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
>                          unsigned int idx, bool override_active_level,
>                          unsigned int debounce, bool *gpio_invert);
> +int mmc_gpio_request_cd_isr(struct mmc_host *host,
> +                           irqreturn_t (*isr)(int irq, void *dev_id));
>  void mmc_gpiod_free_cd(struct mmc_host *host);
>  void mmc_gpiod_request_cd_irq(struct mmc_host *host);
>
>
>

Re: [PATCH 1/3] mmc: omap_hsmmc: remove prepare/complete system suspend support.

[Ulf Hansson]

On 11 December 2014 at 22:43, NeilBrown <neilb@suse.de> wrote:
> The only function of these 'prepare' and 'complete' is to
> disable the 'card detect' irq during suspend.
>
> The commit which added this,
> commit a48ce884d5819d5df2cf1139ab3c43f8e9e419b3
>     mmc: omap_hsmmc: Introduce omap_hsmmc_prepare/complete
>
> justified it by the need to avoid the registration of new devices
> during suspend.
> However mmc_pm_notify will set ->rescan_disable in the 'prepare'
> stage and clear it in the 'complete' stage, so no card detection
> will actually happen.
> Also the interrupt will be disabled before final suspend as part
> of common suspend processing.
>
> So this disabling of the interrupt is unnecessary, and interferes
> with a transition to using common code for card-detect management.
>
> Cc: Felipe Balbi <balbi@ti.com>
> Cc: Venkatraman S <svenkatr@ti.com>
> Cc: Chris Ball <cjb@laptop.org>
> Signed-off-by: NeilBrown <neilb@suse.de>

Thanks! Queued for 3.20.

Kind regards
Uffe

> ---
>  drivers/mmc/host/omap_hsmmc.c          |   50 --------------------------------
>  include/linux/platform_data/mmc-omap.h |    4 ---
>  2 files changed, 54 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 7c71dcdcba8b..537cba8f1de1 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -275,31 +275,6 @@ static int omap_hsmmc_get_cover_state(struct device *dev)
>         return !gpio_get_value_cansleep(mmc->switch_pin);
>  }
>
> -#ifdef CONFIG_PM
> -
> -static int omap_hsmmc_suspend_cdirq(struct device *dev)
> -{
> -       struct omap_hsmmc_host *host = dev_get_drvdata(dev);
> -
> -       disable_irq(host->card_detect_irq);
> -       return 0;
> -}
> -
> -static int omap_hsmmc_resume_cdirq(struct device *dev)
> -{
> -       struct omap_hsmmc_host *host = dev_get_drvdata(dev);
> -
> -       enable_irq(host->card_detect_irq);
> -       return 0;
> -}
> -
> -#else
> -
> -#define omap_hsmmc_suspend_cdirq       NULL
> -#define omap_hsmmc_resume_cdirq                NULL
> -
> -#endif
> -
>  #ifdef CONFIG_REGULATOR
>
>  static int omap_hsmmc_set_power(struct device *dev, int power_on, int vdd)
> @@ -2234,8 +2209,6 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
>                                 "Unable to grab MMC CD IRQ\n");
>                         goto err_irq_cd;
>                 }
> -               host->suspend = omap_hsmmc_suspend_cdirq;
> -               host->resume = omap_hsmmc_resume_cdirq;
>         }
>
>         omap_hsmmc_disable_irq(host);
> @@ -2322,25 +2295,6 @@ static int omap_hsmmc_remove(struct platform_device *pdev)
>  }
>
>  #ifdef CONFIG_PM
> -static int omap_hsmmc_prepare(struct device *dev)
> -{
> -       struct omap_hsmmc_host *host = dev_get_drvdata(dev);
> -
> -       if (host->suspend)
> -               return host->suspend(dev);
> -
> -       return 0;
> -}
> -
> -static void omap_hsmmc_complete(struct device *dev)
> -{
> -       struct omap_hsmmc_host *host = dev_get_drvdata(dev);
> -
> -       if (host->resume)
> -               host->resume(dev);
> -
> -}
> -
>  static int omap_hsmmc_suspend(struct device *dev)
>  {
>         struct omap_hsmmc_host *host = dev_get_drvdata(dev);
> @@ -2398,8 +2352,6 @@ static int omap_hsmmc_resume(struct device *dev)
>  }
>
>  #else
> -#define omap_hsmmc_prepare     NULL
> -#define omap_hsmmc_complete    NULL
>  #define omap_hsmmc_suspend     NULL
>  #define omap_hsmmc_resume      NULL
>  #endif
> @@ -2484,8 +2436,6 @@ static int omap_hsmmc_runtime_resume(struct device *dev)
>  static struct dev_pm_ops omap_hsmmc_dev_pm_ops = {
>         .suspend        = omap_hsmmc_suspend,
>         .resume         = omap_hsmmc_resume,
> -       .prepare        = omap_hsmmc_prepare,
> -       .complete       = omap_hsmmc_complete,
>         .runtime_suspend = omap_hsmmc_runtime_suspend,
>         .runtime_resume = omap_hsmmc_runtime_resume,
>  };
> diff --git a/include/linux/platform_data/mmc-omap.h b/include/linux/platform_data/mmc-omap.h
> index 5c188f4e9bec..929469291406 100644
> --- a/include/linux/platform_data/mmc-omap.h
> +++ b/include/linux/platform_data/mmc-omap.h
> @@ -31,10 +31,6 @@ struct omap_mmc_platform_data {
>         void (*cleanup)(struct device *dev);
>         void (*shutdown)(struct device *dev);
>
> -       /* To handle board related suspend/resume functionality for MMC */
> -       int (*suspend)(struct device *dev, int slot);
> -       int (*resume)(struct device *dev, int slot);
> -
>         /* Return context loss count due to PM states changing */
>         int (*get_context_loss_count)(struct device *dev);
>
>
>

[PATCH 1/3] mmc: omap_hsmmc: remove prepare/complete system suspend support.

[NeilBrown]

The only function of these 'prepare' and 'complete' is to
disable the 'card detect' irq during suspend.

The commit which added this,
commit a48ce884d5819d5df2cf1139ab3c43f8e9e419b3
    mmc: omap_hsmmc: Introduce omap_hsmmc_prepare/complete

justified it by the need to avoid the registration of new devices
during suspend.
However mmc_pm_notify will set ->rescan_disable in the 'prepare'
stage and clear it in the 'complete' stage, so no card detection
will actually happen.
Also the interrupt will be disabled before final suspend as part
of common suspend processing.

So this disabling of the interrupt is unnecessary, and interferes
with a transition to using common code for card-detect management.

Cc: Felipe Balbi <balbi@ti.com>
Cc: Venkatraman S <svenkatr@ti.com>
Cc: Chris Ball <cjb@laptop.org>
Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/mmc/host/omap_hsmmc.c          |   52 --------------------------------
 include/linux/platform_data/mmc-omap.h |    4 --
 2 files changed, 56 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 965672663ef0..6958e6898d03 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -257,33 +257,6 @@ static int omap_hsmmc_get_cover_state(struct device *dev, int slot)
 	return !gpio_get_value_cansleep(mmc->slots[0].switch_pin);
 }
 
-#ifdef CONFIG_PM
-
-static int omap_hsmmc_suspend_cdirq(struct device *dev, int slot)
-{
-	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
-	struct omap_mmc_platform_data *mmc = host->pdata;
-
-	disable_irq(mmc->slots[0].card_detect_irq);
-	return 0;
-}
-
-static int omap_hsmmc_resume_cdirq(struct device *dev, int slot)
-{
-	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
-	struct omap_mmc_platform_data *mmc = host->pdata;
-
-	enable_irq(mmc->slots[0].card_detect_irq);
-	return 0;
-}
-
-#else
-
-#define omap_hsmmc_suspend_cdirq	NULL
-#define omap_hsmmc_resume_cdirq		NULL
-
-#endif
-
 #ifdef CONFIG_REGULATOR
 
 static int omap_hsmmc_set_power(struct device *dev, int slot, int power_on,
@@ -2223,8 +2196,6 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
 				"Unable to grab MMC CD IRQ\n");
 			goto err_irq_cd;
 		}
-		pdata->suspend = omap_hsmmc_suspend_cdirq;
-		pdata->resume = omap_hsmmc_resume_cdirq;
 	}
 
 	omap_hsmmc_disable_irq(host);
@@ -2316,25 +2287,6 @@ static int omap_hsmmc_remove(struct platform_device *pdev)
 }
 
 #ifdef CONFIG_PM
-static int omap_hsmmc_prepare(struct device *dev)
-{
-	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
-
-	if (host->pdata->suspend)
-		return host->pdata->suspend(dev, host->slot_id);
-
-	return 0;
-}
-
-static void omap_hsmmc_complete(struct device *dev)
-{
-	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
-
-	if (host->pdata->resume)
-		host->pdata->resume(dev, host->slot_id);
-
-}
-
 static int omap_hsmmc_suspend(struct device *dev)
 {
 	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
@@ -2392,8 +2344,6 @@ static int omap_hsmmc_resume(struct device *dev)
 }
 
 #else
-#define omap_hsmmc_prepare	NULL
-#define omap_hsmmc_complete	NULL
 #define omap_hsmmc_suspend	NULL
 #define omap_hsmmc_resume	NULL
 #endif
@@ -2478,8 +2428,6 @@ static int omap_hsmmc_runtime_resume(struct device *dev)
 static struct dev_pm_ops omap_hsmmc_dev_pm_ops = {
 	.suspend	= omap_hsmmc_suspend,
 	.resume		= omap_hsmmc_resume,
-	.prepare	= omap_hsmmc_prepare,
-	.complete	= omap_hsmmc_complete,
 	.runtime_suspend = omap_hsmmc_runtime_suspend,
 	.runtime_resume = omap_hsmmc_runtime_resume,
 };
diff --git a/include/linux/platform_data/mmc-omap.h b/include/linux/platform_data/mmc-omap.h
index 51e70cf25cbc..0512e3f27de4 100644
--- a/include/linux/platform_data/mmc-omap.h
+++ b/include/linux/platform_data/mmc-omap.h
@@ -55,10 +55,6 @@ struct omap_mmc_platform_data {
 	void (*cleanup)(struct device *dev);
 	void (*shutdown)(struct device *dev);
 
-	/* To handle board related suspend/resume functionality for MMC */
-	int (*suspend)(struct device *dev, int slot);
-	int (*resume)(struct device *dev, int slot);
-
 	/* Return context loss count due to PM states changing */
 	int (*get_context_loss_count)(struct device *dev);