manifest: Adding nRF Services library #70245

Rafal-Nordic · 2024-03-14T18:47:04Z

Adding nRF Services library to the hal-nordic repo

Depends on zephyrproject-rtos/docker-image#181

github-actions · 2024-03-14T18:47:42Z

Hello @Rafal-Nordic, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

zephyrbot · 2024-03-14T18:48:01Z

The following west manifest projects have been modified in this Pull Request:

Name	Old Revision	New Revision	Diff

Note: This message is automatically posted and updated by the Manifest GitHub Action.

modules/hal_nordic/nrfs/CMakeLists.txt

jfischer-no · 2024-03-27T19:01:14Z

modules/hal_nordic/nrfs/CMakeLists.txt

+# SPDX-License-Identifier: Apache-2.0
+
+if(CONFIG_NRFS)
+    if(CONFIG_NRFS_LOCAL_DOMAIN)


CMakeLists.txt in the hal/nordic/nrfs is not included.

diff --git a/modules/hal_nordic/nrfs/CMakeLists.txt b/modules/hal_nordic/nrfs/CMakeLists.txt index a4edc6be513..eec370cf9b1 100644 --- a/modules/hal_nordic/nrfs/CMakeLists.txt +++ b/modules/hal_nordic/nrfs/CMakeLists.txt @@ -2,6 +2,8 @@ # SPDX-License-Identifier: Apache-2.0 if(CONFIG_NRFS) + add_subdirectory(${ZEPHYR_CURRENT_MODULE_DIR}/nrfs nrfs) + if(CONFIG_NRFS_LOCAL_DOMAIN) zephyr_sources_ifdef(CONFIG_NRFS_BACKEND_IPC_SERVICE_EXTENDED backends/nrfs_backend_ipc_service.c)

CMakeLists are now completely reorganized.

jfischer-no · 2024-03-27T19:05:41Z

modules/hal_nordic/nrfs/Kconfig

+config USB_SERVICE_ENABLED
+	bool "USB service"
+	depends on NRFS_HAS_USB_SERVICE
+	default y


There must not be anything prefixed with USB_ or USB_SERVICE_ or have prompt "USB service" that is not part of the USB support in Zephyr. You need to find another short and concise name for it.

jfischer-no · 2024-03-27T19:07:13Z

modules/hal_nordic/nrfs/Kconfig

+	help
+	  This option enables the nRF Services library
+
+config NRFS_LOCAL_DOMAIN


What exactly is LOCAL_DOMAIN? Is there also REMOTE_DOMAIN?

There are local and global domains in this SoC architecture

"Local domain" name comes from Product Specification and is commonly used in the MDK. nrf54H20 has the following local domains:

App core

Rad core

Secure domain

jfischer-no · 2024-03-27T19:08:40Z

modules/hal_nordic/nrfs/Kconfig

+	help
+	  This option enables the nRF Services Local Domain libraries
+
+config NRFS_BACKEND_IPC_SERVICE


Can NRFS be use without IPC?

It's possible to provide custom implementation of the backend layer. The default one supported by Nordic is the IPC Service.

SysCtrl supports only IPC_SERVICE backend protocol today, but other domains use also other protocols, so I don't exclude that in the future additional protocols will be supported.

jfischer-no · 2024-03-27T19:10:38Z

modules/hal_nordic/nrfs/backends/Kconfig

+config NRFS_BACKEND_IPC_SERVICE_EXTENDED
+	bool "Zephyr IPC service backend for NRFS with additional buffering."
+
+config NRFS_BACKEND_IPC_SERVICE_LITE
+	bool "Use IPC service directly by NRFS"


Why two backends and what are the differences?

I checked that "standard" version doesn't make sense anymore. I'll remove it

jfischer-no · 2024-03-27T19:17:38Z

modules/hal_nordic/nrfs/backends/nrfs_backend_ipc_service.c

+};
+
+static struct ipc_channel_config_t ipc_cpusys_channel_config = {
+	.ipc_instance	 = DEVICE_DT_GET(DT_NODELABEL(ipc_to_cpusys)),


That finally blocked me, I could not fix it myself quickly. Neither ipc_to_cpusys nor cpuapp_cpusys_ipc seems to exist with all the other things that belong to it. It is like a rocket without a propulsion system. Same for the lite backend.

Yes, not all required dts files were moved to the upstream repo. I'm checking how to fix it

Implemented here: #71265

modules/hal_nordic/nrfs/Kconfig

jfischer-no · 2024-05-10T08:03:50Z

modules/hal_nordic/nrfs/dvfs/ld_dvfs_handler.c

+/**
+ * @brief Helper function to set bit in state variable.
+ *
+ * @param bit_pos bit position in state variable.
+ */


Again, please remove redundant comments / redundant doxygen comments. It does not provide valuable information.

jfischer-no · 2024-05-13T08:06:49Z

modules/hal_nordic/nrfs/backends/nrfs_backend_ipc_service.h

+extern "C" {
+#endif
+
+struct __NRFS_PACKED ipc_data_packet_t {


Can we use __packed here?

struct __packed ipc_data_packet {

Please remove _t

jfischer-no · 2024-05-13T08:08:09Z

modules/hal_nordic/nrfs/backends/nrfs_backend_ipc_service.c

+
+#define IPC_INIT_DONE_EVENT (0x01)
+
+struct ipc_channel_config_t {


struct ipc_channel_config {

_t

jfischer-no · 2024-05-13T08:10:14Z

modules/hal_nordic/nrfs/dvfs/ld_dvfs_handler.c

+/**
+ * @brief Task to handle dvfs init procedure.
+ *
+ * @param dummy0
+ * @param dummy1
+ * @param dummy2
+ */


/* Task to handle dvfs init procedure. */

jfischer-no · 2024-05-13T08:11:13Z

modules/hal_nordic/nrfs/dvfs/ld_dvfs_handler.c

+/**
+ * @brief Function to set hsfll to highest frequency when switched to ABB.
+ *
+ */


/* Function to set hsfll to highest frequency when switched to ABB. */

jfischer-no · 2024-05-13T08:12:43Z

modules/hal_nordic/nrfs/dvfs/ld_dvfs_handler.c

+/**
+ * @brief DVFS event handler callback function.
+ *
+ * @param p_evt event to handle
+ * @param context context data
+ */
+void nrfs_dvfs_evt_handler(nrfs_dvfs_evt_t const *p_evt, void *context)


static void nrfs_dvfs_evt_handler(nrfs_dvfs_evt_t const *p_evt, void *context)

jfischer-no · 2024-05-13T08:13:47Z

modules/hal_nordic/nrfs/dvfs/ld_dvfs_handler.c

+/**
+ * @brief Function handling steps for scaling preparation.
+ *
+ * @param oppoint_freq requested frequency oppoint
+ */


/* Function handling steps for scaling preparation. */

jfischer-no · 2024-05-13T08:14:20Z

modules/hal_nordic/nrfs/dvfs/ld_dvfs_handler.c

+/**
+ * @brief Function to check if current operation is down-scaling
+ *
+ * @param target_freq_setting frequency setting target
+ * @return true down-scaling procedure
+ * @return false not down-scaling procedure
+ */


/* Function to check if current operation is down-scaling */

anangl · 2024-05-13T09:10:06Z

modules/hal_nordic/nrfs/CMakeLists.txt

+# Copyright (c) 2024 Nordic Semiconductor ASA
+# SPDX-License-Identifier: Apache-2.0
+
+zephyr_library()


Please move this under if(CONFIG_NRFS) to prevent the following CMake warning:

No SOURCES given to Zephyr library: modules__hal_nordic__nrfs

when NRFS is not enabled.

tmon-nordic · 2024-05-13T12:33:11Z

When I disabled NRFS_DVFS_SERVICE_ENABLED Kconfig symbol, the build failed with undefined references to nrfs_dvfs_init_prepare_request and nrfs_dvfs_ready_to_scale. There is either some missing dependencies in Kconfig symbols or the Kconfigs are simply incorrect (it shouldn't be possible to get broken builds by disabling Kconfig symbols in menuconfig).

Adding nRF Services library to the hal-nordic repo Signed-off-by: Rafal Dyla <rafal.dyla@nordicsemi.no>

Add driver, together with the corresponding dts binding and node in the nRF54H20 SoC definiton, for the nRF temperature sensor that cannot be accessed directly but only through nRF Services (nrfs) layer. Signed-off-by: Andrzej Głąbek <andrzej.glabek@nordicsemi.no>

Add a simple test intended primarily for checking the nRF temperature sensor in its nrfs variant but intentionally was made generic enough that it should be usable with other temperature sensors (after just providing an overlay) and could be helpful in development of new temperature sensor drivers, and possibly extended then if needed. Signed-off-by: Andrzej Głąbek <andrzej.glabek@nordicsemi.no>

… accessed via nrfs Upstream PR: zephyrproject-rtos/zephyr#70245 Add driver, together with the corresponding dts binding and node in the nRF54H20 SoC definiton, for the nRF temperature sensor that cannot be accessed directly but only through nRF Services (nrfs) layer. Signed-off-by: Andrzej Głąbek <andrzej.glabek@nordicsemi.no> (cherry picked from commit 8e76ae58fa3cccd1bb520cfced5992d3fe1c8c55)

Upstream PR: zephyrproject-rtos/zephyr#70245 Add a simple test intended primarily for checking the nRF temperature sensor in its nrfs variant but intentionally was made generic enough that it should be usable with other temperature sensors (after just providing an overlay) and could be helpful in development of new temperature sensor drivers, and possibly extended then if needed. Signed-off-by: Andrzej Głąbek <andrzej.glabek@nordicsemi.no> (cherry picked from commit 37f4572b2c21b56322ae5d8779549e40c38ee36a)

github-actions · 2024-05-15T08:27:09Z

Hi @Rafal-Nordic!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

zephyrbot added manifest manifest-hal_nordic DNM This PR should not be merged (Do Not Merge) labels Mar 14, 2024

carlescufi requested review from nika-nordic, carlescufi, anangl, gmarull and nordicjm March 15, 2024 09:05

Rafal-Nordic force-pushed the nrfs_upstreaming branch from 487e9aa to c5bddb9 Compare March 20, 2024 20:35

zephyrbot added the platform: nRF Nordic nRFx label Mar 20, 2024

zephyrbot requested review from hubertmis and nordic-krch March 20, 2024 20:36

zephyrbot assigned anangl Mar 20, 2024

Rafal-Nordic force-pushed the nrfs_upstreaming branch 3 times, most recently from ddd8d3b to faedfa1 Compare March 23, 2024 07:02

jfischer-no requested changes Mar 27, 2024

View reviewed changes

Rafal-Nordic force-pushed the nrfs_upstreaming branch from faedfa1 to 486c552 Compare April 10, 2024 19:10

zephyrbot added area: Process area: RISCV RISCV Architecture (32-bit & 64-bit) area: Devicetree labels Apr 10, 2024

zephyrbot requested review from decsny, galak, MaureenHelm, nashif and stephanosio April 10, 2024 19:11

Rafal-Nordic force-pushed the nrfs_upstreaming branch from 486c552 to de211c3 Compare April 11, 2024 10:07

tmon-nordic requested changes Apr 11, 2024

View reviewed changes

modules/hal_nordic/nrfs/Kconfig Outdated Show resolved Hide resolved

Rafal-Nordic force-pushed the nrfs_upstreaming branch 2 times, most recently from d3a735d to 101f9e1 Compare May 10, 2024 06:15

zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label May 10, 2024

jfischer-no requested changes May 10, 2024

View reviewed changes

Rafal-Nordic force-pushed the nrfs_upstreaming branch from 101f9e1 to be781fc Compare May 10, 2024 16:24

carlescufi requested review from hubertmis, jfischer-no, ppelikan-nordic, anangl and tmon-nordic May 13, 2024 07:57

jfischer-no requested changes May 13, 2024

View reviewed changes

anangl reviewed May 13, 2024

View reviewed changes

Rafal-Nordic force-pushed the nrfs_upstreaming branch from be781fc to 2044bce Compare May 13, 2024 13:44

carlescufi requested review from anangl and jfischer-no May 14, 2024 10:37

Rafal-Nordic and others added 3 commits May 14, 2024 13:09

manifest: Adding nRF Services library

9000a6e

Adding nRF Services library to the hal-nordic repo Signed-off-by: Rafal Dyla <rafal.dyla@nordicsemi.no>

Rafal-Nordic force-pushed the nrfs_upstreaming branch from 2044bce to 670d4e7 Compare May 14, 2024 11:12

zephyrbot removed manifest manifest-hal_nordic labels May 14, 2024

tmon-nordic approved these changes May 14, 2024

View reviewed changes

jfischer-no approved these changes May 14, 2024

View reviewed changes

jfischer-no mentioned this pull request May 14, 2024

boards: nrf54h20dk: enable USB device controller #72774

Merged

anangl approved these changes May 15, 2024

View reviewed changes

fabiobaltieri merged commit 53c350e into zephyrproject-rtos:main May 15, 2024
24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

manifest: Adding nRF Services library #70245

manifest: Adding nRF Services library #70245

Rafal-Nordic commented Mar 14, 2024 •

edited by carlescufi

github-actions bot commented Mar 14, 2024

zephyrbot commented Mar 14, 2024 •

edited

jfischer-no Mar 27, 2024

Rafal-Nordic Apr 16, 2024

jfischer-no Mar 27, 2024

Rafal-Nordic Apr 16, 2024

jfischer-no Mar 27, 2024

hubertmis Apr 2, 2024

Rafal-Nordic Apr 3, 2024

jfischer-no Mar 27, 2024

ppelikan-nordic Apr 2, 2024

Rafal-Nordic Apr 3, 2024

jfischer-no Mar 27, 2024

Rafal-Nordic Apr 3, 2024

Rafal-Nordic Apr 16, 2024

jfischer-no Mar 27, 2024

Rafal-Nordic Apr 3, 2024

Rafal-Nordic Apr 16, 2024

jfischer-no May 10, 2024

jfischer-no May 13, 2024

jfischer-no May 13, 2024

jfischer-no May 13, 2024

jfischer-no May 13, 2024

jfischer-no May 13, 2024

jfischer-no May 13, 2024

jfischer-no May 13, 2024

anangl May 13, 2024

tmon-nordic commented May 13, 2024 •

edited

github-actions bot commented May 15, 2024


		#define IPC_INIT_DONE_EVENT (0x01)

		struct ipc_channel_config_t {

manifest: Adding nRF Services library #70245

manifest: Adding nRF Services library #70245

Conversation

Rafal-Nordic commented Mar 14, 2024 • edited by carlescufi

github-actions bot commented Mar 14, 2024

zephyrbot commented Mar 14, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmon-nordic commented May 13, 2024 • edited

github-actions bot commented May 15, 2024

Rafal-Nordic commented Mar 14, 2024 •

edited by carlescufi

zephyrbot commented Mar 14, 2024 •

edited

tmon-nordic commented May 13, 2024 •

edited