Re: [PATCH 2/6] platform/x86/uniwill: Handle more WMI events required for TUXEDO devices

[Armin Wolf]

Am 24.11.25 um 19:40 schrieb Werner Sembach:

> Hi
>
> Am 20.11.25 um 01:53 schrieb Armin Wolf:
> > [snip]
> >
> > Anyway, i attached the patch with the device descriptor 
> > infrastructure. The callback called during probe cannot modify
> > the feature bitmap anymore, but i suggest that you simply set the 
> > limit for cTGP to zero. The code responsible for
> > initializing cTGP support can then check if the cTGP limit is zero 
> > and return early. 
>
> Looked into it: whats the reason for the "__ro_after_init" in "static 
> struct uniwill_device_descriptor device_descriptor __ro_after_init;"?
__ro_after_init tells the kernel to mark the variables annotated with it as read-only after module initialization has finished.
I used it to ensure that the static device configuration data cannot be (accidentally) modified after module initialization.

> The thing Ilpo wrote sounded like just .driver_data itself should be 
> read only, but as soon as it has an indirection, like here being 
> copied over to a static variable, read/write is ok.

No, the device descriptor needs to be treated as read-only because, theoretically, multiple instances of the uniwill-laptop driver might probe.
If one of those instances modifies the descriptor, the other instances might run into trouble.

I suggest that in order to dynamically modify the supported_features bitmap, you simply copy said variable from the descriptor
into uniwill_data and then use the probe() callback to modify it.

> Also: Why not just copy the device_descriptor over to uniwill_data 
> instead of a static variable?

uniwill_data is not available during module initialization, so we have store it separately.

> And do I get that correctly: All you can do during the init callback 
> is doing more sophisticated DMI matching?
Inside the callback provided dmi_system_id, you indeed can only do more sophisticated DMI matching. You should
use the probe() callback inside the descriptor to perform device-specific initialization that requires access
to the EC and/or uniwill_data.

Thanks,
Armin Wolf

> Best regards,
>
> Werner
>
> > [snip]