Merge pull request #15228 from jia200x/pr/hal_read_func

ieee802154/radio: add `read` function

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,

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,

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
+     * The transceiver will be in a "FB Lock" state where no more frames are
-     * this function is called, but with framebuffer protection (either
+     * received. This is done in order to avoid overwriting the Frame Buffer
-     * dynamic framebuffer protection or disabled RX). Thus, the frame
+     * with new frame arrivals.  In order to leave this state, the upper layer
-     * won't be overwritten before calling the @ref ieee802154_radio_indication_rx
+     * must set the transceiver state (@ref
-     * function. However, @ref ieee802154_radio_indication_rx MUST be called in
+     * ieee802154_radio_ops::request_set_trx_state).
-     * 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.
      */
     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
+     * It should try to copy the received frame into @p buf
-     * then unlock the framebuffer (in order to be able to receive more
-     * frames).
      *
-     * @pre the device is on and an @ref IEEE802154_RADIO_INDICATION_RX_DONE
+     * @post It's not safe to call this function again before setting the
-     *      event was issued.
+     *       transceiver state to @ref IEEE802154_TRX_STATE_RX_ON (thus flushing
-     *
+     *       the RX FIFO).
-     * @post the state is @ref IEEE802154_TRX_STATE_RX_ON
      *
      * @param[in] dev IEEE802.15.4 device descriptor
-     * @param[out] buf buffer to write the received PSDU frame into. If NULL,
+     * @param[out] buf buffer to write the received PSDU frame into.
-     *             the frame is not copied.
      * @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,
+    int (*read)(ieee802154_dev_t *dev, void *buf, size_t size, ieee802154_rx_info_t *info);
-                         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
+ * @param[out] buf buffer to write the received frame into.
- *             frame is not copied.
  * @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);
 }
 
 /**

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
+     * @post If @ref ieee802154_submac_t::state is @ref IEEE802154_STATE_LISTEN, the
-     * SubMAC is ready to receive frames.
+     * 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
@@ -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);
 }
 
 /**

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;
@@ -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) {}
     }
 }
 

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);