[PATCH 0/4 v4] mmc: tmio, sdhi: provide multiple irq handlers

[PATCH 0/4 v4] mmc: tmio, sdhi: provide multiple irq handlers

[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

[PATCH 0/4 v4] mmc: tmio, sdhi: provide multiple irq handlers

[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

[PATCH 1/4] mmc: tmio: Cache interrupt masks

[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

[PATCH 1/4] mmc: tmio: Cache interrupt masks

[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

[PATCH 2/4] mmc: tmio: Provide separate interrupt handlers

[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

[PATCH 2/4] mmc: tmio: Provide separate interrupt handlers

[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

[PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers

[Simon Horman]

Make use of per-source irq handles if the
platform (data) has multiple irq sources.

Also, as suggested by Guennadi Liakhovetski,
add and use defines the index or irqs in platform data.

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

---

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!)
---
 drivers/mmc/host/sh_mobile_sdhi.c  |   60 ++++++++++++++++++++++-------------
 include/linux/mmc/sh_mobile_sdhi.h |    7 ++++
 2 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 774f643..619df30 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -96,7 +96,9 @@ 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;
+	irqreturn_t (*f)(int irq, void *devid);
+	bool multi_irq = false;
 
 	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
 	if (priv = NULL) {
@@ -153,27 +155,33 @@ 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,
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0) {
+		multi_irq = true;
+		ret = request_irq(irq, tmio_mmc_sdio_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_sdio;
 	}
+
+	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0) {
+		multi_irq = true;
+		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
+				  dev_name(&pdev->dev), host);
+		if (ret)
+			goto eirq_sdcard;
+	} else if (multi_irq)
+		goto eirq_sdcard;
+
+	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+	if (irq < 0)
+		goto eirq_card_detect;
+	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
+	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
+	if (ret)
+		goto eirq_card_detect;
+
 	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 +189,15 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 
 	return ret;
 
-eirq:
+eirq_card_detect:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdcard:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdio:
 	tmio_mmc_host_remove(host);
 eprobe:
 	clk_disable(priv->clk);
@@ -203,7 +219,7 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 
 	tmio_mmc_host_remove(host);
 
-	for (i = 0; i < 3; i++) {
+	for (i = 0; i < SH_MOBILE_SDHI_IRQ_MAX; i++) {
 		irq = platform_get_irq(pdev, i);
 		if (irq >= 0)
 			free_irq(irq, host);
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index bd50b36..80d3629 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -3,6 +3,13 @@
 
 #include <linux/types.h>
 
+enum {
+	SH_MOBILE_SDHI_IRQ_CARD_DETECT = 0,
+	SH_MOBILE_SDHI_IRQ_SDCARD,
+	SH_MOBILE_SDHI_IRQ_SDIO,
+	SH_MOBILE_SDHI_IRQ_MAX
+};
+
 struct platform_device;
 struct tmio_mmc_data;
 
-- 
1.7.5.4

[PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers

[Simon Horman]

Make use of per-source irq handles if the
platform (data) has multiple irq sources.

Also, as suggested by Guennadi Liakhovetski,
add and use defines the index or irqs in platform data.

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

---

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!)
---
 drivers/mmc/host/sh_mobile_sdhi.c  |   60 ++++++++++++++++++++++-------------
 include/linux/mmc/sh_mobile_sdhi.h |    7 ++++
 2 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 774f643..619df30 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -96,7 +96,9 @@ 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;
+	irqreturn_t (*f)(int irq, void *devid);
+	bool multi_irq = false;
 
 	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
 	if (priv == NULL) {
@@ -153,27 +155,33 @@ 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,
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0) {
+		multi_irq = true;
+		ret = request_irq(irq, tmio_mmc_sdio_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_sdio;
 	}
+
+	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0) {
+		multi_irq = true;
+		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
+				  dev_name(&pdev->dev), host);
+		if (ret)
+			goto eirq_sdcard;
+	} else if (multi_irq)
+		goto eirq_sdcard;
+
+	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+	if (irq < 0)
+		goto eirq_card_detect;
+	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
+	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
+	if (ret)
+		goto eirq_card_detect;
+
 	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 +189,15 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 
 	return ret;
 
-eirq:
+eirq_card_detect:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdcard:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdio:
 	tmio_mmc_host_remove(host);
 eprobe:
 	clk_disable(priv->clk);
@@ -203,7 +219,7 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 
 	tmio_mmc_host_remove(host);
 
-	for (i = 0; i < 3; i++) {
+	for (i = 0; i < SH_MOBILE_SDHI_IRQ_MAX; i++) {
 		irq = platform_get_irq(pdev, i);
 		if (irq >= 0)
 			free_irq(irq, host);
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index bd50b36..80d3629 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -3,6 +3,13 @@
 
 #include <linux/types.h>
 
+enum {
+	SH_MOBILE_SDHI_IRQ_CARD_DETECT = 0,
+	SH_MOBILE_SDHI_IRQ_SDCARD,
+	SH_MOBILE_SDHI_IRQ_SDIO,
+	SH_MOBILE_SDHI_IRQ_MAX
+};
+
 struct platform_device;
 struct tmio_mmc_data;
 
-- 
1.7.5.4

[PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Simon Horman]

This is intended to make it easier to correctly order IRQs.

As suggested by Guennadi Liakhovetski.

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>

---

Depends on "mmc: sdhi: Make use of per-source irq handlers"

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   |   12 ++++++------
 arch/arm/mach-shmobile/board-mackerel.c |   18 +++++++++---------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index ce5c251..0d543bb 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xee1000ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(83),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(84),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= gic_spi(85),
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -395,15 +395,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xee1200ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(87),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(88),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + 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..f9d3a93 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1022,15 +1022,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xe68500ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e00) /* SDHI0_SDHI0I0 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0e20) /* SDHI0_SDHI0I1 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0e40) /* SDHI0_SDHI0I2 */,
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1065,15 +1065,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xe68600ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e80), /* SDHI1_SDHI1I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0ea0), /* SDHI1_SDHI1I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0ec0), /* SDHI1_SDHI1I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1116,15 +1116,15 @@ static struct resource sdhi2_resources[] = {
 		.end	= 0xe68700ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x1200), /* SDHI2_SDHI2I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x1220), /* SDHI2_SDHI2I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x1240), /* SDHI2_SDHI2I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
-- 
1.7.5.4

[PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Simon Horman]

This is intended to make it easier to correctly order IRQs.

As suggested by Guennadi Liakhovetski.

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>

---

Depends on "mmc: sdhi: Make use of per-source irq handlers"

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   |   12 ++++++------
 arch/arm/mach-shmobile/board-mackerel.c |   18 +++++++++---------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index ce5c251..0d543bb 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xee1000ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(83),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(84),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= gic_spi(85),
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -395,15 +395,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xee1200ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(87),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(88),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + 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..f9d3a93 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1022,15 +1022,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xe68500ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e00) /* SDHI0_SDHI0I0 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0e20) /* SDHI0_SDHI0I1 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0e40) /* SDHI0_SDHI0I2 */,
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1065,15 +1065,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xe68600ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e80), /* SDHI1_SDHI1I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0ea0), /* SDHI1_SDHI1I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0ec0), /* SDHI1_SDHI1I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1116,15 +1116,15 @@ static struct resource sdhi2_resources[] = {
 		.end	= 0xe68700ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x1200), /* SDHI2_SDHI2I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x1220), /* SDHI2_SDHI2I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x1240), /* SDHI2_SDHI2I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
-- 
1.7.5.4

Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers

[Guennadi Liakhovetski]

On Wed, 17 Aug 2011, Simon Horman wrote:

> Make use of per-source irq handles if the
> platform (data) has multiple irq sources.
> 
> Also, as suggested by Guennadi Liakhovetski,
> add and use defines the index or irqs in platform data.
> 
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> ---
> 
> 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!)
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c  |   60 ++++++++++++++++++++++-------------
>  include/linux/mmc/sh_mobile_sdhi.h |    7 ++++
>  2 files changed, 45 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index 774f643..619df30 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -96,7 +96,9 @@ 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;
> +	irqreturn_t (*f)(int irq, void *devid);
> +	bool multi_irq = false;
>  
>  	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
>  	if (priv = NULL) {
> @@ -153,27 +155,33 @@ 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,
> +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
> +	if (irq >= 0) {
> +		multi_irq = true;
> +		ret = request_irq(irq, tmio_mmc_sdio_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_sdio;
>  	}
> +
> +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> +	if (irq >= 0) {
> +		multi_irq = true;
> +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> +				  dev_name(&pdev->dev), host);
> +		if (ret)
> +			goto eirq_sdcard;
> +	} else if (multi_irq)
> +		goto eirq_sdcard;
> +
> +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> +	if (irq < 0)
> +		goto eirq_card_detect;
> +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> +	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
> +	if (ret)
> +		goto eirq_card_detect;
> +

I still don't see why a multi-IRQ configuration without a card-detect IRQ 
like

static struct resource sdhi_resources[] = {
	[0] = {
		.name	= "SDHI2",
		...,
	},
	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
		.start	= ...,
		.flags	= IORESOURCE_IRQ,
	},
	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
		.start	= ...,
		.flags	= IORESOURCE_IRQ,
	},
};

should be invalid. Especially since we actually want to avoid using the 
controller card-detect IRQ for power efficiency and use a GPIO instead.

Thanks
Guennadi

>  	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 +189,15 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
>  
>  	return ret;
>  
> -eirq:
> +eirq_card_detect:
> +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> +	if (irq >= 0)
> +		free_irq(irq, host);
> +eirq_sdcard:
> +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
> +	if (irq >= 0)
> +		free_irq(irq, host);
> +eirq_sdio:
>  	tmio_mmc_host_remove(host);
>  eprobe:
>  	clk_disable(priv->clk);
> @@ -203,7 +219,7 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
>  
>  	tmio_mmc_host_remove(host);
>  
> -	for (i = 0; i < 3; i++) {
> +	for (i = 0; i < SH_MOBILE_SDHI_IRQ_MAX; i++) {
>  		irq = platform_get_irq(pdev, i);
>  		if (irq >= 0)
>  			free_irq(irq, host);
> diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
> index bd50b36..80d3629 100644
> --- a/include/linux/mmc/sh_mobile_sdhi.h
> +++ b/include/linux/mmc/sh_mobile_sdhi.h
> @@ -3,6 +3,13 @@
>  
>  #include <linux/types.h>
>  
> +enum {
> +	SH_MOBILE_SDHI_IRQ_CARD_DETECT = 0,
> +	SH_MOBILE_SDHI_IRQ_SDCARD,
> +	SH_MOBILE_SDHI_IRQ_SDIO,
> +	SH_MOBILE_SDHI_IRQ_MAX
> +};
> +
>  struct platform_device;
>  struct tmio_mmc_data;
>  
> -- 
> 1.7.5.4
> 

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

Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers

[Guennadi Liakhovetski]

On Wed, 17 Aug 2011, Simon Horman wrote:

> Make use of per-source irq handles if the
> platform (data) has multiple irq sources.
> 
> Also, as suggested by Guennadi Liakhovetski,
> add and use defines the index or irqs in platform data.
> 
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> ---
> 
> 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!)
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c  |   60 ++++++++++++++++++++++-------------
>  include/linux/mmc/sh_mobile_sdhi.h |    7 ++++
>  2 files changed, 45 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index 774f643..619df30 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -96,7 +96,9 @@ 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;
> +	irqreturn_t (*f)(int irq, void *devid);
> +	bool multi_irq = false;
>  
>  	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
>  	if (priv == NULL) {
> @@ -153,27 +155,33 @@ 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,
> +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
> +	if (irq >= 0) {
> +		multi_irq = true;
> +		ret = request_irq(irq, tmio_mmc_sdio_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_sdio;
>  	}
> +
> +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> +	if (irq >= 0) {
> +		multi_irq = true;
> +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> +				  dev_name(&pdev->dev), host);
> +		if (ret)
> +			goto eirq_sdcard;
> +	} else if (multi_irq)
> +		goto eirq_sdcard;
> +
> +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> +	if (irq < 0)
> +		goto eirq_card_detect;
> +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> +	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
> +	if (ret)
> +		goto eirq_card_detect;
> +

I still don't see why a multi-IRQ configuration without a card-detect IRQ 
like

static struct resource sdhi_resources[] = {
	[0] = {
		.name	= "SDHI2",
		...,
	},
	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
		.start	= ...,
		.flags	= IORESOURCE_IRQ,
	},
	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
		.start	= ...,
		.flags	= IORESOURCE_IRQ,
	},
};

should be invalid. Especially since we actually want to avoid using the 
controller card-detect IRQ for power efficiency and use a GPIO instead.

Thanks
Guennadi

>  	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 +189,15 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
>  
>  	return ret;
>  
> -eirq:
> +eirq_card_detect:
> +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> +	if (irq >= 0)
> +		free_irq(irq, host);
> +eirq_sdcard:
> +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
> +	if (irq >= 0)
> +		free_irq(irq, host);
> +eirq_sdio:
>  	tmio_mmc_host_remove(host);
>  eprobe:
>  	clk_disable(priv->clk);
> @@ -203,7 +219,7 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
>  
>  	tmio_mmc_host_remove(host);
>  
> -	for (i = 0; i < 3; i++) {
> +	for (i = 0; i < SH_MOBILE_SDHI_IRQ_MAX; i++) {
>  		irq = platform_get_irq(pdev, i);
>  		if (irq >= 0)
>  			free_irq(irq, host);
> diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
> index bd50b36..80d3629 100644
> --- a/include/linux/mmc/sh_mobile_sdhi.h
> +++ b/include/linux/mmc/sh_mobile_sdhi.h
> @@ -3,6 +3,13 @@
>  
>  #include <linux/types.h>
>  
> +enum {
> +	SH_MOBILE_SDHI_IRQ_CARD_DETECT = 0,
> +	SH_MOBILE_SDHI_IRQ_SDCARD,
> +	SH_MOBILE_SDHI_IRQ_SDIO,
> +	SH_MOBILE_SDHI_IRQ_MAX
> +};
> +
>  struct platform_device;
>  struct tmio_mmc_data;
>  
> -- 
> 1.7.5.4
> 

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

Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers

[Simon Horman]

On Wed, Aug 17, 2011 at 10:20:24AM +0200, Guennadi Liakhovetski wrote:
> On Wed, 17 Aug 2011, Simon Horman wrote:

[snip ]

> > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > +	if (irq >= 0) {
> > +		multi_irq = true;
> > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > +				  dev_name(&pdev->dev), host);
> > +		if (ret)
> > +			goto eirq_sdcard;
> > +	} else if (multi_irq)
> > +		goto eirq_sdcard;
> > +
> > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > +	if (irq < 0)
> > +		goto eirq_card_detect;
> > +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> > +	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
> > +	if (ret)
> > +		goto eirq_card_detect;
> > +
> 
> I still don't see why a multi-IRQ configuration without a card-detect IRQ 
> like
> 
> static struct resource sdhi_resources[] = {
> 	[0] = {
> 		.name	= "SDHI2",
> 		...,
> 	},
> 	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> 		.start	= ...,
> 		.flags	= IORESOURCE_IRQ,
> 	},
> 	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
> 		.start	= ...,
> 		.flags	= IORESOURCE_IRQ,
> 	},
> };
> 
> should be invalid. Especially since we actually want to avoid using the 
> controller card-detect IRQ for power efficiency and use a GPIO instead.

Ok, in this case you would like SH_MOBILE_SDHI_IRQ_SDCARD to use
tmio_mmc_sdcard_irq() ?

Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers

[Simon Horman]

On Wed, Aug 17, 2011 at 10:20:24AM +0200, Guennadi Liakhovetski wrote:
> On Wed, 17 Aug 2011, Simon Horman wrote:

[snip ]

> > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > +	if (irq >= 0) {
> > +		multi_irq = true;
> > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > +				  dev_name(&pdev->dev), host);
> > +		if (ret)
> > +			goto eirq_sdcard;
> > +	} else if (multi_irq)
> > +		goto eirq_sdcard;
> > +
> > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > +	if (irq < 0)
> > +		goto eirq_card_detect;
> > +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> > +	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
> > +	if (ret)
> > +		goto eirq_card_detect;
> > +
> 
> I still don't see why a multi-IRQ configuration without a card-detect IRQ 
> like
> 
> static struct resource sdhi_resources[] = {
> 	[0] = {
> 		.name	= "SDHI2",
> 		...,
> 	},
> 	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> 		.start	= ...,
> 		.flags	= IORESOURCE_IRQ,
> 	},
> 	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
> 		.start	= ...,
> 		.flags	= IORESOURCE_IRQ,
> 	},
> };
> 
> should be invalid. Especially since we actually want to avoid using the 
> controller card-detect IRQ for power efficiency and use a GPIO instead.

Ok, in this case you would like SH_MOBILE_SDHI_IRQ_SDCARD to use
tmio_mmc_sdcard_irq() ?

Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers

[Guennadi Liakhovetski]

On Wed, 17 Aug 2011, Simon Horman wrote:

> On Wed, Aug 17, 2011 at 10:20:24AM +0200, Guennadi Liakhovetski wrote:
> > On Wed, 17 Aug 2011, Simon Horman wrote:
> 
> [snip ]
> 
> > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > > +	if (irq >= 0) {
> > > +		multi_irq = true;
> > > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > > +				  dev_name(&pdev->dev), host);
> > > +		if (ret)
> > > +			goto eirq_sdcard;
> > > +	} else if (multi_irq)
> > > +		goto eirq_sdcard;
> > > +
> > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > > +	if (irq < 0)
> > > +		goto eirq_card_detect;
> > > +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> > > +	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
> > > +	if (ret)
> > > +		goto eirq_card_detect;
> > > +
> > 
> > I still don't see why a multi-IRQ configuration without a card-detect IRQ 
> > like
> > 
> > static struct resource sdhi_resources[] = {
> > 	[0] = {
> > 		.name	= "SDHI2",
> > 		...,
> > 	},
> > 	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> > 		.start	= ...,
> > 		.flags	= IORESOURCE_IRQ,
> > 	},
> > 	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
> > 		.start	= ...,
> > 		.flags	= IORESOURCE_IRQ,
> > 	},
> > };
> > 
> > should be invalid. Especially since we actually want to avoid using the 
> > controller card-detect IRQ for power efficiency and use a GPIO instead.
> 
> Ok, in this case you would like SH_MOBILE_SDHI_IRQ_SDCARD to use
> tmio_mmc_sdcard_irq() ?

This is a good question. I think your erroring out is wrong, but I'm not 
sure what is best here. Using sdcard and sdio ISR in this case seems most 
logical to me. But I don't know if we ever can get hardware, where we 
indeed only have two SDHI interrupts - one for SDIO and one for SDCARD and 
CARD_DETECT. I'm not aware of such hardware so far, so, yes, I'd go with 
SDIO and SDCARD ISRs for now. In any case, this is SDHI internal decision, 
so, we can change it at any time, if we need to.

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

Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers

[Guennadi Liakhovetski]

On Wed, 17 Aug 2011, Simon Horman wrote:

> On Wed, Aug 17, 2011 at 10:20:24AM +0200, Guennadi Liakhovetski wrote:
> > On Wed, 17 Aug 2011, Simon Horman wrote:
> 
> [snip ]
> 
> > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > > +	if (irq >= 0) {
> > > +		multi_irq = true;
> > > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > > +				  dev_name(&pdev->dev), host);
> > > +		if (ret)
> > > +			goto eirq_sdcard;
> > > +	} else if (multi_irq)
> > > +		goto eirq_sdcard;
> > > +
> > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > > +	if (irq < 0)
> > > +		goto eirq_card_detect;
> > > +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> > > +	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
> > > +	if (ret)
> > > +		goto eirq_card_detect;
> > > +
> > 
> > I still don't see why a multi-IRQ configuration without a card-detect IRQ 
> > like
> > 
> > static struct resource sdhi_resources[] = {
> > 	[0] = {
> > 		.name	= "SDHI2",
> > 		...,
> > 	},
> > 	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> > 		.start	= ...,
> > 		.flags	= IORESOURCE_IRQ,
> > 	},
> > 	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
> > 		.start	= ...,
> > 		.flags	= IORESOURCE_IRQ,
> > 	},
> > };
> > 
> > should be invalid. Especially since we actually want to avoid using the 
> > controller card-detect IRQ for power efficiency and use a GPIO instead.
> 
> Ok, in this case you would like SH_MOBILE_SDHI_IRQ_SDCARD to use
> tmio_mmc_sdcard_irq() ?

This is a good question. I think your erroring out is wrong, but I'm not 
sure what is best here. Using sdcard and sdio ISR in this case seems most 
logical to me. But I don't know if we ever can get hardware, where we 
indeed only have two SDHI interrupts - one for SDIO and one for SDCARD and 
CARD_DETECT. I'm not aware of such hardware so far, so, yes, I'd go with 
SDIO and SDCARD ISRs for now. In any case, this is SDHI internal decision, 
so, we can change it at any time, if we need to.

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

Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers

[Simon Horman]

On Wed, Aug 17, 2011 at 12:06:56PM +0200, Guennadi Liakhovetski wrote:
> On Wed, 17 Aug 2011, Simon Horman wrote:
> 
> > On Wed, Aug 17, 2011 at 10:20:24AM +0200, Guennadi Liakhovetski wrote:
> > > On Wed, 17 Aug 2011, Simon Horman wrote:
> > 
> > [snip ]
> > 
> > > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > > > +	if (irq >= 0) {
> > > > +		multi_irq = true;
> > > > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > > > +				  dev_name(&pdev->dev), host);
> > > > +		if (ret)
> > > > +			goto eirq_sdcard;
> > > > +	} else if (multi_irq)
> > > > +		goto eirq_sdcard;
> > > > +
> > > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > > > +	if (irq < 0)
> > > > +		goto eirq_card_detect;
> > > > +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> > > > +	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
> > > > +	if (ret)
> > > > +		goto eirq_card_detect;
> > > > +
> > > 
> > > I still don't see why a multi-IRQ configuration without a card-detect IRQ 
> > > like
> > > 
> > > static struct resource sdhi_resources[] = {
> > > 	[0] = {
> > > 		.name	= "SDHI2",
> > > 		...,
> > > 	},
> > > 	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> > > 		.start	= ...,
> > > 		.flags	= IORESOURCE_IRQ,
> > > 	},
> > > 	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
> > > 		.start	= ...,
> > > 		.flags	= IORESOURCE_IRQ,
> > > 	},
> > > };
> > > 
> > > should be invalid. Especially since we actually want to avoid using the 
> > > controller card-detect IRQ for power efficiency and use a GPIO instead.
> > 
> > Ok, in this case you would like SH_MOBILE_SDHI_IRQ_SDCARD to use
> > tmio_mmc_sdcard_irq() ?
> 
> This is a good question. I think your erroring out is wrong, but I'm not 
> sure what is best here. Using sdcard and sdio ISR in this case seems most 
> logical to me. But I don't know if we ever can get hardware, where we 
> indeed only have two SDHI interrupts - one for SDIO and one for SDCARD and 
> CARD_DETECT. I'm not aware of such hardware so far, so, yes, I'd go with 
> SDIO and SDCARD ISRs for now. In any case, this is SDHI internal decision, 
> so, we can change it at any time, if we need to.

Agreed.

Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers

[Simon Horman]

On Wed, Aug 17, 2011 at 12:06:56PM +0200, Guennadi Liakhovetski wrote:
> On Wed, 17 Aug 2011, Simon Horman wrote:
> 
> > On Wed, Aug 17, 2011 at 10:20:24AM +0200, Guennadi Liakhovetski wrote:
> > > On Wed, 17 Aug 2011, Simon Horman wrote:
> > 
> > [snip ]
> > 
> > > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > > > +	if (irq >= 0) {
> > > > +		multi_irq = true;
> > > > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > > > +				  dev_name(&pdev->dev), host);
> > > > +		if (ret)
> > > > +			goto eirq_sdcard;
> > > > +	} else if (multi_irq)
> > > > +		goto eirq_sdcard;
> > > > +
> > > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > > > +	if (irq < 0)
> > > > +		goto eirq_card_detect;
> > > > +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> > > > +	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
> > > > +	if (ret)
> > > > +		goto eirq_card_detect;
> > > > +
> > > 
> > > I still don't see why a multi-IRQ configuration without a card-detect IRQ 
> > > like
> > > 
> > > static struct resource sdhi_resources[] = {
> > > 	[0] = {
> > > 		.name	= "SDHI2",
> > > 		...,
> > > 	},
> > > 	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> > > 		.start	= ...,
> > > 		.flags	= IORESOURCE_IRQ,
> > > 	},
> > > 	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
> > > 		.start	= ...,
> > > 		.flags	= IORESOURCE_IRQ,
> > > 	},
> > > };
> > > 
> > > should be invalid. Especially since we actually want to avoid using the 
> > > controller card-detect IRQ for power efficiency and use a GPIO instead.
> > 
> > Ok, in this case you would like SH_MOBILE_SDHI_IRQ_SDCARD to use
> > tmio_mmc_sdcard_irq() ?
> 
> This is a good question. I think your erroring out is wrong, but I'm not 
> sure what is best here. Using sdcard and sdio ISR in this case seems most 
> logical to me. But I don't know if we ever can get hardware, where we 
> indeed only have two SDHI interrupts - one for SDIO and one for SDCARD and 
> CARD_DETECT. I'm not aware of such hardware so far, so, yes, I'd go with 
> SDIO and SDCARD ISRs for now. In any case, this is SDHI internal decision, 
> so, we can change it at any time, if we need to.

Agreed.

Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Simon Horman]

On Fri, Aug 19, 2011 at 09:52:17AM +0200, Guennadi Liakhovetski wrote:
> On Fri, 19 Aug 2011, Simon Horman wrote:
> 
> > On Fri, Aug 19, 2011 at 03:51:49PM +0900, Magnus Damm wrote:
> > > On Fri, Aug 19, 2011 at 3:39 PM, Simon Horman <horms@verge.net.au> wrote:
> 
> [snip]
> 
> > > > As we are already on the slippery slope of allowing combinations
> > > > other than 1 (legacy) or 3 (specific) IRQ sources I plan to implement
> > > > a variant of your flag idea. The variation being to use names instead
> > > > because a) that allows the use of platform_get_irq_byname() and b)
> > > > the flags bits seem to be full and not driver-specific.
> > > 
> > > Great, platform_get_irq_byname() seems like a perfect match.
> > > 
> > > May I suggest "hotplug", "data" and "sdio" as names? I don't care very
> > > much about names except keeping them short and precise to prevent
> > > errors that can only be caught during runtime.
> > 
> > Earlier on in the life of this series Guennadi suggested
> > the names "card_detect", "sdcard" and "sdio". While I am
> > not particularly attached to those names the do seem
> > reasonable and are already used consistently by this series.
> > So I would prefer to use those names.
> 
> Just one more thing I forgot to mention in the previous mail: using names 
> also makes the transition simple: first patch all platforms with names (at 
> least those with multiple IRQs, if your legacy fallback implementation 
> will accept unnamed IRQs), and then patch sh_mobile_sdhi.c

Yes, I agree that is a nice feature of using names.

Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Guennadi Liakhovetski]

On Fri, 19 Aug 2011, Simon Horman wrote:

> On Fri, Aug 19, 2011 at 03:51:49PM +0900, Magnus Damm wrote:
> > On Fri, Aug 19, 2011 at 3:39 PM, Simon Horman <horms@verge.net.au> wrote:

[snip]

> > > As we are already on the slippery slope of allowing combinations
> > > other than 1 (legacy) or 3 (specific) IRQ sources I plan to implement
> > > a variant of your flag idea. The variation being to use names instead
> > > because a) that allows the use of platform_get_irq_byname() and b)
> > > the flags bits seem to be full and not driver-specific.
> > 
> > Great, platform_get_irq_byname() seems like a perfect match.
> > 
> > May I suggest "hotplug", "data" and "sdio" as names? I don't care very
> > much about names except keeping them short and precise to prevent
> > errors that can only be caught during runtime.
> 
> Earlier on in the life of this series Guennadi suggested
> the names "card_detect", "sdcard" and "sdio". While I am
> not particularly attached to those names the do seem
> reasonable and are already used consistently by this series.
> So I would prefer to use those names.

Just one more thing I forgot to mention in the previous mail: using names 
also makes the transition simple: first patch all platforms with names (at 
least those with multiple IRQs, if your legacy fallback implementation 
will accept unnamed IRQs), and then patch sh_mobile_sdhi.c

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

Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Guennadi Liakhovetski]

On Fri, 19 Aug 2011, Magnus Damm wrote:

> On Fri, Aug 19, 2011 at 3:39 PM, Simon Horman <horms@verge.net.au> wrote:

[snip]

> > As we are already on the slippery slope of allowing combinations
> > other than 1 (legacy) or 3 (specific) IRQ sources I plan to implement
> > a variant of your flag idea. The variation being to use names instead
> > because a) that allows the use of platform_get_irq_byname() and b)
> > the flags bits seem to be full and not driver-specific.
> 
> Great, platform_get_irq_byname() seems like a perfect match.

Indeed, good idea! This API is (unfortunately) so rarely used, that it was  
pretty much completely absent from my "active dictionary."

> May I suggest "hotplug", "data" and "sdio" as names? I don't care very
> much about names except keeping them short and precise to prevent
> errors that can only be caught during runtime.

I don't care about names all that much either, but as Simon suggested in 
his other mail, consistency is good. Besides, yes, I thought about "data"
as a name for the main SDHI IRQ, but I decided it didn't reflect its 
function very well. Firstly, it is not only data, secondly, SDIO IRQs can 
also announce data availability, I think. The even more precise 
description of these two IRQs would be, perhaps, "synchronous" and 
"asynchronous," but that's probably not very intuitivy.

So, I'll accept any names, that you two agree upon:-)

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

Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Simon Horman]

On Fri, Aug 19, 2011 at 03:51:49PM +0900, Magnus Damm wrote:
> On Fri, Aug 19, 2011 at 3:39 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Fri, Aug 19, 2011 at 02:20:11PM +0900, Simon Horman wrote:
> >> On Fri, Aug 19, 2011 at 01:16:09PM +0900, Magnus Damm wrote:
> >> > On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
> >> > > This is intended to make it easier to correctly order IRQs.
> >> > >
> >> > > As suggested by Guennadi Liakhovetski.
> >> >
> >> > > --- a/arch/arm/mach-shmobile/board-ag5evm.c
> >> > > +++ b/arch/arm/mach-shmobile/board-ag5evm.c
> >> > > @@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
> >> > >                .end    = 0xee1000ff,
> >> > >                .flags  = IORESOURCE_MEM,
> >> > >        },
> >> > > -       [1] = {
> >> > > +       [1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
> >> > >                .start  = gic_spi(83),
> >> > >                .flags  = IORESOURCE_IRQ,
> >> > >        },
> >> > > -       [2] = {
> >> > > +       [1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> >> > >                .start  = gic_spi(84),
> >> > >                .flags  = IORESOURCE_IRQ,
> >> > >        },
> >> > > -       [3] = {
> >> > > +       [1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
> >> > >                .start  = gic_spi(85),
> >> > >                .flags  = IORESOURCE_IRQ,
> >> > >        },
> >> >
> >> > Hm...
> >> >
> >> > So I know you guys have been discussing this back and forth, so I'm
> >> > not sure if jumping in to this thread makes things any better. But
> >> > anyhow, here are my opinions. Feel free to ignore them. =)
> >> >
> >> > First of all, having some kind of association with each IRQ is a good
> >> > thing. I am however not convinced that using the index number of the
> >> > platform device resource irq is the best option. Consider the case
> >> > when someone modifies the SDHI resource in the code above to only
> >> > include this interrupt:
> >> >
> >> >         [1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> >> >                 .start  = gic_spi(84),
> >> >                 .flags  = IORESOURCE_IRQ,
> >> >         },
> >> >
> >> > >From my point of view, the common sense for this would be that only
> >> > the SDCARD interrupt would be enabled and the rest would be disabled
> >> > since they are unspecified. However, with the current code the
> >> > behavior would be something else, and since the index number of SDCARD
> >> > is not matching it will be detected as CARD_DETECT.
> >> >
> >> > So isn't it really ugly to depend on the number of IRQs when they are
> >> > supposed to be used as an index? I've been toying around with this
> >> > driver for a few years now, and when I have a hard time creating
> >> > correct platform data then it's _probably_ a sign that there must be
> >> > better ways to implement this.
> >> >
> >> > I would propose just adding interrupts in struct resource [] as usual,
> >> > and then have thee separate flags in the platform data for each
> >> > interrupt type. If the number of IRQ bits set in the platform data
> >> > flags doesn't match the number of interrupt resources then return
> >> > error. If they match then simply go through each flag set in the
> >> > platform data flags and assign next available interrupt resource. And
> >> > if no flags are set then go for the combined interrupt handler for all
> >> > available interrupt resources.
> >> >
> >> > That's what I would do at least. Any other ideas? Perhaps just keep an
> >> > array of interrupt numbers in the platform data as the sh-sci driver
> >> > does and use the fixed indexes there if non-zero?
> >>
> >> Hi Magnus,
> >>
> >> unfortunately during the course of the review of this series a number
> >> of changes have crept in which I regard as being tangential to the
> >> goal of the series - which is to introduce broken-out handlers (I have
> >> already introduce support for broken-out IRQ sources).
> >>
> >> One area where such changes have occurred is in the subtle altering of the
> >> list of IRQ sources that it is valid to supply (again, support for
> >> broken-out IRQ sources is not introduced by this series).
> >>
> >> With regards to your comment and example above. I believe that your
> >> understanding of my code is incorrect. The configuration you suggest will
> >> be rejected because CARD_DETECT is not supplied, not because of an index
> >> miss-match. It could be made acceptable within the current framework by
> >> simply loosening up the logic a little (specifically to allow CARD_DETECT
> >> to be omitted even if only one other IRQ source is supplied).  Incidently,
> >> I think that would make sense but Guennadi specifically asked for that
> >> combination to be regarded as invalid.
> >
> > [snip]
> >
> > After some discussion off-line I now realise that there is a problem
> > with my implementation. Specifically that it assumes that
> > platform_get_irq() takes into account the index of resource entries -
> > it does not.
> 
> Right, this is a thing that only bites you once. =)
> 
> > As we are already on the slippery slope of allowing combinations
> > other than 1 (legacy) or 3 (specific) IRQ sources I plan to implement
> > a variant of your flag idea. The variation being to use names instead
> > because a) that allows the use of platform_get_irq_byname() and b)
> > the flags bits seem to be full and not driver-specific.
> 
> Great, platform_get_irq_byname() seems like a perfect match.
> 
> May I suggest "hotplug", "data" and "sdio" as names? I don't care very
> much about names except keeping them short and precise to prevent
> errors that can only be caught during runtime.

Earlier on in the life of this series Guennadi suggested
the names "card_detect", "sdcard" and "sdio". While I am
not particularly attached to those names the do seem
reasonable and are already used consistently by this series.
So I would prefer to use those names.

> > And as are already on the slippery slope of allowing combinations
> > if IRQ sources beyond 1 and 3 I intend to implement logic to allow
> > any number of unique specific IRQ sources to be handled. In practice
> > hardware for some of these combinations is unlikely to exist. But that
> > doesn't seem to be a particularly good reason to add extra logic
> > to disallow them in the driver - its up to platform data to specify
> > something valid.
> 
> Yes, I totally agree. Thanks a lot for your help!
> 
> / magnus
> --
> 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
> 

Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Magnus Damm]

On Fri, Aug 19, 2011 at 3:39 PM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Aug 19, 2011 at 02:20:11PM +0900, Simon Horman wrote:
>> On Fri, Aug 19, 2011 at 01:16:09PM +0900, Magnus Damm wrote:
>> > On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
>> > > This is intended to make it easier to correctly order IRQs.
>> > >
>> > > As suggested by Guennadi Liakhovetski.
>> >
>> > > --- a/arch/arm/mach-shmobile/board-ag5evm.c
>> > > +++ b/arch/arm/mach-shmobile/board-ag5evm.c
>> > > @@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
>> > >                .end    = 0xee1000ff,
>> > >                .flags  = IORESOURCE_MEM,
>> > >        },
>> > > -       [1] = {
>> > > +       [1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
>> > >                .start  = gic_spi(83),
>> > >                .flags  = IORESOURCE_IRQ,
>> > >        },
>> > > -       [2] = {
>> > > +       [1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
>> > >                .start  = gic_spi(84),
>> > >                .flags  = IORESOURCE_IRQ,
>> > >        },
>> > > -       [3] = {
>> > > +       [1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
>> > >                .start  = gic_spi(85),
>> > >                .flags  = IORESOURCE_IRQ,
>> > >        },
>> >
>> > Hm...
>> >
>> > So I know you guys have been discussing this back and forth, so I'm
>> > not sure if jumping in to this thread makes things any better. But
>> > anyhow, here are my opinions. Feel free to ignore them. =)
>> >
>> > First of all, having some kind of association with each IRQ is a good
>> > thing. I am however not convinced that using the index number of the
>> > platform device resource irq is the best option. Consider the case
>> > when someone modifies the SDHI resource in the code above to only
>> > include this interrupt:
>> >
>> >         [1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
>> >                 .start  = gic_spi(84),
>> >                 .flags  = IORESOURCE_IRQ,
>> >         },
>> >
>> > >From my point of view, the common sense for this would be that only
>> > the SDCARD interrupt would be enabled and the rest would be disabled
>> > since they are unspecified. However, with the current code the
>> > behavior would be something else, and since the index number of SDCARD
>> > is not matching it will be detected as CARD_DETECT.
>> >
>> > So isn't it really ugly to depend on the number of IRQs when they are
>> > supposed to be used as an index? I've been toying around with this
>> > driver for a few years now, and when I have a hard time creating
>> > correct platform data then it's _probably_ a sign that there must be
>> > better ways to implement this.
>> >
>> > I would propose just adding interrupts in struct resource [] as usual,
>> > and then have thee separate flags in the platform data for each
>> > interrupt type. If the number of IRQ bits set in the platform data
>> > flags doesn't match the number of interrupt resources then return
>> > error. If they match then simply go through each flag set in the
>> > platform data flags and assign next available interrupt resource. And
>> > if no flags are set then go for the combined interrupt handler for all
>> > available interrupt resources.
>> >
>> > That's what I would do at least. Any other ideas? Perhaps just keep an
>> > array of interrupt numbers in the platform data as the sh-sci driver
>> > does and use the fixed indexes there if non-zero?
>>
>> Hi Magnus,
>>
>> unfortunately during the course of the review of this series a number
>> of changes have crept in which I regard as being tangential to the
>> goal of the series - which is to introduce broken-out handlers (I have
>> already introduce support for broken-out IRQ sources).
>>
>> One area where such changes have occurred is in the subtle altering of the
>> list of IRQ sources that it is valid to supply (again, support for
>> broken-out IRQ sources is not introduced by this series).
>>
>> With regards to your comment and example above. I believe that your
>> understanding of my code is incorrect. The configuration you suggest will
>> be rejected because CARD_DETECT is not supplied, not because of an index
>> miss-match. It could be made acceptable within the current framework by
>> simply loosening up the logic a little (specifically to allow CARD_DETECT
>> to be omitted even if only one other IRQ source is supplied).  Incidently,
>> I think that would make sense but Guennadi specifically asked for that
>> combination to be regarded as invalid.
>
> [snip]
>
> After some discussion off-line I now realise that there is a problem
> with my implementation. Specifically that it assumes that
> platform_get_irq() takes into account the index of resource entries -
> it does not.

Right, this is a thing that only bites you once. =)

> As we are already on the slippery slope of allowing combinations
> other than 1 (legacy) or 3 (specific) IRQ sources I plan to implement
> a variant of your flag idea. The variation being to use names instead
> because a) that allows the use of platform_get_irq_byname() and b)
> the flags bits seem to be full and not driver-specific.

Great, platform_get_irq_byname() seems like a perfect match.

May I suggest "hotplug", "data" and "sdio" as names? I don't care very
much about names except keeping them short and precise to prevent
errors that can only be caught during runtime.

> And as are already on the slippery slope of allowing combinations
> if IRQ sources beyond 1 and 3 I intend to implement logic to allow
> any number of unique specific IRQ sources to be handled. In practice
> hardware for some of these combinations is unlikely to exist. But that
> doesn't seem to be a particularly good reason to add extra logic
> to disallow them in the driver - its up to platform data to specify
> something valid.

Yes, I totally agree. Thanks a lot for your help!

/ magnus

Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Magnus Damm]

On Fri, Aug 19, 2011 at 3:39 PM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Aug 19, 2011 at 02:20:11PM +0900, Simon Horman wrote:
>> On Fri, Aug 19, 2011 at 01:16:09PM +0900, Magnus Damm wrote:
>> > On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
>> > > This is intended to make it easier to correctly order IRQs.
>> > >
>> > > As suggested by Guennadi Liakhovetski.
>> >
>> > > --- a/arch/arm/mach-shmobile/board-ag5evm.c
>> > > +++ b/arch/arm/mach-shmobile/board-ag5evm.c
>> > > @@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
>> > >                .end    = 0xee1000ff,
>> > >                .flags  = IORESOURCE_MEM,
>> > >        },
>> > > -       [1] = {
>> > > +       [1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
>> > >                .start  = gic_spi(83),
>> > >                .flags  = IORESOURCE_IRQ,
>> > >        },
>> > > -       [2] = {
>> > > +       [1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
>> > >                .start  = gic_spi(84),
>> > >                .flags  = IORESOURCE_IRQ,
>> > >        },
>> > > -       [3] = {
>> > > +       [1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
>> > >                .start  = gic_spi(85),
>> > >                .flags  = IORESOURCE_IRQ,
>> > >        },
>> >
>> > Hm...
>> >
>> > So I know you guys have been discussing this back and forth, so I'm
>> > not sure if jumping in to this thread makes things any better. But
>> > anyhow, here are my opinions. Feel free to ignore them. =)
>> >
>> > First of all, having some kind of association with each IRQ is a good
>> > thing. I am however not convinced that using the index number of the
>> > platform device resource irq is the best option. Consider the case
>> > when someone modifies the SDHI resource in the code above to only
>> > include this interrupt:
>> >
>> >         [1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
>> >                 .start  = gic_spi(84),
>> >                 .flags  = IORESOURCE_IRQ,
>> >         },
>> >
>> > >From my point of view, the common sense for this would be that only
>> > the SDCARD interrupt would be enabled and the rest would be disabled
>> > since they are unspecified. However, with the current code the
>> > behavior would be something else, and since the index number of SDCARD
>> > is not matching it will be detected as CARD_DETECT.
>> >
>> > So isn't it really ugly to depend on the number of IRQs when they are
>> > supposed to be used as an index? I've been toying around with this
>> > driver for a few years now, and when I have a hard time creating
>> > correct platform data then it's _probably_ a sign that there must be
>> > better ways to implement this.
>> >
>> > I would propose just adding interrupts in struct resource [] as usual,
>> > and then have thee separate flags in the platform data for each
>> > interrupt type. If the number of IRQ bits set in the platform data
>> > flags doesn't match the number of interrupt resources then return
>> > error. If they match then simply go through each flag set in the
>> > platform data flags and assign next available interrupt resource. And
>> > if no flags are set then go for the combined interrupt handler for all
>> > available interrupt resources.
>> >
>> > That's what I would do at least. Any other ideas? Perhaps just keep an
>> > array of interrupt numbers in the platform data as the sh-sci driver
>> > does and use the fixed indexes there if non-zero?
>>
>> Hi Magnus,
>>
>> unfortunately during the course of the review of this series a number
>> of changes have crept in which I regard as being tangential to the
>> goal of the series - which is to introduce broken-out handlers (I have
>> already introduce support for broken-out IRQ sources).
>>
>> One area where such changes have occurred is in the subtle altering of the
>> list of IRQ sources that it is valid to supply (again, support for
>> broken-out IRQ sources is not introduced by this series).
>>
>> With regards to your comment and example above. I believe that your
>> understanding of my code is incorrect. The configuration you suggest will
>> be rejected because CARD_DETECT is not supplied, not because of an index
>> miss-match. It could be made acceptable within the current framework by
>> simply loosening up the logic a little (specifically to allow CARD_DETECT
>> to be omitted even if only one other IRQ source is supplied).  Incidently,
>> I think that would make sense but Guennadi specifically asked for that
>> combination to be regarded as invalid.
>
> [snip]
>
> After some discussion off-line I now realise that there is a problem
> with my implementation. Specifically that it assumes that
> platform_get_irq() takes into account the index of resource entries -
> it does not.

Right, this is a thing that only bites you once. =)

> As we are already on the slippery slope of allowing combinations
> other than 1 (legacy) or 3 (specific) IRQ sources I plan to implement
> a variant of your flag idea. The variation being to use names instead
> because a) that allows the use of platform_get_irq_byname() and b)
> the flags bits seem to be full and not driver-specific.

Great, platform_get_irq_byname() seems like a perfect match.

May I suggest "hotplug", "data" and "sdio" as names? I don't care very
much about names except keeping them short and precise to prevent
errors that can only be caught during runtime.

> And as are already on the slippery slope of allowing combinations
> if IRQ sources beyond 1 and 3 I intend to implement logic to allow
> any number of unique specific IRQ sources to be handled. In practice
> hardware for some of these combinations is unlikely to exist. But that
> doesn't seem to be a particularly good reason to add extra logic
> to disallow them in the driver - its up to platform data to specify
> something valid.

Yes, I totally agree. Thanks a lot for your help!

/ magnus

Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Magnus Damm]

On Fri, Aug 19, 2011 at 2:20 PM, Simon Horman <horms@verge.net.au> wrote:
...
> Moreover, I think that there needs to be some agreement between Guennadi
> and yourself with regards to what IRQ combinations are valid before I (or
> anyone else) can implement something that is acceptable to you both. And
> I think that in the mean time the current implementation is reasonable
> given that it works with all current use-cases (and several non-existent
> ones too).

The most common use cases for SDHI are:

Soldered-on SD/MMC hardware:
1) SDIO module (for wifi for instance)
Data IRQ + SDIO IRQ

2) eMMC chip
Data IRQ

External SD/MMC slot:
3) On hardware platforms with GPIO hotplug:
Data IRQ + SDIO IRQ

4) On hardware platforms without GPIO hotplug:
Hotplug IRQ + Data IRQ + SDIO IRQ

Legacy support:
5) Common ISR
All present IRQs

To keep things simple and flexible I would prefer if the platform
device could freely specify which interrupts to make use of. And still
have the legacy behavior.

Thanks,

/ magnus

Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Magnus Damm]

On Fri, Aug 19, 2011 at 2:20 PM, Simon Horman <horms@verge.net.au> wrote:
...
> Moreover, I think that there needs to be some agreement between Guennadi
> and yourself with regards to what IRQ combinations are valid before I (or
> anyone else) can implement something that is acceptable to you both. And
> I think that in the mean time the current implementation is reasonable
> given that it works with all current use-cases (and several non-existent
> ones too).

The most common use cases for SDHI are:

Soldered-on SD/MMC hardware:
1) SDIO module (for wifi for instance)
Data IRQ + SDIO IRQ

2) eMMC chip
Data IRQ

External SD/MMC slot:
3) On hardware platforms with GPIO hotplug:
Data IRQ + SDIO IRQ

4) On hardware platforms without GPIO hotplug:
Hotplug IRQ + Data IRQ + SDIO IRQ

Legacy support:
5) Common ISR
All present IRQs

To keep things simple and flexible I would prefer if the platform
device could freely specify which interrupts to make use of. And still
have the legacy behavior.

Thanks,

/ magnus

Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Simon Horman]

On Fri, Aug 19, 2011 at 02:20:11PM +0900, Simon Horman wrote:
> On Fri, Aug 19, 2011 at 01:16:09PM +0900, Magnus Damm wrote:
> > On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
> > > This is intended to make it easier to correctly order IRQs.
> > >
> > > As suggested by Guennadi Liakhovetski.
> > 
> > > --- a/arch/arm/mach-shmobile/board-ag5evm.c
> > > +++ b/arch/arm/mach-shmobile/board-ag5evm.c
> > > @@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
> > >                .end    = 0xee1000ff,
> > >                .flags  = IORESOURCE_MEM,
> > >        },
> > > -       [1] = {
> > > +       [1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
> > >                .start  = gic_spi(83),
> > >                .flags  = IORESOURCE_IRQ,
> > >        },
> > > -       [2] = {
> > > +       [1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> > >                .start  = gic_spi(84),
> > >                .flags  = IORESOURCE_IRQ,
> > >        },
> > > -       [3] = {
> > > +       [1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
> > >                .start  = gic_spi(85),
> > >                .flags  = IORESOURCE_IRQ,
> > >        },
> > 
> > Hm...
> > 
> > So I know you guys have been discussing this back and forth, so I'm
> > not sure if jumping in to this thread makes things any better. But
> > anyhow, here are my opinions. Feel free to ignore them. =)
> > 
> > First of all, having some kind of association with each IRQ is a good
> > thing. I am however not convinced that using the index number of the
> > platform device resource irq is the best option. Consider the case
> > when someone modifies the SDHI resource in the code above to only
> > include this interrupt:
> > 
> >         [1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> >                 .start  = gic_spi(84),
> >                 .flags  = IORESOURCE_IRQ,
> >         },
> > 
> > >From my point of view, the common sense for this would be that only
> > the SDCARD interrupt would be enabled and the rest would be disabled
> > since they are unspecified. However, with the current code the
> > behavior would be something else, and since the index number of SDCARD
> > is not matching it will be detected as CARD_DETECT.
> > 
> > So isn't it really ugly to depend on the number of IRQs when they are
> > supposed to be used as an index? I've been toying around with this
> > driver for a few years now, and when I have a hard time creating
> > correct platform data then it's _probably_ a sign that there must be
> > better ways to implement this.
> > 
> > I would propose just adding interrupts in struct resource [] as usual,
> > and then have thee separate flags in the platform data for each
> > interrupt type. If the number of IRQ bits set in the platform data
> > flags doesn't match the number of interrupt resources then return
> > error. If they match then simply go through each flag set in the
> > platform data flags and assign next available interrupt resource. And
> > if no flags are set then go for the combined interrupt handler for all
> > available interrupt resources.
> > 
> > That's what I would do at least. Any other ideas? Perhaps just keep an
> > array of interrupt numbers in the platform data as the sh-sci driver
> > does and use the fixed indexes there if non-zero?
> 
> Hi Magnus,
> 
> unfortunately during the course of the review of this series a number
> of changes have crept in which I regard as being tangential to the
> goal of the series - which is to introduce broken-out handlers (I have
> already introduce support for broken-out IRQ sources).
> 
> One area where such changes have occurred is in the subtle altering of the
> list of IRQ sources that it is valid to supply (again, support for
> broken-out IRQ sources is not introduced by this series).
> 
> With regards to your comment and example above. I believe that your
> understanding of my code is incorrect. The configuration you suggest will
> be rejected because CARD_DETECT is not supplied, not because of an index
> miss-match. It could be made acceptable within the current framework by
> simply loosening up the logic a little (specifically to allow CARD_DETECT
> to be omitted even if only one other IRQ source is supplied).  Incidently,
> I think that would make sense but Guennadi specifically asked for that
> combination to be regarded as invalid.

[snip]

After some discussion off-line I now realise that there is a problem
with my implementation. Specifically that it assumes that
platform_get_irq() takes into account the index of resource entries -
it does not.

As we are already on the slippery slope of allowing combinations
other than 1 (legacy) or 3 (specific) IRQ sources I plan to implement
a variant of your flag idea. The variation being to use names instead
because a) that allows the use of platform_get_irq_byname() and b)
the flags bits seem to be full and not driver-specific.

And as are already on the slippery slope of allowing combinations
if IRQ sources beyond 1 and 3 I intend to implement logic to allow
any number of unique specific IRQ sources to be handled. In practice
hardware for some of these combinations is unlikely to exist. But that
doesn't seem to be a particularly good reason to add extra logic
to disallow them in the driver - its up to platform data to specify
something valid.

Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Simon Horman]

On Fri, Aug 19, 2011 at 01:16:09PM +0900, Magnus Damm wrote:
> On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
> > This is intended to make it easier to correctly order IRQs.
> >
> > As suggested by Guennadi Liakhovetski.
> 
> > --- a/arch/arm/mach-shmobile/board-ag5evm.c
> > +++ b/arch/arm/mach-shmobile/board-ag5evm.c
> > @@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
> >                .end    = 0xee1000ff,
> >                .flags  = IORESOURCE_MEM,
> >        },
> > -       [1] = {
> > +       [1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
> >                .start  = gic_spi(83),
> >                .flags  = IORESOURCE_IRQ,
> >        },
> > -       [2] = {
> > +       [1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> >                .start  = gic_spi(84),
> >                .flags  = IORESOURCE_IRQ,
> >        },
> > -       [3] = {
> > +       [1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
> >                .start  = gic_spi(85),
> >                .flags  = IORESOURCE_IRQ,
> >        },
> 
> Hm...
> 
> So I know you guys have been discussing this back and forth, so I'm
> not sure if jumping in to this thread makes things any better. But
> anyhow, here are my opinions. Feel free to ignore them. =)
> 
> First of all, having some kind of association with each IRQ is a good
> thing. I am however not convinced that using the index number of the
> platform device resource irq is the best option. Consider the case
> when someone modifies the SDHI resource in the code above to only
> include this interrupt:
> 
>         [1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
>                 .start  = gic_spi(84),
>                 .flags  = IORESOURCE_IRQ,
>         },
> 
> >From my point of view, the common sense for this would be that only
> the SDCARD interrupt would be enabled and the rest would be disabled
> since they are unspecified. However, with the current code the
> behavior would be something else, and since the index number of SDCARD
> is not matching it will be detected as CARD_DETECT.
> 
> So isn't it really ugly to depend on the number of IRQs when they are
> supposed to be used as an index? I've been toying around with this
> driver for a few years now, and when I have a hard time creating
> correct platform data then it's _probably_ a sign that there must be
> better ways to implement this.
> 
> I would propose just adding interrupts in struct resource [] as usual,
> and then have thee separate flags in the platform data for each
> interrupt type. If the number of IRQ bits set in the platform data
> flags doesn't match the number of interrupt resources then return
> error. If they match then simply go through each flag set in the
> platform data flags and assign next available interrupt resource. And
> if no flags are set then go for the combined interrupt handler for all
> available interrupt resources.
> 
> That's what I would do at least. Any other ideas? Perhaps just keep an
> array of interrupt numbers in the platform data as the sh-sci driver
> does and use the fixed indexes there if non-zero?

Hi Magnus,

unfortunately during the course of the review of this series a number
of changes have crept in which I regard as being tangential to the
goal of the series - which is to introduce broken-out handlers (I have
already introduce support for broken-out IRQ sources).

One area where such changes have occurred is in the subtle altering of the
list of IRQ sources that it is valid to supply (again, support for
broken-out IRQ sources is not introduced by this series).

With regards to your comment and example above. I believe that your
understanding of my code is incorrect. The configuration you suggest will
be rejected because CARD_DETECT is not supplied, not because of an index
miss-match. It could be made acceptable within the current framework by
simply loosening up the logic a little (specifically to allow CARD_DETECT
to be omitted even if only one other IRQ source is supplied).  Incidently,
I think that would make sense but Guennadi specifically asked for that
combination to be regarded as invalid.

You suggestion to not rely on the array offset is an interesting one
and perhaps it would make things easier (to understand). But I don't
believe it would entirely eliminate the possibility of creating bogus
platform data. And in any case I believe it is tangential to the aim
of this series - to introduce broken-out handlers.

Moreover, I think that there needs to be some agreement between Guennadi
and yourself with regards to what IRQ combinations are valid before I (or
anyone else) can implement something that is acceptable to you both. And
I think that in the mean time the current implementation is reasonable
given that it works with all current use-cases (and several non-existent
ones too).

In short, can we have this discussion in the context of further enhancements?

Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Magnus Damm]

On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
> This is intended to make it easier to correctly order IRQs.
>
> As suggested by Guennadi Liakhovetski.

> --- a/arch/arm/mach-shmobile/board-ag5evm.c
> +++ b/arch/arm/mach-shmobile/board-ag5evm.c
> @@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
>                .end    = 0xee1000ff,
>                .flags  = IORESOURCE_MEM,
>        },
> -       [1] = {
> +       [1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
>                .start  = gic_spi(83),
>                .flags  = IORESOURCE_IRQ,
>        },
> -       [2] = {
> +       [1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
>                .start  = gic_spi(84),
>                .flags  = IORESOURCE_IRQ,
>        },
> -       [3] = {
> +       [1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
>                .start  = gic_spi(85),
>                .flags  = IORESOURCE_IRQ,
>        },

Hm...

So I know you guys have been discussing this back and forth, so I'm
not sure if jumping in to this thread makes things any better. But
anyhow, here are my opinions. Feel free to ignore them. =)

First of all, having some kind of association with each IRQ is a good
thing. I am however not convinced that using the index number of the
platform device resource irq is the best option. Consider the case
when someone modifies the SDHI resource in the code above to only
include this interrupt:

        [1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
                .start  = gic_spi(84),
                .flags  = IORESOURCE_IRQ,
        },

From my point of view, the common sense for this would be that only
the SDCARD interrupt would be enabled and the rest would be disabled
since they are unspecified. However, with the current code the
behavior would be something else, and since the index number of SDCARD
is not matching it will be detected as CARD_DETECT.

So isn't it really ugly to depend on the number of IRQs when they are
supposed to be used as an index? I've been toying around with this
driver for a few years now, and when I have a hard time creating
correct platform data then it's _probably_ a sign that there must be
better ways to implement this.

I would propose just adding interrupts in struct resource [] as usual,
and then have thee separate flags in the platform data for each
interrupt type. If the number of IRQ bits set in the platform data
flags doesn't match the number of interrupt resources then return
error. If they match then simply go through each flag set in the
platform data flags and assign next available interrupt resource. And
if no flags are set then go for the combined interrupt handler for all
available interrupt resources.

That's what I would do at least. Any other ideas? Perhaps just keep an
array of interrupt numbers in the platform data as the sh-sci driver
does and use the fixed indexes there if non-zero?

Thanks,

/ magnus

Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Magnus Damm]

On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
> This is intended to make it easier to correctly order IRQs.
>
> As suggested by Guennadi Liakhovetski.

> --- a/arch/arm/mach-shmobile/board-ag5evm.c
> +++ b/arch/arm/mach-shmobile/board-ag5evm.c
> @@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
>                .end    = 0xee1000ff,
>                .flags  = IORESOURCE_MEM,
>        },
> -       [1] = {
> +       [1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
>                .start  = gic_spi(83),
>                .flags  = IORESOURCE_IRQ,
>        },
> -       [2] = {
> +       [1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
>                .start  = gic_spi(84),
>                .flags  = IORESOURCE_IRQ,
>        },
> -       [3] = {
> +       [1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
>                .start  = gic_spi(85),
>                .flags  = IORESOURCE_IRQ,
>        },

Hm...

So I know you guys have been discussing this back and forth, so I'm
not sure if jumping in to this thread makes things any better. But
anyhow, here are my opinions. Feel free to ignore them. =)

First of all, having some kind of association with each IRQ is a good
thing. I am however not convinced that using the index number of the
platform device resource irq is the best option. Consider the case
when someone modifies the SDHI resource in the code above to only
include this interrupt:

        [1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
                .start  = gic_spi(84),
                .flags  = IORESOURCE_IRQ,
        },

From my point of view, the common sense for this would be that only
the SDCARD interrupt would be enabled and the rest would be disabled
since they are unspecified. However, with the current code the
behavior would be something else, and since the index number of SDCARD
is not matching it will be detected as CARD_DETECT.

So isn't it really ugly to depend on the number of IRQs when they are
supposed to be used as an index? I've been toying around with this
driver for a few years now, and when I have a hard time creating
correct platform data then it's _probably_ a sign that there must be
better ways to implement this.

I would propose just adding interrupts in struct resource [] as usual,
and then have thee separate flags in the platform data for each
interrupt type. If the number of IRQ bits set in the platform data
flags doesn't match the number of interrupt resources then return
error. If they match then simply go through each flag set in the
platform data flags and assign next available interrupt resource. And
if no flags are set then go for the combined interrupt handler for all
available interrupt resources.

That's what I would do at least. Any other ideas? Perhaps just keep an
array of interrupt numbers in the platform data as the sh-sci driver
does and use the fixed indexes there if non-zero?

Thanks,

/ magnus

[PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Simon Horman]

This is intended to make it easier to correctly order IRQs.

As suggested by Guennadi Liakhovetski.

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>

---

Depends on "mmc: sdhi: Make use of per-source irq handlers"

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   |   12 ++++++------
 arch/arm/mach-shmobile/board-mackerel.c |   18 +++++++++---------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index ce5c251..0d543bb 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xee1000ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(83),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(84),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= gic_spi(85),
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -395,15 +395,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xee1200ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(87),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(88),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + 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..f9d3a93 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1022,15 +1022,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xe68500ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e00) /* SDHI0_SDHI0I0 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0e20) /* SDHI0_SDHI0I1 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0e40) /* SDHI0_SDHI0I2 */,
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1065,15 +1065,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xe68600ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e80), /* SDHI1_SDHI1I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0ea0), /* SDHI1_SDHI1I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0ec0), /* SDHI1_SDHI1I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1116,15 +1116,15 @@ static struct resource sdhi2_resources[] = {
 		.end	= 0xe68700ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x1200), /* SDHI2_SDHI2I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x1220), /* SDHI2_SDHI2I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x1240), /* SDHI2_SDHI2I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
-- 
1.7.5.4

[PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Simon Horman]

This is intended to make it easier to correctly order IRQs.

As suggested by Guennadi Liakhovetski.

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>

---

Depends on "mmc: sdhi: Make use of per-source irq handlers"

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   |   12 ++++++------
 arch/arm/mach-shmobile/board-mackerel.c |   18 +++++++++---------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index ce5c251..0d543bb 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xee1000ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(83),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(84),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= gic_spi(85),
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -395,15 +395,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xee1200ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(87),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(88),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + 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..f9d3a93 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1022,15 +1022,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xe68500ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e00) /* SDHI0_SDHI0I0 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0e20) /* SDHI0_SDHI0I1 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0e40) /* SDHI0_SDHI0I2 */,
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1065,15 +1065,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xe68600ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e80), /* SDHI1_SDHI1I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0ea0), /* SDHI1_SDHI1I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0ec0), /* SDHI1_SDHI1I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1116,15 +1116,15 @@ static struct resource sdhi2_resources[] = {
 		.end	= 0xe68700ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x1200), /* SDHI2_SDHI2I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x1220), /* SDHI2_SDHI2I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x1240), /* SDHI2_SDHI2I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
-- 
1.7.5.4

[PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Simon Horman]

This is intended to make it easier to correctly order IRQs.

As suggested by Guennadi Liakhovetski.

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>

---

Depends on "mmc: sdhi: Make use of per-source irq handlers"

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   |   12 ++++++------
 arch/arm/mach-shmobile/board-mackerel.c |   18 +++++++++---------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index ce5c251..0d543bb 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xee1000ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(83),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(84),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= gic_spi(85),
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -395,15 +395,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xee1200ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(87),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(88),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + 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..f9d3a93 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1022,15 +1022,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xe68500ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e00) /* SDHI0_SDHI0I0 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0e20) /* SDHI0_SDHI0I1 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0e40) /* SDHI0_SDHI0I2 */,
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1065,15 +1065,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xe68600ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e80), /* SDHI1_SDHI1I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0ea0), /* SDHI1_SDHI1I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0ec0), /* SDHI1_SDHI1I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1116,15 +1116,15 @@ static struct resource sdhi2_resources[] = {
 		.end	= 0xe68700ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x1200), /* SDHI2_SDHI2I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x1220), /* SDHI2_SDHI2I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x1240), /* SDHI2_SDHI2I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
-- 
1.7.5.4

[PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Simon Horman]

This is intended to make it easier to correctly order IRQs.

As suggested by Guennadi Liakhovetski.

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>

---

Depends on "mmc: sdhi: Make use of per-source irq handlers"

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   |   12 ++++++------
 arch/arm/mach-shmobile/board-mackerel.c |   18 +++++++++---------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index ce5c251..0d543bb 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xee1000ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(83),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(84),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= gic_spi(85),
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -395,15 +395,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xee1200ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(87),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(88),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + 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..f9d3a93 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1022,15 +1022,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xe68500ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e00) /* SDHI0_SDHI0I0 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0e20) /* SDHI0_SDHI0I1 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0e40) /* SDHI0_SDHI0I2 */,
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1065,15 +1065,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xe68600ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e80), /* SDHI1_SDHI1I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0ea0), /* SDHI1_SDHI1I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0ec0), /* SDHI1_SDHI1I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1116,15 +1116,15 @@ static struct resource sdhi2_resources[] = {
 		.end	= 0xe68700ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x1200), /* SDHI2_SDHI2I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x1220), /* SDHI2_SDHI2I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x1240), /* SDHI2_SDHI2I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
-- 
1.7.5.4

Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Simon Horman]

On Tue, Aug 16, 2011 at 08:36:20PM +0900, Simon Horman wrote:
> On Tue, Aug 16, 2011 at 12:13:50PM +0100, Ben Dooks wrote:
> > On Tue, Aug 16, 2011 at 07:11:26PM +0900, Simon Horman wrote:
> > > This is intended to make it easier to correctly order IRQs.
> > > 
> > > As suggested by Guennadi Liakhovetski.
> > > 
> > > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > Cc: Magnus Damm <magnus.damm@gmail.com>
> > > Signed-off-by: Simon Horman <horms@verge.net.au>
> > > 
> > > ---
> > > 
> > > Depends on "mmc: sdhi: Make use of per-source irq handlers"
> > > ---
> > >  arch/arm/mach-shmobile/board-ag5evm.c   |   12 ++++++------
> > >  arch/arm/mach-shmobile/board-mackerel.c |   18 +++++++++---------
> > >  2 files changed, 15 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
> > > index ce5c251..c687f67 100644
> > > --- a/arch/arm/mach-shmobile/board-ag5evm.c
> > > +++ b/arch/arm/mach-shmobile/board-ag5evm.c
> > > @@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
> > >  		.end	= 0xee1000ff,
> > >  		.flags	= IORESOURCE_MEM,
> > >  	},
> > > -	[1] = {
> > > +	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> > >  		.start	= gic_spi(83),
> > >  		.flags	= IORESOURCE_IRQ,
> > >  	},
> > > -	[2] = {
> > > +	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
> > >  		.start	= gic_spi(84),
> > >  		.flags	= IORESOURCE_IRQ,
> > >  	},
> > > -	[3] = {
> > > +	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
> > >  		.start	= gic_spi(85),
> > >  		.flags	= IORESOURCE_IRQ,
> > >  	},
> > 
> > how about naming the irqs?
> 
> Sorry, I'm not sure what you are asking for.

Sorry, I've turned my brain back on...

I assume you are asking for #defines to give names to 83, 84 and 85.
While I think that sounds reasonable it would not be in
keeping with the rest contents of the platform files in question.
So I am a little reluctant to open that can of worms.

Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Simon Horman]

On Tue, Aug 16, 2011 at 12:13:50PM +0100, Ben Dooks wrote:
> On Tue, Aug 16, 2011 at 07:11:26PM +0900, Simon Horman wrote:
> > This is intended to make it easier to correctly order IRQs.
> > 
> > As suggested by Guennadi Liakhovetski.
> > 
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Magnus Damm <magnus.damm@gmail.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> > 
> > ---
> > 
> > Depends on "mmc: sdhi: Make use of per-source irq handlers"
> > ---
> >  arch/arm/mach-shmobile/board-ag5evm.c   |   12 ++++++------
> >  arch/arm/mach-shmobile/board-mackerel.c |   18 +++++++++---------
> >  2 files changed, 15 insertions(+), 15 deletions(-)
> > 
> > diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
> > index ce5c251..c687f67 100644
> > --- a/arch/arm/mach-shmobile/board-ag5evm.c
> > +++ b/arch/arm/mach-shmobile/board-ag5evm.c
> > @@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
> >  		.end	= 0xee1000ff,
> >  		.flags	= IORESOURCE_MEM,
> >  	},
> > -	[1] = {
> > +	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> >  		.start	= gic_spi(83),
> >  		.flags	= IORESOURCE_IRQ,
> >  	},
> > -	[2] = {
> > +	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
> >  		.start	= gic_spi(84),
> >  		.flags	= IORESOURCE_IRQ,
> >  	},
> > -	[3] = {
> > +	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
> >  		.start	= gic_spi(85),
> >  		.flags	= IORESOURCE_IRQ,
> >  	},
> 
> how about naming the irqs?

Sorry, I'm not sure what you are asking for.

Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Ben Dooks]

On Tue, Aug 16, 2011 at 07:11:26PM +0900, Simon Horman wrote:
> This is intended to make it easier to correctly order IRQs.
> 
> As suggested by Guennadi Liakhovetski.
> 
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> ---
> 
> Depends on "mmc: sdhi: Make use of per-source irq handlers"
> ---
>  arch/arm/mach-shmobile/board-ag5evm.c   |   12 ++++++------
>  arch/arm/mach-shmobile/board-mackerel.c |   18 +++++++++---------
>  2 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
> index ce5c251..c687f67 100644
> --- a/arch/arm/mach-shmobile/board-ag5evm.c
> +++ b/arch/arm/mach-shmobile/board-ag5evm.c
> @@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
>  		.end	= 0xee1000ff,
>  		.flags	= IORESOURCE_MEM,
>  	},
> -	[1] = {
> +	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
>  		.start	= gic_spi(83),
>  		.flags	= IORESOURCE_IRQ,
>  	},
> -	[2] = {
> +	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
>  		.start	= gic_spi(84),
>  		.flags	= IORESOURCE_IRQ,
>  	},
> -	[3] = {
> +	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
>  		.start	= gic_spi(85),
>  		.flags	= IORESOURCE_IRQ,
>  	},

how about naming the irqs?

-- 
Ben Dooks, ben@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

[PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Simon Horman]

This is intended to make it easier to correctly order IRQs.

As suggested by Guennadi Liakhovetski.

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

---

Depends on "mmc: sdhi: Make use of per-source irq handlers"
---
 arch/arm/mach-shmobile/board-ag5evm.c   |   12 ++++++------
 arch/arm/mach-shmobile/board-mackerel.c |   18 +++++++++---------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index ce5c251..c687f67 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xee1000ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(83),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(84),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= gic_spi(85),
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -395,15 +395,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xee1200ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(87),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(88),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + 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..bc575eb 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1022,15 +1022,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xe68500ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0e00) /* SDHI0_SDHI0I0 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e20) /* SDHI0_SDHI0I1 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0e40) /* SDHI0_SDHI0I2 */,
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1065,15 +1065,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xe68600ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0e80), /* SDHI1_SDHI1I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0ea0), /* SDHI1_SDHI1I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0ec0), /* SDHI1_SDHI1I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1116,15 +1116,15 @@ static struct resource sdhi2_resources[] = {
 		.end	= 0xe68700ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x1200), /* SDHI2_SDHI2I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x1220), /* SDHI2_SDHI2I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x1240), /* SDHI2_SDHI2I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
-- 
1.7.5.4

[PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum

[Simon Horman]

This is intended to make it easier to correctly order IRQs.

As suggested by Guennadi Liakhovetski.

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

---

Depends on "mmc: sdhi: Make use of per-source irq handlers"
---
 arch/arm/mach-shmobile/board-ag5evm.c   |   12 ++++++------
 arch/arm/mach-shmobile/board-mackerel.c |   18 +++++++++---------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index ce5c251..c687f67 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xee1000ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(83),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(84),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= gic_spi(85),
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -395,15 +395,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xee1200ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(87),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(88),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + 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..bc575eb 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1022,15 +1022,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xe68500ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0e00) /* SDHI0_SDHI0I0 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e20) /* SDHI0_SDHI0I1 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0e40) /* SDHI0_SDHI0I2 */,
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1065,15 +1065,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xe68600ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0e80), /* SDHI1_SDHI1I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0ea0), /* SDHI1_SDHI1I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0ec0), /* SDHI1_SDHI1I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1116,15 +1116,15 @@ static struct resource sdhi2_resources[] = {
 		.end	= 0xe68700ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x1200), /* SDHI2_SDHI2I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x1220), /* SDHI2_SDHI2I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x1240), /* SDHI2_SDHI2I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
-- 
1.7.5.4