openthread: Move OpenThread implementation from net to modules #1

ArekBalysNordic · 2025-04-23T14:13:53Z

Move OpenThread-related code from
zephyr/subsys/net/l2/openthread/openthread.c to
zephyr/modules/openthread/openthread_module.c.

The primary goal of this refactor is to enable the use of OpenThread as an independent module, without the necessity of Zephyr's networking layer.

This change is particularly beneficial for simple applications that have their own implementation of the IEEE802.15.4 driver and do not require a networking layer. These applications can now disable Zephyr's L2 and IEEE802.15.4 shim layers and directly use the OpenThread module, saving valuable kilobytes of memory.

In this approach if the CONFIG_NET_L2_OPENTHREAD
Kconfig option is set, Zephyr's L2 and IEEE802.15.4 layer will be used, and everything will function as before. The main difference is the Zephyr's L2 layer now uses the OpenThread module, no longer implementing it.

If the CONFIG_NET_L2_OPENTHREAD Kconfig option is
not set, the opentherad_init function will be called at system boot in the POST_KERNEL phase.

While most of the functions in include/net/openthread.h have been deprecated, they are still available for use to maintain backwards compatibility.
These functions now act as an interface to the new ones defined in modules/openthread.

…P is re-enabled Add a simple non-XIP transaction before deactivating the QSPI after a XIP transaction is performed. This prevents a CPU hang from occuring when another XIP transaction is attempted after the QSPI is activated again. Upstream PR #: 88967 Signed-off-by: Andrzej Głąbek <[email protected]>

The PSA is a cryptographically secure random number generator. It will be enabled by default, eventually, For now, enable it manually. Signed-off-by: Gordon Klaus <[email protected]>

…and nRF54L20 nRF54L09 and nRF54L20 need double the time for startup of their clocks. Upstream PR #: 88956 Signed-off-by: Michał Stasiak <[email protected]>

rlubos

Commented on some smaller stuff, but otherwise the separation seems pretty good. I wonder if we could have a overlay file to enable this standalone configuration in the upstream openthread shell sample (https://github.com/zephyrproject-rtos/zephyr/tree/main/samples/net/openthread/shell) to ensure it's functional and does not bitrot over time.

Deprecations look fine, although a good migration guide note would be needed.

rlubos · 2025-04-24T09:04:36Z

modules/openthread/platform/openthread.h

+ * @param aMessage The message to receive.
+ * @param aContext The context to pass to the callback.
+ */
+typedef void (*receiveCallback)(otMessage *aMessage, void *aContext);


nit: Function name and params should be in snake case given it's Zephyr's code. And perhaps it'd make sense to prefix the type with openthread, something like openthread_receive_callback.

rlubos · 2025-04-24T09:06:16Z

modules/openthread/platform/openthread.h

+/**
+ * @brief Get OpenThread thread identification.
+ */
+k_tid_t openthread_platform_thread_id_get(void);


Need to include <zephyr/kernel/thread.h> to get access to the k_tid_t type

rlubos · 2025-04-24T09:10:49Z

modules/openthread/platform/openthread.h

+ * @retval !NULL On success.
+ * @retval NULL  On failure.
+ */
+struct otInstance *openthread_platform_get_default_context(void);


This declaration seems redundant?

rlubos · 2025-04-24T09:11:54Z

modules/openthread/platform/openthread.h

+ * @retval !NULL On success.
+ * @retval NULL  On failure.
+ */
+struct otInstance *openthread_platform_get_default_instance(void);


I just wonder if we need the default in the name, since we only have a single OT instance anyway, I'd just shorten it to openthread_platform_get_instance()

rlubos · 2025-04-24T09:13:18Z

modules/openthread/platform/openthread.c

+	return k_mutex_lock(&openthread_lock, K_NO_WAIT);
+}
+
+void openthread_platform_mutex_unlock()


nit: missing void param here and in other functions.

rlubos · 2025-04-24T09:18:50Z

subsys/net/l2/openthread/openthread.c

+
+int openthread_start(struct openthread_context *ot_context)
+{
+	(void)ot_context;


ARG_UNUSED(ot_context), same elsewhere.

rlubos · 2025-04-24T09:20:37Z

subsys/net/l2/openthread/openthread.c


-	NET_DBG("Injecting %s packet to Zephyr net stack",
-		PKT_IS_IPv4(pkt) ? "translated IPv4" : "Ip6");
+	NET_INFO("Injecting %s packet to Zephyr net stack",


Do we really want to bump the log level here, it'd cause massive log flood in default config I believe?

rlubos · 2025-04-24T09:27:00Z

modules/openthread/platform/openthread.h

+	/** Context of the callback which will be passed to the state_changed_cb */
+	void *context;
+
+	/** User data if required */
+	void *user_data;


Now that I think of it, it seems to me that two void pointers for de facto user specified data seem a bit redundant. Perhaps we could just stick to user_data here, the callback would then interpret it however it wants. If it's a pointer to some specific context (like in case of L2), it could just include user data inside it.

And perhaps state_changed_cb could provide a poiner to the related otInstance instead of the context.

kkasperczyk-no · 2025-04-24T09:42:37Z

subsys/net/l2/openthread/openthread.c

 	if (pkt_list_peek(ot_context) == pkt) {
 		pkt_list_remove_last(ot_context);
-		NET_DBG("Got injected Ip6 packet, sending to upper layers");
+		NET_INFO("Got injected Ip6 packet, sending to upper layers");


As Robert mentioned there are log level changes here and there in this file. Is it intended?

kkasperczyk-no · 2025-04-24T09:47:24Z

modules/openthread/platform/openthread.c

+ */
+
+#include <zephyr/logging/log.h>
+LOG_MODULE_REGISTER(module_openthread, CONFIG_OPENTHREAD_PLATFORM_LOG_LEVEL);


All loggers from this directory are named like net_openthread_something. Maybe we should follow it and use net_openthread?

kkasperczyk-no · 2025-04-24T09:49:34Z

modules/openthread/platform/openthread.c

+	openthread_platform_mutex_unlock();
+}
+
+static void ot_joiner_start_handler(otError error, void *context)


Mark context as unused argument.

kkasperczyk-no · 2025-04-24T09:49:46Z

modules/openthread/platform/openthread.c

+	return len;
+}
+
+static void openthread_process(struct k_work *work)


Mark work as unused argument.

kkasperczyk-no · 2025-04-24T09:50:38Z

modules/openthread/platform/openthread.c

+	}
+}
+
+static void ot_state_changed_handler(uint32_t flags, void *context)


Mark context as unused argument. I'm stopping here, please go through the rest of the functions and make sure that unused arguments are marked as unused.

…eral" This reverts commit 3c0b184. Signed-off-by: Adam Kondraciuk <[email protected]>

…als" This reverts commit f07fb31. Signed-off-by: Adam Kondraciuk <[email protected]>

Add a shim that allows using the nRF TDM (Time division multiplexed audio interface) HAL by I2S Zephyr API. Upstream PR #: 82144 Signed-off-by: Adam Kondraciuk <[email protected]>

Some of nRF54's has TDM peripheral instead of I2S. Upstream PR #: 82144 Signed-off-by: Adam Kondraciuk <[email protected]>

ArekBalysNordic · 2025-04-24T12:39:19Z

@kkasperczyk-no @rlubos I modified the approach with headers. Now it is only one global header openthread.h file. I reused as many functions as possible from there. Functions that use openthread_context have been deprecated, and I created new ones without this context alongside.

This reverts commit d6cefa7. Signed-off-by: Jonathan Nilsen <[email protected]>

Upstream PR #: 88898 Changes for this package version: * CTRLSEL is now set appropriately for the SPIS121 pins. * DPPI.LINK configuration is now skipped when processing the 'dppic130' devicetree node. Signed-off-by: Jonathan Nilsen <[email protected]>

Same as commit 6ec9d10 but with the REQUIRED keyword on its own line to attempt to avoid a merge conflict when reverting/reapplying this patch. Signed-off-by: Jonathan Nilsen <[email protected]>

…RF54L09 and nRF54L20" This reverts commit 0afd70b. Signed-off-by: Michał Stasiak <[email protected]>

…o Kconfig Moved target dependent startup time from header file to Kconfig option to allow adding new targets with new .conf file rather that modyfing source of the test. Adjusted startup time for nRF54L09 and nRF54L20. Upstream PR #: 88956 Signed-off-by: Michał Stasiak <[email protected]>

What is changed? - Use the updated TF-M that is compatible with the Zephyr's latest Ethos-U driver repo. - Change the default behavior of TF-M builds to use Ethos driver locally fetched by Zephyr, using west update, instead of downloading it from external repo. Why is this change required? - This is to be inline with Zephyr's rules to not fetch code from external repo. Fixes #81656 Signed-off-by: Sudan Landge <[email protected]> (cherry picked from commit 9b0623e)

Updates the https://github.com/zephyrproject-rtos/trusted-firmware-m/ to fix the build error on stm32u5 due to gcc revision Signed-off-by: Francois Ramu <[email protected]> (cherry picked from commit 676c3d2)

Only set CMAKE_INSTALL_MESSAGE in a single location, remove the unconditional overrides of the value in other locations. This prevents dozens of rather pointless messages appearing in the build log when CONFIG_TFM_BUILD_LOG_QUIET is set. ``` Installing: /home/jordan/code/workspace/build/nrf5340dk/... ``` Signed-off-by: Jordan Yates <[email protected]> (cherry picked from commit b74b092)

…rry-picks Update brings in flash_area_get_sector. Signed-off-by: Dominik Ermel <[email protected]> (cherry picked from commit 789e3f3)

Update west Signed-off-by: Georgios Vasilakis <[email protected]> (cherry picked from commit d8b87d6)

Brings some commits from the upstream TF-M which introduce nRF54L10 ns support. Signed-off-by: Georgios Vasilakis <[email protected]> (cherry picked from commit 2717473)

Update TF-M to 2.1.2 from version 2.1.1. Signed-off-by: Tomi Fontanilles <[email protected]> (cherry picked from commit dfd0dd0)

Temp hack while waiting for NRF54H20_IRON support for Network in nrfutil. Upstream PR #: 88910 Signed-off-by: Sebastian Bøe <[email protected]>

Move OpenThread-related code from zephyr/subsys/net/l2/openthread/openthread.c to zephyr/modules/openthread/platform/openthread.c. The primary goal of this refactor is to enable the use of OpenThread as an independent module, without the necessity of Zephyr's networking layer. This change is particularly beneficial for simple applications that have their own implementation of the IEEE802.15.4 driver and do not require a networking layer. These applications can now disable Zephyr's L2 and IEEE802.15.4 shim layers and directly use the OpenThread module, saving valuable kilobytes of memory. In this approach if the CONFIG_NET_L2_OPENTHREAD Kconfig option is set, Zephyr's L2 and IEEE802.15.4 layer will be used, and everything will function as before. The main difference is the Zephyr's L2 layer now uses the OpenThread module, no longer implementing it. While most of the functions in include/net/openthread.h have been deprecated, they are still available for use to maintain backwards compatibility. Signed-off-by: Arkadiusz Balys <[email protected]>

ArekBalysNordic force-pushed the openthread_move_ot_implementation branch 4 times, most recently from a183839 to 66b9eb2 Compare April 24, 2025 08:06

gordonklaus and others added 2 commits April 24, 2025 10:21

[nrf noup] tests: bluetooth: tester: Enable PSA RNG on nRF54H20

07e10c6

The PSA is a cryptographically secure random number generator. It will be enabled by default, eventually, For now, enable it manually. Signed-off-by: Gordon Klaus <[email protected]>

[nrf fromlist] tests: drivers: clock_control_api: Adjust to nRF54L09 …

0afd70b

…and nRF54L20 nRF54L09 and nRF54L20 need double the time for startup of their clocks. Upstream PR #: 88956 Signed-off-by: Michał Stasiak <[email protected]>

rlubos reviewed Apr 24, 2025

View reviewed changes

kkasperczyk-no reviewed Apr 24, 2025

View reviewed changes

adamkondraciuk added 4 commits April 24, 2025 12:00

Revert "[nrf fromlist] tests: drivers: i2s: Align tests to TDM periph…

785502f

…eral" This reverts commit 3c0b184. Signed-off-by: Adam Kondraciuk <[email protected]>

Revert "[nrf fromlist] drivers: i2s: Add support for nRF TDM peripher…

5c1fbf9

…als" This reverts commit f07fb31. Signed-off-by: Adam Kondraciuk <[email protected]>

[nrf fromlist] drivers: i2s: Add support for nRF TDM peripherals

e22c042

Add a shim that allows using the nRF TDM (Time division multiplexed audio interface) HAL by I2S Zephyr API. Upstream PR #: 82144 Signed-off-by: Adam Kondraciuk <[email protected]>

[nrf fromlist] tests: drivers: i2s: Align tests to TDM peripheral

7221211

Some of nRF54's has TDM peripheral instead of I2S. Upstream PR #: 82144 Signed-off-by: Adam Kondraciuk <[email protected]>

ArekBalysNordic force-pushed the openthread_move_ot_implementation branch from 66b9eb2 to d781fe2 Compare April 24, 2025 12:34

jonathannilsen and others added 14 commits April 24, 2025 15:26

Revert "[nrf noup] moduled: hal_nordic: require nrf-regtool"

d771a3a

This reverts commit d6cefa7. Signed-off-by: Jonathan Nilsen <[email protected]>

[nrf noup] modules: hal_nordic: require nrf-regtool

4e9869f

Same as commit 6ec9d10 but with the REQUIRED keyword on its own line to attempt to avoid a merge conflict when reverting/reapplying this patch. Signed-off-by: Jonathan Nilsen <[email protected]>

Revert "[nrf fromlist] tests: drivers: clock_control_api: Adjust to n…

60cac62

…RF54L09 and nRF54L20" This reverts commit 0afd70b. Signed-off-by: Michał Stasiak <[email protected]>

[nrf fromtree] west.yml : update modules/tee/tf-m/trusted-firmware-m

a97c5a1

Updates the https://github.com/zephyrproject-rtos/trusted-firmware-m/ to fix the build error on stm32u5 due to gcc revision Signed-off-by: Francois Ramu <[email protected]> (cherry picked from commit 676c3d2)

[nrf fromtree] west.yml: Update TrustedFirmware-M with Flash Area che…

5b1d1dc

…rry-picks Update brings in flash_area_get_sector. Signed-off-by: Dominik Ermel <[email protected]> (cherry picked from commit 789e3f3)

[nrf fromtree] manifest: Bring TF-M with nRF54L15 upstream support

0ae7d3d

Update west Signed-off-by: Georgios Vasilakis <[email protected]> (cherry picked from commit d8b87d6)

[nrf fromtree] manifest: Bring TF-M with initial nRF54L10 support

8ac2b70

Brings some commits from the upstream TF-M which introduce nRF54L10 ns support. Signed-off-by: Georgios Vasilakis <[email protected]> (cherry picked from commit 2717473)

[nrf fromtree] manifest: tf-m: update to 2.1.2

532da2e

Update TF-M to 2.1.2 from version 2.1.1. Signed-off-by: Tomi Fontanilles <[email protected]> (cherry picked from commit dfd0dd0)

[nrf fromlist] west: runners: nrf_common: Temp hack for NRF54H20_IRON

a6f08b2

Temp hack while waiting for NRF54H20_IRON support for Network in nrfutil. Upstream PR #: 88910 Signed-off-by: Sebastian Bøe <[email protected]>

ArekBalysNordic force-pushed the openthread_move_ot_implementation branch from d781fe2 to 310e043 Compare April 25, 2025 07:28

ArekBalysNordic closed this Apr 29, 2025

openthread: Move OpenThread implementation from net to modules #1

openthread: Move OpenThread implementation from net to modules #1

Uh oh!

Conversation

ArekBalysNordic commented Apr 23, 2025

Uh oh!

rlubos left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArekBalysNordic commented Apr 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

16 participants