* [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Name SDHI IRQ sources
From: Simon Horman @ 2011-08-19 7:57 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm,
Simon Horman
In-Reply-To: <1313740642-18307-1-git-send-email-horms@verge.net.au>
This allows specific (non-multiplexed) IRQ handlers to be used.
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
Requires
"mmc: sdhi: Allow specific IRQ sources to use corresponding handlers."
v7
* Rework to use named IRQs
v4
* Update for corrected ordering of SH_MOBILE_SDHI_IRQ_SDCARD and
SH_MOBILE_SDHI_IRQ_CARD_DETECT
v2
* Initial release
---
arch/arm/mach-shmobile/board-ag5evm.c | 6 ++++++
arch/arm/mach-shmobile/board-mackerel.c | 6 ++++++
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index ce5c251..e100cad 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -353,14 +353,17 @@ static struct resource sdhi0_resources[] = {
.flags = IORESOURCE_MEM,
},
[1] = {
+ .name = SH_MOBILE_SDHI_IRQ_CARD_DETECT,
.start = gic_spi(83),
.flags = IORESOURCE_IRQ,
},
[2] = {
+ .name = SH_MOBILE_SDHI_IRQ_SDCARD,
.start = gic_spi(84),
.flags = IORESOURCE_IRQ,
},
[3] = {
+ .name = SH_MOBILE_SDHI_IRQ_SDIO,
.start = gic_spi(85),
.flags = IORESOURCE_IRQ,
},
@@ -396,14 +399,17 @@ static struct resource sdhi1_resources[] = {
.flags = IORESOURCE_MEM,
},
[1] = {
+ .name = SH_MOBILE_SDHI_IRQ_CARD_DETECT,
.start = gic_spi(87),
.flags = IORESOURCE_IRQ,
},
[2] = {
+ .name = SH_MOBILE_SDHI_IRQ_SDCARD,
.start = gic_spi(88),
.flags = IORESOURCE_IRQ,
},
[3] = {
+ .name = SH_MOBILE_SDHI_IRQ_SDIO,
.start = gic_spi(89),
.flags = IORESOURCE_IRQ,
},
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index d41c01f..492274f 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1066,14 +1066,17 @@ static struct resource sdhi1_resources[] = {
.flags = IORESOURCE_MEM,
},
[1] = {
+ .name = SH_MOBILE_SDHI_IRQ_CARD_DETECT,
.start = evt2irq(0x0e80), /* SDHI1_SDHI1I0 */
.flags = IORESOURCE_IRQ,
},
[2] = {
+ .name = SH_MOBILE_SDHI_IRQ_SDCARD,
.start = evt2irq(0x0ea0), /* SDHI1_SDHI1I1 */
.flags = IORESOURCE_IRQ,
},
[3] = {
+ .name = SH_MOBILE_SDHI_IRQ_SDIO,
.start = evt2irq(0x0ec0), /* SDHI1_SDHI1I2 */
.flags = IORESOURCE_IRQ,
},
@@ -1117,14 +1120,17 @@ static struct resource sdhi2_resources[] = {
.flags = IORESOURCE_MEM,
},
[1] = {
+ .name = SH_MOBILE_SDHI_IRQ_CARD_DETECT,
.start = evt2irq(0x1200), /* SDHI2_SDHI2I0 */
.flags = IORESOURCE_IRQ,
},
[2] = {
+ .name = SH_MOBILE_SDHI_IRQ_SDCARD,
.start = evt2irq(0x1220), /* SDHI2_SDHI2I1 */
.flags = IORESOURCE_IRQ,
},
[3] = {
+ .name = SH_MOBILE_SDHI_IRQ_SDIO,
.start = evt2irq(0x1240), /* SDHI2_SDHI2I2 */
.flags = IORESOURCE_IRQ,
},
--
1.7.5.4
^ permalink raw reply related
* [PATCH 3/4] mmc: sdhi: Allow name IRQs to use specific handlers
From: Simon Horman @ 2011-08-19 7:57 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm,
Simon Horman
In-Reply-To: <1313740642-18307-1-git-send-email-horms@verge.net.au>
Allow named IRQs to use corresponding specific handlers.
Allow one or more such IRQ source;
Or allow one or more multiplexed (un-named) IRQ source.
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
v7
* As discussed with Magnus Damm
- The logic implemented to handle specific IRQ sources assumes
that platform_get_irq() takes into account the index of resource
entries. However, it does not.
Instead implement a scheme of named IRQs for specific IRQ sources.
And assume that un-named IRQs are (legacy) multiplexed IRQs.
As part of this change alter the logic for allowed IRQ sources as
follows:
Allow one or more specific (named) IRRs or;
One or more multiplexed (un-named) ISRs.
v6
* As discussed with Guennadi Liakhovetski:
- The SDCARD/SDIO change in v5 was not implemented as described.
And the logic wasn't fully described in any case. This
version (hope to!) implement the following logic:
1) 1 IRQ: only resource #0 (CARD_DETECT, use tmio_mmc_irq())
2) 2 or 3 IRQs: compulsory SDCARD and any further IRQs: use respective
specialised ISRs.
v5
* As suggested by Guennadi Liakhovetski:
- Allow only SH_MOBILE_SDHI_IRQ_SDCARD and
SH_MOBILE_SDHI_IRQ_SDIO to be specified in platform data
v4
* As suggested by Guennadi Liakhovetski:
- Correct inverted values of SH_MOBILE_SDHI_IRQ_SDCARD and
SH_MOBILE_SDHI_IRQ_CARD_DETECT
v3
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* As suggested by Guennadi Liakhovetski:
- Merge in patch "mmc: sdhi: Add defines for platform irq indexes"
- Use an enum instead of defines for irq indexes
v2
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* Make use of defines provided by
"mmc: sdhi: Add defines for platform irq indexes"
* As suggested by Guennadi Liakhovetski:
- Don't use a loop to initialise irq handlers, the unrolled version
is easier on the eyes (and exactly the same number of lines of code!)
wip: driver
---
drivers/mmc/host/sh_mobile_sdhi.c | 92 ++++++++++++++++++++++++++----------
include/linux/mmc/sh_mobile_sdhi.h | 4 ++
2 files changed, 71 insertions(+), 25 deletions(-)
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 774f643..c3e9ec7 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -96,7 +96,8 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
struct tmio_mmc_host *host;
char clk_name[8];
- int i, irq, ret;
+ int irq, ret, i = 0;
+ bool multiplexed_isr = true;
priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
if (priv = NULL) {
@@ -153,27 +154,54 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
if (ret < 0)
goto eprobe;
- for (i = 0; i < 3; i++) {
- irq = platform_get_irq(pdev, i);
- if (irq < 0) {
- if (i) {
- continue;
- } else {
- ret = irq;
- goto eirq;
- }
- }
- ret = request_irq(irq, tmio_mmc_irq, 0,
+ /* Allow one or more specific (named) IRRs or;
+ * One or more multiplexed (un-named) ISRs.
+ */
+
+ irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+ if (irq >= 0) {
+ multiplexed_isr = false;
+ ret = request_irq(irq, tmio_mmc_card_detect_irq, 0,
dev_name(&pdev->dev), host);
- if (ret) {
- while (i--) {
- irq = platform_get_irq(pdev, i);
- if (irq >= 0)
- free_irq(irq, host);
- }
- goto eirq;
+ if (ret)
+ goto eirq_card_detect;
+ }
+
+ irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+ if (irq >= 0) {
+ multiplexed_isr = false;
+ ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
+ dev_name(&pdev->dev), host);
+ if (ret)
+ goto eirq_sdcard;
+ }
+
+ irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+ if (irq >= 0) {
+ multiplexed_isr = false;
+ ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
+ dev_name(&pdev->dev), host);
+ if (ret)
+ goto eirq_sdio;
+ }
+
+ if (multiplexed_isr) {
+ while (1) {
+ irq = platform_get_irq(pdev, i);
+ if (irq < 0)
+ break;
+ i++;
+ ret = request_irq(irq, tmio_mmc_irq, 0,
+ dev_name(&pdev->dev), host);
+ if (ret)
+ goto eirq_multiplexed;
}
+
+ /* There must be at least one IRQ source */
+ if (!i)
+ goto eirq_multiplexed;
}
+
dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
mmc_hostname(host->mmc), (unsigned long)
(platform_get_resource(pdev,IORESOURCE_MEM, 0)->start),
@@ -181,7 +209,20 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
return ret;
-eirq:
+eirq_multiplexed:
+ while (i--) {
+ irq = platform_get_irq(pdev, i);
+ free_irq(irq, host);
+ }
+eirq_sdio:
+ irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+ if (irq >= 0)
+ free_irq(irq, host);
+eirq_sdcard:
+ irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+ if (irq >= 0)
+ free_irq(irq, host);
+eirq_card_detect:
tmio_mmc_host_remove(host);
eprobe:
clk_disable(priv->clk);
@@ -197,16 +238,17 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
struct tmio_mmc_host *host = mmc_priv(mmc);
struct sh_mobile_sdhi *priv = container_of(host->pdata, struct sh_mobile_sdhi, mmc_data);
struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
- int i, irq;
+ int i = 0, irq;
p->pdata = NULL;
tmio_mmc_host_remove(host);
- for (i = 0; i < 3; i++) {
- irq = platform_get_irq(pdev, i);
- if (irq >= 0)
- free_irq(irq, host);
+ while (0) {
+ irq = platform_get_irq(pdev, i++);
+ if (irq < 0)
+ break;
+ free_irq(irq, host);
}
clk_disable(priv->clk);
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index bd50b36..71b8054 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -6,6 +6,10 @@
struct platform_device;
struct tmio_mmc_data;
+#define SH_MOBILE_SDHI_IRQ_CARD_DETECT "card_detect"
+#define SH_MOBILE_SDHI_IRQ_SDCARD "sdcard"
+#define SH_MOBILE_SDHI_IRQ_SDIO "sdio"
+
struct sh_mobile_sdhi_info {
int dma_slave_tx;
int dma_slave_rx;
--
1.7.5.4
^ permalink raw reply related
* [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
From: Simon Horman @ 2011-08-19 7:57 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm,
Simon Horman
In-Reply-To: <1313740642-18307-1-git-send-email-horms@verge.net.au>
Provide separate interrupt handlers which may be used by platforms where
SDHI has three interrupt sources.
This patch also removes the commented-out handling of CRC and other errors.
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested
v4
* As suggested by Guennadi Liakhovetski
- Use bool as return type for __tmio_mmc_sdcard_irq() and
__tmio_mmc_card_detect_irq()
v3
* Rebase for updated "mmc: tmio: Cache interrupt masks"
* As suggested by Guennadi Liakhovetski
- Do not alter logic to handle more than one interupt at once
- Add missing "static" to declartion of __tmio_mmc_sdcard_irq()
v2
* As suggested by Guennadi Liakhovetski
- Combine 3 patches into one
- Reduce the number of __tmio_..._irq() functions
- Rename "...card_access..." functions as "...sdcard..."
---
drivers/mmc/host/tmio_mmc.h | 3 +
drivers/mmc/host/tmio_mmc_pio.c | 131 ++++++++++++++++++++++++--------------
2 files changed, 86 insertions(+), 48 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 1cf8db5..3020f98 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
irqreturn_t tmio_mmc_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
unsigned long *flags)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f0c7830..6275e3d 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -545,44 +545,20 @@ out:
spin_unlock(&host->lock);
}
-irqreturn_t tmio_mmc_irq(int irq, void *devid)
+static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
+ int *ireg, int *status)
{
- struct tmio_mmc_host *host = devid;
- struct mmc_host *mmc = host->mmc;
- struct tmio_mmc_data *pdata = host->pdata;
- unsigned int ireg, status;
- unsigned int sdio_ireg, sdio_status;
-
- pr_debug("MMC IRQ begin\n");
-
- status = sd_ctrl_read32(host, CTL_STATUS);
- ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
+ *status = sd_ctrl_read32(host, CTL_STATUS);
+ *ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
- sdio_ireg = 0;
- if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
- sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
- sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
- ~host->sdio_irq_mask;
-
- sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
-
- if (sdio_ireg && !host->sdio_irq_enabled) {
- pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
- sdio_status, host->sdio_irq_mask, sdio_ireg);
- tmio_mmc_enable_sdio_irq(mmc, 0);
- goto out;
- }
-
- if (mmc->caps & MMC_CAP_SDIO_IRQ &&
- sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
- mmc_signal_sdio_irq(mmc);
-
- if (sdio_ireg)
- goto out;
- }
+ pr_debug_status(*status);
+ pr_debug_status(*ireg);
+}
- pr_debug_status(status);
- pr_debug_status(ireg);
+static bool __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
+ int ireg, int status)
+{
+ struct mmc_host *mmc = host->mmc;
/* Card insert / remove attempts */
if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
@@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
!work_pending(&mmc->detect.work))
mmc_detect_change(host->mmc, msecs_to_jiffies(100));
- goto out;
+ return true;
}
- /* CRC and other errors */
-/* if (ireg & TMIO_STAT_ERR_IRQ)
- * handled |= tmio_error_irq(host, irq, stat);
- */
+ return false;
+}
+
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
+{
+ unsigned int ireg, status;
+ struct tmio_mmc_host *host = devid;
+ tmio_mmc_card_irq_status(host, &ireg, &status);
+ __tmio_mmc_card_detect_irq(host, ireg, status);
+
+ return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
+
+static bool __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
+ int ireg, int status)
+{
/* Command completion */
if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
tmio_mmc_ack_mmc_irqs(host,
TMIO_STAT_CMDRESPEND |
TMIO_STAT_CMDTIMEOUT);
tmio_mmc_cmd_irq(host, status);
- goto out;
+ return true;
}
/* Data transfer */
if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
tmio_mmc_pio_irq(host);
- goto out;
+ return true;
}
/* Data transfer completion */
if (ireg & TMIO_STAT_DATAEND) {
tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
tmio_mmc_data_irq(host);
- goto out;
+ return true;
}
- pr_warning("tmio_mmc: Spurious irq, disabling! "
- "0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
- pr_debug_status(status);
- tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
+ return false;
+}
+
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
+{
+ unsigned int ireg, status;
+ struct tmio_mmc_host *host = devid;
+
+ tmio_mmc_card_irq_status(host, &ireg, &status);
+ __tmio_mmc_sdcard_irq(host, ireg, status);
+
+ return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
+
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
+{
+ struct tmio_mmc_host *host = devid;
+ struct mmc_host *mmc = host->mmc;
+ struct tmio_mmc_data *pdata = host->pdata;
+ unsigned int ireg, status;
+
+ if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
+ return IRQ_HANDLED;
+
+ status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
+ ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
+
+ sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
+
+ if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
+ mmc_signal_sdio_irq(mmc);
+
+ return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdio_irq);
+
+irqreturn_t tmio_mmc_irq(int irq, void *devid)
+{
+ struct tmio_mmc_host *host = devid;
+ unsigned int ireg, status;
+
+ pr_debug("MMC IRQ begin\n");
+
+ tmio_mmc_card_irq_status(host, &ireg, &status);
+ if (__tmio_mmc_card_detect_irq(host, ireg, status))
+ return IRQ_HANDLED;
+ if (__tmio_mmc_sdcard_irq(host, ireg, status))
+ return IRQ_HANDLED;
+
+ tmio_mmc_sdio_irq(irq, devid);
-out:
return IRQ_HANDLED;
}
EXPORT_SYMBOL(tmio_mmc_irq);
--
1.7.5.4
^ permalink raw reply related
* [PATCH 1/4] mmc: tmio: Cache interrupt masks
From: Simon Horman @ 2011-08-19 7:57 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm,
Simon Horman
In-Reply-To: <1313740642-18307-1-git-send-email-horms@verge.net.au>
This avoids the need to look up the masks each time an interrupt
is handled.
As suggested by Guennadi.
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested
v3
* As suggested by Guennadi Liakhovetski
- Only read sdcard_irq_mask once and never read sdio_irq_mask,
instead use the cached values as much as possible
v2
* Initial release
---
drivers/mmc/host/tmio_mmc.h | 4 ++++
drivers/mmc/host/tmio_mmc_pio.c | 34 ++++++++++++++++++----------------
2 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index eeaf643..1cf8db5 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -79,6 +79,10 @@ struct tmio_mmc_host {
struct delayed_work delayed_reset_work;
struct work_struct done;
+ /* Cache IRQ mask */
+ u32 sdcard_irq_mask;
+ u32 sdio_irq_mask;
+
spinlock_t lock; /* protect host private data */
unsigned long last_req_ts;
struct mutex ios_lock; /* protect set_ios() context */
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 1f16357..f0c7830 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -48,14 +48,14 @@
void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i)
{
- u32 mask = sd_ctrl_read32(host, CTL_IRQ_MASK) & ~(i & TMIO_MASK_IRQ);
- sd_ctrl_write32(host, CTL_IRQ_MASK, mask);
+ host->sdcard_irq_mask &= ~(i & TMIO_MASK_IRQ);
+ sd_ctrl_write32(host, CTL_IRQ_MASK, host->sdcard_irq_mask);
}
void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i)
{
- u32 mask = sd_ctrl_read32(host, CTL_IRQ_MASK) | (i & TMIO_MASK_IRQ);
- sd_ctrl_write32(host, CTL_IRQ_MASK, mask);
+ host->sdcard_irq_mask |= (i & TMIO_MASK_IRQ);
+ sd_ctrl_write32(host, CTL_IRQ_MASK, host->sdcard_irq_mask);
}
static void tmio_mmc_ack_mmc_irqs(struct tmio_mmc_host *host, u32 i)
@@ -127,11 +127,13 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
if (enable) {
host->sdio_irq_enabled = 1;
+ host->sdio_irq_mask = TMIO_SDIO_MASK_ALL &
+ ~TMIO_SDIO_STAT_IOIRQ;
sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001);
- sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK,
- (TMIO_SDIO_MASK_ALL & ~TMIO_SDIO_STAT_IOIRQ));
+ sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
} else {
- sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, TMIO_SDIO_MASK_ALL);
+ host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
+ sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000);
host->sdio_irq_enabled = 0;
}
@@ -548,26 +550,25 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
struct tmio_mmc_host *host = devid;
struct mmc_host *mmc = host->mmc;
struct tmio_mmc_data *pdata = host->pdata;
- unsigned int ireg, irq_mask, status;
- unsigned int sdio_ireg, sdio_irq_mask, sdio_status;
+ unsigned int ireg, status;
+ unsigned int sdio_ireg, sdio_status;
pr_debug("MMC IRQ begin\n");
status = sd_ctrl_read32(host, CTL_STATUS);
- irq_mask = sd_ctrl_read32(host, CTL_IRQ_MASK);
- ireg = status & TMIO_MASK_IRQ & ~irq_mask;
+ ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
sdio_ireg = 0;
if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
- sdio_irq_mask = sd_ctrl_read16(host, CTL_SDIO_IRQ_MASK);
- sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL & ~sdio_irq_mask;
+ sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
+ ~host->sdio_irq_mask;
sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
if (sdio_ireg && !host->sdio_irq_enabled) {
pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
- sdio_status, sdio_irq_mask, sdio_ireg);
+ sdio_status, host->sdio_irq_mask, sdio_ireg);
tmio_mmc_enable_sdio_irq(mmc, 0);
goto out;
}
@@ -623,9 +624,9 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
}
pr_warning("tmio_mmc: Spurious irq, disabling! "
- "0x%08x 0x%08x 0x%08x\n", status, irq_mask, ireg);
+ "0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
pr_debug_status(status);
- tmio_mmc_disable_mmc_irqs(host, status & ~irq_mask);
+ tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
out:
return IRQ_HANDLED;
@@ -882,6 +883,7 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
tmio_mmc_clk_stop(_host);
tmio_mmc_reset(_host);
+ _host->sdcard_irq_mask = sd_ctrl_read32(_host, CTL_IRQ_MASK);
tmio_mmc_disable_mmc_irqs(_host, TMIO_MASK_ALL);
if (pdata->flags & TMIO_MMC_SDIO_IRQ)
tmio_mmc_enable_sdio_irq(mmc, 0);
--
1.7.5.4
^ permalink raw reply related
* [PATCH 0/4 v7] mmc: tmio, sdhi: provide multiple irq handlers
From: Simon Horman @ 2011-08-19 7:57 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm,
Simon Horman
The SDHI driver already supports making use of up to three interrupt
sources.
This series breaks up the existing interrupt handler into three handlers,
one for card access, one for card detect interrupts, and one for SDIO
interrupts. A cover-all handler, which makes use of these new broken-out
handlers is provided for for the case where there is only one interrupt
source.
This series also wires up the broken-out irq handlers in the SDHI driver
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index
From: Guennadi Liakhovetski @ 2011-08-19 7:52 UTC (permalink / raw)
To: Simon Horman; +Cc: Magnus Damm, linux-mmc, linux-sh, Chris Ball, Paul Mundt
In-Reply-To: <20110819071720.GD16419@verge.net.au>
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/
^ permalink raw reply
* Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index
From: Guennadi Liakhovetski @ 2011-08-19 7:45 UTC (permalink / raw)
To: Magnus Damm; +Cc: Simon Horman, linux-mmc, linux-sh, Chris Ball, Paul Mundt
In-Reply-To: <CANqRtoTu6G=Dut8eQb+aZPrQ9pF383tGHnQ6JNbH=R2wcuuNiA@mail.gmail.com>
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/
^ permalink raw reply
* Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index
From: Simon Horman @ 2011-08-19 7:17 UTC (permalink / raw)
To: Magnus Damm
Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt,
Guennadi Liakhovetski
In-Reply-To: <CANqRtoTu6G=Dut8eQb+aZPrQ9pF383tGHnQ6JNbH=R2wcuuNiA@mail.gmail.com>
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
>
^ permalink raw reply
* Re: [PATCH] mmc: sdhi: initialise mmc_data->flags before use
From: Guennadi Liakhovetski @ 2011-08-19 7:06 UTC (permalink / raw)
To: Magnus Damm; +Cc: Simon Horman, linux-mmc, linux-sh, Chris Ball
In-Reply-To: <CANqRtoRx5MC-smUjuYBgpstcm49_PqwXqG=YEe+dkqVte7eQ9w@mail.gmail.com>
On Fri, 19 Aug 2011, Magnus Damm wrote:
> On Fri, Aug 19, 2011 at 10:07 AM, Simon Horman <horms@verge.net.au> wrote:
> > This corrects a logic error that I introduced in
> > "mmc: sdhi: Add write16_hook"
> >
> > Reported-by: Magnus Damm <magnus.damm@gmail.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> > ---
> > drivers/mmc/host/sh_mobile_sdhi.c | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
>
> Hi Simon,
>
> Thanks for providing a fix for this issue. Correct me if I'm wrong,
> but I believe this patch is suitable for 3.1-rc. So it would be great
> to have it merged rather soon as a fix instead of delaying it to 3.2.
Indeed:
Acked-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Chris, please, push for 3.1.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum
From: Magnus Damm @ 2011-08-19 6:51 UTC (permalink / raw)
To: Simon Horman
Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt,
Guennadi Liakhovetski
In-Reply-To: <20110819063857.GC16419@verge.net.au>
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
^ permalink raw reply
* Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum
From: Magnus Damm @ 2011-08-19 6:44 UTC (permalink / raw)
To: Simon Horman
Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt,
Guennadi Liakhovetski
In-Reply-To: <20110819052004.GB17548@verge.net.au>
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
^ permalink raw reply
* Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index
From: Simon Horman @ 2011-08-19 6:39 UTC (permalink / raw)
To: Magnus Damm
Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt,
Guennadi Liakhovetski
In-Reply-To: <20110819052004.GB17548@verge.net.au>
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.
^ permalink raw reply
* Re: LCDC problem after commit 794d78fea51504bad3880d14f354a9847f318f25
From: Laurent Pinchart @ 2011-08-19 6:24 UTC (permalink / raw)
To: linux-sh
In-Reply-To: <201108080125.34329.rjw@sisk.pl>
Hi Damian,
On Friday 12 August 2011 11:59:10 Laurent Pinchart wrote:
> On Thursday 11 August 2011 21:37:04 Rafael J. Wysocki wrote:
> > On Thursday, August 11, 2011, Laurent Pinchart wrote:
> > > On Tuesday 09 August 2011 12:48:44 Magnus Damm wrote:
> > > > On Mon, Aug 8, 2011 at 10:46 AM, Magnus Damm wrote:
> > > > > On Mon, Aug 8, 2011 at 8:25 AM, Rafael J. Wysocki wrote:
> > > > >> Unfortunately, after commit
> > > > >> 794d78fea51504bad3880d14f354a9847f318f25 (drivers: sh: late
> > > > >> disabling of clocks V2) the LCDC screen on my Mackerel board goes
> > > > >> off (late) during boot and cannot be turned on by any way AFAICS
> > > > >> (the backlight remains on, though).
> > > > >>
> > > > >> It looks like the late disabling of clocks interacts badly with
> > > > >> the clock management done by PM domains.
> > > > >
> > > > > This is most likely showing a so-far-not-handled clock dependency
> > > > > for the LCDC hardware. I'll have a look.
> > > >
> > > > Further investigation shows that this only triggers when the MERAM is
> > > > enabled in the kernel configuration using
> > > > CONFIG_FB_SH_MOBILE_MERAM=y. So this seems more related to MERAM
> > > > power management than LCDC.
> > > >
> > > > Digging a bit deeper makes it clear that the sh_mobile_meram.c driver
> > > > in upstream doesn't seem to include any Runtime PM code. I recall
> > > > seeing various versions posted to the mailing list.
> > > >
> > > > Laurent [CC], can you please provide us with a list of outstanding
> > > > LCDC/MERAM patches to make this work as expected? I realize there are
> > > > quite a few outstanding LCDC patches, but if possible please provide
> > > > a list of the bare minimum to make this work with 3.1-rc.
> > >
> > > I've pushed my pending patches to
> > > http://git.linuxtv.org/pinchartl/fbdev.git in the pm-domains branch.
> > > The first 19 patches have been picked from mailing lists. Hopefully
> > > they're the latest versions that have been posted. The next 11 patches
> > > shouldn't be required to enable runtime PM with the MERAM, but they
> > > provide code cleanup and bug fixes.
> > >
> > > Are Rafael' and Damian's patches ready for mainline ? If so, when will
> > > they be pulled ? They're more than one month old, I was expecting them
> > > to hit v3.1- rc1. The backlog is growing, which makes collaboration
> > > more difficult.
> >
> > Which of my patches are you referring to in particular?
>
> My bad, your patches are in v3.1-rc1. I had rebased my branch on top of the
> wrong tag. We still need to push Damian's MERAM runtime PM patches, as well
> as mine.
Do you plan to push your MERAM runtime PM patches (available at
http://git.linuxtv.org/pinchartl/fbdev.git/shortlog/refs/heads/pm-domains
rebased on top of v3.1-rc2) ?
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
From: Magnus Damm @ 2011-08-19 5:41 UTC (permalink / raw)
To: Simon Horman
Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt,
Guennadi Liakhovetski
In-Reply-To: <20110819045858.GA17548@verge.net.au>
On Fri, Aug 19, 2011 at 1:59 PM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Aug 19, 2011 at 01:27:41PM +0900, Magnus Damm wrote:
>> On Fri, Aug 19, 2011 at 12:30 PM, Simon Horman <horms@verge.net.au> wrote:
>> > On Fri, Aug 19, 2011 at 12:09:50PM +0900, Magnus Damm wrote:
>> >> On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
>> >> > Provide separate interrupt handlers which may be used by platforms where
>> >> > SDHI has three interrupt sources.
>> >> >
>> >> > This patch also removes the commented-out handling of CRC and other errors.
>> >> >
>> >> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>> >> > Cc: Magnus Damm <magnus.damm@gmail.com>
>> >> > Signed-off-by: Simon Horman <horms@verge.net.au>
>> >> >
>> >> > ---
>> >>
>> >> > +irqreturn_t tmio_mmc_irq(int irq, void *devid)
>> >> > +{
>> >> > + struct tmio_mmc_host *host = devid;
>> >> > + unsigned int ireg, status;
>> >> > +
>> >> > + pr_debug("MMC IRQ begin\n");
>> >> > +
>> >> > + tmio_mmc_card_irq_status(host, &ireg, &status);
>> >> > + if (__tmio_mmc_card_detect_irq(host, ireg, status))
>> >> > + return IRQ_HANDLED;
>> >> > + if (__tmio_mmc_sdcard_irq(host, ireg, status))
>> >> > + return IRQ_HANDLED;
>> >> > +
>> >> > + tmio_mmc_sdio_irq(irq, devid);
>> >> >
>> >> > -out:
>> >> > return IRQ_HANDLED;
>> >> > }
>> >> > EXPORT_SYMBOL(tmio_mmc_irq);
>> >>
>> >> Is there any particular reason for returning early in this interrupt
>> >> handler? By returning early I mean the "if ... return IRQ_HANDLED"
>> >> cases above.
>> >>
>> >> I realize the old ISR code in the driver does just this, so if the
>> >> goal is to stay compatible then I guess we should keep this behavior.
>> >> >From my point of view it usually makes more sense to try to handle all
>> >> events that may be associated with the IRQ.
>> >
>> > My original post had the behaviour that you suggest but Guennadi
>> > indicted that he would be much more comfortable with keeping the original
>> > behaviour as it is know to work on a wide range of hardware.
>>
>> I see, thanks for your patience...
>>
>> So I may remember this wrong, but for the 3 different interrupt
>> sources I believe that the SDIO IRQ code was added by Arnd for one of
>> the Renesas SDHI platforms. Back then Ian disliked supporting more
>> than a single interrupt source, so for that reason the SDIO IRQ code
>> was added on top of the common interrupt handler. We had no
>> documentation either, so it was added in a rather random way. I recall
>> the SDIO IRQ being handled before the other interrupt types in the
>> common handler not last, but that's not very important. Anyway, with
>> the fact that SDIO IRQ support was added for SDHI platforms in mind
>> then we can assume that other platforms won't need it. Not sure if
>> this fact will improve our situation or not.
>>
>> As for the two remaining interrupts, I believe they share hardware
>> registers somehow. I guess I'm OK keeping the original behavior
>> somehow, but I still believe it's incorrect. It may not matter very
>> much though since it's rather unlikely that hotplug insertion or eject
>> coincides with the data IRQs.
>
> Hi Magnus,
>
> to be honest I'm unsure if the current behaviour is correct or not.
> But it does appear to work on a wide range of hardware. And as
> tmio_mmc_irq() is an implementation of the single-source ISR intended
> for legacy purposes it does seem reasonable to maintain the existing
> behaviour.
Sure, I agree.
> If there is a need to change the behaviour of tmio_mmc_irq() in some way
> then I think that is best dealt with in a separate patch (series)
> subsequent to the current series that is intended to introduce broken-out
> handlers.
Sounds good, I agree that this should be handled separately if needed.
Thanks,
/ magnus
^ permalink raw reply
* Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index
From: Simon Horman @ 2011-08-19 5:20 UTC (permalink / raw)
To: Magnus Damm
Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt,
Guennadi Liakhovetski
In-Reply-To: <CANqRtoQL4cw2+71P5+1hU-2TUyV=gQJ9X2NN+_e_S3dgKeWWJA@mail.gmail.com>
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?
^ permalink raw reply
* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
From: Simon Horman @ 2011-08-19 4:59 UTC (permalink / raw)
To: Magnus Damm
Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt,
Guennadi Liakhovetski
In-Reply-To: <CANqRtoQDFpr_YTnSObyidMKpTyQXE=W6JDoBXrJ-MpLM1r_wCA@mail.gmail.com>
On Fri, Aug 19, 2011 at 01:27:41PM +0900, Magnus Damm wrote:
> On Fri, Aug 19, 2011 at 12:30 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Fri, Aug 19, 2011 at 12:09:50PM +0900, Magnus Damm wrote:
> >> On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
> >> > Provide separate interrupt handlers which may be used by platforms where
> >> > SDHI has three interrupt sources.
> >> >
> >> > This patch also removes the commented-out handling of CRC and other errors.
> >> >
> >> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> >> > Cc: Magnus Damm <magnus.damm@gmail.com>
> >> > Signed-off-by: Simon Horman <horms@verge.net.au>
> >> >
> >> > ---
> >>
> >> > +irqreturn_t tmio_mmc_irq(int irq, void *devid)
> >> > +{
> >> > + struct tmio_mmc_host *host = devid;
> >> > + unsigned int ireg, status;
> >> > +
> >> > + pr_debug("MMC IRQ begin\n");
> >> > +
> >> > + tmio_mmc_card_irq_status(host, &ireg, &status);
> >> > + if (__tmio_mmc_card_detect_irq(host, ireg, status))
> >> > + return IRQ_HANDLED;
> >> > + if (__tmio_mmc_sdcard_irq(host, ireg, status))
> >> > + return IRQ_HANDLED;
> >> > +
> >> > + tmio_mmc_sdio_irq(irq, devid);
> >> >
> >> > -out:
> >> > return IRQ_HANDLED;
> >> > }
> >> > EXPORT_SYMBOL(tmio_mmc_irq);
> >>
> >> Is there any particular reason for returning early in this interrupt
> >> handler? By returning early I mean the "if ... return IRQ_HANDLED"
> >> cases above.
> >>
> >> I realize the old ISR code in the driver does just this, so if the
> >> goal is to stay compatible then I guess we should keep this behavior.
> >> >From my point of view it usually makes more sense to try to handle all
> >> events that may be associated with the IRQ.
> >
> > My original post had the behaviour that you suggest but Guennadi
> > indicted that he would be much more comfortable with keeping the original
> > behaviour as it is know to work on a wide range of hardware.
>
> I see, thanks for your patience...
>
> So I may remember this wrong, but for the 3 different interrupt
> sources I believe that the SDIO IRQ code was added by Arnd for one of
> the Renesas SDHI platforms. Back then Ian disliked supporting more
> than a single interrupt source, so for that reason the SDIO IRQ code
> was added on top of the common interrupt handler. We had no
> documentation either, so it was added in a rather random way. I recall
> the SDIO IRQ being handled before the other interrupt types in the
> common handler not last, but that's not very important. Anyway, with
> the fact that SDIO IRQ support was added for SDHI platforms in mind
> then we can assume that other platforms won't need it. Not sure if
> this fact will improve our situation or not.
>
> As for the two remaining interrupts, I believe they share hardware
> registers somehow. I guess I'm OK keeping the original behavior
> somehow, but I still believe it's incorrect. It may not matter very
> much though since it's rather unlikely that hotplug insertion or eject
> coincides with the data IRQs.
Hi Magnus,
to be honest I'm unsure if the current behaviour is correct or not.
But it does appear to work on a wide range of hardware. And as
tmio_mmc_irq() is an implementation of the single-source ISR intended
for legacy purposes it does seem reasonable to maintain the existing
behaviour.
If there is a need to change the behaviour of tmio_mmc_irq() in some way
then I think that is best dealt with in a separate patch (series)
subsequent to the current series that is intended to introduce broken-out
handlers.
^ permalink raw reply
* Re: [PATCH 1/4] mmc: tmio: Cache interrupt masks
From: Magnus Damm @ 2011-08-19 4:32 UTC (permalink / raw)
To: Simon Horman
Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt,
Guennadi Liakhovetski
In-Reply-To: <1313716221-20136-2-git-send-email-horms@verge.net.au>
On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
> 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>
Good idea, thanks for this!
/ magnus
^ permalink raw reply
* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
From: Magnus Damm @ 2011-08-19 4:27 UTC (permalink / raw)
To: Simon Horman
Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt,
Guennadi Liakhovetski
In-Reply-To: <20110819033043.GB12106@verge.net.au>
On Fri, Aug 19, 2011 at 12:30 PM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Aug 19, 2011 at 12:09:50PM +0900, Magnus Damm wrote:
>> On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
>> > Provide separate interrupt handlers which may be used by platforms where
>> > SDHI has three interrupt sources.
>> >
>> > This patch also removes the commented-out handling of CRC and other errors.
>> >
>> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>> > Cc: Magnus Damm <magnus.damm@gmail.com>
>> > Signed-off-by: Simon Horman <horms@verge.net.au>
>> >
>> > ---
>>
>> > +irqreturn_t tmio_mmc_irq(int irq, void *devid)
>> > +{
>> > + struct tmio_mmc_host *host = devid;
>> > + unsigned int ireg, status;
>> > +
>> > + pr_debug("MMC IRQ begin\n");
>> > +
>> > + tmio_mmc_card_irq_status(host, &ireg, &status);
>> > + if (__tmio_mmc_card_detect_irq(host, ireg, status))
>> > + return IRQ_HANDLED;
>> > + if (__tmio_mmc_sdcard_irq(host, ireg, status))
>> > + return IRQ_HANDLED;
>> > +
>> > + tmio_mmc_sdio_irq(irq, devid);
>> >
>> > -out:
>> > return IRQ_HANDLED;
>> > }
>> > EXPORT_SYMBOL(tmio_mmc_irq);
>>
>> Is there any particular reason for returning early in this interrupt
>> handler? By returning early I mean the "if ... return IRQ_HANDLED"
>> cases above.
>>
>> I realize the old ISR code in the driver does just this, so if the
>> goal is to stay compatible then I guess we should keep this behavior.
>> >From my point of view it usually makes more sense to try to handle all
>> events that may be associated with the IRQ.
>
> My original post had the behaviour that you suggest but Guennadi
> indicted that he would be much more comfortable with keeping the original
> behaviour as it is know to work on a wide range of hardware.
I see, thanks for your patience...
So I may remember this wrong, but for the 3 different interrupt
sources I believe that the SDIO IRQ code was added by Arnd for one of
the Renesas SDHI platforms. Back then Ian disliked supporting more
than a single interrupt source, so for that reason the SDIO IRQ code
was added on top of the common interrupt handler. We had no
documentation either, so it was added in a rather random way. I recall
the SDIO IRQ being handled before the other interrupt types in the
common handler not last, but that's not very important. Anyway, with
the fact that SDIO IRQ support was added for SDHI platforms in mind
then we can assume that other platforms won't need it. Not sure if
this fact will improve our situation or not.
As for the two remaining interrupts, I believe they share hardware
registers somehow. I guess I'm OK keeping the original behavior
somehow, but I still believe it's incorrect. It may not matter very
much though since it's rather unlikely that hotplug insertion or eject
coincides with the data IRQs.
Thanks for your help!
/ magnus
^ permalink raw reply
* Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum
From: Magnus Damm @ 2011-08-19 4:16 UTC (permalink / raw)
To: Simon Horman
Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt,
Guennadi Liakhovetski
In-Reply-To: <1313716221-20136-5-git-send-email-horms@verge.net.au>
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
^ permalink raw reply
* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
From: Simon Horman @ 2011-08-19 3:30 UTC (permalink / raw)
To: Magnus Damm
Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt,
Guennadi Liakhovetski
In-Reply-To: <CANqRtoTprsGo4yAMtfLGXjd+KQffM5f9rNnXXMFWxg-aL3nyBA@mail.gmail.com>
On Fri, Aug 19, 2011 at 12:09:50PM +0900, Magnus Damm wrote:
> On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
> > Provide separate interrupt handlers which may be used by platforms where
> > SDHI has three interrupt sources.
> >
> > This patch also removes the commented-out handling of CRC and other errors.
> >
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Magnus Damm <magnus.damm@gmail.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> >
> > ---
>
> > +irqreturn_t tmio_mmc_irq(int irq, void *devid)
> > +{
> > + struct tmio_mmc_host *host = devid;
> > + unsigned int ireg, status;
> > +
> > + pr_debug("MMC IRQ begin\n");
> > +
> > + tmio_mmc_card_irq_status(host, &ireg, &status);
> > + if (__tmio_mmc_card_detect_irq(host, ireg, status))
> > + return IRQ_HANDLED;
> > + if (__tmio_mmc_sdcard_irq(host, ireg, status))
> > + return IRQ_HANDLED;
> > +
> > + tmio_mmc_sdio_irq(irq, devid);
> >
> > -out:
> > return IRQ_HANDLED;
> > }
> > EXPORT_SYMBOL(tmio_mmc_irq);
>
> Is there any particular reason for returning early in this interrupt
> handler? By returning early I mean the "if ... return IRQ_HANDLED"
> cases above.
>
> I realize the old ISR code in the driver does just this, so if the
> goal is to stay compatible then I guess we should keep this behavior.
> >From my point of view it usually makes more sense to try to handle all
> events that may be associated with the IRQ.
My original post had the behaviour that you suggest but Guennadi
indicted that he would be much more comfortable with keeping the original
behaviour as it is know to work on a wide range of hardware.
^ permalink raw reply
* Re: [PATCH] mmc: sdhi: initialise mmc_data->flags before use
From: Simon Horman @ 2011-08-19 3:26 UTC (permalink / raw)
To: Magnus Damm; +Cc: linux-mmc, linux-sh, Chris Ball, Guennadi Liakhovetski
In-Reply-To: <CANqRtoRx5MC-smUjuYBgpstcm49_PqwXqG=YEe+dkqVte7eQ9w@mail.gmail.com>
On Fri, Aug 19, 2011 at 12:04:06PM +0900, Magnus Damm wrote:
> On Fri, Aug 19, 2011 at 10:07 AM, Simon Horman <horms@verge.net.au> wrote:
> > This corrects a logic error that I introduced in
> > "mmc: sdhi: Add write16_hook"
> >
> > Reported-by: Magnus Damm <magnus.damm@gmail.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> > ---
> > drivers/mmc/host/sh_mobile_sdhi.c | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
>
> Hi Simon,
>
> Thanks for providing a fix for this issue. Correct me if I'm wrong,
> but I believe this patch is suitable for 3.1-rc. So it would be great
> to have it merged rather soon as a fix instead of delaying it to 3.2.
Yes, I agree.
^ permalink raw reply
* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
From: Magnus Damm @ 2011-08-19 3:09 UTC (permalink / raw)
To: Simon Horman
Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt,
Guennadi Liakhovetski
In-Reply-To: <1313716221-20136-3-git-send-email-horms@verge.net.au>
On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@verge.net.au> wrote:
> Provide separate interrupt handlers which may be used by platforms where
> SDHI has three interrupt sources.
>
> This patch also removes the commented-out handling of CRC and other errors.
>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
>
> ---
> +irqreturn_t tmio_mmc_irq(int irq, void *devid)
> +{
> + struct tmio_mmc_host *host = devid;
> + unsigned int ireg, status;
> +
> + pr_debug("MMC IRQ begin\n");
> +
> + tmio_mmc_card_irq_status(host, &ireg, &status);
> + if (__tmio_mmc_card_detect_irq(host, ireg, status))
> + return IRQ_HANDLED;
> + if (__tmio_mmc_sdcard_irq(host, ireg, status))
> + return IRQ_HANDLED;
> +
> + tmio_mmc_sdio_irq(irq, devid);
>
> -out:
> return IRQ_HANDLED;
> }
> EXPORT_SYMBOL(tmio_mmc_irq);
Is there any particular reason for returning early in this interrupt
handler? By returning early I mean the "if ... return IRQ_HANDLED"
cases above.
I realize the old ISR code in the driver does just this, so if the
goal is to stay compatible then I guess we should keep this behavior.
From my point of view it usually makes more sense to try to handle all
events that may be associated with the IRQ.
Thanks,
/ magnus
^ permalink raw reply
* Re: [PATCH] mmc: sdhi: initialise mmc_data->flags before use
From: Magnus Damm @ 2011-08-19 3:04 UTC (permalink / raw)
To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Guennadi Liakhovetski
In-Reply-To: <1313716027-19470-1-git-send-email-horms@verge.net.au>
On Fri, Aug 19, 2011 at 10:07 AM, Simon Horman <horms@verge.net.au> wrote:
> This corrects a logic error that I introduced in
> "mmc: sdhi: Add write16_hook"
>
> Reported-by: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> ---
> drivers/mmc/host/sh_mobile_sdhi.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
Hi Simon,
Thanks for providing a fix for this issue. Correct me if I'm wrong,
but I believe this patch is suitable for 3.1-rc. So it would be great
to have it merged rather soon as a fix instead of delaying it to 3.2.
Thanks,
/ magnus
^ permalink raw reply
* [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum
From: Simon Horman @ 2011-08-19 1:10 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm,
Simon Horman
In-Reply-To: <1313716221-20136-1-git-send-email-horms@verge.net.au>
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
^ permalink raw reply related
* [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
From: Simon Horman @ 2011-08-19 1:10 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm,
Simon Horman
In-Reply-To: <1313716221-20136-1-git-send-email-horms@verge.net.au>
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>
---
v6
* As discussed with Guennadi Liakhovetski:
- The SDCARD/SDIO change in v5 was not implemented as described.
And the logic wasn't fully described in any case. This
version (hope to!) implement the following logic:
1) 1 IRQ: only resource #0 (CARD_DETECT, use tmio_mmc_irq())
2) 2 or 3 IRQs: compulsory SDCARD and any further IRQs: use respective
specialised ISRs.
v5
* As suggested by Guennadi Liakhovetski:
- Allow only SH_MOBILE_SDHI_IRQ_SDCARD and
SH_MOBILE_SDHI_IRQ_SDIO to be specified in platform data
v4
* As suggested by Guennadi Liakhovetski:
- Correct inverted values of SH_MOBILE_SDHI_IRQ_SDCARD and
SH_MOBILE_SDHI_IRQ_CARD_DETECT
v3
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* As suggested by Guennadi Liakhovetski:
- Merge in patch "mmc: sdhi: Add defines for platform irq indexes"
- Use an enum instead of defines for irq indexes
v2
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* Make use of defines provided by
"mmc: sdhi: Add defines for platform irq indexes"
* As suggested by Guennadi Liakhovetski:
- Don't use a loop to initialise irq handlers, the unrolled version
is easier on the eyes (and exactly the same number of lines of code!)
---
drivers/mmc/host/sh_mobile_sdhi.c | 65 +++++++++++++++++++++++------------
include/linux/mmc/sh_mobile_sdhi.h | 7 ++++
2 files changed, 50 insertions(+), 22 deletions(-)
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 774f643..2fa7bbc 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -96,7 +96,8 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
struct tmio_mmc_host *host;
char clk_name[8];
- int i, irq, ret;
+ int irq, ret, irq_count = 0;
+ irqreturn_t (*f)(int irq, void *devid);
priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
if (priv = NULL) {
@@ -153,27 +154,39 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
if (ret < 0)
goto eprobe;
- for (i = 0; i < 3; i++) {
- irq = platform_get_irq(pdev, i);
- if (irq < 0) {
- if (i) {
- continue;
- } else {
- ret = irq;
- goto eirq;
- }
- }
- ret = request_irq(irq, tmio_mmc_irq, 0,
+ /* Allow a single IRQ resource #0 (CARD_DETECT) which will
+ * use tmio_mmc_irq() or;
+ * Allow 2 or 3 IRQ resources in which case SDCARD is required
+ * and specialised ISRs are used.
+ */
+ irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+ if (irq >= 0) {
+ irq_count++;
+ ret = request_irq(irq, tmio_mmc_sdcard_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_sdcard;
}
+
+ ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+ if (irq_count && irq >= 0) {
+ irq_count++;
+ ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
+ dev_name(&pdev->dev), host);
+ if (ret)
+ goto eirq_sdio;
+ } else if (!irq_count)
+ goto eirq_sdio;
+
+ ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+ if (irq >= 0) {
+ f = irq_count ? 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;
+ } else if (irq_count < 2)
+ 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 +194,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_SDIO);
+ if (irq >= 0)
+ free_irq(irq, host);
+eirq_sdio:
+ irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+ if (irq >= 0)
+ free_irq(irq, host);
+eirq_sdcard:
tmio_mmc_host_remove(host);
eprobe:
clk_disable(priv->clk);
@@ -203,7 +224,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
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox