From ee5adad40150bc9f10f77aabb87d62e12f8e8745 Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Wed, 14 Oct 2020 16:52:44 +0200 Subject: [PATCH 1/2] ieee802154/radio: replace indication mechanism --- cpu/cc2538/radio/cc2538_rf_radio_ops.c | 8 ++-- cpu/nrf52/radio/nrf802154/nrf802154_radio.c | 6 +-- sys/include/net/ieee802154/radio.h | 50 +++++++++------------ sys/include/net/ieee802154/submac.h | 2 +- sys/net/link_layer/ieee802154/submac.c | 2 +- tests/ieee802154_hal/main.c | 2 +- 6 files changed, 30 insertions(+), 40 deletions(-) diff --git a/cpu/cc2538/radio/cc2538_rf_radio_ops.c b/cpu/cc2538/radio/cc2538_rf_radio_ops.c index 98061b4c9d..36863ea493 100644 --- a/cpu/cc2538/radio/cc2538_rf_radio_ops.c +++ b/cpu/cc2538/radio/cc2538_rf_radio_ops.c @@ -160,7 +160,7 @@ static int _len(ieee802154_dev_t *dev) return rfcore_peek_rx_fifo(0) - IEEE802154_FCS_LEN; } -static int _indication_rx(ieee802154_dev_t *dev, void *buf, size_t size, ieee802154_rx_info_t *info) +static int _read(ieee802154_dev_t *dev, void *buf, size_t size, ieee802154_rx_info_t *info) { (void) dev; int res; @@ -169,7 +169,6 @@ static int _indication_rx(ieee802154_dev_t *dev, void *buf, size_t size, ieee802 pkt_len -= IEEE802154_FCS_LEN; if (pkt_len > size) { - RFCORE_SFR_RFST = ISFLUSHRX; return -ENOBUFS; } @@ -204,7 +203,6 @@ static int _indication_rx(ieee802154_dev_t *dev, void *buf, size_t size, ieee802 res = 0; } - RFCORE_SFR_RFST = ISFLUSHRX; return res; } @@ -300,6 +298,7 @@ static int _request_set_trx_state(ieee802154_dev_t *dev, ieee802154_trx_state_t break; case IEEE802154_TRX_STATE_RX_ON: RFCORE_XREG_RFIRQM0 |= RXPKTDONE; + RFCORE_SFR_RFST = ISFLUSHRX; RFCORE_SFR_RFST = ISRXON; break; } @@ -323,6 +322,7 @@ void cc2538_irq_handler(void) /* CRC check */ uint8_t pkt_len = rfcore_peek_rx_fifo(0); if (rfcore_peek_rx_fifo(pkt_len) & CC2538_CRC_BIT_MASK) { + RFCORE_XREG_RFIRQM0 &= RXPKTDONE; cc2538_rf_dev.cb(&cc2538_rf_dev, IEEE802154_RADIO_INDICATION_RX_DONE); } else { @@ -492,10 +492,10 @@ static int _set_csma_params(ieee802154_dev_t *dev, const ieee802154_csma_be_t *b static const ieee802154_radio_ops_t cc2538_rf_ops = { .write = _write, + .read = _read, .request_transmit = _request_transmit, .confirm_transmit = _confirm_transmit, .len = _len, - .indication_rx = _indication_rx, .off = _off, .request_on = _request_on, .confirm_on = _confirm_on, diff --git a/cpu/nrf52/radio/nrf802154/nrf802154_radio.c b/cpu/nrf52/radio/nrf802154/nrf802154_radio.c index 6eedeedc50..b709526b37 100644 --- a/cpu/nrf52/radio/nrf802154/nrf802154_radio.c +++ b/cpu/nrf52/radio/nrf802154/nrf802154_radio.c @@ -204,7 +204,7 @@ static inline int8_t _hwval_to_ieee802154_dbm(uint8_t hwval) return (ED_RSSISCALE * hwval) + ED_RSSIOFFS; } -static int _indication_rx(ieee802154_dev_t *dev, void *buf, size_t max_size, +static int _read(ieee802154_dev_t *dev, void *buf, size_t max_size, ieee802154_rx_info_t *info) { (void) dev; @@ -234,8 +234,6 @@ static int _indication_rx(ieee802154_dev_t *dev, void *buf, size_t max_size, memcpy(buf, &rxbuf[1], pktlen); } - NRF_RADIO->TASKS_START = 1; - return pktlen; } @@ -718,10 +716,10 @@ static int _set_csma_params(ieee802154_dev_t *dev, const ieee802154_csma_be_t *b static const ieee802154_radio_ops_t nrf802154_ops = { .write = _write, + .read = _read, .request_transmit = _request_transmit, .confirm_transmit = _confirm_transmit, .len = _len, - .indication_rx = _indication_rx, .off = _off, .request_on = _request_on, .confirm_on = _confirm_on, diff --git a/sys/include/net/ieee802154/radio.h b/sys/include/net/ieee802154/radio.h index f831b84e6f..0d701b81c0 100644 --- a/sys/include/net/ieee802154/radio.h +++ b/sys/include/net/ieee802154/radio.h @@ -197,14 +197,11 @@ typedef enum { * The transceiver or driver MUST handle the ACK reply if the Ack Request * bit is set in the received frame and promiscuous mode is disabled. * - * The transceiver is in @ref IEEE802154_TRX_STATE_RX_ON state when - * this function is called, but with framebuffer protection (either - * dynamic framebuffer protection or disabled RX). Thus, the frame - * won't be overwritten before calling the @ref ieee802154_radio_indication_rx - * function. However, @ref ieee802154_radio_indication_rx MUST be called in - * order to receive new frames. If there's no interest in the - * frame, the function can be called with a NULL buffer to drop - * the frame. + * The transceiver will be in a "FB Lock" state where no more frames are + * received. This is done in order to avoid overwriting the Frame Buffer + * with new frame arrivals. In order to leave this state, the upper layer + * must set the transceiver state (@ref + * ieee802154_radio_ops::request_set_trx_state). */ IEEE802154_RADIO_INDICATION_RX_DONE, @@ -359,7 +356,8 @@ struct ieee802154_radio_ops { /** * @brief Write a frame into the framebuffer. * - * This function shouldn't do any checks, so the frame MUST be valid. + * This function shouldn't do any checks, so the frame MUST be valid. The + * previous content of the framebuffer is replaced by @p psdu. * * @param[in] dev IEEE802.15.4 device descriptor * @param[in] psdu PSDU frame to be sent @@ -427,21 +425,17 @@ struct ieee802154_radio_ops { int (*len)(ieee802154_dev_t *dev); /** - * @brief Process the RX done indication + * @brief Read a frame from the internal framebuffer * * This function reads the received frame from the internal framebuffer. - * It should try to copy the received frame into @p buf and - * then unlock the framebuffer (in order to be able to receive more - * frames). + * It should try to copy the received frame into @p buf * - * @pre the device is on and an @ref IEEE802154_RADIO_INDICATION_RX_DONE - * event was issued. - * - * @post the state is @ref IEEE802154_TRX_STATE_RX_ON + * @post It's not safe to call this function again before setting the + * transceiver state to @ref IEEE802154_TRX_STATE_RX_ON (thus flushing + * the RX FIFO). * * @param[in] dev IEEE802.15.4 device descriptor - * @param[out] buf buffer to write the received PSDU frame into. If NULL, - * the frame is not copied. + * @param[out] buf buffer to write the received PSDU frame into. * @param[in] size size of @p buf * @param[in] info information of the received frame (LQI, RSSI). Can be * NULL if this information is not needed. @@ -449,9 +443,7 @@ struct ieee802154_radio_ops { * @return number of bytes written in @p buffer (0 if @p buf == NULL) * @return -ENOBUFS if the frame doesn't fit in @p */ - int (*indication_rx)(ieee802154_dev_t *dev, void *buf, size_t size, - ieee802154_rx_info_t *info); - + int (*read)(ieee802154_dev_t *dev, void *buf, size_t size, ieee802154_rx_info_t *info); /** * @brief Turn off the device * @@ -513,7 +505,8 @@ struct ieee802154_radio_ops { * @brief Request a PHY state change * * @note @ref ieee802154_radio_ops::confirm_set_trx_state MUST be used to - * finish the state transition. + * finish the state transition. Also, setting the state to + * @ref IEEE802154_TRX_STATE_RX_ON flushes the RX FIFO. * * @pre the device is on * @@ -759,23 +752,22 @@ static inline int ieee802154_radio_len(ieee802154_dev_t *dev) } /** - * @brief Shortcut to @ref ieee802154_radio_ops::indication_rx + * @brief Shortcut to @ref ieee802154_radio_ops::read * * @param[in] dev IEEE802.15.4 device descriptor - * @param[out] buf buffer to write the received frame into. If NULL, the - * frame is not copied. + * @param[out] buf buffer to write the received frame into. * @param[in] size size of @p buf * @param[in] info information of the received frame (LQI, RSSI). Can be * NULL if this information is not needed. * - * @return result of @ref ieee802154_radio_ops::indication_rx + * @return result of @ref ieee802154_radio_ops::read */ -static inline int ieee802154_radio_indication_rx(ieee802154_dev_t *dev, +static inline int ieee802154_radio_read(ieee802154_dev_t *dev, void *buf, size_t size, ieee802154_rx_info_t *info) { - return dev->driver->indication_rx(dev, buf, size, info); + return dev->driver->read(dev, buf, size, info); } /** diff --git a/sys/include/net/ieee802154/submac.h b/sys/include/net/ieee802154/submac.h index 514de51281..3c06b2cfb9 100644 --- a/sys/include/net/ieee802154/submac.h +++ b/sys/include/net/ieee802154/submac.h @@ -325,7 +325,7 @@ static inline int ieee802154_get_frame_length(ieee802154_submac_t *submac) static inline int ieee802154_read_frame(ieee802154_submac_t *submac, void *buf, size_t len, ieee802154_rx_info_t *info) { - return ieee802154_radio_indication_rx(submac->dev, buf, len, info); + return ieee802154_radio_read(submac->dev, buf, len, info); } /** diff --git a/sys/net/link_layer/ieee802154/submac.c b/sys/net/link_layer/ieee802154/submac.c index 518b737ea0..674b12831d 100644 --- a/sys/net/link_layer/ieee802154/submac.c +++ b/sys/net/link_layer/ieee802154/submac.c @@ -154,7 +154,7 @@ void ieee802154_submac_rx_done_cb(ieee802154_submac_t *submac) if (!_does_handle_ack(dev) && submac->wait_for_ack) { uint8_t ack[3]; - if (ieee802154_radio_indication_rx(dev, ack, 3, NULL) && + if (ieee802154_radio_read(dev, ack, 3, NULL) && ack[0] & IEEE802154_FCF_TYPE_ACK) { ieee802154_submac_ack_timer_cancel(submac); ieee802154_tx_info_t tx_info; diff --git a/tests/ieee802154_hal/main.c b/tests/ieee802154_hal/main.c index 29ab5c57bd..49e5faa194 100644 --- a/tests/ieee802154_hal/main.c +++ b/tests/ieee802154_hal/main.c @@ -111,7 +111,7 @@ void _rx_done_handler(event_t *event) * NOTE: It's possible to call `ieee802154_radio_len` to retrieve the packet * length. Since the buffer is fixed in this test, we don't use it */ - int size = ieee802154_radio_indication_rx(ieee802154_hal_test_get_dev(RADIO_DEFAULT_ID), buffer, 127, &info); + int size = ieee802154_radio_read(ieee802154_hal_test_get_dev(RADIO_DEFAULT_ID), buffer, 127, &info); if (size > 0) { /* Print packet while we wait for the state transition */ _print_packet(size, info.lqi, info.rssi); From b796d9805d004972139f4f2959485a84299d0481 Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Wed, 14 Oct 2020 17:21:42 +0200 Subject: [PATCH 2/2] ieee802154/submac: adapt to radio hal API change --- sys/include/net/ieee802154/submac.h | 6 +++--- sys/net/link_layer/ieee802154/submac.c | 12 ++++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/sys/include/net/ieee802154/submac.h b/sys/include/net/ieee802154/submac.h index 3c06b2cfb9..4890b2acfc 100644 --- a/sys/include/net/ieee802154/submac.h +++ b/sys/include/net/ieee802154/submac.h @@ -76,8 +76,8 @@ typedef struct { * This function is called from the SubMAC to indicate a IEEE 802.15.4 * frame is ready to be fetched from the device. * - * If @ref ieee802154_submac_t::state is @ref IEEE802154_STATE_LISTEN, the - * SubMAC is ready to receive frames. + * @post If @ref ieee802154_submac_t::state is @ref IEEE802154_STATE_LISTEN, the + * SubMAC is ready to receive frames * * @note ACK frames are automatically handled and discarded by the SubMAC. * @param[in] submac pointer to the SubMAC descriptor @@ -89,7 +89,7 @@ typedef struct { * This function is called from the SubMAC to indicate that the TX * procedure finished. * - * If @ref ieee802154_submac_t::state is @ref IEEE802154_STATE_LISTEN, the + * @pre If @ref ieee802154_submac_t::state is @ref IEEE802154_STATE_LISTEN, the * SubMAC is ready to receive frames. * * @param[in] submac pointer to the SubMAC descriptor diff --git a/sys/net/link_layer/ieee802154/submac.c b/sys/net/link_layer/ieee802154/submac.c index 674b12831d..0584ba95a3 100644 --- a/sys/net/link_layer/ieee802154/submac.c +++ b/sys/net/link_layer/ieee802154/submac.c @@ -169,6 +169,18 @@ void ieee802154_submac_rx_done_cb(ieee802154_submac_t *submac) } else { submac->cb->rx_done(submac); + + /* The Radio HAL will be in "FB Lock" state. We need to do a state + * transition here in order to release it */ + ieee802154_trx_state_t next_state = submac->state == IEEE802154_STATE_LISTEN ? IEEE802154_TRX_STATE_RX_ON : IEEE802154_TRX_STATE_TRX_OFF; + + /* Some radios will run some house keeping tasks on RX_DONE (e.g + * sending ACK frames). In such case we need to wait until the radio is + * not busy + */ + while (ieee802154_radio_request_set_trx_state(submac->dev, next_state) == -EBUSY); + + while (ieee802154_radio_confirm_set_trx_state(submac->dev) == -EAGAIN) {} } }