[PATCH 0/8] sdhci: Move real work out of an atomic context

[PATCH 0/8] sdhci: Move real work out of an atomic context

[Anton Vorontsov]

Hi all,

Currently the sdhci driver does everything in the atomic context.
And what is worse, PIO transfers are made from the IRQ handler.

This causes huge latencies (up to 120 ms). On some P2020 SOCs,
DMA and card detection is broken, which means that kernel polls
for the card via PIO transfers every second. Needless to say
that this is quite bad.

So, this patch set reworks sdhci code to avoid atomic context,
almost completely. We only do two device memory operations
in the atomic context, and all the rest is threaded.

I noticed no throughput drop neither with PIO transfers nor
with DMA (tested on MPC8569E CPU), while latencies should be
greatly improved.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 0/8] sdhci: Move real work out of an atomic context

[Matt Fleming]

On Wed, 14 Jul 2010 17:07:28 +0400, Anton Vorontsov <avorontsov@mvista.com> wrote:
> Hi all,
> 
> Currently the sdhci driver does everything in the atomic context.
> And what is worse, PIO transfers are made from the IRQ handler.
> 
> This causes huge latencies (up to 120 ms). On some P2020 SOCs,
> DMA and card detection is broken, which means that kernel polls
> for the card via PIO transfers every second. Needless to say
> that this is quite bad.
> 
> So, this patch set reworks sdhci code to avoid atomic context,
> almost completely. We only do two device memory operations
> in the atomic context, and all the rest is threaded.
> 
> I noticed no throughput drop neither with PIO transfers nor
> with DMA (tested on MPC8569E CPU), while latencies should be
> greatly improved.

I haven't had time to read these patches in detail yet but they all seem
to be sensible changes. A very nice series!

Re: [PATCH 0/8] sdhci: Move real work out of an atomic context

[Andrew Morton]

On Wed, 14 Jul 2010 17:07:28 +0400
Anton Vorontsov <avorontsov@mvista.com> wrote:

> Hi all,
> 
> Currently the sdhci driver does everything in the atomic context.
> And what is worse, PIO transfers are made from the IRQ handler.
> 
> This causes huge latencies (up to 120 ms). On some P2020 SOCs,
> DMA and card detection is broken, which means that kernel polls
> for the card via PIO transfers every second. Needless to say
> that this is quite bad.
> 
> So, this patch set reworks sdhci code to avoid atomic context,
> almost completely. We only do two device memory operations
> in the atomic context, and all the rest is threaded.
> 
> I noticed no throughput drop neither with PIO transfers nor
> with DMA (tested on MPC8569E CPU), while latencies should be
> greatly improved.
> 

The patchset looks good to me, but it'd be nice to hear from the other
people who work on this code, please?

Re: [PATCH 0/8] sdhci: Move real work out of an atomic context

[Andrew Morton]

On Wed, 14 Jul 2010 17:07:28 +0400
Anton Vorontsov <avorontsov@mvista.com> wrote:

> Hi all,
> 
> Currently the sdhci driver does everything in the atomic context.
> And what is worse, PIO transfers are made from the IRQ handler.
> 
> This causes huge latencies (up to 120 ms). On some P2020 SOCs,
> DMA and card detection is broken, which means that kernel polls
> for the card via PIO transfers every second. Needless to say
> that this is quite bad.
> 
> So, this patch set reworks sdhci code to avoid atomic context,
> almost completely. We only do two device memory operations
> in the atomic context, and all the rest is threaded.
> 
> I noticed no throughput drop neither with PIO transfers nor
> with DMA (tested on MPC8569E CPU), while latencies should be
> greatly improved.
> 

This patchset isn't causing any problems yet, but may do so in the
future and will impact the validity of any testing.  It seems to be
kind of stuck.  Should I drop it all?

Re: [PATCH 0/8] sdhci: Move real work out of an atomic context

[Chris Ball]

Hi Andrew,

On Tue, Sep 07, 2010 at 03:38:13PM -0700, Andrew Morton wrote:
> > I noticed no throughput drop neither with PIO transfers nor
> > with DMA (tested on MPC8569E CPU), while latencies should be
> > greatly improved.
> 
> This patchset isn't causing any problems yet, but may do so in the
> future and will impact the validity of any testing.  It seems to be
> kind of stuck.  Should I drop it all?

I suggest keeping it -- I'll find time to test it out here soon, and
will keep it in mind as a possible regression cause.

Thanks,

-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 0/8] sdhci: Move real work out of an atomic context

[Anton Vorontsov]

On Wed, Sep 08, 2010 at 10:37:41PM +0100, Chris Ball wrote:
> Hi Andrew,
> 
> On Tue, Sep 07, 2010 at 03:38:13PM -0700, Andrew Morton wrote:
> > > I noticed no throughput drop neither with PIO transfers nor
> > > with DMA (tested on MPC8569E CPU), while latencies should be
> > > greatly improved.
> > 
> > This patchset isn't causing any problems yet, but may do so in the
> > future and will impact the validity of any testing.  It seems to be
> > kind of stuck.  Should I drop it all?
> 
> I suggest keeping it -- I'll find time to test it out here soon, and
> will keep it in mind as a possible regression cause.

Thanks!

Would be also great if you could point out which patch causes
most of the performance drop (if any)?

Albert, if you could find time, can you also "bisect" the
patchset? I wouldn't want to buy Nintendo WII just to debug the
perf regression. ;-) FWIW, I tried to disable multiblock
read/writes and test with SD cards, and still didn't notice
any performance drops.

Maybe it's SDIO IRQs that cause the performance drop for the
WII case, as we delay them a little bit? Or it could be the
patch that introduces threaded IRQ handler in whole causes
it. If so, I guess we'd need to move some of the processing to
the real IRQ context, keeping the handler lockless (if
possible) or introducing a very fine grained locking.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 0/8] sdhci: Move real work out of an atomic context

[Chris Ball]

Hi Anton,

On Thu, Sep 09, 2010 at 01:57:50AM +0400, Anton Vorontsov wrote:
> Thanks!
> 
> Would be also great if you could point out which patch causes
> most of the performance drop (if any)?
> 
> Albert, if you could find time, can you also "bisect" the
> patchset? I wouldn't want to buy Nintendo WII just to debug the
> perf regression. ;-) FWIW, I tried to disable multiblock
> read/writes and test with SD cards, and still didn't notice
> any performance drops.
> 
> Maybe it's SDIO IRQs that cause the performance drop for the
> WII case, as we delay them a little bit? Or it could be the
> patch that introduces threaded IRQ handler in whole causes
> it. If so, I guess we'd need to move some of the processing to
> the real IRQ context, keeping the handler lockless (if
> possible) or introducing a very fine grained locking.

I didn't know anything about a reported performance drop, and I don't
think Andrew did either -- Albert's test results don't seem to have
made it to this list, or anywhere else that I can see.  Could you 
link to/repost his comments?

(I'll be testing with libertas, so that will stress-test SDIO IRQs.)

Thanks,

-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 0/8] sdhci: Move real work out of an atomic context

[Anton Vorontsov]

On Wed, Sep 08, 2010 at 11:05:48PM +0100, Chris Ball wrote:
> Hi Anton,
> 
> On Thu, Sep 09, 2010 at 01:57:50AM +0400, Anton Vorontsov wrote:
> > Thanks!
> > 
> > Would be also great if you could point out which patch causes
> > most of the performance drop (if any)?
> > 
> > Albert, if you could find time, can you also "bisect" the
> > patchset? I wouldn't want to buy Nintendo WII just to debug the
> > perf regression. ;-) FWIW, I tried to disable multiblock
> > read/writes and test with SD cards, and still didn't notice
> > any performance drops.
> > 
> > Maybe it's SDIO IRQs that cause the performance drop for the
> > WII case, as we delay them a little bit? Or it could be the
> > patch that introduces threaded IRQ handler in whole causes
> > it. If so, I guess we'd need to move some of the processing to
> > the real IRQ context, keeping the handler lockless (if
> > possible) or introducing a very fine grained locking.
> 
> I didn't know anything about a reported performance drop, and I don't
> think Andrew did either -- Albert's test results don't seem to have
> made it to this list, or anywhere else that I can see.  Could you 
> link to/repost his comments?
> 
> (I'll be testing with libertas, so that will stress-test SDIO IRQs.)

Sure thing, here are Albert's results.

----- Forwarded message from Albert Herranz <albert_herranz@yahoo.es> -----

Date: Mon, 02 Aug 2010 21:23:51 +0200
From: Albert Herranz <albert_herranz@yahoo.es>
To: Anton Vorontsov <cbouatmailru@gmail.com>
CC: akpm@linux-foundation.org, mm-commits@vger.kernel.org,
	ben-linux@fluff.org, matt@console-pimps.org, pierre@ossman.eu,
	w.sang@pengutronix.de, mb@bu3sch.de
Subject: Re: + sdhci-use-work-structs-instead-of-tasklets.patch added to -mm
	tree

Hi,

Some initial numbers regarding performance. The patchset seems to cause a noticeable performance drop.
I've run two iperf client tests (see the two invocations of iperf -c) and two iperf server tests (see iperf -s invocation).

== 2.6.33 ==

$ iperf -c 192.168.1.130 
------------------------------------------------------------
Client connecting to 192.168.1.130, TCP port 5001
TCP window size: 16.0 KByte (default)
------------------------------------------------------------
[  3] local 192.168.1.127 port 40119 connected with 192.168.1.130 port 5001
[ ID] Interval       Transfer     Bandwidth
[  3]  0.0-10.1 sec  1.05 MBytes    872 Kbits/sec

$ iperf -c 192.168.1.130 
------------------------------------------------------------
Client connecting to 192.168.1.130, TCP port 5001
TCP window size: 16.0 KByte (default)
------------------------------------------------------------
[  3] local 192.168.1.127 port 40120 connected with 192.168.1.130 port 5001
[ ID] Interval       Transfer     Bandwidth
[  3]  0.0-10.0 sec  1.04 MBytes    870 Kbits/sec

$ iperf -s
------------------------------------------------------------
Server listening on TCP port 5001
TCP window size: 85.3 KByte (default)
------------------------------------------------------------
[  4] local 192.168.1.127 port 5001 connected with 192.168.1.130 port 36691
[ ID] Interval       Transfer     Bandwidth
[  4]  0.0-10.2 sec  3.61 MBytes  2.98 Mbits/sec
[  5] local 192.168.1.127 port 5001 connected with 192.168.1.130 port 36692
[  5]  0.0-10.1 sec  4.94 MBytes  4.09 Mbits/sec


== 2.6.33 + "sdhci: Move real work out of an atomic context" patchset ==

$ iperf -c 192.168.1.130 
------------------------------------------------------------
Client connecting to 192.168.1.130, TCP port 5001
TCP window size: 16.0 KByte (default)
------------------------------------------------------------
[  3] local 192.168.1.127 port 39210 connected with 192.168.1.130 port 5001
[ ID] Interval       Transfer     Bandwidth
[  3]  0.0-10.0 sec    368 KBytes    301 Kbits/sec

$ iperf -c 192.168.1.130 
------------------------------------------------------------
Client connecting to 192.168.1.130, TCP port 5001
TCP window size: 16.0 KByte (default)
------------------------------------------------------------
[  3] local 192.168.1.127 port 39211 connected with 192.168.1.130 port 5001
[ ID] Interval       Transfer     Bandwidth
[  3]  0.0-10.2 sec    440 KBytes    354 Kbits/sec

$ iperf -s
------------------------------------------------------------
Server listening on TCP port 5001
TCP window size: 85.3 KByte (default)
------------------------------------------------------------
[  4] local 192.168.1.127 port 5001 connected with 192.168.1.130 port 57833
[ ID] Interval       Transfer     Bandwidth
[  4]  0.0-10.2 sec  2.37 MBytes  1.95 Mbits/sec
[  5] local 192.168.1.127 port 5001 connected with 192.168.1.130 port 57834
[  5]  0.0-10.2 sec  2.30 MBytes  1.90 Mbits/sec

The subjective feeling is too that the system is slower.

Cheers,
Albert

----- End forwarded message -----

Re: [PATCH 0/8] sdhci: Move real work out of an atomic context

[Chris Ball]

Hi,

On Wed, Sep 08, 2010 at 10:37:41PM +0100, Chris Ball wrote:
> Hi Andrew,
> 
> On Tue, Sep 07, 2010 at 03:38:13PM -0700, Andrew Morton wrote:
> > > I noticed no throughput drop neither with PIO transfers nor
> > > with DMA (tested on MPC8569E CPU), while latencies should be
> > > greatly improved.
> > 
> > This patchset isn't causing any problems yet, but may do so in the
> > future and will impact the validity of any testing.  It seems to be
> > kind of stuck.  Should I drop it all?
> 
> I suggest keeping it -- I'll find time to test it out here soon, and
> will keep it in mind as a possible regression cause.

Am running this now.  The first thing I'm noticing is a repeated BUG():

[    7.288186] Write protecting the kernel read-only data: 1072k
[    7.306446] BUG: sleeping function called from invalid context at kernel/mutex.c:94
[    7.324375] in_atomic(): 1, irqs_disabled(): 0, pid: 532, name: mmc2/0
[    7.340989] Pid: 532, comm: mmc2/0 Not tainted 2.6.35.4_xo1.5-20100908.2141.olpc.44f3b38_DIRTY #1
[    7.360129] Call Trace:
[    7.372843]  [<b04193ce>] __might_sleep+0xd9/0xe0
[    7.387864]  [<b07260cc>] mutex_lock+0x1c/0x2a
[    7.402576]  [<b06396e8>] sdhci_led_control+0x1a/0x41
[    7.417727]  [<b063bece>] led_trigger_event+0x42/0x5c
[    7.432807]  [<b06326f8>] mmc_request_done+0x56/0x6f
[    7.447597]  [<b063a2d1>] sdhci_finish_work+0xc8/0xcd
[    7.462643]  [<b063a209>] ? sdhci_finish_work+0x0/0xcd
[    7.477941]  [<b0432776>] worker_thread+0x165/0x1ed
[    7.492856]  [<b063a209>] ? sdhci_finish_work+0x0/0xcd
[    7.508204]  [<b0435591>] ? autoremove_wake_function+0x0/0x34
[    7.524178]  [<b0432611>] ? worker_thread+0x0/0x1ed
[    7.538953]  [<b04352a0>] kthread+0x63/0x68
[    7.552659]  [<b043523d>] ? kthread+0x0/0x68
[    7.566349]  [<b0402cf6>] kernel_thread_helper+0x6/0x10
[    7.709931] udev: starting version 141
[    7.940374] mmc2: new high speed SDHC card at address e4da
[    8.058165] mmcblk0: mmc2:e4da SU04G 3.69 GiB 
[    8.135730]  mmcblk0: p1 p2

Full dmesg is at http://chris.printf.net/anton-mutex-dmesg.txt. 
Anton, the kernel is 2.6.35.4-olpc plus your patchset from -mm.
I can think about how to test on an upstream kernel instead, but
perhaps your own tests simply didn't hit sdhci_led_control(). 

Andrew, if you want to drop this while the BUG() and potential
performance regressions are worked out, I'd be happy to keep 
testing patches from Anton until it's without regressions here.

Thanks,

-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 0/8] sdhci: Move real work out of an atomic context

[Anton Vorontsov]

On Thu, Sep 09, 2010 at 03:28:34AM +0100, Chris Ball wrote:
[...]
> [    7.372843]  [<b04193ce>] __might_sleep+0xd9/0xe0
> [    7.387864]  [<b07260cc>] mutex_lock+0x1c/0x2a
> [    7.402576]  [<b06396e8>] sdhci_led_control+0x1a/0x41
> [    7.417727]  [<b063bece>] led_trigger_event+0x42/0x5c

led_trigger_even grabs a readlock. :-(

> [    7.432807]  [<b06326f8>] mmc_request_done+0x56/0x6f
> [    7.447597]  [<b063a2d1>] sdhci_finish_work+0xc8/0xcd
> [    7.462643]  [<b063a209>] ? sdhci_finish_work+0x0/0xcd
> [    7.477941]  [<b0432776>] worker_thread+0x165/0x1ed
> [    7.492856]  [<b063a209>] ? sdhci_finish_work+0x0/0xcd
> [    7.508204]  [<b0435591>] ? autoremove_wake_function+0x0/0x34
> [    7.524178]  [<b0432611>] ? worker_thread+0x0/0x1ed
> [    7.538953]  [<b04352a0>] kthread+0x63/0x68
> [    7.552659]  [<b043523d>] ? kthread+0x0/0x68
> [    7.566349]  [<b0402cf6>] kernel_thread_helper+0x6/0x10
> [    7.709931] udev: starting version 141
> [    7.940374] mmc2: new high speed SDHC card at address e4da
> [    8.058165] mmcblk0: mmc2:e4da SU04G 3.69 GiB 
> [    8.135730]  mmcblk0: p1 p2
> 
> Full dmesg is at http://chris.printf.net/anton-mutex-dmesg.txt. 
> Anton, the kernel is 2.6.35.4-olpc plus your patchset from -mm.
> I can think about how to test on an upstream kernel instead, but
> perhaps your own tests simply didn't hit sdhci_led_control(). 

Yep, LEDS support was turned off.

> Andrew, if you want to drop this while the BUG() and potential
> performance regressions are worked out, I'd be happy to keep 
> testing patches from Anton until it's without regressions here.

Thanks Chris.

I also think that it's better to drop these series now,
and meanwhile I'll try to prepare another patchset.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

[PATCH 0/8] sdhci: Move real work out of an atomic context

[Jeremie Samuel]

Hi all,

Currently the sdhci driver does everything in the atomic context.
And what is worse, PIO transfers are made from the IRQ handler.

Some patches were already submitted to solve this issue. But there were 
rejected because they involved new issues.

This set of patches is an evolution of an old patch from Anton Vorontsov.
I tried to fix all the problems involved by the patches. I tested it for 
several time now with SD cards and SDIO.

So, this patch set reworks sdhci code to avoid atomic context,
almost completely.

Thanks,

Jeremie Samuel

Jeremie Samuel (8):
  sdhci: Turn timeout timer into delayed work
  sdhci: Turn tuning timeout timer into delayed work
  sdhci: Use work structs instead of tasklets
  sdhci: Use threaded IRQ handler
  sdhci: Delay led blinking
  sdhci: Turn host->lock into a mutex
  sdhci: Get rid of mdelay()s where it is safe and makes sense
  sdhci: Use jiffies instead of a timeout counter

 drivers/mmc/host/sdhci.c  |  327 ++++++++++++++++++++++-----------------------
 include/linux/mmc/sdhci.h |   13 +-
 2 files changed, 168 insertions(+), 172 deletions(-)

-- 
1.7.10.4

[PATCH 1/8] sdhci: Turn timeout timer into delayed work

[Jeremie Samuel]

There is no need for the timeout handler to run in the atomic
context, so this patch turns timeout timeout into the delayed
work.

Note that the timeout handler still grabs an irqsave spinlock,
we'll deal with it in a separate patch.

Patch based on http://thread.gmane.org/gmane.linux.kernel.mmc/2579.

Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
Signed-off-by: Jeremie Samuel <jeremie.samuel.ext@parrot.com>
---
 drivers/mmc/host/sdhci.c  |   13 +++++++------
 include/linux/mmc/sdhci.h |    3 ++-
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 2ea429c..9d75338 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -13,6 +13,7 @@
  *     - JMicron (hardware and technical support)
  */
 
+#include <linux/workqueue.h>
 #include <linux/delay.h>
 #include <linux/highmem.h>
 #include <linux/io.h>
@@ -1002,7 +1003,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 		mdelay(1);
 	}
 
-	mod_timer(&host->timer, jiffies + 10 * HZ);
+	schedule_delayed_work(&host->timeout_work, 10 * HZ);
 
 	host->cmd = cmd;
 
@@ -2113,7 +2114,7 @@ static void sdhci_tasklet_finish(unsigned long param)
 		return;
 	}
 
-	del_timer(&host->timer);
+	cancel_delayed_work(&host->timeout_work);
 
 	mrq = host->mrq;
 
@@ -2153,12 +2154,12 @@ static void sdhci_tasklet_finish(unsigned long param)
 	sdhci_runtime_pm_put(host);
 }
 
-static void sdhci_timeout_timer(unsigned long data)
+static void sdhci_timeout_work(struct work_struct *wk)
 {
 	struct sdhci_host *host;
 	unsigned long flags;
 
-	host = (struct sdhci_host*)data;
+	host = container_of(wk, struct sdhci_host, timeout_work.work);
 
 	spin_lock_irqsave(&host->lock, flags);
 
@@ -3163,7 +3164,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	tasklet_init(&host->finish_tasklet,
 		sdhci_tasklet_finish, (unsigned long)host);
 
-	setup_timer(&host->timer, sdhci_timeout_timer, (unsigned long)host);
+	INIT_DELAYED_WORK(&host->timeout_work, sdhci_timeout_work);
 
 	if (host->version >= SDHCI_SPEC_300) {
 		init_waitqueue_head(&host->buf_ready_int);
@@ -3266,7 +3267,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
 	sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
 	free_irq(host->irq, host);
 
-	del_timer_sync(&host->timer);
+	flush_delayed_work(&host->timeout_work);
 
 	tasklet_kill(&host->card_tasklet);
 	tasklet_kill(&host->finish_tasklet);
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index b838ffc..0f14546 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -12,6 +12,7 @@
 #define LINUX_MMC_SDHCI_H
 
 #include <linux/scatterlist.h>
+#include <linux/workqueue.h>
 #include <linux/compiler.h>
 #include <linux/types.h>
 #include <linux/io.h>
@@ -159,7 +160,7 @@ struct sdhci_host {
 	struct tasklet_struct card_tasklet;	/* Tasklet structures */
 	struct tasklet_struct finish_tasklet;
 
-	struct timer_list timer;	/* Timer for timeouts */
+	struct delayed_work	timeout_work;	/* Work for timeouts */
 
 	u32 caps;		/* Alternative CAPABILITY_0 */
 	u32 caps1;		/* Alternative CAPABILITY_1 */
-- 
1.7.10.4

[PATCH 2/8] sdhci: Turn tuning timeout timer into delayed work

[Jeremie Samuel]

Same as previous patch

Signed-off-by: Jeremie Samuel <jeremie.samuel.ext@parrot.com>
---
 drivers/mmc/host/sdhci.c  |   46 ++++++++++++++++++++++-----------------------
 include/linux/mmc/sdhci.h |    2 +-
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 9d75338..7d7033b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -53,7 +53,7 @@ static void sdhci_finish_data(struct sdhci_host *);
 static void sdhci_send_command(struct sdhci_host *, struct mmc_command *);
 static void sdhci_finish_command(struct sdhci_host *);
 static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode);
-static void sdhci_tuning_timer(unsigned long data);
+static void sdhci_tuning_timeout_work(struct work_struct *wk);
 static void sdhci_enable_preset_value(struct sdhci_host *host, bool enable);
 
 #ifdef CONFIG_PM_RUNTIME
@@ -256,7 +256,7 @@ static void sdhci_reinit(struct sdhci_host *host)
 	if (host->flags & SDHCI_USING_RETUNING_TIMER) {
 		host->flags &= ~SDHCI_USING_RETUNING_TIMER;
 
-		del_timer_sync(&host->tuning_timer);
+		flush_delayed_work(&host->tuning_timeout_work);
 		host->flags &= ~SDHCI_NEEDS_RETUNING;
 		host->mmc->max_blk_count =
 			(host->quirks & SDHCI_QUIRK_NO_MULTIBLOCK) ? 1 : 65535;
@@ -1363,7 +1363,7 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 		present_state = sdhci_readl(host, SDHCI_PRESENT_STATE);
 		/*
-		 * Check if the re-tuning timer has already expired and there
+		 * Check if the re-tuning timeout has already expired and there
 		 * is no on-going data transfer. If so, we need to execute
 		 * tuning procedure before sending command.
 		 */
@@ -1978,32 +1978,33 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 out:
 	/*
 	 * If this is the very first time we are here, we start the retuning
-	 * timer. Since only during the first time, SDHCI_NEEDS_RETUNING
-	 * flag won't be set, we check this condition before actually starting
-	 * the timer.
+	 * timeout workqueue. Since only during the first time,
+	 * SDHCI_NEEDS_RETUNING flag won't be set, we check this condition
+	 * before actually starting the timeout worqueue.
 	 */
 	if (!(host->flags & SDHCI_NEEDS_RETUNING) && host->tuning_count &&
 	    (host->tuning_mode == SDHCI_TUNING_MODE_1)) {
 		host->flags |= SDHCI_USING_RETUNING_TIMER;
-		mod_timer(&host->tuning_timer, jiffies +
+		schedule_delayed_work(&host->tuning_timeout_work,
 			host->tuning_count * HZ);
 		/* Tuning mode 1 limits the maximum data length to 4MB */
 		mmc->max_blk_count = (4 * 1024 * 1024) / mmc->max_blk_size;
 	} else {
 		host->flags &= ~SDHCI_NEEDS_RETUNING;
-		/* Reload the new initial value for timer */
+		/* Reload the new initial value for timeout workqueue */
 		if (host->tuning_mode == SDHCI_TUNING_MODE_1)
-			mod_timer(&host->tuning_timer, jiffies +
+			schedule_delayed_work(&host->tuning_timeout_work,
 				host->tuning_count * HZ);
 	}
 
 	/*
 	 * In case tuning fails, host controllers which support re-tuning can
-	 * try tuning again at a later time, when the re-tuning timer expires.
+	 * try tuning again at a later time, when the re-tuning timeout
+	 * workqueue expires.
 	 * So for these controllers, we return 0. Since there might be other
 	 * controllers who do not have this capability, we return error for
 	 * them. SDHCI_USING_RETUNING_TIMER means the host is currently using
-	 * a retuning timer to do the retuning for the card.
+	 * a retuning timeout workqueue to do the retuning for the card.
 	 */
 	if (err && (host->flags & SDHCI_USING_RETUNING_TIMER))
 		err = 0;
@@ -2185,12 +2186,12 @@ static void sdhci_timeout_work(struct work_struct *wk)
 	spin_unlock_irqrestore(&host->lock, flags);
 }
 
-static void sdhci_tuning_timer(unsigned long data)
+static void sdhci_tuning_timeout_work(struct work_struct *wk)
 {
 	struct sdhci_host *host;
 	unsigned long flags;
 
-	host = (struct sdhci_host *)data;
+	host = container_of(wk, struct sdhci_host, tuning_timeout_work.work);
 
 	spin_lock_irqsave(&host->lock, flags);
 
@@ -2537,7 +2538,7 @@ int sdhci_suspend_host(struct sdhci_host *host)
 
 	/* Disable tuning since we are suspending */
 	if (host->flags & SDHCI_USING_RETUNING_TIMER) {
-		del_timer_sync(&host->tuning_timer);
+		flush_delayed_work(&host->tuning_timeout_work);
 		host->flags &= ~SDHCI_NEEDS_RETUNING;
 	}
 
@@ -2545,7 +2546,7 @@ int sdhci_suspend_host(struct sdhci_host *host)
 	if (ret) {
 		if (host->flags & SDHCI_USING_RETUNING_TIMER) {
 			host->flags |= SDHCI_NEEDS_RETUNING;
-			mod_timer(&host->tuning_timer, jiffies +
+			schedule_delayed_work(&host->tuning_timeout_work,
 					host->tuning_count * HZ);
 		}
 
@@ -2633,7 +2634,7 @@ int sdhci_runtime_suspend_host(struct sdhci_host *host)
 
 	/* Disable tuning since we are suspending */
 	if (host->flags & SDHCI_USING_RETUNING_TIMER) {
-		del_timer_sync(&host->tuning_timer);
+		flush_delayed_work(&host->tuning_timeout_work);
 		host->flags &= ~SDHCI_NEEDS_RETUNING;
 	}
 
@@ -2987,13 +2988,13 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (caps[1] & SDHCI_DRIVER_TYPE_D)
 		mmc->caps |= MMC_CAP_DRIVER_TYPE_D;
 
-	/* Initial value for re-tuning timer count */
+	/* Initial value for re-tuning timeout count */
 	host->tuning_count = (caps[1] & SDHCI_RETUNING_TIMER_COUNT_MASK) >>
 			      SDHCI_RETUNING_TIMER_COUNT_SHIFT;
 
 	/*
-	 * In case Re-tuning Timer is not disabled, the actual value of
-	 * re-tuning timer will be 2 ^ (n - 1).
+	 * In case Re-tuning Timeout is not disabled, the actual value of
+	 * re-tuning timeout will be 2 ^ (n - 1).
 	 */
 	if (host->tuning_count)
 		host->tuning_count = 1 << (host->tuning_count - 1);
@@ -3169,10 +3170,9 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (host->version >= SDHCI_SPEC_300) {
 		init_waitqueue_head(&host->buf_ready_int);
 
-		/* Initialize re-tuning timer */
-		init_timer(&host->tuning_timer);
-		host->tuning_timer.data = (unsigned long)host;
-		host->tuning_timer.function = sdhci_tuning_timer;
+		/* Initialize re-tuning timeout work */
+		INIT_DELAYED_WORK(&host->tuning_timeout_work,
+					sdhci_tuning_timeout_work);
 	}
 
 	ret = request_irq(host->irq, sdhci_irq, IRQF_SHARED,
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 0f14546..d0f4845 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -175,7 +175,7 @@ struct sdhci_host {
 	unsigned int		tuning_count;	/* Timer count for re-tuning */
 	unsigned int		tuning_mode;	/* Re-tuning mode supported by host */
 #define SDHCI_TUNING_MODE_1	0
-	struct timer_list	tuning_timer;	/* Timer for tuning */
+	struct delayed_work	tuning_timeout_work;	/* Work for tuning timeouts */
 
 	unsigned long private[0] ____cacheline_aligned;
 };
-- 
1.7.10.4

[PATCH 3/8] sdhci: Use work structs instead of tasklets

[Jeremie Samuel]

The driver can happily live without an atomic context and tasklets,
so turn the tasklets into the work structs.

Tasklets handlers still grab irqsave spinlocks, but we'll deal
with it in a separate patch.

Patch based on: http://thread.gmane.org/gmane.linux.kernel.mmc/2579.

Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
Signed-off-by: Jeremie Samuel <jeremie.samuel.ext@parrot.com>
---
 drivers/mmc/host/sdhci.c  |   55 +++++++++++++++++++++------------------------
 include/linux/mmc/sdhci.h |    4 ++--
 2 files changed, 27 insertions(+), 32 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 7d7033b..063c20a 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -967,7 +967,7 @@ static void sdhci_finish_data(struct sdhci_host *host)
 
 		sdhci_send_command(host, data->stop);
 	} else
-		tasklet_schedule(&host->finish_tasklet);
+		schedule_work(&host->finish_work);
 }
 
 static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
@@ -996,7 +996,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 				"inhibit bit(s).\n", mmc_hostname(host->mmc));
 			sdhci_dumpregs(host);
 			cmd->error = -EIO;
-			tasklet_schedule(&host->finish_tasklet);
+			schedule_work(&host->finish_work);
 			return;
 		}
 		timeout--;
@@ -1017,7 +1017,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 		pr_err("%s: Unsupported response type!\n",
 			mmc_hostname(host->mmc));
 		cmd->error = -EINVAL;
-		tasklet_schedule(&host->finish_tasklet);
+		schedule_work(&host->finish_work);
 		return;
 	}
 
@@ -1078,7 +1078,7 @@ static void sdhci_finish_command(struct sdhci_host *host)
 			sdhci_finish_data(host);
 
 		if (!host->cmd->data)
-			tasklet_schedule(&host->finish_tasklet);
+			schedule_work(&host->finish_work);
 
 		host->cmd = NULL;
 	}
@@ -1357,7 +1357,7 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	if (!present || host->flags & SDHCI_DEVICE_DEAD) {
 		host->mrq->cmd->error = -ENOMEDIUM;
-		tasklet_schedule(&host->finish_tasklet);
+		schedule_work(&host->finish_work);
 	} else {
 		u32 present_state;
 
@@ -2062,7 +2062,7 @@ static void sdhci_card_event(struct mmc_host *mmc)
 		sdhci_reset(host, SDHCI_RESET_DATA);
 
 		host->mrq->cmd->error = -ENOMEDIUM;
-		tasklet_schedule(&host->finish_tasklet);
+		schedule_work(&host->finish_work);
 	}
 
 	spin_unlock_irqrestore(&host->lock, flags);
@@ -2087,29 +2087,30 @@ static const struct mmc_host_ops sdhci_ops = {
  *                                                                           *
 \*****************************************************************************/
 
-static void sdhci_tasklet_card(unsigned long param)
+static void sdhci_card_detect_work(struct work_struct *wk)
 {
-	struct sdhci_host *host = (struct sdhci_host*)param;
+	struct sdhci_host *host = container_of(wk, struct sdhci_host,
+						   card_detect_work);
 
 	sdhci_card_event(host->mmc);
 
 	mmc_detect_change(host->mmc, msecs_to_jiffies(200));
 }
 
-static void sdhci_tasklet_finish(unsigned long param)
+static void sdhci_finish_work(struct work_struct *wk)
 {
 	struct sdhci_host *host;
 	unsigned long flags;
 	struct mmc_request *mrq;
 
-	host = (struct sdhci_host*)param;
+	host = container_of(wk, struct sdhci_host, finish_work);
 
 	spin_lock_irqsave(&host->lock, flags);
 
-        /*
-         * If this tasklet gets rescheduled while running, it will
-         * be run again afterwards but without any active request.
-         */
+	/*
+	 * If this work gets rescheduled while running, it will
+	 * be run again afterwards but without any active request.
+	 */
 	if (!host->mrq) {
 		spin_unlock_irqrestore(&host->lock, flags);
 		return;
@@ -2178,7 +2179,7 @@ static void sdhci_timeout_work(struct work_struct *wk)
 			else
 				host->mrq->cmd->error = -ETIMEDOUT;
 
-			tasklet_schedule(&host->finish_tasklet);
+			schedule_work(&host->finish_work);
 		}
 	}
 
@@ -2225,7 +2226,7 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask)
 		host->cmd->error = -EILSEQ;
 
 	if (host->cmd->error) {
-		tasklet_schedule(&host->finish_tasklet);
+		schedule_work(&host->finish_work);
 		return;
 	}
 
@@ -2434,7 +2435,7 @@ again:
 		sdhci_writel(host, intmask & (SDHCI_INT_CARD_INSERT |
 			     SDHCI_INT_CARD_REMOVE), SDHCI_INT_STATUS);
 		intmask &= ~(SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE);
-		tasklet_schedule(&host->card_tasklet);
+		schedule_work(&host->card_detect_work);
 	}
 
 	if (intmask & SDHCI_INT_CMD_MASK) {
@@ -3158,12 +3159,10 @@ int sdhci_add_host(struct sdhci_host *host)
 	mmc->max_blk_count = (host->quirks & SDHCI_QUIRK_NO_MULTIBLOCK) ? 1 : 65535;
 
 	/*
-	 * Init tasklets.
+	 * Init work structs.
 	 */
-	tasklet_init(&host->card_tasklet,
-		sdhci_tasklet_card, (unsigned long)host);
-	tasklet_init(&host->finish_tasklet,
-		sdhci_tasklet_finish, (unsigned long)host);
+	INIT_WORK(&host->card_detect_work, sdhci_card_detect_work);
+	INIT_WORK(&host->finish_work, sdhci_finish_work);
 
 	INIT_DELAYED_WORK(&host->timeout_work, sdhci_timeout_work);
 
@@ -3180,7 +3179,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (ret) {
 		pr_err("%s: Failed to request IRQ %d: %d\n",
 		       mmc_hostname(mmc), host->irq, ret);
-		goto untasklet;
+		return ret;
 	}
 
 	sdhci_init(host, 0);
@@ -3224,10 +3223,6 @@ reset:
 	sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
 	free_irq(host->irq, host);
 #endif
-untasklet:
-	tasklet_kill(&host->card_tasklet);
-	tasklet_kill(&host->finish_tasklet);
-
 	return ret;
 }
 
@@ -3247,7 +3242,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
 				" transfer!\n", mmc_hostname(host->mmc));
 
 			host->mrq->cmd->error = -ENOMEDIUM;
-			tasklet_schedule(&host->finish_tasklet);
+			schedule_work(&host->finish_work);
 		}
 
 		spin_unlock_irqrestore(&host->lock, flags);
@@ -3269,8 +3264,8 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
 
 	flush_delayed_work(&host->timeout_work);
 
-	tasklet_kill(&host->card_tasklet);
-	tasklet_kill(&host->finish_tasklet);
+	flush_work(&host->card_detect_work);
+	flush_work(&host->finish_work);
 
 	if (host->vmmc) {
 		regulator_disable(host->vmmc);
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index d0f4845..152acb5 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -157,8 +157,8 @@ struct sdhci_host {
 	dma_addr_t adma_addr;	/* Mapped ADMA descr. table */
 	dma_addr_t align_addr;	/* Mapped bounce buffer */
 
-	struct tasklet_struct card_tasklet;	/* Tasklet structures */
-	struct tasklet_struct finish_tasklet;
+	struct work_struct	card_detect_work;
+	struct work_struct	finish_work;
 
 	struct delayed_work	timeout_work;	/* Work for timeouts */
 
-- 
1.7.10.4

[PATCH 4/8] sdhci: Use threaded IRQ handler

[Jeremie Samuel]

We only need atomic context to disable SDHCI interrupts, after that
we can run in a kernel thread.

Note that irq handler still grabs an irqsave spinlock, we'll deal
with it in a subsequent patch.

Patch based on: http://thread.gmane.org/gmane.linux.kernel.mmc/2579.

Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
Signed-off-by: Jeremie Samuel <jeremie.samuel.ext@parrot.com>
---
 drivers/mmc/host/sdhci.c |   46 ++++++++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 063c20a..f361fec 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2386,9 +2386,8 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
 	}
 }
 
-static irqreturn_t sdhci_irq(int irq, void *dev_id)
+static irqreturn_t sdhci_irq_thread(int irq, void *dev_id)
 {
-	irqreturn_t result;
 	struct sdhci_host *host = dev_id;
 	u32 intmask, unexpected = 0;
 	int cardint = 0, max_loops = 16;
@@ -2404,15 +2403,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
 
 	intmask = sdhci_readl(host, SDHCI_INT_STATUS);
 
-	if (!intmask || intmask == 0xffffffff) {
-		result = IRQ_NONE;
-		goto out;
-	}
-
 again:
-	DBG("*** %s got interrupt: 0x%08x\n",
-		mmc_hostname(host->mmc), intmask);
-
 	if (intmask & (SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE)) {
 		u32 present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
 			      SDHCI_CARD_PRESENT;
@@ -2472,12 +2463,10 @@ again:
 		sdhci_writel(host, intmask, SDHCI_INT_STATUS);
 	}
 
-	result = IRQ_HANDLED;
-
 	intmask = sdhci_readl(host, SDHCI_INT_STATUS);
 	if (intmask && --max_loops)
 		goto again;
-out:
+
 	spin_unlock(&host->lock);
 
 	if (unexpected) {
@@ -2491,7 +2480,27 @@ out:
 	if (cardint)
 		mmc_signal_sdio_irq(host->mmc);
 
-	return result;
+	intmask = sdhci_readl(host, SDHCI_INT_ENABLE);
+	sdhci_writel(host, intmask, SDHCI_SIGNAL_ENABLE);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t sdhci_irq(int irq, void *dev_id)
+{
+	struct sdhci_host *host = dev_id;
+	u32 intmask = sdhci_readl(host, SDHCI_INT_STATUS);
+
+	if (!intmask || intmask == 0xffffffff)
+		return IRQ_NONE;
+
+	/* Disable interrupts */
+	sdhci_writel(host, 0, SDHCI_SIGNAL_ENABLE);
+
+	DBG("*** %s got interrupt: 0x%08x\n",
+		mmc_hostname(host->mmc), intmask);
+
+	return IRQ_WAKE_THREAD;
 }
 
 /*****************************************************************************\
@@ -2578,8 +2587,9 @@ int sdhci_resume_host(struct sdhci_host *host)
 	}
 
 	if (!device_may_wakeup(mmc_dev(host->mmc))) {
-		ret = request_irq(host->irq, sdhci_irq, IRQF_SHARED,
-				  mmc_hostname(host->mmc), host);
+		ret = request_threaded_irq(host->irq, sdhci_irq,
+			sdhci_irq_thread, IRQF_SHARED,
+			mmc_hostname(host->mmc), host);
 		if (ret)
 			return ret;
 	} else {
@@ -3174,8 +3184,8 @@ int sdhci_add_host(struct sdhci_host *host)
 					sdhci_tuning_timeout_work);
 	}
 
-	ret = request_irq(host->irq, sdhci_irq, IRQF_SHARED,
-		mmc_hostname(mmc), host);
+	ret = request_threaded_irq(host->irq, sdhci_irq, sdhci_irq_thread,
+			  IRQF_SHARED, mmc_hostname(host->mmc), host);
 	if (ret) {
 		pr_err("%s: Failed to request IRQ %d: %d\n",
 		       mmc_hostname(mmc), host->irq, ret);
-- 
1.7.10.4

[PATCH 5/8] sdhci: Delay led blinking

[Jeremie Samuel]

The function sdhci_led_control is called in an atomic context by the
led_trigger driver. So, it is necessary to delay the activation or
deactivation of the led.

Signed-off-by: Jeremie Samuel <jeremie.samuel.ext@parrot.com>
---
 drivers/mmc/host/sdhci.c  |   42 ++++++++++++++++++++++--------------------
 include/linux/mmc/sdhci.h |    1 +
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index f361fec..c2585dd 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -269,8 +269,14 @@ static void sdhci_activate_led(struct sdhci_host *host)
 	u8 ctrl;
 
 	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
-	ctrl |= SDHCI_CTRL_LED;
-	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+#ifdef SDHCI_USE_LEDS_CLASS
+	if (host->brightness && !(ctrl & SDHCI_CTRL_LED)) {
+#else
+	if (!(ctrl & SDHCI_CTRL_LED)) {
+#endif
+		ctrl |= SDHCI_CTRL_LED;
+		sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+	}
 }
 
 static void sdhci_deactivate_led(struct sdhci_host *host)
@@ -278,8 +284,14 @@ static void sdhci_deactivate_led(struct sdhci_host *host)
 	u8 ctrl;
 
 	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
-	ctrl &= ~SDHCI_CTRL_LED;
-	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+#ifdef SDHCI_USE_LEDS_CLASS
+	if (!host->brightness && (ctrl & SDHCI_CTRL_LED)) {
+#else
+	if (ctrl & SDHCI_CTRL_LED) {
+#endif
+		ctrl &= ~SDHCI_CTRL_LED;
+		sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+	}
 }
 
 #ifdef SDHCI_USE_LEDS_CLASS
@@ -287,19 +299,11 @@ static void sdhci_led_control(struct led_classdev *led,
 	enum led_brightness brightness)
 {
 	struct sdhci_host *host = container_of(led, struct sdhci_host, led);
-	unsigned long flags;
-
-	spin_lock_irqsave(&host->lock, flags);
 
 	if (host->runtime_suspended)
-		goto out;
+		return;
 
-	if (brightness == LED_OFF)
-		sdhci_deactivate_led(host);
-	else
-		sdhci_activate_led(host);
-out:
-	spin_unlock_irqrestore(&host->lock, flags);
+	host->brightness = brightness;
 }
 #endif
 
@@ -1321,9 +1325,7 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	WARN_ON(host->mrq != NULL);
 
-#ifndef SDHCI_USE_LEDS_CLASS
 	sdhci_activate_led(host);
-#endif
 
 	/*
 	 * Ensure we don't send the STOP for non-SET_BLOCK_COUNTED
@@ -2145,15 +2147,15 @@ static void sdhci_finish_work(struct work_struct *wk)
 	host->cmd = NULL;
 	host->data = NULL;
 
-#ifndef SDHCI_USE_LEDS_CLASS
-	sdhci_deactivate_led(host);
-#endif
-
 	mmiowb();
 	spin_unlock_irqrestore(&host->lock, flags);
 
 	mmc_request_done(host->mmc, mrq);
 	sdhci_runtime_pm_put(host);
+
+	spin_lock_irqsave(&host->lock, flags);
+	sdhci_deactivate_led(host);
+	spin_unlock_irqrestore(&host->lock, flags);
 }
 
 static void sdhci_timeout_work(struct work_struct *wk)
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 152acb5..8002d17 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -112,6 +112,7 @@ struct sdhci_host {
 #if defined(CONFIG_LEDS_CLASS) || defined(CONFIG_LEDS_CLASS_MODULE)
 	struct led_classdev led;	/* LED control */
 	char led_name[32];
+	enum led_brightness brightness;
 #endif
 
 	spinlock_t lock;	/* Mutex */
-- 
1.7.10.4

[PATCH 6/8] sdhci: Turn host->lock into a mutex

[Jeremie Samuel]

Finally, we can get rid of the host->lock spinlock, and turn it
into a mutex.

This patch does just this.

Patch based on: http://thread.gmane.org/gmane.linux.kernel.mmc/2579.

Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
Signed-off-by: Jeremie Samuel <jeremie.samuel.ext@parrot.com>
---
 drivers/mmc/host/sdhci.c  |   99 ++++++++++++++++++++-------------------------
 include/linux/mmc/sdhci.h |    3 +-
 2 files changed, 46 insertions(+), 56 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c2585dd..10a7684 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -20,6 +20,7 @@
 #include <linux/module.h>
 #include <linux/dma-mapping.h>
 #include <linux/slab.h>
+#include <linux/mutex.h>
 #include <linux/scatterlist.h>
 #include <linux/regulator/consumer.h>
 #include <linux/pm_runtime.h>
@@ -1314,14 +1315,13 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
 	struct sdhci_host *host;
 	int present;
-	unsigned long flags;
 	u32 tuning_opcode;
 
 	host = mmc_priv(mmc);
 
 	sdhci_runtime_pm_get(host);
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->lock);
 
 	WARN_ON(host->mrq != NULL);
 
@@ -1377,9 +1377,9 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 					mmc->card->type == MMC_TYPE_MMC ?
 					MMC_SEND_TUNING_BLOCK_HS200 :
 					MMC_SEND_TUNING_BLOCK;
-				spin_unlock_irqrestore(&host->lock, flags);
+				mutex_unlock(&host->lock);
 				sdhci_execute_tuning(mmc, tuning_opcode);
-				spin_lock_irqsave(&host->lock, flags);
+				mutex_lock(&host->lock);
 
 				/* Restore original mmc_request structure */
 				host->mrq = mrq;
@@ -1393,19 +1393,18 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	}
 
 	mmiowb();
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->lock);
 }
 
 static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
 {
-	unsigned long flags;
 	int vdd_bit = -1;
 	u8 ctrl;
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->lock);
 
 	if (host->flags & SDHCI_DEVICE_DEAD) {
-		spin_unlock_irqrestore(&host->lock, flags);
+		mutex_unlock(&host->lock);
 		if (host->vmmc && ios->power_mode == MMC_POWER_OFF)
 			mmc_regulator_set_ocr(host->mmc, host->vmmc, 0);
 		return;
@@ -1432,9 +1431,9 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
 		vdd_bit = sdhci_set_power(host, ios->vdd);
 
 	if (host->vmmc && vdd_bit != -1) {
-		spin_unlock_irqrestore(&host->lock, flags);
+		mutex_unlock(&host->lock);
 		mmc_regulator_set_ocr(host->mmc, host->vmmc, vdd_bit);
-		spin_lock_irqsave(&host->lock, flags);
+		mutex_lock(&host->lock);
 	}
 
 	if (host->ops->platform_send_init_74_clocks)
@@ -1572,7 +1571,7 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
 		sdhci_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
 
 	mmiowb();
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->lock);
 }
 
 static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
@@ -1617,10 +1616,9 @@ static int sdhci_get_cd(struct mmc_host *mmc)
 
 static int sdhci_check_ro(struct sdhci_host *host)
 {
-	unsigned long flags;
 	int is_readonly;
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->lock);
 
 	if (host->flags & SDHCI_DEVICE_DEAD)
 		is_readonly = 0;
@@ -1630,7 +1628,7 @@ static int sdhci_check_ro(struct sdhci_host *host)
 		is_readonly = !(sdhci_readl(host, SDHCI_PRESENT_STATE)
 				& SDHCI_WRITE_PROTECT);
 
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->lock);
 
 	/* This quirk needs to be replaced by a callback-function later */
 	return host->quirks & SDHCI_QUIRK_INVERTED_WRITE_PROTECT ?
@@ -1701,11 +1699,10 @@ out:
 static void sdhci_enable_sdio_irq(struct mmc_host *mmc, int enable)
 {
 	struct sdhci_host *host = mmc_priv(mmc);
-	unsigned long flags;
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->lock);
 	sdhci_enable_sdio_irq_nolock(host, enable);
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->lock);
 }
 
 static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
@@ -1836,7 +1833,7 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 
 	sdhci_runtime_pm_get(host);
 	disable_irq(host->irq);
-	spin_lock(&host->lock);
+	mutex_lock(&host->lock);
 
 	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
 
@@ -1856,7 +1853,7 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	    requires_tuning_nonuhs)
 		ctrl |= SDHCI_CTRL_EXEC_TUNING;
 	else {
-		spin_unlock(&host->lock);
+		mutex_unlock(&host->lock);
 		enable_irq(host->irq);
 		sdhci_runtime_pm_put(host);
 		return 0;
@@ -1929,7 +1926,7 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 		host->cmd = NULL;
 		host->mrq = NULL;
 
-		spin_unlock(&host->lock);
+		mutex_unlock(&host->lock);
 		enable_irq(host->irq);
 
 		/* Wait for Buffer Read Ready interrupt */
@@ -1937,7 +1934,7 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 					(host->tuning_done == 1),
 					msecs_to_jiffies(50));
 		disable_irq(host->irq);
-		spin_lock(&host->lock);
+		mutex_lock(&host->lock);
 
 		if (!host->tuning_done) {
 			pr_info(DRIVER_NAME ": Timeout waiting for "
@@ -2012,7 +2009,7 @@ out:
 		err = 0;
 
 	sdhci_clear_set_irqs(host, SDHCI_INT_DATA_AVAIL, ier);
-	spin_unlock(&host->lock);
+	mutex_unlock(&host->lock);
 	enable_irq(host->irq);
 	sdhci_runtime_pm_put(host);
 
@@ -2048,9 +2045,8 @@ static void sdhci_enable_preset_value(struct sdhci_host *host, bool enable)
 static void sdhci_card_event(struct mmc_host *mmc)
 {
 	struct sdhci_host *host = mmc_priv(mmc);
-	unsigned long flags;
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->lock);
 
 	/* Check host->mrq first in case we are runtime suspended */
 	if (host->mrq &&
@@ -2067,7 +2063,7 @@ static void sdhci_card_event(struct mmc_host *mmc)
 		schedule_work(&host->finish_work);
 	}
 
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->lock);
 }
 
 static const struct mmc_host_ops sdhci_ops = {
@@ -2102,19 +2098,18 @@ static void sdhci_card_detect_work(struct work_struct *wk)
 static void sdhci_finish_work(struct work_struct *wk)
 {
 	struct sdhci_host *host;
-	unsigned long flags;
 	struct mmc_request *mrq;
 
 	host = container_of(wk, struct sdhci_host, finish_work);
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->lock);
 
 	/*
 	 * If this work gets rescheduled while running, it will
 	 * be run again afterwards but without any active request.
 	 */
 	if (!host->mrq) {
-		spin_unlock_irqrestore(&host->lock, flags);
+		mutex_unlock(&host->lock);
 		return;
 	}
 
@@ -2148,24 +2143,23 @@ static void sdhci_finish_work(struct work_struct *wk)
 	host->data = NULL;
 
 	mmiowb();
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->lock);
 
 	mmc_request_done(host->mmc, mrq);
 	sdhci_runtime_pm_put(host);
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->lock);
 	sdhci_deactivate_led(host);
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->lock);
 }
 
 static void sdhci_timeout_work(struct work_struct *wk)
 {
 	struct sdhci_host *host;
-	unsigned long flags;
 
 	host = container_of(wk, struct sdhci_host, timeout_work.work);
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->lock);
 
 	if (host->mrq) {
 		pr_err("%s: Timeout waiting for hardware "
@@ -2186,21 +2180,20 @@ static void sdhci_timeout_work(struct work_struct *wk)
 	}
 
 	mmiowb();
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->lock);
 }
 
 static void sdhci_tuning_timeout_work(struct work_struct *wk)
 {
 	struct sdhci_host *host;
-	unsigned long flags;
 
 	host = container_of(wk, struct sdhci_host, tuning_timeout_work.work);
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->lock);
 
 	host->flags |= SDHCI_NEEDS_RETUNING;
 
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->lock);
 }
 
 /*****************************************************************************\
@@ -2394,10 +2387,10 @@ static irqreturn_t sdhci_irq_thread(int irq, void *dev_id)
 	u32 intmask, unexpected = 0;
 	int cardint = 0, max_loops = 16;
 
-	spin_lock(&host->lock);
+	mutex_lock(&host->lock);
 
 	if (host->runtime_suspended) {
-		spin_unlock(&host->lock);
+		mutex_unlock(&host->lock);
 		pr_warning("%s: got irq while runtime suspended\n",
 		       mmc_hostname(host->mmc));
 		return IRQ_HANDLED;
@@ -2469,7 +2462,7 @@ again:
 	if (intmask && --max_loops)
 		goto again;
 
-	spin_unlock(&host->lock);
+	mutex_unlock(&host->lock);
 
 	if (unexpected) {
 		pr_err("%s: Unexpected interrupt 0x%08x.\n",
@@ -2642,7 +2635,6 @@ static int sdhci_runtime_pm_put(struct sdhci_host *host)
 
 int sdhci_runtime_suspend_host(struct sdhci_host *host)
 {
-	unsigned long flags;
 	int ret = 0;
 
 	/* Disable tuning since we are suspending */
@@ -2651,15 +2643,15 @@ int sdhci_runtime_suspend_host(struct sdhci_host *host)
 		host->flags &= ~SDHCI_NEEDS_RETUNING;
 	}
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->lock);
 	sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->lock);
 
 	synchronize_irq(host->irq);
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->lock);
 	host->runtime_suspended = true;
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->lock);
 
 	return ret;
 }
@@ -2667,7 +2659,6 @@ EXPORT_SYMBOL_GPL(sdhci_runtime_suspend_host);
 
 int sdhci_runtime_resume_host(struct sdhci_host *host)
 {
-	unsigned long flags;
 	int ret = 0, host_flags = host->flags;
 
 	if (host_flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA)) {
@@ -2685,16 +2676,16 @@ int sdhci_runtime_resume_host(struct sdhci_host *host)
 	sdhci_do_start_signal_voltage_switch(host, &host->mmc->ios);
 	if ((host_flags & SDHCI_PV_ENABLED) &&
 		!(host->quirks2 & SDHCI_QUIRK2_PRESET_VALUE_BROKEN)) {
-		spin_lock_irqsave(&host->lock, flags);
+		mutex_lock(&host->lock);
 		sdhci_enable_preset_value(host, true);
-		spin_unlock_irqrestore(&host->lock, flags);
+		mutex_unlock(&host->lock);
 	}
 
 	/* Set the re-tuning expiration flag */
 	if (host->flags & SDHCI_USING_RETUNING_TIMER)
 		host->flags |= SDHCI_NEEDS_RETUNING;
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->lock);
 
 	host->runtime_suspended = false;
 
@@ -2705,7 +2696,7 @@ int sdhci_runtime_resume_host(struct sdhci_host *host)
 	/* Enable Card Detection */
 	sdhci_enable_card_detection(host);
 
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->lock);
 
 	return ret;
 }
@@ -3114,7 +3105,7 @@ int sdhci_add_host(struct sdhci_host *host)
 		return -ENODEV;
 	}
 
-	spin_lock_init(&host->lock);
+	mutex_init(&host->lock);
 
 	/*
 	 * Maximum number of segments. Depends on if the hardware
@@ -3242,10 +3233,8 @@ EXPORT_SYMBOL_GPL(sdhci_add_host);
 
 void sdhci_remove_host(struct sdhci_host *host, int dead)
 {
-	unsigned long flags;
-
 	if (dead) {
-		spin_lock_irqsave(&host->lock, flags);
+		mutex_lock(&host->lock);
 
 		host->flags |= SDHCI_DEVICE_DEAD;
 
@@ -3257,7 +3246,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
 			schedule_work(&host->finish_work);
 		}
 
-		spin_unlock_irqrestore(&host->lock, flags);
+		mutex_unlock(&host->lock);
 	}
 
 	sdhci_disable_card_detection(host);
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 8002d17..e2624066 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -15,6 +15,7 @@
 #include <linux/workqueue.h>
 #include <linux/compiler.h>
 #include <linux/types.h>
+#include <linux/mutex.h>
 #include <linux/io.h>
 #include <linux/mmc/host.h>
 
@@ -115,7 +116,7 @@ struct sdhci_host {
 	enum led_brightness brightness;
 #endif
 
-	spinlock_t lock;	/* Mutex */
+	struct mutex lock;	/* Mutex */
 
 	int flags;		/* Host attributes */
 #define SDHCI_USE_SDMA		(1<<0)	/* Host is SDMA capable */
-- 
1.7.10.4

[PATCH 7/8] sdhci: Get rid of mdelay()s where it is safe and makes sense

[Jeremie Samuel]

Since we don't run in the atomic context any longer, we can
turn mdelay()s into msleep()s.

The only place where the driver is still using mdelay() is
sdhci_send_command(). There it is possible to use sleepable
delays too, but we don't actually want to force rescheduling
in a hot path.

Sure, we might end up calling msleep() there too, just not
via this safe patch.

PAtch based on: http://thread.gmane.org/gmane.linux.kernel.mmc/2579.

Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
Signed-off-by: Jeremie Samuel <jeremie.samuel.ext@parrot.com>
---
 drivers/mmc/host/sdhci.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 10a7684..4a6b716 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -209,7 +209,7 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
 			return;
 		}
 		timeout--;
-		mdelay(1);
+		msleep(1);
 	}
 
 	if (host->ops->platform_reset_exit)
@@ -1226,7 +1226,7 @@ clock_set:
 			return;
 		}
 		timeout--;
-		mdelay(1);
+		msleep(1);
 	}
 
 	clk |= SDHCI_CLOCK_CARD_EN;
@@ -1300,7 +1300,7 @@ static int sdhci_set_power(struct sdhci_host *host, unsigned short power)
 	 * can apply clock after applying power
 	 */
 	if (host->quirks & SDHCI_QUIRK_DELAY_AFTER_POWER)
-		mdelay(10);
+		msleep(10);
 
 	return power;
 }
@@ -1955,7 +1955,7 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
 		tuning_loop_counter--;
 		timeout--;
-		mdelay(1);
+		msleep(1);
 	} while (ctrl & SDHCI_CTRL_EXEC_TUNING);
 
 	/*
-- 
1.7.10.4

[PATCH 8/8] sdhci: Use jiffies instead of a timeout counter

[Jeremie Samuel]

Just a cosmetic change, should not affect functionality.

Patch based on: http://thread.gmane.org/gmane.linux.kernel.mmc/2579.

Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
Signed-off-by: Jeremie Samuel <jeremie.samuel.ext@parrot.com>
---
 drivers/mmc/host/sdhci.c |   22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 4a6b716..f121822 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -198,17 +198,16 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
 		host->clock = 0;
 
 	/* Wait max 100 ms */
-	timeout = 100;
+	timeout = jiffies + msecs_to_jiffies(100);
 
 	/* hw clears the bit when it's done */
 	while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
-		if (timeout == 0) {
+		if (time_after(jiffies, timeout)) {
 			pr_err("%s: Reset 0x%x never completed.\n",
 				mmc_hostname(host->mmc), (int)mask);
 			sdhci_dumpregs(host);
 			return;
 		}
-		timeout--;
 		msleep(1);
 	}
 
@@ -984,7 +983,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 	WARN_ON(host->cmd);
 
 	/* Wait max 10 ms */
-	timeout = 10;
+	timeout = jiffies + msecs_to_jiffies(10);
 
 	mask = SDHCI_CMD_INHIBIT;
 	if ((cmd->data != NULL) || (cmd->flags & MMC_RSP_BUSY))
@@ -996,7 +995,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 		mask &= ~SDHCI_DATA_INHIBIT;
 
 	while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {
-		if (timeout == 0) {
+		if (time_after(jiffies, timeout)) {
 			pr_err("%s: Controller never released "
 				"inhibit bit(s).\n", mmc_hostname(host->mmc));
 			sdhci_dumpregs(host);
@@ -1004,7 +1003,6 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 			schedule_work(&host->finish_work);
 			return;
 		}
-		timeout--;
 		mdelay(1);
 	}
 
@@ -1216,16 +1214,15 @@ clock_set:
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
 
 	/* Wait max 20 ms */
-	timeout = 20;
+	timeout = jiffies + msecs_to_jiffies(20);
 	while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
 		& SDHCI_CLOCK_INT_STABLE)) {
-		if (timeout == 0) {
+		if (time_after(jiffies, timeout)) {
 			pr_err("%s: Internal clock never "
 				"stabilised.\n", mmc_hostname(host->mmc));
 			sdhci_dumpregs(host);
 			return;
 		}
-		timeout--;
 		msleep(1);
 	}
 
@@ -1878,12 +1875,12 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	 * Issue CMD19 repeatedly till Execute Tuning is set to 0 or the number
 	 * of loops reaches 40 times or a timeout of 150ms occurs.
 	 */
-	timeout = 150;
+	timeout = jiffies + msecs_to_jiffies(150);
 	do {
 		struct mmc_command cmd = {0};
 		struct mmc_request mrq = {NULL};
 
-		if (!tuning_loop_counter && !timeout)
+		if (!tuning_loop_counter && time_after(jiffies, timeout))
 			break;
 
 		cmd.opcode = opcode;
@@ -1954,7 +1951,6 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 
 		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
 		tuning_loop_counter--;
-		timeout--;
 		msleep(1);
 	} while (ctrl & SDHCI_CTRL_EXEC_TUNING);
 
@@ -1962,7 +1958,7 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	 * The Host Driver has exhausted the maximum number of loops allowed,
 	 * so use fixed sampling frequency.
 	 */
-	if (!tuning_loop_counter || !timeout) {
+	if (!tuning_loop_counter || time_after(jiffies, timeout)) {
 		ctrl &= ~SDHCI_CTRL_TUNED_CLK;
 		sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
 	} else {
-- 
1.7.10.4

Re: [PATCH 0/8] sdhci: Move real work out of an atomic context

[Jeremie Samuel]

Hi,

I posted these patches a few weeks ago. Is it possible to get a feedback for this submission?

Thank you for your help.

-- 
Jeremie Samuel              Parrot S.A.
Software Engineer           14, quai de Jemmapes
R&D/OS Platform             75010 Paris, France
http://www.parrot.com

On 24/05/2013 18:00, Jérémie Samuel wrote:

> Hi all,
>
> Currently the sdhci driver does everything in the atomic context.
> And what is worse, PIO transfers are made from the IRQ handler.
>
> Some patches were already submitted to solve this issue. But there were
> rejected because they involved new issues.
>
> This set of patches is an evolution of an old patch from Anton Vorontsov.
> I tried to fix all the problems involved by the patches. I tested it for
> several time now with SD cards and SDIO.
>
> So, this patch set reworks sdhci code to avoid atomic context,
> almost completely.
>
> Thanks,
>
> Jeremie Samuel
>
> Jeremie Samuel (8):
>    sdhci: Turn timeout timer into delayed work
>    sdhci: Turn tuning timeout timer into delayed work
>    sdhci: Use work structs instead of tasklets
>    sdhci: Use threaded IRQ handler
>    sdhci: Delay led blinking
>    sdhci: Turn host->lock into a mutex
>    sdhci: Get rid of mdelay()s where it is safe and makes sense
>    sdhci: Use jiffies instead of a timeout counter
>
>   drivers/mmc/host/sdhci.c  |  327 ++++++++++++++++++++++-----------------------
>   include/linux/mmc/sdhci.h |   13 +-
>   2 files changed, 168 insertions(+), 172 deletions(-)
>

Re: [PATCH 0/8] sdhci: Move real work out of an atomic context

[Chris Ball]

Hi Jeremie,

On Thu, Jun 13 2013, Jeremie Samuel wrote:
> I posted these patches a few weeks ago. Is it possible to get a
> feedback for this submission?

Please could you resubmit in two weeks or so, so that we can spend a
full cycle with these in linux-next before including them in Linux
3.12 if there are no problems?  Thanks!

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

[PATCH 0/8] sdhci: Move real work out of an atomic context

[Jeremie Samuel]

Hi all,

Currently the sdhci driver does everything in the atomic context.
And what is worse, PIO transfers are made from the IRQ handler.

Some patches were already submitted to solve this issue. But there were
rejected because they involved new issues.

This set of patches is an evolution of an old patch from Anton Vorontsov.
I tried to fix all the problems involved by the patches. I tested it for
several time now with SD cards and SDIO.

So, this patch set reworks sdhci code to avoid atomic context,
almost completely.

Thanks,

Jeremie Samuel

Jeremie Samuel (8):
  sdhci: Turn timeout timer into delayed work
  sdhci: Turn tuning timeout timer into delayed work
  sdhci: Use work structs instead of tasklets
  sdhci: Use threaded IRQ handler
  sdhci: Delay led blinking
  sdhci: Turn host->lock into a mutex
  sdhci: Get rid of mdelay()s where it is safe and makes sense
  sdhci: Use jiffies instead of a timeout counter

 drivers/mmc/host/sdhci.c  |  327 ++++++++++++++++++++++-----------------------
 include/linux/mmc/sdhci.h |   13 +-
 2 files changed, 168 insertions(+), 172 deletions(-)

-- 
1.7.10.4

Re: [PATCH 0/8] sdhci: Move real work out of an atomic context

[Philip Rakity]

On Jul 9, 2013, at 4:44 PM, Jeremie Samuel <jeremie.samuel.ext@parrot.com> wrote:

> Hi all,
> 
> Currently the sdhci driver does everything in the atomic context.
> And what is worse, PIO transfers are made from the IRQ handler.
> 
> Some patches were already submitted to solve this issue. But there were
> rejected because they involved new issues.
> 
> This set of patches is an evolution of an old patch from Anton Vorontsov.
> I tried to fix all the problems involved by the patches. I tested it for
> several time now with SD cards and SDIO.
> 
> So, this patch set reworks sdhci code to avoid atomic context,
> almost completely.
> 
> Thanks,
> 

Running DDR50, SDR104 or HS200 what is the performance impact ?


> Jeremie Samuel
> 
> Jeremie Samuel (8):
>  sdhci: Turn timeout timer into delayed work
>  sdhci: Turn tuning timeout timer into delayed work
>  sdhci: Use work structs instead of tasklets
>  sdhci: Use threaded IRQ handler
>  sdhci: Delay led blinking
>  sdhci: Turn host->lock into a mutex
>  sdhci: Get rid of mdelay()s where it is safe and makes sense
>  sdhci: Use jiffies instead of a timeout counter
> 
> drivers/mmc/host/sdhci.c  |  327 ++++++++++++++++++++++-----------------------
> include/linux/mmc/sdhci.h |   13 +-
> 2 files changed, 168 insertions(+), 172 deletions(-)
> 
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] sdhci: Move real work out of an atomic context

[Jeremie Samuel]

On 09/07/2013 17:52, Philip Rakity wrote:

> On Jul 9, 2013, at 4:44 PM, Jeremie Samuel<jeremie.samuel.ext@parrot.com>  wrote:
>
>> Hi all,
>>
>> Currently the sdhci driver does everything in the atomic context.
>> And what is worse, PIO transfers are made from the IRQ handler.
>>
>> Some patches were already submitted to solve this issue. But there were
>> rejected because they involved new issues.
>>
>> This set of patches is an evolution of an old patch from Anton Vorontsov.
>> I tried to fix all the problems involved by the patches. I tested it for
>> several time now with SD cards and SDIO.
>>
>> So, this patch set reworks sdhci code to avoid atomic context,
>> almost completely.
>>
>> Thanks,
>>
> Running DDR50, SDR104 or HS200 what is the performance impact ?
I've realised performance tests with iozone benchmark and the performance impact was approximatively 0.5%.

>
>
>> Jeremie Samuel
>>
>> Jeremie Samuel (8):
>>   sdhci: Turn timeout timer into delayed work
>>   sdhci: Turn tuning timeout timer into delayed work
>>   sdhci: Use work structs instead of tasklets
>>   sdhci: Use threaded IRQ handler
>>   sdhci: Delay led blinking
>>   sdhci: Turn host->lock into a mutex
>>   sdhci: Get rid of mdelay()s where it is safe and makes sense
>>   sdhci: Use jiffies instead of a timeout counter
>>
>> drivers/mmc/host/sdhci.c  |  327 ++++++++++++++++++++++-----------------------
>> include/linux/mmc/sdhci.h |   13 +-
>> 2 files changed, 168 insertions(+), 172 deletions(-)
>>
>> -- 
>> 1.7.10.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/8] sdhci: Move real work out of an atomic context

[Jeremie Samuel]

Hi all,

Currently the sdhci driver does everything in the atomic context.
And what is worse, PIO transfers are made from the IRQ handler.

Some patches were already submitted to solve this issue. But there were
rejected because they involved new issues.

This set of patches is an evolution of an old patch from Anton Vorontsov.
I tried to fix all the problems involved by the patches. I tested it for
several time now with SD cards and SDIO.

So, this patch set reworks sdhci code to avoid atomic context,
almost completely.

Thanks,

Jeremie Samuel

Jeremie Samuel (8):
  sdhci: Turn timeout timer into delayed work
  sdhci: Turn tuning timeout timer into delayed work
  sdhci: Use work structs instead of tasklets
  sdhci: Use threaded IRQ handler
  sdhci: Delay led blinking
  sdhci: Turn host->lock into a mutex
  sdhci: Get rid of mdelay()s where it is safe and makes sense
  sdhci: Use jiffies instead of a timeout counter

 drivers/mmc/host/sdhci-esdhc-imx.c |    4 +-
 drivers/mmc/host/sdhci-pxav3.c     |   10 +-
 drivers/mmc/host/sdhci-s3c.c       |    5 +-
 drivers/mmc/host/sdhci.c           |  329 ++++++++++++++++++------------------
 include/linux/mmc/sdhci.h          |   13 +-
 5 files changed, 177 insertions(+), 184 deletions(-)

-- 
1.7.10.4