Re: [PATCH v2 2/2] platform/x86: wmi: fail wmi_driver_register when no GUID is found

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 7/31/20 10:21 PM, Daniel Dadap wrote:
> If a driver registers with WMI, and none of the GUIDs in its ID table
> is present on the system, then that driver will not be probed and
> automatically loaded. However, it is still possible to load such a
> driver explicitly on a system which doesn't include the relevant
> hardware.
> 
> Update wmi_driver_register to test for the presence of at least one
> GUID from the driver's ID table at driver registration time, and
> fail registration if none are found.

This would make the WMI bus different from all the other kernel
bus subsystems where one can happily load drivers even if there
is no hardware using them.

And this would also break being able to manually bind a different
(hopefully compatible but different) guid device through
/sys/bus/wmi/drivers/foo/bind

So NACK to this one from me.

Note please do send a new version of patch 1/2 of this sets addressing
Andy's remarks to the other similar patch you did.

Regards,

Hans





> 
> Signed-off-by: Daniel Dadap <ddadap@nvidia.com>
> ---
>  drivers/platform/x86/wmi.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index 941739db7199..19aa23d1cf8e 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -130,6 +130,21 @@ static bool find_guid(const char *guid_string, struct wmi_block **out)
>  	return false;
>  }
>  
> +static bool find_driver_guid(const struct wmi_driver *wdriver)
> +{
> +	const struct wmi_device_id *id;
> +
> +	if (wdriver == NULL)
> +		return false;
> +
> +	for (id = wdriver->id_table; *id->guid_string; id++) {
> +		if (find_guid(id->guid_string, NULL))
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>  static const void *find_guid_context(struct wmi_block *wblock,
>  				      struct wmi_driver *wdriver)
>  {
> @@ -1419,6 +1434,9 @@ static int acpi_wmi_probe(struct platform_device *device)
>  int __must_check __wmi_driver_register(struct wmi_driver *driver,
>  				       struct module *owner)
>  {
> +	if (!find_driver_guid(driver))
> +		return -ENODEV;
> +
>  	driver->driver.owner = owner;
>  	driver->driver.bus = &wmi_bus_type;
>  
>