From: "Darren Hart (VMware)" <dvh...@infradead.org>

The Microsoft WMI documentation requires all data blocks to implement
the Query Control Method (WQxx). If we encounter a data block not
implementing this control method, issue a warning, and ignore the data
block. Remove the "readable" attribute as all data blocks must be
readable (query-able).

Be consistent with the language in the documentation, replace the
"writable" attribute with "setable".

Simplify (flatten) the control flow of wmi_create_device a bit while
we are updating it for the above changes.

Signed-off-by: Darren Hart (VMware) <dvh...@infradead.org>
Cc: Andy Lutomirski <l...@amacapital.net>
Cc: Mario Limonciello <mario_limoncie...@dell.com>
Cc: Pali Rohár <pali.ro...@gmail.com>
Cc: Rafael Wysocki <r...@rjwysocki.net>
Cc: linux-kernel@vger.kernel.org
Cc: platform-driver-...@vger.kernel.org
Cc: linux-a...@vger.kernel.org
---
 drivers/platform/x86/wmi.c | 117 +++++++++++++++++++++++----------------------
 include/linux/wmi.h        |   7 +--
 2 files changed, 63 insertions(+), 61 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 8dd9988..cc55952 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -69,7 +69,7 @@ struct wmi_block {
        wmi_notify_handler handler;
        void *handler_data;
 
-       bool read_takes_no_args;        /* only defined if readable */
+       bool read_takes_no_args;
 };
 
 
@@ -694,28 +694,18 @@ static ssize_t object_id_show(struct device *dev, struct 
device_attribute *attr,
 }
 static DEVICE_ATTR_RO(object_id);
 
-static ssize_t readable_show(struct device *dev, struct device_attribute *attr,
-                            char *buf)
+static ssize_t setable_show(struct device *dev, struct device_attribute *attr,
+                           char *buf)
 {
        struct wmi_device *wdev = dev_to_wdev(dev);
 
-       return sprintf(buf, "%d\n", (int)wdev->readable);
+       return sprintf(buf, "%d\n", (int)wdev->setable);
 }
-static DEVICE_ATTR_RO(readable);
-
-static ssize_t writeable_show(struct device *dev, struct device_attribute 
*attr,
-                             char *buf)
-{
-       struct wmi_device *wdev = dev_to_wdev(dev);
-
-       return sprintf(buf, "%d\n", (int)wdev->writeable);
-}
-static DEVICE_ATTR_RO(writeable);
+static DEVICE_ATTR_RO(setable);
 
 static struct attribute *wmi_data_attrs[] = {
        &dev_attr_object_id.attr,
-       &dev_attr_readable.attr,
-       &dev_attr_writeable.attr,
+       &dev_attr_setable.attr,
        NULL,
 };
 ATTRIBUTE_GROUPS(wmi_data);
@@ -833,63 +823,74 @@ static struct device_type wmi_type_data = {
        .release = wmi_dev_release,
 };
 
-static void wmi_create_device(struct device *wmi_bus_dev,
+static int wmi_create_device(struct device *wmi_bus_dev,
                             const struct guid_block *gblock,
                             struct wmi_block *wblock,
                             struct acpi_device *device)
 {
-       wblock->dev.dev.bus = &wmi_bus_type;
-       wblock->dev.dev.parent = wmi_bus_dev;
-
-       dev_set_name(&wblock->dev.dev, "%pUL", gblock->guid);
+       struct acpi_device_info *info;
+       char method[5];
+       int result;
 
        if (gblock->flags & ACPI_WMI_EVENT) {
                wblock->dev.dev.type = &wmi_type_event;
-       } else if (gblock->flags & ACPI_WMI_METHOD) {
+               goto out_init;
+       }
+
+       if (gblock->flags & ACPI_WMI_METHOD) {
                wblock->dev.dev.type = &wmi_type_method;
-       } else {
-               struct acpi_device_info *info;
-               char method[5];
-               int result;
+               goto out_init;
+       }
 
-               wblock->dev.dev.type = &wmi_type_data;
+       /*
+        * Data Block Query Control Method (WQxx by convention) is
+        * required per the WMI documentation. If it is not present,
+        * we ignore this data block.
+        */
+       strcpy(method, "WQ");
+       strncat(method, wblock->gblock.object_id, 2);
+       result = get_subobj_info(device->handle, method, &info);
+
+       if (result) {
+               dev_warn(wmi_bus_dev,
+                        "%s data block query control method not found",
+                        method);
+               return result;
+       }
 
-               strcpy(method, "WQ");
-               strncat(method, wblock->gblock.object_id, 2);
-               result = get_subobj_info(device->handle, method, &info);
+       wblock->dev.dev.type = &wmi_type_data;
 
-               if (result == 0) {
-                       wblock->dev.readable = true;
+       /*
+        * The Microsoft documentation specifically states:
+        *
+        *   Data blocks registered with only a single instance
+        *   can ignore the parameter.
+        *
+        * ACPICA will get mad at us if we call the method with the wrong number
+        * of arguments, so check what our method expects.  (On some Dell
+        * laptops, WQxx may not be a method at all.)
+        */
+       if (info->type != ACPI_TYPE_METHOD || info->param_count == 0)
+               wblock->read_takes_no_args = true;
 
-                       /*
-                        * The Microsoft documentation specifically states:
-                        *
-                        *   Data blocks registered with only a single instance
-                        *   can ignore the parameter.
-                        *
-                        * ACPICA will get mad at us if we call the method
-                        * with the wrong number of arguments, so check what
-                        * our method expects.  (On some Dell laptops, WQxx
-                        * may not be a method at all.)
-                        */
-                       if (info->type != ACPI_TYPE_METHOD ||
-                           info->param_count == 0)
-                               wblock->read_takes_no_args = true;
+       kfree(info);
 
-                       kfree(info);
-               }
+       strcpy(method, "WS");
+       strncat(method, wblock->gblock.object_id, 2);
+       result = get_subobj_info(device->handle, method, NULL);
 
-               strcpy(method, "WS");
-               strncat(method, wblock->gblock.object_id, 2);
-               result = get_subobj_info(device->handle, method, NULL);
+       if (result == 0)
+               wblock->dev.setable = true;
 
-               if (result == 0) {
-                       wblock->dev.writeable = true;
-               }
+ out_init:
+       wblock->dev.dev.bus = &wmi_bus_type;
+       wblock->dev.dev.parent = wmi_bus_dev;
 
-       }
+       dev_set_name(&wblock->dev.dev, "%pUL", gblock->guid);
 
        device_initialize(&wblock->dev.dev);
+
+       return 0;
 }
 
 static void wmi_free_devices(struct acpi_device *device)
@@ -985,7 +986,11 @@ static int parse_wdg(struct device *wmi_bus_dev, struct 
acpi_device *device)
                wblock->acpi_device = device;
                wblock->gblock = gblock[i];
 
-               wmi_create_device(wmi_bus_dev, &gblock[i], wblock, device);
+               retval = wmi_create_device(wmi_bus_dev, &gblock[i], wblock, 
device);
+               if (retval) {
+                       kfree(wblock);
+                       continue;
+               }
 
                list_add_tail(&wblock->list, &wmi_block_list);
 
diff --git a/include/linux/wmi.h b/include/linux/wmi.h
index a283768..cd0d773 100644
--- a/include/linux/wmi.h
+++ b/include/linux/wmi.h
@@ -22,11 +22,8 @@
 struct wmi_device {
        struct device dev;
 
-       /*
-        * These are true for data objects that support reads and writes,
-        * respectively.
-        */
-       bool readable, writeable;
+        /* True for data blocks implementing the Set Control Method */
+       bool setable;
 };
 
 /* Caller must kfree the result. */
-- 
2.9.4

Reply via email to