Skip to content

drivers: ieee802154: introduce channel pages #62413

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

Merged
cfriedt merged 3 commits into from Sep 27, 2023
Merged

drivers: ieee802154: introduce channel pages #62413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
09f5281
drivers: ieee802154: cc1200: SUN-FSK compatibility
fg-cfh Sep 25, 2023
ca562e9
drivers: ieee802154: cc13xx_cc26xx: readability improvement
fg-cfh Sep 25, 2023
7037664
drivers: ieee802154: introduce channel pages
fg-cfh Sep 2, 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
17 changes: 17 additions & 0 deletions doc/releases/release-notes-3.5.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,23 @@ Drivers and Sensors

* IEEE 802.15.4

* 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.

Comment on lines +180 to +196
Copy link
Contributor

@jfischer-no jfischer-no Sep 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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?

Copy link
Member

@carlescufi carlescufi Sep 27, 2023

Choose a reason for hiding this comment

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

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?

Copy link
Author

@ghost ghost Sep 27, 2023 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

@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.

Copy link
Contributor

@jfischer-no jfischer-no Sep 27, 2023

Choose a reason for hiding this comment

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

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?

Copy link
Author

@ghost ghost Sep 27, 2023 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@kartben kartben Sep 27, 2023

Choose a reason for hiding this comment

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

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

Copy link
Member

@carlescufi carlescufi Sep 27, 2023

Choose a reason for hiding this comment

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

I agree with the statements from @fgrandel and @kartben, but I will ask @rlubos and @jukkar to weigh in.

Copy link
Member

@nandojve nandojve Sep 27, 2023

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@jfischer-no jfischer-no Sep 27, 2023

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

@rlubos rlubos Sep 28, 2023

Choose a reason for hiding this comment

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

I agree, the list of different components constituting "Networking API" is just too broad to have it covered by a single entry.

* Interrupt Controller

* GIC: Architecture version selection is now based on the device tree
Expand Down
16 changes: 12 additions & 4 deletions drivers/ieee802154/Kconfig.cc1200
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ menuconfig IEEE802154_CC1200
bool "TI CC1200 Driver support"
default y
depends on DT_HAS_TI_CC1200_ENABLED
select NET_L2_IEEE802154_SUB_GHZ

if IEEE802154_CC1200

Expand Down Expand Up @@ -85,13 +84,22 @@ choice
Set the RF preset you want to use.

config IEEE802154_CC1200_RF_SET_0
bool "868MHz - 50Kbps - 2-GFSK - IEEE 802.15.4g compliant - ETSI"
bool "IEEE 802.15.4g SUN MR-FSK, 863MHz band, mode #1 - channel page 9, 34 channels, 50Kbps (ETSI)"
help
This is a legacy IEEE 802.15.4g-2012 SUN MR-FSK PHY that does no
longer exist in recent standards (IEEE 802.15.4-2015+).

config IEEE802154_CC1200_RF_SET_1
bool "920MHz - 50Kbps - 2-GFSK - IEEE 802.15.4g compliant - ARIB"
bool "IEEE 802.15.4g SUN MR-FSK 920MHz band, mode #1 - channel page 9, 39 channels, 50Kbps (ARIB)"
help
This is a legacy IEEE 802.15.4g-2012 SUN MR-FSK PHY that does no
longer exist in recent standards (IEEE 802.15.4-2015+).

config IEEE802154_CC1200_RF_SET_2
bool "434MHz - 50Kbps - 2-GFSK - IEEE 802.15.4g compliant - ETSI"
bool "IEEE 802.15.4 Non-Standard 2-GFSK 433MHz band - channel page 9, 15 channels, 50Kbps (ETSI)"
help
This is a non-standard PHY similar to the IEEE 802.15.4g-2012 SUN
MR-FSK PHY but not in one of the standard bands.

endchoice

Expand Down
1 change: 0 additions & 1 deletion drivers/ieee802154/Kconfig.cc13xx_cc26xx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ menuconfig IEEE802154_CC13XX_CC26XX_SUB_GHZ
bool "TI CC13xx / CC26xx IEEE 802.15.4g driver support"
default y
depends on DT_HAS_TI_CC13XX_CC26XX_IEEE802154_SUBGHZ_ENABLED
select NET_L2_IEEE802154_SUB_GHZ if NET_L2_IEEE802154

if IEEE802154_CC13XX_CC26XX_SUB_GHZ

Expand Down
1 change: 0 additions & 1 deletion drivers/ieee802154/Kconfig.rf2xx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ menuconfig IEEE802154_RF2XX
depends on DT_HAS_ATMEL_RF2XX_ENABLED
select SPI
select GPIO
select NET_L2_IEEE802154_SUB_GHZ if NET_L2_IEEE802154

if IEEE802154_RF2XX

Expand Down
25 changes: 22 additions & 3 deletions drivers/ieee802154/ieee802154_b91.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ LOG_MODULE_REGISTER(LOG_MODULE_NAME);


/* B91 data structure */
static struct b91_data data;
static struct b91_data data;

/* Set filter PAN ID */
static int b91_set_pan_id(uint16_t pan_id)
Expand Down Expand Up @@ -403,7 +403,7 @@ static enum ieee802154_hw_caps b91_get_capabilities(const struct device *dev)
{
ARG_UNUSED(dev);

return IEEE802154_HW_FCS | IEEE802154_HW_2_4_GHZ | IEEE802154_HW_FILTER |
return IEEE802154_HW_FCS | IEEE802154_HW_FILTER |
IEEE802154_HW_TX_RX_ACK | IEEE802154_HW_RX_TX_ACK;
}

Expand All @@ -428,10 +428,14 @@ static int b91_set_channel(const struct device *dev, uint16_t channel)
{
ARG_UNUSED(dev);

if (channel < 11 || channel > 26) {
if (channel > 26) {
return -EINVAL;
}

if (channel < 11) {
return -ENOTSUP;
}

if (data.current_channel != channel) {
data.current_channel = channel;
rf_set_chn(B91_LOGIC_CHANNEL_TO_PHYSICAL(channel));
Expand Down Expand Up @@ -585,6 +589,20 @@ static int b91_configure(const struct device *dev,
return -ENOTSUP;
}

/* driver-allocated attribute memory - constant across all driver instances */
IEEE802154_DEFINE_PHY_SUPPORTED_CHANNELS(drv_attr, 11, 26);

/* API implementation: attr_get */
static int b91_attr_get(const struct device *dev, enum ieee802154_attr attr,
struct ieee802154_attr_value *value)
{
ARG_UNUSED(dev);

return ieee802154_attr_get_channel_page_and_range(
attr, IEEE802154_ATTR_PHY_CHANNEL_PAGE_ZERO_OQPSK_2450_BPSK_868_915,
&drv_attr.phy_supported_channels, value);
}

/* IEEE802154 driver APIs structure */
static struct ieee802154_radio_api b91_radio_api = {
.iface_api.init = b91_iface_init,
Expand All @@ -598,6 +616,7 @@ static struct ieee802154_radio_api b91_radio_api = {
.tx = b91_tx,
.ed_scan = b91_ed_scan,
.configure = b91_configure,
.attr_get = b91_attr_get,
};


Expand Down
43 changes: 31 additions & 12 deletions drivers/ieee802154/ieee802154_cc1200.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ static void cc1200_rx(void *arg)
*******************/
static enum ieee802154_hw_caps cc1200_get_capabilities(const struct device *dev)
{
return IEEE802154_HW_FCS | IEEE802154_HW_SUB_GHZ;
return IEEE802154_HW_FCS;
}

static int cc1200_cca(const struct device *dev)
Expand All @@ -543,6 +543,15 @@ static int cc1200_cca(const struct device *dev)
static int cc1200_set_channel(const struct device *dev, uint16_t channel)
{
struct cc1200_context *cc1200 = dev->data;
uint32_t freq;

/* As SUN FSK provides a host of configurations with extremely different
* channel counts it doesn't make sense to validate (aka -EINVAL) a
* global upper limit on the number of supported channels on this page.
*/
if (channel > IEEE802154_CC1200_CHANNEL_LIMIT) {
return -ENOTSUP;
}

/* Unlike usual 15.4 chips, cc1200 is closer to a bare metal radio modem
* and thus does not provide any means to select a channel directly, but
Expand All @@ -552,14 +561,16 @@ static int cc1200_set_channel(const struct device *dev, uint16_t channel)
* See rf_evaluate_freq_setting() above.
*/

if (atomic_get(&cc1200->rx) == 0) {
uint32_t freq = rf_evaluate_freq_setting(dev, channel);
if (atomic_get(&cc1200->rx) != 0) {
return -EIO;
}

if (!write_reg_freq(dev, freq) ||
rf_calibrate(dev)) {
LOG_ERR("Could not set channel %u", channel);
return -EIO;
}
freq = rf_evaluate_freq_setting(dev, channel);

if (!write_reg_freq(dev, freq) ||
rf_calibrate(dev)) {
LOG_ERR("Could not set channel %u", channel);
return -EIO;
}

return 0;
Expand Down Expand Up @@ -692,11 +703,19 @@ static int cc1200_stop(const struct device *dev)
return 0;
}

static uint16_t cc1200_get_channel_count(const struct device *dev)
/* driver-allocated attribute memory - constant across all driver instances as
* this driver's channel range is configured via a global KConfig setting.
*/
IEEE802154_DEFINE_PHY_SUPPORTED_CHANNELS(drv_attr, 0, IEEE802154_CC1200_CHANNEL_LIMIT);

static int cc1200_attr_get(const struct device *dev, enum ieee802154_attr attr,
struct ieee802154_attr_value *value)
{
struct cc1200_context *cc1200 = dev->data;
ARG_UNUSED(dev);

return cc1200->rf_settings->channel_limit;
return ieee802154_attr_get_channel_page_and_range(
attr, IEEE802154_ATTR_PHY_CHANNEL_PAGE_NINE_SUN_PREDEFINED,
&drv_attr.phy_supported_channels, value);
}

/******************
Expand Down Expand Up @@ -801,7 +820,7 @@ static struct ieee802154_radio_api cc1200_radio_api = {
.tx = cc1200_tx,
.start = cc1200_start,
.stop = cc1200_stop,
.get_subg_channel_count = cc1200_get_channel_count,
.attr_get = cc1200_attr_get,
};

NET_DEVICE_DT_INST_DEFINE(0, cc1200_init, NULL, &cc1200_context_data,
Expand Down
9 changes: 6 additions & 3 deletions drivers/ieee802154/ieee802154_cc1200_rf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@

#if defined(CONFIG_IEEE802154_CC1200_RF_SET_0)

#define IEEE802154_CC1200_CHANNEL_LIMIT 33

const struct cc1200_rf_registers_set cc1200_rf_settings = {
.chan_center_freq0 = 863125,
.channel_limit = 33,
.channel_spacing = 2000, /* 200 KHz */
.registers = {
0x6F, /* SYNC3 */
Expand Down Expand Up @@ -133,9 +134,10 @@ const struct cc1200_rf_registers_set cc1200_rf_settings = {

#elif defined(CONFIG_IEEE802154_CC1200_RF_SET_1)

#define IEEE802154_CC1200_CHANNEL_LIMIT 38

const struct cc1200_rf_registers_set cc1200_rf_settings = {
.chan_center_freq0 = 920600,
.channel_limit = 38,
.channel_spacing = 2000, /* 200 KHz */
.registers = {
0x6F, /* SYNC3 */
Expand Down Expand Up @@ -243,9 +245,10 @@ const struct cc1200_rf_registers_set cc1200_rf_settings = {

#elif defined(CONFIG_IEEE802154_CC1200_RF_SET_2)

#define IEEE802154_CC1200_CHANNEL_LIMIT 14

const struct cc1200_rf_registers_set cc1200_rf_settings = {
.chan_center_freq0 = 433164,
.channel_limit = 14,
.channel_spacing = 2000, /* 200 KHz */
.registers = {
0x6F, /* SYNC3 */
Expand Down
51 changes: 29 additions & 22 deletions drivers/ieee802154/ieee802154_cc13xx_cc26xx.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ static void client_event_callback(RF_Handle h, RF_ClientEvent event, void *arg)
static enum ieee802154_hw_caps
ieee802154_cc13xx_cc26xx_get_capabilities(const struct device *dev)
{
return IEEE802154_HW_FCS | IEEE802154_HW_2_4_GHZ | IEEE802154_HW_FILTER |
return IEEE802154_HW_FCS | IEEE802154_HW_FILTER |
IEEE802154_HW_RX_TX_ACK | IEEE802154_HW_TX_RX_ACK | IEEE802154_HW_CSMA |
IEEE802154_HW_RETRANSMISSION;
}
Expand Down Expand Up @@ -155,42 +155,36 @@ static inline int ieee802154_cc13xx_cc26xx_channel_to_frequency(
__ASSERT_NO_MSG(frequency != NULL);
__ASSERT_NO_MSG(fractFreq != NULL);

if (channel >= IEEE802154_2_4_GHZ_CHANNEL_MIN
&& channel <= IEEE802154_2_4_GHZ_CHANNEL_MAX) {
*frequency = 2405 + 5 * (channel - IEEE802154_2_4_GHZ_CHANNEL_MIN);
/* See IEEE 802.15.4-2020, section 10.1.3.3. */
if (channel >= 11 && channel <= 26) {
*frequency = 2405 + 5 * (channel - 11);
*fractFreq = 0;
return 0;
} else {
/* TODO: Support sub-GHz for CC13xx rather than having separate drivers */
*frequency = 0;
*fractFreq = 0;
return -EINVAL;
return channel < 11 ? -ENOTSUP : -EINVAL;
}

return 0;
}

static int ieee802154_cc13xx_cc26xx_set_channel(const struct device *dev,
uint16_t channel)
{
int r;
int ret;
RF_CmdHandle cmd_handle;
RF_EventMask reason;
uint16_t freq, fract;
struct ieee802154_cc13xx_cc26xx_data *drv_data = dev->data;

/* TODO Support sub-GHz for CC13xx */
if (channel < 11 || channel > 26) {
return -EINVAL;
}

r = ieee802154_cc13xx_cc26xx_channel_to_frequency(
channel, &freq, &fract);
if (r < 0) {
return -EINVAL;
ret = ieee802154_cc13xx_cc26xx_channel_to_frequency(channel, &freq, &fract);
if (ret < 0) {
return ret;
}

/* Abort FG and BG processes */
if (ieee802154_cc13xx_cc26xx_stop(dev) < 0) {
r = -EIO;
ret = -EIO;
goto out;
}

Expand All @@ -205,7 +199,7 @@ static int ieee802154_cc13xx_cc26xx_set_channel(const struct device *dev,
RF_PriorityNormal, NULL, 0);
if (reason != RF_EventLastCmdDone) {
LOG_ERR("Failed to set frequency: 0x%" PRIx64, reason);
r = -EIO;
ret = -EIO;
goto out;
}

Expand All @@ -217,15 +211,15 @@ static int ieee802154_cc13xx_cc26xx_set_channel(const struct device *dev,
cmd_ieee_rx_callback, RF_EventRxEntryDone);
if (cmd_handle < 0) {
LOG_ERR("Failed to post RX command (%d)", cmd_handle);
r = -EIO;
ret = -EIO;
goto out;
}

r = 0;
ret = 0;

out:
k_mutex_unlock(&drv_data->tx_mutex);
return r;
return ret;
}

/* TODO remove when rf driver bugfix is pulled in */
Expand Down Expand Up @@ -514,6 +508,18 @@ ieee802154_cc13xx_cc26xx_configure(const struct device *dev,
return -ENOTSUP;
}

/* driver-allocated attribute memory - constant across all driver instances */
IEEE802154_DEFINE_PHY_SUPPORTED_CHANNELS(drv_attr, 11, 26);

static int ieee802154_cc13xx_cc26xx_attr_get(const struct device *dev, enum ieee802154_attr attr,
struct ieee802154_attr_value *value)
{
ARG_UNUSED(dev);

return ieee802154_attr_get_channel_page_and_range(
attr, IEEE802154_ATTR_PHY_CHANNEL_PAGE_ZERO_OQPSK_2450_BPSK_868_915,
&drv_attr.phy_supported_channels, value);
}

static void ieee802154_cc13xx_cc26xx_data_init(const struct device *dev)
{
Expand Down Expand Up @@ -576,6 +582,7 @@ static struct ieee802154_radio_api ieee802154_cc13xx_cc26xx_radio_api = {
.start = ieee802154_cc13xx_cc26xx_start,
.stop = ieee802154_cc13xx_cc26xx_stop_if,
.configure = ieee802154_cc13xx_cc26xx_configure,
.attr_get = ieee802154_cc13xx_cc26xx_attr_get,
};

/** RF patches to use (note: RF core keeps a pointer to this, so no stack). */
Expand Down
2 changes: 1 addition & 1 deletion drivers/ieee802154/ieee802154_cc13xx_cc26xx.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

/* For O-QPSK the physical and MAC timing symbol rates are the same, see section 12.3.3. */
#define IEEE802154_2450MHZ_OQPSK_SYMBOLS_PER_SECOND \
IEEE802154_PHY_SYMBOLS_PER_SECOND(IEEE802154_PHY_OQPSK_2450MHZ_SYMBOL_PERIOD_US)
IEEE802154_PHY_SYMBOLS_PER_SECOND(IEEE802154_PHY_OQPSK_780_TO_2450MHZ_SYMBOL_PERIOD_NS)

/* PHY PIB attribute phyCcaMode - CCA Mode 3: Carrier sense with energy above threshold, see
* section 11.3, table 11-2 and section 10.2.8
Expand Down
Loading