sdio: add quirk for spurious SDIO IRQ

From: Per Forlin <[email protected]>

Daniel Drake reported an issue in the libertas sdio client that was triggered by the sdio_single_irq functionality. His SDIO device seems to raise an interrupt even though there are no bits set in the CCCR_INTx register. This behaviour is not supported by the sdio_single_irq feature nor the SDIO spec. The purpose of the sdio_single_irq feature is to avoid the overhead of checking the CCCR_INTx registers, this result in no error handling of the case if there is a pending IRQ with none CCCR_INTx bits set.

This patchset adds a quirk to support this spurious IRQ issue and also report a warning if an SDIO interrupt is raised but none CCCR_INTx bits are set.

Changes since v1:
* Replace public sdio function to enable disable SDIO single IRQ function with a quirk

Per Forlin (2):
sdio: add quirk to handle pending IRQ in case of none CCCR_INTx bits
sdio: report error if pending IRQ but none CCCR_INTx bits

 drivers/mmc/core/sdio_irq.c |    9 +++++++--
 include/linux/mmc/card.h    |    1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

1.7.4.1

From: Per Forlin <[email protected]>

Add quirk MMC_QUIRK_SDIO_IRQ_CCCR_INTX_0 to handle SDIO device that may have pending IRQ when none bits are set in CCCR_INTx.

This IRQ issue was identified in the libertas sdio driver where the SDIO device seems to raise an interrupt even if there were none function bits in CCCR_INTx set. SDIO single irq feature will call irq handler directly in case of only one interrupt registered. It will not check the CCCR_INTx register and miss if the register is 0 and call the irq handler anyway. This IRQ issue is not defined by the SDIO spec.
Use this quirk to work around this SDIO IRQ issue for a specific device.

---
 drivers/mmc/core/sdio_irq.c |    4 ++--
 include/linux/mmc/card.h    |    1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

index 03ead02..df1e946 100644
@@ -204,7 +204,8 @@ static void sdio_single_irq_set(struct mmc_card *card)
 	int i;
 
 	card->sdio_single_irq = NULL;
-	if ((card->host->caps & MMC_CAP_SDIO_IRQ) &&
+	if (!(card->quirks & MMC_QUIRK_SDIO_IRQ_CCCR_INTX_0) &&
+	    (card->host->caps & MMC_CAP_SDIO_IRQ) &&
 	    card->host->sdio_irqs == 1)
 		for (i = 0; i < card->sdio_funcs; i++) {
 		       func = card->sdio_func[i];

@@ -301,4 +302,3 @@ int sdio_release_irq(struct sdio_func *func)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(sdio_release_irq);
-

index 7b4fd7b..852021c 100644
@@ -175,6 +175,7 @@ struct mmc_card {
 #define MMC_QUIRK_DISABLE_CD	(1<<5)		/* disconnect CD/DAT[3] resistor */
 #define MMC_QUIRK_INAND_CMD38	(1<<6)		/* iNAND devices have broken CMD38 */
 #define MMC_QUIRK_BLK_NO_CMD23	(1<<7)		/* Avoid CMD23 for regular multiblock */
+#define MMC_QUIRK_SDIO_IRQ_CCCR_INTX_0 (1<<7)	/* SDIO card has IRQ even if CCCR_INTx is 0 */
 
 	unsigned int		erase_size;	/* erase size in sectors */
  	unsigned int		erase_shift;	/* if erase unit is power 2 */

1.7.4.1

From: Per Forlin <[email protected]>

Return error in case of pending IRQ but none functions bits in CCCR_INTx are set.

---
 drivers/mmc/core/sdio_irq.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

index df1e946..4288d92 100644
@@ -50,6 +50,11 @@ static int process_sdio_pending_irqs(struct mmc_card *card)
 		return ret;
 	}
 
+	if (!pending) {
+		printk(KERN_WARNING "%s: pending IRQ but none function bits\n",
+		       mmc_card_id(card));
+		ret = -EINVAL;
+	}
 	count = 0;
 	for (i = 1; i <= 7; i++) {
 		if (pending & (1 << i)) {

1.7.4.1

Thanks for your observation.
Typo, it should be (1 << 8)
I'll update the patch but I don't plan post a new version yet. The root cause issue in the libertas is resolved. If the fix turns out to be needed later on I resend it.

Thanks,
Per

Given that the issue can be fixed locally to the libertas driver, I'd suggest not merging this series until truly unfixable issues come up, as Daniel said.

Nicolas

Hi,

Yep, agreed. Dan, please go ahead and submit the libertas patch and let us know if there are any problems getting it in for 3.0. (Ideally by the next -rc!)

Thanks very much, everyone,

- Chris.

Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child

I agree too.

Thanks,
Per