Hi, On 8/29/22 22:14, Mario Limonciello wrote: > The WMI subsystem in the kernel currently tracks WMI devices by > a GUID string not by ACPI device. The GUID used by the `wmi-bmof` > module however is available from many devices on nearly every machine. > > This originally was though to be a bug, but as it happens on most > machines it is a design mistake. It has been fixed by tying an ACPI > device to the driver with struct wmi_driver. So drivers that have > moved over to struct wmi_driver can actually support multiple > instantiations of a GUID without any problem. > > Add an allow list into wmi.c for GUIDs that the drivers that are known > to use struct wmi_driver. The list is populated with `wmi-bmof` right > now. The additional instances of that in sysfs with be suffixed with -%d > > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > --- > v1->v2: > * Change to an allow list for wmi-bmof and suffix the extra instances Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans > > drivers/platform/x86/wmi.c | 45 ++++++++++++++++++++++++++++++-------- > 1 file changed, 36 insertions(+), 9 deletions(-) > > diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c > index aed293b5af81..2997dad79e8b 100644 > --- a/drivers/platform/x86/wmi.c > +++ b/drivers/platform/x86/wmi.c > @@ -105,6 +105,12 @@ static const struct acpi_device_id wmi_device_ids[] = { > }; > MODULE_DEVICE_TABLE(acpi, wmi_device_ids); > > +/* allow duplicate GUIDs as these device drivers use struct wmi_driver */ > +static const char * const allow_duplicates[] = { > + "05901221-D566-11D1-B2F0-00A0C9062910", /* wmi-bmof */ > + NULL, > +}; > + > static struct platform_driver acpi_wmi_driver = { > .driver = { > .name = "acpi-wmi", > @@ -1073,6 +1079,19 @@ static const struct device_type wmi_type_data = { > .release = wmi_dev_release, > }; > > +static int guid_count(const guid_t *guid) > +{ > + struct wmi_block *wblock; > + int count = 0; > + > + list_for_each_entry(wblock, &wmi_block_list, list) { > + if (guid_equal(&wblock->gblock.guid, guid)) > + count++; > + } > + > + return count; > +} > + > static int wmi_create_device(struct device *wmi_bus_dev, > struct wmi_block *wblock, > struct acpi_device *device) > @@ -1080,6 +1099,7 @@ static int wmi_create_device(struct device *wmi_bus_dev, > struct acpi_device_info *info; > char method[WMI_ACPI_METHOD_NAME_SIZE]; > int result; > + uint count; > > if (wblock->gblock.flags & ACPI_WMI_EVENT) { > wblock->dev.dev.type = &wmi_type_event; > @@ -1134,7 +1154,11 @@ static int wmi_create_device(struct device *wmi_bus_dev, > wblock->dev.dev.bus = &wmi_bus_type; > wblock->dev.dev.parent = wmi_bus_dev; > > - dev_set_name(&wblock->dev.dev, "%pUL", &wblock->gblock.guid); > + count = guid_count(&wblock->gblock.guid); > + if (count) > + dev_set_name(&wblock->dev.dev, "%pUL-%d", &wblock->gblock.guid, count); > + else > + dev_set_name(&wblock->dev.dev, "%pUL", &wblock->gblock.guid); > > device_initialize(&wblock->dev.dev); > > @@ -1154,11 +1178,20 @@ static void wmi_free_devices(struct acpi_device *device) > } > } > > -static bool guid_already_parsed(struct acpi_device *device, const guid_t *guid) > +static bool guid_already_parsed_for_legacy(struct acpi_device *device, const guid_t *guid) > { > struct wmi_block *wblock; > > list_for_each_entry(wblock, &wmi_block_list, list) { > + /* skip warning and register if we know the driver will use struct wmi_driver */ > + for (int i = 0; allow_duplicates[i] != NULL; i++) { > + guid_t tmp; > + > + if (guid_parse(allow_duplicates[i], &tmp)) > + continue; > + if (guid_equal(&tmp, guid)) > + return false; > + } > if (guid_equal(&wblock->gblock.guid, guid)) { > /* > * Because we historically didn't track the relationship > @@ -1208,13 +1241,7 @@ static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) > if (debug_dump_wdg) > wmi_dump_wdg(&gblock[i]); > > - /* > - * Some WMI devices, like those for nVidia hooks, have a > - * duplicate GUID. It's not clear what we should do in this > - * case yet, so for now, we'll just ignore the duplicate > - * for device creation. > - */ > - if (guid_already_parsed(device, &gblock[i].guid)) > + if (guid_already_parsed_for_legacy(device, &gblock[i].guid)) > continue; > > wblock = kzalloc(sizeof(*wblock), GFP_KERNEL);
