Andrew Morton wrote:
On Fri, 10 Jul 2009 15:43:09 +0300
Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>From 242fae6293adec671b14354f215217354f5076a0 Mon Sep 17 00:00:00 2001
From: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Date: Sat, 16 May 2009 10:32:34 +0300
Subject: [PATCH] omap_hsmmc: prevent races with irq handler
If an unexpected interrupt occurs while preparing the
next request, an oops can occur.
For example, a new request is setting up DMA for data
transfer so host->data is not NULL. An unexpected
transfer complete (TC) interrupt comes along and
the interrupt handler sets host->data to NULL. Oops!
Prevent that by disabling interrupts while setting up
a new request.
Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
---
drivers/mmc/host/omap_hsmmc.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 28563d6..38e1410 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -452,6 +452,13 @@ mmc_omap_start_command(struct mmc_omap_host *host, struct mmc_command *cmd,
if (host->use_dma)
cmdreg |= DMA_EN;
+ /*
+ * In an interrupt context (i.e. STOP command), the interrupt is already
+ * enabled, otherwise it is not (i.e. new request).
+ */
+ if (!in_interrupt())
+ enable_irq(host->irq);
+
OMAP_HSMMC_WRITE(host->base, ARG, cmd->arg);
OMAP_HSMMC_WRITE(host->base, CMD, cmdreg);
}
@@ -1011,6 +1018,13 @@ static void omap_mmc_request(struct mmc_host *mmc, struct mmc_request *req)
struct mmc_omap_host *host = mmc_priv(mmc);
int err;
+ /*
+ * Prevent races with the interrupt handler because of unexpected
+ * interrupts, but not if we are already in interrupt context i.e.
+ * retries.
+ */
+ if (!in_interrupt())
+ disable_irq(host->irq);
WARN_ON(host->mrq != NULL);
host->mrq = req;
err = mmc_omap_prepare_data(host, req);
@@ -1019,6 +1033,8 @@ static void omap_mmc_request(struct mmc_host *mmc, struct mmc_request *req)
if (req->data)
req->data->error = err;
host->mrq = NULL;
+ if (!in_interrupt())
+ enable_irq(host->irq);
mmc_request_done(mmc, req);
return;
}
That seems pretty crude. Disabling an interrupt line can be expensive,
and will shut off any other innocent devices which share the line.
The usual and superior way of fixing races such as this is
spin_lock_irq[save]().
From f54be3678907b85a00d9f51e3ce4c828fc21ee95 Mon Sep 17 00:00:00 2001
From: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Date: Mon, 27 Jul 2009 12:00:17 +0300
Subject: [PATCH] omap_hsmmc: use spin_lock not disable_irq
Disabling an interrupt line can be expensive. The usual and
superior way of fixing races such as this is spin_lock_irqsave.
Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
---
drivers/mmc/host/omap_hsmmc.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 8900bae..9850280 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -146,6 +146,8 @@ struct omap_hsmmc_host {
struct work_struct mmc_carddetect_work;
void __iomem *base;
resource_size_t mapbase;
+ spinlock_t irq_lock;
+ unsigned long flags;
unsigned int id;
unsigned int dma_len;
unsigned int dma_sg_idx;
@@ -462,7 +464,7 @@ omap_hsmmc_start_command(struct omap_hsmmc_host *host, struct mmc_command *cmd,
* enabled, otherwise it is not (i.e. new request).
*/
if (!in_interrupt())
- enable_irq(host->irq);
+ spin_unlock_irqrestore(&host->irq_lock, host->flags);
OMAP_HSMMC_WRITE(host->base, ARG, cmd->arg);
OMAP_HSMMC_WRITE(host->base, CMD, cmdreg);
@@ -626,11 +628,14 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)
struct mmc_data *data;
int end_cmd = 0, end_trans = 0, status;
+ spin_lock(&host->irq_lock);
+
if (host->mrq == NULL) {
OMAP_HSMMC_WRITE(host->base, STAT,
OMAP_HSMMC_READ(host->base, STAT));
/* Flush posted write */
OMAP_HSMMC_READ(host->base, STAT);
+ spin_unlock(&host->irq_lock);
return IRQ_HANDLED;
}
@@ -696,6 +701,8 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)
if ((end_trans || (status & TC)) && host->mrq)
omap_hsmmc_xfer_done(host, data);
+ spin_unlock(&host->irq_lock);
+
return IRQ_HANDLED;
}
@@ -1066,7 +1073,7 @@ static void omap_hsmmc_request(struct mmc_host *mmc, struct mmc_request *req)
* retries.
*/
if (!in_interrupt()) {
- disable_irq(host->irq);
+ spin_lock_irqsave(&host->irq_lock, host->flags);
/*
* Protect the card from I/O if there is a possibility
* it can be removed.
@@ -1085,7 +1092,7 @@ static void omap_hsmmc_request(struct mmc_host *mmc, struct mmc_request *req)
req->cmd->error = -EBADF;
if (req->data)
req->data->error = -EBADF;
- enable_irq(host->irq);
+ spin_unlock_irqrestore(&host->irq_lock, host->flags);
mmc_request_done(mmc, req);
return;
} else if (host->reqs_blocked)
@@ -1100,7 +1107,7 @@ static void omap_hsmmc_request(struct mmc_host *mmc, struct mmc_request *req)
req->data->error = err;
host->mrq = NULL;
if (!in_interrupt())
- enable_irq(host->irq);
+ spin_unlock_irqrestore(&host->irq_lock, host->flags);
mmc_request_done(mmc, req);
return;
}
@@ -1653,6 +1660,7 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
mmc->f_max = 52000000;
sema_init(&host->sem, 1);
+ spin_lock_init(&host->irq_lock);
host->iclk = clk_get(&pdev->dev, "ick");
if (IS_ERR(host->iclk)) {
--
1.5.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
