Re: [PATCH v2 10/17] drm/msm/a6xx: Update HFI definitions

[Konrad Dybcio]

On 3/30/26 10:27 PM, Akhil P Oommen wrote:
> On 3/30/2026 4:45 PM, Konrad Dybcio wrote:
>> On 3/27/26 1:13 AM, Akhil P Oommen wrote:
>>> Update the HFI definitions to support additional GMU based power
>>> features.
>>>
>>> Signed-off-by: Akhil P Oommen <akhilpo@xxxxxxxxxxxxxxxx>
>>> ---
>>
>> Whether you want to proceed with bitfields or not:
>>
>> Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
> 
> Thanks. I still feel it is bitfield layout is a 'bit' complicated.
> 
> I did an experiment:
> 
> #include <stdio.h>
> #include <stdint.h>
> #include <stddef.h>
> 
> struct packed_u8 {
>         uint16_t mitigation_time;
>         uint8_t current_rating    : 6;
>         uint8_t num_phases        : 6;
>         uint8_t path_notification : 1;
>         uint8_t extra_features    : 3;
> } __packed;
> 
> 
> void main() {
> 
>         struct packed_u8 data = {
>                  .mitigation_time  = 0xffff,
>                  .current_rating   = 0x3f,  /* all 6 bits set */
>                  .num_phases       = 0x3f,
>                  .path_notification = 1,
>                  .extra_features   = 0x7,
>          };
> 
>         printf("Akhil 0x%x\n", *((uint32_t *) &data));
> }
> 
> The output I got in Kaanapali is: Akhil 0x7f3fffff
> 
> This means that the compiler inserted a padding between current_rating
> and num_phases.

That's because __packed doesn't work outside the kernel - you're just
creating a variable named __packed, so this is effectively the same as:

struct foo {
        int a;
        int b;
        unsigned char c : 2;
} __hotdog;

int main () {
        printf("Akhil 0x%x\n", *((uint32_t *) &__hotdog));
}

Outside the kernel tree, you need to use the full annoying __attribute__((foo))
syntax:

include/linux/compiler_attributes.h:#define __packed                        __attribute__((__packed__))

with that changed, we get:

Akhil 0xffffffff

which is the expected behavior

Konrad