Update data structures doxygen code to be consistent and more easily readable

[tammyleino]

The data structures are not documented consistently, and the way they are currently documented, the member description does not show up in the documentation. This proposal will follow the convention set forth by Zephyr.

Example:

before:

/**
 * struct rpmsg_endpoint - binds a local rpmsg address to its user
 * @name: name of the service supported
 * @rdev: pointer to the rpmsg device
 * @addr: local address of the endpoint
 * @dest_addr: address of the default remote endpoint binded.
 * @cb: user rx callback, return value of this callback is reserved
 *      for future use, for now, only allow RPMSG_SUCCESS as return value.
 * @ns_unbind_cb: end point service unbind callback, called when remote
 *                ept is destroyed.
 * @node: end point node.
 * @priv: private data for the driver's use
 *
 * In essence, an rpmsg endpoint represents a listener on the rpmsg bus, as
 * it binds an rpmsg address with an rx callback handler.
 */
struct rpmsg_endpoint {
    char name[RPMSG_NAME_SIZE];
    struct rpmsg_device *rdev;
    uint32_t addr;
    uint32_t dest_addr;
    rpmsg_ept_cb cb;
    rpmsg_ns_unbind_cb ns_unbind_cb;
    struct metal_list node;
    void *priv;
};

after:

 /**
  * @brief structure that binds a local rpmsg address to its user
  *
  * In essence, an rpmsg endpoint represents a listener on the rpmsg bus, as
  * it binds an rpmsg address with an rx callback handler.
  */
 struct rpmsg_endpoint {
];      /** Name of the service  supported */
    char name[RPMSG_NAME_SIZE

    /** Pointer to the rpmsg device */
    struct rpmsg_device *rdev;        

    /** Local address of the endpoint */
    uint32_t addr;                    

    /** Address of the default remote endpoint binded */
    uint32_t dest_addr;               

        /** User rx callback, return value of this callback is reserved
     *  for future use, for now, only allow RPMSG_SUCCESS
     * as return value
     */
    rpmsg_ept_cb cb;

    /** Endpoint service unbind
     *   callback, called when remote ept is destroyed
     */
    rpmsg_ns_unbind_cb ns_unbind_cb;  

    /** Endpoint node */
    struct metal_list node;

    /** Private data for the driver's use */
    void *priv;                       
 };

Secondly, update Doxyfile to remove line number / file from description of data structures members

[tammyleino]

Which data structures do we want to consider public? Here is the current list that gets published in the documentation. Not all members are documented, and I am going to be hard-pressed to flesh it all out before release.

No documentation of members: Elf32_Ehdr elf32_info Elf32_Phdr Elf32_Rel   Elf32_Rela   Elf32_Shdr   Elf32_Sym   Elf64_Ehdr   elf64_info   Elf64_Phdr   Elf64_Rel   Elf64_Rela   Elf64_Shdr   Elf64_Sym   rpmsg_rpc_data   rpmsg_rpc_syscall   rpmsg_rpc_syscall_header
vring_used_elem

Fully documented: fw_rsc_carveout   fw_rsc_devmem   fw_rsc_hdr   fw_rsc_trace   fw_rsc_vdev   fw_rsc_vdev_vring   fw_rsc_vendor   image_store_ops   loader_ops   remoteproc   remoteproc_mem   remoteproc_ops   remoteproc_virtio   resource_table   rpmsg_device   rpmsg_device_ops   rpmsg_endpoint   rpmsg_hdr   rpmsg_ns_msg   rpmsg_rpc_answer   rpmsg_rpc_client_services   rpmsg_rpc_clt   rpmsg_rpc_services   rpmsg_rpc_svr    rpmsg_virtio_config   rpmsg_virtio_device   rpmsg_virtio_shm_pool   virtio_device   virtio_vring_info   vring_desc   vring_avail   vring_used   

@wmamills @arnopo @edmooring

[tammyleino]

First PR opened for already documented data structures here https://github.com/OpenAMP/open-amp/pull/507

[arnopo]

Which data structures do we want to consider public? Here is the current list that gets published in the documentation. Not all members are documented, and I am going to be hard-pressed to flesh it all out before release.

No documentation of members: Elf32_Ehdr elf32_info Elf32_Phdr Elf32_Rel   Elf32_Rela   Elf32_Shdr   Elf32_Sym   Elf64_Ehdr   elf64_info   Elf64_Phdr   Elf64_Rel   Elf64_Rela   Elf64_Shdr   Elf64_Sym   rpmsg_rpc_data   rpmsg_rpc_request   rpmsg_rpc_syscall   rpmsg_rpc_syscall_header virtio_device_id   virtio_dispatch   virtio_feature_desc   virtqueue   virtqueue_buf   vq_desc_extra   vring   vring_alloc_info   vring_avail   vring_used   vring_used_elem

From my point of view; would be nice to comment

• vring,   vring_alloc_info,   vring_avail,   vring_used,   vring_used_elem =>Descriptions are available in https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html

• virtio_device_id,   virtio_dispatch,   virtio_feature_desc,   virtqueue,   virtqueue_buf,   vq_desc_extra => Probably more complex to comment, If you don't know how to comment it, I will try to do it for the release.

  For the ELF* structure, no strong need

[tammyleino]

Which data structures do we want to consider public? Here is the current list that gets published in the documentation. Not all members are documented, and I am going to be hard-pressed to flesh it all out before release. No documentation of members: Elf32_Ehdr elf32_info Elf32_Phdr Elf32_Rel Elf32_Rela Elf32_Shdr Elf32_Sym Elf64_Ehdr elf64_info Elf64_Phdr Elf64_Rel Elf64_Rela Elf64_Shdr Elf64_Sym rpmsg_rpc_data rpmsg_rpc_request rpmsg_rpc_syscall rpmsg_rpc_syscall_header virtio_device_id virtio_dispatch virtio_feature_desc virtqueue virtqueue_buf vq_desc_extra vring vring_alloc_info vring_avail vring_used vring_used_elem

From my point of view; would be nice to comment

* vring,   vring_alloc_info,   vring_avail,   vring_used,   vring_used_elem
  =>Descriptions are available in https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html

* virtio_device_id,   virtio_dispatch,   virtio_feature_desc,   virtqueue,   virtqueue_buf,   vq_desc_extra
  => Probably more complex to comment, If you don't know how to comment it, I will try to do it for the release.

For the ELF* structure, no strong need

I was able to find info for vring_avail, vring_used and vring from the reference above, but cannot find the others. If we could please merge my PR and then you could open a new one with additional comments, that would be ideal for me.

Should I mark the others private so they don't show up in the doc?

[arnopo]

I was able to find info for vring_avail, vring_used and vring from the reference above, but cannot find the others. If we could please merge my PR and then you could open a new one with additional comments, that would be ideal for me.

That sound good to me

Should I mark the others private so they don't show up in the doc?

I prefer to keep them in the doc even if uncommented. Could you list in this issue the structures that are not yet commented?

[tammyleino]

@arnopo I updated the list above to show the currently undocumented and documented data structures. We can leave this ticket open until all data structures are documented. It would be nice to at least have a description for each one, even if the members are not documented.

[arnopo]

Following structures are commented in https://github.com/OpenAMP/open-amp/pull/510:

• virtio_device_id
• virtio_feature_desc
• virtio_dispatch
• virtqueue_buf
• virtqueue
• vring_alloc_info

[github-actions[bot]]

This issue has been marked as a stale issue because it has been open (more than) 45 days with no activity.

[arnopo]

Following page give a up to date status on documented/undocumented structures: https://openamp.readthedocs.io/en/latest/doxygen/openamp/annotated.html

Seems not useful to document ELFxx structures. Perhaps we could remove the elf_loader.h from the API list as it is more dedicated to an internal usage. @tnmysh @edmooring : any opinion?

Remaining structure to document:

 virtio_ident
 rpmsg_rpc_data  
 rpmsg_rpc_syscall  
 rpmsg_rpc_syscall_header
 vring_used_elem

[tnmysh]

Seems not useful to document ELFxx structures. Perhaps we could remove the elf_loader.h from the API list as it is more dedicated to an internal usage. @tnmysh @edmooring : any opinion?

Correct, elf loading is more internal usage of remoteproc framework. @arnopo , To me it's okay to remove elfxx structures and elf_loader.h.

[github-actions[bot]]

This issue has been marked as a stale issue because it has been open (more than) 45 days with no activity.