Re: [PATCH RFC v2 1/2] platform/x86: wmi: Add wmidev_get_acpi_device_uid()

[Armin Wolf <W_Armin@gmx.de>]

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);
>
>   /**
>