On 7/31/20 3:52 PM, Limonciello, Mario wrote:
-----Original Message----- From: Daniel Dadap <ddadap@xxxxxxxxxx> Sent: Friday, July 31, 2020 3:22 PM To: platform-driver-x86@xxxxxxxxxxxxxxx; Limonciello, Mario; pobrn@xxxxxxxxxxxxxx Cc: andy@xxxxxxxxxxxxx; dvhart@xxxxxxxxxxxxx; aplattner@xxxxxxxxxx; Daniel Dadap Subject: [PATCH v2 2/2] platform/x86: wmi: fail wmi_driver_register when no GUID is found [EXTERNAL EMAIL] 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. Signed-off-by: Daniel Dadap <ddadap@xxxxxxxxxx> --- 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; +} +As a point of preference, why not in this function return -ENODEV directly rather than it be boolean and map all errors to -ENODEV?
Sure, if others think this behavior change is useful/desirable, I'd be happy to change find_driver_guid() to return an error code directly, and maybe return -EINVAL in the wdriver == NULL case.
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; +Then the logic here can be something like: ret = find_driver_guid(driver); if (ret) return ret;driver->driver.owner = owner; driver->driver.bus = &wmi_bus_type; -- 2.18.4S
