drivers: ieee802154: introduce channel pages #62413
Conversation
ghost
self-assigned this
ghost
marked this pull request as ready for review
ghost
requested review from
MaureenHelm,
andy-liu-telink,
carlescufi,
cfriedt,
galak,
jciupis,
jukkar,
nandojve,
nashif,
rlubos,
tbursztyka and
vaishnavachath
as code owners
zephyrbot
added
platform: TI SimpleLink
cfriedt
left a comment
cfriedt
left a comment
There was a problem hiding this comment.
Looks overall agreeable.
Great work with the cc13xx and enumerating pages.
Maybe not needed in this rev, and perhaps it's already available, but it would be good to have DT attributes for the radio device to allow board-Level overrides (there are almost always board level overrides).
I'll save my approval for after others have had a round.
There was a problem hiding this comment.
I like it, except that I think it should go into device tree, mainly because there will always be some configuration at the board level.
It definitely makes sense to have defaults too though.
I'm might not there yet, but maybe there are already DT bindings included...
There was a problem hiding this comment.
@cfriedt True - at least for SubG/UWB. The 2.4G O-QPSK drivers (= the Thread PHY) are probably all limited to that one PHY, configuration of channel ranges for scanning/hopping is already in the making. Currently the TI cc13xx driver also only supports one option - but that will need to change in the future and then we somehow need to configure this, agreed. A few open questions why I couldn't do this yet:
- I don't know yet what exactly is required - we'll probably get a mixture of custom and standard PHY settings.
- Should the standard settings not be configured independently of the driver (aka generic PHY base configs per instance similarly to other driver APIs base dtsi yaml binding files, see config dimensions below)?
- Also not sure whether Kconfig or DT is right, currently the TI drivers don't support multiple instances and have all their config in Kconfig. Maybe it's better to keep config consistently in Kconfig for now and move it to DT when actually supporting multiple instances?
What follows is mostly "note to self" - no need to read the details. I just like the opportunity to jot down my current thoughts re PHY configuration in the whole IEEE 802.15.4 subsystem in public so people can comment and I can refer to it in the future and let my bad memory forget it for now.
Right now we probably have less need for non-standard/vendor specific extensions in existing drivers than it seems. Most existing configuration options are standard options in the wrong place. This PR is proof of it but there's a lot more. Another example is just what you pointed to @cfriedt, CC12xx and CC13xx support the same SUN FSK standard PHYs, so it doesn't make a lot of sense to keep these in the driver. Probably some common IEEE 802.15.4 driver yaml makes sense there. How can we support this w/o excessive complexity in the configuration?
The two SubG TI drivers also support lots of non-standard PHYs which need to be configurable. Therefore vendor/protocol specific options on all levels also need to be catered for (see 2, 3, 5, 6 below) even if it's the smaller problem, as those can easily be encapsulated and we already have a good framework for it.
The PHY specification is more than half of the standard and is still the fastest growing part. First we need to thoroughly understand the structure of the IEEE 802.15.4 PHY configuration space and then we have to systematically map it onto existing Zephyr architecture elements. Once we have the full picture it will be easy to find the "right" place for each and every requirement that might come up in the future - it may be DT it also may be not. Luckily the standardization committee has done most of the hard work, we just need to apply their concepts.
Here comes my current mind model of the required multi-dimensional IEEE 802.15.4 PHY configuration space which embeds DT but goes beyond. It is spanned by several orthogonal dimensions. Please let me know what I'm missing:
-
Granularity
- Global defaults: 100% standard based (this is mostly what this PR starts off)
- Global (const): Kconfig
- Per driver (const): Kconfig
- Per driver instance/interface (const): DT
- Per driver instance/interface (dynamic): net_mgmt (app) & driver_api->configure() (L2)
-
Horizontal Layers
- PHY aspects of driver hardware
- PHY aspects of driver firmware (may be multiple per hardware)
- L2 <-> PHY configuration interfaces
- upper layers <-> PHY configuration interfaces (to be avoided - but bad design of upper layers may force us)
-
Vertical Stacks
There are very few differences between native L2 and Thread relating to PHY configuration. Thread only fills a few gaps in the standard to improve cross-vendor compatibility. Standard protocols (vanilla WPANs, DTME, TSCH, CSL, RIT, ranging, SRM, etc.) are decoupled from PHY by design, so probably no need for extra PHY config there. We need to cater for a few OT implementation quirks unrelated to the Thread standard and we already have an architecture to do so. The same mechanism will work for PHY extensions/refinements of protocols we may want to support in the future (ISA 100.11a, ...).
-
PHY
This PR only gives a glimpse of what can be configured on a per PHY level. Some of the PHYs are mostly standardized (e.g. O-QPSK, BPSK), others are highly configurable (SUN FSK, LECIM, TVWS, to a lesser extent UWB). Providing a good configuration model requires intimate knowledge of the PHY specific physics, modulation and coding approach which is challenging. The best we can do for now is ensuring that the configuration architecture includes well encapsulated driver-agnostic "configuration namespaces/containers" for each PHY. Luckily the standard has done the hard work of extracting what is common across PHYs and pushing down and prestructuring the PHY-specific data model. We just need to read and follow the standard here - this PR is an example of that approach.
-
Hardware
Recent hardware is increasingly standard conforming. But there are some quirks. Vendors try to differentiate via non-standard options or try to create their own de-facto standard. TI's CCxxxx (from 11xx up to the latest 26x7) is a good example as it is highly configurable on PHY level which makes it very flexible for custom applications. All datasheets I've opened document at least a few non-standard options. The configuration can easily be isolated in this case in driver specific defaults, Kconfig, DT and our new API for well-encapsulated custom extensions to runtime configuration. So this is not a problem in practice. For non-standard options I propose we be tolerant and live the diversity to resist premature abstractions. If patterns emerge in practice we can still iterate upon them to make them driver agnostic.
The opposite is true for standard options: We need to be much more careful in the future to extract configuration defined by the standard that is per definition NOT hardware specific such that drivers don't re-invent the wheel all the time (as can be seen in current drivers). We can repeat the same O-QSPK symbol rate in 10 defines in the code - I don't mind. But exposing users to 10 configuration options - one per driver - that essentially all manipulate the same standard PHY PIB attribute is really bad.
-
Vendor Firmware
Vendor specific driver firmware can have tons of configuration options - some standard some not (nRF5 is a good example). Our challenge here will again be to extract what is part of the standard so that users wanting to switch between drivers or using multiple of them don't have to learn each and every driver firmware configuration model. Of course there are limits to what makes sense and we should resist the temptation for premature abstraction. Again my approach would be to live diversity and wait for patterns to emerge when it comes to non-standard configuration options. But we should be very strict in the future to extract standard options whenever possible rather than letting driver maintainers simply wrap their vendor-specific and mostly non-reusable custom API for those.
Of course none of this should be introduced just for the sake of it but strictly on an as-needed basis. I'm spanning up this framework to avoid the mistake of the past that the first driver requiring some configuration inside this space establishes it in some driver specific Kconfig variable which we then have to support forever although it was probably some standardized PHY configuration that would have much better been supported via driver-agnostic DT or runtime configuration.
My intention is all about the user facing API becoming simpler, more vendor agnostic, future proof and stable. That's also why I'm being a little defensive, conservative and slow - we've been shooting a little to quickly in the past which now causes us some extra work and headache (see this PR) - but that's also normal and not a big problem IMO.
jciupis
left a comment
jciupis
left a comment
There was a problem hiding this comment.
The PR looks good overall, I only spotted some minor typos and mistakes in the doc. I'm not approving yet to verify on hardware first, as requested.
ghost
mentioned this pull request
jfischer-no
left a comment
jfischer-no
left a comment
There was a problem hiding this comment.
This patch has some stray and not directly related to the commit message changes, and needs some additional work.
Preparative change to introduce build-time configured channel pages. This fixes the description of the driver's available PHYs and makes channel page and channel range independent from runtime attributes. Signed-off-by: Florian Grandel <[email protected]>
|
@nandojve @rlubos Would you mind re-approving? I had to push again due to some valid feedback from @jfischer-no 's second review. Nothing functional at all just some shuffling around of commits and a better commit message. I made sure that you can confirm this by clicking on the compare button above so that you do not unnecessarily waste your time. |
@cfriedt @fgrandel , Thanks the changes looks good, yes this helps the wpanusb/serial support in a good way. we had some hack/Kconfigs to do few of these things : https://git.beagleboard.org/beagleconnect/zephyr/wpanusb_bc/-/blob/master/prj.conf?ref_type=heads#L32 and this set of changes helps to make it much cleaner. |
|
Hi @fgrandel , Organizational topic idea: Is possible to create a GH project or an issue that allows us to track current/future enhancements? My intention is not to read the content/feature itself but instead have the bullets that we know what will be in one or another release, for instance. The IEEE 802.15.4 seems to growing these days and it will be nice understand what Feature/PR goes on one release to ensure proper HW tests. This could help us to look and understand what it is implemented and not too. In this #38657 there is a list of features that it is easy to track. Maybe we need both issue to track IEEE 802.15.4 features and a project to understand what goes in the releases. Feel free to implement if make sense : ) |
@nandojve If I understand you correctly, you're proposing that we maintain a proper subsystem roadmap, right? This is a great idea - thanks for that proposal. And I love it even more that we conceive this roadmap as an open community effort that we jointly maintain as you propose. I'm currently preparing a presentation for the Linux Plumbers Conference in November where I collect all changes that happened over the last months (by topic area) plus I'm presenting the opportunities that I see in the subsystem for the upcoming releases that have been unlocked by these past changes. Be aware that this is early work in progress but feel free to comment.
Some things already exist:
Do you personally plan any project for the future that I should add to that roadmap? If that's the case I'll do so immediately. Some more background for those who stumble upon this comment: @nandojve and I identified the following potential opportunities for driver maintainers (this list is tailored to the rf2xx driver and of course far from complete, just to give you the rough idea):
Apart form that ranging (precise indoor localization/navigation) is a big opportunity in the subsystem - I'm inviting everyone in the community to look at FiRa/802.15.4z and extend the existing DW1000 driver in that direction or introduce new drivers (e.g. NXP) that support ranging. |
@jfischer-no As we seem to agree by now, your feedback should have been non-blocking, nevertheless for most instances I've accepted it or - if not - given an argued explanation why not. I therefore kindly ask you to unblock the PR ASAP. I've taken note of the relevance of the Linux Coding Guidelines in Zephyr and I do of course share your opinion that commits should be well documented, as small as possible and be self-contained so I hope these topics have now found due and pragmatic representation in this PR. |
Aligns the name of the return value variable with what is used elsewhere in the driver and the subsystem for improved readability and consistency. Signed-off-by: Florian Grandel <[email protected]>
Replaces the previous approach to define bands via hardware capabilities by the standard conforming concept of channel pages. In the short term this allows us to correctly calculate the PHY specific symbol rate and several parameters that directly depend from the symbol rate and were previously not being correctly calculated for some of the drivers whose channel pages could not be represented previously: * We now support sub-nanosecond precision symbol rates for UWB. Rounding errors are being minimized by switching from a divide-then-multiply approach to a multiply-then-divide approach. * UWB HRP: symbol rate depends on channel page specific preamble symbol rate which again requires the pulse repetition value to be known * Several MAC timings are being corrected based on the now correctly calculated symbol rates, namely aTurnaroundTime, aUnitBackoffPeriod, aBaseSuperframeDuration. In the long term, this change unlocks such highly promising functional areas as UWB ranging and SUN-PHY channel hopping in the SubG area (plus of course any other PHY specific feature). Signed-off-by: Florian Grandel <[email protected]>
@nandojve, @fgrandel - you might want to simply use the release plan for feature tracking. That would make the most sense and at least would keep release managers in the loop implicitly. |
|
@jfischer-no please revisit |
| * A new mandatory method attr_get() was introduced into ieee802154_radio_api. | ||
| Drivers need to implement at least | ||
| IEEE802154_ATTR_PHY_SUPPORTED_CHANNEL_PAGES and | ||
| IEEE802154_ATTR_PHY_SUPPORTED_CHANNEL_RANGES. | ||
| * The hardware capabilities IEEE802154_HW_2_4_GHZ and IEEE802154_HW_SUB_GHZ | ||
| were removed as they were not aligned with the standard and some already | ||
| existing drivers couldn't properly express their channel page and channel | ||
| range (notably SUN FSK and HRP UWB drivers). The capabilities were replaced | ||
| by the standard conforming new driver attribute | ||
| IEEE802154_ATTR_PHY_SUPPORTED_CHANNEL_PAGES that fits all in-tree drivers. | ||
| * The method get_subg_channel_count() was removed from ieee802154_radio_api. | ||
| This method could not properly express the channel range of existing drivers | ||
| (notably SUN FSK drivers that implement channel pages > 0 and may not have | ||
| zero-based channel ranges or UWB drivers that could not be represented at | ||
| all). The method was replaced by the new driver attribute | ||
| IEEE802154_ATTR_PHY_SUPPORTED_CHANNEL_RANGES that fits all in-tree drivers. | ||
|
|
There was a problem hiding this comment.
It looks to me like a stable API Change (IEEE 802.15.4 driver interface API), should not part of it be added to “API Changes” section of the release notes? To be fear there is not “API Changes” section in doc/releases/release-notes-3.5.rst.
I also could not find an RFC mail sent to the devel list about this, is there a new undocumented process or is it not needed here?
There was a problem hiding this comment.
That's a good point. However, I can't find the ieee802154 marked as stable here:
https://docs.zephyrproject.org/latest/develop/api/overview.html
should not part of it be added to “API Changes” section of the release notes?
Stable API changes now belong in the migration guide, not in the release notes (hence why no section).
@fgrandel should we consider this API as unstable for now?
There was a problem hiding this comment.
@carlescufi Yes, one of the PRs blocked by this PR does just that. :-)
See #62503, more specifically https://github.com/zephyrproject-rtos/zephyr/pull/62503/files#diff-925c8ea16c15775440380c5e134bba4f8ba66aed774012b72b149e1b2210b884
The documentation has been less than satisfactory so far. And that PR fixes a lot of it.
There was a problem hiding this comment.
This is part of networking APIs, stable since 1.0
https://docs.zephyrproject.org/latest/develop/api/overview.html#api-overview
Networking APIs
The steps for stable API changes are described here
https://docs.zephyrproject.org/latest/develop/api/api_lifecycle.html#introducing-incompatible-changes.
Do we follow it or cherry-pick it case-by-case?
There was a problem hiding this comment.
This is part of networking APIs, stable since 1.0
@carlescufi I personally believe that subsuming all of IEEE 802.15.4 L2, driver and net_mgmt under network APIs does not make sense for versioning purposes. These three APIs have very different rythms of change and audiences. I therefore propose we treat those as separate.
L2: internal API needed to interface with the net stack, the IEEE 802.15.4 specific part (ieee802154_context/pkt) very unstable due to active protocol development, audience: subsystem maintainers only
drivers: internal API, currently very unstable, requires a lot of change, audience: driver maintainers only
net_mgmt: API exposed to applications, mostly only additions are required, could be stable but I wouldn't do that, yet, audience: application developers
The only API that really causes headaches to existing applications when we break it is net_mgmt - and that's the one we've kept stable. The driver API will break out-of-tree drivers, but so far I haven't heard of a single existing instance of one from the community. The L2 API is not a problem at all AFAICS as it is used in-tree only.
If we subsume all L2/L1s under network we'd also have no way to differentiate the stability of Ethernet, Bluetooth, CAN and IEEE 802.15.4.
For all practical purposes, @rlubos, @jukkar and I considered the existing user base of those APIs small and the changes required for its transformation many so that we agreed to treat it de facto as unstable over the last three releases (which I'm proposing to formalize now).
This strategy seems to have worked out well. AFAICS we didn't get a single complaint and a lot of positive feedback for that approach from the community.
There was a problem hiding this comment.
This is part of networking APIs, stable since 1.0 https://docs.zephyrproject.org/latest/develop/api/overview.html#api-overview Networking APIs
Unfortunately I don't think it's all that clear, and we don't really have a single source of truth for what's the status of each API within the Networking area.
For example that page you're linking also has CoAP, or MQTT, that are clearly labelled unstable, and many other entries that just don't happen to have ever been properly qualified in the documentation (ex. TFTP seems to be experimental if one looks at the Kconfig, but what should one make of it being listed on that page with no clear warning it's experimental?)
There was a problem hiding this comment.
Hi @carlescufi ,
Just a remind that this IEEE 802.15.4 don't follow 100% the standard and many issues have been addressed recently. I would say the API is far to be stable and there are many changes in the backlog (which is not visible yet). My attempt to give the idea about a central point that have the features already in/planned goes in the direction to create some visibility about work already done and planned. As an example, the #49775 is already an example about things that we have been discussing over time and that already require possible changes/extensions in the API.
There was a problem hiding this comment.
okay, it convinces me that it is okay to cherry-pick Radio API to be unstable.
But then why cannot this change be a commit before breaking changes, for the avoidance of any doubts?
There was a problem hiding this comment.
I agree, the list of different components constituting "Networking API" is just too broad to have it covered by a single entry.
@cfriedt Good idea! Thanks for the pointer. I'll have a look at that project and see how I can interface existing activities with it. |
11 participants
TLDR;
This PR fixes some non-standard conforming aspects of the IEEE 802.15.4 driver API by introducing the standard conforming concept of channel pages (see IEEE 802.15.4-2020, section 10.1.3).
Obs: The DNM label was only attached to ensure that all driver maintainers have a fair chance to review this before it's being merged. No reason not to review this PR.UPDATE: Removed DNM tag as all important reviews have come in. @jciupis As you're on vacation now, @rlubos was so kind to take over. :-)
Driver maintainers: Ideally you try this out on real hardware before ACK. I already did so for the CC13/26xx SubG driver myself.
Detailed Rationale
This is the lowest hanging fruit in the direction of RFC #61227 (vendor/L2 agnostic driver API) which was recently agreed upon in the Arch WG. It is a good example of how the principles introduced there immediately make the driver API more vendor-agnostic.
Introducing channel pages requires non-trivial changes to the driver API and must therefore be well justified:
This change makes use of the recently introduced PHY PIB attributes getter and hopefully demonstrates and proves its usefulness.