| Message ID | 20250309-awcc-uid-v2-1-5338c6380b2f@gmail.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | platform/x86: alienware-wmi-wmax: Extend support to many devices | expand |
Am 09.03.25 um 06:13 schrieb Kurt Borja: > Add a non-deprecated version of wmi_get_acpi_device_uid(). Hi, i can understand you motivation behind this patch, but this is IMHO the wrong approach. Nothing guarantees us that a given ACPI _UID value will reliably signal the presence of WMI methods now or in the future. Because of this i am against this patch. The correct way would be to use the BMOF data to check for missing WMI methods. I did some work on that which i will announce soon. Thanks, Armin Wolf > Signed-off-by: Kurt Borja <kuurtb@gmail.com> > --- > drivers/platform/x86/wmi.c | 20 +++++++++++++++++--- > include/linux/wmi.h | 2 ++ > 2 files changed, 19 insertions(+), 3 deletions(-) > > diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c > index e46453750d5f1475eb87342b1c5fd04fe20df335..39f379777ad08efd3cda7313f293522c68773dbe 100644 > --- a/drivers/platform/x86/wmi.c > +++ b/drivers/platform/x86/wmi.c > @@ -636,7 +636,6 @@ EXPORT_SYMBOL_GPL(wmi_has_guid); > */ > char *wmi_get_acpi_device_uid(const char *guid_string) > { > - struct wmi_block *wblock; > struct wmi_device *wdev; > char *uid; > > @@ -644,8 +643,7 @@ char *wmi_get_acpi_device_uid(const char *guid_string) > if (IS_ERR(wdev)) > return NULL; > > - wblock = container_of(wdev, struct wmi_block, dev); > - uid = acpi_device_uid(wblock->acpi_device); > + uid = wmidev_get_acpi_device_uid(wdev); > > wmi_device_put(wdev); > > @@ -653,6 +651,22 @@ char *wmi_get_acpi_device_uid(const char *guid_string) > } > EXPORT_SYMBOL_GPL(wmi_get_acpi_device_uid); > > +/** > + * wmidev_get_acpi_device_uid() - Get _UID name of a WMI device > + * @wdev: A wmi bus device from a driver > + * > + * Find the _UID of the ACPI device associated with this WMI device. > + * > + * Return: The ACPI _UID field or NULL if there is no _UID > + */ > +char *wmidev_get_acpi_device_uid(struct wmi_device *wdev) > +{ > + struct wmi_block *wblock = container_of(wdev, struct wmi_block, dev); > + > + return acpi_device_uid(wblock->acpi_device); > +} > +EXPORT_SYMBOL_GPL(wmidev_get_acpi_device_uid); > + > /* > * sysfs interface > */ > diff --git a/include/linux/wmi.h b/include/linux/wmi.h > index 10751c8e5e6a0ad3ac9ae317b6f8ecfb14c9a983..625c52ee125219aaa23cf946333af33ad118aafa 100644 > --- a/include/linux/wmi.h > +++ b/include/linux/wmi.h > @@ -46,6 +46,8 @@ extern union acpi_object *wmidev_block_query(struct wmi_device *wdev, > > acpi_status wmidev_block_set(struct wmi_device *wdev, u8 instance, const struct acpi_buffer *in); > > +char *wmidev_get_acpi_device_uid(struct wmi_device *wdev); > + > u8 wmidev_instance_count(struct wmi_device *wdev); > > /** >
On Tue Mar 11, 2025 at 2:21 PM -05, Armin Wolf wrote: > Am 09.03.25 um 06:13 schrieb Kurt Borja: > >> Add a non-deprecated version of wmi_get_acpi_device_uid(). > > Hi, > > i can understand you motivation behind this patch, but this is IMHO the wrong approach. > Nothing guarantees us that a given ACPI _UID value will reliably signal the presence of > WMI methods now or in the future. Because of this i am against this patch. Yes, in the end it is a leap of faith. I think that it's not very risky as AFAIK AWCC didn't even exist when devices used the legacy RGB interface. Also as Mario mentioned, only a handful of devices supported the legacy interface, compared to the large amount of systems with the AWCC one. But of course, there is always a possibility of regressions for some unknown edge case. > > The correct way would be to use the BMOF data to check for missing WMI methods. I did > some work on that which i will announce soon. Anyways, this is very cool and indeed the correct approach. Hopefully I'll be able to help a bit, as I spent some time studying Pali's decoder.
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index e46453750d5f1475eb87342b1c5fd04fe20df335..39f379777ad08efd3cda7313f293522c68773dbe 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -636,7 +636,6 @@ EXPORT_SYMBOL_GPL(wmi_has_guid); */ char *wmi_get_acpi_device_uid(const char *guid_string) { - struct wmi_block *wblock; struct wmi_device *wdev; char *uid; @@ -644,8 +643,7 @@ char *wmi_get_acpi_device_uid(const char *guid_string) if (IS_ERR(wdev)) return NULL; - wblock = container_of(wdev, struct wmi_block, dev); - uid = acpi_device_uid(wblock->acpi_device); + uid = wmidev_get_acpi_device_uid(wdev); wmi_device_put(wdev); @@ -653,6 +651,22 @@ char *wmi_get_acpi_device_uid(const char *guid_string) } EXPORT_SYMBOL_GPL(wmi_get_acpi_device_uid); +/** + * wmidev_get_acpi_device_uid() - Get _UID name of a WMI device + * @wdev: A wmi bus device from a driver + * + * Find the _UID of the ACPI device associated with this WMI device. + * + * Return: The ACPI _UID field or NULL if there is no _UID + */ +char *wmidev_get_acpi_device_uid(struct wmi_device *wdev) +{ + struct wmi_block *wblock = container_of(wdev, struct wmi_block, dev); + + return acpi_device_uid(wblock->acpi_device); +} +EXPORT_SYMBOL_GPL(wmidev_get_acpi_device_uid); + /* * sysfs interface */ diff --git a/include/linux/wmi.h b/include/linux/wmi.h index 10751c8e5e6a0ad3ac9ae317b6f8ecfb14c9a983..625c52ee125219aaa23cf946333af33ad118aafa 100644 --- a/include/linux/wmi.h +++ b/include/linux/wmi.h @@ -46,6 +46,8 @@ extern union acpi_object *wmidev_block_query(struct wmi_device *wdev, acpi_status wmidev_block_set(struct wmi_device *wdev, u8 instance, const struct acpi_buffer *in); +char *wmidev_get_acpi_device_uid(struct wmi_device *wdev); + u8 wmidev_instance_count(struct wmi_device *wdev); /**
Add a non-deprecated version of wmi_get_acpi_device_uid(). Signed-off-by: Kurt Borja <kuurtb@gmail.com> --- drivers/platform/x86/wmi.c | 20 +++++++++++++++++--- include/linux/wmi.h | 2 ++ 2 files changed, 19 insertions(+), 3 deletions(-)