[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f6b96a90-938f-4277-a9ca-92d99285f3d9@tuxedocomputers.com>
Date: Tue, 25 Nov 2025 15:05:44 +0100
From: Werner Sembach <wse@...edocomputers.com>
To: Armin Wolf <W_Armin@....de>, hansg@...nel.org,
ilpo.jarvinen@...ux.intel.com
Cc: platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/6] platform/x86/uniwill: Handle more WMI events required
for TUXEDO devices
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]
>>
Powered by blists - more mailing lists