Nrfx 6118 add icbmsg to egpio

[magp-nordic]

Based on #16592, please, review only the last 5 commits.
Rebased on main.
PRs that affect this one:
#17669 (aligned)
#17173 (aligned)

[NordicBuilder]

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

Name	Old Revision	New Revision	Diff
zephyr	nrfconnect/sdk-zephyr@8005d4e	nrfconnect/sdk-zephyr#2003	nrfconnect/sdk-zephyr#2003/files

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

[NordicBuilder]

CI Information

^{To view the history of this post, clich the 'edited' button above}
Build number: 11

Inputs:

Sources:

sdk-nrf: PR head: 96c541d9ea7d26a448389d59b1f9b838c823683e

more details

sdk-nrf:

PR head: 96c541d9ea7d26a448389d59b1f9b838c823683e
merge base: 32bd0b96604393f1d8be7f94da967dec4c82913c
target head (main): 2fc583f30d262ac40df254277e78f234f43c9823
Diff

Github labels

Enabled	Name	Description
	ci-disabled	Disable the ci execution
	ci-all-test	Run all of ci, no test spec filtering will be done
	ci-force-downstream	Force execution of downstream even if twister fails
	ci-run-twister	Force run twister
	ci-run-zephyr-twister	Force run zephyr twister

List of changed files detected by CI (17)

applications
│  ├── sdp
│  │  ├── gpio
│  │  │  ├── CMakeLists.txt
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuflpr.overlay
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuflpr_icbmsg.overlay
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuflpr_icmsg.overlay
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuflpr_mbox.overlay
│  │  │  ├── sample.yaml
│  │  │  ├── src
│  │  │  │  ├── backend
│  │  │  │  │  │ backend.h
cmake
│  ├── sysbuild
│  │  │ sdp.cmake
drivers
│  ├── gpio
│  │  ├── CMakeLists.txt
│  │  ├── Kconfig
│  │  │ gpio_nrfe.h
scripts
│  ├── twister
│  │  ├── alt
│  │  │  ├── zephyr
│  │  │  │  ├── samples
│  │  │  │  │  ├── basic
│  │  │  │  │  │  ├── blinky
│  │  │  │  │  │  │  │ sample.yaml
snippets
│  ├── emulated-gpio
│  │  ├── icbmsg
│  │  │  ├── boards
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp.overlay
│  │  │  ├── emulated-gpio.overlay
│  │  │  │ snippet.yml
sysbuild
│  │ Kconfig.sdp
tests
│  ├── drivers
│  │  ├── gpio
│  │  │  ├── egpio_basic_api
│  │  │  │  │ testcase.yaml

Outputs:

Toolchain

Version: 3dd8985b56
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:3dd8985b56_81ed5a52d6

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

• ◻️ Toolchain - Skipped: existing toolchain is used
• ✅ Build twister
  • sdk-nrf test count: 1368
• ❌ Integration tests
  • ❌ test-fw-nrfconnect-boot
  • ✅ test-sdk-find-my
  • ❌ test-low-level
  • ✅ test-sdk-mcuboot
  • ✅ test-sdk-dfu

Disabled integration tests

• • desktop52_verification
  • doc-internal
  • test_ble_nrf_config
  • test-fw-nrfconnect-apps
  • test-fw-nrfconnect-ble_mesh
  • test-fw-nrfconnect-ble_samples
  • test-fw-nrfconnect-chip
  • test-fw-nrfconnect-fem
  • test-fw-nrfconnect-nfc
  • test-fw-nrfconnect-nrf-iot_cloud
  • test-fw-nrfconnect-nrf-iot_libmodem-nrf
  • test-fw-nrfconnect-nrf-iot_lwm2m
  • test-fw-nrfconnect-nrf-iot_mosh
  • test-fw-nrfconnect-nrf-iot_nrf_provisioning
  • test-fw-nrfconnect-nrf-iot_positioning
  • test-fw-nrfconnect-nrf-iot_samples
  • test-fw-nrfconnect-nrf-iot_serial_lte_modem
  • test-fw-nrfconnect-nrf-iot_thingy91
  • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
  • test-fw-nrfconnect-nrf_crypto
  • test-fw-nrfconnect-proprietary_esb
  • test-fw-nrfconnect-ps
  • test-fw-nrfconnect-rpc
  • test-fw-nrfconnect-rs
  • test-fw-nrfconnect-tfm
  • test-fw-nrfconnect-thread
  • test-fw-nrfconnect-zigbee
  • test-sdk-audio
  • test-sdk-pmic-samples
  • test-sdk-sidewalk
  • test-sdk-wifi
  • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

[magp-nordic]

Now based on #17256 (first commit)
Please, do not review yet, needs some alignment after #16592 was merged.

[NordicBuilder]

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

[jaz1-nordic]

nitpicks. Additionally please resolve complaints from CI.

[magp-nordic]

Rebased on main, ready for review

[masz-nordic]

@nordicjm can you take a look?

[nordicjm]

Based on #16592, please, review only the last 5 commits.

There's 6 commits, so surely this needs a rebase?

[magp-nordic]

@nordicjm It's already rebased on main, I forgot to update the description comment.

[tejlmand]

need some clarification regarding the icbmsg backend and overlays / snippet.

[tejlmand]

would like some more information on this.

Other boards, such as nrf54h20dk, nrf5340dk, thingy53, are defining default ipc nodes, which can then have the status changed from disabled to okay.

Any reason the nrf54l15dk doesn't define a default (perhaps disabled) ipc node ?

Examples upstream:
https:/zephyrproject-rtos/zephyr/blob/main/boards/nordic/nrf54h20dk/nrf54h20dk_nrf54h20-ipc_conf.dtsi
https:/zephyrproject-rtos/zephyr/blob/1726443d9d2584bc6adb7fd08d63a5fc645ce09a/boards/nordic/nrf54h20dk/nrf54h20dk_nrf54h20_cpuapp.dts#L159

Or at SoC level, like the nrf5340:
https:/zephyrproject-rtos/zephyr/blob/1726443d9d2584bc6adb7fd08d63a5fc645ce09a/dts/arm/nordic/nrf5340_cpunet.dtsi#L349-L350
https:/zephyrproject-rtos/zephyr/blob/1726443d9d2584bc6adb7fd08d63a5fc645ce09a/dts/arm/nordic/nrf5340_cpuapp.dtsi#L110-L111

@gmarull perhaps you have some input regarding this.

[masz-nordic]

My understanding is that H20 has established strict resource allocation scheme (in this case VEVIF/BELLBOARD channels and shared memory) due to complexity and security architecture.
Meanwhile, L15 has only two CPUs with access to all resources by default, so it is up to developer to decide what to use. It is much closer to 53's case - I'm not aware what was the reasoning behind those defaults.
@hubertmis can you comment?

[tejlmand]

Meanwhile, L15 has only two CPUs with access to all resources by default, so it is up to developer to decide what to use.

but in current case, you are asking every sample to create the overlay and decide on the ram allocation.
Which in most case will result in users looking for a template like this file, and start copying that around.

If the board provide a sane set of defaults, then a sample should be a to just set the node to enabled and not worry about other parts.
Users with special need can still adjust memory allocation as they find fit.

[magp-nordic]

@tejlmand, can we change that in a follow-up PR? I wouldn't want to change it here since this PR only adds another backend to eGPIO. If I were to move the definition of the IPC node to board or soc dts, I would also have to modify overlays for other eGPIO backends (mbox and icmsg), which is not the topic of this PR.
@hubertmis, can you share your opinion on where the IPC definition should be placed?

[tejlmand]

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.

[masz-nordic]

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.

[magp-nordic]

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.

[tejlmand]

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.

[magp-nordic]

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

[tejlmand]

this overlay is always being applied regardless of the board being built for.

This is not a good use of snippet, as it ties the snippet hard to the board in use.

[magp-nordic]

If my understanding is correct, that was the point: to have this overlay applied to every build with SDP, no matter what board is used. FLPR has to be turned on because it emulates a peripheral. If a board does not have FLPR, the build will fail, as it should, because a peripheral cannot be emulated (at least that is the assumption behind SDP).
@jaz1-nordic could you confirm?

[jaz1-nordic]

This is not a general snippet that may have been enabled on every board. This snippet applies only and exclusively to software-defined peripherals and, as the name suggests, is used to add and enable egpio on the cpuflpr core. Additionally, it is applied by sysbuild mechanisms for specific, previously selected boards. Not directly. The only thing I can find fault with is the naming of cpuflpr_vpr, which may vary depending on the board/SoC. So maybe move the entire code to an overlay file for a specific board?

[magp-nordic]

The only thing I can find fault with is the naming of cpuflpr_vpr, which may vary depending on the board/SoC.

I checked, and nRF54H20 is the only other platform, that also has FLPR defined in dts and it also uses cpuflpr_vpr name. So it should not be a problem.

[magp-nordic]

@nrfconnect/ncs-co-drivers please, review