Skip to content

Nrfx 6118 add icbmsg to egpio #17321

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
merged 9 commits into from
Oct 22, 2024
Merged

Nrfx 6118 add icbmsg to egpio #17321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b060d4a
applications: sdp: gpio: move SRAM partitions to backend overlays
magp-nordic Oct 18, 2024
5d47510
application: sdp: add icbmsg backend for eGPIO
magp-nordic Sep 13, 2024
533b732
drivers: gpio: add icbmsg backend to eGPIO
magp-nordic Sep 13, 2024
0b050df
cmake: sysbuild: add icbmsg backend to eGPIO
magp-nordic Sep 13, 2024
f047771
snippets: emulated-gpio: add icbmsg backend for eGPIO
magp-nordic Sep 13, 2024
2b59517
sysbuild: add icbmsg as possible eGPIO backend
magp-nordic Oct 4, 2024
4267e9b
samples: basic: blinky: add eGPIO testcase
magp-nordic Oct 3, 2024
4b98781
tests: drivers: gpio: add test case for icbmsg backend
magp-nordic Oct 18, 2024
1c37e62
tests: drivers: gpio: egpio_basic_api: switch to `k_sleep`
masz-nordic Oct 22, 2024
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
1 change: 1 addition & 0 deletions applications/sdp/gpio/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ project(emulated_gpio)

target_sources(app PRIVATE src/main.c)
target_sources_ifdef(CONFIG_GPIO_NRFE_EGPIO_BACKEND_ICMSG app PRIVATE src/backend/backend_icmsg.c)
target_sources_ifdef(CONFIG_GPIO_NRFE_EGPIO_BACKEND_ICBMSG app PRIVATE src/backend/backend_icmsg.c)
target_sources_ifdef(CONFIG_GPIO_NRFE_EGPIO_BACKEND_MBOX app PRIVATE src/backend/backend_mbox.c)
22 changes: 0 additions & 22 deletions applications/sdp/gpio/boards/nrf54l15dk_nrf54l15_cpuflpr.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,6 @@
* SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
*/

/ {
soc {
reserved-memory {
#address-cells = <1>;
#size-cells = <1>;

sram_tx: memory@2003c000 {
reg = <0x2003c000 0x0800>;
};

sram_rx: memory@2003c800 {
reg = <0x2003c800 0x0800>;
};
};
};
};

&cpuflpr_sram {
reg = <0x2003d000 DT_SIZE_K(12)>;
ranges = <0x0 0x2003d000 0x3000>;
};

&cpuflpr_vevif_rx {
status = "okay";
interrupts = <16 NRF_DEFAULT_IRQ_PRIORITY>;
Expand Down
40 changes: 40 additions & 0 deletions applications/sdp/gpio/boards/nrf54l15dk_nrf54l15_cpuflpr_icbmsg.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright (c) 2024 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
*/

/ {
soc {
reserved-memory {
#address-cells = <1>;
#size-cells = <1>;

sram_tx: memory@2003b800 {
reg = <0x2003b800 0x0800>;
};

sram_rx: memory@2003c000 {
reg = <0x2003c000 0x0800>;
};
};
};

ipc {
ipc0: ipc0 {
compatible = "zephyr,ipc-icbmsg";
tx-region = <&sram_tx>;
rx-region = <&sram_rx>;
tx-blocks = <18>;
rx-blocks = <16>;
mboxes = <&cpuflpr_vevif_rx 16>, <&cpuflpr_vevif_tx 20>;
mbox-names = "rx", "tx";
status = "okay";
};
};
};

&cpuflpr_sram {
reg = <0x2003c800 DT_SIZE_K(14)>;
ranges = <0x0 0x2003c800 0x3800>;
};
20 changes: 20 additions & 0 deletions applications/sdp/gpio/boards/nrf54l15dk_nrf54l15_cpuflpr_icmsg.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,21 @@
*/

/ {
soc {
reserved-memory {
#address-cells = <1>;
#size-cells = <1>;

sram_tx: memory@2003c000 {
reg = <0x2003c000 0x0800>;
};

sram_rx: memory@2003c800 {
reg = <0x2003c800 0x0800>;
};
};
};

ipc {
ipc0: ipc0 {
compatible = "zephyr,ipc-icmsg";
Expand All @@ -16,3 +31,8 @@
};
};
};

&cpuflpr_sram {
reg = <0x2003d000 DT_SIZE_K(12)>;
ranges = <0x0 0x2003d000 0x3000>;
};
20 changes: 20 additions & 0 deletions applications/sdp/gpio/boards/nrf54l15dk_nrf54l15_cpuflpr_mbox.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,29 @@
*/

/ {
soc {
reserved-memory {
#address-cells = <1>;
#size-cells = <1>;

sram_tx: memory@2003c000 {
reg = <0x2003c000 0x0800>;
};

sram_rx: memory@2003c800 {
reg = <0x2003c800 0x0800>;
};
};
};

mbox_consumer: mbox_consumer {
compatible = "vnd,mbox-consumer";
mboxes = <&cpuflpr_vevif_rx 16>, <&cpuflpr_vevif_tx 20>;
mbox-names = "rx", "tx";
};
};

&cpuflpr_sram {
reg = <0x2003d000 DT_SIZE_K(12)>;
ranges = <0x0 0x2003d000 0x3000>;
};
13 changes: 13 additions & 0 deletions applications/sdp/gpio/sample.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,16 @@ tests:
- CONFIG_GPIO_NRFE_EGPIO_BACKEND_ICMSG=y
- CONFIG_IPC_SERVICE=y
- CONFIG_IPC_SERVICE_BACKEND_ICMSG=y

applications.sdp.egpio_icbmsg:
sysbuild: true
build_only: true
platform_allow: nrf54l15dk/nrf54l15/cpuflpr
tags: ci_build sysbuild
extra_args:
EXTRA_DTC_OVERLAY_FILE="./boards/nrf54l15dk_nrf54l15_cpuflpr_icbmsg.overlay"
extra_configs:
- CONFIG_GPIO_NRFE_EGPIO_BACKEND_ICBMSG=y
- CONFIG_IPC_SERVICE=y
- CONFIG_IPC_SERVICE_BACKEND_ICBMSG=y
- CONFIG_IPC_SERVICE_BACKEND_ICBMSG_NUM_EP=1
4 changes: 3 additions & 1 deletion applications/sdp/gpio/src/backend/backend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@

#include <drivers/gpio/nrfe_gpio.h>

#if !defined(CONFIG_GPIO_NRFE_EGPIO_BACKEND_ICMSG) && !defined(CONFIG_GPIO_NRFE_EGPIO_BACKEND_MBOX)
#if !defined(CONFIG_GPIO_NRFE_EGPIO_BACKEND_ICMSG) && \
!defined(CONFIG_GPIO_NRFE_EGPIO_BACKEND_MBOX) && \
!defined(CONFIG_GPIO_NRFE_EGPIO_BACKEND_ICBMSG)
#error "Define communication backend type"
#endif

Expand Down
12 changes: 12 additions & 0 deletions cmake/sysbuild/sdp.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,18 @@ function(egpio_update_kconfig)
endif()
sysbuild_cache_set(VAR ${DEFAULT_IMAGE}_SNIPPET APPEND REMOVE_DUPLICATES "emulated-gpio-icmsg")
message(STATUS "eGPIO: Using ICMSG backend")
elseif(SB_CONFIG_EGPIO_BACKEND_ICBMSG)
foreach(image ${PRE_CMAKE_IMAGES})
set_config_bool(${image} CONFIG_GPIO_NRFE_EGPIO_BACKEND_ICBMSG y)
set_property(TARGET ${image} APPEND_STRING PROPERTY CONFIG "CONFIG_IPC_SERVICE_BACKEND_ICBMSG_NUM_EP=1\n")
endforeach()
set_config_bool(flpr_egpio CONFIG_IPC_SERVICE y)
set_config_bool(flpr_egpio CONFIG_IPC_SERVICE_BACKEND_ICBMSG y)
if(DEFINED SB_CONFIG_SOC_NRF54L15)
sysbuild_cache_set(VAR flpr_egpio_EXTRA_DTC_OVERLAY_FILE APPEND REMOVE_DUPLICATES "./boards/nrf54l15dk_nrf54l15_cpuflpr_icbmsg.overlay")
endif()
sysbuild_cache_set(VAR ${DEFAULT_IMAGE}_SNIPPET APPEND REMOVE_DUPLICATES "emulated-gpio-icbmsg")
message(STATUS "eGPIO: Using ICBMSG backend")
endif()
endfunction()

Expand Down
2 changes: 1 addition & 1 deletion drivers/gpio/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
zephyr_library_amend()

if(CONFIG_GPIO_NRFE)
if(CONFIG_GPIO_NRFE_EGPIO_BACKEND_ICMSG)
if(CONFIG_GPIO_NRFE_EGPIO_BACKEND_ICMSG OR CONFIG_GPIO_NRFE_EGPIO_BACKEND_ICBMSG)
zephyr_library_sources(gpio_nrfe_icmsg.c)
elseif(CONFIG_GPIO_NRFE_EGPIO_BACKEND_MBOX)
zephyr_library_sources(gpio_nrfe_mbox.c)
Expand Down
5 changes: 5 additions & 0 deletions drivers/gpio/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,9 @@ config GPIO_NRFE_EGPIO_BACKEND_ICMSG
help
Use ICMSG backend driver for eGPIO.

config GPIO_NRFE_EGPIO_BACKEND_ICBMSG
bool "ICBMSG backend for eGPIO"
help
Use ICBMSG backend driver for eGPIO.

endchoice
4 changes: 3 additions & 1 deletion drivers/gpio/gpio_nrfe.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
#include <drivers/gpio/nrfe_gpio.h>
#include <zephyr/device.h>

#if !defined(CONFIG_GPIO_NRFE_EGPIO_BACKEND_ICMSG) && !defined(CONFIG_GPIO_NRFE_EGPIO_BACKEND_MBOX)
#if !defined(CONFIG_GPIO_NRFE_EGPIO_BACKEND_ICMSG) && \
!defined(CONFIG_GPIO_NRFE_EGPIO_BACKEND_MBOX) && \
!defined(CONFIG_GPIO_NRFE_EGPIO_BACKEND_ICBMSG)
#error "Configure communication backend type"
#endif

Expand Down
19 changes: 19 additions & 0 deletions scripts/twister/alt/zephyr/samples/basic/blinky/sample.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,22 @@ tests:
regex:
- "LED state: ON"
- "LED state: OFF"

sample.basic.blinky.egpio_icbmsg:
sysbuild: true
platform_allow:
- nrf54l15dk/nrf54l15/cpuapp
integration_platforms:
- nrf54l15dk/nrf54l15/cpuapp
extra_args: SB_CONFIG_SDP=y
SB_CONFIG_EGPIO_FLPR_APPLICATION=y
SB_CONFIG_EGPIO_BACKEND_ICBMSG=y
SB_CONFIG_PARTITION_MANAGER=n
EXTRA_DTC_OVERLAY_FILE="./boards/nrf54l15dk_nrf54l15_cpuapp_egpio.overlay"
harness: console
harness_config:
type: multi_line
ordered: true
regex:
- "LED state: ON"
- "LED state: OFF"
75 changes: 75 additions & 0 deletions snippets/emulated-gpio/icbmsg/boards/nrf54l15dk_nrf54l15_cpuapp.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* Copyright (c) 2024 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
*/

/ {
soc {
reserved-memory {
#address-cells = <1>;
#size-cells = <1>;

cpuflpr_code_partition: image@165000 {
/* FLPR core code partition */
reg = <0x165000 DT_SIZE_K(96)>;
};

sram_rx: memory@2003b800 {
reg = <0x2003b800 0x0800>;
};

sram_tx: memory@2003c000 {
reg = <0x2003c000 0x0800>;
};
};


cpuflpr_sram_code_data: memory@2003c800 {
compatible = "mmio-sram";
reg = <0x2003c800 DT_SIZE_K(14)>;
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x2003c800 0x3800>;
};
};

ipc {
ipc0: ipc0 {
compatible = "zephyr,ipc-icbmsg";
tx-region = <&sram_tx>;
rx-region = <&sram_rx>;
tx-blocks = <16>;
rx-blocks = <18>;
mboxes = <&cpuapp_vevif_rx 20>, <&cpuapp_vevif_tx 16>;
mbox-names = "rx", "tx";
status = "okay";
};
};
};

&cpuapp_sram {
reg = <0x20000000 DT_SIZE_K(242)>;
ranges = <0x0 0x20000000 0x3d000>;
};

&cpuflpr_vpr {
execution-memory = <&cpuflpr_sram_code_data>;
source-memory = <&cpuflpr_code_partition>;
};

&gpio2 {
status = "disabled";
};

&cpuapp_vevif_rx {
status = "okay";
};

&cpuapp_vevif_tx {
status = "okay";
};

&egpio {
status = "okay";
};
17 changes: 17 additions & 0 deletions snippets/emulated-gpio/icbmsg/emulated-gpio.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/* Copyright (c) 2024 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
*/

&cpuflpr_vpr {
status = "okay";

egpio: gpio {
compatible = "nordic,nrfe-gpio";
gpio-controller;
#gpio-cells = <0x2>;
ngpios = <0x10>;
status = "disabled";
port = <0x2>;
};
};
14 changes: 14 additions & 0 deletions snippets/emulated-gpio/icbmsg/snippet.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#
Copy link
Contributor

Choose a reason for hiding this comment

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

we are seeing too many SoC specific snippets for cases where either a more generic naming / approach could have been used, so I would like some more details regarding the use-cases this is intended for.

Could this snippet be more generic / useful in more cases where a backend is to be changed to icbmsg, but for other transport, in short eGPIO is used as transport for nRF54l15 but other transport could be used on other SoCs.

Copy link
Contributor

Choose a reason for hiding this comment

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

So I understand, the approach would be to rework this snippet to into two - one for generic "use icbmsg backend" and another for eGPIO specific stuff (which would also make other eGPIO snippets leaner). Then user would select a combination if two snippets when building eGPIO for given backend. Is that correct?
The issue I see with making this more generic is we specify here IDs of VEVIF channels and buffer sizes. Those might not be applicable to generic cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see another problem with separating backend from SDP used(eGPIO here): there is a possibility, that some SDPs will not support all backends, for example, icbmsg could be excluded due to being too "heavy". But that probably could be resolved by adding an if statement and error message in some CMake file.

Copy link
Contributor

Choose a reason for hiding this comment

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

So I understand, the approach would be to rework this snippet to into two - one for generic "use icbmsg backend" and another for eGPIO specific stuff (which would also make other eGPIO snippets leaner). Then user would select a combination if two snippets when building eGPIO for given backend. Is that correct?

I was actually considering still a single snippet.
The snippet being intended for ipc, and for ipc we can support different backends, in this case this snippet is for the icbmsg.
Generally the transport between the cores are more or less decided by the SoC design.

Thus a single snippet for ipc-icbmsg where the board / SoC part of the snippet decides how which overlay files to use.
That would allow sharing an ipc-icbmsg across multi-core SoC, like nRF5340, nrf54h20, nrf54l15.
https://docs.zephyrproject.org/latest/build/snippets/writing.html#board-specific-settings
Note, it's also possible to match on board qualifier, such as nrf54l15/cpuflpr or nrf5340/cpunet to match SoC, regardless of the board.

As for eGPIO, then if that should always be there for a ipc-icbmsg backend, then it can be a (forced) part of the snippet.
But iiuc then the eGPIO is just an example on how to do a SDP, but another sample / user might want to do something else, correct ?
If eGPIO is just an example, then it probably live best by itself as a sample, but if it is required for icbmsg, then I do think it can be part of a generic ipc-icbmsg snippet.

Copy link
Contributor Author

@magp-nordic magp-nordic Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@tejlmand Let me know if I understand you correctly: you want the same overlay to be applied when we are using a specific backend, for example, icbmsg, regardless of what SDP is being used, right? So that, if we add an implementation of another SDP, we wouldn't have to create another folder and copy overlays for the backends, just use the existing ones. Right now this would come down to renaming emulated-gpio folder to sdp and moving emulated-gpio.overlay into a commonplace (for example, just into the folder renamed to sdp). Am I right?

# Copyright (c) 2024 Nordic Semiconductor
#
# SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
#

name: emulated-gpio-icbmsg
append:
EXTRA_DTC_OVERLAY_FILE: emulated-gpio.overlay

boards:
nrf54l15dk/nrf54l15/cpuapp:
append:
EXTRA_DTC_OVERLAY_FILE: boards/nrf54l15dk_nrf54l15_cpuapp.overlay
5 changes: 5 additions & 0 deletions sysbuild/Kconfig.sdp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ config EGPIO_BACKEND_ICMSG
help
Use ICMSG backend driver for eGPIO.

config EGPIO_BACKEND_ICBMSG
bool "ICBMSG backend for eGPIO"
help
Use ICBMSG backend driver for eGPIO.

endchoice

endif # SDP
Expand Down
Loading
Loading