* [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum
From: Simon Horman @ 2011-08-17 10:59 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Guennadi Liakhovetski, Magnus Damm, Paul Mundt,
Simon Horman
In-Reply-To: <1313578756-10413-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-17 10:59 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Guennadi Liakhovetski, Magnus Damm, Paul Mundt,
Simon Horman
In-Reply-To: <1313578756-10413-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>
---
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 | 59 ++++++++++++++++++++++-------------
include/linux/mmc/sh_mobile_sdhi.h | 7 ++++
2 files changed, 44 insertions(+), 22 deletions(-)
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 774f643..73ecadb 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,32 @@ 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;
}
+
+ 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 +188,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 +218,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
* [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
From: Simon Horman @ 2011-08-17 10:59 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Guennadi Liakhovetski, Magnus Damm, Paul Mundt,
Simon Horman
In-Reply-To: <1313578756-10413-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-17 10:59 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Guennadi Liakhovetski, Magnus Damm, Paul Mundt,
Simon Horman
In-Reply-To: <1313578756-10413-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 v5] mmc: tmio, sdhi: provide multiple irq handlers
From: Simon Horman @ 2011-08-17 10:59 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Guennadi Liakhovetski, Magnus Damm, Paul Mundt,
Simon Horman
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
^ permalink raw reply
* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
From: Simon Horman @ 2011-08-17 10:46 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Magnus Damm
In-Reply-To: <Pine.LNX.4.64.1108171154270.18317@axis700.grange>
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.
^ permalink raw reply
* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
From: Guennadi Liakhovetski @ 2011-08-17 10:06 UTC (permalink / raw)
To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Magnus Damm
In-Reply-To: <20110817094908.GB3561@verge.net.au>
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/
^ permalink raw reply
* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
From: Simon Horman @ 2011-08-17 9:49 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Magnus Damm
In-Reply-To: <Pine.LNX.4.64.1108171010350.18317@axis700.grange>
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() ?
^ permalink raw reply
* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
From: Guennadi Liakhovetski @ 2011-08-17 8:20 UTC (permalink / raw)
To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Magnus Damm
In-Reply-To: <1313542255-31662-4-git-send-email-horms@verge.net.au>
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/
^ permalink raw reply
* [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum
From: Simon Horman @ 2011-08-17 0:50 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm,
Simon Horman
In-Reply-To: <1313542255-31662-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-17 0:50 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm,
Simon Horman
In-Reply-To: <1313542255-31662-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>
---
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
^ permalink raw reply related
* [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
From: Simon Horman @ 2011-08-17 0:50 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm,
Simon Horman
In-Reply-To: <1313542255-31662-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-17 0:50 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm,
Simon Horman
In-Reply-To: <1313542255-31662-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 v4] mmc: tmio, sdhi: provide multiple irq handlers
From: Simon Horman @ 2011-08-17 0:50 UTC (permalink / raw)
To: linux-mmc, linux-sh
Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm,
Simon Horman
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
^ permalink raw reply
* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
From: Simon Horman @ 2011-08-16 13:45 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm
In-Reply-To: <Pine.LNX.4.64.1108161438530.13913@axis700.grange>
On Tue, Aug 16, 2011 at 02:40:25PM +0200, Guennadi Liakhovetski wrote:
> On Tue, 16 Aug 2011, Simon Horman wrote:
>
> > On Tue, Aug 16, 2011 at 01:11:42PM +0200, Guennadi Liakhovetski wrote:
> > > On Tue, 16 Aug 2011, Simon Horman wrote:
[snip]
> > > Sorry, I still don't understand why you check "CARD_DETECT" and request
> > > "sdcard_irq." Am I missing something or was my comment not clear enough in
> > > the previous review?
> > >
> > > > + if (ret)
> > > > + goto eirq_card_detect;
> > > > + } else if (multi_irq)
> > > > + goto eirq_card_detect;
> > >
> > > ? Sorry, have you maybe missed my review?
> > >
> > > > +
> > > > + ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > > > + if (irq < 0)
> > > > + goto eirq_sdcard;
> > > > + f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> > >
> > > Same here
> >
> > Sorry, that is a bug.
> >
> > As the code seems to work on my board I think it should be fixed by
> > inverting the values of SH_MOBILE_SDHI_IRQ_SDCARD and
> > SH_MOBILE_SDHI_IRQ_CARD_DETECT.
> >
> > I will check that and repost.
>
> Right, SDIO works, /proc/interrupts has
>
> 96: 5 sh7372-intca-level sh_mobile_sdhi.0
> 97: 4116138 sh7372-intca-level sh_mobile_sdhi.0
> 98: 44822 sh7372-intca-level sh_mobile_sdhi.0
>
> So, indeed, #0 is card-detect, #1 is sdcard, #2 is sdio, your enum is
> wrong and request_irq() swapped ISRs compensate for that mistake.
Thanks.
I swapped around the enum, made the appropriate changes elsewhere
and things do seem to work correctly. Its a bit late now so I'll hold
off on reposting until tomorrow.
^ permalink raw reply
* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
From: Guennadi Liakhovetski @ 2011-08-16 12:40 UTC (permalink / raw)
To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm
In-Reply-To: <20110816115141.GE3110@verge.net.au>
On Tue, 16 Aug 2011, Simon Horman wrote:
> On Tue, Aug 16, 2011 at 01:11:42PM +0200, Guennadi Liakhovetski wrote:
> > On Tue, 16 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>
> > >
> > > ---
> > >
> > > 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..d792705 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_CARD_DETECT);
> > > + if (irq >= 0) {
> > > + multi_irq = true;
> > > + ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > > + dev_name(&pdev->dev), host);
> >
> > Sorry, I still don't understand why you check "CARD_DETECT" and request
> > "sdcard_irq." Am I missing something or was my comment not clear enough in
> > the previous review?
> >
> > > + if (ret)
> > > + goto eirq_card_detect;
> > > + } else if (multi_irq)
> > > + goto eirq_card_detect;
> >
> > ? Sorry, have you maybe missed my review?
> >
> > > +
> > > + ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > > + if (irq < 0)
> > > + goto eirq_sdcard;
> > > + f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> >
> > Same here
>
> Sorry, that is a bug.
>
> As the code seems to work on my board I think it should be fixed by
> inverting the values of SH_MOBILE_SDHI_IRQ_SDCARD and
> SH_MOBILE_SDHI_IRQ_CARD_DETECT.
>
> I will check that and repost.
Right, SDIO works, /proc/interrupts has
96: 5 sh7372-intca-level sh_mobile_sdhi.0
97: 4116138 sh7372-intca-level sh_mobile_sdhi.0
98: 44822 sh7372-intca-level sh_mobile_sdhi.0
So, indeed, #0 is card-detect, #1 is sdcard, #2 is sdio, your enum is
wrong and request_irq() swapped ISRs compensate for that mistake.
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-16 12:23 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-mmc, linux-sh, Chris Ball, Guennadi Liakhovetski,
Magnus Damm
In-Reply-To: <20110816113620.GC3110@verge.net.au>
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.
^ permalink raw reply
* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
From: Simon Horman @ 2011-08-16 11:51 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm
In-Reply-To: <Pine.LNX.4.64.1108161232520.13913@axis700.grange>
On Tue, Aug 16, 2011 at 01:11:42PM +0200, Guennadi Liakhovetski wrote:
> On Tue, 16 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>
> >
> > ---
> >
> > 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..d792705 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_CARD_DETECT);
> > + if (irq >= 0) {
> > + multi_irq = true;
> > + ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > + dev_name(&pdev->dev), host);
>
> Sorry, I still don't understand why you check "CARD_DETECT" and request
> "sdcard_irq." Am I missing something or was my comment not clear enough in
> the previous review?
>
> > + if (ret)
> > + goto eirq_card_detect;
> > + } else if (multi_irq)
> > + goto eirq_card_detect;
>
> ? Sorry, have you maybe missed my review?
>
> > +
> > + ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > + if (irq < 0)
> > + goto eirq_sdcard;
> > + f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
>
> Same here
Sorry, that is a bug.
As the code seems to work on my board I think it should be fixed by
inverting the values of SH_MOBILE_SDHI_IRQ_SDCARD and
SH_MOBILE_SDHI_IRQ_CARD_DETECT.
I will check that and repost.
^ permalink raw reply
* Re: [PATCH 2/5] mmc: tmio: Provide separate interrupt handlers
From: Simon Horman @ 2011-08-16 11:45 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-sh, linux-mmc, Chris Ball, Magnus Damm, Paul Mundt
In-Reply-To: <Pine.LNX.4.64.1108161250470.13913@axis700.grange>
On Tue, Aug 16, 2011 at 01:13:06PM +0200, Guennadi Liakhovetski wrote:
> On Tue, 16 Aug 2011, Simon Horman wrote:
>
> > On Tue, Aug 16, 2011 at 09:41:52AM +0200, Guennadi Liakhovetski wrote:
> > > On Tue, 16 Aug 2011, Simon Horman wrote:
>
> [snip]
>
> > > > +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);
> > > > + __tmio_mmc_card_detect_irq(host, ireg, status);
> > >
> > > Same here - I would return, if a card hot-plug event occurred.
> >
> > Will do.
> >
> > > > + __tmio_mmc_sdcard_irq(host, ireg, status);
> > >
> > > Ditto
> > >
> > > > +
> > > > + tmio_mmc_sdio_irq(irq, devid);
> > >
> > > Any specific reason, why you now process SDIO IRQs last?
> >
> > I believe this is in keeping with the ordering implied by original code.
>
> I believe it's not. The original version did SDIO first, then hotplug,
> then normal IO.
My reading of the original code is that SDIO was the lowest priority
although its code appeared near the top of tmio_mmc_irq().
irqreturn_t tmio_mmc_irq(int irq, void *devid)
{
...
status = sd_ctrl_read32(host, CTL_STATUS);
irq_mask = sd_ctrl_read32(host, CTL_IRQ_MASK);
ireg = status & TMIO_MASK_IRQ & ~irq_mask;
sdio_ireg = 0;
if (!ireg pdata->flags & TMIO_MMC_SDIO_IRQ) {
/* Handle SDIO Interrupt */
...
goto out;
}
/* Handle Card detect Interrupts */
/* Handle other Interrupts */
...
}
> I'm not necessarily saying, that the original code was
> correct or better, I'm just saying, it was different. As for which one we
> should prefer... I think, I'd check for hotplug first: if the card is
> removed, no reason to try to communicate with it. And we have to first
> report a new card, before reporting any IRQs from it. But then - IO or
> SDIO as second? Well, since SDIO IRQs are asynchronous, it shouldn't be a
> big problem for them to wait a bit, whereas normal IO IRQs are card's
> response to host's actions, so, the originator might want to know the
> result before processing any asynchronous events. So, I actually like your
> ordering better... But I'll give it a short spin with SDIO, unless you do
> it yourself.
I intend to test my code with SDIO, however I don't have access to hardware
at this exact moment. So if you could do so, that would be great.
^ permalink raw reply
* Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index
From: Simon Horman @ 2011-08-16 11:36 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-mmc, linux-sh, Chris Ball, Guennadi Liakhovetski,
Magnus Damm
In-Reply-To: <20110816111350.GC27653@trinity.fluff.org>
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.
^ permalink raw reply
* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
From: Simon Horman @ 2011-08-16 11:35 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm
In-Reply-To: <Pine.LNX.4.64.1108161226410.13913@axis700.grange>
On Tue, Aug 16, 2011 at 01:14:01PM +0200, Guennadi Liakhovetski wrote:
> On Tue, 16 Aug 2011, Simon Horman wrote:
>
> > Provide separate interrupt handlers which may be used by platforms where
> > SDHI has three interrupt sources.
> >
> > This patch also removes the commented-out handling of CRC and other errors.
> >
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Magnus Damm <magnus.damm@gmail.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> >
> > ---
> >
> > * SDCARD portion tested on AP4/Mackerel
> > * SDIO portion untested
> >
> > v3
> > * Rebase for updated "mmc: tmio: Cache interrupt masks"
> > * As suggested by Guennadi Liakhovetski
> > - Do not alter logic to handle more than one interupt at once
> > - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()
> >
> > v2
> > * As suggested by Guennadi Liakhovetski
> > - Combine 3 patches into one
> > - Reduce the number of __tmio_..._irq() functions
> > - Rename "...card_access..." functions as "...sdcard..."
> > ---
> > drivers/mmc/host/tmio_mmc.h | 3 +
> > drivers/mmc/host/tmio_mmc_pio.c | 131 ++++++++++++++++++++++++--------------
> > 2 files changed, 86 insertions(+), 48 deletions(-)
> >
> > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> > index 1cf8db5..3020f98 100644
> > --- a/drivers/mmc/host/tmio_mmc.h
> > +++ b/drivers/mmc/host/tmio_mmc.h
> > @@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
> > void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
> > void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
> > irqreturn_t tmio_mmc_irq(int irq, void *devid);
> > +irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
> > +irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
> > +irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
> >
> > static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
> > unsigned long *flags)
> > diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> > index f0c7830..37adfb2 100644
> > --- a/drivers/mmc/host/tmio_mmc_pio.c
> > +++ b/drivers/mmc/host/tmio_mmc_pio.c
> > @@ -545,44 +545,20 @@ out:
> > spin_unlock(&host->lock);
> > }
> >
> > -irqreturn_t tmio_mmc_irq(int irq, void *devid)
> > +static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
> > + int *ireg, int *status)
> > {
> > - struct tmio_mmc_host *host = devid;
> > - struct mmc_host *mmc = host->mmc;
> > - struct tmio_mmc_data *pdata = host->pdata;
> > - unsigned int ireg, status;
> > - unsigned int sdio_ireg, sdio_status;
> > -
> > - pr_debug("MMC IRQ begin\n");
> > -
> > - status = sd_ctrl_read32(host, CTL_STATUS);
> > - ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
> > -
> > - sdio_ireg = 0;
> > - if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
> > - sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
> > - sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
> > - ~host->sdio_irq_mask;
> > + *status = sd_ctrl_read32(host, CTL_STATUS);
> > + *ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
> >
> > - sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
> > -
> > - if (sdio_ireg && !host->sdio_irq_enabled) {
> > - pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
> > - sdio_status, host->sdio_irq_mask, sdio_ireg);
> > - tmio_mmc_enable_sdio_irq(mmc, 0);
> > - goto out;
> > - }
> > -
> > - if (mmc->caps & MMC_CAP_SDIO_IRQ &&
> > - sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
> > - mmc_signal_sdio_irq(mmc);
> > -
> > - if (sdio_ireg)
> > - goto out;
> > - }
> > + pr_debug_status(*status);
> > + pr_debug_status(*ireg);
> > +}
> >
> > - pr_debug_status(status);
> > - pr_debug_status(ireg);
> > +static int __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
> > + int ireg, int status)
> > +{
> > + struct mmc_host *mmc = host->mmc;
> >
> > /* Card insert / remove attempts */
> > if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
> > @@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
> > ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
> > !work_pending(&mmc->detect.work))
> > mmc_detect_change(host->mmc, msecs_to_jiffies(100));
> > - goto out;
> > + return 1;
> > }
> >
> > - /* CRC and other errors */
> > -/* if (ireg & TMIO_STAT_ERR_IRQ)
> > - * handled |= tmio_error_irq(host, irq, stat);
> > - */
> > + return 0;
> > +}
> > +
> > +irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
> > +{
> > + unsigned int ireg, status;
> > + struct tmio_mmc_host *host = devid;
> >
> > + tmio_mmc_card_irq_status(host, &ireg, &status);
> > + __tmio_mmc_card_detect_irq(host, ireg, status);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
> > +
> > +static int __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
> > + int ireg, int status)
> > +{
> > /* Command completion */
> > if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
> > tmio_mmc_ack_mmc_irqs(host,
> > TMIO_STAT_CMDRESPEND |
> > TMIO_STAT_CMDTIMEOUT);
> > tmio_mmc_cmd_irq(host, status);
> > - goto out;
> > + return 1;
> > }
> >
> > /* Data transfer */
> > if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
> > tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
> > tmio_mmc_pio_irq(host);
> > - goto out;
> > + return 1;
> > }
> >
> > /* Data transfer completion */
> > if (ireg & TMIO_STAT_DATAEND) {
> > tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
> > tmio_mmc_data_irq(host);
> > - goto out;
> > + return 1;
> > }
> >
> > - pr_warning("tmio_mmc: Spurious irq, disabling! "
> > - "0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
> > - pr_debug_status(status);
> > - tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
> > + return 0;
> > +}
> > +
> > +irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
> > +{
> > + unsigned int ireg, status;
> > + struct tmio_mmc_host *host = devid;
> > +
> > + tmio_mmc_card_irq_status(host, &ireg, &status);
> > + __tmio_mmc_sdcard_irq(host, ireg, status);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
> > +
> > +irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
> > +{
> > + struct tmio_mmc_host *host = devid;
> > + struct mmc_host *mmc = host->mmc;
> > + struct tmio_mmc_data *pdata = host->pdata;
> > + unsigned int ireg, status;
> > +
> > + if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
> > + return IRQ_HANDLED;
> > +
> > + status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
> > + ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
> > +
> > + sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
> > +
> > + if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
> > + mmc_signal_sdio_irq(mmc);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +EXPORT_SYMBOL(tmio_mmc_sdio_irq);
> > +
> > +irqreturn_t tmio_mmc_irq(int irq, void *devid)
> > +{
> > + struct tmio_mmc_host *host = devid;
> > + unsigned int ireg, status;
> > +
> > + pr_debug("MMC IRQ begin\n");
> > +
> > + tmio_mmc_card_irq_status(host, &ireg, &status);
> > + if (__tmio_mmc_card_detect_irq(host, ireg, status))
> > + return IRQ_HANDLED;
> > + if (__tmio_mmc_sdcard_irq(host, ireg, status))
> > + return IRQ_HANDLED;
>
> You use the two above functions as "bool," which is also logical. So, I'd
> also declare them "bool" and return true or false instead of 1 and 0.
Sure, will do.
^ permalink raw reply
* Re: [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
From: Guennadi Liakhovetski @ 2011-08-16 11:14 UTC (permalink / raw)
To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm
In-Reply-To: <1313489486-831-3-git-send-email-horms@verge.net.au>
On Tue, 16 Aug 2011, Simon Horman wrote:
> Provide separate interrupt handlers which may be used by platforms where
> SDHI has three interrupt sources.
>
> This patch also removes the commented-out handling of CRC and other errors.
>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
>
> ---
>
> * SDCARD portion tested on AP4/Mackerel
> * SDIO portion untested
>
> v3
> * Rebase for updated "mmc: tmio: Cache interrupt masks"
> * As suggested by Guennadi Liakhovetski
> - Do not alter logic to handle more than one interupt at once
> - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()
>
> v2
> * As suggested by Guennadi Liakhovetski
> - Combine 3 patches into one
> - Reduce the number of __tmio_..._irq() functions
> - Rename "...card_access..." functions as "...sdcard..."
> ---
> drivers/mmc/host/tmio_mmc.h | 3 +
> drivers/mmc/host/tmio_mmc_pio.c | 131 ++++++++++++++++++++++++--------------
> 2 files changed, 86 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 1cf8db5..3020f98 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
> void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
> void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
> irqreturn_t tmio_mmc_irq(int irq, void *devid);
> +irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
> +irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
> +irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
>
> static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
> unsigned long *flags)
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index f0c7830..37adfb2 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -545,44 +545,20 @@ out:
> spin_unlock(&host->lock);
> }
>
> -irqreturn_t tmio_mmc_irq(int irq, void *devid)
> +static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
> + int *ireg, int *status)
> {
> - struct tmio_mmc_host *host = devid;
> - struct mmc_host *mmc = host->mmc;
> - struct tmio_mmc_data *pdata = host->pdata;
> - unsigned int ireg, status;
> - unsigned int sdio_ireg, sdio_status;
> -
> - pr_debug("MMC IRQ begin\n");
> -
> - status = sd_ctrl_read32(host, CTL_STATUS);
> - ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
> -
> - sdio_ireg = 0;
> - if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
> - sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
> - sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
> - ~host->sdio_irq_mask;
> + *status = sd_ctrl_read32(host, CTL_STATUS);
> + *ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
>
> - sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
> -
> - if (sdio_ireg && !host->sdio_irq_enabled) {
> - pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
> - sdio_status, host->sdio_irq_mask, sdio_ireg);
> - tmio_mmc_enable_sdio_irq(mmc, 0);
> - goto out;
> - }
> -
> - if (mmc->caps & MMC_CAP_SDIO_IRQ &&
> - sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
> - mmc_signal_sdio_irq(mmc);
> -
> - if (sdio_ireg)
> - goto out;
> - }
> + pr_debug_status(*status);
> + pr_debug_status(*ireg);
> +}
>
> - pr_debug_status(status);
> - pr_debug_status(ireg);
> +static int __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
> + int ireg, int status)
> +{
> + struct mmc_host *mmc = host->mmc;
>
> /* Card insert / remove attempts */
> if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
> @@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
> ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
> !work_pending(&mmc->detect.work))
> mmc_detect_change(host->mmc, msecs_to_jiffies(100));
> - goto out;
> + return 1;
> }
>
> - /* CRC and other errors */
> -/* if (ireg & TMIO_STAT_ERR_IRQ)
> - * handled |= tmio_error_irq(host, irq, stat);
> - */
> + return 0;
> +}
> +
> +irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
> +{
> + unsigned int ireg, status;
> + struct tmio_mmc_host *host = devid;
>
> + tmio_mmc_card_irq_status(host, &ireg, &status);
> + __tmio_mmc_card_detect_irq(host, ireg, status);
> +
> + return IRQ_HANDLED;
> +}
> +EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
> +
> +static int __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
> + int ireg, int status)
> +{
> /* Command completion */
> if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
> tmio_mmc_ack_mmc_irqs(host,
> TMIO_STAT_CMDRESPEND |
> TMIO_STAT_CMDTIMEOUT);
> tmio_mmc_cmd_irq(host, status);
> - goto out;
> + return 1;
> }
>
> /* Data transfer */
> if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
> tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
> tmio_mmc_pio_irq(host);
> - goto out;
> + return 1;
> }
>
> /* Data transfer completion */
> if (ireg & TMIO_STAT_DATAEND) {
> tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
> tmio_mmc_data_irq(host);
> - goto out;
> + return 1;
> }
>
> - pr_warning("tmio_mmc: Spurious irq, disabling! "
> - "0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
> - pr_debug_status(status);
> - tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
> + return 0;
> +}
> +
> +irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
> +{
> + unsigned int ireg, status;
> + struct tmio_mmc_host *host = devid;
> +
> + tmio_mmc_card_irq_status(host, &ireg, &status);
> + __tmio_mmc_sdcard_irq(host, ireg, status);
> +
> + return IRQ_HANDLED;
> +}
> +EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
> +
> +irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
> +{
> + struct tmio_mmc_host *host = devid;
> + struct mmc_host *mmc = host->mmc;
> + struct tmio_mmc_data *pdata = host->pdata;
> + unsigned int ireg, status;
> +
> + if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
> + return IRQ_HANDLED;
> +
> + status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
> + ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
> +
> + sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
> +
> + if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
> + mmc_signal_sdio_irq(mmc);
> +
> + return IRQ_HANDLED;
> +}
> +EXPORT_SYMBOL(tmio_mmc_sdio_irq);
> +
> +irqreturn_t tmio_mmc_irq(int irq, void *devid)
> +{
> + struct tmio_mmc_host *host = devid;
> + unsigned int ireg, status;
> +
> + pr_debug("MMC IRQ begin\n");
> +
> + tmio_mmc_card_irq_status(host, &ireg, &status);
> + if (__tmio_mmc_card_detect_irq(host, ireg, status))
> + return IRQ_HANDLED;
> + if (__tmio_mmc_sdcard_irq(host, ireg, status))
> + return IRQ_HANDLED;
You use the two above functions as "bool," which is also logical. So, I'd
also declare them "bool" and return true or false instead of 1 and 0.
> +
> + tmio_mmc_sdio_irq(irq, devid);
>
> -out:
> return IRQ_HANDLED;
> }
> EXPORT_SYMBOL(tmio_mmc_irq);
> --
> 1.7.5.4
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index
From: Ben Dooks @ 2011-08-16 11:13 UTC (permalink / raw)
To: Simon Horman
Cc: linux-mmc, linux-sh, Chris Ball, Guennadi Liakhovetski,
Magnus Damm
In-Reply-To: <1313489486-831-5-git-send-email-horms@verge.net.au>
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.
^ permalink raw reply
* Re: [PATCH 2/5] mmc: tmio: Provide separate interrupt handlers
From: Guennadi Liakhovetski @ 2011-08-16 11:13 UTC (permalink / raw)
To: Simon Horman; +Cc: linux-sh, linux-mmc, Chris Ball, Magnus Damm, Paul Mundt
In-Reply-To: <20110816075919.GA12097@verge.net.au>
On Tue, 16 Aug 2011, Simon Horman wrote:
> On Tue, Aug 16, 2011 at 09:41:52AM +0200, Guennadi Liakhovetski wrote:
> > On Tue, 16 Aug 2011, Simon Horman wrote:
[snip]
> > > +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);
> > > + __tmio_mmc_card_detect_irq(host, ireg, status);
> >
> > Same here - I would return, if a card hot-plug event occurred.
>
> Will do.
>
> > > + __tmio_mmc_sdcard_irq(host, ireg, status);
> >
> > Ditto
> >
> > > +
> > > + tmio_mmc_sdio_irq(irq, devid);
> >
> > Any specific reason, why you now process SDIO IRQs last?
>
> I believe this is in keeping with the ordering implied by original code.
I believe it's not. The original version did SDIO first, then hotplug,
then normal IO. I'm not necessarily saying, that the original code was
correct or better, I'm just saying, it was different. As for which one we
should prefer... I think, I'd check for hotplug first: if the card is
removed, no reason to try to communicate with it. And we have to first
report a new card, before reporting any IRQs from it. But then - IO or
SDIO as second? Well, since SDIO IRQs are asynchronous, it shouldn't be a
big problem for them to wait a bit, whereas normal IO IRQs are card's
response to host's actions, so, the originator might want to know the
result before processing any asynchronous events. So, I actually like your
ordering better... But I'll give it a short spin with SDIO, unless you do
it yourself.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
From: Guennadi Liakhovetski @ 2011-08-16 11:11 UTC (permalink / raw)
To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm
In-Reply-To: <1313489486-831-4-git-send-email-horms@verge.net.au>
On Tue, 16 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>
>
> ---
>
> 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..d792705 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_CARD_DETECT);
> + if (irq >= 0) {
> + multi_irq = true;
> + ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> + dev_name(&pdev->dev), host);
Sorry, I still don't understand why you check "CARD_DETECT" and request
"sdcard_irq." Am I missing something or was my comment not clear enough in
the previous review?
> + if (ret)
> + goto eirq_card_detect;
> + } else if (multi_irq)
> + goto eirq_card_detect;
? Sorry, have you maybe missed my review?
> +
> + ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> + if (irq < 0)
> + goto eirq_sdcard;
> + f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
Same here
Thanks
Guennadi
> + ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
> + if (ret)
> + goto eirq_sdcard;
> +
> 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_sdcard:
> + irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> + if (irq >= 0)
> + free_irq(irq, host);
> +eirq_card_detect:
> + 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..5182496 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_SDCARD = 0,
> + SH_MOBILE_SDHI_IRQ_CARD_DETECT,
> + 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/
^ permalink raw reply
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