Skip to content
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

Support building zephyr with upstream open-amp and libmetal #25

Merged
merged 2 commits into from
Oct 24, 2023

Conversation

wmamills
Copy link
Collaborator

@wmamills wmamills commented Oct 17, 2023

Prototype stage only.

TODO:
DONE: modify to work on Windows also
DONE: Get rid of wam remote stuff
WIP will be a different PR: Add CI github action workflow

@wmamills wmamills requested review from uLipe and arnopo October 17, 2023 19:31
@wmamills wmamills marked this pull request as draft October 17, 2023 19:31
@wmamills
Copy link
Collaborator Author

wmamills commented Oct 17, 2023

I have updated the *-module branches with a new commit that removes the use of the symlink.
That is hard to show here in the PR but here is a pointer:
https://github.com/wmamills/openamp-system-reference/commits/open-amp-module

Please note the commented lines to find the libmetal headers and libs in the open-amp-module branch above.
I believe that is redundant with what Zephyr cmake already does for every module.
It works when I did not update them to the new paths and it works when I commented them out.

Copy link
Collaborator

@arnopo arnopo left a comment

Choose a reason for hiding this comment

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

I tested with success, I also proposed an alternative to squash the libmetal-module and open-amp-module in one

west.yml Outdated Show resolved Hide resolved
@uLipe
Copy link
Collaborator

uLipe commented Oct 19, 2023

Just adding to the conversation, I also tested with no effort, it worked very well.

west.yml Outdated
@@ -7,6 +7,8 @@ manifest:
url-base: https://github.com/OpenAMP
- name: zephyr
url-base: https://github.com/zyphyrproject-rtos
- name: wam
url-base: https://github.com/wmamills
Copy link
Collaborator

@arnopo arnopo Oct 19, 2023

Choose a reason for hiding this comment

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

could you update to rely on https://github.com/OpenAMP/openamp-zephyr-modules?
I know that there is a way to specify the revision as a PR reference, waiting merge of OpenAMP/openamp-zephyr-modules#1, but i can not remeber the exact syntax for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I got it. But seems better to address is a separate PR

    - name: openamp-zephyr-modules
      remote: openamp-zephyr-modules
      repo-path: openamp-system-reference
      revision: pull/1/head

wmamills and others added 2 commits October 23, 2023 19:56
Use Zephyr from upstream when we don't have patches as it looks better

Update to the v3.5.0 release as kv260_r5 needs it and
we should pre-test with it before Zephyr updates to our
2023.10.* release.

Signed-off-by: Bill Mills <bill.mills@linaro.org>
This manifest allows us to use our repos for the open-amp and libmetal
libraries.  As our libraries do not have the zephyr dir, they are not
seem as zephyr modules.

To make this work easily with zephyr we also add a zephyr glue layer.
This layer _is_ seen as a zephyr module and integrates both libraries.

Several approaches were tried for this and this was the chosen method for
the following reasons:

Pros:
* All needed files are provided by just doing the west update
* We don't need to set any ENV vars each time like EXTRA_ZEPHYR_MODULES
* It works with our samples and zephyr stock samples
* The version of open-amp and libmetal are directly controlled in
  the manifest
* It is low maintenance as the module boiler plate is unlikely to change
  much.

Cons:
* One extra repo

Signed-off-by: Bill Mills <bill.mills@linaro.org>
Co-authored-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
@wmamills
Copy link
Collaborator Author

This is now clean and ready. This is no longer an RFC.
@arnopo please take a look

@wmamills wmamills marked this pull request as ready for review October 24, 2023 00:03
@wmamills wmamills changed the title RFC: Support building zephyr with upstream open-amp and libmetal Support building zephyr with upstream open-amp and libmetal Oct 24, 2023
@wmamills
Copy link
Collaborator Author

@uLipe any comment?

@uLipe
Copy link
Collaborator

uLipe commented Oct 24, 2023

@wmamill, no, still LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants