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

[Werner Sembach]

Am 25.11.25 um 01:50 schrieb 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.
Hope it's ok that in my rfc the copy in driver data is no longer read only, but 
is outside of the probe only accessed via a getter method.
> > 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.
Every instance in my RFC has now it's own copy of the feature bitmap.
> 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.
Yes, I did that.
> > 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.
sorry I'm stupid xD
> > 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.

I sent a RFC based on your PoC patch, I would suggest moving the discussion 
thread there.

Thanks so far for your PoC/WIP patch

Best regards

Werner

> Thanks,
> Armin Wolf
>
> > Best regards,
> >
> > Werner
> >
> > > [snip]