Platform: Library: Move mailbox_sw_reg_read64() from posix mailbox.h #9589
Conversation
Without this change testbench IPC4 build fails with copier with implicit definition of the function. The mailbox_sw_reg_read64() function should be there in the same platform library mailbox.h header as the 32 bit version of similar function. The header mailbox.h for all Intel platforms is organized similarly. The argument type is changed to size_t from uint32_t to follow other similar functions. Signed-off-by: Seppo Ingalsuo <[email protected]>
| static inline uint64_t mailbox_sw_reg_read64(uint32_t offset) | ||
| { |
There was a problem hiding this comment.
Can testbench include this header ? We really want to get rid of platform/include now with Zephyr.
There was a problem hiding this comment.
The failure is not from testbench source code but rest of SOF code built into testbench.
There was a problem hiding this comment.
@lgirdwood @singalsu This is not really Zephyr build here, but we have sof/posix implement our RTOS abstraction. I think you could drop "#include <platform/lib/mailbox.h>" from this file to simplify the implementation and have no-op implementations for all mailbox_sw_reg*() functions right here in posix/include/sof/lib/mailbox.h.
But there are many users (one can also built a host/library build of SOF using the Zephyr posix platform), so very easy to miss a case.
singalsu
requested review from
plbossart,
mmaka1,
lbetlej,
dbaluta and
kv2019i
as code owners
There was a problem hiding this comment.
I'm good with this. It's complicated we have multiple concept/facilities in SOF for host builds -- we have 1) "sof/posix" RTOS implementation (alternative to Zephyr and XTOS), 2) we have sof/src/platform/posix definition (used with all RTOS'es), 3) we have CONFIG_LIBRARY Kconfig, and finally 4) we have native_sim Zephyr builds (like scripts/fuzz.sh). Then we have multiple users, testbench, the SOF ALSA plugin, fuzzer that use one or more of these facilities depending on their specific neeeds.
In this case of copier.c, it requires a SOF mailbox abstraction from the platform layer, and current sof/posix does not implement this. So this patch seems to be the correct fix based on the abstractions we currently have in place. It's a good question whether we maintain too many options for host builds, or should we limit (and make the codebase easier to follow in this area).
| static inline uint64_t mailbox_sw_reg_read64(uint32_t offset) | ||
| { |
There was a problem hiding this comment.
@lgirdwood @singalsu This is not really Zephyr build here, but we have sof/posix implement our RTOS abstraction. I think you could drop "#include <platform/lib/mailbox.h>" from this file to simplify the implementation and have no-op implementations for all mailbox_sw_reg*() functions right here in posix/include/sof/lib/mailbox.h.
But there are many users (one can also built a host/library build of SOF using the Zephyr posix platform), so very easy to miss a case.
Without this change testbench IPC4 build fails with copier with implicit definition of the function. The mailbox_sw_reg_read64() function should be there in the same platform library mailbox.h header as the 32 bit version of similar function. The header mailbox.h for all Intel platforms is organized similarly.
The argument type is changed to size_t from uint32_t to follow other similar functions.