mmc: tmio: Fix race condition resulting in spurious interrupts

There is a race condition in the tmio_mmc_irq() interrupt handler,
caused by the presence of a while loop, which results in warnings of
spurious interrupts. This was found on an HP iPAQ hx4700 whose HTC
ASIC3 reportedly incorporates the Toshiba TC6380AF controller.

Towards the end of a multiple read (CMD18) operation the handler clears
the final RXRDY status bit in the first loop iteration, sees the DATAEND
status bit at the bottom of the loop, and so clears the DATAEND status
bit in the second loop iteration. However the DATAEND interrupt is still
queued in the system somewhere and can't be delivered until the handler
has returned. This second interrupt is then reported as spurious in the
next call to the handler. Likewise for single read (CMD17) operations.
And something similar occurs for multiple write (CMD25) and single write
(CMD24) operations, where CMDRESPEND and TXRQ status bits are cleared in
a single call.

In these cases the interrupt handler clears two separate interrupts when
it should only clear the one interrupt for which it was invoked. The fix
is to remove the while loop.

Signed-off-by: Paul Parsons <lost.distance@yahoo.com>
Signed-off-by: Chris Ball <cjb@laptop.org>

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 4d17807..f7dd3b1 100644 (file)
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -569,58 +569,46 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
        pr_debug_status(status);
        pr_debug_status(ireg);
 
-       if (!ireg) {
-               tmio_mmc_disable_mmc_irqs(host, status & ~irq_mask);
-
-               pr_warning("tmio_mmc: Spurious irq, disabling! "
-                       "0x%08x 0x%08x 0x%08x\n", status, irq_mask, ireg);
-               pr_debug_status(status);
-
+       /* Card insert / remove attempts */
+       if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
+               tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_CARD_INSERT |
+                       TMIO_STAT_CARD_REMOVE);
+               mmc_detect_change(host->mmc, msecs_to_jiffies(100));
                goto out;
        }
 
-       while (ireg) {
-               /* Card insert / remove attempts */
-               if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
-                       tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_CARD_INSERT |
-                               TMIO_STAT_CARD_REMOVE);
-                       mmc_detect_change(host->mmc, msecs_to_jiffies(100));
-               }
-
-               /* CRC and other errors */
-/*             if (ireg & TMIO_STAT_ERR_IRQ)
- *                     handled |= tmio_error_irq(host, irq, stat);
+       /* CRC and other errors */
+/*     if (ireg & TMIO_STAT_ERR_IRQ)
+ *             handled |= tmio_error_irq(host, irq, stat);
  */
 
-               /* 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);
-               }
-
-               /* 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);
-               }
-
-               /* Data transfer completion */
-               if (ireg & TMIO_STAT_DATAEND) {
-                       tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
-                       tmio_mmc_data_irq(host);
-               }
+       /* 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;
+       }
 
-               /* Check status - keep going until we've handled it all */
-               status = sd_ctrl_read32(host, CTL_STATUS);
-               irq_mask = sd_ctrl_read32(host, CTL_IRQ_MASK);
-               ireg = status & TMIO_MASK_IRQ & ~irq_mask;
+       /* 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;
+       }
 
-               pr_debug("Status at end of loop: %08x\n", status);
-               pr_debug_status(status);
+       /* Data transfer completion */
+       if (ireg & TMIO_STAT_DATAEND) {
+               tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
+               tmio_mmc_data_irq(host);
+               goto out;
        }
-       pr_debug("MMC IRQ end\n");
+
+       pr_warning("tmio_mmc: Spurious irq, disabling! "
+               "0x%08x 0x%08x 0x%08x\n", status, irq_mask, ireg);
+       pr_debug_status(status);
+       tmio_mmc_disable_mmc_irqs(host, status & ~irq_mask);
 
 out:
        return IRQ_HANDLED;