[PATCH-v2 0/4] mmc: switch to 1-bit mode which stopping clocks.

[PATCH-v2 0/4] mmc: switch to 1-bit mode which stopping clocks.

[NeilBrown]

This is a new version which moves most of the code into
mmc/core so it is available to all drivers.

I was wrong about not being able to sleep in pm_runtime callbacks.
There are spinlocks, but not the ones I thought they were...

Anyway, I tried switching to 1-bit mode from within the
runtime_suspend callback and hit a different problem.
The sequence to switch to 1-bit involves calling set_ios
which, for omap_hsmmc at least, calls pm_runtime_get_sync().

Calling pm_runtime_get_sync from the runtime_suspend callback
deadlocks.

It would be possible to remove the pm_runtime_get_sync call
from set_ios() functions, and instead call host->enable(),
host->disable() around those mmc_set_ios() calls which don't
have the host claimed.  However that seems a bit fragile to me.

So for now I'm persisting with doing the width change from
a work-queue.  This set of patches does that.

If you think calling host->enable/disable around mmc_set_ios() calls
does make sense, I can post my other patch for review.

Thanks,
NeilBrown


---

NeilBrown (4):
      mmc: core: fold mmc_set_bus_width calls into sdio_enable_4bit_bus.
      mmc: core: allow non-blocking form of mmc_claim_host
      mmc: sdio: support switching to 1-bit before turning off clocks
      mmc: omap_hsmmc: switch to 1-bit before stopping clocks.


 drivers/mmc/core/core.c       |   21 +++++++++---
 drivers/mmc/core/sdio.c       |   74 +++++++++++++++++++++++++++++------------
 drivers/mmc/host/omap_hsmmc.c |   13 ++++++-
 include/linux/mmc/core.h      |    2 +
 include/linux/mmc/host.h      |    2 +
 5 files changed, 83 insertions(+), 29 deletions(-)

--
Signature

[PATCH 1/4] mmc: core: fold mmc_set_bus_width calls into sdio_enable_4bit_bus.

[NeilBrown]

Every call to sdio_enable_4bit_bus is followed (on success) but a call
to mmc_set_bus_width().

To simplify the code, include those calls directly in
sdio_enable_4bit_bus().

Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/mmc/core/sdio.c |   32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index ce6cc47206b0..5bc6c7dbbd60 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -293,19 +293,22 @@ static int sdio_enable_4bit_bus(struct mmc_card *card)
 	int err;
 
 	if (card->type == MMC_TYPE_SDIO)
-		return sdio_enable_wide(card);
-
-	if ((card->host->caps & MMC_CAP_4_BIT_DATA) &&
-		(card->scr.bus_widths & SD_SCR_BUS_WIDTH_4)) {
+		err = sdio_enable_wide(card);
+	else if ((card->host->caps & MMC_CAP_4_BIT_DATA) &&
+		 (card->scr.bus_widths & SD_SCR_BUS_WIDTH_4)) {
 		err = mmc_app_set_bus_width(card, MMC_BUS_WIDTH_4);
 		if (err)
 			return err;
+		err = sdio_enable_wide(card);
+		if (err <= 0)
+			mmc_app_set_bus_width(card, MMC_BUS_WIDTH_1);
 	} else
 		return 0;
 
-	err = sdio_enable_wide(card);
-	if (err <= 0)
-		mmc_app_set_bus_width(card, MMC_BUS_WIDTH_1);
+	if (err > 0) {
+		mmc_set_bus_width(card->host, MMC_BUS_WIDTH_4);
+		err = 0;
+	}
 
 	return err;
 }
@@ -547,13 +550,8 @@ static int mmc_sdio_init_uhs_card(struct mmc_card *card)
 	/*
 	 * Switch to wider bus (if supported).
 	 */
-	if (card->host->caps & MMC_CAP_4_BIT_DATA) {
+	if (card->host->caps & MMC_CAP_4_BIT_DATA)
 		err = sdio_enable_4bit_bus(card);
-		if (err > 0) {
-			mmc_set_bus_width(card->host, MMC_BUS_WIDTH_4);
-			err = 0;
-		}
-	}
 
 	/* Set the driver strength for the card */
 	sdio_select_driver_type(card);
@@ -803,9 +801,7 @@ try_again:
 		 * Switch to wider bus (if supported).
 		 */
 		err = sdio_enable_4bit_bus(card);
-		if (err > 0)
-			mmc_set_bus_width(card->host, MMC_BUS_WIDTH_4);
-		else if (err)
+		if (err)
 			goto remove;
 	}
 finish:
@@ -983,10 +979,6 @@ static int mmc_sdio_resume(struct mmc_host *host)
 	} else if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) {
 		/* We may have switched to 1-bit mode during suspend */
 		err = sdio_enable_4bit_bus(host->card);
-		if (err > 0) {
-			mmc_set_bus_width(host, MMC_BUS_WIDTH_4);
-			err = 0;
-		}
 	}
 
 	if (!err && host->sdio_irqs) {

[PATCH 2/4] mmc: core: allow non-blocking form of mmc_claim_host

[NeilBrown]

Change the handling for the 'abort' flag so that if
it is set, but we can claim the host, then do the claim,
rather than aborting.

When the abort is async this just means that a race between aborting
an allowing a claim is resolved slightly differently.  Any
code must already be able to handle 'abort' being set just as the host
is claimed.

This allows extra functionality.  If __mmc_claim_host() is called
with an 'abort' pointer which is initialized to '1', it will effect a
non-blocking 'claim'.

Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/mmc/core/core.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index e9eb721e3664..051198073d21 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -912,10 +912,11 @@ int __mmc_claim_host(struct mmc_host *host, atomic_t *abort)
 		spin_lock_irqsave(&host->lock, flags);
 	}
 	set_current_state(TASK_RUNNING);
-	if (!stop) {
+	if (!host->claimed || host->claimer == current) {
 		host->claimed = 1;
 		host->claimer = current;
 		host->claim_cnt += 1;
+		stop = 0;
 	} else
 		wake_up(&host->wq);
 	spin_unlock_irqrestore(&host->lock, flags);

[PATCH 3/4] mmc: sdio: support switching to 1-bit before turning off clocks

[NeilBrown]

According to section 7.1.2 of

http://www.sandisk.com/media/File/OEM/Manuals/SD_SDIO_specsv1.pdf

    In the case where the interrupt mechanism is used to wake the host while
    the card is in a low power state (i.e. no clocks), Both the card and the
    host shall be placed into the 1-bit SD mode prior to stopping the clock.


This is particularly important for the Marvell "libertas" wifi chip
in the GTA04.  While in 4-bit mode it will only signal an interrupt
when the clock is running (which is why setting CLKEXTFREE is
important in omap_hsmmc).
In 1-bit mode, the interrupt is asynchronous (explained in OMAP3
TRM description of the CIRQ flag to MMCHS_STAT:

  In 1-bit mode, interrupt source is asynchronous (can be a source of
  asynchronous wakeup).
  In 4-bit mode, interrupt source is sampled during the interrupt
  cycle.

)

It is awkward to simply set 1-bit mode in ->runtime_suspend
as that will call mmc_set_ios which calls ops->set_ios(),
which will likely call pm_runtime_get_sync(), on the device that
is currently suspending.  This deadlocks.

So:
 - create a work_struct to schedule setting of 1-bit mode
 - introduce an 'sdio_narrowed' state flag which transitions:
     0 (normal) -> 1 (convert to 1-bit pending) ->
         2 (have switch to 1-bit mode) -> 0 (normal)
 - create a function mmc_sdio_want_no_clocks() which can be called
   when the driver wants to turn off clocks (presumably after an
   idle timeout).  This either succeeds (in 1-bit mode) or fails
   and schedules the work to switch to 1-bit mode.
 - when the host is claimed, if sdio_narrowed is 2, restore the
   4-bit bus
 - When the host is released, if sdio_narrowed is 1, then some
   caller other  than our worker claimed the host first, so
   clear sdio_narrowed.

This all allows a graceful and race-free switch to 1-bit mode
before switching off the clocks, if SDIO interrupts are enabled.

A host should call mmc_sdio_want_no_clocks() when about to turn of
clocks if sdio interrupts are enabled, and the ->disable() function
should not use a timeout (pm_runtime_put_autosuspend) if
->sdio_narrowed is 2.

Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/mmc/core/core.c  |   18 ++++++++++++++----
 drivers/mmc/core/sdio.c  |   42 +++++++++++++++++++++++++++++++++++++++++-
 include/linux/mmc/core.h |    2 ++
 include/linux/mmc/host.h |    2 ++
 4 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 051198073d21..21068fe75c30 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -921,8 +921,14 @@ int __mmc_claim_host(struct mmc_host *host, atomic_t *abort)
 		wake_up(&host->wq);
 	spin_unlock_irqrestore(&host->lock, flags);
 	remove_wait_queue(&host->wq, &wait);
-	if (host->ops->enable && !stop && host->claim_cnt == 1)
-		host->ops->enable(host);
+	if (!stop && host->claim_cnt == 1) {
+		if (host->ops->enable)
+			host->ops->enable(host);
+		if (atomic_read(&host->sdio_narrowed) == 2) {
+			sdio_enable_4bit_bus(host->card);
+			atomic_set(&host->sdio_narrowed, 0);
+		}
+	}
 	return stop;
 }
 
@@ -941,8 +947,12 @@ void mmc_release_host(struct mmc_host *host)
 
 	WARN_ON(!host->claimed);
 
-	if (host->ops->disable && host->claim_cnt == 1)
-		host->ops->disable(host);
+	if (host->claim_cnt == 1) {
+		if (atomic_read(&host->sdio_narrowed) == 1)
+			atomic_set(&host->sdio_narrowed, 0);
+		if (host->ops->disable)
+			host->ops->disable(host);
+	}
 
 	spin_lock_irqsave(&host->lock, flags);
 	if (--host->claim_cnt) {
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 5bc6c7dbbd60..9761e4d5f49b 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -288,7 +288,7 @@ static int sdio_disable_wide(struct mmc_card *card)
 }
 
 
-static int sdio_enable_4bit_bus(struct mmc_card *card)
+int sdio_enable_4bit_bus(struct mmc_card *card)
 {
 	int err;
 
@@ -313,6 +313,45 @@ static int sdio_enable_4bit_bus(struct mmc_card *card)
 	return err;
 }
 
+static void mmc_sdio_width_work(struct work_struct *work)
+{
+	struct mmc_host *host = container_of(work, struct mmc_host,
+					     sdio_width_work);
+	atomic_t noblock;
+
+	atomic_set(&noblock, 1);
+	if (__mmc_claim_host(host, &noblock))
+		return;
+	if (atomic_read(&host->sdio_narrowed) != 1) {
+		/* Nothing to do */
+		mmc_release_host(host);
+		return;
+	}
+	if (sdio_disable_wide(host->card) == 0)
+		atomic_set(&host->sdio_narrowed, 2);
+	else
+		atomic_set(&host->sdio_narrowed, 0);
+	mmc_release_host(host);
+}
+
+int mmc_sdio_want_no_clocks(struct mmc_host *host)
+{
+	if (!(host->caps & MMC_CAP_SDIO_IRQ) ||
+	    host->ios.bus_width == MMC_BUS_WIDTH_1)
+		/* Safe to turn off clocks */
+		return 1;
+
+
+	/* In 4-bit mode the card needs the clock
+	 * to deliver interrupts, so it isn't safe
+	 * to turn of clocks just yet
+	 */
+	atomic_add_unless(&host->sdio_narrowed, 1, 1);
+
+	schedule_work(&host->sdio_width_work);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mmc_sdio_want_no_clocks);
 
 /*
  * Test if the card supports high-speed mode and, if so, switch to it.
@@ -1100,6 +1139,7 @@ int mmc_attach_sdio(struct mmc_host *host)
 		goto err;
 	}
 
+	INIT_WORK(&host->sdio_width_work, mmc_sdio_width_work);
 	/*
 	 * Detect and init the card.
 	 */
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 160448f920ac..faf6d1be0971 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -212,4 +212,6 @@ struct device_node;
 extern u32 mmc_vddrange_to_ocrmask(int vdd_min, int vdd_max);
 extern int mmc_of_parse_voltage(struct device_node *np, u32 *mask);
 
+extern int sdio_enable_4bit_bus(struct mmc_card *card);
+extern int mmc_sdio_want_no_clocks(struct mmc_host *host);
 #endif /* LINUX_MMC_CORE_H */
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 0c8cbe5d1550..7e6a54c49a15 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -350,6 +350,8 @@ struct mmc_host {
 	struct task_struct	*sdio_irq_thread;
 	bool			sdio_irq_pending;
 	atomic_t		sdio_irq_thread_abort;
+	struct work_struct	sdio_width_work;
+	atomic_t		sdio_narrowed; /* 1==pending, 2==complete*/
 
 	mmc_pm_flag_t		pm_flags;	/* requested pm features */
 

[PATCH 4/4] mmc: omap_hsmmc: switch to 1-bit before stopping clocks.

[NeilBrown]

Make use of the new mmc_sdio_want_no_clocks() call to avoid stopping
clocks while SD Card interrupts are enabled and we aren't in
1-bit mode.

Also stop clocks immediately in omap_hsmmc_disable_fclk() if
1-bit mode has been entered for this purpose.

With this, I can use my libertas wifi with a 4-bit bus, with
interrupts and runtime power-management enabled, and get around
14Mb/sec throughput (which is the best I've seen).

Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/mmc/host/omap_hsmmc.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index f84cfb01716d..14fce3b92633 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1791,9 +1791,12 @@ static int omap_hsmmc_disable_fclk(struct mmc_host *mmc)
 {
 	struct omap_hsmmc_host *host = mmc_priv(mmc);
 
-	pm_runtime_mark_last_busy(host->dev);
-	pm_runtime_put_autosuspend(host->dev);
-
+	if (atomic_read(&mmc->sdio_narrowed) == 2)
+		pm_runtime_put_sync(host->dev);
+	else {
+		pm_runtime_mark_last_busy(host->dev);
+		pm_runtime_put_autosuspend(host->dev);
+	}
 	return 0;
 }
 
@@ -2311,6 +2314,10 @@ static int omap_hsmmc_runtime_suspend(struct device *dev)
 	spin_lock_irqsave(&host->irq_lock, flags);
 	if ((host->mmc->caps & MMC_CAP_SDIO_IRQ) &&
 	    (host->flags & HSMMC_SDIO_IRQ_ENABLED)) {
+		if (mmc_sdio_want_no_clocks(host->mmc) == 0) {
+			ret = -EBUSY;
+			goto abort;
+		}
 		/* disable sdio irq handling to prevent race */
 		OMAP_HSMMC_WRITE(host->base, ISE, 0);
 		OMAP_HSMMC_WRITE(host->base, IE, 0);

[PATCH 4/4] mmc: omap_hsmmc: switch to 1-bit before stopping clocks.

[NeilBrown]

Make use of the new mmc_sdio_want_no_clocks() call to avoid stopping
clocks while SD Card interrupts are enabled and we aren't in
1-bit mode.

Also stop clocks immediately in omap_hsmmc_disable_fclk() if
1-bit mode has been entered for this purpose.

With this, I can use my libertas wifi with a 4-bit bus, with
interrupts and runtime power-management enabled, and get around
14Mb/sec throughput (which is the best I've seen).

Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/mmc/host/omap_hsmmc.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index f84cfb01716d..14fce3b92633 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1791,9 +1791,12 @@ static int omap_hsmmc_disable_fclk(struct mmc_host *mmc)
 {
 	struct omap_hsmmc_host *host = mmc_priv(mmc);
 
-	pm_runtime_mark_last_busy(host->dev);
-	pm_runtime_put_autosuspend(host->dev);
-
+	if (atomic_read(&mmc->sdio_narrowed) == 2)
+		pm_runtime_put_sync(host->dev);
+	else {
+		pm_runtime_mark_last_busy(host->dev);
+		pm_runtime_put_autosuspend(host->dev);
+	}
 	return 0;
 }
 
@@ -2311,6 +2314,10 @@ static int omap_hsmmc_runtime_suspend(struct device *dev)
 	spin_lock_irqsave(&host->irq_lock, flags);
 	if ((host->mmc->caps & MMC_CAP_SDIO_IRQ) &&
 	    (host->flags & HSMMC_SDIO_IRQ_ENABLED)) {
+		if (mmc_sdio_want_no_clocks(host->mmc) == 0) {
+			ret = -EBUSY;
+			goto abort;
+		}
 		/* disable sdio irq handling to prevent race */
 		OMAP_HSMMC_WRITE(host->base, ISE, 0);
 		OMAP_HSMMC_WRITE(host->base, IE, 0);