All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4 v7] mmc: tmio, sdhi: provide multiple irq handlers
@ 2011-08-19  7:57 ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  7:57 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm, Simon Horman

The SDHI driver already supports making use of up to three interrupt
sources.

This series breaks up the existing interrupt handler into three handlers,
one for card access, one for card detect interrupts, and one for SDIO
interrupts.  A cover-all handler, which makes use of these new broken-out
handlers is provided for for the case where there is only one interrupt
source.

This series also wires up the broken-out irq handlers in the SDHI driver

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



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

* [PATCH 0/4 v7] mmc: tmio, sdhi: provide multiple irq handlers
@ 2011-08-19  7:57 ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  7:57 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm, Simon Horman

The SDHI driver already supports making use of up to three interrupt
sources.

This series breaks up the existing interrupt handler into three handlers,
one for card access, one for card detect interrupts, and one for SDIO
interrupts.  A cover-all handler, which makes use of these new broken-out
handlers is provided for for the case where there is only one interrupt
source.

This series also wires up the broken-out irq handlers in the SDHI driver

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



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

* [PATCH 1/4] mmc: tmio: Cache interrupt masks
  2011-08-19  7:57 ` Simon Horman
@ 2011-08-19  7:57   ` Simon Horman
  -1 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  7:57 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm, Simon Horman

This avoids the need to look up the masks each time an interrupt
is handled.

As suggested by Guennadi.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested

v3
* As suggested by Guennadi Liakhovetski
  - Only read sdcard_irq_mask once and never read sdio_irq_mask,
    instead use the cached values as much as possible

v2
* Initial release
---
 drivers/mmc/host/tmio_mmc.h     |    4 ++++
 drivers/mmc/host/tmio_mmc_pio.c |   34 ++++++++++++++++++----------------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index eeaf643..1cf8db5 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -79,6 +79,10 @@ struct tmio_mmc_host {
 	struct delayed_work	delayed_reset_work;
 	struct work_struct	done;
 
+	/* Cache IRQ mask */
+	u32			sdcard_irq_mask;
+	u32			sdio_irq_mask;
+
 	spinlock_t		lock;		/* protect host private data */
 	unsigned long		last_req_ts;
 	struct mutex		ios_lock;	/* protect set_ios() context */
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 1f16357..f0c7830 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -48,14 +48,14 @@
 
 void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i)
 {
-	u32 mask = sd_ctrl_read32(host, CTL_IRQ_MASK) & ~(i & TMIO_MASK_IRQ);
-	sd_ctrl_write32(host, CTL_IRQ_MASK, mask);
+	host->sdcard_irq_mask &= ~(i & TMIO_MASK_IRQ);
+	sd_ctrl_write32(host, CTL_IRQ_MASK, host->sdcard_irq_mask);
 }
 
 void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i)
 {
-	u32 mask = sd_ctrl_read32(host, CTL_IRQ_MASK) | (i & TMIO_MASK_IRQ);
-	sd_ctrl_write32(host, CTL_IRQ_MASK, mask);
+	host->sdcard_irq_mask |= (i & TMIO_MASK_IRQ);
+	sd_ctrl_write32(host, CTL_IRQ_MASK, host->sdcard_irq_mask);
 }
 
 static void tmio_mmc_ack_mmc_irqs(struct tmio_mmc_host *host, u32 i)
@@ -127,11 +127,13 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
 
 	if (enable) {
 		host->sdio_irq_enabled = 1;
+		host->sdio_irq_mask = TMIO_SDIO_MASK_ALL &
+					~TMIO_SDIO_STAT_IOIRQ;
 		sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001);
-		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK,
-			(TMIO_SDIO_MASK_ALL & ~TMIO_SDIO_STAT_IOIRQ));
+		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
 	} else {
-		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, TMIO_SDIO_MASK_ALL);
+		host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
+		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
 		sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000);
 		host->sdio_irq_enabled = 0;
 	}
@@ -548,26 +550,25 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 	struct tmio_mmc_host *host = devid;
 	struct mmc_host *mmc = host->mmc;
 	struct tmio_mmc_data *pdata = host->pdata;
-	unsigned int ireg, irq_mask, status;
-	unsigned int sdio_ireg, sdio_irq_mask, sdio_status;
+	unsigned int ireg, status;
+	unsigned int sdio_ireg, sdio_status;
 
 	pr_debug("MMC IRQ begin\n");
 
 	status = sd_ctrl_read32(host, CTL_STATUS);
-	irq_mask = sd_ctrl_read32(host, CTL_IRQ_MASK);
-	ireg = status & TMIO_MASK_IRQ & ~irq_mask;
+	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
 
 	sdio_ireg = 0;
 	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
 		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
-		sdio_irq_mask = sd_ctrl_read16(host, CTL_SDIO_IRQ_MASK);
-		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL & ~sdio_irq_mask;
+		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
+				~host->sdio_irq_mask;
 
 		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
 
 		if (sdio_ireg && !host->sdio_irq_enabled) {
 			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
-				   sdio_status, sdio_irq_mask, sdio_ireg);
+				   sdio_status, host->sdio_irq_mask, sdio_ireg);
 			tmio_mmc_enable_sdio_irq(mmc, 0);
 			goto out;
 		}
@@ -623,9 +624,9 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 	}
 
 	pr_warning("tmio_mmc: Spurious irq, disabling! "
-		"0x%08x 0x%08x 0x%08x\n", status, irq_mask, ireg);
+		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
 	pr_debug_status(status);
-	tmio_mmc_disable_mmc_irqs(host, status & ~irq_mask);
+	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
 
 out:
 	return IRQ_HANDLED;
@@ -882,6 +883,7 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	tmio_mmc_clk_stop(_host);
 	tmio_mmc_reset(_host);
 
+	_host->sdcard_irq_mask = sd_ctrl_read32(_host, CTL_IRQ_MASK);
 	tmio_mmc_disable_mmc_irqs(_host, TMIO_MASK_ALL);
 	if (pdata->flags & TMIO_MMC_SDIO_IRQ)
 		tmio_mmc_enable_sdio_irq(mmc, 0);
-- 
1.7.5.4


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

* [PATCH 1/4] mmc: tmio: Cache interrupt masks
@ 2011-08-19  7:57   ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  7:57 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm, Simon Horman

This avoids the need to look up the masks each time an interrupt
is handled.

As suggested by Guennadi.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested

v3
* As suggested by Guennadi Liakhovetski
  - Only read sdcard_irq_mask once and never read sdio_irq_mask,
    instead use the cached values as much as possible

v2
* Initial release
---
 drivers/mmc/host/tmio_mmc.h     |    4 ++++
 drivers/mmc/host/tmio_mmc_pio.c |   34 ++++++++++++++++++----------------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index eeaf643..1cf8db5 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -79,6 +79,10 @@ struct tmio_mmc_host {
 	struct delayed_work	delayed_reset_work;
 	struct work_struct	done;
 
+	/* Cache IRQ mask */
+	u32			sdcard_irq_mask;
+	u32			sdio_irq_mask;
+
 	spinlock_t		lock;		/* protect host private data */
 	unsigned long		last_req_ts;
 	struct mutex		ios_lock;	/* protect set_ios() context */
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 1f16357..f0c7830 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -48,14 +48,14 @@
 
 void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i)
 {
-	u32 mask = sd_ctrl_read32(host, CTL_IRQ_MASK) & ~(i & TMIO_MASK_IRQ);
-	sd_ctrl_write32(host, CTL_IRQ_MASK, mask);
+	host->sdcard_irq_mask &= ~(i & TMIO_MASK_IRQ);
+	sd_ctrl_write32(host, CTL_IRQ_MASK, host->sdcard_irq_mask);
 }
 
 void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i)
 {
-	u32 mask = sd_ctrl_read32(host, CTL_IRQ_MASK) | (i & TMIO_MASK_IRQ);
-	sd_ctrl_write32(host, CTL_IRQ_MASK, mask);
+	host->sdcard_irq_mask |= (i & TMIO_MASK_IRQ);
+	sd_ctrl_write32(host, CTL_IRQ_MASK, host->sdcard_irq_mask);
 }
 
 static void tmio_mmc_ack_mmc_irqs(struct tmio_mmc_host *host, u32 i)
@@ -127,11 +127,13 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
 
 	if (enable) {
 		host->sdio_irq_enabled = 1;
+		host->sdio_irq_mask = TMIO_SDIO_MASK_ALL &
+					~TMIO_SDIO_STAT_IOIRQ;
 		sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001);
-		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK,
-			(TMIO_SDIO_MASK_ALL & ~TMIO_SDIO_STAT_IOIRQ));
+		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
 	} else {
-		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, TMIO_SDIO_MASK_ALL);
+		host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
+		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
 		sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000);
 		host->sdio_irq_enabled = 0;
 	}
@@ -548,26 +550,25 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 	struct tmio_mmc_host *host = devid;
 	struct mmc_host *mmc = host->mmc;
 	struct tmio_mmc_data *pdata = host->pdata;
-	unsigned int ireg, irq_mask, status;
-	unsigned int sdio_ireg, sdio_irq_mask, sdio_status;
+	unsigned int ireg, status;
+	unsigned int sdio_ireg, sdio_status;
 
 	pr_debug("MMC IRQ begin\n");
 
 	status = sd_ctrl_read32(host, CTL_STATUS);
-	irq_mask = sd_ctrl_read32(host, CTL_IRQ_MASK);
-	ireg = status & TMIO_MASK_IRQ & ~irq_mask;
+	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
 
 	sdio_ireg = 0;
 	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
 		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
-		sdio_irq_mask = sd_ctrl_read16(host, CTL_SDIO_IRQ_MASK);
-		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL & ~sdio_irq_mask;
+		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
+				~host->sdio_irq_mask;
 
 		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
 
 		if (sdio_ireg && !host->sdio_irq_enabled) {
 			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
-				   sdio_status, sdio_irq_mask, sdio_ireg);
+				   sdio_status, host->sdio_irq_mask, sdio_ireg);
 			tmio_mmc_enable_sdio_irq(mmc, 0);
 			goto out;
 		}
@@ -623,9 +624,9 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 	}
 
 	pr_warning("tmio_mmc: Spurious irq, disabling! "
-		"0x%08x 0x%08x 0x%08x\n", status, irq_mask, ireg);
+		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
 	pr_debug_status(status);
-	tmio_mmc_disable_mmc_irqs(host, status & ~irq_mask);
+	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
 
 out:
 	return IRQ_HANDLED;
@@ -882,6 +883,7 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	tmio_mmc_clk_stop(_host);
 	tmio_mmc_reset(_host);
 
+	_host->sdcard_irq_mask = sd_ctrl_read32(_host, CTL_IRQ_MASK);
 	tmio_mmc_disable_mmc_irqs(_host, TMIO_MASK_ALL);
 	if (pdata->flags & TMIO_MMC_SDIO_IRQ)
 		tmio_mmc_enable_sdio_irq(mmc, 0);
-- 
1.7.5.4


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

* [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
  2011-08-19  7:57 ` Simon Horman
@ 2011-08-19  7:57   ` Simon Horman
  -1 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  7:57 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm, Simon Horman

Provide separate interrupt handlers which may be used by platforms where
SDHI has three interrupt sources.

This patch also removes the commented-out handling of CRC and other errors.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested

v4
* As suggested by Guennadi Liakhovetski
  - Use bool as return type for __tmio_mmc_sdcard_irq() and
    __tmio_mmc_card_detect_irq()

v3
* Rebase for updated "mmc: tmio: Cache interrupt masks"
* As suggested by Guennadi Liakhovetski
  - Do not alter logic to handle more than one interupt at once
  - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()

v2
* As suggested by Guennadi Liakhovetski
  - Combine 3 patches into one
  - Reduce the number of __tmio_..._irq() functions
  - Rename "...card_access..." functions as "...sdcard..."
---
 drivers/mmc/host/tmio_mmc.h     |    3 +
 drivers/mmc/host/tmio_mmc_pio.c |  131 ++++++++++++++++++++++++--------------
 2 files changed, 86 insertions(+), 48 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 1cf8db5..3020f98 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
 void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 irqreturn_t tmio_mmc_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
 
 static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
 					 unsigned long *flags)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f0c7830..6275e3d 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -545,44 +545,20 @@ out:
 	spin_unlock(&host->lock);
 }
 
-irqreturn_t tmio_mmc_irq(int irq, void *devid)
+static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
+				       int *ireg, int *status)
 {
-	struct tmio_mmc_host *host = devid;
-	struct mmc_host *mmc = host->mmc;
-	struct tmio_mmc_data *pdata = host->pdata;
-	unsigned int ireg, status;
-	unsigned int sdio_ireg, sdio_status;
-
-	pr_debug("MMC IRQ begin\n");
-
-	status = sd_ctrl_read32(host, CTL_STATUS);
-	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
+	*status = sd_ctrl_read32(host, CTL_STATUS);
+	*ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
 
-	sdio_ireg = 0;
-	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
-		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
-		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
-				~host->sdio_irq_mask;
-
-		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
-
-		if (sdio_ireg && !host->sdio_irq_enabled) {
-			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
-				   sdio_status, host->sdio_irq_mask, sdio_ireg);
-			tmio_mmc_enable_sdio_irq(mmc, 0);
-			goto out;
-		}
-
-		if (mmc->caps & MMC_CAP_SDIO_IRQ &&
-			sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
-			mmc_signal_sdio_irq(mmc);
-
-		if (sdio_ireg)
-			goto out;
-	}
+	pr_debug_status(*status);
+	pr_debug_status(*ireg);
+}
 
-	pr_debug_status(status);
-	pr_debug_status(ireg);
+static bool __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
+				      int ireg, int status)
+{
+	struct mmc_host *mmc = host->mmc;
 
 	/* Card insert / remove attempts */
 	if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
@@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
 		    !work_pending(&mmc->detect.work))
 			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
-		goto out;
+		return true;
 	}
 
-	/* CRC and other errors */
-/*	if (ireg & TMIO_STAT_ERR_IRQ)
- *		handled |= tmio_error_irq(host, irq, stat);
- */
+	return false;
+}
+
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
 
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_card_detect_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
+
+static bool __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
+				 int ireg, int status)
+{
 	/* Command completion */
 	if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
 		tmio_mmc_ack_mmc_irqs(host,
 			     TMIO_STAT_CMDRESPEND |
 			     TMIO_STAT_CMDTIMEOUT);
 		tmio_mmc_cmd_irq(host, status);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer */
 	if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
 		tmio_mmc_pio_irq(host);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer completion */
 	if (ireg & TMIO_STAT_DATAEND) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
 		tmio_mmc_data_irq(host);
-		goto out;
+		return true;
 	}
 
-	pr_warning("tmio_mmc: Spurious irq, disabling! "
-		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
-	pr_debug_status(status);
-	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
+	return false;
+}
+
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_sdcard_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
+
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	struct mmc_host *mmc = host->mmc;
+	struct tmio_mmc_data *pdata = host->pdata;
+	unsigned int ireg, status;
+
+	if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
+		return IRQ_HANDLED;
+
+	status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
+	ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
+
+	sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
+
+	if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
+		mmc_signal_sdio_irq(mmc);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdio_irq);
+
+irqreturn_t tmio_mmc_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	unsigned int ireg, status;
+
+	pr_debug("MMC IRQ begin\n");
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	if (__tmio_mmc_card_detect_irq(host, ireg, status))
+		return IRQ_HANDLED;
+	if (__tmio_mmc_sdcard_irq(host, ireg, status))
+		return IRQ_HANDLED;
+
+	tmio_mmc_sdio_irq(irq, devid);
 
-out:
 	return IRQ_HANDLED;
 }
 EXPORT_SYMBOL(tmio_mmc_irq);
-- 
1.7.5.4


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

* [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
@ 2011-08-19  7:57   ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  7:57 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm, Simon Horman

Provide separate interrupt handlers which may be used by platforms where
SDHI has three interrupt sources.

This patch also removes the commented-out handling of CRC and other errors.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested

v4
* As suggested by Guennadi Liakhovetski
  - Use bool as return type for __tmio_mmc_sdcard_irq() and
    __tmio_mmc_card_detect_irq()

v3
* Rebase for updated "mmc: tmio: Cache interrupt masks"
* As suggested by Guennadi Liakhovetski
  - Do not alter logic to handle more than one interupt at once
  - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()

v2
* As suggested by Guennadi Liakhovetski
  - Combine 3 patches into one
  - Reduce the number of __tmio_..._irq() functions
  - Rename "...card_access..." functions as "...sdcard..."
---
 drivers/mmc/host/tmio_mmc.h     |    3 +
 drivers/mmc/host/tmio_mmc_pio.c |  131 ++++++++++++++++++++++++--------------
 2 files changed, 86 insertions(+), 48 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 1cf8db5..3020f98 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
 void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 irqreturn_t tmio_mmc_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
 
 static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
 					 unsigned long *flags)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f0c7830..6275e3d 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -545,44 +545,20 @@ out:
 	spin_unlock(&host->lock);
 }
 
-irqreturn_t tmio_mmc_irq(int irq, void *devid)
+static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
+				       int *ireg, int *status)
 {
-	struct tmio_mmc_host *host = devid;
-	struct mmc_host *mmc = host->mmc;
-	struct tmio_mmc_data *pdata = host->pdata;
-	unsigned int ireg, status;
-	unsigned int sdio_ireg, sdio_status;
-
-	pr_debug("MMC IRQ begin\n");
-
-	status = sd_ctrl_read32(host, CTL_STATUS);
-	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
+	*status = sd_ctrl_read32(host, CTL_STATUS);
+	*ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
 
-	sdio_ireg = 0;
-	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
-		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
-		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
-				~host->sdio_irq_mask;
-
-		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
-
-		if (sdio_ireg && !host->sdio_irq_enabled) {
-			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
-				   sdio_status, host->sdio_irq_mask, sdio_ireg);
-			tmio_mmc_enable_sdio_irq(mmc, 0);
-			goto out;
-		}
-
-		if (mmc->caps & MMC_CAP_SDIO_IRQ &&
-			sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
-			mmc_signal_sdio_irq(mmc);
-
-		if (sdio_ireg)
-			goto out;
-	}
+	pr_debug_status(*status);
+	pr_debug_status(*ireg);
+}
 
-	pr_debug_status(status);
-	pr_debug_status(ireg);
+static bool __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
+				      int ireg, int status)
+{
+	struct mmc_host *mmc = host->mmc;
 
 	/* Card insert / remove attempts */
 	if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
@@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
 		    !work_pending(&mmc->detect.work))
 			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
-		goto out;
+		return true;
 	}
 
-	/* CRC and other errors */
-/*	if (ireg & TMIO_STAT_ERR_IRQ)
- *		handled |= tmio_error_irq(host, irq, stat);
- */
+	return false;
+}
+
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
 
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_card_detect_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
+
+static bool __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
+				 int ireg, int status)
+{
 	/* Command completion */
 	if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
 		tmio_mmc_ack_mmc_irqs(host,
 			     TMIO_STAT_CMDRESPEND |
 			     TMIO_STAT_CMDTIMEOUT);
 		tmio_mmc_cmd_irq(host, status);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer */
 	if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
 		tmio_mmc_pio_irq(host);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer completion */
 	if (ireg & TMIO_STAT_DATAEND) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
 		tmio_mmc_data_irq(host);
-		goto out;
+		return true;
 	}
 
-	pr_warning("tmio_mmc: Spurious irq, disabling! "
-		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
-	pr_debug_status(status);
-	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
+	return false;
+}
+
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_sdcard_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
+
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	struct mmc_host *mmc = host->mmc;
+	struct tmio_mmc_data *pdata = host->pdata;
+	unsigned int ireg, status;
+
+	if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
+		return IRQ_HANDLED;
+
+	status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
+	ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
+
+	sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
+
+	if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
+		mmc_signal_sdio_irq(mmc);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdio_irq);
+
+irqreturn_t tmio_mmc_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	unsigned int ireg, status;
+
+	pr_debug("MMC IRQ begin\n");
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	if (__tmio_mmc_card_detect_irq(host, ireg, status))
+		return IRQ_HANDLED;
+	if (__tmio_mmc_sdcard_irq(host, ireg, status))
+		return IRQ_HANDLED;
+
+	tmio_mmc_sdio_irq(irq, devid);
 
-out:
 	return IRQ_HANDLED;
 }
 EXPORT_SYMBOL(tmio_mmc_irq);
-- 
1.7.5.4


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

* [PATCH 3/4] mmc: sdhi: Allow name IRQs to use specific handlers
  2011-08-19  7:57 ` Simon Horman
@ 2011-08-19  7:57   ` Simon Horman
  -1 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  7:57 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm, Simon Horman

Allow named IRQs to use corresponding specific  handlers.
Allow one or more such IRQ source;
Or allow one or more multiplexed (un-named) IRQ source.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

v7
* As discussed with Magnus Damm
  - The logic implemented to handle specific IRQ sources assumes
    that platform_get_irq() takes into account the index of resource
    entries. However, it does not.

    Instead implement a scheme of named IRQs for specific IRQ sources.
    And assume that un-named IRQs are (legacy) multiplexed IRQs.

    As part of this change alter the logic for allowed IRQ sources as
    follows:
        Allow one or more specific (named) IRRs or;
        One or more multiplexed (un-named) ISRs.

v6
* As discussed with Guennadi Liakhovetski:
  - The SDCARD/SDIO change in v5 was not implemented as described.
    And the logic wasn't fully described in any case. This
    version (hope to!) implement the following logic:
    1) 1 IRQ: only resource #0 (CARD_DETECT, use tmio_mmc_irq())
    2) 2 or 3 IRQs: compulsory SDCARD and any further IRQs: use respective
       specialised ISRs.

v5
* As suggested by Guennadi Liakhovetski:
  - Allow only SH_MOBILE_SDHI_IRQ_SDCARD and
    SH_MOBILE_SDHI_IRQ_SDIO to be specified in platform data

v4
* As suggested by Guennadi Liakhovetski:
  - Correct inverted values of SH_MOBILE_SDHI_IRQ_SDCARD and
    SH_MOBILE_SDHI_IRQ_CARD_DETECT

v3
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* As suggested by Guennadi Liakhovetski:
  - Merge in patch "mmc: sdhi: Add defines for platform irq indexes"
  - Use an enum instead of defines for irq indexes

v2
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* Make use of defines provided by
  "mmc: sdhi: Add defines for platform irq indexes"
* As suggested by Guennadi Liakhovetski:
  - Don't use a loop to initialise irq handlers, the unrolled version
    is easier on the eyes (and exactly the same number of lines of code!)

wip: driver
---
 drivers/mmc/host/sh_mobile_sdhi.c  |   92 ++++++++++++++++++++++++++----------
 include/linux/mmc/sh_mobile_sdhi.h |    4 ++
 2 files changed, 71 insertions(+), 25 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 774f643..c3e9ec7 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -96,7 +96,8 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
 	struct tmio_mmc_host *host;
 	char clk_name[8];
-	int i, irq, ret;
+	int irq, ret, i = 0;
+	bool multiplexed_isr = true;
 
 	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
 	if (priv = NULL) {
@@ -153,27 +154,54 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto eprobe;
 
-	for (i = 0; i < 3; i++) {
-		irq = platform_get_irq(pdev, i);
-		if (irq < 0) {
-			if (i) {
-				continue;
-			} else {
-				ret = irq;
-				goto eirq;
-			}
-		}
-		ret = request_irq(irq, tmio_mmc_irq, 0,
+	/* Allow one or more specific (named) IRRs or;
+	 * One or more multiplexed (un-named) ISRs.
+	 */
+
+	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+	if (irq >= 0) {
+		multiplexed_isr = false;
+		ret = request_irq(irq, tmio_mmc_card_detect_irq, 0,
 				  dev_name(&pdev->dev), host);
-		if (ret) {
-			while (i--) {
-				irq = platform_get_irq(pdev, i);
-				if (irq >= 0)
-					free_irq(irq, host);
-			}
-			goto eirq;
+		if (ret)
+			goto eirq_card_detect;
+	}
+
+	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0) {
+		multiplexed_isr = false;
+		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
+				  dev_name(&pdev->dev), host);
+		if (ret)
+			goto eirq_sdcard;
+	}
+
+	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0) {
+		multiplexed_isr = false;
+		ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
+				  dev_name(&pdev->dev), host);
+		if (ret)
+			goto eirq_sdio;
+	}
+
+	if (multiplexed_isr) {
+		while (1) {
+			irq = platform_get_irq(pdev, i);
+			if (irq < 0)
+				break;
+			i++;
+			ret = request_irq(irq, tmio_mmc_irq, 0,
+				          dev_name(&pdev->dev), host);
+			if (ret)
+				goto eirq_multiplexed;
 		}
+
+		/* There must be at least one IRQ source */
+		if (!i)
+			goto eirq_multiplexed;
 	}
+
 	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
 		 mmc_hostname(host->mmc), (unsigned long)
 		 (platform_get_resource(pdev,IORESOURCE_MEM, 0)->start),
@@ -181,7 +209,20 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 
 	return ret;
 
-eirq:
+eirq_multiplexed:
+	while (i--) {
+		irq = platform_get_irq(pdev, i);
+		free_irq(irq, host);
+	}
+eirq_sdio:
+	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdcard:
+	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_card_detect:
 	tmio_mmc_host_remove(host);
 eprobe:
 	clk_disable(priv->clk);
@@ -197,16 +238,17 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 	struct tmio_mmc_host *host = mmc_priv(mmc);
 	struct sh_mobile_sdhi *priv = container_of(host->pdata, struct sh_mobile_sdhi, mmc_data);
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
-	int i, irq;
+	int i = 0, irq;
 
 	p->pdata = NULL;
 
 	tmio_mmc_host_remove(host);
 
-	for (i = 0; i < 3; i++) {
-		irq = platform_get_irq(pdev, i);
-		if (irq >= 0)
-			free_irq(irq, host);
+	while (0) {
+		irq = platform_get_irq(pdev, i++);
+		if (irq < 0)
+			break;
+		free_irq(irq, host);
 	}
 
 	clk_disable(priv->clk);
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index bd50b36..71b8054 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -6,6 +6,10 @@
 struct platform_device;
 struct tmio_mmc_data;
 
+#define SH_MOBILE_SDHI_IRQ_CARD_DETECT	"card_detect"
+#define SH_MOBILE_SDHI_IRQ_SDCARD	"sdcard"
+#define SH_MOBILE_SDHI_IRQ_SDIO		"sdio"
+
 struct sh_mobile_sdhi_info {
 	int dma_slave_tx;
 	int dma_slave_rx;
-- 
1.7.5.4


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

* [PATCH 3/4] mmc: sdhi: Allow name IRQs to use specific handlers
@ 2011-08-19  7:57   ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  7:57 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm, Simon Horman

Allow named IRQs to use corresponding specific  handlers.
Allow one or more such IRQ source;
Or allow one or more multiplexed (un-named) IRQ source.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

v7
* As discussed with Magnus Damm
  - The logic implemented to handle specific IRQ sources assumes
    that platform_get_irq() takes into account the index of resource
    entries. However, it does not.

    Instead implement a scheme of named IRQs for specific IRQ sources.
    And assume that un-named IRQs are (legacy) multiplexed IRQs.

    As part of this change alter the logic for allowed IRQ sources as
    follows:
        Allow one or more specific (named) IRRs or;
        One or more multiplexed (un-named) ISRs.

v6
* As discussed with Guennadi Liakhovetski:
  - The SDCARD/SDIO change in v5 was not implemented as described.
    And the logic wasn't fully described in any case. This
    version (hope to!) implement the following logic:
    1) 1 IRQ: only resource #0 (CARD_DETECT, use tmio_mmc_irq())
    2) 2 or 3 IRQs: compulsory SDCARD and any further IRQs: use respective
       specialised ISRs.

v5
* As suggested by Guennadi Liakhovetski:
  - Allow only SH_MOBILE_SDHI_IRQ_SDCARD and
    SH_MOBILE_SDHI_IRQ_SDIO to be specified in platform data

v4
* As suggested by Guennadi Liakhovetski:
  - Correct inverted values of SH_MOBILE_SDHI_IRQ_SDCARD and
    SH_MOBILE_SDHI_IRQ_CARD_DETECT

v3
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* As suggested by Guennadi Liakhovetski:
  - Merge in patch "mmc: sdhi: Add defines for platform irq indexes"
  - Use an enum instead of defines for irq indexes

v2
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* Make use of defines provided by
  "mmc: sdhi: Add defines for platform irq indexes"
* As suggested by Guennadi Liakhovetski:
  - Don't use a loop to initialise irq handlers, the unrolled version
    is easier on the eyes (and exactly the same number of lines of code!)

wip: driver
---
 drivers/mmc/host/sh_mobile_sdhi.c  |   92 ++++++++++++++++++++++++++----------
 include/linux/mmc/sh_mobile_sdhi.h |    4 ++
 2 files changed, 71 insertions(+), 25 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 774f643..c3e9ec7 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -96,7 +96,8 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
 	struct tmio_mmc_host *host;
 	char clk_name[8];
-	int i, irq, ret;
+	int irq, ret, i = 0;
+	bool multiplexed_isr = true;
 
 	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
 	if (priv == NULL) {
@@ -153,27 +154,54 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto eprobe;
 
-	for (i = 0; i < 3; i++) {
-		irq = platform_get_irq(pdev, i);
-		if (irq < 0) {
-			if (i) {
-				continue;
-			} else {
-				ret = irq;
-				goto eirq;
-			}
-		}
-		ret = request_irq(irq, tmio_mmc_irq, 0,
+	/* Allow one or more specific (named) IRRs or;
+	 * One or more multiplexed (un-named) ISRs.
+	 */
+
+	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+	if (irq >= 0) {
+		multiplexed_isr = false;
+		ret = request_irq(irq, tmio_mmc_card_detect_irq, 0,
 				  dev_name(&pdev->dev), host);
-		if (ret) {
-			while (i--) {
-				irq = platform_get_irq(pdev, i);
-				if (irq >= 0)
-					free_irq(irq, host);
-			}
-			goto eirq;
+		if (ret)
+			goto eirq_card_detect;
+	}
+
+	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0) {
+		multiplexed_isr = false;
+		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
+				  dev_name(&pdev->dev), host);
+		if (ret)
+			goto eirq_sdcard;
+	}
+
+	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0) {
+		multiplexed_isr = false;
+		ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
+				  dev_name(&pdev->dev), host);
+		if (ret)
+			goto eirq_sdio;
+	}
+
+	if (multiplexed_isr) {
+		while (1) {
+			irq = platform_get_irq(pdev, i);
+			if (irq < 0)
+				break;
+			i++;
+			ret = request_irq(irq, tmio_mmc_irq, 0,
+				          dev_name(&pdev->dev), host);
+			if (ret)
+				goto eirq_multiplexed;
 		}
+
+		/* There must be at least one IRQ source */
+		if (!i)
+			goto eirq_multiplexed;
 	}
+
 	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
 		 mmc_hostname(host->mmc), (unsigned long)
 		 (platform_get_resource(pdev,IORESOURCE_MEM, 0)->start),
@@ -181,7 +209,20 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 
 	return ret;
 
-eirq:
+eirq_multiplexed:
+	while (i--) {
+		irq = platform_get_irq(pdev, i);
+		free_irq(irq, host);
+	}
+eirq_sdio:
+	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdcard:
+	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_card_detect:
 	tmio_mmc_host_remove(host);
 eprobe:
 	clk_disable(priv->clk);
@@ -197,16 +238,17 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 	struct tmio_mmc_host *host = mmc_priv(mmc);
 	struct sh_mobile_sdhi *priv = container_of(host->pdata, struct sh_mobile_sdhi, mmc_data);
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
-	int i, irq;
+	int i = 0, irq;
 
 	p->pdata = NULL;
 
 	tmio_mmc_host_remove(host);
 
-	for (i = 0; i < 3; i++) {
-		irq = platform_get_irq(pdev, i);
-		if (irq >= 0)
-			free_irq(irq, host);
+	while (0) {
+		irq = platform_get_irq(pdev, i++);
+		if (irq < 0)
+			break;
+		free_irq(irq, host);
 	}
 
 	clk_disable(priv->clk);
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index bd50b36..71b8054 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -6,6 +6,10 @@
 struct platform_device;
 struct tmio_mmc_data;
 
+#define SH_MOBILE_SDHI_IRQ_CARD_DETECT	"card_detect"
+#define SH_MOBILE_SDHI_IRQ_SDCARD	"sdcard"
+#define SH_MOBILE_SDHI_IRQ_SDIO		"sdio"
+
 struct sh_mobile_sdhi_info {
 	int dma_slave_tx;
 	int dma_slave_rx;
-- 
1.7.5.4


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

* [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Name SDHI IRQ sources
  2011-08-19  7:57 ` Simon Horman
@ 2011-08-19  7:57   ` Simon Horman
  -1 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  7:57 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm, Simon Horman

This allows specific (non-multiplexed) IRQ handlers to be used.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

Requires
"mmc: sdhi: Allow specific IRQ sources to use corresponding handlers."

v7
* Rework to use named IRQs

v4
* Update for corrected ordering of SH_MOBILE_SDHI_IRQ_SDCARD and
  SH_MOBILE_SDHI_IRQ_CARD_DETECT

v2
* Initial release
---
 arch/arm/mach-shmobile/board-ag5evm.c   |    6 ++++++
 arch/arm/mach-shmobile/board-mackerel.c |    6 ++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index ce5c251..e100cad 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -353,14 +353,17 @@ static struct resource sdhi0_resources[] = {
 		.flags	= IORESOURCE_MEM,
 	},
 	[1] = {
+		.name	= SH_MOBILE_SDHI_IRQ_CARD_DETECT,
 		.start	= gic_spi(83),
 		.flags	= IORESOURCE_IRQ,
 	},
 	[2] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDCARD,
 		.start	= gic_spi(84),
 		.flags	= IORESOURCE_IRQ,
 	},
 	[3] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDIO,
 		.start	= gic_spi(85),
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -396,14 +399,17 @@ static struct resource sdhi1_resources[] = {
 		.flags	= IORESOURCE_MEM,
 	},
 	[1] = {
+		.name	= SH_MOBILE_SDHI_IRQ_CARD_DETECT,
 		.start	= gic_spi(87),
 		.flags	= IORESOURCE_IRQ,
 	},
 	[2] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDCARD,
 		.start	= gic_spi(88),
 		.flags	= IORESOURCE_IRQ,
 	},
 	[3] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDIO,
 		.start	= gic_spi(89),
 		.flags	= IORESOURCE_IRQ,
 	},
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index d41c01f..492274f 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1066,14 +1066,17 @@ static struct resource sdhi1_resources[] = {
 		.flags	= IORESOURCE_MEM,
 	},
 	[1] = {
+		.name	= SH_MOBILE_SDHI_IRQ_CARD_DETECT,
 		.start	= evt2irq(0x0e80), /* SDHI1_SDHI1I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
 	[2] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDCARD,
 		.start	= evt2irq(0x0ea0), /* SDHI1_SDHI1I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
 	[3] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDIO,
 		.start	= evt2irq(0x0ec0), /* SDHI1_SDHI1I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1117,14 +1120,17 @@ static struct resource sdhi2_resources[] = {
 		.flags	= IORESOURCE_MEM,
 	},
 	[1] = {
+		.name	= SH_MOBILE_SDHI_IRQ_CARD_DETECT,
 		.start	= evt2irq(0x1200), /* SDHI2_SDHI2I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
 	[2] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDCARD,
 		.start	= evt2irq(0x1220), /* SDHI2_SDHI2I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
 	[3] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDIO,
 		.start	= evt2irq(0x1240), /* SDHI2_SDHI2I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
-- 
1.7.5.4


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

* [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Name SDHI IRQ sources
@ 2011-08-19  7:57   ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  7:57 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm, Simon Horman

This allows specific (non-multiplexed) IRQ handlers to be used.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

Requires
"mmc: sdhi: Allow specific IRQ sources to use corresponding handlers."

v7
* Rework to use named IRQs

v4
* Update for corrected ordering of SH_MOBILE_SDHI_IRQ_SDCARD and
  SH_MOBILE_SDHI_IRQ_CARD_DETECT

v2
* Initial release
---
 arch/arm/mach-shmobile/board-ag5evm.c   |    6 ++++++
 arch/arm/mach-shmobile/board-mackerel.c |    6 ++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index ce5c251..e100cad 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -353,14 +353,17 @@ static struct resource sdhi0_resources[] = {
 		.flags	= IORESOURCE_MEM,
 	},
 	[1] = {
+		.name	= SH_MOBILE_SDHI_IRQ_CARD_DETECT,
 		.start	= gic_spi(83),
 		.flags	= IORESOURCE_IRQ,
 	},
 	[2] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDCARD,
 		.start	= gic_spi(84),
 		.flags	= IORESOURCE_IRQ,
 	},
 	[3] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDIO,
 		.start	= gic_spi(85),
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -396,14 +399,17 @@ static struct resource sdhi1_resources[] = {
 		.flags	= IORESOURCE_MEM,
 	},
 	[1] = {
+		.name	= SH_MOBILE_SDHI_IRQ_CARD_DETECT,
 		.start	= gic_spi(87),
 		.flags	= IORESOURCE_IRQ,
 	},
 	[2] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDCARD,
 		.start	= gic_spi(88),
 		.flags	= IORESOURCE_IRQ,
 	},
 	[3] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDIO,
 		.start	= gic_spi(89),
 		.flags	= IORESOURCE_IRQ,
 	},
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index d41c01f..492274f 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1066,14 +1066,17 @@ static struct resource sdhi1_resources[] = {
 		.flags	= IORESOURCE_MEM,
 	},
 	[1] = {
+		.name	= SH_MOBILE_SDHI_IRQ_CARD_DETECT,
 		.start	= evt2irq(0x0e80), /* SDHI1_SDHI1I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
 	[2] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDCARD,
 		.start	= evt2irq(0x0ea0), /* SDHI1_SDHI1I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
 	[3] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDIO,
 		.start	= evt2irq(0x0ec0), /* SDHI1_SDHI1I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1117,14 +1120,17 @@ static struct resource sdhi2_resources[] = {
 		.flags	= IORESOURCE_MEM,
 	},
 	[1] = {
+		.name	= SH_MOBILE_SDHI_IRQ_CARD_DETECT,
 		.start	= evt2irq(0x1200), /* SDHI2_SDHI2I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
 	[2] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDCARD,
 		.start	= evt2irq(0x1220), /* SDHI2_SDHI2I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
 	[3] = {
+		.name	= SH_MOBILE_SDHI_IRQ_SDIO,
 		.start	= evt2irq(0x1240), /* SDHI2_SDHI2I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
-- 
1.7.5.4


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

* Re: [PATCH 0/4 v7] mmc: tmio, sdhi: provide multiple irq handlers
  2011-08-19  7:57 ` Simon Horman
@ 2011-08-24  5:37   ` Simon Horman
  -1 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-24  5:37 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm

On Fri, Aug 19, 2011 at 04:57:18PM +0900, Simon Horman wrote:
> The SDHI driver already supports making use of up to three interrupt
> sources.
> 
> This series breaks up the existing interrupt handler into three handlers,
> one for card access, one for card detect interrupts, and one for SDIO
> interrupts.  A cover-all handler, which makes use of these new broken-out
> handlers is provided for for the case where there is only one interrupt
> source.
> 
> This series also wires up the broken-out irq handlers in the SDHI driver

Magnus, Guennadi,

when you get a chance could I get an ack or otherwise on these changes.


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

* Re: [PATCH 0/4 v7] mmc: tmio, sdhi: provide multiple irq handlers
@ 2011-08-24  5:37   ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-24  5:37 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm

On Fri, Aug 19, 2011 at 04:57:18PM +0900, Simon Horman wrote:
> The SDHI driver already supports making use of up to three interrupt
> sources.
> 
> This series breaks up the existing interrupt handler into three handlers,
> one for card access, one for card detect interrupts, and one for SDIO
> interrupts.  A cover-all handler, which makes use of these new broken-out
> handlers is provided for for the case where there is only one interrupt
> source.
> 
> This series also wires up the broken-out irq handlers in the SDHI driver

Magnus, Guennadi,

when you get a chance could I get an ack or otherwise on these changes.


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

* Re: [PATCH 0/4 v7] mmc: tmio, sdhi: provide multiple irq handlers
  2011-08-24  5:37   ` Simon Horman
@ 2011-08-24  9:47     ` Magnus Damm
  -1 siblings, 0 replies; 36+ messages in thread
From: Magnus Damm @ 2011-08-24  9:47 UTC (permalink / raw)
  To: Simon Horman
  Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Guennadi Liakhovetski

On Wed, Aug 24, 2011 at 2:37 PM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Aug 19, 2011 at 04:57:18PM +0900, Simon Horman wrote:
>> The SDHI driver already supports making use of up to three interrupt
>> sources.
>>
>> This series breaks up the existing interrupt handler into three handlers,
>> one for card access, one for card detect interrupts, and one for SDIO
>> interrupts.  A cover-all handler, which makes use of these new broken-out
>> handlers is provided for for the case where there is only one interrupt
>> source.
>>
>> This series also wires up the broken-out irq handlers in the SDHI driver
>
> Magnus, Guennadi,
>
> when you get a chance could I get an ack or otherwise on these changes.

Hi Simon,

Your latest revision of SDHI patches looked very good, thanks for your effort!

I prefer to give acks to each of the patches individually if possible,
but I'm a bit confused by all the versions floating around. So for
now, please add my acked-by for all patches in this series.

In the future, please consider a) including version number in the
subject of the patch email and b) including a list of all the patches
(including version) in [PATCH 0/...]. That will make it easier to
track down each individual patch.

Thank you!

/ magnus

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

* Re: [PATCH 0/4 v7] mmc: tmio, sdhi: provide multiple irq handlers
@ 2011-08-24  9:47     ` Magnus Damm
  0 siblings, 0 replies; 36+ messages in thread
From: Magnus Damm @ 2011-08-24  9:47 UTC (permalink / raw)
  To: Simon Horman
  Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Guennadi Liakhovetski

On Wed, Aug 24, 2011 at 2:37 PM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Aug 19, 2011 at 04:57:18PM +0900, Simon Horman wrote:
>> The SDHI driver already supports making use of up to three interrupt
>> sources.
>>
>> This series breaks up the existing interrupt handler into three handlers,
>> one for card access, one for card detect interrupts, and one for SDIO
>> interrupts.  A cover-all handler, which makes use of these new broken-out
>> handlers is provided for for the case where there is only one interrupt
>> source.
>>
>> This series also wires up the broken-out irq handlers in the SDHI driver
>
> Magnus, Guennadi,
>
> when you get a chance could I get an ack or otherwise on these changes.

Hi Simon,

Your latest revision of SDHI patches looked very good, thanks for your effort!

I prefer to give acks to each of the patches individually if possible,
but I'm a bit confused by all the versions floating around. So for
now, please add my acked-by for all patches in this series.

In the future, please consider a) including version number in the
subject of the patch email and b) including a list of all the patches
(including version) in [PATCH 0/...]. That will make it easier to
track down each individual patch.

Thank you!

/ magnus

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

* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
  2011-08-19  4:59           ` Simon Horman
@ 2011-08-19  5:41             ` Magnus Damm
  -1 siblings, 0 replies; 36+ messages in thread
From: Magnus Damm @ 2011-08-19  5:41 UTC (permalink / raw)
  To: Simon Horman
  Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Guennadi Liakhovetski

On Fri, Aug 19, 2011 at 1:59 PM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Aug 19, 2011 at 01:27:41PM +0900, Magnus Damm wrote:
>> On Fri, Aug 19, 2011 at 12:30 PM, Simon Horman <horms@verge.net.au> wrote:
>> > On Fri, Aug 19, 2011 at 12:09:50PM +0900, Magnus Damm wrote:
>> >> On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
>> >> > Provide separate interrupt handlers which may be used by platforms where
>> >> > SDHI has three interrupt sources.
>> >> >
>> >> > This patch also removes the commented-out handling of CRC and other errors.
>> >> >
>> >> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>> >> > Cc: Magnus Damm <magnus.damm@gmail.com>
>> >> > Signed-off-by: Simon Horman <horms@verge.net.au>
>> >> >
>> >> > ---
>> >>
>> >> > +irqreturn_t tmio_mmc_irq(int irq, void *devid)
>> >> > +{
>> >> > +       struct tmio_mmc_host *host = devid;
>> >> > +       unsigned int ireg, status;
>> >> > +
>> >> > +       pr_debug("MMC IRQ begin\n");
>> >> > +
>> >> > +       tmio_mmc_card_irq_status(host, &ireg, &status);
>> >> > +       if (__tmio_mmc_card_detect_irq(host, ireg, status))
>> >> > +               return IRQ_HANDLED;
>> >> > +       if (__tmio_mmc_sdcard_irq(host, ireg, status))
>> >> > +               return IRQ_HANDLED;
>> >> > +
>> >> > +       tmio_mmc_sdio_irq(irq, devid);
>> >> >
>> >> > -out:
>> >> >        return IRQ_HANDLED;
>> >> >  }
>> >> >  EXPORT_SYMBOL(tmio_mmc_irq);
>> >>
>> >> Is there any particular reason for returning early in this interrupt
>> >> handler? By returning early I mean the "if ... return IRQ_HANDLED"
>> >> cases above.
>> >>
>> >> I realize the old ISR code in the driver does just this, so if the
>> >> goal is to stay compatible then I guess we should keep this behavior.
>> >> >From my point of view it usually makes more sense to try to handle all
>> >> events that may be associated with the IRQ.
>> >
>> > My original post had the behaviour that you suggest but Guennadi
>> > indicted that he would be much more comfortable with keeping the original
>> > behaviour as it is know to work on a wide range of hardware.
>>
>> I see, thanks for your patience...
>>
>> So I may remember this wrong, but for the 3 different interrupt
>> sources I believe that the SDIO IRQ code was added by Arnd for one of
>> the Renesas SDHI platforms. Back then Ian disliked supporting more
>> than a single interrupt source, so for that reason the SDIO IRQ code
>> was added on top of the common interrupt handler. We had no
>> documentation either, so it was added in a rather random way. I recall
>> the SDIO IRQ being handled before the other interrupt types in the
>> common handler not last, but that's not very important. Anyway, with
>> the fact that SDIO IRQ support was added for SDHI platforms in mind
>> then we can assume that other platforms won't need it. Not sure if
>> this fact will improve our situation or not.
>>
>> As for the two remaining interrupts, I believe they share hardware
>> registers somehow. I guess I'm OK keeping the original behavior
>> somehow, but I still believe it's incorrect. It may not matter very
>> much though since it's rather unlikely that hotplug insertion or eject
>> coincides with the data IRQs.
>
> Hi Magnus,
>
> to be honest I'm unsure if the current behaviour is correct or not.
> But it does appear to work on a wide range of hardware. And as
> tmio_mmc_irq() is an implementation of the single-source ISR intended
> for legacy purposes it does seem reasonable to maintain the existing
> behaviour.

Sure, I agree.

> If there is a need to change the behaviour of tmio_mmc_irq() in some way
> then I think that is best dealt with in a separate patch (series)
> subsequent to the current series that is intended to introduce broken-out
> handlers.

Sounds good, I agree that this should be handled separately if needed.

Thanks,

/ magnus

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

* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
@ 2011-08-19  5:41             ` Magnus Damm
  0 siblings, 0 replies; 36+ messages in thread
From: Magnus Damm @ 2011-08-19  5:41 UTC (permalink / raw)
  To: Simon Horman
  Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Guennadi Liakhovetski

On Fri, Aug 19, 2011 at 1:59 PM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Aug 19, 2011 at 01:27:41PM +0900, Magnus Damm wrote:
>> On Fri, Aug 19, 2011 at 12:30 PM, Simon Horman <horms@verge.net.au> wrote:
>> > On Fri, Aug 19, 2011 at 12:09:50PM +0900, Magnus Damm wrote:
>> >> On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
>> >> > Provide separate interrupt handlers which may be used by platforms where
>> >> > SDHI has three interrupt sources.
>> >> >
>> >> > This patch also removes the commented-out handling of CRC and other errors.
>> >> >
>> >> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>> >> > Cc: Magnus Damm <magnus.damm@gmail.com>
>> >> > Signed-off-by: Simon Horman <horms@verge.net.au>
>> >> >
>> >> > ---
>> >>
>> >> > +irqreturn_t tmio_mmc_irq(int irq, void *devid)
>> >> > +{
>> >> > +       struct tmio_mmc_host *host = devid;
>> >> > +       unsigned int ireg, status;
>> >> > +
>> >> > +       pr_debug("MMC IRQ begin\n");
>> >> > +
>> >> > +       tmio_mmc_card_irq_status(host, &ireg, &status);
>> >> > +       if (__tmio_mmc_card_detect_irq(host, ireg, status))
>> >> > +               return IRQ_HANDLED;
>> >> > +       if (__tmio_mmc_sdcard_irq(host, ireg, status))
>> >> > +               return IRQ_HANDLED;
>> >> > +
>> >> > +       tmio_mmc_sdio_irq(irq, devid);
>> >> >
>> >> > -out:
>> >> >        return IRQ_HANDLED;
>> >> >  }
>> >> >  EXPORT_SYMBOL(tmio_mmc_irq);
>> >>
>> >> Is there any particular reason for returning early in this interrupt
>> >> handler? By returning early I mean the "if ... return IRQ_HANDLED"
>> >> cases above.
>> >>
>> >> I realize the old ISR code in the driver does just this, so if the
>> >> goal is to stay compatible then I guess we should keep this behavior.
>> >> >From my point of view it usually makes more sense to try to handle all
>> >> events that may be associated with the IRQ.
>> >
>> > My original post had the behaviour that you suggest but Guennadi
>> > indicted that he would be much more comfortable with keeping the original
>> > behaviour as it is know to work on a wide range of hardware.
>>
>> I see, thanks for your patience...
>>
>> So I may remember this wrong, but for the 3 different interrupt
>> sources I believe that the SDIO IRQ code was added by Arnd for one of
>> the Renesas SDHI platforms. Back then Ian disliked supporting more
>> than a single interrupt source, so for that reason the SDIO IRQ code
>> was added on top of the common interrupt handler. We had no
>> documentation either, so it was added in a rather random way. I recall
>> the SDIO IRQ being handled before the other interrupt types in the
>> common handler not last, but that's not very important. Anyway, with
>> the fact that SDIO IRQ support was added for SDHI platforms in mind
>> then we can assume that other platforms won't need it. Not sure if
>> this fact will improve our situation or not.
>>
>> As for the two remaining interrupts, I believe they share hardware
>> registers somehow. I guess I'm OK keeping the original behavior
>> somehow, but I still believe it's incorrect. It may not matter very
>> much though since it's rather unlikely that hotplug insertion or eject
>> coincides with the data IRQs.
>
> Hi Magnus,
>
> to be honest I'm unsure if the current behaviour is correct or not.
> But it does appear to work on a wide range of hardware. And as
> tmio_mmc_irq() is an implementation of the single-source ISR intended
> for legacy purposes it does seem reasonable to maintain the existing
> behaviour.

Sure, I agree.

> If there is a need to change the behaviour of tmio_mmc_irq() in some way
> then I think that is best dealt with in a separate patch (series)
> subsequent to the current series that is intended to introduce broken-out
> handlers.

Sounds good, I agree that this should be handled separately if needed.

Thanks,

/ magnus

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

* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
  2011-08-19  4:27         ` Magnus Damm
@ 2011-08-19  4:59           ` Simon Horman
  -1 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  4:59 UTC (permalink / raw)
  To: Magnus Damm
  Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Guennadi Liakhovetski

On Fri, Aug 19, 2011 at 01:27:41PM +0900, Magnus Damm wrote:
> On Fri, Aug 19, 2011 at 12:30 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Fri, Aug 19, 2011 at 12:09:50PM +0900, Magnus Damm wrote:
> >> On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
> >> > Provide separate interrupt handlers which may be used by platforms where
> >> > SDHI has three interrupt sources.
> >> >
> >> > This patch also removes the commented-out handling of CRC and other errors.
> >> >
> >> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> >> > Cc: Magnus Damm <magnus.damm@gmail.com>
> >> > Signed-off-by: Simon Horman <horms@verge.net.au>
> >> >
> >> > ---
> >>
> >> > +irqreturn_t tmio_mmc_irq(int irq, void *devid)
> >> > +{
> >> > +       struct tmio_mmc_host *host = devid;
> >> > +       unsigned int ireg, status;
> >> > +
> >> > +       pr_debug("MMC IRQ begin\n");
> >> > +
> >> > +       tmio_mmc_card_irq_status(host, &ireg, &status);
> >> > +       if (__tmio_mmc_card_detect_irq(host, ireg, status))
> >> > +               return IRQ_HANDLED;
> >> > +       if (__tmio_mmc_sdcard_irq(host, ireg, status))
> >> > +               return IRQ_HANDLED;
> >> > +
> >> > +       tmio_mmc_sdio_irq(irq, devid);
> >> >
> >> > -out:
> >> >        return IRQ_HANDLED;
> >> >  }
> >> >  EXPORT_SYMBOL(tmio_mmc_irq);
> >>
> >> Is there any particular reason for returning early in this interrupt
> >> handler? By returning early I mean the "if ... return IRQ_HANDLED"
> >> cases above.
> >>
> >> I realize the old ISR code in the driver does just this, so if the
> >> goal is to stay compatible then I guess we should keep this behavior.
> >> >From my point of view it usually makes more sense to try to handle all
> >> events that may be associated with the IRQ.
> >
> > My original post had the behaviour that you suggest but Guennadi
> > indicted that he would be much more comfortable with keeping the original
> > behaviour as it is know to work on a wide range of hardware.
> 
> I see, thanks for your patience...
> 
> So I may remember this wrong, but for the 3 different interrupt
> sources I believe that the SDIO IRQ code was added by Arnd for one of
> the Renesas SDHI platforms. Back then Ian disliked supporting more
> than a single interrupt source, so for that reason the SDIO IRQ code
> was added on top of the common interrupt handler. We had no
> documentation either, so it was added in a rather random way. I recall
> the SDIO IRQ being handled before the other interrupt types in the
> common handler not last, but that's not very important. Anyway, with
> the fact that SDIO IRQ support was added for SDHI platforms in mind
> then we can assume that other platforms won't need it. Not sure if
> this fact will improve our situation or not.
> 
> As for the two remaining interrupts, I believe they share hardware
> registers somehow. I guess I'm OK keeping the original behavior
> somehow, but I still believe it's incorrect. It may not matter very
> much though since it's rather unlikely that hotplug insertion or eject
> coincides with the data IRQs.

Hi Magnus,

to be honest I'm unsure if the current behaviour is correct or not.
But it does appear to work on a wide range of hardware. And as
tmio_mmc_irq() is an implementation of the single-source ISR intended
for legacy purposes it does seem reasonable to maintain the existing
behaviour.

If there is a need to change the behaviour of tmio_mmc_irq() in some way
then I think that is best dealt with in a separate patch (series)
subsequent to the current series that is intended to introduce broken-out
handlers.

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

* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
@ 2011-08-19  4:59           ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  4:59 UTC (permalink / raw)
  To: Magnus Damm
  Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Guennadi Liakhovetski

On Fri, Aug 19, 2011 at 01:27:41PM +0900, Magnus Damm wrote:
> On Fri, Aug 19, 2011 at 12:30 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Fri, Aug 19, 2011 at 12:09:50PM +0900, Magnus Damm wrote:
> >> On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
> >> > Provide separate interrupt handlers which may be used by platforms where
> >> > SDHI has three interrupt sources.
> >> >
> >> > This patch also removes the commented-out handling of CRC and other errors.
> >> >
> >> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> >> > Cc: Magnus Damm <magnus.damm@gmail.com>
> >> > Signed-off-by: Simon Horman <horms@verge.net.au>
> >> >
> >> > ---
> >>
> >> > +irqreturn_t tmio_mmc_irq(int irq, void *devid)
> >> > +{
> >> > +       struct tmio_mmc_host *host = devid;
> >> > +       unsigned int ireg, status;
> >> > +
> >> > +       pr_debug("MMC IRQ begin\n");
> >> > +
> >> > +       tmio_mmc_card_irq_status(host, &ireg, &status);
> >> > +       if (__tmio_mmc_card_detect_irq(host, ireg, status))
> >> > +               return IRQ_HANDLED;
> >> > +       if (__tmio_mmc_sdcard_irq(host, ireg, status))
> >> > +               return IRQ_HANDLED;
> >> > +
> >> > +       tmio_mmc_sdio_irq(irq, devid);
> >> >
> >> > -out:
> >> >        return IRQ_HANDLED;
> >> >  }
> >> >  EXPORT_SYMBOL(tmio_mmc_irq);
> >>
> >> Is there any particular reason for returning early in this interrupt
> >> handler? By returning early I mean the "if ... return IRQ_HANDLED"
> >> cases above.
> >>
> >> I realize the old ISR code in the driver does just this, so if the
> >> goal is to stay compatible then I guess we should keep this behavior.
> >> >From my point of view it usually makes more sense to try to handle all
> >> events that may be associated with the IRQ.
> >
> > My original post had the behaviour that you suggest but Guennadi
> > indicted that he would be much more comfortable with keeping the original
> > behaviour as it is know to work on a wide range of hardware.
> 
> I see, thanks for your patience...
> 
> So I may remember this wrong, but for the 3 different interrupt
> sources I believe that the SDIO IRQ code was added by Arnd for one of
> the Renesas SDHI platforms. Back then Ian disliked supporting more
> than a single interrupt source, so for that reason the SDIO IRQ code
> was added on top of the common interrupt handler. We had no
> documentation either, so it was added in a rather random way. I recall
> the SDIO IRQ being handled before the other interrupt types in the
> common handler not last, but that's not very important. Anyway, with
> the fact that SDIO IRQ support was added for SDHI platforms in mind
> then we can assume that other platforms won't need it. Not sure if
> this fact will improve our situation or not.
> 
> As for the two remaining interrupts, I believe they share hardware
> registers somehow. I guess I'm OK keeping the original behavior
> somehow, but I still believe it's incorrect. It may not matter very
> much though since it's rather unlikely that hotplug insertion or eject
> coincides with the data IRQs.

Hi Magnus,

to be honest I'm unsure if the current behaviour is correct or not.
But it does appear to work on a wide range of hardware. And as
tmio_mmc_irq() is an implementation of the single-source ISR intended
for legacy purposes it does seem reasonable to maintain the existing
behaviour.

If there is a need to change the behaviour of tmio_mmc_irq() in some way
then I think that is best dealt with in a separate patch (series)
subsequent to the current series that is intended to introduce broken-out
handlers.

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

* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
  2011-08-19  3:30       ` Simon Horman
@ 2011-08-19  4:27         ` Magnus Damm
  -1 siblings, 0 replies; 36+ messages in thread
From: Magnus Damm @ 2011-08-19  4:27 UTC (permalink / raw)
  To: Simon Horman
  Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Guennadi Liakhovetski

On Fri, Aug 19, 2011 at 12:30 PM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Aug 19, 2011 at 12:09:50PM +0900, Magnus Damm wrote:
>> On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
>> > Provide separate interrupt handlers which may be used by platforms where
>> > SDHI has three interrupt sources.
>> >
>> > This patch also removes the commented-out handling of CRC and other errors.
>> >
>> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>> > Cc: Magnus Damm <magnus.damm@gmail.com>
>> > Signed-off-by: Simon Horman <horms@verge.net.au>
>> >
>> > ---
>>
>> > +irqreturn_t tmio_mmc_irq(int irq, void *devid)
>> > +{
>> > +       struct tmio_mmc_host *host = devid;
>> > +       unsigned int ireg, status;
>> > +
>> > +       pr_debug("MMC IRQ begin\n");
>> > +
>> > +       tmio_mmc_card_irq_status(host, &ireg, &status);
>> > +       if (__tmio_mmc_card_detect_irq(host, ireg, status))
>> > +               return IRQ_HANDLED;
>> > +       if (__tmio_mmc_sdcard_irq(host, ireg, status))
>> > +               return IRQ_HANDLED;
>> > +
>> > +       tmio_mmc_sdio_irq(irq, devid);
>> >
>> > -out:
>> >        return IRQ_HANDLED;
>> >  }
>> >  EXPORT_SYMBOL(tmio_mmc_irq);
>>
>> Is there any particular reason for returning early in this interrupt
>> handler? By returning early I mean the "if ... return IRQ_HANDLED"
>> cases above.
>>
>> I realize the old ISR code in the driver does just this, so if the
>> goal is to stay compatible then I guess we should keep this behavior.
>> >From my point of view it usually makes more sense to try to handle all
>> events that may be associated with the IRQ.
>
> My original post had the behaviour that you suggest but Guennadi
> indicted that he would be much more comfortable with keeping the original
> behaviour as it is know to work on a wide range of hardware.

I see, thanks for your patience...

So I may remember this wrong, but for the 3 different interrupt
sources I believe that the SDIO IRQ code was added by Arnd for one of
the Renesas SDHI platforms. Back then Ian disliked supporting more
than a single interrupt source, so for that reason the SDIO IRQ code
was added on top of the common interrupt handler. We had no
documentation either, so it was added in a rather random way. I recall
the SDIO IRQ being handled before the other interrupt types in the
common handler not last, but that's not very important. Anyway, with
the fact that SDIO IRQ support was added for SDHI platforms in mind
then we can assume that other platforms won't need it. Not sure if
this fact will improve our situation or not.

As for the two remaining interrupts, I believe they share hardware
registers somehow. I guess I'm OK keeping the original behavior
somehow, but I still believe it's incorrect. It may not matter very
much though since it's rather unlikely that hotplug insertion or eject
coincides with the data IRQs.

Thanks for your help!

/ magnus

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

* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
@ 2011-08-19  4:27         ` Magnus Damm
  0 siblings, 0 replies; 36+ messages in thread
From: Magnus Damm @ 2011-08-19  4:27 UTC (permalink / raw)
  To: Simon Horman
  Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Guennadi Liakhovetski

On Fri, Aug 19, 2011 at 12:30 PM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Aug 19, 2011 at 12:09:50PM +0900, Magnus Damm wrote:
>> On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
>> > Provide separate interrupt handlers which may be used by platforms where
>> > SDHI has three interrupt sources.
>> >
>> > This patch also removes the commented-out handling of CRC and other errors.
>> >
>> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>> > Cc: Magnus Damm <magnus.damm@gmail.com>
>> > Signed-off-by: Simon Horman <horms@verge.net.au>
>> >
>> > ---
>>
>> > +irqreturn_t tmio_mmc_irq(int irq, void *devid)
>> > +{
>> > +       struct tmio_mmc_host *host = devid;
>> > +       unsigned int ireg, status;
>> > +
>> > +       pr_debug("MMC IRQ begin\n");
>> > +
>> > +       tmio_mmc_card_irq_status(host, &ireg, &status);
>> > +       if (__tmio_mmc_card_detect_irq(host, ireg, status))
>> > +               return IRQ_HANDLED;
>> > +       if (__tmio_mmc_sdcard_irq(host, ireg, status))
>> > +               return IRQ_HANDLED;
>> > +
>> > +       tmio_mmc_sdio_irq(irq, devid);
>> >
>> > -out:
>> >        return IRQ_HANDLED;
>> >  }
>> >  EXPORT_SYMBOL(tmio_mmc_irq);
>>
>> Is there any particular reason for returning early in this interrupt
>> handler? By returning early I mean the "if ... return IRQ_HANDLED"
>> cases above.
>>
>> I realize the old ISR code in the driver does just this, so if the
>> goal is to stay compatible then I guess we should keep this behavior.
>> >From my point of view it usually makes more sense to try to handle all
>> events that may be associated with the IRQ.
>
> My original post had the behaviour that you suggest but Guennadi
> indicted that he would be much more comfortable with keeping the original
> behaviour as it is know to work on a wide range of hardware.

I see, thanks for your patience...

So I may remember this wrong, but for the 3 different interrupt
sources I believe that the SDIO IRQ code was added by Arnd for one of
the Renesas SDHI platforms. Back then Ian disliked supporting more
than a single interrupt source, so for that reason the SDIO IRQ code
was added on top of the common interrupt handler. We had no
documentation either, so it was added in a rather random way. I recall
the SDIO IRQ being handled before the other interrupt types in the
common handler not last, but that's not very important. Anyway, with
the fact that SDIO IRQ support was added for SDHI platforms in mind
then we can assume that other platforms won't need it. Not sure if
this fact will improve our situation or not.

As for the two remaining interrupts, I believe they share hardware
registers somehow. I guess I'm OK keeping the original behavior
somehow, but I still believe it's incorrect. It may not matter very
much though since it's rather unlikely that hotplug insertion or eject
coincides with the data IRQs.

Thanks for your help!

/ magnus

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

* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
  2011-08-19  3:09     ` Magnus Damm
@ 2011-08-19  3:30       ` Simon Horman
  -1 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  3:30 UTC (permalink / raw)
  To: Magnus Damm
  Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Guennadi Liakhovetski

On Fri, Aug 19, 2011 at 12:09:50PM +0900, Magnus Damm wrote:
> On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
> > Provide separate interrupt handlers which may be used by platforms where
> > SDHI has three interrupt sources.
> >
> > This patch also removes the commented-out handling of CRC and other errors.
> >
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Magnus Damm <magnus.damm@gmail.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> >
> > ---
> 
> > +irqreturn_t tmio_mmc_irq(int irq, void *devid)
> > +{
> > +       struct tmio_mmc_host *host = devid;
> > +       unsigned int ireg, status;
> > +
> > +       pr_debug("MMC IRQ begin\n");
> > +
> > +       tmio_mmc_card_irq_status(host, &ireg, &status);
> > +       if (__tmio_mmc_card_detect_irq(host, ireg, status))
> > +               return IRQ_HANDLED;
> > +       if (__tmio_mmc_sdcard_irq(host, ireg, status))
> > +               return IRQ_HANDLED;
> > +
> > +       tmio_mmc_sdio_irq(irq, devid);
> >
> > -out:
> >        return IRQ_HANDLED;
> >  }
> >  EXPORT_SYMBOL(tmio_mmc_irq);
> 
> Is there any particular reason for returning early in this interrupt
> handler? By returning early I mean the "if ... return IRQ_HANDLED"
> cases above.
> 
> I realize the old ISR code in the driver does just this, so if the
> goal is to stay compatible then I guess we should keep this behavior.
> >From my point of view it usually makes more sense to try to handle all
> events that may be associated with the IRQ.

My original post had the behaviour that you suggest but Guennadi
indicted that he would be much more comfortable with keeping the original
behaviour as it is know to work on a wide range of hardware.



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

* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
@ 2011-08-19  3:30       ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  3:30 UTC (permalink / raw)
  To: Magnus Damm
  Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Guennadi Liakhovetski

On Fri, Aug 19, 2011 at 12:09:50PM +0900, Magnus Damm wrote:
> On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
> > Provide separate interrupt handlers which may be used by platforms where
> > SDHI has three interrupt sources.
> >
> > This patch also removes the commented-out handling of CRC and other errors.
> >
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Magnus Damm <magnus.damm@gmail.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> >
> > ---
> 
> > +irqreturn_t tmio_mmc_irq(int irq, void *devid)
> > +{
> > +       struct tmio_mmc_host *host = devid;
> > +       unsigned int ireg, status;
> > +
> > +       pr_debug("MMC IRQ begin\n");
> > +
> > +       tmio_mmc_card_irq_status(host, &ireg, &status);
> > +       if (__tmio_mmc_card_detect_irq(host, ireg, status))
> > +               return IRQ_HANDLED;
> > +       if (__tmio_mmc_sdcard_irq(host, ireg, status))
> > +               return IRQ_HANDLED;
> > +
> > +       tmio_mmc_sdio_irq(irq, devid);
> >
> > -out:
> >        return IRQ_HANDLED;
> >  }
> >  EXPORT_SYMBOL(tmio_mmc_irq);
> 
> Is there any particular reason for returning early in this interrupt
> handler? By returning early I mean the "if ... return IRQ_HANDLED"
> cases above.
> 
> I realize the old ISR code in the driver does just this, so if the
> goal is to stay compatible then I guess we should keep this behavior.
> >From my point of view it usually makes more sense to try to handle all
> events that may be associated with the IRQ.

My original post had the behaviour that you suggest but Guennadi
indicted that he would be much more comfortable with keeping the original
behaviour as it is know to work on a wide range of hardware.



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

* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
  2011-08-19  1:10   ` Simon Horman
@ 2011-08-19  3:09     ` Magnus Damm
  -1 siblings, 0 replies; 36+ messages in thread
From: Magnus Damm @ 2011-08-19  3:09 UTC (permalink / raw)
  To: Simon Horman
  Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Guennadi Liakhovetski

On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
> Provide separate interrupt handlers which may be used by platforms where
> SDHI has three interrupt sources.
>
> This patch also removes the commented-out handling of CRC and other errors.
>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
>
> ---

> +irqreturn_t tmio_mmc_irq(int irq, void *devid)
> +{
> +       struct tmio_mmc_host *host = devid;
> +       unsigned int ireg, status;
> +
> +       pr_debug("MMC IRQ begin\n");
> +
> +       tmio_mmc_card_irq_status(host, &ireg, &status);
> +       if (__tmio_mmc_card_detect_irq(host, ireg, status))
> +               return IRQ_HANDLED;
> +       if (__tmio_mmc_sdcard_irq(host, ireg, status))
> +               return IRQ_HANDLED;
> +
> +       tmio_mmc_sdio_irq(irq, devid);
>
> -out:
>        return IRQ_HANDLED;
>  }
>  EXPORT_SYMBOL(tmio_mmc_irq);

Is there any particular reason for returning early in this interrupt
handler? By returning early I mean the "if ... return IRQ_HANDLED"
cases above.

I realize the old ISR code in the driver does just this, so if the
goal is to stay compatible then I guess we should keep this behavior.
From my point of view it usually makes more sense to try to handle all
events that may be associated with the IRQ.

Thanks,

/ magnus

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

* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
@ 2011-08-19  3:09     ` Magnus Damm
  0 siblings, 0 replies; 36+ messages in thread
From: Magnus Damm @ 2011-08-19  3:09 UTC (permalink / raw)
  To: Simon Horman
  Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Guennadi Liakhovetski

On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
> Provide separate interrupt handlers which may be used by platforms where
> SDHI has three interrupt sources.
>
> This patch also removes the commented-out handling of CRC and other errors.
>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
>
> ---

> +irqreturn_t tmio_mmc_irq(int irq, void *devid)
> +{
> +       struct tmio_mmc_host *host = devid;
> +       unsigned int ireg, status;
> +
> +       pr_debug("MMC IRQ begin\n");
> +
> +       tmio_mmc_card_irq_status(host, &ireg, &status);
> +       if (__tmio_mmc_card_detect_irq(host, ireg, status))
> +               return IRQ_HANDLED;
> +       if (__tmio_mmc_sdcard_irq(host, ireg, status))
> +               return IRQ_HANDLED;
> +
> +       tmio_mmc_sdio_irq(irq, devid);
>
> -out:
>        return IRQ_HANDLED;
>  }
>  EXPORT_SYMBOL(tmio_mmc_irq);

Is there any particular reason for returning early in this interrupt
handler? By returning early I mean the "if ... return IRQ_HANDLED"
cases above.

I realize the old ISR code in the driver does just this, so if the
goal is to stay compatible then I guess we should keep this behavior.
From my point of view it usually makes more sense to try to handle all
events that may be associated with the IRQ.

Thanks,

/ magnus

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

* [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
  2011-08-19  1:10 [PATCH 0/4 v6] " Simon Horman
@ 2011-08-19  1:10   ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  1:10 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm, Simon Horman

Provide separate interrupt handlers which may be used by platforms where
SDHI has three interrupt sources.

This patch also removes the commented-out handling of CRC and other errors.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested

v4
* As suggested by Guennadi Liakhovetski
  - Use bool as return type for __tmio_mmc_sdcard_irq() and
    __tmio_mmc_card_detect_irq()

v3
* Rebase for updated "mmc: tmio: Cache interrupt masks"
* As suggested by Guennadi Liakhovetski
  - Do not alter logic to handle more than one interupt at once
  - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()

v2
* As suggested by Guennadi Liakhovetski
  - Combine 3 patches into one
  - Reduce the number of __tmio_..._irq() functions
  - Rename "...card_access..." functions as "...sdcard..."
---
 drivers/mmc/host/tmio_mmc.h     |    3 +
 drivers/mmc/host/tmio_mmc_pio.c |  131 ++++++++++++++++++++++++--------------
 2 files changed, 86 insertions(+), 48 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 1cf8db5..3020f98 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
 void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 irqreturn_t tmio_mmc_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
 
 static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
 					 unsigned long *flags)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f0c7830..6275e3d 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -545,44 +545,20 @@ out:
 	spin_unlock(&host->lock);
 }
 
-irqreturn_t tmio_mmc_irq(int irq, void *devid)
+static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
+				       int *ireg, int *status)
 {
-	struct tmio_mmc_host *host = devid;
-	struct mmc_host *mmc = host->mmc;
-	struct tmio_mmc_data *pdata = host->pdata;
-	unsigned int ireg, status;
-	unsigned int sdio_ireg, sdio_status;
-
-	pr_debug("MMC IRQ begin\n");
-
-	status = sd_ctrl_read32(host, CTL_STATUS);
-	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
+	*status = sd_ctrl_read32(host, CTL_STATUS);
+	*ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
 
-	sdio_ireg = 0;
-	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
-		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
-		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
-				~host->sdio_irq_mask;
-
-		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
-
-		if (sdio_ireg && !host->sdio_irq_enabled) {
-			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
-				   sdio_status, host->sdio_irq_mask, sdio_ireg);
-			tmio_mmc_enable_sdio_irq(mmc, 0);
-			goto out;
-		}
-
-		if (mmc->caps & MMC_CAP_SDIO_IRQ &&
-			sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
-			mmc_signal_sdio_irq(mmc);
-
-		if (sdio_ireg)
-			goto out;
-	}
+	pr_debug_status(*status);
+	pr_debug_status(*ireg);
+}
 
-	pr_debug_status(status);
-	pr_debug_status(ireg);
+static bool __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
+				      int ireg, int status)
+{
+	struct mmc_host *mmc = host->mmc;
 
 	/* Card insert / remove attempts */
 	if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
@@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
 		    !work_pending(&mmc->detect.work))
 			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
-		goto out;
+		return true;
 	}
 
-	/* CRC and other errors */
-/*	if (ireg & TMIO_STAT_ERR_IRQ)
- *		handled |= tmio_error_irq(host, irq, stat);
- */
+	return false;
+}
+
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
 
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_card_detect_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
+
+static bool __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
+				 int ireg, int status)
+{
 	/* Command completion */
 	if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
 		tmio_mmc_ack_mmc_irqs(host,
 			     TMIO_STAT_CMDRESPEND |
 			     TMIO_STAT_CMDTIMEOUT);
 		tmio_mmc_cmd_irq(host, status);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer */
 	if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
 		tmio_mmc_pio_irq(host);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer completion */
 	if (ireg & TMIO_STAT_DATAEND) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
 		tmio_mmc_data_irq(host);
-		goto out;
+		return true;
 	}
 
-	pr_warning("tmio_mmc: Spurious irq, disabling! "
-		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
-	pr_debug_status(status);
-	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
+	return false;
+}
+
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_sdcard_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
+
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	struct mmc_host *mmc = host->mmc;
+	struct tmio_mmc_data *pdata = host->pdata;
+	unsigned int ireg, status;
+
+	if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
+		return IRQ_HANDLED;
+
+	status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
+	ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
+
+	sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
+
+	if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
+		mmc_signal_sdio_irq(mmc);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdio_irq);
+
+irqreturn_t tmio_mmc_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	unsigned int ireg, status;
+
+	pr_debug("MMC IRQ begin\n");
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	if (__tmio_mmc_card_detect_irq(host, ireg, status))
+		return IRQ_HANDLED;
+	if (__tmio_mmc_sdcard_irq(host, ireg, status))
+		return IRQ_HANDLED;
+
+	tmio_mmc_sdio_irq(irq, devid);
 
-out:
 	return IRQ_HANDLED;
 }
 EXPORT_SYMBOL(tmio_mmc_irq);
-- 
1.7.5.4


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

* [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
@ 2011-08-19  1:10   ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-19  1:10 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm, Simon Horman

Provide separate interrupt handlers which may be used by platforms where
SDHI has three interrupt sources.

This patch also removes the commented-out handling of CRC and other errors.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested

v4
* As suggested by Guennadi Liakhovetski
  - Use bool as return type for __tmio_mmc_sdcard_irq() and
    __tmio_mmc_card_detect_irq()

v3
* Rebase for updated "mmc: tmio: Cache interrupt masks"
* As suggested by Guennadi Liakhovetski
  - Do not alter logic to handle more than one interupt at once
  - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()

v2
* As suggested by Guennadi Liakhovetski
  - Combine 3 patches into one
  - Reduce the number of __tmio_..._irq() functions
  - Rename "...card_access..." functions as "...sdcard..."
---
 drivers/mmc/host/tmio_mmc.h     |    3 +
 drivers/mmc/host/tmio_mmc_pio.c |  131 ++++++++++++++++++++++++--------------
 2 files changed, 86 insertions(+), 48 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 1cf8db5..3020f98 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
 void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 irqreturn_t tmio_mmc_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
 
 static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
 					 unsigned long *flags)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f0c7830..6275e3d 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -545,44 +545,20 @@ out:
 	spin_unlock(&host->lock);
 }
 
-irqreturn_t tmio_mmc_irq(int irq, void *devid)
+static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
+				       int *ireg, int *status)
 {
-	struct tmio_mmc_host *host = devid;
-	struct mmc_host *mmc = host->mmc;
-	struct tmio_mmc_data *pdata = host->pdata;
-	unsigned int ireg, status;
-	unsigned int sdio_ireg, sdio_status;
-
-	pr_debug("MMC IRQ begin\n");
-
-	status = sd_ctrl_read32(host, CTL_STATUS);
-	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
+	*status = sd_ctrl_read32(host, CTL_STATUS);
+	*ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
 
-	sdio_ireg = 0;
-	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
-		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
-		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
-				~host->sdio_irq_mask;
-
-		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
-
-		if (sdio_ireg && !host->sdio_irq_enabled) {
-			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
-				   sdio_status, host->sdio_irq_mask, sdio_ireg);
-			tmio_mmc_enable_sdio_irq(mmc, 0);
-			goto out;
-		}
-
-		if (mmc->caps & MMC_CAP_SDIO_IRQ &&
-			sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
-			mmc_signal_sdio_irq(mmc);
-
-		if (sdio_ireg)
-			goto out;
-	}
+	pr_debug_status(*status);
+	pr_debug_status(*ireg);
+}
 
-	pr_debug_status(status);
-	pr_debug_status(ireg);
+static bool __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
+				      int ireg, int status)
+{
+	struct mmc_host *mmc = host->mmc;
 
 	/* Card insert / remove attempts */
 	if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
@@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
 		    !work_pending(&mmc->detect.work))
 			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
-		goto out;
+		return true;
 	}
 
-	/* CRC and other errors */
-/*	if (ireg & TMIO_STAT_ERR_IRQ)
- *		handled |= tmio_error_irq(host, irq, stat);
- */
+	return false;
+}
+
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
 
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_card_detect_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
+
+static bool __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
+				 int ireg, int status)
+{
 	/* Command completion */
 	if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
 		tmio_mmc_ack_mmc_irqs(host,
 			     TMIO_STAT_CMDRESPEND |
 			     TMIO_STAT_CMDTIMEOUT);
 		tmio_mmc_cmd_irq(host, status);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer */
 	if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
 		tmio_mmc_pio_irq(host);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer completion */
 	if (ireg & TMIO_STAT_DATAEND) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
 		tmio_mmc_data_irq(host);
-		goto out;
+		return true;
 	}
 
-	pr_warning("tmio_mmc: Spurious irq, disabling! "
-		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
-	pr_debug_status(status);
-	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
+	return false;
+}
+
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_sdcard_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
+
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	struct mmc_host *mmc = host->mmc;
+	struct tmio_mmc_data *pdata = host->pdata;
+	unsigned int ireg, status;
+
+	if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
+		return IRQ_HANDLED;
+
+	status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
+	ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
+
+	sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
+
+	if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
+		mmc_signal_sdio_irq(mmc);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdio_irq);
+
+irqreturn_t tmio_mmc_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	unsigned int ireg, status;
+
+	pr_debug("MMC IRQ begin\n");
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	if (__tmio_mmc_card_detect_irq(host, ireg, status))
+		return IRQ_HANDLED;
+	if (__tmio_mmc_sdcard_irq(host, ireg, status))
+		return IRQ_HANDLED;
+
+	tmio_mmc_sdio_irq(irq, devid);
 
-out:
 	return IRQ_HANDLED;
 }
 EXPORT_SYMBOL(tmio_mmc_irq);
-- 
1.7.5.4


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

* [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
  2011-08-17 10:59 [PATCH 0/4 v5] mmc: tmio, sdhi: provide multiple irq handlers Simon Horman
@ 2011-08-17 10:59   ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-17 10:59 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Guennadi Liakhovetski, Magnus Damm, Paul Mundt, Simon Horman

Provide separate interrupt handlers which may be used by platforms where
SDHI has three interrupt sources.

This patch also removes the commented-out handling of CRC and other errors.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested

v4
* As suggested by Guennadi Liakhovetski
  - Use bool as return type for __tmio_mmc_sdcard_irq() and
    __tmio_mmc_card_detect_irq()

v3
* Rebase for updated "mmc: tmio: Cache interrupt masks"
* As suggested by Guennadi Liakhovetski
  - Do not alter logic to handle more than one interupt at once
  - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()

v2
* As suggested by Guennadi Liakhovetski
  - Combine 3 patches into one
  - Reduce the number of __tmio_..._irq() functions
  - Rename "...card_access..." functions as "...sdcard..."
---
 drivers/mmc/host/tmio_mmc.h     |    3 +
 drivers/mmc/host/tmio_mmc_pio.c |  131 ++++++++++++++++++++++++--------------
 2 files changed, 86 insertions(+), 48 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 1cf8db5..3020f98 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
 void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 irqreturn_t tmio_mmc_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
 
 static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
 					 unsigned long *flags)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f0c7830..6275e3d 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -545,44 +545,20 @@ out:
 	spin_unlock(&host->lock);
 }
 
-irqreturn_t tmio_mmc_irq(int irq, void *devid)
+static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
+				       int *ireg, int *status)
 {
-	struct tmio_mmc_host *host = devid;
-	struct mmc_host *mmc = host->mmc;
-	struct tmio_mmc_data *pdata = host->pdata;
-	unsigned int ireg, status;
-	unsigned int sdio_ireg, sdio_status;
-
-	pr_debug("MMC IRQ begin\n");
-
-	status = sd_ctrl_read32(host, CTL_STATUS);
-	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
+	*status = sd_ctrl_read32(host, CTL_STATUS);
+	*ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
 
-	sdio_ireg = 0;
-	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
-		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
-		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
-				~host->sdio_irq_mask;
-
-		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
-
-		if (sdio_ireg && !host->sdio_irq_enabled) {
-			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
-				   sdio_status, host->sdio_irq_mask, sdio_ireg);
-			tmio_mmc_enable_sdio_irq(mmc, 0);
-			goto out;
-		}
-
-		if (mmc->caps & MMC_CAP_SDIO_IRQ &&
-			sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
-			mmc_signal_sdio_irq(mmc);
-
-		if (sdio_ireg)
-			goto out;
-	}
+	pr_debug_status(*status);
+	pr_debug_status(*ireg);
+}
 
-	pr_debug_status(status);
-	pr_debug_status(ireg);
+static bool __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
+				      int ireg, int status)
+{
+	struct mmc_host *mmc = host->mmc;
 
 	/* Card insert / remove attempts */
 	if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
@@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
 		    !work_pending(&mmc->detect.work))
 			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
-		goto out;
+		return true;
 	}
 
-	/* CRC and other errors */
-/*	if (ireg & TMIO_STAT_ERR_IRQ)
- *		handled |= tmio_error_irq(host, irq, stat);
- */
+	return false;
+}
+
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
 
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_card_detect_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
+
+static bool __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
+				 int ireg, int status)
+{
 	/* Command completion */
 	if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
 		tmio_mmc_ack_mmc_irqs(host,
 			     TMIO_STAT_CMDRESPEND |
 			     TMIO_STAT_CMDTIMEOUT);
 		tmio_mmc_cmd_irq(host, status);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer */
 	if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
 		tmio_mmc_pio_irq(host);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer completion */
 	if (ireg & TMIO_STAT_DATAEND) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
 		tmio_mmc_data_irq(host);
-		goto out;
+		return true;
 	}
 
-	pr_warning("tmio_mmc: Spurious irq, disabling! "
-		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
-	pr_debug_status(status);
-	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
+	return false;
+}
+
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_sdcard_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
+
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	struct mmc_host *mmc = host->mmc;
+	struct tmio_mmc_data *pdata = host->pdata;
+	unsigned int ireg, status;
+
+	if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
+		return IRQ_HANDLED;
+
+	status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
+	ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
+
+	sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
+
+	if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
+		mmc_signal_sdio_irq(mmc);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdio_irq);
+
+irqreturn_t tmio_mmc_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	unsigned int ireg, status;
+
+	pr_debug("MMC IRQ begin\n");
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	if (__tmio_mmc_card_detect_irq(host, ireg, status))
+		return IRQ_HANDLED;
+	if (__tmio_mmc_sdcard_irq(host, ireg, status))
+		return IRQ_HANDLED;
+
+	tmio_mmc_sdio_irq(irq, devid);
 
-out:
 	return IRQ_HANDLED;
 }
 EXPORT_SYMBOL(tmio_mmc_irq);
-- 
1.7.5.4


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

* [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
@ 2011-08-17 10:59   ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-17 10:59 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Guennadi Liakhovetski, Magnus Damm, Paul Mundt, Simon Horman

Provide separate interrupt handlers which may be used by platforms where
SDHI has three interrupt sources.

This patch also removes the commented-out handling of CRC and other errors.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested

v4
* As suggested by Guennadi Liakhovetski
  - Use bool as return type for __tmio_mmc_sdcard_irq() and
    __tmio_mmc_card_detect_irq()

v3
* Rebase for updated "mmc: tmio: Cache interrupt masks"
* As suggested by Guennadi Liakhovetski
  - Do not alter logic to handle more than one interupt at once
  - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()

v2
* As suggested by Guennadi Liakhovetski
  - Combine 3 patches into one
  - Reduce the number of __tmio_..._irq() functions
  - Rename "...card_access..." functions as "...sdcard..."
---
 drivers/mmc/host/tmio_mmc.h     |    3 +
 drivers/mmc/host/tmio_mmc_pio.c |  131 ++++++++++++++++++++++++--------------
 2 files changed, 86 insertions(+), 48 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 1cf8db5..3020f98 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
 void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 irqreturn_t tmio_mmc_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
 
 static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
 					 unsigned long *flags)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f0c7830..6275e3d 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -545,44 +545,20 @@ out:
 	spin_unlock(&host->lock);
 }
 
-irqreturn_t tmio_mmc_irq(int irq, void *devid)
+static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
+				       int *ireg, int *status)
 {
-	struct tmio_mmc_host *host = devid;
-	struct mmc_host *mmc = host->mmc;
-	struct tmio_mmc_data *pdata = host->pdata;
-	unsigned int ireg, status;
-	unsigned int sdio_ireg, sdio_status;
-
-	pr_debug("MMC IRQ begin\n");
-
-	status = sd_ctrl_read32(host, CTL_STATUS);
-	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
+	*status = sd_ctrl_read32(host, CTL_STATUS);
+	*ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
 
-	sdio_ireg = 0;
-	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
-		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
-		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
-				~host->sdio_irq_mask;
-
-		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
-
-		if (sdio_ireg && !host->sdio_irq_enabled) {
-			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
-				   sdio_status, host->sdio_irq_mask, sdio_ireg);
-			tmio_mmc_enable_sdio_irq(mmc, 0);
-			goto out;
-		}
-
-		if (mmc->caps & MMC_CAP_SDIO_IRQ &&
-			sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
-			mmc_signal_sdio_irq(mmc);
-
-		if (sdio_ireg)
-			goto out;
-	}
+	pr_debug_status(*status);
+	pr_debug_status(*ireg);
+}
 
-	pr_debug_status(status);
-	pr_debug_status(ireg);
+static bool __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
+				      int ireg, int status)
+{
+	struct mmc_host *mmc = host->mmc;
 
 	/* Card insert / remove attempts */
 	if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
@@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
 		    !work_pending(&mmc->detect.work))
 			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
-		goto out;
+		return true;
 	}
 
-	/* CRC and other errors */
-/*	if (ireg & TMIO_STAT_ERR_IRQ)
- *		handled |= tmio_error_irq(host, irq, stat);
- */
+	return false;
+}
+
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
 
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_card_detect_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
+
+static bool __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
+				 int ireg, int status)
+{
 	/* Command completion */
 	if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
 		tmio_mmc_ack_mmc_irqs(host,
 			     TMIO_STAT_CMDRESPEND |
 			     TMIO_STAT_CMDTIMEOUT);
 		tmio_mmc_cmd_irq(host, status);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer */
 	if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
 		tmio_mmc_pio_irq(host);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer completion */
 	if (ireg & TMIO_STAT_DATAEND) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
 		tmio_mmc_data_irq(host);
-		goto out;
+		return true;
 	}
 
-	pr_warning("tmio_mmc: Spurious irq, disabling! "
-		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
-	pr_debug_status(status);
-	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
+	return false;
+}
+
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_sdcard_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
+
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	struct mmc_host *mmc = host->mmc;
+	struct tmio_mmc_data *pdata = host->pdata;
+	unsigned int ireg, status;
+
+	if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
+		return IRQ_HANDLED;
+
+	status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
+	ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
+
+	sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
+
+	if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
+		mmc_signal_sdio_irq(mmc);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdio_irq);
+
+irqreturn_t tmio_mmc_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	unsigned int ireg, status;
+
+	pr_debug("MMC IRQ begin\n");
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	if (__tmio_mmc_card_detect_irq(host, ireg, status))
+		return IRQ_HANDLED;
+	if (__tmio_mmc_sdcard_irq(host, ireg, status))
+		return IRQ_HANDLED;
+
+	tmio_mmc_sdio_irq(irq, devid);
 
-out:
 	return IRQ_HANDLED;
 }
 EXPORT_SYMBOL(tmio_mmc_irq);
-- 
1.7.5.4


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

* [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
  2011-08-17  0:50 [PATCH 0/4 v4] mmc: tmio, sdhi: provide multiple irq handlers Simon Horman
@ 2011-08-17  0:50   ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-17  0:50 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm, Simon Horman

Provide separate interrupt handlers which may be used by platforms where
SDHI has three interrupt sources.

This patch also removes the commented-out handling of CRC and other errors.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested

v4
* As suggested by Guennadi Liakhovetski
  - Use bool as return type for __tmio_mmc_sdcard_irq() and
    __tmio_mmc_card_detect_irq()

v3
* Rebase for updated "mmc: tmio: Cache interrupt masks"
* As suggested by Guennadi Liakhovetski
  - Do not alter logic to handle more than one interupt at once
  - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()

v2
* As suggested by Guennadi Liakhovetski
  - Combine 3 patches into one
  - Reduce the number of __tmio_..._irq() functions
  - Rename "...card_access..." functions as "...sdcard..."
---
 drivers/mmc/host/tmio_mmc.h     |    3 +
 drivers/mmc/host/tmio_mmc_pio.c |  131 ++++++++++++++++++++++++--------------
 2 files changed, 86 insertions(+), 48 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 1cf8db5..3020f98 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
 void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 irqreturn_t tmio_mmc_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
 
 static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
 					 unsigned long *flags)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f0c7830..6275e3d 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -545,44 +545,20 @@ out:
 	spin_unlock(&host->lock);
 }
 
-irqreturn_t tmio_mmc_irq(int irq, void *devid)
+static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
+				       int *ireg, int *status)
 {
-	struct tmio_mmc_host *host = devid;
-	struct mmc_host *mmc = host->mmc;
-	struct tmio_mmc_data *pdata = host->pdata;
-	unsigned int ireg, status;
-	unsigned int sdio_ireg, sdio_status;
-
-	pr_debug("MMC IRQ begin\n");
-
-	status = sd_ctrl_read32(host, CTL_STATUS);
-	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
+	*status = sd_ctrl_read32(host, CTL_STATUS);
+	*ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
 
-	sdio_ireg = 0;
-	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
-		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
-		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
-				~host->sdio_irq_mask;
-
-		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
-
-		if (sdio_ireg && !host->sdio_irq_enabled) {
-			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
-				   sdio_status, host->sdio_irq_mask, sdio_ireg);
-			tmio_mmc_enable_sdio_irq(mmc, 0);
-			goto out;
-		}
-
-		if (mmc->caps & MMC_CAP_SDIO_IRQ &&
-			sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
-			mmc_signal_sdio_irq(mmc);
-
-		if (sdio_ireg)
-			goto out;
-	}
+	pr_debug_status(*status);
+	pr_debug_status(*ireg);
+}
 
-	pr_debug_status(status);
-	pr_debug_status(ireg);
+static bool __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
+				      int ireg, int status)
+{
+	struct mmc_host *mmc = host->mmc;
 
 	/* Card insert / remove attempts */
 	if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
@@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
 		    !work_pending(&mmc->detect.work))
 			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
-		goto out;
+		return true;
 	}
 
-	/* CRC and other errors */
-/*	if (ireg & TMIO_STAT_ERR_IRQ)
- *		handled |= tmio_error_irq(host, irq, stat);
- */
+	return false;
+}
+
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
 
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_card_detect_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
+
+static bool __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
+				 int ireg, int status)
+{
 	/* Command completion */
 	if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
 		tmio_mmc_ack_mmc_irqs(host,
 			     TMIO_STAT_CMDRESPEND |
 			     TMIO_STAT_CMDTIMEOUT);
 		tmio_mmc_cmd_irq(host, status);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer */
 	if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
 		tmio_mmc_pio_irq(host);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer completion */
 	if (ireg & TMIO_STAT_DATAEND) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
 		tmio_mmc_data_irq(host);
-		goto out;
+		return true;
 	}
 
-	pr_warning("tmio_mmc: Spurious irq, disabling! "
-		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
-	pr_debug_status(status);
-	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
+	return false;
+}
+
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_sdcard_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
+
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	struct mmc_host *mmc = host->mmc;
+	struct tmio_mmc_data *pdata = host->pdata;
+	unsigned int ireg, status;
+
+	if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
+		return IRQ_HANDLED;
+
+	status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
+	ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
+
+	sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
+
+	if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
+		mmc_signal_sdio_irq(mmc);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdio_irq);
+
+irqreturn_t tmio_mmc_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	unsigned int ireg, status;
+
+	pr_debug("MMC IRQ begin\n");
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	if (__tmio_mmc_card_detect_irq(host, ireg, status))
+		return IRQ_HANDLED;
+	if (__tmio_mmc_sdcard_irq(host, ireg, status))
+		return IRQ_HANDLED;
+
+	tmio_mmc_sdio_irq(irq, devid);
 
-out:
 	return IRQ_HANDLED;
 }
 EXPORT_SYMBOL(tmio_mmc_irq);
-- 
1.7.5.4


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

* [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
@ 2011-08-17  0:50   ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-17  0:50 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm, Simon Horman

Provide separate interrupt handlers which may be used by platforms where
SDHI has three interrupt sources.

This patch also removes the commented-out handling of CRC and other errors.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested

v4
* As suggested by Guennadi Liakhovetski
  - Use bool as return type for __tmio_mmc_sdcard_irq() and
    __tmio_mmc_card_detect_irq()

v3
* Rebase for updated "mmc: tmio: Cache interrupt masks"
* As suggested by Guennadi Liakhovetski
  - Do not alter logic to handle more than one interupt at once
  - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()

v2
* As suggested by Guennadi Liakhovetski
  - Combine 3 patches into one
  - Reduce the number of __tmio_..._irq() functions
  - Rename "...card_access..." functions as "...sdcard..."
---
 drivers/mmc/host/tmio_mmc.h     |    3 +
 drivers/mmc/host/tmio_mmc_pio.c |  131 ++++++++++++++++++++++++--------------
 2 files changed, 86 insertions(+), 48 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 1cf8db5..3020f98 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
 void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 irqreturn_t tmio_mmc_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
 
 static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
 					 unsigned long *flags)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f0c7830..6275e3d 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -545,44 +545,20 @@ out:
 	spin_unlock(&host->lock);
 }
 
-irqreturn_t tmio_mmc_irq(int irq, void *devid)
+static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
+				       int *ireg, int *status)
 {
-	struct tmio_mmc_host *host = devid;
-	struct mmc_host *mmc = host->mmc;
-	struct tmio_mmc_data *pdata = host->pdata;
-	unsigned int ireg, status;
-	unsigned int sdio_ireg, sdio_status;
-
-	pr_debug("MMC IRQ begin\n");
-
-	status = sd_ctrl_read32(host, CTL_STATUS);
-	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
+	*status = sd_ctrl_read32(host, CTL_STATUS);
+	*ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
 
-	sdio_ireg = 0;
-	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
-		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
-		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
-				~host->sdio_irq_mask;
-
-		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
-
-		if (sdio_ireg && !host->sdio_irq_enabled) {
-			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
-				   sdio_status, host->sdio_irq_mask, sdio_ireg);
-			tmio_mmc_enable_sdio_irq(mmc, 0);
-			goto out;
-		}
-
-		if (mmc->caps & MMC_CAP_SDIO_IRQ &&
-			sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
-			mmc_signal_sdio_irq(mmc);
-
-		if (sdio_ireg)
-			goto out;
-	}
+	pr_debug_status(*status);
+	pr_debug_status(*ireg);
+}
 
-	pr_debug_status(status);
-	pr_debug_status(ireg);
+static bool __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
+				      int ireg, int status)
+{
+	struct mmc_host *mmc = host->mmc;
 
 	/* Card insert / remove attempts */
 	if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
@@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
 		    !work_pending(&mmc->detect.work))
 			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
-		goto out;
+		return true;
 	}
 
-	/* CRC and other errors */
-/*	if (ireg & TMIO_STAT_ERR_IRQ)
- *		handled |= tmio_error_irq(host, irq, stat);
- */
+	return false;
+}
+
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
 
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_card_detect_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
+
+static bool __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
+				 int ireg, int status)
+{
 	/* Command completion */
 	if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
 		tmio_mmc_ack_mmc_irqs(host,
 			     TMIO_STAT_CMDRESPEND |
 			     TMIO_STAT_CMDTIMEOUT);
 		tmio_mmc_cmd_irq(host, status);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer */
 	if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
 		tmio_mmc_pio_irq(host);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer completion */
 	if (ireg & TMIO_STAT_DATAEND) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
 		tmio_mmc_data_irq(host);
-		goto out;
+		return true;
 	}
 
-	pr_warning("tmio_mmc: Spurious irq, disabling! "
-		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
-	pr_debug_status(status);
-	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
+	return false;
+}
+
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_sdcard_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
+
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	struct mmc_host *mmc = host->mmc;
+	struct tmio_mmc_data *pdata = host->pdata;
+	unsigned int ireg, status;
+
+	if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
+		return IRQ_HANDLED;
+
+	status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
+	ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
+
+	sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
+
+	if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
+		mmc_signal_sdio_irq(mmc);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdio_irq);
+
+irqreturn_t tmio_mmc_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	unsigned int ireg, status;
+
+	pr_debug("MMC IRQ begin\n");
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	if (__tmio_mmc_card_detect_irq(host, ireg, status))
+		return IRQ_HANDLED;
+	if (__tmio_mmc_sdcard_irq(host, ireg, status))
+		return IRQ_HANDLED;
+
+	tmio_mmc_sdio_irq(irq, devid);
 
-out:
 	return IRQ_HANDLED;
 }
 EXPORT_SYMBOL(tmio_mmc_irq);
-- 
1.7.5.4


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

* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
  2011-08-16 11:14     ` Guennadi Liakhovetski
@ 2011-08-16 11:35       ` Simon Horman
  -1 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-16 11:35 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm

On Tue, Aug 16, 2011 at 01:14:01PM +0200, Guennadi Liakhovetski wrote:
> On Tue, 16 Aug 2011, Simon Horman wrote:
> 
> > Provide separate interrupt handlers which may be used by platforms where
> > SDHI has three interrupt sources.
> > 
> > This patch also removes the commented-out handling of CRC and other errors.
> > 
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Magnus Damm <magnus.damm@gmail.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> > 
> > ---
> > 
> > * SDCARD portion tested on AP4/Mackerel
> > * SDIO portion untested
> > 
> > v3
> > * Rebase for updated "mmc: tmio: Cache interrupt masks"
> > * As suggested by Guennadi Liakhovetski
> >   - Do not alter logic to handle more than one interupt at once
> >   - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()
> > 
> > v2
> > * As suggested by Guennadi Liakhovetski
> >   - Combine 3 patches into one
> >   - Reduce the number of __tmio_..._irq() functions
> >   - Rename "...card_access..." functions as "...sdcard..."
> > ---
> >  drivers/mmc/host/tmio_mmc.h     |    3 +
> >  drivers/mmc/host/tmio_mmc_pio.c |  131 ++++++++++++++++++++++++--------------
> >  2 files changed, 86 insertions(+), 48 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> > index 1cf8db5..3020f98 100644
> > --- a/drivers/mmc/host/tmio_mmc.h
> > +++ b/drivers/mmc/host/tmio_mmc.h
> > @@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
> >  void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
> >  void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
> >  irqreturn_t tmio_mmc_irq(int irq, void *devid);
> > +irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
> > +irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
> > +irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
> >  
> >  static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
> >  					 unsigned long *flags)
> > diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> > index f0c7830..37adfb2 100644
> > --- a/drivers/mmc/host/tmio_mmc_pio.c
> > +++ b/drivers/mmc/host/tmio_mmc_pio.c
> > @@ -545,44 +545,20 @@ out:
> >  	spin_unlock(&host->lock);
> >  }
> >  
> > -irqreturn_t tmio_mmc_irq(int irq, void *devid)
> > +static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
> > +				       int *ireg, int *status)
> >  {
> > -	struct tmio_mmc_host *host = devid;
> > -	struct mmc_host *mmc = host->mmc;
> > -	struct tmio_mmc_data *pdata = host->pdata;
> > -	unsigned int ireg, status;
> > -	unsigned int sdio_ireg, sdio_status;
> > -
> > -	pr_debug("MMC IRQ begin\n");
> > -
> > -	status = sd_ctrl_read32(host, CTL_STATUS);
> > -	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
> > -
> > -	sdio_ireg = 0;
> > -	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
> > -		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
> > -		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
> > -				~host->sdio_irq_mask;
> > +	*status = sd_ctrl_read32(host, CTL_STATUS);
> > +	*ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
> >  
> > -		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
> > -
> > -		if (sdio_ireg && !host->sdio_irq_enabled) {
> > -			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
> > -				   sdio_status, host->sdio_irq_mask, sdio_ireg);
> > -			tmio_mmc_enable_sdio_irq(mmc, 0);
> > -			goto out;
> > -		}
> > -
> > -		if (mmc->caps & MMC_CAP_SDIO_IRQ &&
> > -			sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
> > -			mmc_signal_sdio_irq(mmc);
> > -
> > -		if (sdio_ireg)
> > -			goto out;
> > -	}
> > +	pr_debug_status(*status);
> > +	pr_debug_status(*ireg);
> > +}
> >  
> > -	pr_debug_status(status);
> > -	pr_debug_status(ireg);
> > +static int __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
> > +				      int ireg, int status)
> > +{
> > +	struct mmc_host *mmc = host->mmc;
> >  
> >  	/* Card insert / remove attempts */
> >  	if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
> > @@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
> >  		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
> >  		    !work_pending(&mmc->detect.work))
> >  			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
> > -		goto out;
> > +		return 1;
> >  	}
> >  
> > -	/* CRC and other errors */
> > -/*	if (ireg & TMIO_STAT_ERR_IRQ)
> > - *		handled |= tmio_error_irq(host, irq, stat);
> > - */
> > +	return 0;
> > +}
> > +
> > +irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
> > +{
> > +	unsigned int ireg, status;
> > +	struct tmio_mmc_host *host = devid;
> >  
> > +	tmio_mmc_card_irq_status(host, &ireg, &status);
> > +	__tmio_mmc_card_detect_irq(host, ireg, status);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
> > +
> > +static int __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
> > +				 int ireg, int status)
> > +{
> >  	/* Command completion */
> >  	if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
> >  		tmio_mmc_ack_mmc_irqs(host,
> >  			     TMIO_STAT_CMDRESPEND |
> >  			     TMIO_STAT_CMDTIMEOUT);
> >  		tmio_mmc_cmd_irq(host, status);
> > -		goto out;
> > +		return 1;
> >  	}
> >  
> >  	/* Data transfer */
> >  	if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
> >  		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
> >  		tmio_mmc_pio_irq(host);
> > -		goto out;
> > +		return 1;
> >  	}
> >  
> >  	/* Data transfer completion */
> >  	if (ireg & TMIO_STAT_DATAEND) {
> >  		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
> >  		tmio_mmc_data_irq(host);
> > -		goto out;
> > +		return 1;
> >  	}
> >  
> > -	pr_warning("tmio_mmc: Spurious irq, disabling! "
> > -		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
> > -	pr_debug_status(status);
> > -	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
> > +	return 0;
> > +}
> > +
> > +irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
> > +{
> > +	unsigned int ireg, status;
> > +	struct tmio_mmc_host *host = devid;
> > +
> > +	tmio_mmc_card_irq_status(host, &ireg, &status);
> > +	__tmio_mmc_sdcard_irq(host, ireg, status);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
> > +
> > +irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
> > +{
> > +	struct tmio_mmc_host *host = devid;
> > +	struct mmc_host *mmc = host->mmc;
> > +	struct tmio_mmc_data *pdata = host->pdata;
> > +	unsigned int ireg, status;
> > +
> > +	if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
> > +		return IRQ_HANDLED;
> > +
> > +	status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
> > +	ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
> > +
> > +	sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
> > +
> > +	if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
> > +		mmc_signal_sdio_irq(mmc);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +EXPORT_SYMBOL(tmio_mmc_sdio_irq);
> > +
> > +irqreturn_t tmio_mmc_irq(int irq, void *devid)
> > +{
> > +	struct tmio_mmc_host *host = devid;
> > +	unsigned int ireg, status;
> > +
> > +	pr_debug("MMC IRQ begin\n");
> > +
> > +	tmio_mmc_card_irq_status(host, &ireg, &status);
> > +	if (__tmio_mmc_card_detect_irq(host, ireg, status))
> > +		return IRQ_HANDLED;
> > +	if (__tmio_mmc_sdcard_irq(host, ireg, status))
> > +		return IRQ_HANDLED;
> 
> You use the two above functions as "bool," which is also logical. So, I'd 
> also declare them "bool" and return true or false instead of 1 and 0.

Sure, will do.

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

* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
@ 2011-08-16 11:35       ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-16 11:35 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm

On Tue, Aug 16, 2011 at 01:14:01PM +0200, Guennadi Liakhovetski wrote:
> On Tue, 16 Aug 2011, Simon Horman wrote:
> 
> > Provide separate interrupt handlers which may be used by platforms where
> > SDHI has three interrupt sources.
> > 
> > This patch also removes the commented-out handling of CRC and other errors.
> > 
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Magnus Damm <magnus.damm@gmail.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> > 
> > ---
> > 
> > * SDCARD portion tested on AP4/Mackerel
> > * SDIO portion untested
> > 
> > v3
> > * Rebase for updated "mmc: tmio: Cache interrupt masks"
> > * As suggested by Guennadi Liakhovetski
> >   - Do not alter logic to handle more than one interupt at once
> >   - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()
> > 
> > v2
> > * As suggested by Guennadi Liakhovetski
> >   - Combine 3 patches into one
> >   - Reduce the number of __tmio_..._irq() functions
> >   - Rename "...card_access..." functions as "...sdcard..."
> > ---
> >  drivers/mmc/host/tmio_mmc.h     |    3 +
> >  drivers/mmc/host/tmio_mmc_pio.c |  131 ++++++++++++++++++++++++--------------
> >  2 files changed, 86 insertions(+), 48 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> > index 1cf8db5..3020f98 100644
> > --- a/drivers/mmc/host/tmio_mmc.h
> > +++ b/drivers/mmc/host/tmio_mmc.h
> > @@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
> >  void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
> >  void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
> >  irqreturn_t tmio_mmc_irq(int irq, void *devid);
> > +irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
> > +irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
> > +irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
> >  
> >  static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
> >  					 unsigned long *flags)
> > diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> > index f0c7830..37adfb2 100644
> > --- a/drivers/mmc/host/tmio_mmc_pio.c
> > +++ b/drivers/mmc/host/tmio_mmc_pio.c
> > @@ -545,44 +545,20 @@ out:
> >  	spin_unlock(&host->lock);
> >  }
> >  
> > -irqreturn_t tmio_mmc_irq(int irq, void *devid)
> > +static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
> > +				       int *ireg, int *status)
> >  {
> > -	struct tmio_mmc_host *host = devid;
> > -	struct mmc_host *mmc = host->mmc;
> > -	struct tmio_mmc_data *pdata = host->pdata;
> > -	unsigned int ireg, status;
> > -	unsigned int sdio_ireg, sdio_status;
> > -
> > -	pr_debug("MMC IRQ begin\n");
> > -
> > -	status = sd_ctrl_read32(host, CTL_STATUS);
> > -	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
> > -
> > -	sdio_ireg = 0;
> > -	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
> > -		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
> > -		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
> > -				~host->sdio_irq_mask;
> > +	*status = sd_ctrl_read32(host, CTL_STATUS);
> > +	*ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
> >  
> > -		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
> > -
> > -		if (sdio_ireg && !host->sdio_irq_enabled) {
> > -			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
> > -				   sdio_status, host->sdio_irq_mask, sdio_ireg);
> > -			tmio_mmc_enable_sdio_irq(mmc, 0);
> > -			goto out;
> > -		}
> > -
> > -		if (mmc->caps & MMC_CAP_SDIO_IRQ &&
> > -			sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
> > -			mmc_signal_sdio_irq(mmc);
> > -
> > -		if (sdio_ireg)
> > -			goto out;
> > -	}
> > +	pr_debug_status(*status);
> > +	pr_debug_status(*ireg);
> > +}
> >  
> > -	pr_debug_status(status);
> > -	pr_debug_status(ireg);
> > +static int __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
> > +				      int ireg, int status)
> > +{
> > +	struct mmc_host *mmc = host->mmc;
> >  
> >  	/* Card insert / remove attempts */
> >  	if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
> > @@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
> >  		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
> >  		    !work_pending(&mmc->detect.work))
> >  			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
> > -		goto out;
> > +		return 1;
> >  	}
> >  
> > -	/* CRC and other errors */
> > -/*	if (ireg & TMIO_STAT_ERR_IRQ)
> > - *		handled |= tmio_error_irq(host, irq, stat);
> > - */
> > +	return 0;
> > +}
> > +
> > +irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
> > +{
> > +	unsigned int ireg, status;
> > +	struct tmio_mmc_host *host = devid;
> >  
> > +	tmio_mmc_card_irq_status(host, &ireg, &status);
> > +	__tmio_mmc_card_detect_irq(host, ireg, status);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
> > +
> > +static int __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
> > +				 int ireg, int status)
> > +{
> >  	/* Command completion */
> >  	if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
> >  		tmio_mmc_ack_mmc_irqs(host,
> >  			     TMIO_STAT_CMDRESPEND |
> >  			     TMIO_STAT_CMDTIMEOUT);
> >  		tmio_mmc_cmd_irq(host, status);
> > -		goto out;
> > +		return 1;
> >  	}
> >  
> >  	/* Data transfer */
> >  	if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
> >  		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
> >  		tmio_mmc_pio_irq(host);
> > -		goto out;
> > +		return 1;
> >  	}
> >  
> >  	/* Data transfer completion */
> >  	if (ireg & TMIO_STAT_DATAEND) {
> >  		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
> >  		tmio_mmc_data_irq(host);
> > -		goto out;
> > +		return 1;
> >  	}
> >  
> > -	pr_warning("tmio_mmc: Spurious irq, disabling! "
> > -		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
> > -	pr_debug_status(status);
> > -	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
> > +	return 0;
> > +}
> > +
> > +irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
> > +{
> > +	unsigned int ireg, status;
> > +	struct tmio_mmc_host *host = devid;
> > +
> > +	tmio_mmc_card_irq_status(host, &ireg, &status);
> > +	__tmio_mmc_sdcard_irq(host, ireg, status);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
> > +
> > +irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
> > +{
> > +	struct tmio_mmc_host *host = devid;
> > +	struct mmc_host *mmc = host->mmc;
> > +	struct tmio_mmc_data *pdata = host->pdata;
> > +	unsigned int ireg, status;
> > +
> > +	if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
> > +		return IRQ_HANDLED;
> > +
> > +	status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
> > +	ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
> > +
> > +	sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
> > +
> > +	if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
> > +		mmc_signal_sdio_irq(mmc);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +EXPORT_SYMBOL(tmio_mmc_sdio_irq);
> > +
> > +irqreturn_t tmio_mmc_irq(int irq, void *devid)
> > +{
> > +	struct tmio_mmc_host *host = devid;
> > +	unsigned int ireg, status;
> > +
> > +	pr_debug("MMC IRQ begin\n");
> > +
> > +	tmio_mmc_card_irq_status(host, &ireg, &status);
> > +	if (__tmio_mmc_card_detect_irq(host, ireg, status))
> > +		return IRQ_HANDLED;
> > +	if (__tmio_mmc_sdcard_irq(host, ireg, status))
> > +		return IRQ_HANDLED;
> 
> You use the two above functions as "bool," which is also logical. So, I'd 
> also declare them "bool" and return true or false instead of 1 and 0.

Sure, will do.

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

* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
  2011-08-16 10:11   ` Simon Horman
@ 2011-08-16 11:14     ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 36+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-16 11:14 UTC (permalink / raw)
  To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm

On Tue, 16 Aug 2011, Simon Horman wrote:

> Provide separate interrupt handlers which may be used by platforms where
> SDHI has three interrupt sources.
> 
> This patch also removes the commented-out handling of CRC and other errors.
> 
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> ---
> 
> * SDCARD portion tested on AP4/Mackerel
> * SDIO portion untested
> 
> v3
> * Rebase for updated "mmc: tmio: Cache interrupt masks"
> * As suggested by Guennadi Liakhovetski
>   - Do not alter logic to handle more than one interupt at once
>   - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()
> 
> v2
> * As suggested by Guennadi Liakhovetski
>   - Combine 3 patches into one
>   - Reduce the number of __tmio_..._irq() functions
>   - Rename "...card_access..." functions as "...sdcard..."
> ---
>  drivers/mmc/host/tmio_mmc.h     |    3 +
>  drivers/mmc/host/tmio_mmc_pio.c |  131 ++++++++++++++++++++++++--------------
>  2 files changed, 86 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 1cf8db5..3020f98 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
>  void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
>  void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
>  irqreturn_t tmio_mmc_irq(int irq, void *devid);
> +irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
> +irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
> +irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
>  
>  static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
>  					 unsigned long *flags)
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index f0c7830..37adfb2 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -545,44 +545,20 @@ out:
>  	spin_unlock(&host->lock);
>  }
>  
> -irqreturn_t tmio_mmc_irq(int irq, void *devid)
> +static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
> +				       int *ireg, int *status)
>  {
> -	struct tmio_mmc_host *host = devid;
> -	struct mmc_host *mmc = host->mmc;
> -	struct tmio_mmc_data *pdata = host->pdata;
> -	unsigned int ireg, status;
> -	unsigned int sdio_ireg, sdio_status;
> -
> -	pr_debug("MMC IRQ begin\n");
> -
> -	status = sd_ctrl_read32(host, CTL_STATUS);
> -	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
> -
> -	sdio_ireg = 0;
> -	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
> -		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
> -		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
> -				~host->sdio_irq_mask;
> +	*status = sd_ctrl_read32(host, CTL_STATUS);
> +	*ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
>  
> -		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
> -
> -		if (sdio_ireg && !host->sdio_irq_enabled) {
> -			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
> -				   sdio_status, host->sdio_irq_mask, sdio_ireg);
> -			tmio_mmc_enable_sdio_irq(mmc, 0);
> -			goto out;
> -		}
> -
> -		if (mmc->caps & MMC_CAP_SDIO_IRQ &&
> -			sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
> -			mmc_signal_sdio_irq(mmc);
> -
> -		if (sdio_ireg)
> -			goto out;
> -	}
> +	pr_debug_status(*status);
> +	pr_debug_status(*ireg);
> +}
>  
> -	pr_debug_status(status);
> -	pr_debug_status(ireg);
> +static int __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
> +				      int ireg, int status)
> +{
> +	struct mmc_host *mmc = host->mmc;
>  
>  	/* Card insert / remove attempts */
>  	if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
> @@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
>  		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
>  		    !work_pending(&mmc->detect.work))
>  			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
> -		goto out;
> +		return 1;
>  	}
>  
> -	/* CRC and other errors */
> -/*	if (ireg & TMIO_STAT_ERR_IRQ)
> - *		handled |= tmio_error_irq(host, irq, stat);
> - */
> +	return 0;
> +}
> +
> +irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
> +{
> +	unsigned int ireg, status;
> +	struct tmio_mmc_host *host = devid;
>  
> +	tmio_mmc_card_irq_status(host, &ireg, &status);
> +	__tmio_mmc_card_detect_irq(host, ireg, status);
> +
> +	return IRQ_HANDLED;
> +}
> +EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
> +
> +static int __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
> +				 int ireg, int status)
> +{
>  	/* Command completion */
>  	if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
>  		tmio_mmc_ack_mmc_irqs(host,
>  			     TMIO_STAT_CMDRESPEND |
>  			     TMIO_STAT_CMDTIMEOUT);
>  		tmio_mmc_cmd_irq(host, status);
> -		goto out;
> +		return 1;
>  	}
>  
>  	/* Data transfer */
>  	if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
>  		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
>  		tmio_mmc_pio_irq(host);
> -		goto out;
> +		return 1;
>  	}
>  
>  	/* Data transfer completion */
>  	if (ireg & TMIO_STAT_DATAEND) {
>  		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
>  		tmio_mmc_data_irq(host);
> -		goto out;
> +		return 1;
>  	}
>  
> -	pr_warning("tmio_mmc: Spurious irq, disabling! "
> -		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
> -	pr_debug_status(status);
> -	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
> +	return 0;
> +}
> +
> +irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
> +{
> +	unsigned int ireg, status;
> +	struct tmio_mmc_host *host = devid;
> +
> +	tmio_mmc_card_irq_status(host, &ireg, &status);
> +	__tmio_mmc_sdcard_irq(host, ireg, status);
> +
> +	return IRQ_HANDLED;
> +}
> +EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
> +
> +irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
> +{
> +	struct tmio_mmc_host *host = devid;
> +	struct mmc_host *mmc = host->mmc;
> +	struct tmio_mmc_data *pdata = host->pdata;
> +	unsigned int ireg, status;
> +
> +	if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
> +		return IRQ_HANDLED;
> +
> +	status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
> +	ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
> +
> +	sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
> +
> +	if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
> +		mmc_signal_sdio_irq(mmc);
> +
> +	return IRQ_HANDLED;
> +}
> +EXPORT_SYMBOL(tmio_mmc_sdio_irq);
> +
> +irqreturn_t tmio_mmc_irq(int irq, void *devid)
> +{
> +	struct tmio_mmc_host *host = devid;
> +	unsigned int ireg, status;
> +
> +	pr_debug("MMC IRQ begin\n");
> +
> +	tmio_mmc_card_irq_status(host, &ireg, &status);
> +	if (__tmio_mmc_card_detect_irq(host, ireg, status))
> +		return IRQ_HANDLED;
> +	if (__tmio_mmc_sdcard_irq(host, ireg, status))
> +		return IRQ_HANDLED;

You use the two above functions as "bool," which is also logical. So, I'd 
also declare them "bool" and return true or false instead of 1 and 0.

> +
> +	tmio_mmc_sdio_irq(irq, devid);
>  
> -out:
>  	return IRQ_HANDLED;
>  }
>  EXPORT_SYMBOL(tmio_mmc_irq);
> -- 
> 1.7.5.4

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
@ 2011-08-16 11:14     ` Guennadi Liakhovetski
  0 siblings, 0 replies; 36+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-16 11:14 UTC (permalink / raw)
  To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm

On Tue, 16 Aug 2011, Simon Horman wrote:

> Provide separate interrupt handlers which may be used by platforms where
> SDHI has three interrupt sources.
> 
> This patch also removes the commented-out handling of CRC and other errors.
> 
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> ---
> 
> * SDCARD portion tested on AP4/Mackerel
> * SDIO portion untested
> 
> v3
> * Rebase for updated "mmc: tmio: Cache interrupt masks"
> * As suggested by Guennadi Liakhovetski
>   - Do not alter logic to handle more than one interupt at once
>   - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()
> 
> v2
> * As suggested by Guennadi Liakhovetski
>   - Combine 3 patches into one
>   - Reduce the number of __tmio_..._irq() functions
>   - Rename "...card_access..." functions as "...sdcard..."
> ---
>  drivers/mmc/host/tmio_mmc.h     |    3 +
>  drivers/mmc/host/tmio_mmc_pio.c |  131 ++++++++++++++++++++++++--------------
>  2 files changed, 86 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 1cf8db5..3020f98 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
>  void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
>  void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
>  irqreturn_t tmio_mmc_irq(int irq, void *devid);
> +irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
> +irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
> +irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
>  
>  static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
>  					 unsigned long *flags)
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index f0c7830..37adfb2 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -545,44 +545,20 @@ out:
>  	spin_unlock(&host->lock);
>  }
>  
> -irqreturn_t tmio_mmc_irq(int irq, void *devid)
> +static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
> +				       int *ireg, int *status)
>  {
> -	struct tmio_mmc_host *host = devid;
> -	struct mmc_host *mmc = host->mmc;
> -	struct tmio_mmc_data *pdata = host->pdata;
> -	unsigned int ireg, status;
> -	unsigned int sdio_ireg, sdio_status;
> -
> -	pr_debug("MMC IRQ begin\n");
> -
> -	status = sd_ctrl_read32(host, CTL_STATUS);
> -	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
> -
> -	sdio_ireg = 0;
> -	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
> -		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
> -		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
> -				~host->sdio_irq_mask;
> +	*status = sd_ctrl_read32(host, CTL_STATUS);
> +	*ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
>  
> -		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
> -
> -		if (sdio_ireg && !host->sdio_irq_enabled) {
> -			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
> -				   sdio_status, host->sdio_irq_mask, sdio_ireg);
> -			tmio_mmc_enable_sdio_irq(mmc, 0);
> -			goto out;
> -		}
> -
> -		if (mmc->caps & MMC_CAP_SDIO_IRQ &&
> -			sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
> -			mmc_signal_sdio_irq(mmc);
> -
> -		if (sdio_ireg)
> -			goto out;
> -	}
> +	pr_debug_status(*status);
> +	pr_debug_status(*ireg);
> +}
>  
> -	pr_debug_status(status);
> -	pr_debug_status(ireg);
> +static int __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
> +				      int ireg, int status)
> +{
> +	struct mmc_host *mmc = host->mmc;
>  
>  	/* Card insert / remove attempts */
>  	if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
> @@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
>  		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
>  		    !work_pending(&mmc->detect.work))
>  			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
> -		goto out;
> +		return 1;
>  	}
>  
> -	/* CRC and other errors */
> -/*	if (ireg & TMIO_STAT_ERR_IRQ)
> - *		handled |= tmio_error_irq(host, irq, stat);
> - */
> +	return 0;
> +}
> +
> +irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
> +{
> +	unsigned int ireg, status;
> +	struct tmio_mmc_host *host = devid;
>  
> +	tmio_mmc_card_irq_status(host, &ireg, &status);
> +	__tmio_mmc_card_detect_irq(host, ireg, status);
> +
> +	return IRQ_HANDLED;
> +}
> +EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
> +
> +static int __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
> +				 int ireg, int status)
> +{
>  	/* Command completion */
>  	if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
>  		tmio_mmc_ack_mmc_irqs(host,
>  			     TMIO_STAT_CMDRESPEND |
>  			     TMIO_STAT_CMDTIMEOUT);
>  		tmio_mmc_cmd_irq(host, status);
> -		goto out;
> +		return 1;
>  	}
>  
>  	/* Data transfer */
>  	if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
>  		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
>  		tmio_mmc_pio_irq(host);
> -		goto out;
> +		return 1;
>  	}
>  
>  	/* Data transfer completion */
>  	if (ireg & TMIO_STAT_DATAEND) {
>  		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
>  		tmio_mmc_data_irq(host);
> -		goto out;
> +		return 1;
>  	}
>  
> -	pr_warning("tmio_mmc: Spurious irq, disabling! "
> -		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
> -	pr_debug_status(status);
> -	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
> +	return 0;
> +}
> +
> +irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
> +{
> +	unsigned int ireg, status;
> +	struct tmio_mmc_host *host = devid;
> +
> +	tmio_mmc_card_irq_status(host, &ireg, &status);
> +	__tmio_mmc_sdcard_irq(host, ireg, status);
> +
> +	return IRQ_HANDLED;
> +}
> +EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
> +
> +irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
> +{
> +	struct tmio_mmc_host *host = devid;
> +	struct mmc_host *mmc = host->mmc;
> +	struct tmio_mmc_data *pdata = host->pdata;
> +	unsigned int ireg, status;
> +
> +	if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
> +		return IRQ_HANDLED;
> +
> +	status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
> +	ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
> +
> +	sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
> +
> +	if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
> +		mmc_signal_sdio_irq(mmc);
> +
> +	return IRQ_HANDLED;
> +}
> +EXPORT_SYMBOL(tmio_mmc_sdio_irq);
> +
> +irqreturn_t tmio_mmc_irq(int irq, void *devid)
> +{
> +	struct tmio_mmc_host *host = devid;
> +	unsigned int ireg, status;
> +
> +	pr_debug("MMC IRQ begin\n");
> +
> +	tmio_mmc_card_irq_status(host, &ireg, &status);
> +	if (__tmio_mmc_card_detect_irq(host, ireg, status))
> +		return IRQ_HANDLED;
> +	if (__tmio_mmc_sdcard_irq(host, ireg, status))
> +		return IRQ_HANDLED;

You use the two above functions as "bool," which is also logical. So, I'd 
also declare them "bool" and return true or false instead of 1 and 0.

> +
> +	tmio_mmc_sdio_irq(irq, devid);
>  
> -out:
>  	return IRQ_HANDLED;
>  }
>  EXPORT_SYMBOL(tmio_mmc_irq);
> -- 
> 1.7.5.4

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
  2011-08-16 10:11 [PATCH 0/4 v3] mmc: tmio, sdhi: provide multiple irq handlers Simon Horman
@ 2011-08-16 10:11   ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-16 10:11 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Guennadi Liakhovetski, Magnus Damm, Simon Horman

Provide separate interrupt handlers which may be used by platforms where
SDHI has three interrupt sources.

This patch also removes the commented-out handling of CRC and other errors.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested

v3
* Rebase for updated "mmc: tmio: Cache interrupt masks"
* As suggested by Guennadi Liakhovetski
  - Do not alter logic to handle more than one interupt at once
  - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()

v2
* As suggested by Guennadi Liakhovetski
  - Combine 3 patches into one
  - Reduce the number of __tmio_..._irq() functions
  - Rename "...card_access..." functions as "...sdcard..."
---
 drivers/mmc/host/tmio_mmc.h     |    3 +
 drivers/mmc/host/tmio_mmc_pio.c |  131 ++++++++++++++++++++++++--------------
 2 files changed, 86 insertions(+), 48 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 1cf8db5..3020f98 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
 void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 irqreturn_t tmio_mmc_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
 
 static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
 					 unsigned long *flags)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f0c7830..37adfb2 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -545,44 +545,20 @@ out:
 	spin_unlock(&host->lock);
 }
 
-irqreturn_t tmio_mmc_irq(int irq, void *devid)
+static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
+				       int *ireg, int *status)
 {
-	struct tmio_mmc_host *host = devid;
-	struct mmc_host *mmc = host->mmc;
-	struct tmio_mmc_data *pdata = host->pdata;
-	unsigned int ireg, status;
-	unsigned int sdio_ireg, sdio_status;
-
-	pr_debug("MMC IRQ begin\n");
-
-	status = sd_ctrl_read32(host, CTL_STATUS);
-	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
-
-	sdio_ireg = 0;
-	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
-		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
-		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
-				~host->sdio_irq_mask;
+	*status = sd_ctrl_read32(host, CTL_STATUS);
+	*ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
 
-		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
-
-		if (sdio_ireg && !host->sdio_irq_enabled) {
-			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
-				   sdio_status, host->sdio_irq_mask, sdio_ireg);
-			tmio_mmc_enable_sdio_irq(mmc, 0);
-			goto out;
-		}
-
-		if (mmc->caps & MMC_CAP_SDIO_IRQ &&
-			sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
-			mmc_signal_sdio_irq(mmc);
-
-		if (sdio_ireg)
-			goto out;
-	}
+	pr_debug_status(*status);
+	pr_debug_status(*ireg);
+}
 
-	pr_debug_status(status);
-	pr_debug_status(ireg);
+static int __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
+				      int ireg, int status)
+{
+	struct mmc_host *mmc = host->mmc;
 
 	/* Card insert / remove attempts */
 	if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
@@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
 		    !work_pending(&mmc->detect.work))
 			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
-		goto out;
+		return 1;
 	}
 
-	/* CRC and other errors */
-/*	if (ireg & TMIO_STAT_ERR_IRQ)
- *		handled |= tmio_error_irq(host, irq, stat);
- */
+	return 0;
+}
+
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
 
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_card_detect_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
+
+static int __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
+				 int ireg, int status)
+{
 	/* Command completion */
 	if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
 		tmio_mmc_ack_mmc_irqs(host,
 			     TMIO_STAT_CMDRESPEND |
 			     TMIO_STAT_CMDTIMEOUT);
 		tmio_mmc_cmd_irq(host, status);
-		goto out;
+		return 1;
 	}
 
 	/* Data transfer */
 	if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
 		tmio_mmc_pio_irq(host);
-		goto out;
+		return 1;
 	}
 
 	/* Data transfer completion */
 	if (ireg & TMIO_STAT_DATAEND) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
 		tmio_mmc_data_irq(host);
-		goto out;
+		return 1;
 	}
 
-	pr_warning("tmio_mmc: Spurious irq, disabling! "
-		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
-	pr_debug_status(status);
-	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
+	return 0;
+}
+
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_sdcard_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
+
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	struct mmc_host *mmc = host->mmc;
+	struct tmio_mmc_data *pdata = host->pdata;
+	unsigned int ireg, status;
+
+	if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
+		return IRQ_HANDLED;
+
+	status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
+	ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
+
+	sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
+
+	if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
+		mmc_signal_sdio_irq(mmc);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdio_irq);
+
+irqreturn_t tmio_mmc_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	unsigned int ireg, status;
+
+	pr_debug("MMC IRQ begin\n");
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	if (__tmio_mmc_card_detect_irq(host, ireg, status))
+		return IRQ_HANDLED;
+	if (__tmio_mmc_sdcard_irq(host, ireg, status))
+		return IRQ_HANDLED;
+
+	tmio_mmc_sdio_irq(irq, devid);
 
-out:
 	return IRQ_HANDLED;
 }
 EXPORT_SYMBOL(tmio_mmc_irq);
-- 
1.7.5.4


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

* [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
@ 2011-08-16 10:11   ` Simon Horman
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Horman @ 2011-08-16 10:11 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Guennadi Liakhovetski, Magnus Damm, Simon Horman

Provide separate interrupt handlers which may be used by platforms where
SDHI has three interrupt sources.

This patch also removes the commented-out handling of CRC and other errors.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested

v3
* Rebase for updated "mmc: tmio: Cache interrupt masks"
* As suggested by Guennadi Liakhovetski
  - Do not alter logic to handle more than one interupt at once
  - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()

v2
* As suggested by Guennadi Liakhovetski
  - Combine 3 patches into one
  - Reduce the number of __tmio_..._irq() functions
  - Rename "...card_access..." functions as "...sdcard..."
---
 drivers/mmc/host/tmio_mmc.h     |    3 +
 drivers/mmc/host/tmio_mmc_pio.c |  131 ++++++++++++++++++++++++--------------
 2 files changed, 86 insertions(+), 48 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 1cf8db5..3020f98 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
 void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 irqreturn_t tmio_mmc_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
 
 static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
 					 unsigned long *flags)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f0c7830..37adfb2 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -545,44 +545,20 @@ out:
 	spin_unlock(&host->lock);
 }
 
-irqreturn_t tmio_mmc_irq(int irq, void *devid)
+static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
+				       int *ireg, int *status)
 {
-	struct tmio_mmc_host *host = devid;
-	struct mmc_host *mmc = host->mmc;
-	struct tmio_mmc_data *pdata = host->pdata;
-	unsigned int ireg, status;
-	unsigned int sdio_ireg, sdio_status;
-
-	pr_debug("MMC IRQ begin\n");
-
-	status = sd_ctrl_read32(host, CTL_STATUS);
-	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
-
-	sdio_ireg = 0;
-	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
-		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
-		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
-				~host->sdio_irq_mask;
+	*status = sd_ctrl_read32(host, CTL_STATUS);
+	*ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
 
-		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
-
-		if (sdio_ireg && !host->sdio_irq_enabled) {
-			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
-				   sdio_status, host->sdio_irq_mask, sdio_ireg);
-			tmio_mmc_enable_sdio_irq(mmc, 0);
-			goto out;
-		}
-
-		if (mmc->caps & MMC_CAP_SDIO_IRQ &&
-			sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
-			mmc_signal_sdio_irq(mmc);
-
-		if (sdio_ireg)
-			goto out;
-	}
+	pr_debug_status(*status);
+	pr_debug_status(*ireg);
+}
 
-	pr_debug_status(status);
-	pr_debug_status(ireg);
+static int __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
+				      int ireg, int status)
+{
+	struct mmc_host *mmc = host->mmc;
 
 	/* Card insert / remove attempts */
 	if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
@@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
 		    !work_pending(&mmc->detect.work))
 			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
-		goto out;
+		return 1;
 	}
 
-	/* CRC and other errors */
-/*	if (ireg & TMIO_STAT_ERR_IRQ)
- *		handled |= tmio_error_irq(host, irq, stat);
- */
+	return 0;
+}
+
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
 
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_card_detect_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
+
+static int __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
+				 int ireg, int status)
+{
 	/* Command completion */
 	if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
 		tmio_mmc_ack_mmc_irqs(host,
 			     TMIO_STAT_CMDRESPEND |
 			     TMIO_STAT_CMDTIMEOUT);
 		tmio_mmc_cmd_irq(host, status);
-		goto out;
+		return 1;
 	}
 
 	/* Data transfer */
 	if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
 		tmio_mmc_pio_irq(host);
-		goto out;
+		return 1;
 	}
 
 	/* Data transfer completion */
 	if (ireg & TMIO_STAT_DATAEND) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
 		tmio_mmc_data_irq(host);
-		goto out;
+		return 1;
 	}
 
-	pr_warning("tmio_mmc: Spurious irq, disabling! "
-		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
-	pr_debug_status(status);
-	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
+	return 0;
+}
+
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_sdcard_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
+
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	struct mmc_host *mmc = host->mmc;
+	struct tmio_mmc_data *pdata = host->pdata;
+	unsigned int ireg, status;
+
+	if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
+		return IRQ_HANDLED;
+
+	status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
+	ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
+
+	sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
+
+	if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
+		mmc_signal_sdio_irq(mmc);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdio_irq);
+
+irqreturn_t tmio_mmc_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	unsigned int ireg, status;
+
+	pr_debug("MMC IRQ begin\n");
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	if (__tmio_mmc_card_detect_irq(host, ireg, status))
+		return IRQ_HANDLED;
+	if (__tmio_mmc_sdcard_irq(host, ireg, status))
+		return IRQ_HANDLED;
+
+	tmio_mmc_sdio_irq(irq, devid);
 
-out:
 	return IRQ_HANDLED;
 }
 EXPORT_SYMBOL(tmio_mmc_irq);
-- 
1.7.5.4


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

end of thread, other threads:[~2011-08-24  9:47 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-19  7:57 [PATCH 0/4 v7] mmc: tmio, sdhi: provide multiple irq handlers Simon Horman
2011-08-19  7:57 ` Simon Horman
2011-08-19  7:57 ` [PATCH 1/4] mmc: tmio: Cache interrupt masks Simon Horman
2011-08-19  7:57   ` Simon Horman
2011-08-19  7:57 ` [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers Simon Horman
2011-08-19  7:57   ` Simon Horman
2011-08-19  7:57 ` [PATCH 3/4] mmc: sdhi: Allow name IRQs to use specific handlers Simon Horman
2011-08-19  7:57   ` Simon Horman
2011-08-19  7:57 ` [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Name SDHI IRQ sources Simon Horman
2011-08-19  7:57   ` Simon Horman
2011-08-24  5:37 ` [PATCH 0/4 v7] mmc: tmio, sdhi: provide multiple irq handlers Simon Horman
2011-08-24  5:37   ` Simon Horman
2011-08-24  9:47   ` Magnus Damm
2011-08-24  9:47     ` Magnus Damm
  -- strict thread matches above, loose matches on Subject: below --
2011-08-19  1:10 [PATCH 0/4 v6] " Simon Horman
2011-08-19  1:10 ` [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers Simon Horman
2011-08-19  1:10   ` Simon Horman
2011-08-19  3:09   ` Magnus Damm
2011-08-19  3:09     ` Magnus Damm
2011-08-19  3:30     ` Simon Horman
2011-08-19  3:30       ` Simon Horman
2011-08-19  4:27       ` Magnus Damm
2011-08-19  4:27         ` Magnus Damm
2011-08-19  4:59         ` Simon Horman
2011-08-19  4:59           ` Simon Horman
2011-08-19  5:41           ` Magnus Damm
2011-08-19  5:41             ` Magnus Damm
2011-08-17 10:59 [PATCH 0/4 v5] mmc: tmio, sdhi: provide multiple irq handlers Simon Horman
2011-08-17 10:59 ` [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers Simon Horman
2011-08-17 10:59   ` Simon Horman
2011-08-17  0:50 [PATCH 0/4 v4] mmc: tmio, sdhi: provide multiple irq handlers Simon Horman
2011-08-17  0:50 ` [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers Simon Horman
2011-08-17  0:50   ` Simon Horman
2011-08-16 10:11 [PATCH 0/4 v3] mmc: tmio, sdhi: provide multiple irq handlers Simon Horman
2011-08-16 10:11 ` [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers Simon Horman
2011-08-16 10:11   ` Simon Horman
2011-08-16 11:14   ` Guennadi Liakhovetski
2011-08-16 11:14     ` Guennadi Liakhovetski
2011-08-16 11:35     ` Simon Horman
2011-08-16 11:35       ` Simon Horman

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.