From 4ebbda844c19d2d3ee6be43d395083082583f707 Mon Sep 17 00:00:00 2001 From: Thomas Eichinger Date: Mon, 4 Apr 2016 11:29:03 -0300 Subject: [PATCH 1/2] drivers/at86rf2xx: improve precondition checks on state transition The rational behind this change is the following: If the transceiver is in any *_BUSY state when `at86rf2xx_set_state()` gets called this would bypass the `(state == old_state)` check and unneeded state transitions could be triggered. --- drivers/at86rf2xx/at86rf2xx_getset.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/at86rf2xx/at86rf2xx_getset.c b/drivers/at86rf2xx/at86rf2xx_getset.c index 8c70b1496d..3fb82c7e12 100644 --- a/drivers/at86rf2xx/at86rf2xx_getset.c +++ b/drivers/at86rf2xx/at86rf2xx_getset.c @@ -429,9 +429,6 @@ void at86rf2xx_set_state(at86rf2xx_t *dev, uint8_t state) { uint8_t old_state = at86rf2xx_get_status(dev); - if (state == old_state) { - return; - } /* make sure there is no ongoing transmission, or state transition already * in progress */ while (old_state == AT86RF2XX_STATE_BUSY_RX_AACK || @@ -440,6 +437,10 @@ void at86rf2xx_set_state(at86rf2xx_t *dev, uint8_t state) old_state = at86rf2xx_get_status(dev); } + if (state == old_state) { + return; + } + /* we need to go via PLL_ON if we are moving between RX_AACK_ON <-> TX_ARET_ON */ if ((old_state == AT86RF2XX_STATE_RX_AACK_ON && state == AT86RF2XX_STATE_TX_ARET_ON) || From 66db33b662d5c3d1ba0b233c4a7df2abc256693a Mon Sep 17 00:00:00 2001 From: Thomas Eichinger Date: Tue, 1 Nov 2016 07:26:57 -0700 Subject: [PATCH 2/2] drivers/at86rf2xx: prevent a possible race condition after state change It was pointed out that after a state change to RX_AACK_ON reading back the state to confirm the transition can fail due to an imidiate change into BUSY_RX_AACK between the successful change on the transceiver and querying the state. For this we exclude the readback of the state for transitions to RX_AACK_ON. --- drivers/at86rf2xx/at86rf2xx_getset.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/at86rf2xx/at86rf2xx_getset.c b/drivers/at86rf2xx/at86rf2xx_getset.c index 3fb82c7e12..e93e5f74e9 100644 --- a/drivers/at86rf2xx/at86rf2xx_getset.c +++ b/drivers/at86rf2xx/at86rf2xx_getset.c @@ -421,7 +421,16 @@ void at86rf2xx_set_option(at86rf2xx_t *dev, uint16_t option, bool state) static inline void _set_state(at86rf2xx_t *dev, uint8_t state) { at86rf2xx_reg_write(dev, AT86RF2XX_REG__TRX_STATE, state); - while (at86rf2xx_get_status(dev) != state); + + /* To prevent a possible race condition when changing to + * RX_AACK_ON state the state doesn't get read back in that + * case. See discussion + * in https://github.com/RIOT-OS/RIOT/pull/5244 + */ + if (state != AT86RF2XX_STATE_RX_AACK_ON) { + while (at86rf2xx_get_status(dev) != state); + } + dev->state = state; }