Attention is currently required from: Nicholas Chin, Nicholas Sudsgaard.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80577?usp=email )
Change subject: include/device/azalia.h: Add enum for misc field ......................................................................
Patch Set 1:
(1 comment)
File src/include/device/azalia.h:
https://review.coreboot.org/c/coreboot/+/80577/comment/fcb5621a_8cbe2164 : PS1, Line 110: enum AzaliaPinCfgMisc misc:4; I just saw this patch go by and I don't know what this driver is about, but just wanted to mention that I'd generally caution about using enumerated types in bitfields like this. Maybe that's just superstition but I think the general folk wisdom in C is to never mix types within a bitfield because the details of how different compilers allocate that can be unpredictable. (There's also not much benefit of using the enum types here since you don't get any extra type checking from that.)
I would recommend writing a type like this this way: ``` union AzaliaPinConfiguration { uint32_t value; struct { uint32_t port:2; uint32_t location:6; uint32_t device:4; uint32_t connection:4; uint32_t color:4; uint32_t misc:4; uint32_t association:4; uint32_t sequence:4; }; } ``` Using the same fixed-width type for every field should make sure the compiler allocates all fields within that single 4-byte region. You shouldn't need any explicit alignment or packing attributes, since the alignment defaults to the alignment of the uint32_t type and there's no reason for the compiler to insert padding between bitfields of the same type. (In cases where not all bits of the full 4-byte space are used, it would also be best practice to insert an explicit "padding" field at the end for the remaining bits so that the compiler doesn't need to make any padding decisions by itself.)