From d86df5b2174da094d57c0c1369f24671605ada47 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 26 Feb 2021 10:32:24 +0100 Subject: [PATCH 01/12] sys/net/netopt: make NETOPT_TX_END_IRQ and friends read-only Reasoning: - Many drivers already just generate these events regardless of configuration. This will make behavior more consistent - Run time configuration increases ROM, complexity and maintaining effort - At least NETOPT_TX_END_IRQ is now de facto mandatory, as the new netdev driver API requires NETDEV_EVENT_TX_COMPLETE to always be generated. --- sys/include/net/netopt.h | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/sys/include/net/netopt.h b/sys/include/net/netopt.h index c5db2808ba..0a87aeaf9d 100644 --- a/sys/include/net/netopt.h +++ b/sys/include/net/netopt.h @@ -249,37 +249,43 @@ typedef enum { */ NETOPT_RAWMODE, /** - * @brief (@ref netopt_enable_t) trigger interrupt at reception start + * @brief (@ref netopt_enable_t) Used to check if the driver generates NETDEV_EVENT_RX_STARTED + * events * * It is mostly triggered after the preamble is correctly received * - * @note not all transceivers may support this interrupt + * @warning This value is read-only and cannot be configured at run-time */ NETOPT_RX_START_IRQ, /** - * @brief (@ref netopt_enable_t) trigger interrupt after frame reception + * @brief (@ref netopt_enable_t) Used to check if the driver generates + * NETDEV_EVENT_RX_COMPLETE events * * This interrupt is triggered after a complete frame is received. * - * @note in case a transceiver does not support this interrupt, the event - * may be triggered by the driver + * @note In case a transceiver does not support this interrupt, the event + * may be triggered by the driver + * @warning This value is read-only and cannot be configured at run-time */ NETOPT_RX_END_IRQ, /** - * @brief (@ref netopt_enable_t) trigger interrupt at transmission start + * @brief (@ref netopt_enable_t) Used to check if the driver generates NETDEV_EVENT_TX_STARTED + * events * * This interrupt is triggered when the transceiver starts to send out the * frame. * - * @note in case a transceiver does not support this interrupt, the event - * may be triggered by the driver + * @note In case a transceiver does not support this interrupt, the event + * may be triggered by the driver + * @warning This value is read-only and cannot be configured at run-time */ NETOPT_TX_START_IRQ, /** - * @brief (@ref netopt_enable_t) trigger interrupt after frame transmission + * @brief (@ref netopt_enable_t) Used to check if the driver generates + * NETDEV_EVENT_TX_COMPLETE events * * This interrupt is triggered when the full frame has been transmitted. * From 94e0ef47c79003d6bb0889b66ccd727a217f4297 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 26 Feb 2021 10:40:12 +0100 Subject: [PATCH 02/12] sys/net/gnrc/gomach: update use of NETOPT_TX_END_IRQ and friends With the API change, it is no longer possible to enable these IRQs at run time. Instead, query if those are supported (with DEVELHELP). --- sys/net/gnrc/link_layer/gomach/gomach.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/sys/net/gnrc/link_layer/gomach/gomach.c b/sys/net/gnrc/link_layer/gomach/gomach.c index 318c53f41f..f8b2c28318 100644 --- a/sys/net/gnrc/link_layer/gomach/gomach.c +++ b/sys/net/gnrc/link_layer/gomach/gomach.c @@ -149,12 +149,16 @@ static void gomach_reinit_radio(gnrc_netif_t *netif) sizeof(netif->l2addr)); } - /* Enable RX-start and TX-started and TX-END interrupts. */ - netopt_enable_t enable = NETOPT_ENABLE; - netif->dev->driver->set(netif->dev, NETOPT_RX_START_IRQ, &enable, sizeof(enable)); - netif->dev->driver->set(netif->dev, NETOPT_RX_END_IRQ, &enable, sizeof(enable)); - netif->dev->driver->set(netif->dev, NETOPT_TX_END_IRQ, &enable, sizeof(enable)); - + /* Check if RX-start and TX-started and TX-END interrupts are supported */ + if (IS_ACTIVE(DEVELHELP)) { + netopt_enable_t enable; + netif->dev->driver->get(netif->dev, NETOPT_RX_START_IRQ, &enable, sizeof(enable)); + assert(enable == NETOPT_ENABLE); + netif->dev->driver->get(netif->dev, NETOPT_RX_END_IRQ, &enable, sizeof(enable)); + assert(enable == NETOPT_ENABLE); + netif->dev->driver->get(netif->dev, NETOPT_TX_END_IRQ, &enable, sizeof(enable)); + assert(enable == NETOPT_ENABLE); + } } static void _gomach_rtt_cb(void *arg) From ff081392631e94bf60d4ae628ed68b4de0fab82a Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 26 Feb 2021 10:42:04 +0100 Subject: [PATCH 03/12] sys/net/gnrc/lwmac: update use of NETOPT_TX_END_IRQ and friends With the API change, it is no longer possible to enable these IRQs at run time. Instead, query if those are supported (with DEVELHELP). --- sys/net/gnrc/link_layer/lwmac/lwmac.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/sys/net/gnrc/link_layer/lwmac/lwmac.c b/sys/net/gnrc/link_layer/lwmac/lwmac.c index c2c44afed4..ba2b2ddbad 100644 --- a/sys/net/gnrc/link_layer/lwmac/lwmac.c +++ b/sys/net/gnrc/link_layer/lwmac/lwmac.c @@ -107,13 +107,16 @@ static void lwmac_reinit_radio(gnrc_netif_t *netif) sizeof(netif->l2addr)); } - /* Enable RX-start and TX-started and TX-END interrupts. */ - netopt_enable_t enable = NETOPT_ENABLE; - netif->dev->driver->set(netif->dev, NETOPT_RX_START_IRQ, &enable, sizeof(enable)); - netif->dev->driver->set(netif->dev, NETOPT_RX_END_IRQ, &enable, sizeof(enable)); - netif->dev->driver->set(netif->dev, NETOPT_TX_START_IRQ, &enable, sizeof(enable)); - netif->dev->driver->set(netif->dev, NETOPT_TX_END_IRQ, &enable, sizeof(enable)); - + /* Check if RX-start and TX-started and TX-END interrupts are supported */ + if (IS_ACTIVE(DEVELHELP)) { + netopt_enable_t enable; + netif->dev->driver->get(netif->dev, NETOPT_RX_START_IRQ, &enable, sizeof(enable)); + assert(enable == NETOPT_ENABLE); + netif->dev->driver->get(netif->dev, NETOPT_RX_END_IRQ, &enable, sizeof(enable)); + assert(enable == NETOPT_ENABLE); + netif->dev->driver->get(netif->dev, NETOPT_TX_END_IRQ, &enable, sizeof(enable)); + assert(enable == NETOPT_ENABLE); + } } static gnrc_pktsnip_t *_make_netif_hdr(uint8_t *mhr) From 80d56488ccca722a5bba30a79ba88406c3779d37 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 26 Feb 2021 10:46:15 +0100 Subject: [PATCH 04/12] sys/net/gnrc/netif: update use of NETOPT_TX_END_IRQ and friends With the API change, it is no longer possible to enable these IRQs at run time. Instead, query if the needed events are supported (with DEVELHELP). --- sys/net/gnrc/netif/gnrc_netif.c | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/sys/net/gnrc/netif/gnrc_netif.c b/sys/net/gnrc/netif/gnrc_netif.c index 0eb6113d48..2f3e260029 100644 --- a/sys/net/gnrc/netif/gnrc_netif.c +++ b/sys/net/gnrc/netif/gnrc_netif.c @@ -52,7 +52,7 @@ #include "debug.h" static void _update_l2addr_from_dev(gnrc_netif_t *netif); -static void _configure_netdev(netdev_t *dev); +static void _check_netdev_capabilities(netdev_t *dev); static void *_gnrc_netif_thread(void *args); static void _event_cb(netdev_t *dev, netdev_event_t event); @@ -412,11 +412,6 @@ int gnrc_netif_set_from_netdev(gnrc_netif_t *netif, case NETOPT_IEEE802154_PHY: gnrc_netif_ipv6_init_mtu(netif); break; - case NETOPT_STATE: - if (*((netopt_state_t *)opt->data) == NETOPT_STATE_RESET) { - _configure_netdev(netif->dev); - } - break; default: break; } @@ -1261,18 +1256,20 @@ static void _init_from_device(gnrc_netif_t *netif) _update_l2addr_from_dev(netif); } -static void _configure_netdev(netdev_t *dev) +static void _check_netdev_capabilities(netdev_t *dev) { - /* Enable RX- and TX-complete interrupts */ - static const netopt_enable_t enable = NETOPT_ENABLE; - int res = dev->driver->set(dev, NETOPT_RX_END_IRQ, &enable, sizeof(enable)); - if (res < 0) { - DEBUG("gnrc_netif: enable NETOPT_RX_END_IRQ failed: %d\n", res); - } - if (IS_USED(MODULE_NETSTATS_L2) || IS_USED(MODULE_GNRC_NETIF_PKTQ)) { - res = dev->driver->set(dev, NETOPT_TX_END_IRQ, &enable, sizeof(enable)); - if (res < 0) { - DEBUG("gnrc_netif: enable NETOPT_TX_END_IRQ failed: %d\n", res); + /* Check whether RX- and TX-complete interrupts are supported by the driver */ + if (IS_ACTIVE(DEVELHELP)) { + netopt_enable_t enable = NETOPT_ENABLE; + int res = dev->driver->get(dev, NETOPT_RX_END_IRQ, &enable, sizeof(enable)); + if ((res != sizeof(enable)) || (enable != NETOPT_ENABLE)) { + LOG_WARNING("NETOPT_RX_END_IRQ not implemented by driver\n"); + } + if (IS_USED(MODULE_NETSTATS_L2) || IS_USED(MODULE_GNRC_NETIF_PKTQ)) { + res = dev->driver->get(dev, NETOPT_TX_END_IRQ, &enable, sizeof(enable)); + if ((res != sizeof(enable)) || (enable != NETOPT_ENABLE)) { + LOG_WARNING("NETOPT_TX_END_IRQ not implemented by driver\n"); + } } } } @@ -1667,7 +1664,7 @@ static void *_gnrc_netif_thread(void *args) return NULL; } netif_register(&netif->netif); - _configure_netdev(dev); + _check_netdev_capabilities(dev); netif->ops->init(netif); #if DEVELHELP assert(options_tested); From 52c8be9da9a4ae2658f62bb2edd1536ed8a2034a Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 26 Feb 2021 10:52:24 +0100 Subject: [PATCH 05/12] drivers/mrf24j40: drop unused define MRF24J40_OPT_REQ_AUTO_ACK is never used and wrongly documented. Let's just drop it. --- drivers/include/mrf24j40.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/include/mrf24j40.h b/drivers/include/mrf24j40.h index cfc89c4a30..e9af526835 100644 --- a/drivers/include/mrf24j40.h +++ b/drivers/include/mrf24j40.h @@ -104,8 +104,6 @@ extern "C" { * start */ #define MRF24J40_OPT_TELL_RX_END (0x4000) /**< notify MAC layer on RX * finished */ -#define MRF24J40_OPT_REQ_AUTO_ACK (0x8000) /**< notify MAC layer on RX - * finished */ /** @} */ From a18ec987bf0a72aef1cbd28ac15c18d99596ef0a Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 26 Feb 2021 10:53:22 +0100 Subject: [PATCH 06/12] drivers/mrf24j40: make TX/RX IRQs read only This brings the implementation in sync with the API. --- drivers/include/mrf24j40.h | 8 ----- drivers/mrf24j40/mrf24j40.c | 2 +- drivers/mrf24j40/mrf24j40_netdev.c | 48 ++---------------------------- 3 files changed, 4 insertions(+), 54 deletions(-) diff --git a/drivers/include/mrf24j40.h b/drivers/include/mrf24j40.h index e9af526835..6a65044ab0 100644 --- a/drivers/include/mrf24j40.h +++ b/drivers/include/mrf24j40.h @@ -96,14 +96,6 @@ extern "C" { #define MRF24J40_OPT_PROMISCUOUS (0x0200) /**< promiscuous mode * active */ #define MRF24J40_OPT_PRELOADING (0x0400) /**< preloading enabled */ -#define MRF24J40_OPT_TELL_TX_START (0x0800) /**< notify MAC layer on TX - * start */ -#define MRF24J40_OPT_TELL_TX_END (0x1000) /**< notify MAC layer on TX - * finished */ -#define MRF24J40_OPT_TELL_RX_START (0x2000) /**< notify MAC layer on RX - * start */ -#define MRF24J40_OPT_TELL_RX_END (0x4000) /**< notify MAC layer on RX - * finished */ /** @} */ diff --git a/drivers/mrf24j40/mrf24j40.c b/drivers/mrf24j40/mrf24j40.c index d3c9b1bb6e..360895733e 100644 --- a/drivers/mrf24j40/mrf24j40.c +++ b/drivers/mrf24j40/mrf24j40.c @@ -156,7 +156,7 @@ void mrf24j40_tx_exec(mrf24j40_t *dev) else { mrf24j40_reg_write_short(dev, MRF24J40_REG_TXNCON, MRF24J40_TXNCON_TXNTRIG); } - if (netdev->event_callback && (dev->netdev.flags & MRF24J40_OPT_TELL_TX_START)) { + if (netdev->event_callback) { netdev->event_callback(netdev, NETDEV_EVENT_TX_STARTED); } } diff --git a/drivers/mrf24j40/mrf24j40_netdev.c b/drivers/mrf24j40/mrf24j40_netdev.c index ddddef13e1..d93df48856 100644 --- a/drivers/mrf24j40/mrf24j40_netdev.c +++ b/drivers/mrf24j40/mrf24j40_netdev.c @@ -231,26 +231,10 @@ static int _get(netdev_t *netdev, netopt_t opt, void *val, size_t max_len) break; case NETOPT_RX_START_IRQ: - *((netopt_enable_t *)val) = - !!(dev->netdev.flags & MRF24J40_OPT_TELL_RX_START); - res = sizeof(netopt_enable_t); - break; - case NETOPT_RX_END_IRQ: - *((netopt_enable_t *)val) = - !!(dev->netdev.flags & MRF24J40_OPT_TELL_RX_END); - res = sizeof(netopt_enable_t); - break; - case NETOPT_TX_START_IRQ: - *((netopt_enable_t *)val) = - !!(dev->netdev.flags & MRF24J40_OPT_TELL_TX_START); - res = sizeof(netopt_enable_t); - break; - case NETOPT_TX_END_IRQ: - *((netopt_enable_t *)val) = - !!(dev->netdev.flags & MRF24J40_OPT_TELL_TX_END); + *((netopt_enable_t *)val) = NETOPT_ENABLE; res = sizeof(netopt_enable_t); break; @@ -486,30 +470,6 @@ static int _set(netdev_t *netdev, netopt_t opt, const void *val, size_t len) res = sizeof(netopt_enable_t); break; - case NETOPT_RX_START_IRQ: - mrf24j40_set_option(dev, MRF24J40_OPT_TELL_RX_START, - ((const bool *)val)[0]); - res = sizeof(netopt_enable_t); - break; - - case NETOPT_RX_END_IRQ: - mrf24j40_set_option(dev, MRF24J40_OPT_TELL_RX_END, - ((const bool *)val)[0]); - res = sizeof(netopt_enable_t); - break; - - case NETOPT_TX_START_IRQ: - mrf24j40_set_option(dev, MRF24J40_OPT_TELL_TX_START, - ((const bool *)val)[0]); - res = sizeof(netopt_enable_t); - break; - - case NETOPT_TX_END_IRQ: - mrf24j40_set_option(dev, MRF24J40_OPT_TELL_TX_END, - ((const bool *)val)[0]); - res = sizeof(netopt_enable_t); - break; - case NETOPT_CSMA: mrf24j40_set_option(dev, MRF24J40_OPT_CSMA, ((const bool *)val)[0]); @@ -570,7 +530,7 @@ static void _isr(netdev_t *netdev) dev->pending &= ~(MRF24J40_TASK_TX_READY); DEBUG("[mrf24j40] EVT - TX_END\n"); #ifdef MODULE_NETSTATS_L2 - if (netdev->event_callback && (dev->netdev.flags & MRF24J40_OPT_TELL_TX_END)) { + if (netdev->event_callback) { uint8_t txstat = mrf24j40_reg_read_short(dev, MRF24J40_REG_TXSTAT); dev->tx_retries = (txstat >> MRF24J40_TXSTAT_MAX_FRAME_RETRIES_SHIFT); /* transmission failed */ @@ -595,9 +555,7 @@ static void _isr(netdev_t *netdev) /* Receive interrupt occurred */ if (dev->pending & MRF24J40_TASK_RX_READY) { DEBUG("[mrf24j40] EVT - RX_END\n"); - if ((dev->netdev.flags & MRF24J40_OPT_TELL_RX_END)) { - netdev->event_callback(netdev, NETDEV_EVENT_RX_COMPLETE); - } + netdev->event_callback(netdev, NETDEV_EVENT_RX_COMPLETE); dev->pending &= ~(MRF24J40_TASK_RX_READY); } DEBUG("[mrf24j40] END IRQ\n"); From fba104c2ce44c836236aaad5eecc82f70eb2119e Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 26 Feb 2021 11:03:45 +0100 Subject: [PATCH 07/12] drivers/kw2xrf: make TX/RX IRQs read only This brings the implementation in sync with the API. --- drivers/include/kw2xrf.h | 8 ------- drivers/kw2xrf/kw2xrf.c | 4 ++++ drivers/kw2xrf/kw2xrf_getset.c | 33 ---------------------------- drivers/kw2xrf/kw2xrf_netdev.c | 39 +--------------------------------- 4 files changed, 5 insertions(+), 79 deletions(-) diff --git a/drivers/include/kw2xrf.h b/drivers/include/kw2xrf.h index 69901def61..5b63b4200c 100644 --- a/drivers/include/kw2xrf.h +++ b/drivers/include/kw2xrf.h @@ -95,14 +95,6 @@ extern "C" { #define KW2XRF_OPT_PROMISCUOUS (0x0200) /**< promiscuous mode * active */ #define KW2XRF_OPT_PRELOADING (0x0400) /**< preloading enabled */ -#define KW2XRF_OPT_TELL_TX_START (0x0800) /**< notify MAC layer on TX - * start */ -#define KW2XRF_OPT_TELL_TX_END (0x1000) /**< notify MAC layer on TX - * finished */ -#define KW2XRF_OPT_TELL_RX_START (0x2000) /**< notify MAC layer on RX - * start */ -#define KW2XRF_OPT_TELL_RX_END (0x4000) /**< notify MAC layer on RX - * finished */ #define KW2XRF_OPT_AUTOACK (0x8000) /**< enable automatically ACK * for incommint packet */ /** @} */ diff --git a/drivers/kw2xrf/kw2xrf.c b/drivers/kw2xrf/kw2xrf.c index 006254662e..7dc272cef2 100644 --- a/drivers/kw2xrf/kw2xrf.c +++ b/drivers/kw2xrf/kw2xrf.c @@ -66,6 +66,10 @@ void kw2xrf_setup(kw2xrf_t *dev, const kw2xrf_params_t *params) dev->pending_tx = 0; kw2xrf_spi_init(dev); kw2xrf_set_power_mode(dev, KW2XRF_IDLE); + DEBUG("[kw2xrf] enabling RX/TX completion and start events"); + kw2xrf_clear_dreg_bit(dev, MKW2XDM_PHY_CTRL2, MKW2XDM_PHY_CTRL2_RX_WMRK_MSK); + kw2xrf_clear_dreg_bit(dev, MKW2XDM_PHY_CTRL2, MKW2XDM_PHY_CTRL2_RXMSK); + kw2xrf_clear_dreg_bit(dev, MKW2XDM_PHY_CTRL2, MKW2XDM_PHY_CTRL2_TXMSK); DEBUG("[kw2xrf] setup finished\n"); } diff --git a/drivers/kw2xrf/kw2xrf_getset.c b/drivers/kw2xrf/kw2xrf_getset.c index e69e006a4c..3e2d628475 100644 --- a/drivers/kw2xrf/kw2xrf_getset.c +++ b/drivers/kw2xrf/kw2xrf_getset.c @@ -398,22 +398,6 @@ void kw2xrf_set_option(kw2xrf_t *dev, uint16_t option, bool state) MKW2XDM_PHY_CTRL1_RXACKRQD); break; - case KW2XRF_OPT_TELL_RX_START: - kw2xrf_clear_dreg_bit(dev, MKW2XDM_PHY_CTRL2, - MKW2XDM_PHY_CTRL2_RX_WMRK_MSK); - break; - - case KW2XRF_OPT_TELL_RX_END: - kw2xrf_clear_dreg_bit(dev, MKW2XDM_PHY_CTRL2, - MKW2XDM_PHY_CTRL2_RXMSK); - break; - - case KW2XRF_OPT_TELL_TX_END: - kw2xrf_clear_dreg_bit(dev, MKW2XDM_PHY_CTRL2, - MKW2XDM_PHY_CTRL2_TXMSK); - break; - - case KW2XRF_OPT_TELL_TX_START: default: /* do nothing */ break; @@ -453,23 +437,6 @@ void kw2xrf_set_option(kw2xrf_t *dev, uint16_t option, bool state) MKW2XDM_PHY_CTRL1_RXACKRQD); break; - case KW2XRF_OPT_TELL_RX_START: - kw2xrf_set_dreg_bit(dev, MKW2XDM_PHY_CTRL2, - MKW2XDM_PHY_CTRL2_RX_WMRK_MSK); - break; - - case KW2XRF_OPT_TELL_RX_END: - kw2xrf_set_dreg_bit(dev, MKW2XDM_PHY_CTRL2, - MKW2XDM_PHY_CTRL2_RXMSK); - break; - - case KW2XRF_OPT_TELL_TX_END: - kw2xrf_set_dreg_bit(dev, MKW2XDM_PHY_CTRL2, - MKW2XDM_PHY_CTRL2_TXMSK); - break; - - - case KW2XRF_OPT_TELL_TX_START: default: /* do nothing */ break; diff --git a/drivers/kw2xrf/kw2xrf_netdev.c b/drivers/kw2xrf/kw2xrf_netdev.c index 4ee4b17ec0..3f18bd2a9a 100644 --- a/drivers/kw2xrf/kw2xrf_netdev.c +++ b/drivers/kw2xrf/kw2xrf_netdev.c @@ -298,23 +298,10 @@ int _get(netdev_t *netdev, netopt_t opt, void *value, size_t len) return sizeof(netopt_enable_t); case NETOPT_RX_START_IRQ: - *((netopt_enable_t *)value) = - !!(dev->netdev.flags & KW2XRF_OPT_TELL_RX_START); - return sizeof(netopt_enable_t); - case NETOPT_RX_END_IRQ: - *((netopt_enable_t *)value) = - !!(dev->netdev.flags & KW2XRF_OPT_TELL_RX_END); - return sizeof(netopt_enable_t); - case NETOPT_TX_START_IRQ: - *((netopt_enable_t *)value) = - !!(dev->netdev.flags & KW2XRF_OPT_TELL_TX_START); - return sizeof(netopt_enable_t); - case NETOPT_TX_END_IRQ: - *((netopt_enable_t *)value) = - !!(dev->netdev.flags & KW2XRF_OPT_TELL_TX_END); + *((netopt_enable_t *)value) = NETOPT_ENABLE; return sizeof(netopt_enable_t); case NETOPT_AUTOCCA: @@ -482,30 +469,6 @@ static int _set(netdev_t *netdev, netopt_t opt, const void *value, size_t len) res = sizeof(netopt_enable_t); break; - case NETOPT_RX_START_IRQ: - kw2xrf_set_option(dev, KW2XRF_OPT_TELL_RX_START, - ((bool *)value)[0]); - res = sizeof(netopt_enable_t); - break; - - case NETOPT_RX_END_IRQ: - kw2xrf_set_option(dev, KW2XRF_OPT_TELL_RX_END, - ((bool *)value)[0]); - res = sizeof(netopt_enable_t); - break; - - case NETOPT_TX_START_IRQ: - kw2xrf_set_option(dev, KW2XRF_OPT_TELL_TX_START, - ((bool *)value)[0]); - res = sizeof(netopt_enable_t); - break; - - case NETOPT_TX_END_IRQ: - kw2xrf_set_option(dev, KW2XRF_OPT_TELL_TX_END, - ((bool *)value)[0]); - res = sizeof(netopt_enable_t); - break; - case NETOPT_AUTOCCA: kw2xrf_set_option(dev, KW2XRF_OPT_AUTOCCA, ((bool *)value)[0]); From 134f323fb197783ea870bc80d93c7fff791068b5 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 26 Feb 2021 11:14:54 +0100 Subject: [PATCH 08/12] drivers/kw41zrf: make TX/RX IRQs read only This brings the implementation in sync with the API. --- drivers/kw41zrf/include/kw41zrf_getset.h | 4 -- drivers/kw41zrf/kw41zrf.c | 3 ++ drivers/kw41zrf/kw41zrf_getset.c | 32 ------------- drivers/kw41zrf/kw41zrf_netdev.c | 58 +++--------------------- 4 files changed, 10 insertions(+), 87 deletions(-) diff --git a/drivers/kw41zrf/include/kw41zrf_getset.h b/drivers/kw41zrf/include/kw41zrf_getset.h index 5148941912..0238a6421b 100644 --- a/drivers/kw41zrf/include/kw41zrf_getset.h +++ b/drivers/kw41zrf/include/kw41zrf_getset.h @@ -33,10 +33,6 @@ extern "C" { #define KW41ZRF_OPT_CSMA (0x01u) /**< use CSMA/CA algorithm for sending */ #define KW41ZRF_OPT_PROMISCUOUS (0x02u) /**< promiscuous mode active */ #define KW41ZRF_OPT_PRELOADING (0x04u) /**< preloading enabled */ -#define KW41ZRF_OPT_TELL_TX_START (0x08u) /**< notify MAC layer on TX start */ -#define KW41ZRF_OPT_TELL_TX_END (0x10u) /**< notify MAC layer on TX finished */ -#define KW41ZRF_OPT_TELL_RX_START (0x20u) /**< notify MAC layer on RX start */ -#define KW41ZRF_OPT_TELL_RX_END (0x40u) /**< notify MAC layer on RX finished */ #define KW41ZRF_OPT_AUTOACK (0x80u) /**< automatic sending of ACKs */ #define KW41ZRF_OPT_ACK_PENDING (0x81u) /**< set pending bit on auto ACKs */ /** @} */ diff --git a/drivers/kw41zrf/kw41zrf.c b/drivers/kw41zrf/kw41zrf.c index 94a18857e6..08684e4ed4 100644 --- a/drivers/kw41zrf/kw41zrf.c +++ b/drivers/kw41zrf/kw41zrf.c @@ -96,6 +96,9 @@ int kw41zrf_init(kw41zrf_t *dev, kw41zrf_cb_t cb) /* Allow radio interrupts */ kw41zrf_unmask_irqs(); + DEBUG("[kw41zrf] enabling RX start IRQs\n"); + bit_clear32(&ZLL->PHY_CTRL, ZLL_PHY_CTRL_RX_WMRK_MSK_SHIFT); + DEBUG("[kw41zrf] init finished\n"); return 0; diff --git a/drivers/kw41zrf/kw41zrf_getset.c b/drivers/kw41zrf/kw41zrf_getset.c index ac8e6cc276..0db41996ac 100644 --- a/drivers/kw41zrf/kw41zrf_getset.c +++ b/drivers/kw41zrf/kw41zrf_getset.c @@ -195,7 +195,6 @@ void kw41zrf_set_option(kw41zrf_t *dev, uint8_t option, uint8_t state) case KW41ZRF_OPT_PROMISCUOUS: case KW41ZRF_OPT_AUTOACK: case KW41ZRF_OPT_ACK_PENDING: - case KW41ZRF_OPT_TELL_RX_START: LOG_ERROR("[kw41zrf] Attempt to modify option %04x while radio is sleeping\n", (unsigned) option); assert(0); @@ -234,22 +233,6 @@ void kw41zrf_set_option(kw41zrf_t *dev, uint8_t option, uint8_t state) bit_set32(&ZLL->SAM_TABLE, ZLL_SAM_TABLE_ACK_FRM_PND_SHIFT); break; - case KW41ZRF_OPT_TELL_RX_START: - DEBUG("[kw41zrf] enable: TELL_RX_START\n"); - bit_clear32(&ZLL->PHY_CTRL, ZLL_PHY_CTRL_RX_WMRK_MSK_SHIFT); - break; - - case KW41ZRF_OPT_TELL_RX_END: - DEBUG("[kw41zrf] enable: TELL_RX_END\n"); - break; - - case KW41ZRF_OPT_TELL_TX_END: - DEBUG("[kw41zrf] enable: TELL_TX_END\n"); - break; - - case KW41ZRF_OPT_TELL_TX_START: - DEBUG("[kw41zrf] enable: TELL_TX_START (ignored)\n"); - default: /* do nothing */ break; @@ -280,21 +263,6 @@ void kw41zrf_set_option(kw41zrf_t *dev, uint8_t option, uint8_t state) bit_clear32(&ZLL->SAM_TABLE, ZLL_SAM_TABLE_ACK_FRM_PND_SHIFT); break; - case KW41ZRF_OPT_TELL_RX_START: - DEBUG("[kw41zrf] disable: TELL_RX_START\n"); - bit_set32(&ZLL->PHY_CTRL, ZLL_PHY_CTRL_RX_WMRK_MSK_SHIFT); - break; - - case KW41ZRF_OPT_TELL_RX_END: - DEBUG("[kw41zrf] disable: TELL_RX_END\n"); - break; - - case KW41ZRF_OPT_TELL_TX_END: - DEBUG("[kw41zrf] disable: TELL_TX_END\n"); - break; - - case KW41ZRF_OPT_TELL_TX_START: - DEBUG("[kw41zrf] disable: TELL_TX_START (ignored)\n"); default: /* do nothing */ break; diff --git a/drivers/kw41zrf/kw41zrf_netdev.c b/drivers/kw41zrf/kw41zrf_netdev.c index 4496499470..0ee1d18068 100644 --- a/drivers/kw41zrf/kw41zrf_netdev.c +++ b/drivers/kw41zrf/kw41zrf_netdev.c @@ -457,27 +457,11 @@ int kw41zrf_netdev_get(netdev_t *netdev, netopt_t opt, void *value, size_t len) return sizeof(netopt_enable_t); case NETOPT_RX_START_IRQ: - assert(len >= sizeof(netopt_enable_t)); - *((netopt_enable_t *)value) = - !!(dev->flags & KW41ZRF_OPT_TELL_RX_START); - return sizeof(netopt_enable_t); - case NETOPT_RX_END_IRQ: - assert(len >= sizeof(netopt_enable_t)); - *((netopt_enable_t *)value) = - !!(dev->flags & KW41ZRF_OPT_TELL_RX_END); - return sizeof(netopt_enable_t); - case NETOPT_TX_START_IRQ: - assert(len >= sizeof(netopt_enable_t)); - *((netopt_enable_t *)value) = - !!(dev->flags & KW41ZRF_OPT_TELL_TX_START); - return sizeof(netopt_enable_t); - case NETOPT_TX_END_IRQ: assert(len >= sizeof(netopt_enable_t)); - *((netopt_enable_t *)value) = - !!(dev->flags & KW41ZRF_OPT_TELL_TX_END); + *((netopt_enable_t *)value) = NETOPT_ENABLE; return sizeof(netopt_enable_t); case NETOPT_CSMA: @@ -661,27 +645,6 @@ static int kw41zrf_netdev_set(netdev_t *netdev, netopt_t opt, const void *value, res = sizeof(const netopt_enable_t); break; - case NETOPT_RX_END_IRQ: - assert(len <= sizeof(const netopt_enable_t)); - kw41zrf_set_option(dev, KW41ZRF_OPT_TELL_RX_END, - *((const netopt_enable_t *)value)); - res = sizeof(const netopt_enable_t); - break; - - case NETOPT_TX_START_IRQ: - assert(len <= sizeof(const netopt_enable_t)); - kw41zrf_set_option(dev, KW41ZRF_OPT_TELL_TX_START, - *((const netopt_enable_t *)value)); - res = sizeof(const netopt_enable_t); - break; - - case NETOPT_TX_END_IRQ: - assert(len <= sizeof(const netopt_enable_t)); - kw41zrf_set_option(dev, KW41ZRF_OPT_TELL_TX_END, - *((const netopt_enable_t *)value)); - res = sizeof(const netopt_enable_t); - break; - case NETOPT_CSMA_RETRIES: assert(len <= sizeof(uint8_t)); dev->csma_max_backoffs = *((const uint8_t*)value); @@ -765,13 +728,6 @@ static int kw41zrf_netdev_set(netdev_t *netdev, netopt_t opt, const void *value, res = sizeof(const netopt_enable_t); break; - case NETOPT_RX_START_IRQ: - assert(len <= sizeof(const netopt_enable_t)); - kw41zrf_set_option(dev, KW41ZRF_OPT_TELL_RX_START, - *((const netopt_enable_t *)value)); - res = sizeof(const netopt_enable_t); - break; - case NETOPT_CSMA: assert(len <= sizeof(const netopt_enable_t)); kw41zrf_set_option(dev, KW41ZRF_OPT_CSMA, @@ -931,7 +887,7 @@ static uint32_t _isr_event_seq_t_ccairq(kw41zrf_t *dev, uint32_t irqsts) kw41zrf_abort_sequence(dev); kw41zrf_set_sequence(dev, dev->idle_seq); - if (dev->flags & KW41ZRF_OPT_TELL_TX_END) { + if (dev->netdev.netdev.event_callback) { dev->netdev.netdev.event_callback(&dev->netdev.netdev, NETDEV_EVENT_TX_MEDIUM_BUSY); LOG_INFO("[kw41zrf] dropping frame after %u backoffs\n", dev->csma_num_backoffs); @@ -945,7 +901,7 @@ static uint32_t _isr_event_seq_t_ccairq(kw41zrf_t *dev, uint32_t irqsts) ZLL_LQI_AND_RSSI_CCA1_ED_FNL_SHIFT), dev->csma_num_backoffs ); - if (dev->flags & KW41ZRF_OPT_TELL_TX_START) { + if (dev->netdev.netdev.event_callback) { /* TX will start automatically after CCA check succeeded */ dev->netdev.netdev.event_callback(&dev->netdev.netdev, NETDEV_EVENT_TX_STARTED); } @@ -962,7 +918,7 @@ static uint32_t _isr_event_seq_r(kw41zrf_t *dev, uint32_t irqsts) if (irqsts & ZLL_IRQSTS_RXWTRMRKIRQ_MASK) { DEBUG("[kw41zrf] RXWTRMRKIRQ (R)\n"); handled_irqs |= ZLL_IRQSTS_RXWTRMRKIRQ_MASK; - if (dev->flags & KW41ZRF_OPT_TELL_RX_START) { + if (dev->netdev.netdev.event_callback) { dev->netdev.netdev.event_callback(&dev->netdev.netdev, NETDEV_EVENT_RX_STARTED); } } @@ -1017,7 +973,7 @@ static uint32_t _isr_event_seq_r(kw41zrf_t *dev, uint32_t irqsts) /* Block XCVSEQ_RECEIVE until netdev->recv has been called */ dev->recv_blocked = 1; kw41zrf_set_sequence(dev, dev->idle_seq); - if (dev->flags & KW41ZRF_OPT_TELL_RX_END) { + if (dev->netdev.netdev.event_callback) { dev->netdev.netdev.event_callback(&dev->netdev.netdev, NETDEV_EVENT_RX_COMPLETE); } return handled_irqs; @@ -1045,7 +1001,7 @@ static uint32_t _isr_event_seq_t(kw41zrf_t *dev, uint32_t irqsts) DEBUG("[kw41zrf] SEQIRQ (T)\n"); handled_irqs |= ZLL_IRQSTS_SEQIRQ_MASK; - if (dev->flags & KW41ZRF_OPT_TELL_TX_END) { + if (dev->netdev.netdev.event_callback) { dev->netdev.netdev.event_callback(&dev->netdev.netdev, NETDEV_EVENT_TX_COMPLETE); } KW41ZRF_LED_TX_OFF; @@ -1126,7 +1082,7 @@ static uint32_t _isr_event_seq_tr(kw41zrf_t *dev, uint32_t irqsts) assert(!kw41zrf_is_dsm()); kw41zrf_set_sequence(dev, dev->idle_seq); - if (dev->flags & KW41ZRF_OPT_TELL_TX_END) { + if (dev->netdev.netdev.event_callback) { if (seq_ctrl_sts & ZLL_SEQ_CTRL_STS_TC3_ABORTED_MASK) { LOG_DEBUG("[kw41zrf] RXACK timeout (TR)\n"); dev->netdev.netdev.event_callback(&dev->netdev.netdev, NETDEV_EVENT_TX_NOACK); From 08063bebfe507d92300c0364ffe0772a3465c6df Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 26 Feb 2021 11:23:45 +0100 Subject: [PATCH 09/12] drivers/at86rf215: make TX/RX IRQs read only This brings the implementation in sync with the API. --- drivers/at86rf215/at86rf215.c | 3 ++ drivers/at86rf215/at86rf215_getset.c | 8 ----- drivers/at86rf215/at86rf215_netdev.c | 53 ++++------------------------ drivers/include/at86rf215.h | 4 --- 4 files changed, 9 insertions(+), 59 deletions(-) diff --git a/drivers/at86rf215/at86rf215.c b/drivers/at86rf215/at86rf215.c index 2e0822dd6b..6a88d527d4 100644 --- a/drivers/at86rf215/at86rf215.c +++ b/drivers/at86rf215/at86rf215.c @@ -93,6 +93,9 @@ void at86rf215_reset_and_cfg(at86rf215_t *dev) /* default to requesting ACKs, just like at86rf2xx */ const netopt_enable_t enable = NETOPT_ENABLE; netdev_ieee802154_set(&dev->netdev, NETOPT_ACK_REQ, &enable, sizeof(enable)); + + /* enable RX start IRQs */ + at86rf215_reg_or(dev, dev->BBC->RG_IRQM, BB_IRQ_RXAM); } void at86rf215_reset(at86rf215_t *dev) diff --git a/drivers/at86rf215/at86rf215_getset.c b/drivers/at86rf215/at86rf215_getset.c index 7a1f65f13a..e5fd8e702a 100644 --- a/drivers/at86rf215/at86rf215_getset.c +++ b/drivers/at86rf215/at86rf215_getset.c @@ -269,14 +269,6 @@ void at86rf215_set_option(at86rf215_t *dev, uint16_t option, bool state) : (dev->flags & ~option); switch (option) { - case AT86RF215_OPT_TELL_RX_START: - if (state) { - at86rf215_reg_or(dev, dev->BBC->RG_IRQM, BB_IRQ_RXAM); - } else { - at86rf215_reg_and(dev, dev->BBC->RG_IRQM, ~BB_IRQ_RXAM); - } - - break; case AT86RF215_OPT_PROMISCUOUS: if (state) { at86rf215_reg_or(dev, dev->BBC->RG_AFC0, AFC0_PM_MASK); diff --git a/drivers/at86rf215/at86rf215_netdev.c b/drivers/at86rf215/at86rf215_netdev.c index baf87da98b..1582223d2b 100644 --- a/drivers/at86rf215/at86rf215_netdev.c +++ b/drivers/at86rf215/at86rf215_netdev.c @@ -287,23 +287,10 @@ static int _get(netdev_t *netdev, netopt_t opt, void *val, size_t max_len) return sizeof(netopt_enable_t); case NETOPT_RX_START_IRQ: - *((netopt_enable_t *)val) = - !!(dev->flags & AT86RF215_OPT_TELL_RX_START); - return sizeof(netopt_enable_t); - case NETOPT_RX_END_IRQ: - *((netopt_enable_t *)val) = - !!(dev->flags & AT86RF215_OPT_TELL_RX_END); - return sizeof(netopt_enable_t); - case NETOPT_TX_START_IRQ: - *((netopt_enable_t *)val) = - !!(dev->flags & AT86RF215_OPT_TELL_TX_START); - return sizeof(netopt_enable_t); - case NETOPT_TX_END_IRQ: - *((netopt_enable_t *)val) = - !!(dev->flags & AT86RF215_OPT_TELL_TX_END); + *((netopt_enable_t *)val) = NETOPT_ENABLE; return sizeof(netopt_enable_t); case NETOPT_CSMA: @@ -587,30 +574,6 @@ static int _set(netdev_t *netdev, netopt_t opt, const void *val, size_t len) res = sizeof(netopt_enable_t); break; - case NETOPT_RX_START_IRQ: - at86rf215_set_option(dev, AT86RF215_OPT_TELL_RX_START, - ((const bool *)val)[0]); - res = sizeof(netopt_enable_t); - break; - - case NETOPT_RX_END_IRQ: - at86rf215_set_option(dev, AT86RF215_OPT_TELL_RX_END, - ((const bool *)val)[0]); - res = sizeof(netopt_enable_t); - break; - - case NETOPT_TX_START_IRQ: - at86rf215_set_option(dev, AT86RF215_OPT_TELL_TX_START, - ((const bool *)val)[0]); - res = sizeof(netopt_enable_t); - break; - - case NETOPT_TX_END_IRQ: - at86rf215_set_option(dev, AT86RF215_OPT_TELL_TX_END, - ((const bool *)val)[0]); - res = sizeof(netopt_enable_t); - break; - case NETOPT_CSMA: at86rf215_set_option(dev, AT86RF215_OPT_CSMA, ((const bool *)val)[0]); @@ -869,7 +832,7 @@ static void _tx_end(at86rf215_t *dev, netdev_event_t event) at86rf215_tx_done(dev); - if (dev->flags & AT86RF215_OPT_TELL_TX_END) { + if (netdev->event_callback) { netdev->event_callback(netdev, event); } @@ -1028,9 +991,7 @@ static void _isr(netdev_t *netdev) uint8_t bb_irqs_enabled = BB_IRQ_RXFE | BB_IRQ_TXFE; /* not using IRQMM because we want to know about AGCH */ - if (dev->flags & AT86RF215_OPT_TELL_RX_START) { - bb_irqs_enabled |= BB_IRQ_RXAM; - } + bb_irqs_enabled |= BB_IRQ_RXAM; rf_irq_mask = at86rf215_reg_read(dev, dev->RF->RG_IRQS); bb_irq_mask = at86rf215_reg_read(dev, dev->BBC->RG_IRQS); @@ -1140,8 +1101,7 @@ static void _isr(netdev_t *netdev) at86rf215_rf_cmd(dev, CMD_RF_TX); /* This also tells the upper layer about retransmissions - should it be like that? */ - if (netdev->event_callback && - (dev->flags & AT86RF215_OPT_TELL_TX_START)) { + if (netdev->event_callback) { netdev->event_callback(netdev, NETDEV_EVENT_TX_STARTED); } } @@ -1177,8 +1137,7 @@ static void _isr(netdev_t *netdev) break; } - if ((bb_irq_mask & BB_IRQ_RXAM) && - (dev->flags & AT86RF215_OPT_TELL_RX_END)) { + if ((bb_irq_mask & BB_IRQ_RXAM) && netdev->event_callback) { /* will be executed in the same thread */ netdev->event_callback(netdev, NETDEV_EVENT_RX_STARTED); } @@ -1191,7 +1150,7 @@ static void _isr(netdev_t *netdev) bb_irq_mask &= ~BB_IRQ_RXFE; - if (dev->flags & AT86RF215_OPT_TELL_RX_END) { + if (netdev->event_callback) { /* will be executed in the same thread */ netdev->event_callback(netdev, NETDEV_EVENT_RX_COMPLETE); } diff --git a/drivers/include/at86rf215.h b/drivers/include/at86rf215.h index d9ef609a4d..97acfd9b59 100644 --- a/drivers/include/at86rf215.h +++ b/drivers/include/at86rf215.h @@ -303,10 +303,6 @@ typedef enum { * @name Internal device option flags * @{ */ -#define AT86RF215_OPT_TELL_TX_START (0x0001) /**< notify MAC layer on TX start */ -#define AT86RF215_OPT_TELL_TX_END (0x0002) /**< notify MAC layer on TX finished */ -#define AT86RF215_OPT_TELL_RX_START (0x0004) /**< notify MAC layer on RX start */ -#define AT86RF215_OPT_TELL_RX_END (0x0008) /**< notify MAC layer on RX finished */ #define AT86RF215_OPT_CSMA (0x0010) /**< CSMA active */ #define AT86RF215_OPT_PROMISCUOUS (0x0020) /**< promiscuous mode active */ #define AT86RF215_OPT_PRELOADING (0x0040) /**< preloading enabled */ From 1d0ee42046943780a753da7fefc957cccac19d3a Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 26 Feb 2021 11:32:45 +0100 Subject: [PATCH 10/12] drivers/at86rf2xx: make TX/RX IRQs read only This brings the implementation in sync with the API. --- drivers/at86rf2xx/at86rf2xx.c | 11 +++++-- drivers/at86rf2xx/at86rf2xx_getset.c | 8 ----- drivers/at86rf2xx/at86rf2xx_netdev.c | 49 ++++------------------------ drivers/include/at86rf2xx.h | 14 ++------ 4 files changed, 16 insertions(+), 66 deletions(-) diff --git a/drivers/at86rf2xx/at86rf2xx.c b/drivers/at86rf2xx/at86rf2xx.c index 987765249a..866877938d 100644 --- a/drivers/at86rf2xx/at86rf2xx.c +++ b/drivers/at86rf2xx/at86rf2xx.c @@ -169,6 +169,7 @@ static void at86rf2xx_enable_smart_idle(at86rf2xx_t *dev) void at86rf2xx_reset(at86rf2xx_t *dev) { + uint8_t tmp; netdev_ieee802154_reset(&dev->netdev); /* Reset state machine to ensure a known state */ @@ -204,7 +205,7 @@ void at86rf2xx_reset(at86rf2xx_t *dev) #if !defined(MODULE_AT86RFA1) && !defined(MODULE_AT86RFR2) /* don't populate masked interrupt flags to IRQ_STATUS register */ - uint8_t tmp = at86rf2xx_reg_read(dev, AT86RF2XX_REG__TRX_CTRL_1); + tmp = at86rf2xx_reg_read(dev, AT86RF2XX_REG__TRX_CTRL_1); tmp &= ~(AT86RF2XX_TRX_CTRL_1_MASK__IRQ_MASK_MODE); at86rf2xx_reg_write(dev, AT86RF2XX_REG__TRX_CTRL_1, tmp); #endif @@ -243,6 +244,11 @@ void at86rf2xx_reset(at86rf2xx_t *dev) /* go into RX state */ at86rf2xx_set_state(dev, AT86RF2XX_PHY_STATE_RX); + /* Enable RX start IRQ */ + tmp = at86rf2xx_reg_read(dev, AT86RF2XX_REG__IRQ_MASK); + tmp |= AT86RF2XX_IRQ_STATUS_MASK__RX_START; + at86rf2xx_reg_write(dev, AT86RF2XX_REG__IRQ_MASK, tmp); + DEBUG("at86rf2xx_reset(): reset complete.\n"); } @@ -292,8 +298,7 @@ void at86rf2xx_tx_exec(at86rf2xx_t *dev) /* trigger sending of pre-loaded frame */ at86rf2xx_reg_write(dev, AT86RF2XX_REG__TRX_STATE, AT86RF2XX_TRX_STATE__TX_START); - if (netdev->event_callback && - (dev->flags & AT86RF2XX_OPT_TELL_TX_START)) { + if (netdev->event_callback) { netdev->event_callback(netdev, NETDEV_EVENT_TX_STARTED); } } diff --git a/drivers/at86rf2xx/at86rf2xx_getset.c b/drivers/at86rf2xx/at86rf2xx_getset.c index 91e491b541..126e1619e4 100644 --- a/drivers/at86rf2xx/at86rf2xx_getset.c +++ b/drivers/at86rf2xx/at86rf2xx_getset.c @@ -446,14 +446,6 @@ void at86rf2xx_set_option(at86rf2xx_t *dev, uint16_t option, bool state) : (tmp | AT86RF2XX_CSMA_SEED_1__AACK_DIS_ACK); at86rf2xx_reg_write(dev, AT86RF2XX_REG__CSMA_SEED_1, tmp); break; - case AT86RF2XX_OPT_TELL_RX_START: - DEBUG("[at86rf2xx] opt: %s SFD IRQ\n", - (state ? "enable" : "disable")); - tmp = at86rf2xx_reg_read(dev, AT86RF2XX_REG__IRQ_MASK); - tmp = (state) ? (tmp | AT86RF2XX_IRQ_STATUS_MASK__RX_START) - : (tmp & ~AT86RF2XX_IRQ_STATUS_MASK__RX_START); - at86rf2xx_reg_write(dev, AT86RF2XX_REG__IRQ_MASK, tmp); - break; case AT86RF2XX_OPT_ACK_PENDING: DEBUG("[at86rf2xx] opt: enabling pending ACKs\n"); tmp = at86rf2xx_reg_read(dev, AT86RF2XX_REG__CSMA_SEED_1); diff --git a/drivers/at86rf2xx/at86rf2xx_netdev.c b/drivers/at86rf2xx/at86rf2xx_netdev.c index 29a1b7ad52..6988dfbcfe 100644 --- a/drivers/at86rf2xx/at86rf2xx_netdev.c +++ b/drivers/at86rf2xx/at86rf2xx_netdev.c @@ -346,23 +346,10 @@ static int _get(netdev_t *netdev, netopt_t opt, void *val, size_t max_len) break; case NETOPT_RX_START_IRQ: - *((netopt_enable_t *)val) = - !!(dev->flags & AT86RF2XX_OPT_TELL_RX_START); - return sizeof(netopt_enable_t); - case NETOPT_RX_END_IRQ: - *((netopt_enable_t *)val) = - !!(dev->flags & AT86RF2XX_OPT_TELL_RX_END); - return sizeof(netopt_enable_t); - case NETOPT_TX_START_IRQ: - *((netopt_enable_t *)val) = - !!(dev->flags & AT86RF2XX_OPT_TELL_TX_START); - return sizeof(netopt_enable_t); - case NETOPT_TX_END_IRQ: - *((netopt_enable_t *)val) = - !!(dev->flags & AT86RF2XX_OPT_TELL_TX_END); + *((netopt_enable_t *)val) = NETOPT_ENABLE; return sizeof(netopt_enable_t); case NETOPT_CSMA: @@ -601,30 +588,6 @@ static int _set(netdev_t *netdev, netopt_t opt, const void *val, size_t len) } break; - case NETOPT_RX_START_IRQ: - at86rf2xx_set_option(dev, AT86RF2XX_OPT_TELL_RX_START, - ((const bool *)val)[0]); - res = sizeof(netopt_enable_t); - break; - - case NETOPT_RX_END_IRQ: - at86rf2xx_set_option(dev, AT86RF2XX_OPT_TELL_RX_END, - ((const bool *)val)[0]); - res = sizeof(netopt_enable_t); - break; - - case NETOPT_TX_START_IRQ: - at86rf2xx_set_option(dev, AT86RF2XX_OPT_TELL_TX_START, - ((const bool *)val)[0]); - res = sizeof(netopt_enable_t); - break; - - case NETOPT_TX_END_IRQ: - at86rf2xx_set_option(dev, AT86RF2XX_OPT_TELL_TX_END, - ((const bool *)val)[0]); - res = sizeof(netopt_enable_t); - break; - case NETOPT_CSMA: if (!IS_ACTIVE(AT86RF2XX_BASIC_MODE)) { at86rf2xx_set_option(dev, AT86RF2XX_OPT_CSMA, @@ -716,7 +679,7 @@ static void _isr_send_complete(at86rf2xx_t *dev, uint8_t trac_status) DEBUG("[at86rf2xx] EVT - TX_END\n"); - if (netdev->event_callback && (dev->flags & AT86RF2XX_OPT_TELL_TX_END)) { + if (netdev->event_callback) { switch (trac_status) { #ifdef MODULE_OPENTHREAD case AT86RF2XX_TRX_STATE__TRAC_SUCCESS: @@ -751,7 +714,10 @@ static void _isr_send_complete(at86rf2xx_t *dev, uint8_t trac_status) static inline void _isr_recv_complete(netdev_t *netdev) { - at86rf2xx_t *dev = (at86rf2xx_t *) netdev; + at86rf2xx_t *dev = (at86rf2xx_t *)netdev; + if (!netdev->event_callback) { + return; + } if (IS_ACTIVE(AT86RF2XX_BASIC_MODE)) { uint8_t phy_status = at86rf2xx_reg_read(dev, AT86RF2XX_REG__PHY_RSSI); bool crc_ok = phy_status & AT86RF2XX_PHY_RSSI_MASK__RX_CRC_VALID; @@ -803,9 +769,6 @@ static void _isr(netdev_t *netdev) if ((state == AT86RF2XX_PHY_STATE_RX) || (state == AT86RF2XX_PHY_STATE_RX_BUSY)) { DEBUG("[at86rf2xx] EVT - RX_END\n"); - if (!(dev->flags & AT86RF2XX_OPT_TELL_RX_END)) { - return; - } _isr_recv_complete(netdev); diff --git a/drivers/include/at86rf2xx.h b/drivers/include/at86rf2xx.h index 04afb884f8..d9a20e5eec 100644 --- a/drivers/include/at86rf2xx.h +++ b/drivers/include/at86rf2xx.h @@ -177,21 +177,11 @@ extern "C" { * @name Internal device option flags * @{ */ -#define AT86RF2XX_OPT_TELL_TX_START (0x0001) /**< notify MAC layer on TX - * start */ -#define AT86RF2XX_OPT_TELL_TX_END (0x0002) /**< notify MAC layer on TX - * finished */ -#define AT86RF2XX_OPT_TELL_RX_START (0x0004) /**< notify MAC layer on RX - * start */ -#define AT86RF2XX_OPT_TELL_RX_END (0x0008) /**< notify MAC layer on RX - * finished */ #define AT86RF2XX_OPT_CSMA (0x0010) /**< CSMA active */ -#define AT86RF2XX_OPT_PROMISCUOUS (0x0020) /**< promiscuous mode - * active */ +#define AT86RF2XX_OPT_PROMISCUOUS (0x0020) /**< promiscuous mode active */ #define AT86RF2XX_OPT_PRELOADING (0x0040) /**< preloading enabled */ #define AT86RF2XX_OPT_AUTOACK (0x0080) /**< Auto ACK active */ -#define AT86RF2XX_OPT_ACK_PENDING (0x0100) /**< ACK frames with data - * pending */ +#define AT86RF2XX_OPT_ACK_PENDING (0x0100) /**< ACK frames with data pending */ /** @} */ From 3a378c24a23febaf38e3383924e26f19df7f5525 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 26 Feb 2021 11:39:18 +0100 Subject: [PATCH 11/12] drivers/cc2420: make TX/RX IRQs read only This brings the implementation in sync with the API. --- drivers/cc2420/cc2420.c | 2 +- drivers/cc2420/cc2420_getset.c | 16 ---------------- drivers/cc2420/cc2420_netdev.c | 21 ++------------------- drivers/cc2420/include/cc2420_registers.h | 17 ++++------------- 4 files changed, 7 insertions(+), 49 deletions(-) diff --git a/drivers/cc2420/cc2420.c b/drivers/cc2420/cc2420.c index 7e6e46d0f8..2ce998b399 100644 --- a/drivers/cc2420/cc2420.c +++ b/drivers/cc2420/cc2420.c @@ -145,7 +145,7 @@ size_t cc2420_tx_prepare(cc2420_t *dev, const iolist_t *iolist) void cc2420_tx_exec(cc2420_t *dev) { /* trigger the transmission */ - if (dev->options & CC2420_OPT_TELL_TX_START) { + if (dev->netdev.netdev.event_callback) { dev->netdev.netdev.event_callback(&dev->netdev.netdev, NETDEV_EVENT_TX_STARTED); } diff --git a/drivers/cc2420/cc2420_getset.c b/drivers/cc2420/cc2420_getset.c index a0bed1c68b..4c33b4f3fa 100644 --- a/drivers/cc2420/cc2420_getset.c +++ b/drivers/cc2420/cc2420_getset.c @@ -199,14 +199,6 @@ int cc2420_set_option(cc2420_t *dev, uint16_t option, bool state) DEBUG("cc2420: set_opt: CC2420_OPT_PRELOADING\n"); break; - case CC2420_OPT_TELL_TX_START: - case CC2420_OPT_TELL_TX_END: - case CC2420_OPT_TELL_RX_START: - case CC2420_OPT_TELL_RX_END: - DEBUG("cc2420: set_opt: TX/RX START/END\n"); - /* TODO */ - break; - default: return -ENOTSUP; } @@ -242,14 +234,6 @@ int cc2420_set_option(cc2420_t *dev, uint16_t option, bool state) DEBUG("cc2420: clr_opt: CC2420_OPT_PRELOADING\n"); break; - case CC2420_OPT_TELL_TX_START: - case CC2420_OPT_TELL_TX_END: - case CC2420_OPT_TELL_RX_START: - case CC2420_OPT_TELL_RX_END: - DEBUG("cc2420: clr_opt: TX/RX START/END\n"); - /* TODO */ - break; - default: return -ENOTSUP; } diff --git a/drivers/cc2420/cc2420_netdev.c b/drivers/cc2420/cc2420_netdev.c index 010c4ce3ea..4d238d478f 100644 --- a/drivers/cc2420/cc2420_netdev.c +++ b/drivers/cc2420/cc2420_netdev.c @@ -213,16 +213,11 @@ static int _get(netdev_t *netdev, netopt_t opt, void *val, size_t max_len) return opt_state(val, (dev->options & CC2420_OPT_PROMISCUOUS)); case NETOPT_RX_START_IRQ: - return opt_state(val, (dev->options & CC2420_OPT_TELL_RX_START)); - case NETOPT_RX_END_IRQ: - return opt_state(val, (dev->options & CC2420_OPT_TELL_TX_END)); - case NETOPT_TX_START_IRQ: - return opt_state(val, (dev->options & CC2420_OPT_TELL_RX_START)); - case NETOPT_TX_END_IRQ: - return opt_state(val, (dev->options & CC2420_OPT_TELL_RX_END)); + *((netopt_enable_t *)val) = NETOPT_ENABLE; + return sizeof(netopt_enable_t); default: return -ENOTSUP; @@ -280,18 +275,6 @@ static int _set(netdev_t *netdev, netopt_t opt, const void *val, size_t val_len) case NETOPT_PROMISCUOUSMODE: return cc2420_set_option(dev, CC2420_OPT_PROMISCUOUS, to_bool(val)); - case NETOPT_RX_START_IRQ: - return cc2420_set_option(dev, CC2420_OPT_TELL_RX_START, to_bool(val)); - - case NETOPT_RX_END_IRQ: - return cc2420_set_option(dev, CC2420_OPT_TELL_RX_END, to_bool(val)); - - case NETOPT_TX_START_IRQ: - return cc2420_set_option(dev, CC2420_OPT_TELL_TX_START, to_bool(val)); - - case NETOPT_TX_END_IRQ: - return cc2420_set_option(dev, CC2420_OPT_TELL_TX_END, to_bool(val)); - default: return ext; } diff --git a/drivers/cc2420/include/cc2420_registers.h b/drivers/cc2420/include/cc2420_registers.h index 00725aa1fb..83708c9408 100644 --- a/drivers/cc2420/include/cc2420_registers.h +++ b/drivers/cc2420/include/cc2420_registers.h @@ -31,17 +31,8 @@ extern "C" { */ #define CC2420_OPT_AUTOACK (0x0001) /**< auto ACKs active */ #define CC2420_OPT_CSMA (0x0002) /**< CSMA active */ -#define CC2420_OPT_PROMISCUOUS (0x0004) /**< promiscuous mode - * active */ +#define CC2420_OPT_PROMISCUOUS (0x0004) /**< promiscuous mode active */ #define CC2420_OPT_PRELOADING (0x0008) /**< preloading enabled */ -#define CC2420_OPT_TELL_TX_START (0x0010) /**< notify MAC layer on TX - * start */ -#define CC2420_OPT_TELL_TX_END (0x0020) /**< notify MAC layer on TX - * finished */ -#define CC2420_OPT_TELL_RX_START (0x0040) /**< notify MAC layer on RX - * start */ -#define CC2420_OPT_TELL_RX_END (0x0080) /**< notify MAC layer on RX - * finished */ /** @} */ /** @@ -49,11 +40,11 @@ extern "C" { * @{ */ enum { - CC2420_GOTO_PD, /**< power down */ + CC2420_GOTO_PD, /**< power down */ CC2420_GOTO_IDLE, /**< idle */ - CC2420_GOTO_RX, /**< receive state */ + CC2420_GOTO_RX, /**< receive state */ CC2420_GOTO_TXON, /**< transmit packet without CCA */ - CC2420_GOTO_TXONCCA /**< transmit packet using CCA */ + CC2420_GOTO_TXONCCA /**< transmit packet using CCA */ }; /** From 4cfcc4ee9eb1040d9dcd6759336517098b3727cc Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Tue, 2 Mar 2021 16:44:24 +0100 Subject: [PATCH 12/12] tests/driver_xbee: Update Makefile.ci The additional warning message increases the size of the .rodata section. For the Arduino Mega2560 this is placed in RAM (and flash), as flash is not mapped into the data address space. --- tests/driver_xbee/Makefile.ci | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/driver_xbee/Makefile.ci b/tests/driver_xbee/Makefile.ci index 309c8ed3d1..58ac828540 100644 --- a/tests/driver_xbee/Makefile.ci +++ b/tests/driver_xbee/Makefile.ci @@ -1,6 +1,7 @@ BOARD_INSUFFICIENT_MEMORY := \ arduino-duemilanove \ arduino-leonardo \ + arduino-mega2560 \ arduino-nano \ arduino-uno \ atmega328p \