cw1200: Don't perform SPI transfers in interrupt context
authorSolomon Peachy <pizza@shaftnet.org>
Wed, 28 Aug 2013 00:29:46 +0000 (20:29 -0400)
committerJohn W. Linville <linville@tuxdriver.com>
Mon, 9 Sep 2013 18:40:53 +0000 (14:40 -0400)
When we get an interrupt from the hardware, the first thing the driver does
is tell the device to mask off the interrupt line.  Unfortunately this
involves a SPI transaction in interrupt context.  Some (most?) SPI
controllers perform the transfer asynchronously and try to sleep.
This is bad, and triggers a BUG().

So, work around this by using adding a hwbus hook for the cw1200 driver
core to call.  The cw1200_spi driver translates this into
irq_disable()/irq_enable() calls instead, which can safely be called in
interrupt context.

Apparently the platforms I used to develop the cw1200_spi driver used
synchronous spi_sync() implementations, which is why this didn't surface
until now.

Many thanks to Dave Sizeburns for the inital bug report and his services
as a tester.

Signed-off-by: Solomon Peachy <pizza@shaftnet.org>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
drivers/net/wireless/cw1200/cw1200_spi.c
drivers/net/wireless/cw1200/fwio.c
drivers/net/wireless/cw1200/hwbus.h
drivers/net/wireless/cw1200/hwio.c

index d06376014bcda61bd73d92adeac2b4bf65640883..c31580ba883b2c6bb21cbfd9517acb93dee77b39 100644 (file)
@@ -41,6 +41,7 @@ struct hwbus_priv {
        const struct cw1200_platform_data_spi *pdata;
        spinlock_t              lock; /* Serialize all bus operations */
        int claimed;
+       int irq_disabled;
 };
 
 #define SDIO_TO_SPI_ADDR(addr) ((addr & 0x1f)>>2)
@@ -230,6 +231,8 @@ static irqreturn_t cw1200_spi_irq_handler(int irq, void *dev_id)
        struct hwbus_priv *self = dev_id;
 
        if (self->core) {
+               disable_irq_nosync(self->func->irq);
+               self->irq_disabled = 1;
                cw1200_irq_handler(self->core);
                return IRQ_HANDLED;
        } else {
@@ -263,13 +266,22 @@ exit:
 
 static int cw1200_spi_irq_unsubscribe(struct hwbus_priv *self)
 {
-       int ret = 0;
-
        pr_debug("SW IRQ unsubscribe\n");
        disable_irq_wake(self->func->irq);
        free_irq(self->func->irq, self);
 
-       return ret;
+       return 0;
+}
+
+static int cw1200_spi_irq_enable(struct hwbus_priv *self, int enable)
+{
+       /* Disables are handled by the interrupt handler */
+       if (enable && self->irq_disabled) {
+               enable_irq(self->func->irq);
+               self->irq_disabled = 0;
+       }
+
+       return 0;
 }
 
 static int cw1200_spi_off(const struct cw1200_platform_data_spi *pdata)
@@ -349,6 +361,7 @@ static struct hwbus_ops cw1200_spi_hwbus_ops = {
        .unlock                 = cw1200_spi_unlock,
        .align_size             = cw1200_spi_align_size,
        .power_mgmt             = cw1200_spi_pm,
+       .irq_enable             = cw1200_spi_irq_enable,
 };
 
 /* Probe Function to be called by SPI stack when device is discovered */
index acdff0f7f952e03fa25b51b47a9e83eda37d85c6..0b2061bbc68bfa99ba9bce91dcdba18b99984879 100644 (file)
@@ -485,7 +485,7 @@ int cw1200_load_firmware(struct cw1200_common *priv)
 
        /* Enable interrupt signalling */
        priv->hwbus_ops->lock(priv->hwbus_priv);
-       ret = __cw1200_irq_enable(priv, 1);
+       ret = __cw1200_irq_enable(priv, 2);
        priv->hwbus_ops->unlock(priv->hwbus_priv);
        if (ret < 0)
                goto unsubscribe;
index 8b2fc831c3de75f6f33799cc12c6174bad6d837b..51dfb3a90735521f18a1c572440e3ec8e486f5c8 100644 (file)
@@ -28,6 +28,7 @@ struct hwbus_ops {
        void (*unlock)(struct hwbus_priv *self);
        size_t (*align_size)(struct hwbus_priv *self, size_t size);
        int (*power_mgmt)(struct hwbus_priv *self, bool suspend);
+       int (*irq_enable)(struct hwbus_priv *self, int enable);
 };
 
 #endif /* CW1200_HWBUS_H */
index ff230b7aeedd646e1591d01bdd1514afa86d7f0b..41bd7615ccaa31f2ba8bf8c183a88db0a73bca60 100644 (file)
@@ -273,6 +273,21 @@ int __cw1200_irq_enable(struct cw1200_common *priv, int enable)
        u16 val16;
        int ret;
 
+       /* We need to do this hack because the SPI layer can sleep on I/O
+          and the general path involves I/O to the device in interrupt
+          context.
+
+          However, the initial enable call needs to go to the hardware.
+
+          We don't worry about shutdown because we do a full reset which
+          clears the interrupt enabled bits.
+       */
+       if (priv->hwbus_ops->irq_enable) {
+               ret = priv->hwbus_ops->irq_enable(priv->hwbus_priv, enable);
+               if (ret || enable < 2)
+                       return ret;
+       }
+
        if (HIF_8601_SILICON == priv->hw_type) {
                ret = __cw1200_reg_read_32(priv, ST90TDS_CONFIG_REG_ID, &val32);
                if (ret < 0) {