Skip to content

Improve the porting guide page - #86

Open
sipke wants to merge 6 commits into
OpenAMP:mainfrom
sipke:feature/improve-the-porting-guide-page
Open

Improve the porting guide page#86
sipke wants to merge 6 commits into
OpenAMP:mainfrom
sipke:feature/improve-the-porting-guide-page

Conversation

@sipke

@sipke sipke commented May 1, 2026

Copy link
Copy Markdown

Changes to address #85

Added high level overview attempting to address suggestions in the issue to get feedback on direction.
Also added some questions to the issue to get more input for future changes to this PR.

@sipke
sipke force-pushed the feature/improve-the-porting-guide-page branch from d959cdc to 3dcae1d Compare May 11, 2026 04:58
having duplicated embedded doxygen content results in the warning
WARNING: Duplicate C++ declaration
As these APIs are already detailed in the remoteproc documentation make
a reference to them rather than repeat the embed.

Signed-off-by: Sipke Vriend <sipke@direktembedded.com>
@sipke
sipke force-pushed the feature/improve-the-porting-guide-page branch from 3dcae1d to 0778cc5 Compare June 1, 2026 07:19
@sipke
sipke marked this pull request as ready for review June 1, 2026 07:21
@sipke

sipke commented Jun 1, 2026

Copy link
Copy Markdown
Author

During review note also the comment for 47ae5ad which includes a snippet from source in openamp-system-reference. If we think this is a good direction for other snippets, it would be better to use the start and end tags as detailed in that comment, rather than line numbers.

@arnopo arnopo added this to the Release V2026.10 milestone Jun 2, 2026

@arnopo arnopo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi @sipke

Please find some comments. As mentioned in one of my comments, this pull request might need a few iterations and must be reviewed by other project members.

Comment thread docs/porting_guide.rst Outdated
Comment thread docs/porting_guide.rst Outdated
Comment thread docs/porting_guide.rst
Comment thread docs/porting_guide.rst Outdated
Comment thread docs/porting_guide.rst Outdated
Comment thread docs/porting_guide.rst Outdated
Comment thread docs/porting_guide.rst Outdated
Comment thread docs/porting_guide.rst Outdated
Comment thread docs/porting_guide.rst
Comment thread protocol_details/resource_tbl.rst Outdated
@sipke
sipke force-pushed the feature/improve-the-porting-guide-page branch 2 times, most recently from ad38d12 to 85ebc56 Compare June 29, 2026 05:35
Comment thread docs/porting_guide.rst Outdated
Comment thread docs/porting_guide.rst Outdated
Comment thread docs/porting_guide.rst Outdated
Comment thread docs/porting_guide.rst Outdated
- :ref:`Remoteproc<overview-remoteproc-work-label>` on the remote processor only,
with the main processor using an existing
:ref:`Remoteproc<overview-remoteproc-work-label>` implementation (e.g., Linux Remoteproc)
and no IPC.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One criticism of OpenAMP is that the remote processor framework is required on the remote device side. that why I would split remoteproc in 3 feature:

  • remote processor live cycle management ( implemented on main processor)
  • remoteproc virtio : the transport layer of the virtio protocol
  • the resource table

In fact, on the remote side, the full remote processor framework is not required. Only the remote processor virtio transport layer is required. the resource table is also only mandatory for communication with Linux.
In addition, if no inter-process communication (IPC) is implemented, the remote side does not need the remote processor framework.

see my proposal in comment https://github.com/OpenAMP/openamp-docs/pull/86/changes#r3340380433

Comment thread docs/porting_guide.rst Outdated
Comment thread docs/porting_guide.rst Outdated
Comment thread docs/porting_guide.rst Outdated
Comment thread docs/porting_guide.rst
@nathalie-ckc

Copy link
Copy Markdown
Collaborator

2026-07-15 System Reference call:
Although Arnaud has proposed a direction, open to others' ideas on direction too. Add your proposals here.

@sipke
sipke force-pushed the feature/improve-the-porting-guide-page branch from 85ebc56 to df96c36 Compare July 27, 2026 06:09
@nathalie-ckc

Copy link
Copy Markdown
Collaborator

2026-07-29 System Reference call:
@arnopo has given feedback from ST perspective. Do any of the other HW vendors have different view on what is important for porting guide? (@glneo , @tnmysh , @iuliana-prodan )

Comment thread docs/porting_guide.rst Outdated
Shared memory forms the :ref:`physical layer<rpmsg-layers-work-label>` for
:doc:`RPMsg <../docs/rpmsg_design>` protocol.
The specific memory type and layout are implementation dependent, but should be a dedicated
SRAM or DDR region accessible by both cores. Caching is enabled in OpenAMP with the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Whether or not the memory is caching memory is not chosen by OpenAMP, it is an intrinsic property of the memory as defined by the host operating system. The WITH_DCACHE flag only sets if OpenAMP should do cache maintenance operations or not.

Additionally, since the cacheability of a memory region is a property of the hardware/port we have agreed to move this flag to libmetal where the other hardware specific options are located. This whole last sentence can just be dropped.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Additionally, since the cacheability of a memory region is a property of the hardware/port we have agreed to move this flag to libmetal where the other hardware specific options are located. This whole last sentence can just be dropped.

I agree with the objective. However, since it has not been done yet, we need to document the current implementation in the porting guide.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sure, but this sentence is not matching what we have currently either. That flag doesn't set if caching is enabled or not, only if we perform cache ops or not, suggest:

"Cache maintenance operations are performed by OpenAMP if the WITH_DCACHE cmake flag is set. It should be enabled if your shared memory region is caching and the core you are building for is not cache coherent with the remote side."

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I agree with @gleno on improving the sentence with some changes.

%s/OpenAMP/open-amp/ We refer to OpenAMP as a whole project, and open-amp as a library. I would replace 'OpenAMP' with open-amp and insert link to the open-amp library repo.

Another question, should we also mention DMA operations can be used, and zero-copy is possible?

@arnopo I am not aware on the support of zero-copy in the open-amp library. If it is available, then it is worth mentioning.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Replaced sentence.
@tnmysh I think currently the main distinction in the documentation is "OpenAMP" for the libary and "OpenAMP Project" for the project. I will effect other pages too if it is to change.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@arnopo I am not aware on the support of zero-copy in the open-amp library. If it is available, then it is worth mentioning.

good point I have created an issue : #95

Comment thread docs/porting_guide.rst
Comment thread docs/porting_guide.rst
- the :ref:`RPMsg<overview-rpmsg-work-label>` buffer
- the :ref:`Virtio Rings<docs/data_structures_content:Shared virtqueue structure>`
- the :ref:`Resource Table<resource-table>`
(optional if the RPMsg host is not the Linux kernel)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is the resource table really optional if RPMsg host is not the linux?

I think if host and remote both is using open-amp library for RPMsg communication then we need resource table.

@arnopo should we remove this optional comment? Or we if you are aware of any such example, we should provide an example for such a case?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Comment thread docs/porting_guide.rst
in the driver role.

The
`load_fw <https://github.com/OpenAMP/openamp-system-reference/tree/main/examples/legacy_apps/examples/load_fw>`_

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I am not sure, if we should mention load_fw demo. It's not being verified for a very long time, and not sure if it is even working.

I vote to remove this line. @arnopo and others have any opinion on this?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I haven't tested this.
If we intend to keep it, we should test it for every release. Otherwise, I'd vote to remove it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I am quite puzzled by this. It has the advantage of providing an application example. If we decide not to document it, we should also remove it from the system reference repository.

Comment thread docs/porting_guide.rst Outdated
----------------------

Use of Remoteproc framework for the remote or device role is optional and when used is for
resource table management, dynamically or statically.

@tnmysh tnmysh Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should define what 'dynamically' and 'statically' means?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

adjusted

Comment thread docs/porting_guide.rst Outdated

. No newline at end of file
For an example of setting up Remoteproc and Virtio for a remote device refer to
`zynqmp platform_info.c <https://github.com/OpenAMP/openamp-system-reference/blob/main/examples/legacy_apps/machine/xlnx/zynqmp/platform_info.c>`_

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

%s/zynqmp platform_info.c/zynqmp_r5 platform_info.c

and Please use this link instead: https://github.com/OpenAMP/openamp-system-reference/blob/main/examples/legacy_apps/machine/xlnx/zynqmp_r5/platform_info.c

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

changed

Comment thread docs/porting_guide.rst Outdated
}

. No newline at end of file
For an example of setting up Remoteproc and Virtio for a remote device refer to

@tnmysh tnmysh Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

"Setting up remoteproc and virtio for a remote device'.

I think it's worth mentioning that to setup 'remoteproc' ops, you'd need platform driver: For example: Cortex-R5 remoteproc ops are defined here: https://github.com/OpenAMP/openamp-system-reference/blob/main/examples/legacy_apps/machine/xlnx/zynqmp_r5/zynqmp_r5_a53_rproc.c

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I am sorry, reference to zynqmp_r5_a53_rproc.c should go in the section: platform specific remoteproc driver.

Then in the section platform sepcific rpmsg we should rewrite the line as:

"Setting up remoteproc virtio for a remote device'.

The 'and' gives idea that we are talking about 'remoteproc' separately, but we are really talking about 'remoteproc virtio'.

Thanks.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

added example to platform specific remoteproc driver and
remove the and from remoteproc virtio

Comment thread openamp/glossary.rst
LCM, Life-Cycle Management
MMU, Memory Management Unit
MPU, Memory Protection Unit
RAM, Random Access Memory

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should add DMA too.

DMA - Direct Memory Access

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

added

Comment thread protocol_details/resource_tbl.rst Outdated
themselves, each of which has a 32 bit type. These in remote context will likely be memory carveouts
for locations of parts of the remote system and virtio device definitions.


Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

extra line remove?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

removed

add some hw acronymns that will be used in porting guide changes to the
glossary.

Signed-off-by: Sipke Vriend <sipke@direktembedded.com>
@sipke
sipke force-pushed the feature/improve-the-porting-guide-page branch from df96c36 to 04fdd7c Compare August 24, 2026 04:38
Comment thread docs/porting_guide.rst
in the driver role.

The
`load_fw <https://github.com/OpenAMP/openamp-system-reference/tree/main/examples/legacy_apps/examples/load_fw>`_

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I haven't tested this.
If we intend to keep it, we should test it for every release. Otherwise, I'd vote to remove it.

Comment thread docs/porting_guide.rst
`RPMsg Virtio <https://github.com/OpenAMP/open-amp/blob/main/lib/include/openamp/rpmsg_virtio.h>`_
these are aliased as
:openamp_doc_link:`RPMSG_HOST <RPMSG_HOST>` and :openamp_doc_link:`RPMSG_REMOTE <RPMSG_REMOTE>`
respectively.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Respectively binds VIRTIO_DEV_DEVICE to RPMSG_HOST and VIRTIO_DEV_DRIVER to RPMSG_REMOTE. That is backwards - rpmsg_virtio.h defines RPMSG_HOST as VIRTIO_DEV_DRIVER and RPMSG_REMOTE as VIRTIO_DEV_DEVICE.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yep, backwards, reversed it to correct.

Comment thread docs/porting_guide.rst Outdated
remoteproc_start(&rproc);
.. literalinclude:: ../openamp-system-reference/examples/legacy_apps/examples/load_fw/load_fw.c
:language: c
:lines: 21-57

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Instead of line numbers which may need adjusting each release, try:

.. literalinclude:: ../openamp-system-reference/examples/legacy_apps/examples/load_fw/load_fw.c
   :language: c
   :start-at: int load_exectuable_block
   :end-before: /* end of lifecycle demo */

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes, I mentioned in comment that if this method of including source is preferred over previous inline code snippet, it would be better to use start-at and end-before tags. I was thinking specific tags in the source comments, as it may also help show that certain code snippets are being used in documentation (pseudo link between the two), however your suggestion of using source lines also works. Less chance of it changing than line numbers, but more than specific tags.
47ae5ad

Changed to your suggestion using source, but might be worth considering specific tags for future such snippets? or is that what you meant also with the /* end of lifecycle demo */ as I could not find that in main source.

Comment thread docs/porting_guide.rst Outdated
===========================

When using Remoteproc, the
`Virtio transport layer <https://github.com/OpenAMP/open-amp/main/lib/remoteproc/remoteproc_virtio.c>`_

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
`Virtio transport layer <https://github.com/OpenAMP/open-amp/main/lib/remoteproc/remoteproc_virtio.c>`_
`Virtio transport layer <https://github.com/OpenAMP/open-amp/blob/main/lib/remoteproc/remoteproc_virtio.c>`_

Broken link, missing blob segment.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

thanks. updated.


.. doxygenstruct:: fw_rsc_vdev
:project: openamp_doc_embed

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It should also include fw_rsc_vdev_vring and struct resource_table.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

added

Comment thread docs/porting_guide.rst
The specific memory type and layout are implementation dependent, but should be a dedicated
SRAM or DDR region accessible by both cores.
Cache maintenance operations are performed by OpenAMP if the
`WITH_DCACHE <https://github.com/OpenAMP/open-amp/blob/main/cmake/options.cmake>`_ cmake flag

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

open-amp exposes finer-grained options: WITH_DCACHE_VRINGS, WITH_DCACHE_BUFFERS, WITH_DCACHE_RSC_TABLE.
On systems where only part of the shared region is cacheable, the granular flags matter for both correctness and cycle cost.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@iuliana-prodan : The use of these cmake options has been deprecated (by you) in v2023.10 😃 . The code has been removed for now

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You're right, @arnopo, it's here: OpenAMP commit.

I completely forgot about that :(

Comment thread docs/porting_guide.rst Outdated
SRAM or DDR region accessible by both cores.
Cache maintenance operations are performed by OpenAMP if the
`WITH_DCACHE <https://github.com/OpenAMP/open-amp/blob/main/cmake/options.cmake>`_ cmake flag
is set. It should be enabled if your shared memory region is caching and the core you are

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

s/ shared memory region is caching/ shared memory region is cacheable

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

changed

Comment thread docs/porting_guide.rst Outdated
- the :ref:`Resource Table<resource-table>`
(optional if the RPMsg host is not the Linux kernel)

- Optional, but recommended, mailboxes acting as a ring bell interrupt on processors to

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

s/ring bell interrupt/doorbell interrupt

Comment thread protocol_details/resource_tbl.rst Outdated
`remoteproc header <https://github.com/OpenAMP/open-amp/blob/main/lib/include/openamp/remoteproc.h>`_.
The resource table is effectively a list of resource definitions, with each entry detailed by the
corresponding :ref:`resource structure<resource-structure>`, and the Remoteproc framework
is responsible for configuring the resources need by the remote processor (shared memory, trace

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

s/the resources need by/needed by

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

changed

Comment thread docs/porting_guide.rst
}

. No newline at end of file
For an example of setting up Remoteproc Virtio for a remote device refer to

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Virtio vs VirtIO or Remoteproc vs remoteproc vs Remote Proc Design.
Pick one of each and enforce it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Adjusted to VirtIO and Remoteproc

Sipke Vriend added 4 commits September 14, 2026 12:30
different implementation will use varying combinations of hardware,
RPMsg and Remoteproc so provide an overview of these.

Signed-off-by: Sipke Vriend <sipke@direktembedded.com>
To further detail what a resource table contains, embed the doxygen
from the remoteproc.h structure definitions.

Signed-off-by: Sipke Vriend <sipke@direktembedded.com>
Use sphinx's literalinclude to add code snippets from examples rather
than having code in the documentation.
This helps in the documentation being updated when the code is and shows
the user where an example can be studied further.
This uses line numbers, which may need adjusting each release, but there
would be an option to change code to include start and end tags as follows
to avoid line updates.
e.g.
.. literalinclude:: example.py
   :start-after: "# start function_x"
   :end-before: "# end function_y"

Signed-off-by: Sipke Vriend <sipke@direktembedded.com>

# Conflicts:
#	docs/porting_guide.rst
Reword this section slightly and link to an example rather than include
a code snippet.

Signed-off-by: Sipke Vriend <sipke@direktembedded.com>
@sipke
sipke force-pushed the feature/improve-the-porting-guide-page branch 2 times, most recently from fb95208 to a369141 Compare September 14, 2026 04:30
@sipke

sipke commented Sep 14, 2026

Copy link
Copy Markdown
Author

I tried rebasing this against main but there were some warnings from the breathe extension, in the PR build output, claiming not to find functions, though structs seemed to be found.
e.g.
doxygenfunction: Cannot find function “metal_allocate_memory” in doxygen xml
so reverted back to the previous main commit this PR was using just to continue review.
Was thinking its related to the change in commit 3bd6260, but not sure why only functions, so maybe just an update in some python 3.14 sphinx libraries from db7463b.
If anyone already knows what needs changing, let me know. If not will investigate what changed to fail this and what needs to be corrected.

@arnopo

arnopo commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

I tried rebasing this against main but there were some warnings from the breathe extension, in the PR build output, claiming not to find functions, though structs seemed to be found. e.g. doxygenfunction: Cannot find function “metal_allocate_memory” in doxygen xml so reverted back to the previous main commit this PR was using just to continue review. Was thinking its related to the change in commit 3bd6260, but not sure why only functions, so maybe just an update in some python 3.14 sphinx libraries from db7463b. If anyone already knows what needs changing, let me know. If not will investigate what changed to fail this and what needs to be corrected.

I had a look at the builds on Read the Docs. Yes, the issue seems to be linked to one of these two commits. I have no idea, from my side, what change is needed.

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.

6 participants