[riot-notifications] [RIOT-OS/RIOT] [RFC] at86rf2xx: implement radio HAL (#16535)

José Alamos notifications at github.com
Mon Jun 7 16:29:53 CEST 2021

The RIOT community cares a lot about code quality.
Therefore, before describing what your contribution is about, we would like
you to make sure that your modifications are compliant with the RIOT
coding conventions, see https://github.com/RIOT-OS/RIOT/wiki/Coding-conventions.

### Contribution description
This (draft) PR implements the Radio HAL interface for `at86rf2xx`. This is the first SPI radio that runs the Radio HAL, so I had to adapt some stuff that I would like to discuss with the community (see Discussion).

This port is resilient against https://github.com/RIOT-OS/RIOT/pull/11256. After many debugging hours, forcing the radio to PLL_ON before a transition from RX_ON to TX_ON seems to be the only way to prevent the bug described there, since there's a small gap which makes it impossible to detect if there are incoming frames even when:
a) The transceiver state is checked before `request_set_trx_state`
b) The function checks if there are pending interrupts for the radio
c) The SHR detection is disabled right before checking a) and b)

The port has been tested against other radio in stressful conditions and it seems to work. There are some pending stuff before it's mergeable: 

- [ ] Basic Mode support
- [ ] Periph implementation
- [ ] Some cleanup
- [ ] Adapt to gnrc_netif's thread_flag mechanism

And of course, it would nice to resolve the discussion presented above.

Put here the description of your contribution:
- describe which part(s) of RIOT is (are) involved
- if it's a bug fix, describe the bug that it solves and how it is solved
- you can also give more information to reviewers about how to test your changes

### Discussion

Contrary to netdev, it was decided that the ISR offloading (a.k.a Bottom Half processing) would be independent of the upper layer, since not all radios require ISR processing or some might require a slightly different mechanism (e.g devices with 2 radios).

For radios that require ISR offloading, we need a Bottom Half Processing interface than can indicate there are pending IRQs and be able to process the events. I see 2 patterns that we might follow:

1. Implement a simple indication/response pattern that indicates the BH to call a driver specific "task handler" when an interrupt is triggered. This patterns was chosen for this draft and for this case 2 functions are defined:
    - `at86rf2xx_irq_handler`: To be implemented by the network stack. This function is called from ISR context and indicates the BH to process the radio events. The argument of this function (e.g pointer to the HAL) is assigned during `at86rf2xx_init`.
    - `at86rf2xx_task_handler`: This function process the pending IRQs and calls the Radio HAL CB functions.
2. Implement several mechanisms to process ISR functions with different mechanisms (event_thread, thread flags, msg_t, etc). This would require to implement functions like `at86rf2xx_init_evq` or similar to setup the BH.

The first approach has the advantage that it can be easily adapted to any network stack specific mechanism, with the cost of a BH implementation for every driver - network stack combination.
The second approach simplifies simplifies bootstrap code of the drivers. We could eventually decouple the initialization of the drivers from the initialization of the network stacks. The price to pay in this case is the fact he implementation of the drivers would be slightly more complex, since each driver has to implement variants of every mechanism. Also, the drivers would be hardcoded to a specific set of BH mechanisms

Opinions on this?
If there's a better way to process the events, please report it :)

### Testing procedure
Try `tests/ieee802154_hal` and `examples/gnrc_networking`.

I haven't configured the dependencies properly. Therefore it's required to manually include `gnrc_netif_events` and `netdev_ieee802154_submac` for `examples/gnrc_networking`
Details steps to test your contribution:
- which test/example to compile for which board and is there a 'test' command
- how to know that it was not working/available in master
- the expected success test output

### Issues/PRs references
Depends on #16533 and #16534 
Examples: Fixes #1234. See also #5678. Depends on PR #9876.

Please use keywords (e.g., fixes, resolve) with the links to the issues you
resolved, this way they will be automatically closed when your pull request
is merged. See https://help.github.com/articles/closing-issues-using-keywords/.

You can view, comment on, or merge this pull request online at:


-- Commit Summary --

  * ieee802154/submac: fix default SubGHz page
  * ieee802154/submac: call confirm_set_trx_state after initialization
  * netdev_ieee802154_submac: fix initialization code
  * ieee802154/radio_hal: detach hal descriptor from driver
  * at86rf2xx: implement radio hal interface
  * tests/ieee802154_hal: add at86rf2xx startup code
  * gnrc_netif/init_devs: add at86rf2xx startup code
  * tests/ieee802154_submac: whitelist at86rf2xx based boards

-- File Changes --

    M drivers/at86rf2xx/at86rf2xx.c (12)
    M drivers/at86rf2xx/at86rf2xx_getset.c (79)
    M drivers/at86rf2xx/at86rf2xx_internal.c (2)
    M drivers/at86rf2xx/at86rf2xx_netdev.c (2)
    A drivers/at86rf2xx/at86rf2xx_radio_ops.c (633)
    M drivers/include/at86rf2xx.h (21)
    M drivers/include/net/netdev/ieee802154_submac.h (5)
    M drivers/netdev_ieee802154_submac/netdev_ieee802154_submac.c (44)
    M sys/include/net/ieee802154/radio.h (3)
    M sys/include/net/ieee802154/submac.h (12)
    M sys/net/gnrc/netif/init_devs/auto_init_at86rf2xx.c (45)
    M sys/net/link_layer/ieee802154/submac.c (38)
    M tests/ieee802154_hal/Makefile (1)
    M tests/ieee802154_hal/init_devs.c (47)
    M tests/ieee802154_hal/main.c (26)
    M tests/ieee802154_submac/Makefile (1)

-- Patch Links --


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210607/b9d0192f/attachment.htm>

More information about the notifications mailing list