Skip to content

Conversation

@arnopo
Copy link
Contributor

@arnopo arnopo commented Jun 21, 2019

This pull request proposes an implementation of the RPMSg protocol based on a resource table.
The resource table can be use to declare resources shared between the main and coprocessor. It is integrated in the co-processor firmware in a specific section. This section can be recognized and parsed by the Linux OS kernel to initialize shared resources such as trace and RPMsg protocol.

Based on this resource table, the objective is to propose a sample that is platform agnostic and that answers to the Linux rpmsg client sample integrated in the Linux kernel distribution.

For time being this pull request only implements the co-processor side, and could be extended in future to also implement the master part.

@zephyrbot

This comment has been minimized.

@arnopo
Copy link
Contributor Author

arnopo commented Jun 24, 2019

@galak Please, could you take a look to this pull request and associated OpenAMP pull request: zephyrproject-rtos/open-amp#2.
I'm not sure that adding the resource table directly in open-amp module is the right solution, so if you have better suggestion, don't hesitate!

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

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

doc changes LTGM, thanks!

@arnopo arnopo added the area: IPC Inter-Process Communication label Jul 3, 2019
Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

First batch of comments

Copy link
Member

Choose a reason for hiding this comment

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

Expect conflicts with #17660

Copy link
Member

Choose a reason for hiding this comment

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

#17660 is now merged, you'll have to report this patch in scripts/dts/gen_defines.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@erwango erwango self-requested a review July 29, 2019 15:38
@arnopo arnopo requested a review from ulfalizer as a code owner July 30, 2019 09:47
Copy link
Contributor

@ulfalizer ulfalizer left a comment

Choose a reason for hiding this comment

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

Review comments fixed

@jhedberg
Copy link
Member

jhedberg commented Jan 7, 2020

@arnopo could you please rebase this and fix the conflicts? Is this otherwise ready to be merged?

@arnopo
Copy link
Contributor Author

arnopo commented Jan 7, 2020

@jhedberg
yes i will rebase it,
for me it is then ready for merge but would be better first that @galak approve it.

@arnopo
Copy link
Contributor Author

arnopo commented Feb 6, 2020

@galak: gentle reminder. This PR still waits your review :)

@jhedberg
Copy link
Member

This seems to have been neglected for awhile. I just triggered a Shippable re-run. @galak could you take a look?

Copy link
Contributor

@galak galak left a comment

Choose a reason for hiding this comment

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

In general looks good, a few updates based on changes I made to PR #17553 that we should reflect here.

Copy link
Contributor

@galak galak left a comment

Choose a reason for hiding this comment

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

Do you see having more things in lib/open-amp/. Wonder if we need the rsc_table dir, or if we can flatten things a little?

@arnopo
Copy link
Contributor Author

arnopo commented Mar 26, 2020

Thanks @galak for the review, i will push a new version ASAP ( likely tomorrow)

@arnopo
Copy link
Contributor Author

arnopo commented Mar 26, 2020

Do you see having more things in lib/open-amp/. Wonder if we need the rsc_table dir, or if we can flatten things a little?

No more things it pipe for the moment, ok i will suppress the rsc_dtable dir

@arnopo
Copy link
Contributor Author

arnopo commented Mar 27, 2020

Do you see having more things in lib/open-amp/. Wonder if we need the rsc_table dir, or if we can flatten things a little?

No more things it pipe for the moment, ok i will suppress the rsc_dtable dir

rsc_table dir has been suppressed

Copy link
Contributor

@galak galak left a comment

Choose a reason for hiding this comment

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

In general looks good, some minor updates. Should we get PR #14750 merged first, after which I think this should be good.

@arnopo
Copy link
Contributor Author

arnopo commented Mar 27, 2020

Shippable issue seems not linked to this PR:

INFO    - 435/782 mps2_an385                tests/net/socket/select/net.socket.select          FAILED Failed (qemu 5.352s)
INFO    - /home/buildslave/src/github.com/zephyrproject-rtos/zephyr/sanity-out/mps2_an385/tests/net/socket/select/net.socket.select/handler.log
ERROR   - *** Booting Zephyr OS build zephyr-v2.2.0-812-g60dd11610e6d  ***
Running test suite socket_select
===================================================================
starting test - test_fd_set
PASS - test_fd_set
===================================================================
starting test - test_select

    Assertion failed at WEST_TOPDIR/zephyr/tests/net/socket/select/src/main.c:115: test_select: (tstamp >= 30U && tstamp <= 30 + FUZZ is false)

@arnopo
Copy link
Contributor Author

arnopo commented Mar 31, 2020

Shippable issue seems not linked to this PR:

INFO    - 435/782 mps2_an385                tests/net/socket/select/net.socket.select          FAILED Failed (qemu 5.352s)
INFO    - /home/buildslave/src/github.com/zephyrproject-rtos/zephyr/sanity-out/mps2_an385/tests/net/socket/select/net.socket.select/handler.log
ERROR   - *** Booting Zephyr OS build zephyr-v2.2.0-812-g60dd11610e6d  ***
Running test suite socket_select
===================================================================
starting test - test_fd_set
PASS - test_fd_set
===================================================================
starting test - test_select

    Assertion failed at WEST_TOPDIR/zephyr/tests/net/socket/select/src/main.c:115: test_select: (tstamp >= 30U && tstamp <= 30 + FUZZ is false)

outdated as solve by a rebase

arnopo added 4 commits April 1, 2020 09:45
The resource table is needed by the Linux kernel OS
for a rpmsg generic support, but is also recognised by OpenAMP.
This table allows to add trace based on the RAM console
and to support rpmsg protocol.

Signed-off-by: Arnaud Pouliquen <[email protected]>
Rebase the resource table management to
the new implementation in open-amp module

Signed-off-by: Arnaud Pouliquen <[email protected]>
64 kB of memory is reserved for the inter-processor
communication. this makes sense only if RPMsg is used.
Allow to use this memory for firmware data by default.

Signed-off-by: Arnaud Pouliquen <[email protected]>
This sample is designed to respond to the Linux
rpmsg sample client.
It should be platform independent and based on the
the integration of a resource table in the elf file.

Signed-off-by: Arnaud Pouliquen <[email protected]>
@arnopo
Copy link
Contributor Author

arnopo commented Apr 1, 2020

@galak,
i tested your updates and rebase to force the re-run of some stalled checks,

@galak galak merged commit f6800aa into zephyrproject-rtos:master Apr 1, 2020
@arnopo arnopo deleted the OpenAMP_rsc_table_sample branch April 4, 2025 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants