bt: ipc: Add ipc multiendpoint

[pszkotak]

Multiprotocol on the nRF53 requires multiple IPC channels to communicate
between the APP and the NET core. One way is to duplicate the entire openamp
instance, the other one is to use a single instance with multiple endpoints.

Aim of this PR is to achieve the goal using the simpler single instance
with the multiple endpoints.

At this point hci_rpmsg net core sample and peripheral_hr app core sample link with the new implementation.

To build:

cd ncs/zephyr/samples/bluetooth/hci_rpmsg
west build -b nrf5340dk_nrf5340_cpunet -- -DCONF_FILE="prj.conf overlay-multiendpoint-ipc.conf"

cd ncs/zephyr/samples/bluetooth/peripheral_hr
west build -b nrf5340dk_nrf5340_cpuapp -- -DCONF_FILE="prj.conf overlay-multiendpoint-ipc.conf"

For now we are aware of the following issues that may need addressing:

• All samples build without the need for the overlay (use the rpmsg_service across all samples?)
• Do not directly depend on the IPM peripheral. How to do it?

Signed-off-by: Piotr Szkotak [email protected]

[github-actions]

The following projects have a revision update in this Pull Request:

Name	Old Revision	New Revision
hal_nordic	zephyrproject-rtos/hal_nordic@f163c53	zephyrproject-rtos/hal_nordic@74e4ab0 (master)

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

[hubertmis]

Thanks @pszkotak for posting this PR! It's crucial to enable multiple services on nRF53 network core. Currently only HCI, or 802.15.4 serialization is available (not simultaneously), what is a limitation preventing many use-cases.

I've noticed that apart from hci_rpmsg and peripheral_hr samples, this PR also enables the use of the new IPC service for all 802.15.4 samples built for nRF53 SoC.

It looks that this PR partially fixes #20419. The missing part are priorities of endpoints requested by @nordic-krch. But for the use-case described by @pszkotak (simultaneous HCI and 802.15.4 serialization) the priorities support is not required. Because of that I think it is worth to merge this PR and add extend this module with priorities support later.

[hubertmis]

@ioannisg, while clearing Compliance errors I've added you as a code owner for the new IPC service. I've chosen you, because you are already code owner of some IPM drivers. Based on that I've assumed you have expertise in IPC area. If I chose poorly, please advice us who would be a better candidate to own this module.

[arnopo]

Very interesting PR! this is something that was clearly missing...

It would be interesting to split the rpmsg-service.c into 2 separate files

• rpmsg-service.c:
  • rpmsg_init_vdev
  • services related to the rpmsg endpoint
  • IPM callback and process
• rpmsg_backend.c ( or another name):
  • metal initialization
  • vrings initialization
  • IPM initilaization

The reason of this suggestion is that there are 2 different topologies supported in Zephyr.

• Zephyr <-> Zephyr
  This is the topology you implement.
• Linux <-> Zephyr
  The zephyr is running on a co-processor and Linux on an application processor. The vrings for the inter-processor communication are defined in a shared table named resource table, and initialized by the Linux.
  A sample is available here:
  https://github.com/zephyrproject-rtos/zephyr/blob/master/samples/subsys/ipc/openamp_rsc_table/src/main_remote.c

Splitting code in 2 files would allow to extend your dev in a next step to support the second topology and potentially new ones.

[hubertmis]

It would be interesting to split the rpmsg-service.c into 2 separate files

I've added a commit splitting the module. @arnopo, please have a look if this is what you had in mind.

[arnopo]

It would be interesting to split the rpmsg-service.c into 2 separate files

I've added a commit splitting the module. @arnopo, please have a look if this is what you had in mind.

It would be interesting to split the rpmsg-service.c into 2 separate files

I've added a commit splitting the module. @arnopo, please have a look if this is what you had in mind.

Thanks! Yes this is what i had in mind.
Perhaps something is missing regarding the IO struct that seems used uninitialized in rpmsg_init_vdev call...

[hubertmis]

this function should at least return an error if called after the init step for the slave

Fixed, verifying order of execution using a simple flag. Returning -EINPROGRESS if endpoint registration is requested too late.

[kapi-no]

Approving as an intermediate step, As discussed with @hubertmis, I am hoping to see my review suggestions for API improvements implemented in the subsequent PRs.

[nashif]

Looks much better now, one last minor request. Can you move rpmsg_service into subsys/ipc/rpmsg_service ? Subsystems need to be generic, rpmsg_service is an implementation of an IPC, and we will have others.

[hubertmis]

Can you move rpmsg_service into subsys/ipc/rpmsg_service ?

Sure. Just pushed changes. Should be ready for review.

[ioannisg]

Needs to get the right SHA for the nRF module

[hubertmis]

This PR is now blocked by #31224 , because of hal_nordic dependency

[ioannisg]

@ioannisg, while clearing Compliance errors I've added you as a code owner for the new IPC service. I've chosen you, because you are already code owner of some IPM drivers. Based on that I've assumed you have expertise in IPC area. If I chose poorly, please advice us who would be a better candidate to own this module.

Thanks, that should be fine for now :)