Skip to content

ieee802154/at86rf2xx: port to Radio HAL #19015

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jia200x wants to merge 11 commits into
base: master
Choose a base branch
from
Open

ieee802154/at86rf2xx: port to Radio HAL #19015

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ea59bf2
ieee802154/nrf802154: fix missing ieee802154 dependency
jia200x Jan 20, 2023
ed94bb8
ieee802154/radio: add support for cipher_ops
jia200x Feb 23, 2023
c55f242
ieee802154/at86rf2xx: add initial Radio HAL support
jia200x Feb 23, 2023
64b482a
tests/ieee802154_hal: add support for AT86RF2XX
jia200x Dec 1, 2022
3a9ab03
tests/ieee802154_submac: add support for AT86RF2XX radios
jia200x Jan 20, 2023
f854a1c
tests/driver_at86rf2xx/Makefile.ci: add board to blacklist
jia200x Jan 20, 2023
ef40268
fixup! ieee802154/radio: add support for cipher_ops
jia200x Apr 13, 2023
3ddbc6d
fixup! ieee802154/at86rf2xx: add initial Radio HAL support
jia200x Apr 13, 2023
844fd5f
fixup! ieee802154/at86rf2xx: add initial Radio HAL support
jia200x Apr 13, 2023
c718568
fixup! ieee802154/at86rf2xx: add initial Radio HAL support
jia200x Apr 13, 2023
a8990e5
fixup! tests/ieee802154_hal: add support for AT86RF2XX
jia200x Apr 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cpu/nrf52/Makefile.dep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ ifneq (,$(filter nrf802154,$(USEMODULE)))
FEATURES_REQUIRED += periph_timer
FEATURES_REQUIRED += radio_nrf802154
USEMODULE += luid
USEMODULE += ieee802154
ifneq (,$(filter netdev,$(USEMODULE)))
USEMODULE += netdev_ieee802154_submac
endif
Expand Down
10 changes: 5 additions & 5 deletions drivers/at86rf2xx/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ menuconfig MODULE_AT86RF2XX
prompt "AT86RF2xx series radios" if !(MODULE_NETDEV_DEFAULT && HAVE_AT86RF2XX)
default (MODULE_NETDEV_DEFAULT && HAVE_AT86RF2XX)
depends on TEST_KCONFIG
select MODULE_IEEE802154
select MODULE_NETDEV
select MODULE_NETDEV_IEEE802154
select MODULE_NETDEV_LEGACY_API
select MODULE_ZTIMER
select MODULE_ZTIMER_USEC
select MODULE_EVENT
select MODULE_EVENT_THREAD
select MODULE_EVENT_THREAD_HIGHEST
select HAVE_IEEE802154_RADIO_HAL_INTERFACE

if MODULE_AT86RF2XX

Expand Down Expand Up @@ -84,7 +84,7 @@ config MODULE_AT86RF2XX_AES_SPI

config AT86RF2XX_OQPSK
bool "O-QPSK support"
select MODULE_NETDEV_IEEE802154_OQPSK
select MODULE_NETDEV_IEEE802154_OQPSK if MODULE_NETDEV
default y

endif # MODULE_AT86RF2XX
Expand Down
11 changes: 7 additions & 4 deletions drivers/at86rf2xx/Makefile.dep
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
DEFAULT_MODULE += auto_init_at86rf2xx
DEFAULT_MODULE += netdev_ieee802154_oqpsk

USEMODULE += ztimer
USEMODULE += ztimer_usec
USEMODULE += ieee802154
USEMODULE += netdev_ieee802154
USEMODULE += netdev_legacy_api
USEMODULE += event_thread
USEMODULE += event_thread_highest

ifneq (,$(filter netdev,$(USEMODULE)))
USEMODULE += netdev_ieee802154_submac
DEFAULT_MODULE += netdev_ieee802154_oqpsk
endif

# only needed for SPI based variants
ifeq (,$(filter at86rfa1 at86rfr2,$(USEMODULE)))

@benpicco benpicco Feb 24, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ifeq (,$(filter at86rfa1 at86rfr2,$(USEMODULE)))
ifeq (,$(filter at86rfa1 at86rfr2,$(USEMODULE)))
ifneq (,$(filter ieee802154_security,$(USEMODULE)))
USEMODULE += at86rf2xx_aes_spi
endif

@fabian18 fabian18 Feb 24, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep it optional whether to use hardware offloading. IIRC pings were actually faster without.

@benpicco benpicco Feb 24, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's probably due to all the spi_aquire() - we should move those to the entry points of rf_ops instead of doing it for every register read, but that's for a follow-up.

@fabian18 fabian18 Feb 24, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and at86rf230 does not have this feature

@jia200x jia200x Feb 24, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should I then just leave it as it is? or catch the at86rf230 case?

@fabian18 fabian18 Feb 24, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, we would not need cipher_modes if we know there is only at86rf2xx in use and for example not an additional at86rf215 or other 802.15.4 transceiver (I know its probably an uncommon case). To gain more from it we should test if we need cipher_ops or at86rf2xx_aes_spi.

USEMODULE += ieee802154_security

   text    data     bss     dec     hex filename
 106496     272   22328  129096   1f848 /home/fabian/forks/RIOT/examples/gnrc_networking/bin/miot-nucleo-f767zi/gnrc_networking.elf


ping -s 120 -c 50 fe80::4c49:6f54:4f76:102%7

2023-02-24 19:41:44,944 # --- fe80::4c49:6f54:4f76:102%7 PING statistics ---
2023-02-24 19:41:44,950 # 50 packets transmitted, 50 packets received, 0% packet loss
2023-02-24 19:41:44,953 # round-trip min/avg/max = 22.881/24.924/28.171 ms

USEMODULE += "ieee802154_security at86rf2xx_aes_spi"

   text    data     bss     dec     hex filename
 107328     272   22328  129928   1fb88 /home/fabian/forks/RIOT/examples/gnrc_networking/bin/miot-nucleo-f767zi/gnrc_networking.elf


ping -s 120 -c 50 fe80::4c49:6f54:4f76:102%7

2023-02-24 19:45:36,758 # --- fe80::4c49:6f54:4f76:102%7 PING statistics ---
2023-02-24 19:45:36,764 # 50 packets transmitted, 50 packets received, 0% packet loss
2023-02-24 19:45:36,768 # round-trip min/avg/max = 26.763/29.079/31.253 ms

@benpicco benpicco Feb 24, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably introduce a netdev_ieee802154_submac_soft_cipher (netdev_ieee802154_submac_sw_cipher?) (analogous to the proposed netdev_ieee802154_submac_soft_retrans) that drivers without HW crypto can then select.

@fabian18 fabian18 Feb 24, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. If we have that the right modules are selected dynamically base on the transceivers.
But changing the dependency of at86rf2xx_aes_spi in this PR is not really advisable.
Or would you still do it here?

@benpicco benpicco Feb 24, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no strong feelings about this, I was just surprised that it was not build in.
That's probably also why @jia200x didn't notice that the current version crashes when at86rf2xx_aes_spi is enabled, because when you want to test ieee802154_security it just works - because the HW acceleration is not used.

@jia200x jia200x Apr 13, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then should I just leave it as it is for now? We can address that in a follow up.

Expand Down
97 changes: 1 addition & 96 deletions drivers/at86rf2xx/at86rf2xx.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
#define ENABLE_DEBUG 0
#include "debug.h"

static void at86rf2xx_disable_clock_output(at86rf2xx_t *dev)
void at86rf2xx_disable_clock_output(at86rf2xx_t *dev)
{
#if AT86RF2XX_IS_PERIPH
(void) dev;
Expand Down Expand Up @@ -73,101 +73,6 @@ void at86rf2xx_disable_smart_idle(at86rf2xx_t *dev)
#endif
}

void at86rf2xx_reset(at86rf2xx_t *dev)
{
uint8_t tmp;

/* Reset state machine to ensure a known state */
if (dev->state == AT86RF2XX_STATE_P_ON) {
at86rf2xx_set_state(dev, AT86RF2XX_STATE_FORCE_TRX_OFF);
}

/* set default channel, page and TX power */
at86rf2xx_configure_phy(dev, AT86RF2XX_DEFAULT_CHANNEL, AT86RF2XX_DEFAULT_PAGE, AT86RF2XX_DEFAULT_TXPOWER);

/* set default options */

if (!IS_ACTIVE(AT86RF2XX_BASIC_MODE)) {
at86rf2xx_set_option(dev, AT86RF2XX_OPT_AUTOACK, true);
at86rf2xx_set_option(dev, AT86RF2XX_OPT_CSMA, true);
}

/* enable safe mode (protect RX FIFO until reading data starts) */
at86rf2xx_reg_write(dev, AT86RF2XX_REG__TRX_CTRL_2,
AT86RF2XX_TRX_CTRL_2_MASK__RX_SAFE_MODE);

#if !AT86RF2XX_IS_PERIPH
/* don't populate masked interrupt flags to IRQ_STATUS register */
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

/* configure smart idle listening feature */
at86rf2xx_enable_smart_idle(dev);

/* disable clock output to save power */
at86rf2xx_disable_clock_output(dev);

/* enable interrupts */
at86rf2xx_reg_write(dev, AT86RF2XX_REG__IRQ_MASK,
AT86RF2XX_IRQ_STATUS_MASK__TRX_END);

/* enable TX start interrupt for retry counter */
#if AT86RF2XX_HAVE_TX_START_IRQ
at86rf2xx_reg_write(dev, AT86RF2XX_REG__IRQ_MASK1,
AT86RF2XX_IRQ_STATUS_MASK1__TX_START);
#endif

/* clear interrupt flags */
at86rf2xx_reg_read(dev, AT86RF2XX_REG__IRQ_STATUS);

/* State to return after receiving or transmitting */
dev->idle_state = AT86RF2XX_PHY_STATE_RX;
/* 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");
}

void at86rf2xx_tx_prepare(at86rf2xx_t *dev)
{
uint8_t state;

dev->pending_tx++;
state = at86rf2xx_set_state(dev, AT86RF2XX_PHY_STATE_TX);
if (state != AT86RF2XX_PHY_STATE_TX) {
dev->idle_state = state;
}
dev->tx_frame_len = IEEE802154_FCS_LEN;
}

size_t at86rf2xx_tx_load(at86rf2xx_t *dev, const uint8_t *data,
size_t len, size_t offset)
{
dev->tx_frame_len += (uint8_t)len;
at86rf2xx_sram_write(dev, offset + 1, data, len);
return offset + len;
}

void at86rf2xx_tx_exec(at86rf2xx_t *dev)
{
#if AT86RF2XX_HAVE_RETRIES
dev->tx_retries = -1;
#endif

/* write frame length field in FIFO */
at86rf2xx_sram_write(dev, 0, &(dev->tx_frame_len), 1);
/* trigger sending of pre-loaded frame */
at86rf2xx_reg_write(dev, AT86RF2XX_REG__TRX_STATE,
AT86RF2XX_TRX_STATE__TX_START);
}

bool at86rf2xx_cca(at86rf2xx_t *dev)
{
uint8_t reg;
Expand Down
9 changes: 2 additions & 7 deletions drivers/at86rf2xx/at86rf2xx_getset.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,9 +353,6 @@ void at86rf2xx_set_option(at86rf2xx_t *dev, uint16_t option, bool state)

DEBUG("set option %i to %i\n", option, state);

/* set option field */
dev->flags = (state) ? (dev->flags | option)
: (dev->flags & ~option);
/* trigger option specific actions */
switch (option) {
case AT86RF2XX_OPT_CSMA:
Expand Down Expand Up @@ -434,8 +431,6 @@ static inline void _set_state(at86rf2xx_t *dev, uint8_t state, uint8_t cmd)
else {
while (at86rf2xx_get_status(dev) == AT86RF2XX_STATE_IN_PROGRESS) {}
}

dev->state = state;
}

uint8_t at86rf2xx_set_state(at86rf2xx_t *dev, uint8_t state)
Expand Down Expand Up @@ -473,18 +468,18 @@ uint8_t at86rf2xx_set_state(at86rf2xx_t *dev, uint8_t state)
/* Go to SLEEP mode from TRX_OFF */
#if AT86RF2XX_IS_PERIPH
/* reset interrupts states in device */
dev->irq_status = 0;
/* Setting SLPTR bit brings radio transceiver to sleep in in TRX_OFF*/
*AT86RF2XX_REG__TRXPR |= (AT86RF2XX_TRXPR_SLPTR);
#else
gpio_set(dev->params.sleep_pin);
#endif
dev->state = state;
dev->sleep = true;
}
else {
if (old_state == AT86RF2XX_STATE_SLEEP) {
DEBUG("at86rf2xx: waking up from sleep mode\n");
at86rf2xx_assert_awake(dev);
dev->sleep = false;
}
_set_state(dev, state, state);
}
Expand Down
20 changes: 11 additions & 9 deletions drivers/at86rf2xx/at86rf2xx_internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ void at86rf2xx_fb_stop(const at86rf2xx_t *dev)
uint8_t at86rf2xx_get_status(const at86rf2xx_t *dev)
{
/* if sleeping immediately return state */
if (dev->state == AT86RF2XX_STATE_SLEEP) {
return dev->state;
if (dev->sleep) {
return AT86RF2XX_STATE_SLEEP;
}

return (at86rf2xx_reg_read(dev, AT86RF2XX_REG__TRX_STATUS)
Expand All @@ -118,6 +118,7 @@ uint8_t at86rf2xx_get_status(const at86rf2xx_t *dev)

void at86rf2xx_assert_awake(at86rf2xx_t *dev)
{
uint8_t state;
if (at86rf2xx_get_status(dev) == AT86RF2XX_STATE_SLEEP) {
/* wake up and wait for transition to TRX_OFF */
#if AT86RF2XX_IS_PERIPH
Expand All @@ -135,14 +136,15 @@ void at86rf2xx_assert_awake(at86rf2xx_t *dev)
* Spin until we are actually awake
*/
do {
dev->state = at86rf2xx_reg_read(dev, AT86RF2XX_REG__TRX_STATUS)
state = at86rf2xx_reg_read(dev, AT86RF2XX_REG__TRX_STATUS)
& AT86RF2XX_TRX_STATUS_MASK__TRX_STATUS;
} while (dev->state != AT86RF2XX_TRX_STATUS__TRX_OFF);
} while (state != AT86RF2XX_TRX_STATUS__TRX_OFF);
}
}

void at86rf2xx_hardware_reset(at86rf2xx_t *dev)
{
uint8_t state;
/* trigger hardware reset */
#if AT86RF2XX_IS_PERIPH
/* set reset Bit */
Expand All @@ -158,10 +160,10 @@ void at86rf2xx_hardware_reset(at86rf2xx_t *dev)
* it remains P_ON. Otherwise, it should go to TRX_OFF
*/
do {
dev->state = at86rf2xx_reg_read(dev, AT86RF2XX_REG__TRX_STATUS)
state = at86rf2xx_reg_read(dev, AT86RF2XX_REG__TRX_STATUS)
& AT86RF2XX_TRX_STATUS_MASK__TRX_STATUS;
} while ((dev->state != AT86RF2XX_STATE_TRX_OFF)
&& (dev->state != AT86RF2XX_STATE_P_ON));
} while ((state != AT86RF2XX_STATE_TRX_OFF)
&& (state != AT86RF2XX_STATE_P_ON));
}

#if AT86RF2XX_RANDOM_NUMBER_GENERATOR
Expand All @@ -186,15 +188,15 @@ void at86rf2xx_get_random(at86rf2xx_t *dev, uint8_t *data, size_t len)
#endif

#if !AT86RF2XX_IS_PERIPH
void at86rf2xx_spi_init(at86rf2xx_t *dev, void (*irq_handler)(void *arg))
void at86rf2xx_spi_init(at86rf2xx_t *dev, void (*irq_handler)(void *arg), void *ctx)
{
/* initialize GPIOs */
spi_init_cs(dev->params.spi, dev->params.cs_pin);
gpio_init(dev->params.sleep_pin, GPIO_OUT);
gpio_clear(dev->params.sleep_pin);
gpio_init(dev->params.reset_pin, GPIO_OUT);
gpio_set(dev->params.reset_pin);
gpio_init_int(dev->params.int_pin, GPIO_IN, GPIO_RISING, irq_handler, dev);
gpio_init_int(dev->params.int_pin, GPIO_IN, GPIO_RISING, irq_handler, ctx);

/* Intentionally check if bus can be acquired, if assertions are on */
if (!IS_ACTIVE(NDEBUG)) {
Expand Down
Loading