The is_tx flag did not protect adequately against races when switching
from receiving to transmitting. This patch removes the unnecessary
spinlock and enforces proper synchronization along the interrupt
processing chain (source, handler, and worker), before entering
transmit mode.

The mechanism for allowing transmits to be interrupted had two flaws:
1) it tried to return with the transmission potentially never
completed, thus risking a subsequent BUG_ON(lp->is_tx), and
2) it attempted to leave BUSY_TX with STATE_RX_ON, which according to
the data sheet is not valid.

This patch force the transceiver into PLL_ON/TX_ON state on
interruption and then has a common path for transiting to RX_ON and
clearing is_tx.

Note that the synchronization overhead introduced by this patch
is about 100 us on a Ben NanoNote.

Signed-off-by: Werner Almesberger <wer...@almesberger.net>

---
 drivers/ieee802154/at86rf230.c |   32 ++++++++++----------------------
 1 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/ieee802154/at86rf230.c b/drivers/ieee802154/at86rf230.c
index 62d6075..00d7a68 100644
--- a/drivers/ieee802154/at86rf230.c
+++ b/drivers/ieee802154/at86rf230.c
@@ -58,8 +58,7 @@ struct at86rf230_local {
 
        struct ieee802154_dev *dev;
 
-       spinlock_t lock;
-       unsigned is_tx:1; /* P: lock */
+       volatile unsigned is_tx:1; /* P: lock */
 };
 
 
@@ -336,22 +335,24 @@ at86rf230_xmit(struct ieee802154_dev *dev, struct sk_buff 
*skb)
 {
        struct at86rf230_local *lp = dev->priv;
        int rc;
-       unsigned long flags;
 
        pr_debug("%s\n", __func__);
 
        might_sleep();
 
-       spin_lock_irqsave(&lp->lock, flags);
        BUG_ON(lp->is_tx);
-       lp->is_tx = 1;
+
        INIT_COMPLETION(lp->tx_complete);
-       spin_unlock_irqrestore(&lp->lock, flags);
 
        rc = at86rf230_state(dev, STATE_FORCE_TX_ON, STATE_TX_ON);
        if (rc)
                goto err;
 
+       synchronize_irq(lp->spi->irq);
+       flush_work(&lp->irqwork);
+
+       lp->is_tx = 1;
+
        rc = at86rf230_write_fbuf(lp, skb->data, skb->len);
        if (rc)
                goto err_rx;
@@ -368,18 +369,12 @@ at86rf230_xmit(struct ieee802154_dev *dev, struct sk_buff 
*skb)
 
        rc = wait_for_completion_interruptible(&lp->tx_complete);
        if (rc < 0)
-               goto err_rx;
-
-       rc = at86rf230_state(dev, STATE_RX_ON, STATE_RX_ON);
-
-       return rc;
+               at86rf230_state(dev, STATE_FORCE_TX_ON, STATE_TX_ON);
 
 err_rx:
        at86rf230_state(dev, STATE_RX_ON, STATE_RX_ON);
 err:
-       spin_lock_irqsave(&lp->lock, flags);
        lp->is_tx = 0;
-       spin_unlock_irqrestore(&lp->lock, flags);
 
        return rc;
 }
@@ -430,7 +425,6 @@ static void at86rf230_irqwork(struct work_struct *work)
                container_of(work, struct at86rf230_local, irqwork);
        u8 status = 0, val;
        int rc;
-       unsigned long flags;
 
        dev_dbg(&lp->spi->dev, "IRQ Worker\n");
 
@@ -446,15 +440,10 @@ static void at86rf230_irqwork(struct work_struct *work)
 
                if (status & IRQ_TRX_END) {
                        status &= ~IRQ_TRX_END;
-                       spin_lock_irqsave(&lp->lock, flags);
-                       if (lp->is_tx) {
-                               lp->is_tx = 0;
-                               spin_unlock_irqrestore(&lp->lock, flags);
+                       if (lp->is_tx)
                                complete(&lp->tx_complete);
-                       } else {
-                               spin_unlock_irqrestore(&lp->lock, flags);
+                       else
                                at86rf230_rx(lp);
-                       }
                }
 
        } while (status != 0);
@@ -642,7 +631,6 @@ static int __devinit at86rf230_probe(struct spi_device *spi)
 
        mutex_init(&lp->bmux);
        INIT_WORK(&lp->irqwork, at86rf230_irqwork);
-       spin_lock_init(&lp->lock);
        init_completion(&lp->tx_complete);
 
        spi_set_drvdata(spi, lp);
-- 
1.7.0.4


------------------------------------------------------------------------------
EditLive Enterprise is the world's most technically advanced content
authoring tool. Experience the power of Track Changes, Inline Image
Editing and ensure content is compliant with Accessibility Checking.
http://p.sf.net/sfu/ephox-dev2dev
_______________________________________________
Linux-zigbee-devel mailing list
Linux-zigbee-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel

Reply via email to