Skip to content

ASoC: SOF: ipc: replace "enum sof_comp_type" field with "uint32_t"#5148

Merged
bardliao merged 1 commit intothesofproject:topic/sof-devfrom
LaurentiuM1234:fix/ipc_struct_offsets
Aug 23, 2024
Merged

ASoC: SOF: ipc: replace "enum sof_comp_type" field with "uint32_t"#5148
bardliao merged 1 commit intothesofproject:topic/sof-devfrom
LaurentiuM1234:fix/ipc_struct_offsets

Conversation

@LaurentiuM1234
Copy link

@LaurentiuM1234 LaurentiuM1234 commented Aug 20, 2024

Normally, the type of enums is "unsigned int" or "int". GCC has the "-fshort-enums" option, which instructs the compiler to use the smallest data type that can hold all the values in the enum (i.e: char, short, int or their unsigned variants).

According to the GCC documentation, "-fshort-enums" may be default on some targets. This seems to be the case for SOF when built for a certain 32-bit ARM platform.

On Linux, this is not the case (tested with "aarch64-linux-gnu-gcc") which means enums such as "enum sof_comp_type" will end up having different sizes on Linux and SOF. Since "enum sof_comp_type" is used in IPC-related structures such as "struct sof_ipc_comp", this means the fields of the structures will end up being placed at different offsets. This, in turn, leads to SOF not being able to properly interpret data passed from Linux.

With this in mind, replace "enum sof_comp_type" from "struct sof_ipc_comp" with "uint32_t".

Fixes: thesofproject/sof#9378
SOF-side change: thesofproject/sof#9379

Normally, the type of enums is "unsigned int" or "int". GCC has
the "-fshort-enums" option, which instructs the compiler to
use the smallest data type that can hold all the values in
the enum (i.e: char, short, int or their unsigned variants).

According to the GCC documentation, "-fshort-enums" may be
default on some targets. This seems to be the case for SOF
when built for a certain 32-bit ARM platform.

On Linux, this is not the case (tested with "aarch64-linux-gnu-gcc")
which means enums such as "enum sof_comp_type" will end up having
different sizes on Linux and SOF. Since "enum sof_comp_type" is used in
IPC-related structures such as "struct sof_ipc_comp", this means
the fields of the structures will end up being placed at different
offsets. This, in turn, leads to SOF not being able to properly
interpret data passed from Linux.

With this in mind, replace "enum sof_comp_type" from
"struct sof_ipc_comp" with "uint32_t".

Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

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

if you are changing multiple enums, one might expect a symmetrical change as in thesofproject/sof@32e81a5

??

@LaurentiuM1234
Copy link
Author

if you are changing multiple enums, one might expect a symmetrical change as in thesofproject/sof@32e81a5

??

yep, that's what you'd normally expect, but while looking through the kernel it seemed like struct ipc4_pin_props was not present anywhere (git grep) (this is the only difference between the FW and Linux changes). Also, oddly enough, it doesn't even seem to be used anywhere in the FW either? (except for the structures that use it that, themselves, are not used anywhere AFAICT). See comments from thesofproject/sof#9379 on the struct ipc4_pin_props-related change. Feel free to pitch in.

@LaurentiuM1234 LaurentiuM1234 marked this pull request as ready for review August 22, 2024 10:30
@bardliao bardliao merged commit f2d0899 into thesofproject:topic/sof-dev Aug 23, 2024
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.

[BUG] Mismatch between Linux structure fields offsets and SOF when using arm-zephyr-eabi-gcc

4 participants