rpmsg: Make RPMSG_BUFFER_SIZE a fixed define#319
rpmsg: Make RPMSG_BUFFER_SIZE a fixed define#319supergaute wants to merge 1 commit intoOpenAMP:mainfrom
Conversation
Since the rpmsg_virtio.h file is exported, it was possible to have different values for RPMSG_BUFFER_SIZE when open-amp is built and when the library is included in an application. By making this parameter a CMake option and hard-coding the value in rpmsg_virtio.h, it will make sure this parameter cannot differ between building of open-amp and the user application. Signed-off-by: Gaute Nilsson <gaute.nilsson@siemens-energy.com>
| #ifndef RPMSG_BUFFER_SIZE | ||
| #define RPMSG_BUFFER_SIZE (512) | ||
| #endif | ||
| #define RPMSG_BUFFER_SIZE @RPMSG_BUFFER_SIZE@ |
There was a problem hiding this comment.
Today the open-amp can be used without the cmakeand a library build, but with files directly build in the application project.
This break the legacy , imposing the cmake usage (only optional version_def.h is handled by cmake).
And this would not guarantee that include files would be aligned with a dynamic library.
On the other side if the application projetc define the RPMSG_BUFFER_SIZE and open-amp library build as a sub project could this solve the issue?
I also wonder if a new APi to get the rpmsg _size such as the introduce in Linux kernel https://elixir.bootlin.com/linux/v5.16-rc2/source/include/linux/rpmsg.h#L189) would be more reliable.
The drawback would be for static allocation.
There was a problem hiding this comment.
I think making things more dependent on cmake is a possible problem. Also, how many people ever build with RPMSG_BUFFER_SIZE anything but 512?
There was a problem hiding this comment.
Well, I can only speak for myself, but I use OpenAMP with FreeRTOS, build with CMake and RPMSG_BUFFER_SIZE set to 2048.
What is so special about using RPMSG_BUFFER_SIZE to anything other than 512?
There was a problem hiding this comment.
From MPOV, it is also a valid use case to decrease the buffer size for MCU to MCU communications, to limit the shared memory.
I think we also have to think about evolution such as #155 pull request is also a possible evolution that would make the size negociated.
That's why a new API could be nice to get the buffer effective size.
But as mentionned before this is not compatible with a static allocation. that said, that could allow to check the alignement between the library and the application...
There was a problem hiding this comment.
Well, I can only speak for myself, but I use OpenAMP with FreeRTOS, build with CMake and RPMSG_BUFFER_SIZE set to 2048.
What is so special about using RPMSG_BUFFER_SIZE to anything other than 512?
The only thing special about 512 is that Linux remoteproc is hard-coded to 512. I hadn't thought of the MCU->MCU case or other such configurations.
There was a problem hiding this comment.
Could you confirm that #328 answer to your need and that we can close this one?
There was a problem hiding this comment.
Without any answer, I consider that the #328 answer to your need, please reopen this pull request if it is not the case
4 participants
Since the rpmsg_virtio.h file is exported, it was possible to have
different values for RPMSG_BUFFER_SIZE when open-amp is built and
when the library is included in an application.
By making this parameter a CMake option and hard-coding the value
in rpmsg_virtio.h, it will make sure this parameter cannot differ
between building of open-amp and the user application.
Signed-off-by: Gaute Nilsson gaute.nilsson@siemens-energy.com