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

Changes B for Issue 38 #42

Open
wants to merge 19 commits into
base: main-next
Choose a base branch
from
Open

Changes B for Issue 38 #42

wants to merge 19 commits into from

Conversation

sipke
Copy link

@sipke sipke commented Mar 24, 2025

Addresses additional items for issue 38

This should merge after pull request #41 as it is on top of those changes

  • move "If you want to contribute and port OpenAMP to your platform read more about OpenAMP porting here." to contributing page
  • add link to virtio spec in openamp-governance-and-guidelines
  • to rewrite this chapter ( remove standardization notions).
  • Avoid duplication with https://www.openampproject.org/governance/ in this pages. to determine if a part of the gouvernance should be moved ghere
  • no roadmap defined, to remove chapter
  • no platform maintainer defined, to remove chapter and/or rewrite according to gouvernance page
  • "History" to move to OpenAMp project part?
  • "input ring buffer" and "free ring buffer" is not really current terminology in Virtio lingo. Maybe just keep the standard used and available terminology.
  • It's not clear from the title of this image that it refers to the available ring buffer.
  • The fastest fix is to remove the last phrase which mentions the "referred to as a hypervisorless-virtio" statement.
  • Following short-forms should be explained somewhere - expand on openAMP specific acronyms
  • Add glossary
  • openamp examples are deprecated so move reference to reference repository
  • NXP i.MX8M Plus is also supported.

Sipke Vriend added 11 commits March 6, 2025 09:35
Refer issue 38: OpenAMP#38
Remove "once loaded and started using Remoteproc."
Refer issue 38: OpenAMP#38
Remove or set optional resource table in demo diagrams.
replace remoteproc by remoteproc virtio in message diagrams.
Change diagrom to use Main processor terminology for main/host computer.
Refer issue 38: OpenAMP#38
change reference to code from opena-amp repository to
openamp-system repository
Refer issue 38: OpenAMP#38
Proxy and RPC chapter should be renamed Rpmsg service.

Did not remove Proxy yet, as still "to discuss if we keep the proxy
service that is not generic."
to avoid confusion between host and guest, move architecture and demo
pages to use main controller and remote rather than host and remote.
Change "They allow the users" to user to correct grammar.
review request to use matrices spelling
address review from Arnaud
"The diagram is not accurate for the ./rpmsg_export_dev usage. Between
rpmsg_ctrl and app_rpmsg_tty, there is a Name Service (NS) RPmsg from the
main processor to the remote processor(inverted on diagram).
In other words, rpmsg_export_dev allows creating an rpmsg TTY (or raw)
device instance on the main processor. In such a case, the main processor
sends an NS rpmsg message to the remote processor to inform it of the
creation.

Another suggestion: To clarify the diagram, it would be helpful to add
boxes or notes to separate and explain the subsequences. This would help
readers understand the relation between the demos and the diagram."
Refer issue 38: OpenAMP#38
Address review comment:
move "If you want to contribute and port OpenAMP to your platform
read more about OpenAMP porting here." to contributing page
Refer issue 38: OpenAMP#38
Address review comment:
add link to the virtio spec
Refer issue 38: OpenAMP#38
Address review comment:
 to rewrite this chapter ( remove standardization notions).
Sipke Vriend added 7 commits March 26, 2025 20:30
Refer issue 38: OpenAMP#38
Address review comment:
 "History" to move to OpenAMp project part?
Refer issue 38: OpenAMP#38
Address review comments:
* "input ring buffer" and "free ring buffer" is not really current
terminology in Virtio lingo. Maybe just keep the standard used and
available terminology.
* It's not clear from the title of this image that it refers to the
available ring buffer.
Refer issue 38: OpenAMP#38
Address review comment:
* The fastest fix is to remove the last phrase which mentions the "referred to as a
hypervisorless-virtio" statement.
Expand on the OpenAMP specific acronyms on first usage, so they make
sense later in bullet points

Address Review comment:
* Following short-forms should be explained somewhere:
Add a link to glossary and start filling out a table of terms.
openamp examples are deprecated so move reference.
Refer issue 38: OpenAMP#38
Address review comment:
NXP i.MX8M Plus is also supported.
Copy link
Collaborator

@edmooring edmooring left a comment

Choose a reason for hiding this comment

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

Looks good to go.

review change
rather than link to specific version of virtio, link to upper level
listing of all versions.
there isn't a latest version it seems, so let reader decide where to go.
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