* [PATCH v2 0/8] sdhci: Move real work out of an atomic context
@ 2013-10-21 15:25 Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 1/8] sdhci: Turn timeout timer into delayed work Jeremie Samuel
` (9 more replies)
0 siblings, 10 replies; 13+ messages in thread
From: Jeremie Samuel @ 2013-10-21 15:25 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-bcm-kona.c | 2 +-
drivers/mmc/host/sdhci-dove.c | 6 +-
drivers/mmc/host/sdhci-esdhc-imx.c | 4 +-
drivers/mmc/host/sdhci-pxav3.c | 10 +-
drivers/mmc/host/sdhci-s3c.c | 7 +-
drivers/mmc/host/sdhci-sirf.c | 4 +-
drivers/mmc/host/sdhci-spear.c | 2 +-
drivers/mmc/host/sdhci.c | 329 ++++++++++++++++++------------------
include/linux/mmc/sdhci.h | 13 +-
9 files changed, 185 insertions(+), 192 deletions(-)
--
1.7.10.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/8] sdhci: Turn timeout timer into delayed work
2013-10-21 15:25 [PATCH v2 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
@ 2013-10-21 15:25 ` Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 2/8] sdhci: Turn tuning " Jeremie Samuel
` (8 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Jeremie Samuel @ 2013-10-21 15:25 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] 13+ messages in thread
* [PATCH v2 2/8] sdhci: Turn tuning timeout timer into delayed work
2013-10-21 15:25 [PATCH v2 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 1/8] sdhci: Turn timeout timer into delayed work Jeremie Samuel
@ 2013-10-21 15:25 ` Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 3/8] sdhci: Use work structs instead of tasklets Jeremie Samuel
` (7 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Jeremie Samuel @ 2013-10-21 15:25 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] 13+ messages in thread
* [PATCH v2 3/8] sdhci: Use work structs instead of tasklets
2013-10-21 15:25 [PATCH v2 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 1/8] sdhci: Turn timeout timer into delayed work Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 2/8] sdhci: Turn tuning " Jeremie Samuel
@ 2013-10-21 15:25 ` Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 4/8] sdhci: Use threaded IRQ handler Jeremie Samuel
` (6 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Jeremie Samuel @ 2013-10-21 15:25 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-bcm-kona.c | 2 +-
drivers/mmc/host/sdhci-dove.c | 6 ++--
drivers/mmc/host/sdhci-s3c.c | 2 +-
drivers/mmc/host/sdhci-sirf.c | 4 +--
drivers/mmc/host/sdhci-spear.c | 2 +-
drivers/mmc/host/sdhci.c | 55 +++++++++++++++++--------------------
include/linux/mmc/sdhci.h | 4 +--
7 files changed, 35 insertions(+), 40 deletions(-)
diff --git a/drivers/mmc/host/sdhci-bcm-kona.c b/drivers/mmc/host/sdhci-bcm-kona.c
index 7a190fe..9e3652c 100644
--- a/drivers/mmc/host/sdhci-bcm-kona.c
+++ b/drivers/mmc/host/sdhci-bcm-kona.c
@@ -120,7 +120,7 @@ static void sdhci_bcm_kona_sd_init(struct sdhci_host *host)
* Software emulation of the SD card insertion/removal. Set insert=1 for insert
* and insert=0 for removal. The card detection is done by GPIO. For Broadcom
* IP to function properly the bit 0 of CORESTAT register needs to be set/reset
- * to generate the CD IRQ handled in sdhci.c which schedules card_tasklet.
+ * to generate the CD IRQ handled in sdhci.c which schedules card_detect_work.
*/
static int sdhci_bcm_kona_sd_card_emulate(struct sdhci_host *host, int insert)
{
diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c
index 8424839..45c7f25 100644
--- a/drivers/mmc/host/sdhci-dove.c
+++ b/drivers/mmc/host/sdhci-dove.c
@@ -39,7 +39,7 @@ static irqreturn_t sdhci_dove_carddetect_irq(int irq, void *data)
{
struct sdhci_host *host = data;
- tasklet_schedule(&host->card_tasklet);
+ schedule_schedule(&host->card_detect_work);
return IRQ_HANDLED;
}
@@ -149,8 +149,8 @@ static int sdhci_dove_probe(struct platform_device *pdev)
goto err_sdhci_add;
/*
- * We must request the IRQ after sdhci_add_host(), as the tasklet only
- * gets setup in sdhci_add_host() and we oops.
+ * We must request the IRQ after sdhci_add_host(), as the workqueue
+ * only gets setup in sdhci_add_host() and we oops.
*/
if (gpio_is_valid(priv->gpio_cd)) {
ret = request_irq(gpio_to_irq(priv->gpio_cd),
diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 6debda9..2a4d843 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -395,7 +395,7 @@ static void sdhci_s3c_notify_change(struct platform_device *dev, int state)
clk_disable_unprepare(sc->clk_io);
#endif
}
- tasklet_schedule(&host->card_tasklet);
+ schedule_work(&host->card_detect_work);
spin_unlock_irqrestore(&host->lock, flags);
}
}
diff --git a/drivers/mmc/host/sdhci-sirf.c b/drivers/mmc/host/sdhci-sirf.c
index 696122c..4ec2ea3 100644
--- a/drivers/mmc/host/sdhci-sirf.c
+++ b/drivers/mmc/host/sdhci-sirf.c
@@ -80,8 +80,8 @@ static int sdhci_sirf_probe(struct platform_device *pdev)
goto err_sdhci_add;
/*
- * We must request the IRQ after sdhci_add_host(), as the tasklet only
- * gets setup in sdhci_add_host() and we oops.
+ * We must request the IRQ after sdhci_add_host(), as the workqueue
+ * only gets setup in sdhci_add_host() and we oops.
*/
if (gpio_is_valid(priv->gpio_cd)) {
ret = mmc_gpio_request_cd(host->mmc, priv->gpio_cd, 0);
diff --git a/drivers/mmc/host/sdhci-spear.c b/drivers/mmc/host/sdhci-spear.c
index 2dba9f8..9b6459d 100644
--- a/drivers/mmc/host/sdhci-spear.c
+++ b/drivers/mmc/host/sdhci-spear.c
@@ -65,7 +65,7 @@ static irqreturn_t sdhci_gpio_irq(int irq, void *dev_id)
}
/* inform sdhci driver about card insertion/removal */
- tasklet_schedule(&host->card_tasklet);
+ schedule_work(&host->card_detect_work);
return IRQ_HANDLED;
}
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] 13+ messages in thread
* [PATCH v2 4/8] sdhci: Use threaded IRQ handler
2013-10-21 15:25 [PATCH v2 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
` (2 preceding siblings ...)
2013-10-21 15:25 ` [PATCH v2 3/8] sdhci: Use work structs instead of tasklets Jeremie Samuel
@ 2013-10-21 15:25 ` Jeremie Samuel
2013-10-30 19:30 ` Olof Johansson
2013-10-21 15:25 ` [PATCH v2 5/8] sdhci: Delay led blinking Jeremie Samuel
` (5 subsequent siblings)
9 siblings, 1 reply; 13+ messages in thread
From: Jeremie Samuel @ 2013-10-21 15:25 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] 13+ messages in thread
* [PATCH v2 5/8] sdhci: Delay led blinking
2013-10-21 15:25 [PATCH v2 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
` (3 preceding siblings ...)
2013-10-21 15:25 ` [PATCH v2 4/8] sdhci: Use threaded IRQ handler Jeremie Samuel
@ 2013-10-21 15:25 ` Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 6/8] sdhci: Turn host->lock into a mutex Jeremie Samuel
` (4 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Jeremie Samuel @ 2013-10-21 15:25 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] 13+ messages in thread
* [PATCH v2 6/8] sdhci: Turn host->lock into a mutex
2013-10-21 15:25 [PATCH v2 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
` (4 preceding siblings ...)
2013-10-21 15:25 ` [PATCH v2 5/8] sdhci: Delay led blinking Jeremie Samuel
@ 2013-10-21 15:25 ` Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 7/8] sdhci: Get rid of mdelay()s where it is safe and makes sense Jeremie Samuel
` (3 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Jeremie Samuel @ 2013-10-21 15:25 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 2a4d843..0210ceb 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
}
schedule_work(&host->card_detect_work);
- 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] 13+ messages in thread
* [PATCH v2 7/8] sdhci: Get rid of mdelay()s where it is safe and makes sense
2013-10-21 15:25 [PATCH v2 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
` (5 preceding siblings ...)
2013-10-21 15:25 ` [PATCH v2 6/8] sdhci: Turn host->lock into a mutex Jeremie Samuel
@ 2013-10-21 15:25 ` Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 8/8] sdhci: Use jiffies instead of a timeout counter Jeremie Samuel
` (2 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Jeremie Samuel @ 2013-10-21 15:25 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] 13+ messages in thread
* [PATCH v2 8/8] sdhci: Use jiffies instead of a timeout counter
2013-10-21 15:25 [PATCH v2 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
` (6 preceding siblings ...)
2013-10-21 15:25 ` [PATCH v2 7/8] sdhci: Get rid of mdelay()s where it is safe and makes sense Jeremie Samuel
@ 2013-10-21 15:25 ` Jeremie Samuel
2013-10-21 15:31 ` [PATCH v2 0/8] sdhci: Move real work out of an atomic context Chris Ball
2013-10-21 20:02 ` Chris Ball
9 siblings, 0 replies; 13+ messages in thread
From: Jeremie Samuel @ 2013-10-21 15:25 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] 13+ messages in thread
* Re: [PATCH v2 0/8] sdhci: Move real work out of an atomic context
2013-10-21 15:25 [PATCH v2 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
` (7 preceding siblings ...)
2013-10-21 15:25 ` [PATCH v2 8/8] sdhci: Use jiffies instead of a timeout counter Jeremie Samuel
@ 2013-10-21 15:31 ` Chris Ball
2013-10-21 15:52 ` Jeremie Samuel
2013-10-21 20:02 ` Chris Ball
9 siblings, 1 reply; 13+ messages in thread
From: Chris Ball @ 2013-10-21 15:31 UTC (permalink / raw)
To: Jeremie Samuel; +Cc: linux-mmc, Grégor Boirie, Matthieu Castet
Hi Jeremie,
On Mon, Oct 21 2013, Jeremie Samuel wrote:
> 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.
Could we get a changelog from patchset v1 to v2, for the record?
(And isn't this more like v4 of your series?)
You don't have to resend the patchset, just replying with the
changelog here is fine.
Thanks!
- Chris.
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/8] sdhci: Move real work out of an atomic context
2013-10-21 15:31 ` [PATCH v2 0/8] sdhci: Move real work out of an atomic context Chris Ball
@ 2013-10-21 15:52 ` Jeremie Samuel
0 siblings, 0 replies; 13+ messages in thread
From: Jeremie Samuel @ 2013-10-21 15:52 UTC (permalink / raw)
To: Chris Ball; +Cc: linux-mmc, Grégor Boirie, Matthieu Castet
v2:
Patches on sdhci-bcm-kona.c, sdhci-dove.c, sdhci-s3c.c, sdhci-sirf.c,
sdhci-spear.c in order to consider the change tasklet -> workqueue
Version of 16 oct 13:
Patches on sdhci-esdhc-imx.c, sdhci-pxav3.c, sdhci-s3c.c in order to
consider the change spinlock -> mutex
Version of 06 jul 13:
Resend in order to include the patches in kernel 3.12
Version of 24 may 13:
First version of my patches (based on Anton Vorontsov's patches http://thread.gmane.org/gmane.linux.kernel.mmc/2579)
Sorry for the issues. It's my first submission on a Linux mailing list.
Cordially,
--
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 17:31, Chris Ball wrote:
> Hi Jeremie,
>
> On Mon, Oct 21 2013, Jeremie Samuel wrote:
>> 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.
> Could we get a changelog from patchset v1 to v2, for the record?
> (And isn't this more like v4 of your series?)
>
> You don't have to resend the patchset, just replying with the
> changelog here is fine.
>
> Thanks!
>
> - Chris.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/8] sdhci: Move real work out of an atomic context
2013-10-21 15:25 [PATCH v2 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
` (8 preceding siblings ...)
2013-10-21 15:31 ` [PATCH v2 0/8] sdhci: Move real work out of an atomic context Chris Ball
@ 2013-10-21 20:02 ` Chris Ball
9 siblings, 0 replies; 13+ messages in thread
From: Chris Ball @ 2013-10-21 20:02 UTC (permalink / raw)
To: Jeremie Samuel; +Cc: linux-mmc, Grégor Boirie, Matthieu Castet
Hi Jeremie,
On Mon, Oct 21 2013, Jeremie Samuel wrote:
> 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.
I've merged this series to mmc-next for 3.13 now. Thanks very much!
- Chris.
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/8] sdhci: Use threaded IRQ handler
2013-10-21 15:25 ` [PATCH v2 4/8] sdhci: Use threaded IRQ handler Jeremie Samuel
@ 2013-10-30 19:30 ` Olof Johansson
0 siblings, 0 replies; 13+ messages in thread
From: Olof Johansson @ 2013-10-30 19:30 UTC (permalink / raw)
To: Jeremie Samuel
Cc: Chris Ball, linux-mmc, Grégor Boirie, Matthieu Castet,
Anton Vorontsov
Hi,
On Mon, Oct 21, 2013 at 8:25 AM, Jeremie Samuel
<jeremie.samuel.ext@parrot.com> wrote:
> 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>
It seems like this patch is causing problems on Marvell Dove (such as
SolidRun Cubox). I've bisected a boot issue on that hardware down on
linux-next.
A good boot without this patch (i.e. with this patch reverted)
contains two spurious interrupts at probe time, both of them with
timeout+error set.
Below is the output with MMC_DEBUG turned on. Given the proximity to
the merge window, if we can't sort this out I think I'd prefer to see
this patch merged and retargeted to 3.14...
Good boot without this patch:
[ 0.962882] sdhci: Secure Digital Host Controller Interface driver
[ 0.969045] sdhci: Copyright(c) Pierre Ossman
[ 0.973375] sdhci-pltfm: SDHCI platform and OF driver helper
[ 0.979149] sdhci [sdhci_add_host()]: mmc0: Auto-CMD23 unavailable
[ 0.985320] mmc0: no vqmmc regulator found
[ 0.989396] mmc0: no vmmc regulator found
[ 0.993409] sdhci: =========== REGISTER DUMP (mmc0)===========
[ 0.999221] sdhci: Sys addr: 0x00000000 | Version: 0x00000000
[ 1.005035] sdhci: Blk size: 0x00000000 | Blk cnt: 0x00000000
[ 1.010840] sdhci: Argument: 0x00000000 | Trn mode: 0x00000001
[ 1.016656] sdhci: Present: 0x017f0000 | Host ctl: 0x00000000
[ 1.022461] sdhci: Power: 0x0000000c | Blk gap: 0x00000000
[ 1.028275] sdhci: Wake-up: 0x00000000 | Clock: 0x00000100
[ 1.034080] sdhci: Timeout: 0x0000000e | Int stat: 0x00000000
[ 1.039888] sdhci: Int enab: 0x00ff0003 | Sig enab: 0x00ff0003
[ 1.045701] sdhci: AC12 err: 0x00000000 | Slot int: 0x00000000
[ 1.051500] sdhci: Caps: 0x01e032b2 | Caps_1: 0x00000000
[ 1.057313] sdhci: Cmd: 0x00000000 | Max curr: 0x00000000
[ 1.063111] sdhci: Host ctl2: 0x00000000
[ 1.067014] sdhci: ===========================================
[ 1.073201] mmc0: clock 0Hz busmode 2 powermode 1 cs 0 Vdd 21 width
0 timing 0
[ 1.094119] mmc0: clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 21
width 0 timing 0
[ 1.114117] mmc0: SDHCI controller on f1092000.sdio-host
[f1092000.sdio-host] using DMA
[ 1.122155] mmc0: mmc_rescan_try_freq: trying to init card at 400000 Hz
[ 1.128750] mmc0: starting CMD52 arg 00000c00 flags 00000195
[ 1.134710] sdhci [sdhci_irq()]: *** mmc0 got interrupt: 0x00018000
[ 1.140953] mmc0: Unexpected interrupt 0x00008000.
[ 1.145717] sdhci: =========== REGISTER DUMP (mmc0)===========
[ 1.151516] sdhci: Sys addr: 0x00000000 | Version: 0x00000000
[ 1.157313] sdhci: Blk size: 0x00000000 | Blk cnt: 0x00000000
[ 1.163110] sdhci: Argument: 0x00000c00 | Trn mode: 0x00000001
[ 1.168908] sdhci: Present: 0x01ff0000 | Host ctl: 0x00000001
[ 1.174705] sdhci: Power: 0x0000000f | Blk gap: 0x00000000
[ 1.180503] sdhci: Wake-up: 0x00000000 | Clock: 0x00004007
[ 1.186300] sdhci: Timeout: 0x0000000e | Int stat: 0x00000000
[ 1.192099] sdhci: Int enab: 0x00ff0083 | Sig enab: 0x00ff0083
[ 1.197903] sdhci: AC12 err: 0x00000000 | Slot int: 0x00000000
[ 1.203702] sdhci: Caps: 0x01e032b2 | Caps_1: 0x00000000
[ 1.209499] sdhci: Cmd: 0x0000341a | Max curr: 0x00000000
[ 1.215295] sdhci: Host ctl2: 0x00000000
[ 1.219191] sdhci: ===========================================
[ 1.225037] mmc0: req done (CMD52): -110: 00000000 00000000 00000000 00000000
[ 1.232144] mmc0: starting CMD52 arg 80000c08 flags 00000195
[ 1.237807] ata1: SATA link down (SStatus 0 SControl F300)
[ 1.243266] sdhci [sdhci_irq()]: *** mmc0 got interrupt: 0x00018000
[ 1.249503] mmc0: Unexpected interrupt 0x00008000.
[ 1.254270] sdhci: =========== REGISTER DUMP (mmc0)===========
[ 1.260069] sdhci: Sys addr: 0x00000000 | Version: 0x00000000
[ 1.265866] sdhci: Blk size: 0x00000000 | Blk cnt: 0x00000000
[ 1.271664] sdhci: Argument: 0x80000c08 | Trn mode: 0x00000001
[ 1.277462] sdhci: Present: 0x01ff0000 | Host ctl: 0x00000001
[ 1.283266] sdhci: Power: 0x0000000f | Blk gap: 0x00000000
[ 1.289064] sdhci: Wake-up: 0x00000000 | Clock: 0x00004007
[ 1.294862] sdhci: Timeout: 0x0000000e | Int stat: 0x00000000
[ 1.300660] sdhci: Int enab: 0x00ff0083 | Sig enab: 0x00ff0083
[ 1.306456] sdhci: AC12 err: 0x00000000 | Slot int: 0x00000000
[ 1.312255] sdhci: Caps: 0x01e032b2 | Caps_1: 0x00000000
[ 1.318052] sdhci: Cmd: 0x0000341a | Max curr: 0x00000000
[ 1.323848] sdhci: Host ctl2: 0x00000000
[ 1.327745] sdhci: ===========================================
A bad boot with this patch applied instead only gives:
[ 0.962875] sdhci: Secure Digital Host Controller Interface driver
[ 0.969042] sdhci: Copyright(c) Pierre Ossman
[ 0.973373] sdhci-pltfm: SDHCI platform and OF driver helper
[ 0.979146] sdhci [sdhci_add_host()]: mmc0: Auto-CMD23 unavailable
[ 0.985318] mmc0: no vqmmc regulator found
[ 0.989394] mmc0: no vmmc regulator found
[ 0.993478] sdhci: =========== REGISTER DUMP (mmc0)===========
[ 0.999290] sdhci: Sys addr: 0x00000000 | Version: 0x00000000
[ 1.005104] sdhci: Blk size: 0x00000000 | Blk cnt: 0x00000000
[ 1.010907] sdhci: Argument: 0x00000000 | Trn mode: 0x00000001
[ 1.016715] sdhci: Present: 0x017f0000 | Host ctl: 0x00000000
[ 1.022519] sdhci: Power: 0x0000000c | Blk gap: 0x00000000
[ 1.028326] sdhci: Wake-up: 0x00000000 | Clock: 0x00000100
[ 1.034139] sdhci: Timeout: 0x0000000e | Int stat: 0x00000000
[ 1.039938] sdhci: Int enab: 0x00ff0003 | Sig enab: 0x00ff0003
[ 1.045751] sdhci: AC12 err: 0x00000000 | Slot int: 0x00000000
[ 1.051550] sdhci: Caps: 0x01e032b2 | Caps_1: 0x00000000
[ 1.057363] sdhci: Cmd: 0x00000000 | Max curr: 0x00000000
[ 1.063161] sdhci: Host ctl2: 0x00000000
[ 1.067064] sdhci: ===========================================
[ 1.073351] mmc0: clock 0Hz busmode 2 powermode 1 cs 0 Vdd 21 width
0 timing 0
[ 1.094093] mmc0: clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 21
width 0 timing 0
[ 1.114093] mmc0: SDHCI controller on f1092000.sdio-host
[f1092000.sdio-host] using DMA
[ 1.122130] mmc0: mmc_rescan_try_freq: trying to init card at 400000 Hz
[ 1.128731] mmc0: starting CMD52 arg 00000c00 flags 00000195
[ 1.134692] sdhci [sdhci_irq()]: *** mmc0 got interrupt: 0x00018000
[ 1.140936] sdhci [sdhci_irq()]: *** mmc0 got interrupt: 0x00018000
[ 1.147183] sdhci [sdhci_irq()]: *** mmc0 got interrupt: 0x00018000
[ ... repeat forever ]
-Olof
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-10-30 19:30 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-21 15:25 [PATCH v2 0/8] sdhci: Move real work out of an atomic context Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 1/8] sdhci: Turn timeout timer into delayed work Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 2/8] sdhci: Turn tuning " Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 3/8] sdhci: Use work structs instead of tasklets Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 4/8] sdhci: Use threaded IRQ handler Jeremie Samuel
2013-10-30 19:30 ` Olof Johansson
2013-10-21 15:25 ` [PATCH v2 5/8] sdhci: Delay led blinking Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 6/8] sdhci: Turn host->lock into a mutex Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 7/8] sdhci: Get rid of mdelay()s where it is safe and makes sense Jeremie Samuel
2013-10-21 15:25 ` [PATCH v2 8/8] sdhci: Use jiffies instead of a timeout counter Jeremie Samuel
2013-10-21 15:31 ` [PATCH v2 0/8] sdhci: Move real work out of an atomic context Chris Ball
2013-10-21 15:52 ` Jeremie Samuel
2013-10-21 20:02 ` Chris Ball
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.