| Message ID | 20170606030302.GC27270@fury (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Mon, Jun 5, 2017 at 8:03 PM, Darren Hart <dvhart@infradead.org> wrote: > On Thu, Jun 01, 2017 at 10:43:39PM +0200, Michał Kępień wrote: >> I know I have probably started sounding like a broken record by now, but >> I still have not seen any response (apart from the typos getting fixed) >> to my comments on this patch which I posted in January 2016 [1]. >> >> None of the issues I found back then are really critical, but I did >> point out a potential memory leak (granted, an unlikely one), so it >> might be a good idea to at least take a second look before merging. >> >> [1] https://www.spinics.net/lists/platform-driver-x86/msg08201.html > > Thanks for being persistent, some good points in there. I'd like to just squash > these into this patch (9/16), but I'll include them here for an ack from you and > Andy L. that this is what you meant, and consistent with his > intent/understanding: > Looks good to me. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On Mon, Jun 5, 2017 at 8:03 PM, Darren Hart <dvhart@infradead.org> wrote: > > On Thu, Jun 01, 2017 at 10:43:39PM +0200, Michał Kępień wrote: > >> I know I have probably started sounding like a broken record by now, but > >> I still have not seen any response (apart from the typos getting fixed) > >> to my comments on this patch which I posted in January 2016 [1]. > >> > >> None of the issues I found back then are really critical, but I did > >> point out a potential memory leak (granted, an unlikely one), so it > >> might be a good idea to at least take a second look before merging. > >> > >> [1] https://www.spinics.net/lists/platform-driver-x86/msg08201.html > > > > Thanks for being persistent, some good points in there. I'd like to just squash > > these into this patch (9/16), but I'll include them here for an ack from you and > > Andy L. that this is what you meant, and consistent with his > > intent/understanding: > > > > Looks good to me. Same here.
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index bfc0a3f..fbce876 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -863,14 +863,7 @@ static void wmi_free_devices(struct acpi_device *device) list_for_each_entry_safe(wblock, next, &wmi_block_list, list) { if (wblock->acpi_device == device) { list_del(&wblock->list); - if (wblock->dev.dev.bus) { - /* Device was initialized. */ - device_del(&wblock->dev.dev); - put_device(&wblock->dev.dev); - } else { - /* device_initialize was not called. */ - kfree(wblock); - } + device_unregister(&wblock->dev.dev); } } } @@ -903,9 +896,9 @@ static bool guid_already_parsed(struct acpi_device *device, static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) { struct acpi_buffer out = {ACPI_ALLOCATE_BUFFER, NULL}; - union acpi_object *obj; const struct guid_block *gblock; struct wmi_block *wblock, *next; + union acpi_object *obj; acpi_status status; int retval = 0; u32 i, total; @@ -958,24 +951,27 @@ static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) } } -out_free_pointer: - kfree(out.pointer); - /* * Now that all of the devices are created, add them to the * device tree and probe subdrivers. */ list_for_each_entry_safe(wblock, next, &wmi_block_list, list) { - if (wblock->acpi_device == device) { - if (device_add(&wblock->dev.dev) != 0) { - dev_err(wmi_bus_dev, - "failed to register %pULL\n", - wblock->gblock.guid); - list_del(&wblock->list); - } + if (wblock->acpi_device != device) + continue; + + retval = device_add(&wblock->dev.dev); + if (retval) { + dev_err(wmi_bus_dev, "failed to register %pULL\n", + wblock->gblock.guid); + if (debug_event) + wmi_method_enable(wblock, 0); + list_del(&wblock->list); + put_device(&wblock->dev.dev); } } +out_free_pointer: + kfree(out.pointer); return retval; }