Skip to content

OpenAMP as service for multiple users #20419

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.

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

Closed
nordic-krch opened this issue Nov 7, 2019 · 14 comments
Closed

OpenAMP as service for multiple users #20419

nordic-krch opened this issue Nov 7, 2019 · 14 comments
Assignees
Labels
area: IPC Inter-Process Communication Enhancement Changes/Updates/Additions to existing features

Comments

@nordic-krch
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
Currently, open_amp is available only as a simple demo (samples/subsys/ipc/openamp) where initialization is performed inside main.c of the example. It's not generic enough to add it to any application and it does not show how multiple, independent users shall use open_amp. There is a need to support registration of multiple users to open_amp instance to exchange messages. Multi-domain logger or shell capable of executing commands on remote core are examples of generic services that may be added along application specific open_amp communication.

Different open_amp users will have different priorities and that should also be handled by this open_amp manager.

Describe the solution you'd like
I would like to see a module or device which will manage single open_amp instance. I would like to register an endpoint with specified priority. open_amp manager would be initilized during startup.

@nordic-krch nordic-krch added Feature Request A request for a new feature area: IPC Inter-Process Communication labels Nov 7, 2019
@nordic-krch
Copy link
Collaborator Author

@galak @arnopo do you know if this is planned in near future?

@carlescufi carlescufi changed the title Open_amp as service for multiple users OpenAMP as service for multiple users Nov 7, 2019
@arnopo
Copy link
Collaborator

arnopo commented Nov 7, 2019

@nordic-krch :
Today it is already possible to have several users registering endpoints/ services, but it is not possible to deal with priorities of the RPMsgs or limit end point bandwidth( flow control).
I don't see a restriction in having several threads that send RPMsgs. This should help you to manage priorities in transmission.
In reception it is different, no assumption on the message destination, it is a FIFO mechanism.

if you want to see an example of simple instance created in a device, you can have a look here. This is the shared I2C demo we showed during the ELCE in Lyon. please notice that in this demo The application processor is running a Linux and the coprocessor is running a Zephyr (i'm reworking my the Linux code associated to upload it on my github...)
sample: https://github.com/arnopo/zephyr_stm32mp1/tree/I2C_demo/samples/subsys/ipc/openamp_rsc_table/i2c.
virtual i2c driver:
https://github.com/arnopo/zephyr_stm32mp1/blob/I2C_demo/drivers/i2c/i2c_rpmsg.c

Then please find here the list of the future topics to discuss in OpenAMP community:
https://github.com/OpenAMP/open-amp/wiki#Future_topics

@nordic-krch
Copy link
Collaborator Author

@arnopo thanks for the answer. Still today there are couple of things missing in the openamp framework:

  • there is no system initialization. It must be initialized by the sample (like in your example). It means that standard, single core sample cannot run on 2 core SoC without modifications. For example standard bluetooth sample cannot run on nrf5340 which has 2 cores (application+network) because someone needs to initialize openamp. It could be hci driver but then if there is another module willing to use openamp it must initilize own instance of openamp.
  • as you mentioned, there is no end point priorities scheme. If i have an application which is using openamp for bluetooth hci messages and logging on another endpoint then i would prefer to get log message dropped rather than bluetooth messages.

@arnopo
Copy link
Collaborator

arnopo commented Nov 13, 2019

@arnopo thanks for the answer. Still today there are couple of things missing in the openamp framework:

  • there is no system initialization. It must be initialized by the sample (like in your example). It means that standard, single core sample cannot run on 2 core SoC without modifications. For example standard bluetooth sample cannot run on nrf5340 which has 2 cores (application+network) because someone needs to initialize openamp. It could be hci driver but then if there is another module willing to use openamp it must initilize own instance of openamp.

Your point is not crystal clear for me....so please tell me if i misunderstood
Yes you have to manage the master/slave notion in system initialization (e.g. application core is master, network core slave). But in sample involving OpenAMP you have (generally) a RPMsg service provider and a RPMsg service client, with one implemented on a core and the other one implemented on the the other core. So anyway (IHMO), not possible to run exactly the same sample on both core.
Concerning driver and module: they should be independent as they just have to register endpoint to declare services.

  • as you mentioned, there is no end point priorities scheme. If i have an application which is using openamp for bluetooth hci messages and logging on another endpoint then i would prefer to get log message dropped rather than bluetooth messages.
    Today a back up solution could be to duplicate vrings. I did not test it but should be possible. For sure this solution would be not optimized in term of foot print.

@nordic-krch
Copy link
Collaborator Author

@arnopo i was not clear then. I meant that i want to run bluetooth sample on application core. Application core has no radio but has OpenAMP hci driver. So i enable it in kconfig and that should run. OpenAMP instance should be initialized under the hood by the system and hci driver should only create/attach its endpoint. Currently, to do that we had to add OpenAMP instance initialization in hci driver. With that approach if i want to enable another module which is using OpenAMP I need to initialize OpenAMP instance there too, assuming that OpenAMP supports multiple instances on single IPM.

I would like to see a service in zephyr where after enabling OpenAMP in kconfig I get OpenAMP device which i can use in any module. In module which is using OpenAMP it would look something like:

struct device *oamp_dev = device_get_binding("OpenAMP");
struct oamp_ep *ep = oamp_lib_create_ep(oamp_dev, OAMP_PRIO_HIGH, bind_cb, rx_cb);

//when binded
int err = oamp_lib_tx(ep, data, len);

Today complex initialization procedure (metallib, ipm, openamp,..) is done in sample code: https://github.com/zephyrproject-rtos/zephyr/blob/master/samples/subsys/ipc/openamp/src/main.c

@arnopo
Copy link
Collaborator

arnopo commented Nov 14, 2019

@nordic-krch
Thanks for the clarification!
Yes there is a point to improve here. if you have a look to my example (https://github.com/arnopo/zephyr_stm32mp1/blob/I2C_demo/drivers/i2c/i2c_rpmsg.c) i need to use the exported "rpdev" variable that is the phandle to the OpenAMP instance.

And you are right need a generic way to declare/initiate RPMsg based on DT with rpmsg client declared as childs.
Then the module associated to the rpmsg node should be compatible with the both way to initialize the RPMSG:

@nordic-krch
Copy link
Collaborator Author

Yes there is a point to improve here.

@arnopo @galak given that, is there any plans to improve that in near future?

@arnopo
Copy link
Collaborator

arnopo commented Nov 15, 2019

Not in near future on my side.
Personally i would be in favour of first integrating the ongoing OpenAMP PRs:

And then, in a second step, find a comment way to ease the OpenAMP initialization relying on DT.

@carlescufi carlescufi added Enhancement Changes/Updates/Additions to existing features and removed Feature Request A request for a new feature labels Mar 24, 2020
@carlescufi
Copy link
Member

Not in near future on my side.
Personally i would be in favour of first integrating the ongoing OpenAMP PRs:

@arnopo this is now done

And then, in a second step, find a comment way to ease the OpenAMP initialization relying on DT.

@arnopo are you working on this? CC @ioannisg

@arnopo
Copy link
Collaborator

arnopo commented Jul 17, 2020

Not in near future on my side.
Personally i would be in favour of first integrating the ongoing OpenAMP PRs:

@arnopo this is now done

And then, in a second step, find a comment way to ease the OpenAMP initialization relying on DT.

@arnopo are you working on this? CC @ioannisg

@ioannisg : Unfortunately, I haven't found the bandwidth to work on it yet. And I probably won't have the bandwidth for a while.
i started some stuff when creating the I2C over RPmsg driver (https://github.com/arnopo/zephyr_stm32mp1/commits/I2C_demo)
but i did not found time to create the associated OpenAMP manager.
If you are interested in implementing the service don't hesitate. At minimum i can provide some support and review...

@carlescufi
Copy link
Member

@galak this is coming up as a requirement, is there plans on your side to address this?

@nandojve
Copy link
Member

Hi! This thing start to hit my door too. Is there any update/roadmap?

@carlescufi
Copy link
Member

Hi! This thing start to hit my door too. Is there any update/roadmap?

@nandojve See #31115

@carlescufi
Copy link
Member

This is now complete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: IPC Inter-Process Communication Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

No branches or pull requests

6 participants