Merge pull request #7115 from DipSwitch/pr/fix/at86rf2xx_setstate

driver/at86rf2xx: Fix possible race condition in at86rf2xx_configure
RIOT-OS · Jun 26, 2017 · c76fdf5 · c76fdf5
2 parents 664d03d + cbbc44e
commit c76fdf5
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.