All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] sdhci: Move real work out of an atomic context
@ 2013-10-16 16:20 Jeremie Samuel
  2013-10-16 16:20 ` [PATCH 1/8] sdhci: Turn timeout timer into delayed work Jeremie Samuel
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Jeremie Samuel @ 2013-10-16 16:20 UTC (permalink / raw)
  To: Chris Ball; +Cc: linux-mmc, Grégor Boirie, Matthieu Castet, 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/8] sdhci: Turn timeout timer into delayed work
  2013-10-16 16:20 [PATCH 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
@ 2013-10-16 16:20 ` Jeremie Samuel
  2013-10-16 16:20 ` [PATCH 2/8] sdhci: Turn tuning " Jeremie Samuel
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Jeremie Samuel @ 2013-10-16 16:20 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, Grégor Boirie, Matthieu Castet, Jeremie Samuel,
	Anton Vorontsov

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 66bce7a..b312ec6 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>
@@ -1013,7 +1014,7 @@ 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;
 
@@ -2140,7 +2141,7 @@ static void sdhci_tasklet_finish(unsigned long param)
 		return;
 	}
 
-	del_timer(&host->timer);
+	cancel_delayed_work(&host->timeout_work);
 
 	mrq = host->mrq;
 
@@ -2180,12 +2181,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);
 
@@ -3213,7 +3214,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);
@@ -3316,7 +3317,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 3e781b8..98cf934 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>
@@ -163,7 +164,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/8] sdhci: Turn tuning timeout timer into delayed work
  2013-10-16 16:20 [PATCH 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
  2013-10-16 16:20 ` [PATCH 1/8] sdhci: Turn timeout timer into delayed work Jeremie Samuel
@ 2013-10-16 16:20 ` Jeremie Samuel
  2013-10-16 16:20 ` [PATCH 3/8] sdhci: Use work structs instead of tasklets Jeremie Samuel
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Jeremie Samuel @ 2013-10-16 16:20 UTC (permalink / raw)
  To: Chris Ball; +Cc: linux-mmc, Grégor Boirie, Matthieu Castet, 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 b312ec6..03c9995 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -52,7 +52,7 @@ static void sdhci_finish_data(struct sdhci_host *);
 
 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
@@ -267,7 +267,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;
@@ -1380,7 +1380,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.
 		 */
@@ -2002,32 +2002,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;
@@ -2212,12 +2213,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);
 
@@ -2564,7 +2565,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;
 	}
 
@@ -2572,7 +2573,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);
 		}
 
@@ -2676,7 +2677,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;
 	}
 
@@ -3034,13 +3035,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);
@@ -3219,10 +3220,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);
 	}
 
 	sdhci_init(host, 0);
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 98cf934..2b0f4f3 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -180,7 +180,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/8] sdhci: Use work structs instead of tasklets
  2013-10-16 16:20 [PATCH 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
  2013-10-16 16:20 ` [PATCH 1/8] sdhci: Turn timeout timer into delayed work Jeremie Samuel
  2013-10-16 16:20 ` [PATCH 2/8] sdhci: Turn tuning " Jeremie Samuel
@ 2013-10-16 16:20 ` Jeremie Samuel
  2013-10-21  1:49   ` Chris Ball
  2013-10-16 16:20 ` [PATCH 4/8] sdhci: Use threaded IRQ handler Jeremie Samuel
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Jeremie Samuel @ 2013-10-16 16:20 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, Grégor Boirie, Matthieu Castet, Jeremie Samuel,
	Anton Vorontsov

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 03c9995..362a838 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -978,7 +978,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);
 }
 
 void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
@@ -1007,7 +1007,7 @@ 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--;
@@ -1028,7 +1028,7 @@ 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;
 	}
 
@@ -1090,7 +1090,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;
 	}
@@ -1374,7 +1374,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;
 
@@ -2089,7 +2089,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);
@@ -2114,29 +2114,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;
@@ -2205,7 +2206,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);
 		}
 	}
 
@@ -2252,7 +2253,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;
 	}
 
@@ -2461,7 +2462,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) {
@@ -3208,12 +3209,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);
 
@@ -3232,7 +3231,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;
 	}
 
 #ifdef CONFIG_MMC_DEBUG
@@ -3274,10 +3273,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;
 }
 
@@ -3297,7 +3292,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);
@@ -3319,8 +3314,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 2b0f4f3..05cd76c 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -161,8 +161,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/8] sdhci: Use threaded IRQ handler
  2013-10-16 16:20 [PATCH 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
                   ` (2 preceding siblings ...)
  2013-10-16 16:20 ` [PATCH 3/8] sdhci: Use work structs instead of tasklets Jeremie Samuel
@ 2013-10-16 16:20 ` Jeremie Samuel
  2013-10-16 16:20 ` [PATCH 5/8] sdhci: Delay led blinking Jeremie Samuel
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Jeremie Samuel @ 2013-10-16 16:20 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, Grégor Boirie, Matthieu Castet, Jeremie Samuel,
	Anton Vorontsov

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 362a838..0bb892c 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2413,9 +2413,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;
@@ -2431,15 +2430,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;
@@ -2499,12 +2490,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) {
@@ -2518,7 +2507,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;
 }
 
 /*****************************************************************************\
@@ -2605,8 +2614,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 {
@@ -3226,8 +3236,8 @@ int sdhci_add_host(struct sdhci_host *host)
 
 	sdhci_init(host, 0);
 
-	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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/8] sdhci: Delay led blinking
  2013-10-16 16:20 [PATCH 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
                   ` (3 preceding siblings ...)
  2013-10-16 16:20 ` [PATCH 4/8] sdhci: Use threaded IRQ handler Jeremie Samuel
@ 2013-10-16 16:20 ` Jeremie Samuel
  2013-10-16 16:20 ` [PATCH 6/8] sdhci: Turn host->lock into a mutex Jeremie Samuel
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Jeremie Samuel @ 2013-10-16 16:20 UTC (permalink / raw)
  To: Chris Ball; +Cc: linux-mmc, Grégor Boirie, Matthieu Castet, 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 0bb892c..be91ca1 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -280,8 +280,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)
@@ -289,8 +295,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
@@ -298,19 +310,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
 
@@ -1338,9 +1342,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
@@ -2172,15 +2174,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 05cd76c..3bb6ad6 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -115,6 +115,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 6/8] sdhci: Turn host->lock into a mutex
  2013-10-16 16:20 [PATCH 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
                   ` (4 preceding siblings ...)
  2013-10-16 16:20 ` [PATCH 5/8] sdhci: Delay led blinking Jeremie Samuel
@ 2013-10-16 16:20 ` Jeremie Samuel
  2013-10-16 16:20 ` [PATCH 7/8] sdhci: Get rid of mdelay()s where it is safe and makes sense Jeremie Samuel
  2013-10-16 16:20 ` [PATCH 8/8] sdhci: Use jiffies instead of a timeout counter Jeremie Samuel
  7 siblings, 0 replies; 14+ messages in thread
From: Jeremie Samuel @ 2013-10-16 16:20 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, Grégor Boirie, Matthieu Castet, Jeremie Samuel,
	Anton Vorontsov

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-esdhc-imx.c |    4 +-
 drivers/mmc/host/sdhci-pxav3.c     |   10 ++--
 drivers/mmc/host/sdhci-s3c.c       |    5 +-
 drivers/mmc/host/sdhci.c           |  101 ++++++++++++++++--------------------
 include/linux/mmc/sdhci.h          |    3 +-
 5 files changed, 55 insertions(+), 68 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index b9899e9..1b3b444 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -641,12 +641,12 @@ static int esdhc_send_tuning_cmd(struct sdhci_host *host, u32 opcode)
 	init_completion(&(mrq.completion));
 
 	disable_irq(host->irq);
-	spin_lock(&host->lock);
+	mutex_lock(&host->lock);
 	host->mrq = &mrq;
 
 	sdhci_send_command(host, mrq.cmd);
 
-	spin_unlock(&host->lock);
+	mutex_unlock(&host->lock);
 	enable_irq(host->irq);
 
 	wait_for_completion(&mrq.completion);
diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
index 793dacd..fb761a9 100644
--- a/drivers/mmc/host/sdhci-pxav3.c
+++ b/drivers/mmc/host/sdhci-pxav3.c
@@ -378,12 +378,11 @@ static int sdhci_pxav3_runtime_suspend(struct device *dev)
 {
 	struct sdhci_host *host = dev_get_drvdata(dev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	unsigned long flags;
 
 	if (pltfm_host->clk) {
-		spin_lock_irqsave(&host->lock, flags);
+		mutex_lock(&host->lock);
 		host->runtime_suspended = true;
-		spin_unlock_irqrestore(&host->lock, flags);
+		mutex_unlock(&host->lock);
 
 		clk_disable_unprepare(pltfm_host->clk);
 	}
@@ -395,14 +394,13 @@ static int sdhci_pxav3_runtime_resume(struct device *dev)
 {
 	struct sdhci_host *host = dev_get_drvdata(dev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	unsigned long flags;
 
 	if (pltfm_host->clk) {
 		clk_prepare_enable(pltfm_host->clk);
 
-		spin_lock_irqsave(&host->lock, flags);
+		mutex_lock(&host->lock);
 		host->runtime_suspended = false;
-		spin_unlock_irqrestore(&host->lock, flags);
+		mutex_unlock(&host->lock);
 	}
 
 	return 0;
diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 6debda9..d114c29 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -376,10 +376,9 @@ static void sdhci_s3c_notify_change(struct platform_device *dev, int state)
 #ifdef CONFIG_PM_RUNTIME
 	struct sdhci_s3c *sc = sdhci_priv(host);
 #endif
-	unsigned long flags;
 
 	if (host) {
-		spin_lock_irqsave(&host->lock, flags);
+		mutex_lock(&host->lock);
 		if (state) {
 			dev_dbg(&dev->dev, "card inserted.\n");
 #ifdef CONFIG_PM_RUNTIME
@@ -396,7 +395,7 @@ static void sdhci_s3c_notify_change(struct platform_device *dev, int state)
 #endif
 		}
 		tasklet_schedule(&host->card_tasklet);
-		spin_unlock_irqrestore(&host->lock, flags);
+		mutex_unlock(&host->lock);
 	}
 }
 
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index be91ca1..ce7c99b 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>
@@ -1331,14 +1332,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);
 
@@ -1394,9 +1394,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;
@@ -1410,19 +1410,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;
@@ -1449,9 +1448,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)
@@ -1588,7 +1587,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)
@@ -1633,10 +1632,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;
@@ -1646,7 +1644,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 ?
@@ -1717,11 +1715,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,
@@ -1852,7 +1849,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);
 
@@ -1872,14 +1869,14 @@ 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;
 	}
 
 	if (host->ops->platform_execute_tuning) {
-		spin_unlock(&host->lock);
+		mutex_unlock(&host->lock);
 		enable_irq(host->irq);
 		err = host->ops->platform_execute_tuning(host, opcode);
 		sdhci_runtime_pm_put(host);
@@ -1953,7 +1950,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 */
@@ -1961,7 +1958,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 "
@@ -2036,7 +2033,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);
 
@@ -2072,13 +2069,12 @@ 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;
 
 	/* First check if client has provided their own card event */
 	if (host->ops->card_event)
 		host->ops->card_event(host);
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->lock);
 
 	/* Check host->mrq first in case we are runtime suspended */
 	if (host->mrq && !sdhci_do_get_cd(host)) {
@@ -2094,7 +2090,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 = {
@@ -2129,19 +2125,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;
 	}
 
@@ -2175,24 +2170,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 "
@@ -2213,21 +2207,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);
 }
 
 /*****************************************************************************\
@@ -2421,10 +2414,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;
@@ -2496,7 +2489,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",
@@ -2685,7 +2678,6 @@ static void sdhci_runtime_pm_bus_off(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 */
@@ -2694,15 +2686,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;
 }
@@ -2710,7 +2702,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)) {
@@ -2728,16 +2719,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;
 
@@ -2748,7 +2739,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;
 }
@@ -3164,7 +3155,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
@@ -3292,10 +3283,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;
 
@@ -3307,7 +3296,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 3bb6ad6..2f6a97a 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>
 
@@ -118,7 +119,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 7/8] sdhci: Get rid of mdelay()s where it is safe and makes sense
  2013-10-16 16:20 [PATCH 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
                   ` (5 preceding siblings ...)
  2013-10-16 16:20 ` [PATCH 6/8] sdhci: Turn host->lock into a mutex Jeremie Samuel
@ 2013-10-16 16:20 ` Jeremie Samuel
  2013-10-16 16:20 ` [PATCH 8/8] sdhci: Use jiffies instead of a timeout counter Jeremie Samuel
  7 siblings, 0 replies; 14+ messages in thread
From: Jeremie Samuel @ 2013-10-16 16:20 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, Grégor Boirie, Matthieu Castet, Jeremie Samuel,
	Anton Vorontsov

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 ce7c99b..843e99d 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -220,7 +220,7 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
 			return;
 		}
 		timeout--;
-		mdelay(1);
+		msleep(1);
 	}
 
 	if (host->ops->platform_reset_exit)
@@ -1238,7 +1238,7 @@ clock_set:
 			return;
 		}
 		timeout--;
-		mdelay(1);
+		msleep(1);
 	}
 
 	clk |= SDHCI_CLOCK_CARD_EN;
@@ -1317,7 +1317,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;
 }
@@ -1979,7 +1979,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 8/8] sdhci: Use jiffies instead of a timeout counter
  2013-10-16 16:20 [PATCH 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
                   ` (6 preceding siblings ...)
  2013-10-16 16:20 ` [PATCH 7/8] sdhci: Get rid of mdelay()s where it is safe and makes sense Jeremie Samuel
@ 2013-10-16 16:20 ` Jeremie Samuel
  7 siblings, 0 replies; 14+ messages in thread
From: Jeremie Samuel @ 2013-10-16 16:20 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, Grégor Boirie, Matthieu Castet, Jeremie Samuel,
	Anton Vorontsov

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 843e99d..3e148c0 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -209,17 +209,16 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
 	}
 
 	/* 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);
 	}
 
@@ -995,7 +994,7 @@ 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))
@@ -1007,7 +1006,7 @@ 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);
@@ -1015,7 +1014,6 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 			schedule_work(&host->finish_work);
 			return;
 		}
-		timeout--;
 		mdelay(1);
 	}
 
@@ -1228,16 +1226,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);
 	}
 
@@ -1902,12 +1899,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;
@@ -1978,7 +1975,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);
 
@@ -1986,7 +1982,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/8] sdhci: Use work structs instead of tasklets
  2013-10-16 16:20 ` [PATCH 3/8] sdhci: Use work structs instead of tasklets Jeremie Samuel
@ 2013-10-21  1:49   ` Chris Ball
  2013-10-21  8:36     ` Jeremie Samuel
  0 siblings, 1 reply; 14+ messages in thread
From: Chris Ball @ 2013-10-21  1:49 UTC (permalink / raw)
  To: Jeremie Samuel
  Cc: linux-mmc, Grégor Boirie, Matthieu Castet, Anton Vorontsov

Hi Jeremie,

On Wed, Oct 16 2013, Jeremie Samuel wrote:
> 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>
> [..]
> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
> index 2b0f4f3..05cd76c 100644
> --- a/include/linux/mmc/sdhci.h
> +++ b/include/linux/mmc/sdhci.h
> @@ -161,8 +161,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;

More compile errors:

 λ git grep card_tasklet drivers/mmc/host  
drivers/mmc/host/sdhci-bcm-kona.c: * to generate the CD IRQ handled in sdhci.c which schedules card_tasklet.
drivers/mmc/host/sdhci-dove.c:  tasklet_schedule(&host->card_tasklet);
drivers/mmc/host/sdhci-s3c.c:           tasklet_schedule(&host->card_tasklet);
drivers/mmc/host/sdhci-spear.c: tasklet_schedule(&host->card_tasklet);

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/8] sdhci: Use work structs instead of tasklets
  2013-10-21  1:49   ` Chris Ball
@ 2013-10-21  8:36     ` Jeremie Samuel
  2013-10-21 11:44       ` Chris Ball
  0 siblings, 1 reply; 14+ messages in thread
From: Jeremie Samuel @ 2013-10-21  8:36 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, Grégor Boirie, Matthieu Castet, Anton Vorontsov

Hi Chris,

Thanks for the feedback.

Do you have a way for me to compile the driver for all the architectures 
impacted by the patch? So I could send a patch which compile for all 
these architectures.

Cordially,

Jeremie

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

On 21/10/2013 03:49, Chris Ball wrote:
> Hi Jeremie,
>
> On Wed, Oct 16 2013, Jeremie Samuel wrote:
>> 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>
>> [..]
>> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
>> index 2b0f4f3..05cd76c 100644
>> --- a/include/linux/mmc/sdhci.h
>> +++ b/include/linux/mmc/sdhci.h
>> @@ -161,8 +161,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;
> More compile errors:
>
>   λ git grep card_tasklet drivers/mmc/host
> drivers/mmc/host/sdhci-bcm-kona.c: * to generate the CD IRQ handled in sdhci.c which schedules card_tasklet.
> drivers/mmc/host/sdhci-dove.c:  tasklet_schedule(&host->card_tasklet);
> drivers/mmc/host/sdhci-s3c.c:           tasklet_schedule(&host->card_tasklet);
> drivers/mmc/host/sdhci-spear.c: tasklet_schedule(&host->card_tasklet);
>
> Thanks,
>
> - Chris.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/8] sdhci: Use work structs instead of tasklets
  2013-10-21  8:36     ` Jeremie Samuel
@ 2013-10-21 11:44       ` Chris Ball
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Ball @ 2013-10-21 11:44 UTC (permalink / raw)
  To: Jeremie Samuel
  Cc: linux-mmc, Grégor Boirie, Matthieu Castet, Anton Vorontsov

Hi Jeremie,

On Mon, Oct 21 2013, Jeremie Samuel wrote:
> Do you have a way for me to compile the driver for all the
> architectures impacted by the patch? So I could send a patch which
> compile for all these architectures.

If you can get an ARM cross-compiler going, it would look something
like this:

$ mkdir obj.dove obj.imx obj.s3c obj.spear obj.tegra

$ cp arch/arm/configs/dove_defconfig obj.dove
$ cp arch/arm/configs/imx_v6_v7_defconfig obj.imx
$ cp arch/arm/configs/s3c6400_defconfig obj.s3c
$ cp arch/arm/configs/spear3xx_defconfig obj.spear
$ cp arch/arm/configs/tegra_defconfig obj.tegra

$ ARCH=arm CROSS_COMPILE=arm-linux-gnu- make -j12 O=obj.dove
$ ARCH=arm CROSS_COMPILE=arm-linux-gnu- make -j12 O=obj.imx
$ ARCH=arm CROSS_COMPILE=arm-linux-gnu- make -j12 O=obj.s3c
$ ARCH=arm CROSS_COMPILE=arm-linux-gnu- make -j12 O=obj.spear
$ ARCH=arm CROSS_COMPILE=arm-linux-gnu- make -j12 O=obj.tegra

There's also sdhci-sirf, but it has its own architecture.

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/8] sdhci: Use work structs instead of tasklets
  2013-07-09 15:44 [PATCH 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
@ 2013-07-09 15:44 ` Jeremie Samuel
  0 siblings, 0 replies; 14+ messages in thread
From: Jeremie Samuel @ 2013-07-09 15:44 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, matthieu.castet, gregor.boirie, Jeremie Samuel,
	Anton Vorontsov

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 06ad0bb..ab635d5 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -979,7 +979,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)
@@ -1008,7 +1008,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--;
@@ -1029,7 +1029,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;
 	}
 
@@ -1090,7 +1090,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;
 	}
@@ -1374,7 +1374,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;
 
@@ -2081,7 +2081,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);
@@ -2106,29 +2106,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;
@@ -2197,7 +2198,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);
 		}
 	}
 
@@ -2244,7 +2245,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;
 	}
 
@@ -2453,7 +2454,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) {
@@ -3197,12 +3198,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);
 
@@ -3219,7 +3218,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);
@@ -3263,10 +3262,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;
 }
 
@@ -3286,7 +3281,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);
@@ -3308,8 +3303,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 33efb73..0cc97ce 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -161,8 +161,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/8] sdhci: Use work structs instead of tasklets
  2013-05-24 16:00 [PATCH 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
@ 2013-05-24 16:00 ` Jeremie Samuel
  0 siblings, 0 replies; 14+ messages in thread
From: Jeremie Samuel @ 2013-05-24 16:00 UTC (permalink / raw)
  To: linux-mmc, Chris Ball
  Cc: matthieu.castet, gregor.boirie, Jeremie Samuel, Anton Vorontsov

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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2013-10-21 11:44 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-16 16:20 [PATCH 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
2013-10-16 16:20 ` [PATCH 1/8] sdhci: Turn timeout timer into delayed work Jeremie Samuel
2013-10-16 16:20 ` [PATCH 2/8] sdhci: Turn tuning " Jeremie Samuel
2013-10-16 16:20 ` [PATCH 3/8] sdhci: Use work structs instead of tasklets Jeremie Samuel
2013-10-21  1:49   ` Chris Ball
2013-10-21  8:36     ` Jeremie Samuel
2013-10-21 11:44       ` Chris Ball
2013-10-16 16:20 ` [PATCH 4/8] sdhci: Use threaded IRQ handler Jeremie Samuel
2013-10-16 16:20 ` [PATCH 5/8] sdhci: Delay led blinking Jeremie Samuel
2013-10-16 16:20 ` [PATCH 6/8] sdhci: Turn host->lock into a mutex Jeremie Samuel
2013-10-16 16:20 ` [PATCH 7/8] sdhci: Get rid of mdelay()s where it is safe and makes sense Jeremie Samuel
2013-10-16 16:20 ` [PATCH 8/8] sdhci: Use jiffies instead of a timeout counter Jeremie Samuel
  -- strict thread matches above, loose matches on Subject: below --
2013-07-09 15:44 [PATCH 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
2013-07-09 15:44 ` [PATCH 3/8] sdhci: Use work structs instead of tasklets Jeremie Samuel
2013-05-24 16:00 [PATCH 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
2013-05-24 16:00 ` [PATCH 3/8] sdhci: Use work structs instead of tasklets Jeremie Samuel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.