driver/at86rf2xx: fix possible race condition in state machine

DipSwitch · Jun 24, 2017 · cbbc44e · cbbc44e
1 parent e2cce62
commit cbbc44e
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 31 deletions.
diff --git a/drivers/at86rf2xx/at86rf2xx.c b/drivers/at86rf2xx/at86rf2xx.c
@@ -136,18 +136,10 @@ size_t at86rf2xx_send(at86rf2xx_t *dev, uint8_t *data, size_t len)
 
 void at86rf2xx_tx_prepare(at86rf2xx_t *dev)
 {
-    uint8_t state;
-
     dev->pending_tx++;
-    /* make sure ongoing transmissions are finished */
-    do {
-        state = at86rf2xx_get_status(dev);
-    } while (state == AT86RF2XX_STATE_BUSY_RX_AACK ||
-             state == AT86RF2XX_STATE_BUSY_TX_ARET);
-    if (state != AT86RF2XX_STATE_TX_ARET_ON) {
-        dev->idle_state = state;
-    }
-    at86rf2xx_set_state(dev, AT86RF2XX_STATE_TX_ARET_ON);
+
+    dev->idle_state = at86rf2xx_set_state(dev, AT86RF2XX_STATE_TX_ARET_ON);
+
     dev->tx_frame_len = IEEE802154_FCS_LEN;
 }
 

diff --git a/drivers/at86rf2xx/at86rf2xx_getset.c b/drivers/at86rf2xx/at86rf2xx_getset.c
@@ -444,29 +444,25 @@ static inline void _set_state(at86rf2xx_t *dev, uint8_t state, uint8_t cmd)
     dev->state = state;
 }
 
-void at86rf2xx_set_state(at86rf2xx_t *dev, uint8_t state)
+uint8_t at86rf2xx_set_state(at86rf2xx_t *dev, uint8_t state)
 {
-    uint8_t old_state = at86rf2xx_get_status(dev);
-
-    if (state == old_state) {
-        return;
-    }
-
-    if (state == AT86RF2XX_STATE_FORCE_TRX_OFF) {
-        _set_state(dev, AT86RF2XX_STATE_TRX_OFF, state);
-        return;
-    }
+    uint8_t old_state;
 
     /* make sure there is no ongoing transmission, or state transition already
      * in progress */
-    while (old_state == AT86RF2XX_STATE_BUSY_RX_AACK ||
-           old_state == AT86RF2XX_STATE_BUSY_TX_ARET ||
-           old_state == AT86RF2XX_STATE_IN_PROGRESS) {
+    do {
         old_state = at86rf2xx_get_status(dev);
+    } while (old_state == AT86RF2XX_STATE_BUSY_RX_AACK ||
+             old_state == AT86RF2XX_STATE_BUSY_TX_ARET ||
+             old_state == AT86RF2XX_STATE_IN_PROGRESS);
+
+    if (state == AT86RF2XX_STATE_FORCE_TRX_OFF) {
+        _set_state(dev, AT86RF2XX_STATE_TRX_OFF, state);
+        return old_state;
     }
 
     if (state == old_state) {
-        return;
+        return old_state;
     }
 
     /* we need to go via PLL_ON if we are moving between RX_AACK_ON <-> TX_ARET_ON */
@@ -493,6 +489,8 @@ void at86rf2xx_set_state(at86rf2xx_t *dev, uint8_t state)
     } else {
         _set_state(dev, state, state);
     }
+
+    return old_state;
 }
 
 void at86rf2xx_reset_state_machine(at86rf2xx_t *dev)

diff --git a/drivers/at86rf2xx/at86rf2xx_internal.c b/drivers/at86rf2xx/at86rf2xx_internal.c
@@ -147,10 +147,8 @@ void at86rf2xx_hardware_reset(at86rf2xx_t *dev)
 
 void at86rf2xx_configure_phy(at86rf2xx_t *dev)
 {
-    uint8_t state = at86rf2xx_get_status(dev);
-
     /* we must be in TRX_OFF before changing the PHY configuration */
-    at86rf2xx_set_state(dev, AT86RF2XX_STATE_TRX_OFF);
+    uint8_t prev_state = at86rf2xx_set_state(dev, AT86RF2XX_STATE_TRX_OFF);
 
 #ifdef MODULE_AT86RF212B
     /* The TX power register must be updated after changing the channel if
@@ -200,7 +198,7 @@ void at86rf2xx_configure_phy(at86rf2xx_t *dev)
 #endif
 
     /* Return to the state we had before reconfiguring */
-    at86rf2xx_set_state(dev, state);
+    at86rf2xx_set_state(dev, prev_state);
 }
 
 #if defined(MODULE_AT86RF233) || defined(MODULE_AT86RF231)

diff --git a/drivers/include/at86rf2xx.h b/drivers/include/at86rf2xx.h
@@ -385,8 +385,10 @@ void at86rf2xx_set_option(at86rf2xx_t *dev, uint16_t option, bool state);
  *
  * @param[in] dev           device to change state of
  * @param[in] state         the targeted new state
+ *
+ * @return                  the previous state before the new state was set
  */
-void at86rf2xx_set_state(at86rf2xx_t *dev, uint8_t state);
+uint8_t at86rf2xx_set_state(at86rf2xx_t *dev, uint8_t state);
 
 /**
  * @brief   Reset the internal state machine to TRX_OFF mode.