2023-12-18 20:19:23

by Armin Wolf

[permalink] [raw]
Subject: [PATCH RESEND v2 0/6] platform/x86: wmi: ACPI improvements

This patch series improves the ACPI handling inside the ACPI WMI driver.
The first patch removes an unused variable, while the second patch
changes the order in which the ACPI handlers are removed on shutdown.
The third patch simplifies the error handling during probe by using
devres to manage devie resources, while the next two patches decouple
the ACPI notify handler from the wmi_block_list. The last patch
simplifies yet another ACPI-related function.

All patches have been tested on a Dell Inspiron 3505 and appear to work.

Changes since v1:
- fix ACPI handler devres ordering

Armin Wolf (6):
platform/x86: wmi: Remove unused variable in address space handler
platform/x86: wmi: Remove ACPI handlers after WMI devices
platform/x86: wmi: Use devres for resource handling
platform/x86: wmi: Create WMI bus device first
platform/x86: wmi: Decouple ACPI notify handler from wmi_block_list
platform/x86: wmi: Simplify get_subobj_info()

drivers/platform/x86/wmi.c | 143 ++++++++++++++++++-------------------
1 file changed, 71 insertions(+), 72 deletions(-)

--
2.39.2



2023-12-18 20:20:32

by Armin Wolf

[permalink] [raw]
Subject: [PATCH RESEND v2 4/6] platform/x86: wmi: Create WMI bus device first

Create the WMI bus device first so that it can be used
by the ACPI handlers.

Reviewed-by: Hans de Goede <[email protected]>
Signed-off-by: Armin Wolf <[email protected]>
---
drivers/platform/x86/wmi.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 4bc5da70c1b0..e2bfdc61c4ce 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -1276,6 +1276,17 @@ static int acpi_wmi_probe(struct platform_device *device)
return -ENODEV;
}

+ wmi_bus_dev = device_create(&wmi_bus_class, &device->dev, MKDEV(0, 0), NULL, "wmi_bus-%s",
+ dev_name(&device->dev));
+ if (IS_ERR(wmi_bus_dev))
+ return PTR_ERR(wmi_bus_dev);
+
+ error = devm_add_action_or_reset(&device->dev, acpi_wmi_remove_bus_device, wmi_bus_dev);
+ if (error < 0)
+ return error;
+
+ dev_set_drvdata(&device->dev, wmi_bus_dev);
+
status = acpi_install_address_space_handler(acpi_device->handle,
ACPI_ADR_SPACE_EC,
&acpi_wmi_ec_space_handler,
@@ -1302,17 +1313,6 @@ static int acpi_wmi_probe(struct platform_device *device)
if (error < 0)
return error;

- wmi_bus_dev = device_create(&wmi_bus_class, &device->dev, MKDEV(0, 0),
- NULL, "wmi_bus-%s", dev_name(&device->dev));
- if (IS_ERR(wmi_bus_dev))
- return PTR_ERR(wmi_bus_dev);
-
- error = devm_add_action_or_reset(&device->dev, acpi_wmi_remove_bus_device, wmi_bus_dev);
- if (error < 0)
- return error;
-
- dev_set_drvdata(&device->dev, wmi_bus_dev);
-
error = parse_wdg(wmi_bus_dev, device);
if (error) {
pr_err("Failed to parse WDG method\n");
--
2.39.2


2023-12-18 20:21:17

by Armin Wolf

[permalink] [raw]
Subject: [PATCH RESEND v2 6/6] platform/x86: wmi: Simplify get_subobj_info()

All callers who call get_subobj_info() with **info being NULL
should better use acpi_has_method() instead.
Convert the only caller who does this to acpi_has_method()
to drop the dummy info handling.

Reviewed-by: Hans de Goede <[email protected]>
Signed-off-by: Armin Wolf <[email protected]>
---
drivers/platform/x86/wmi.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 559a99ebc624..a7cfcbf92432 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -132,23 +132,19 @@ static const void *find_guid_context(struct wmi_block *wblock,
static int get_subobj_info(acpi_handle handle, const char *pathname,
struct acpi_device_info **info)
{
- struct acpi_device_info *dummy_info, **info_ptr;
acpi_handle subobj_handle;
acpi_status status;

- status = acpi_get_handle(handle, (char *)pathname, &subobj_handle);
+ status = acpi_get_handle(handle, pathname, &subobj_handle);
if (status == AE_NOT_FOUND)
return -ENOENT;
- else if (ACPI_FAILURE(status))
- return -EIO;

- info_ptr = info ? info : &dummy_info;
- status = acpi_get_object_info(subobj_handle, info_ptr);
if (ACPI_FAILURE(status))
return -EIO;

- if (!info)
- kfree(dummy_info);
+ status = acpi_get_object_info(subobj_handle, info);
+ if (ACPI_FAILURE(status))
+ return -EIO;

return 0;
}
@@ -998,9 +994,7 @@ static int wmi_create_device(struct device *wmi_bus_dev,
kfree(info);

get_acpi_method_name(wblock, 'S', method);
- result = get_subobj_info(device->handle, method, NULL);
-
- if (result == 0)
+ if (acpi_has_method(device->handle, method))
wblock->dev.setable = true;

out_init:
--
2.39.2