mmc: omap_hsmmc: make more use of mmc library functionality

PATCH Superseded NeilBrown 2 participants 7 messages Nov 7, 2014 – Nov 21, 2014

Subsystems (2)

MMC, SD and SDIO OMAP hs MMC support

lore

Share & tools

lore

NeilBrown

author

2014-11-07 23:52 UTC #0 lore raw

cover mmc: omap_hsmmc: make more use of mmc library functionality

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 no 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           |   21 ++++
 drivers/mmc/host/omap_hsmmc.c          |  158 +++++---------------------------
 include/linux/mmc/slot-gpio.h          |    2 
 include/linux/platform_data/mmc-omap.h |    4 -
 4 files changed, 47 insertions(+), 138 deletions(-)

--
Signature

NeilBrown

author

2014-11-07 23:52 UTC #1 lore raw

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

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

Cc Venkatraman S <[email protected]>

Cc Chris Ball <[email protected]>

Signed-off-by NeilBrown <[email protected]>

---
 drivers/mmc/host/omap_hsmmc.c          |   52 --------------------------------
 include/linux/platform_data/mmc-omap.h |    4 --
 2 files changed, 56 deletions(-)

drivers/mmc/host/omap_hsmmc.c +0 ~~-52~~

index 965672663ef0..6958e6898d03 100644
@@ -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,
 };

include/linux/platform_data/mmc-omap.h +0 -4

index 51e70cf25cbc..0512e3f27de4 100644
@@ -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);

NeilBrown

author

2014-11-07 23:52 UTC #2 lore raw

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

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

---
 drivers/mmc/core/slot-gpio.c  |   21 +++++++++++-
 drivers/mmc/host/omap_hsmmc.c |   73 ++++++++++-------------------------------
 include/linux/mmc/slot-gpio.h |    2 +
 3 files changed, 39 insertions(+), 57 deletions(-)

drivers/mmc/core/slot-gpio.c +20 -1

index 5f89cb83d5f0..f3bc51f9aba9 100644
@@ -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,22 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
 }
 EXPORT_SYMBOL(mmc_gpiod_request_cd_irq);
 
+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

drivers/mmc/host/omap_hsmmc.c +17 ~~-56~~

159 lines not on this page · raw message

include/linux/mmc/slot-gpio.h +2 -0

index d2433381e828..a119892cebdb 100644
@@ -25,6 +25,8 @@ void mmc_gpio_free_cd(struct mmc_host *host);
 int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
 			 unsigned int idx, bool override_active_level,
 			 unsigned int debounce);
+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);

NeilBrown

author

2014-11-07 23:52 UTC #3 lore raw

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

In reply to NeilBrown

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

Signed-off-by NeilBrown <[email protected]>

---
 drivers/mmc/host/omap_hsmmc.c |   33 ++++++++-------------------------
 1 file changed, 8 insertions(+), 25 deletions(-)

drivers/mmc/host/omap_hsmmc.c +8 ~~-25~~

index 3fe02cf077ec..06362c1ee31d 100644
@@ -1903,13 +1903,6 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
 {
 	struct omap_mmc_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)

@@ -1920,34 +1913,20 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
 
 	/* This driver only supports 1 slot */
 	pdata->nr_slots = 1;
-	pdata->slots[0].switch_pin = cd_gpio;
-	pdata->slots[0].gpio_wp = wp_gpio;
+	pdata->slots[0].switch_pin = -EINVAL;
+	pdata->slots[0].gpio_wp = -EINVAL;
 
 	if (of_find_property(np, "ti,non-removable", NULL)) {
 		pdata->slots[0].nonremovable = true;
 		pdata->slots[0].no_regulator_off_init = true;
 	}
-	of_property_read_u32(np, "bus-width", &bus_width);
-	if (bus_width == 4)
-		pdata->slots[0].caps |= MMC_CAP_4_BIT_DATA;
-	else if (bus_width == 8)
-		pdata->slots[0].caps |= MMC_CAP_8_BIT_DATA;
 
 	if (of_find_property(np, "ti,needs-special-reset", NULL))
 		pdata->slots[0].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->slots[0].features |= HSMMC_HAS_HSPE_SUPPORT;
 
-	if (of_find_property(np, "keep-power-in-suspend", NULL))
-		pdata->slots[0].pm_caps |= MMC_PM_KEEP_POWER;
-
-	if (of_find_property(np, "enable-sdio-wakeup", NULL))
-		pdata->slots[0].pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
-
 	return pdata;
 }
 #else

@@ -2014,6 +1993,10 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
 	if (ret)
 		goto err1;
 
+	ret = mmc_of_parse(mmc);
+	if (ret)
+		goto err1;
+
 	host		= mmc_priv(mmc);
 	host->mmc	= mmc;
 	host->pdata	= pdata;

@@ -2039,7 +2022,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);

@@ -2093,7 +2076,7 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
 	if (mmc_slot(host).nonremovable)
 		mmc->caps |= MMC_CAP_NONREMOVABLE;
 
-	mmc->pm_caps = mmc_slot(host).pm_caps;
+	mmc->pm_caps |= mmc_slot(host).pm_caps;
 
 	omap_hsmmc_conf_bus_power(host);

Ulf Hansson

2014-11-19 10:14 UTC #4 lore raw

In reply to NeilBrown

Quoted 23 lines · NeilBrown

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 no of the history or justification for those differences.

Quoted 7 lines

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

I like the looks of this patchset, but it needs a rebase.

Kind regards
Uffe

Quoted 18 lines

---
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           |   21 ++++
 drivers/mmc/host/omap_hsmmc.c          |  158 +++++---------------------------
 include/linux/mmc/slot-gpio.h          |    2
 include/linux/platform_data/mmc-omap.h |    4 -
 4 files changed, 47 insertions(+), 138 deletions(-)
--
Signature

NeilBrown

author

2014-11-19 21:44 UTC #5 lore raw

Quoted 28 lines · On Wed, 19 Nov 2014 11:14:24 +0100 Ulf Hansson <[email protected]> wrote:

Quoted 27 lines · NeilBrown
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 no 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.

Quoted 5 lines

Quoted 3 lines
Thanks,
NeilBrown
I like the looks of this patchset, but it needs a rebase.

Thanks. What should I rebase against? Is 3.18-rc sufficient or is there some other tree I should work against?

Thanks,
NeilBrown

Quoted 27 lines

Kind regards
Uffe
Quoted 9 lines
---
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.
Quoted 9 lines
 drivers/mmc/core/slot-gpio.c           |   21 ++++
 drivers/mmc/host/omap_hsmmc.c          |  158 +++++---------------------------
 include/linux/mmc/slot-gpio.h          |    2
 include/linux/platform_data/mmc-omap.h |    4 -
 4 files changed, 47 insertions(+), 138 deletions(-)
--
Signature
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Ulf Hansson

2014-11-21 12:56 UTC #6 lore raw

Quoted 7 lines

Thanks. What should I rebase against? Is 3.18-rc sufficient or is there some other tree I should work against?
Thanks,
NeilBrown

git://git.linaro.org/people/ulf.hansson/mmc.git

Kind regards
Uffe