[PATCH v3 0/9] Convert most Dell WMI drivers to use the new buffer-based API

[PATCH v3 0/9] Convert most Dell WMI drivers to use the new buffer-based API

[Armin Wolf]

The new buffer-based WMI API improves the compatibility between
different ACPI firmware implementations by performing marshalling/
unmarshalling of WMI buffers like the original Windows driver.

Convert most Dell WMI drivers to use this new API. This also
removes all ACPI-related code from those drivers because the
new buffer-based WMI API uses abstract WMI buffer objects instead
of ACPI objects.

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

The last three patches contain some misc. cleanups for the WMI
driver core itself. The most important change is a fix for modprobe
to verify any WMI GUID strings from WMI drivers and convert them
to uppercase if necessary. This should fix autoloading for WMI
drivers that use WMI GUID strings with lowercase letters.

Changes since v2:
- add Acked-by tag from Guenter
- rework buffer parsing in dell-wmi-descriptor as suggested by Gergo

Changes since v1:
- do not perform pointer arithmetic on void pointers in dell-smm-hwmon

Armin Wolf (9):
  platform/x86: dell-descriptor: Use new buffer-based WMI API
  platform/x86: dell-privacy: Use new buffer-based WMI API
  platform/x86: dell-smbios-wmi: Use new buffer-based WMI API
  platform/x86: dell-wmi-base: Use new buffer-based WMI API
  platform/x86: dell-ddv: Use new buffer-based WMI API
  hwmon: (dell-smm) Use new buffer-based WMI API
  platform/wmi: Make wmi_bus_class const
  platform/wmi: Make sysfs attributes const
  modpost: Handle malformed WMI GUID strings

 .../wmi/driver-development-guide.rst          |   2 +-
 drivers/hwmon/dell-smm-hwmon.c                |  47 ++---
 drivers/platform/wmi/core.c                   |  34 +--
 drivers/platform/x86/dell/dell-smbios-wmi.c   |  46 +++--
 drivers/platform/x86/dell/dell-wmi-base.c     |  68 +++---
 drivers/platform/x86/dell/dell-wmi-ddv.c      | 194 ++++++++++--------
 .../platform/x86/dell/dell-wmi-descriptor.c   |  94 ++++-----
 drivers/platform/x86/dell/dell-wmi-privacy.c  |  78 ++++---
 scripts/mod/file2alias.c                      |  28 ++-
 9 files changed, 315 insertions(+), 276 deletions(-)

-- 
2.39.5

[PATCH v3 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API

[Armin Wolf]

Use the new buffer-based WMI API to also support ACPI firmware
implementations that do not use ACPI buffers for the descriptor.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 .../platform/x86/dell/dell-wmi-descriptor.c   | 94 +++++++++----------
 1 file changed, 47 insertions(+), 47 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-wmi-descriptor.c b/drivers/platform/x86/dell/dell-wmi-descriptor.c
index c2a180202719..fe42eb8bbd79 100644
--- a/drivers/platform/x86/dell/dell-wmi-descriptor.c
+++ b/drivers/platform/x86/dell/dell-wmi-descriptor.c
@@ -7,7 +7,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/acpi.h>
+#include <linux/compiler_attributes.h>
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/wmi.h>
@@ -15,6 +15,24 @@
 
 #define DELL_WMI_DESCRIPTOR_GUID "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
 
+/*
+ * Descriptor buffer is 128 byte long and contains:
+ *
+ *       Name             Offset  Length  Value
+ * Vendor Signature          0       4    "DELL"
+ * Object Signature          4       4    " WMI"
+ * WMI Interface Version     8       4    <version>
+ * WMI buffer length        12       4    <length>
+ * WMI hotfix number        16       4    <hotfix>
+ */
+struct descriptor {
+	char vendor_signature[4];
+	char object_signature[4];
+	__le32 interface_version;
+	__le32 buffer_length;
+	__le32 hotfix_number;
+} __packed;
+
 struct descriptor_priv {
 	struct list_head list;
 	u32 interface_version;
@@ -88,76 +106,58 @@ bool dell_wmi_get_hotfix(u32 *hotfix)
 }
 EXPORT_SYMBOL_GPL(dell_wmi_get_hotfix);
 
-/*
- * Descriptor buffer is 128 byte long and contains:
- *
- *       Name             Offset  Length  Value
- * Vendor Signature          0       4    "DELL"
- * Object Signature          4       4    " WMI"
- * WMI Interface Version     8       4    <version>
- * WMI buffer length        12       4    <length>
- * WMI hotfix number        16       4    <hotfix>
- */
-static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
-				     const void *context)
+static int dell_wmi_descriptor_probe(struct wmi_device *wdev, const void *context)
 {
-	union acpi_object *obj = NULL;
 	struct descriptor_priv *priv;
-	u32 *buffer;
+	struct wmi_buffer buffer;
+	struct descriptor *desc;
 	int ret;
 
-	obj = wmidev_block_query(wdev, 0);
-	if (!obj) {
-		dev_err(&wdev->dev, "failed to read Dell WMI descriptor\n");
-		ret = -EIO;
-		goto out;
-	}
+	ret = wmidev_query_block(wdev, 0, &buffer);
+	if (ret < 0)
+		return ret;
 
-	if (obj->type != ACPI_TYPE_BUFFER) {
-		dev_err(&wdev->dev, "Dell descriptor has wrong type\n");
+	if (buffer.length < sizeof(*desc)) {
+		dev_err(&wdev->dev,
+			"Dell descriptor buffer contains not enough data (%zu)\n",
+			buffer.length);
 		ret = -EINVAL;
 		descriptor_valid = ret;
 		goto out;
 	}
 
-	/* Although it's not technically a failure, this would lead to
-	 * unexpected behavior
-	 */
-	if (obj->buffer.length != 128) {
-		dev_err(&wdev->dev,
-			"Dell descriptor buffer has unexpected length (%d)\n",
-			obj->buffer.length);
-		ret = -EINVAL;
+	desc = buffer.data;
+
+	if (strncmp(desc->vendor_signature, "DELL", sizeof(desc->vendor_signature))) {
+		dev_err(&wdev->dev, "Dell descriptor buffer has invalid vendor signature (%4ph)\n",
+			desc->vendor_signature);
+		ret = -ENOMSG;
 		descriptor_valid = ret;
 		goto out;
 	}
 
-	buffer = (u32 *)obj->buffer.pointer;
-
-	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0) {
-		dev_err(&wdev->dev, "Dell descriptor buffer has invalid signature (%8ph)\n",
-			buffer);
-		ret = -EINVAL;
+	if (strncmp(desc->object_signature, " WMI", sizeof(desc->vendor_signature))) {
+		dev_err(&wdev->dev, "Dell descriptor buffer has invalid object signature (%4ph)\n",
+			desc->object_signature);
+		ret = -ENOMSG;
 		descriptor_valid = ret;
 		goto out;
 	}
 	descriptor_valid = 0;
 
-	if (buffer[2] != 0 && buffer[2] != 1)
-		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%lu)\n",
-			(unsigned long) buffer[2]);
-
-	priv = devm_kzalloc(&wdev->dev, sizeof(struct descriptor_priv),
-	GFP_KERNEL);
+	if (le32_to_cpu(desc->interface_version) > 2)
+		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%u)\n",
+			 le32_to_cpu(desc->interface_version));
 
+	priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv) {
 		ret = -ENOMEM;
 		goto out;
 	}
 
-	priv->interface_version = buffer[2];
-	priv->size = buffer[3];
-	priv->hotfix = buffer[4];
+	priv->interface_version = le32_to_cpu(desc->interface_version);
+	priv->size = le32_to_cpu(desc->buffer_length);
+	priv->hotfix = le32_to_cpu(desc->hotfix_number);
 	ret = 0;
 	dev_set_drvdata(&wdev->dev, priv);
 	mutex_lock(&list_mutex);
@@ -170,7 +170,7 @@ static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
 		(unsigned long) priv->hotfix);
 
 out:
-	kfree(obj);
+	kfree(buffer.data);
 	return ret;
 }
 
-- 
2.39.5

[PATCH v3 2/9] platform/x86: dell-privacy: Use new buffer-based WMI API

[Armin Wolf]

Use the new buffer-based WMI API to also support ACPI firmware
implementations that do not use ACPI buffers for the device state.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/dell/dell-wmi-privacy.c | 78 ++++++++++----------
 1 file changed, 38 insertions(+), 40 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-wmi-privacy.c b/drivers/platform/x86/dell/dell-wmi-privacy.c
index ed099a431ea4..470273cc2fd2 100644
--- a/drivers/platform/x86/dell/dell-wmi-privacy.c
+++ b/drivers/platform/x86/dell/dell-wmi-privacy.c
@@ -9,6 +9,7 @@
 
 #include <linux/acpi.h>
 #include <linux/bitops.h>
+#include <linux/compiler_attributes.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
 #include <linux/list.h>
@@ -25,6 +26,26 @@
 #define DELL_PRIVACY_CAMERA_EVENT 0x2
 #define led_to_priv(c)       container_of(c, struct privacy_wmi_data, cdev)
 
+/*
+ * Describes the Device State class exposed by BIOS which can be consumed by
+ * various applications interested in knowing the Privacy feature capabilities.
+ * class DeviceState
+ * {
+ *  [key, read] string InstanceName;
+ *  [read] boolean ReadOnly;
+ *
+ *  [WmiDataId(1), read] uint32 DevicesSupported;
+ *   0 - None; 0x1 - Microphone; 0x2 - Camera; 0x4 - ePrivacy  Screen
+ *
+ *  [WmiDataId(2), read] uint32 CurrentState;
+ *   0 - Off; 1 - On; Bit0 - Microphone; Bit1 - Camera; Bit2 - ePrivacyScreen
+ * };
+ */
+struct device_state {
+	__le32 devices_supported;
+	__le32 current_state;
+} __packed;
+
 /*
  * The wmi_list is used to store the privacy_priv struct with mutex protecting
  */
@@ -185,59 +206,36 @@ static struct attribute *privacy_attrs[] = {
 };
 ATTRIBUTE_GROUPS(privacy);
 
-/*
- * Describes the Device State class exposed by BIOS which can be consumed by
- * various applications interested in knowing the Privacy feature capabilities.
- * class DeviceState
- * {
- *  [key, read] string InstanceName;
- *  [read] boolean ReadOnly;
- *
- *  [WmiDataId(1), read] uint32 DevicesSupported;
- *   0 - None; 0x1 - Microphone; 0x2 - Camera; 0x4 - ePrivacy  Screen
- *
- *  [WmiDataId(2), read] uint32 CurrentState;
- *   0 - Off; 1 - On; Bit0 - Microphone; Bit1 - Camera; Bit2 - ePrivacyScreen
- * };
- */
 static int get_current_status(struct wmi_device *wdev)
 {
 	struct privacy_wmi_data *priv = dev_get_drvdata(&wdev->dev);
-	union acpi_object *obj_present;
-	u32 *buffer;
-	int ret = 0;
+	struct device_state *state;
+	struct wmi_buffer buffer;
+	int ret;
 
 	if (!priv) {
 		dev_err(&wdev->dev, "dell privacy priv is NULL\n");
 		return -EINVAL;
 	}
+
 	/* check privacy support features and device states */
-	obj_present = wmidev_block_query(wdev, 0);
-	if (!obj_present) {
-		dev_err(&wdev->dev, "failed to read Binary MOF\n");
-		return -EIO;
-	}
+	ret = wmidev_query_block(wdev, 0, &buffer);
+	if (ret < 0)
+		return ret;
 
-	if (obj_present->type != ACPI_TYPE_BUFFER) {
-		dev_err(&wdev->dev, "Binary MOF is not a buffer!\n");
-		ret = -EIO;
-		goto obj_free;
-	}
-	/*  Although it's not technically a failure, this would lead to
-	 *  unexpected behavior
-	 */
-	if (obj_present->buffer.length != 8) {
-		dev_err(&wdev->dev, "Dell privacy buffer has unexpected length (%d)!\n",
-				obj_present->buffer.length);
+	if (buffer.length < sizeof(*state)) {
+		dev_err(&wdev->dev, "Dell privacy buffer contains not enough data (%zu)!\n",
+			buffer.length);
 		ret = -EINVAL;
-		goto obj_free;
+		goto buffer_free;
 	}
-	buffer = (u32 *)obj_present->buffer.pointer;
-	priv->features_present = buffer[0];
-	priv->last_status = buffer[1];
 
-obj_free:
-	kfree(obj_present);
+	state = buffer.data;
+	priv->features_present = le32_to_cpu(state->devices_supported);
+	priv->last_status = le32_to_cpu(state->current_state);
+
+buffer_free:
+	kfree(buffer.data);
 	return ret;
 }
 
-- 
2.39.5

[PATCH v3 3/9] platform/x86: dell-smbios-wmi: Use new buffer-based WMI API

[Armin Wolf]

Use the new buffer-based WMI API to also support ACPI firmware
implementations that do not use ACPI buffers for returning the
results of a SMBIOS call.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/dell/dell-smbios-wmi.c | 46 +++++++++++----------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-smbios-wmi.c b/drivers/platform/x86/dell/dell-smbios-wmi.c
index a7dca8c59d60..3c05b48354b3 100644
--- a/drivers/platform/x86/dell/dell-smbios-wmi.c
+++ b/drivers/platform/x86/dell/dell-smbios-wmi.c
@@ -50,38 +50,42 @@ static inline struct wmi_smbios_priv *get_first_smbios_priv(void)
 
 static int run_smbios_call(struct wmi_device *wdev)
 {
-	struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL};
-	struct wmi_smbios_priv *priv;
-	struct acpi_buffer input;
-	union acpi_object *obj;
-	acpi_status status;
-
-	priv = dev_get_drvdata(&wdev->dev);
-	input.length = priv->req_buf_size - sizeof(u64);
-	input.pointer = &priv->buf->std;
+	struct wmi_smbios_priv *priv = dev_get_drvdata(&wdev->dev);
+	const struct wmi_buffer input = {
+		.length = priv->req_buf_size - sizeof(u64),
+		.data = &priv->buf->std,
+	};
+	struct wmi_buffer output;
+	int ret;
 
 	dev_dbg(&wdev->dev, "evaluating: %u/%u [%x,%x,%x,%x]\n",
 		priv->buf->std.cmd_class, priv->buf->std.cmd_select,
 		priv->buf->std.input[0], priv->buf->std.input[1],
 		priv->buf->std.input[2], priv->buf->std.input[3]);
 
-	status = wmidev_evaluate_method(wdev, 0, 1, &input, &output);
-	if (ACPI_FAILURE(status))
-		return -EIO;
-	obj = (union acpi_object *)output.pointer;
-	if (obj->type != ACPI_TYPE_BUFFER) {
-		dev_dbg(&wdev->dev, "received type: %d\n", obj->type);
-		if (obj->type == ACPI_TYPE_INTEGER)
-			dev_dbg(&wdev->dev, "SMBIOS call failed: %llu\n",
-				obj->integer.value);
-		kfree(output.pointer);
+	ret = wmidev_invoke_method(wdev, 0, 1, &input, &output);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * The output buffer returned by the WMI method should have at least the size
+	 * of the input buffer. Because the Windows WMI implementation ignores any surplus
+	 * data returned by a WMI method call we emulate this behavior here.
+	 *
+	 * Additionally the ACPI firmware might return buffers with not enough data to
+	 * signal an error, so we only print a debug message here.
+	 */
+	if (output.length < input.length) {
+		dev_dbg(&wdev->dev, "SMBIOS call returned not enough data (%zu)\n", output.length);
+		kfree(output.data);
 		return -EIO;
 	}
-	memcpy(input.pointer, obj->buffer.pointer, obj->buffer.length);
+
+	memcpy(input.data, output.data, input.length);
 	dev_dbg(&wdev->dev, "result: [%08x,%08x,%08x,%08x]\n",
 		priv->buf->std.output[0], priv->buf->std.output[1],
 		priv->buf->std.output[2], priv->buf->std.output[3]);
-	kfree(output.pointer);
+	kfree(output.data);
 
 	return 0;
 }
-- 
2.39.5

[PATCH v3 4/9] platform/x86: dell-wmi-base: Use new buffer-based WMI API

[Armin Wolf]

Use the new buffer-based WMI API to also support ACPI firmware
implementations that do not use ACPI buffers for the event data.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/dell/dell-wmi-base.c | 68 ++++++++++++-----------
 1 file changed, 35 insertions(+), 33 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-wmi-base.c b/drivers/platform/x86/dell/dell-wmi-base.c
index 4eefbade2f5e..4a7ab9fb3f81 100644
--- a/drivers/platform/x86/dell/dell-wmi-base.c
+++ b/drivers/platform/x86/dell/dell-wmi-base.c
@@ -13,6 +13,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/compiler_attributes.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -408,7 +409,8 @@ static void dell_wmi_switch_event(struct input_dev **subdev,
 	input_sync(*subdev);
 }
 
-static int dell_wmi_process_key(struct wmi_device *wdev, int type, int code, u16 *buffer, int remaining)
+static int dell_wmi_process_key(struct wmi_device *wdev, int type, int code, __le16 *buffer,
+				int remaining)
 {
 	struct dell_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
 	const struct key_entry *key;
@@ -440,15 +442,15 @@ static int dell_wmi_process_key(struct wmi_device *wdev, int type, int code, u16
 	} else if (type == 0x0011 && code == 0xe070 && remaining > 0) {
 		dell_wmi_switch_event(&priv->tabletswitch_dev,
 				      "Dell tablet mode switch",
-				      SW_TABLET_MODE, !buffer[0]);
+				      SW_TABLET_MODE, !le16_to_cpu(buffer[0]));
 		return 1;
 	} else if (type == 0x0012 && code == 0x000c && remaining > 0) {
 		/* Eprivacy toggle, switch to "on" key entry for on events */
-		if (buffer[0] == 2)
+		if (le16_to_cpu(buffer[0]) == 2)
 			key++;
 		used = 1;
 	} else if (type == 0x0012 && code == 0x000d && remaining > 0) {
-		value = (buffer[2] == 2);
+		value = (le16_to_cpu(buffer[2]) == 2);
 		used = 1;
 	}
 
@@ -457,24 +459,17 @@ static int dell_wmi_process_key(struct wmi_device *wdev, int type, int code, u16
 	return used;
 }
 
-static void dell_wmi_notify(struct wmi_device *wdev,
-			    union acpi_object *obj)
+static void dell_wmi_notify(struct wmi_device *wdev, const struct wmi_buffer *buffer)
 {
 	struct dell_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
-	u16 *buffer_entry, *buffer_end;
-	acpi_size buffer_size;
+	__le16 *buffer_entry, *buffer_end;
+	size_t buffer_size;
 	int len, i;
 
-	if (obj->type != ACPI_TYPE_BUFFER) {
-		pr_warn("bad response type %x\n", obj->type);
-		return;
-	}
+	pr_debug("Received WMI event (%*ph)\n", (int)buffer->length, buffer->data);
 
-	pr_debug("Received WMI event (%*ph)\n",
-		obj->buffer.length, obj->buffer.pointer);
-
-	buffer_entry = (u16 *)obj->buffer.pointer;
-	buffer_size = obj->buffer.length/2;
+	buffer_entry = buffer->data;
+	buffer_size = buffer->length / 2;
 	buffer_end = buffer_entry + buffer_size;
 
 	/*
@@ -490,12 +485,12 @@ static void dell_wmi_notify(struct wmi_device *wdev,
 	 * one event on devices with WMI interface version 0.
 	 */
 	if (priv->interface_version == 0 && buffer_entry < buffer_end)
-		if (buffer_end > buffer_entry + buffer_entry[0] + 1)
-			buffer_end = buffer_entry + buffer_entry[0] + 1;
+		if (buffer_end > buffer_entry + le16_to_cpu(buffer_entry[0]) + 1)
+			buffer_end = buffer_entry + le16_to_cpu(buffer_entry[0]) + 1;
 
 	while (buffer_entry < buffer_end) {
 
-		len = buffer_entry[0];
+		len = le16_to_cpu(buffer_entry[0]);
 		if (len == 0)
 			break;
 
@@ -508,11 +503,11 @@ static void dell_wmi_notify(struct wmi_device *wdev,
 
 		pr_debug("Process buffer (%*ph)\n", len*2, buffer_entry);
 
-		switch (buffer_entry[1]) {
+		switch (le16_to_cpu(buffer_entry[1])) {
 		case 0x0000: /* One key pressed or event occurred */
 			if (len > 2)
-				dell_wmi_process_key(wdev, buffer_entry[1],
-						     buffer_entry[2],
+				dell_wmi_process_key(wdev, le16_to_cpu(buffer_entry[1]),
+						     le16_to_cpu(buffer_entry[2]),
 						     buffer_entry + 3,
 						     len - 3);
 			/* Extended data is currently ignored */
@@ -520,22 +515,29 @@ static void dell_wmi_notify(struct wmi_device *wdev,
 		case 0x0010: /* Sequence of keys pressed */
 		case 0x0011: /* Sequence of events occurred */
 			for (i = 2; i < len; ++i)
-				i += dell_wmi_process_key(wdev, buffer_entry[1],
-							  buffer_entry[i],
+				i += dell_wmi_process_key(wdev, le16_to_cpu(buffer_entry[1]),
+							  le16_to_cpu(buffer_entry[i]),
 							  buffer_entry + i,
 							  len - i - 1);
 			break;
 		case 0x0012:
-			if ((len > 4) && dell_privacy_process_event(buffer_entry[1], buffer_entry[3],
-								    buffer_entry[4]))
-				/* dell_privacy_process_event has handled the event */;
-			else if (len > 2)
-				dell_wmi_process_key(wdev, buffer_entry[1], buffer_entry[2],
+			if (len > 4) {
+				if (dell_privacy_process_event(le16_to_cpu(buffer_entry[1]),
+							       le16_to_cpu(buffer_entry[3]),
+							       le16_to_cpu(buffer_entry[4])))
+					break;
+			}
+
+			/* dell_privacy_process_event has not handled the event */
+
+			if (len > 2)
+				dell_wmi_process_key(wdev, le16_to_cpu(buffer_entry[1]),
+						     le16_to_cpu(buffer_entry[2]),
 						     buffer_entry + 3, len - 3);
+
 			break;
 		default: /* Unknown event */
-			pr_info("Unknown WMI event type 0x%x\n",
-				(int)buffer_entry[1]);
+			pr_info("Unknown WMI event type 0x%x\n", le16_to_cpu(buffer_entry[1]));
 			break;
 		}
 
@@ -821,7 +823,7 @@ static struct wmi_driver dell_wmi_driver = {
 	.id_table = dell_wmi_id_table,
 	.probe = dell_wmi_probe,
 	.remove = dell_wmi_remove,
-	.notify = dell_wmi_notify,
+	.notify_new = dell_wmi_notify,
 };
 
 static int __init dell_wmi_init(void)
-- 
2.39.5

[PATCH v3 5/9] platform/x86: dell-ddv: Use new buffer-based WMI API

[Armin Wolf]

Use the new buffer-based WMI API to also support ACPI firmware
implementations that do not use ACPI intergers/strings/packages
for exchanging data.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/dell/dell-wmi-ddv.c | 194 ++++++++++++-----------
 1 file changed, 105 insertions(+), 89 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-wmi-ddv.c b/drivers/platform/x86/dell/dell-wmi-ddv.c
index 62e3d060f038..a744fd21b8af 100644
--- a/drivers/platform/x86/dell/dell-wmi-ddv.c
+++ b/drivers/platform/x86/dell/dell-wmi-ddv.c
@@ -7,8 +7,8 @@
 
 #define pr_format(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/acpi.h>
 #include <linux/bitfield.h>
+#include <linux/compiler_attributes.h>
 #include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/device/driver.h>
@@ -99,6 +99,11 @@ enum dell_ddv_method {
 	DELL_DDV_THERMAL_SENSOR_INFORMATION	= 0x22,
 };
 
+struct dell_wmi_buffer {
+	__le32 raw_size;
+	u8 raw_data[];
+} __packed;
+
 struct fan_sensor_entry {
 	u8 type;
 	__le16 rpm;
@@ -126,7 +131,7 @@ struct dell_wmi_ddv_sensors {
 	bool active;
 	struct mutex lock;	/* protect caching */
 	unsigned long timestamp;
-	union acpi_object *obj;
+	struct dell_wmi_buffer *buffer;
 	u64 entries;
 };
 
@@ -158,105 +163,122 @@ static const char * const fan_dock_labels[] = {
 	"Docking Chipset Fan",
 };
 
-static int dell_wmi_ddv_query_type(struct wmi_device *wdev, enum dell_ddv_method method, u32 arg,
-				   union acpi_object **result, acpi_object_type type)
+static int dell_wmi_ddv_query(struct wmi_device *wdev, enum dell_ddv_method method, u32 arg,
+			      struct wmi_buffer *output)
 {
-	struct acpi_buffer out = { ACPI_ALLOCATE_BUFFER, NULL };
-	const struct acpi_buffer in = {
-		.length = sizeof(arg),
-		.pointer = &arg,
+	__le32 arg2 = cpu_to_le32(arg);
+	const struct wmi_buffer input = {
+		.length = sizeof(arg2),
+		.data = &arg2,
 	};
-	union acpi_object *obj;
-	acpi_status ret;
-
-	ret = wmidev_evaluate_method(wdev, 0x0, method, &in, &out);
-	if (ACPI_FAILURE(ret))
-		return -EIO;
-
-	obj = out.pointer;
-	if (!obj)
-		return -ENODATA;
 
-	if (obj->type != type) {
-		kfree(obj);
-		return -ENOMSG;
-	}
-
-	*result = obj;
-
-	return 0;
+	return wmidev_invoke_method(wdev, 0x0, method, &input, output);
 }
 
 static int dell_wmi_ddv_query_integer(struct wmi_device *wdev, enum dell_ddv_method method,
 				      u32 arg, u32 *res)
 {
-	union acpi_object *obj;
+	struct wmi_buffer output;
+	__le32 *argr;
 	int ret;
 
-	ret = dell_wmi_ddv_query_type(wdev, method, arg, &obj, ACPI_TYPE_INTEGER);
+	ret = dell_wmi_ddv_query(wdev, method, arg, &output);
 	if (ret < 0)
 		return ret;
 
-	if (obj->integer.value <= U32_MAX)
-		*res = (u32)obj->integer.value;
-	else
-		ret = -ERANGE;
+	if (output.length >= sizeof(*argr)) {
+		argr = output.data;
+		*res = le32_to_cpu(*argr);
+	} else {
+		ret = -EIO;
+	}
 
-	kfree(obj);
+	kfree(output.data);
 
 	return ret;
 }
 
 static int dell_wmi_ddv_query_buffer(struct wmi_device *wdev, enum dell_ddv_method method,
-				     u32 arg, union acpi_object **result)
+				     u32 arg, struct dell_wmi_buffer **result)
 {
-	union acpi_object *obj;
-	u64 buffer_size;
+	struct dell_wmi_buffer *buffer;
+	struct wmi_buffer output;
+	size_t buffer_size;
 	int ret;
 
-	ret = dell_wmi_ddv_query_type(wdev, method, arg, &obj, ACPI_TYPE_PACKAGE);
+	ret = dell_wmi_ddv_query(wdev, method, arg, &output);
 	if (ret < 0)
 		return ret;
 
-	if (obj->package.count != 2 ||
-	    obj->package.elements[0].type != ACPI_TYPE_INTEGER ||
-	    obj->package.elements[1].type != ACPI_TYPE_BUFFER) {
-		ret = -ENOMSG;
+	if (output.length < sizeof(*buffer)) {
+		ret = -EIO;
 
 		goto err_free;
 	}
 
-	buffer_size = obj->package.elements[0].integer.value;
-
-	if (!buffer_size) {
+	buffer = output.data;
+	if (!le32_to_cpu(buffer->raw_size)) {
 		ret = -ENODATA;
 
 		goto err_free;
 	}
 
-	if (buffer_size > obj->package.elements[1].buffer.length) {
+	buffer_size = struct_size(buffer, raw_data, le32_to_cpu(buffer->raw_size));
+	if (buffer_size > output.length) {
 		dev_warn(&wdev->dev,
-			 FW_WARN "WMI buffer size (%llu) exceeds ACPI buffer size (%d)\n",
-			 buffer_size, obj->package.elements[1].buffer.length);
+			 FW_WARN "Dell WMI buffer size (%zu) exceeds WMI buffer size (%zu)\n",
+			 buffer_size, output.length);
 		ret = -EMSGSIZE;
 
 		goto err_free;
 	}
 
-	*result = obj;
+	*result = buffer;
 
 	return 0;
 
 err_free:
-	kfree(obj);
+	kfree(output.data);
 
 	return ret;
 }
 
-static int dell_wmi_ddv_query_string(struct wmi_device *wdev, enum dell_ddv_method method,
-				     u32 arg, union acpi_object **result)
+static ssize_t dell_wmi_ddv_query_string(struct wmi_device *wdev, enum dell_ddv_method method,
+					 u32 arg, char *buf, size_t length)
 {
-	return dell_wmi_ddv_query_type(wdev, method, arg, result, ACPI_TYPE_STRING);
+	struct wmi_buffer output;
+	struct wmi_string *str;
+	size_t str_size;
+	ssize_t count;
+	int ret;
+
+	ret = dell_wmi_ddv_query(wdev, method, arg, &output);
+	if (ret < 0)
+		return ret;
+
+	if (output.length < sizeof(*str)) {
+		count = -EIO;
+
+		goto err_free;
+	}
+
+	str = output.data;
+	str_size = sizeof(*str) + le16_to_cpu(str->length);
+	if (str_size > output.length) {
+		dev_warn(&wdev->dev,
+			 FW_WARN "WMI string size (%zu) exceeds WMI buffer size (%zu)\n",
+			 str_size, output.length);
+		count = -EMSGSIZE;
+
+		goto err_free;
+	}
+
+	count = wmi_string_to_utf8s(str, buf, length);
+
+err_free:
+	kfree(output.data);
+
+	return count;
 }
 
 /*
@@ -265,28 +287,26 @@ static int dell_wmi_ddv_query_string(struct wmi_device *wdev, enum dell_ddv_meth
 static int dell_wmi_ddv_update_sensors(struct wmi_device *wdev, enum dell_ddv_method method,
 				       struct dell_wmi_ddv_sensors *sensors, size_t entry_size)
 {
+	struct dell_wmi_buffer *buffer;
 	u64 buffer_size, rem, entries;
-	union acpi_object *obj;
-	u8 *buffer;
 	int ret;
 
-	if (sensors->obj) {
+	if (sensors->buffer) {
 		if (time_before(jiffies, sensors->timestamp + HZ))
 			return 0;
 
-		kfree(sensors->obj);
-		sensors->obj = NULL;
+		kfree(sensors->buffer);
+		sensors->buffer = NULL;
 	}
 
-	ret = dell_wmi_ddv_query_buffer(wdev, method, 0, &obj);
+	ret = dell_wmi_ddv_query_buffer(wdev, method, 0, &buffer);
 	if (ret < 0)
 		return ret;
 
 	/* buffer format sanity check */
-	buffer_size = obj->package.elements[0].integer.value;
-	buffer = obj->package.elements[1].buffer.pointer;
+	buffer_size = le32_to_cpu(buffer->raw_size);
 	entries = div64_u64_rem(buffer_size, entry_size, &rem);
-	if (rem != 1 || buffer[buffer_size - 1] != 0xff) {
+	if (rem != 1 || buffer->raw_data[buffer_size - 1] != 0xff) {
 		ret = -ENOMSG;
 		goto err_free;
 	}
@@ -296,14 +316,14 @@ static int dell_wmi_ddv_update_sensors(struct wmi_device *wdev, enum dell_ddv_me
 		goto err_free;
 	}
 
-	sensors->obj = obj;
+	sensors->buffer = buffer;
 	sensors->entries = entries;
 	sensors->timestamp = jiffies;
 
 	return 0;
 
 err_free:
-	kfree(obj);
+	kfree(buffer);
 
 	return ret;
 }
@@ -328,7 +348,7 @@ static int dell_wmi_ddv_fan_read_channel(struct dell_wmi_ddv_data *data, u32 att
 	if (channel >= data->fans.entries)
 		return -ENXIO;
 
-	entry = (struct fan_sensor_entry *)data->fans.obj->package.elements[1].buffer.pointer;
+	entry = (struct fan_sensor_entry *)data->fans.buffer->raw_data;
 	switch (attr) {
 	case hwmon_fan_input:
 		*val = get_unaligned_le16(&entry[channel].rpm);
@@ -354,7 +374,7 @@ static int dell_wmi_ddv_temp_read_channel(struct dell_wmi_ddv_data *data, u32 at
 	if (channel >= data->temps.entries)
 		return -ENXIO;
 
-	entry = (struct thermal_sensor_entry *)data->temps.obj->package.elements[1].buffer.pointer;
+	entry = (struct thermal_sensor_entry *)data->temps.buffer->raw_data;
 	switch (attr) {
 	case hwmon_temp_input:
 		*val = entry[channel].now * 1000;
@@ -411,7 +431,7 @@ static int dell_wmi_ddv_fan_read_string(struct dell_wmi_ddv_data *data, int chan
 	if (channel >= data->fans.entries)
 		return -ENXIO;
 
-	entry = (struct fan_sensor_entry *)data->fans.obj->package.elements[1].buffer.pointer;
+	entry = (struct fan_sensor_entry *)data->fans.buffer->raw_data;
 	type = entry[channel].type;
 	switch (type) {
 	case 0x00 ... 0x07:
@@ -442,7 +462,7 @@ static int dell_wmi_ddv_temp_read_string(struct dell_wmi_ddv_data *data, int cha
 	if (channel >= data->temps.entries)
 		return -ENXIO;
 
-	entry = (struct thermal_sensor_entry *)data->temps.obj->package.elements[1].buffer.pointer;
+	entry = (struct thermal_sensor_entry *)data->temps.buffer->raw_data;
 	switch (entry[channel].type) {
 	case 0x00:
 		*str = "CPU";
@@ -553,8 +573,8 @@ static void dell_wmi_ddv_hwmon_cache_invalidate(struct dell_wmi_ddv_sensors *sen
 		return;
 
 	mutex_lock(&sensors->lock);
-	kfree(sensors->obj);
-	sensors->obj = NULL;
+	kfree(sensors->buffer);
+	sensors->buffer = NULL;
 	mutex_unlock(&sensors->lock);
 }
 
@@ -564,7 +584,7 @@ static void dell_wmi_ddv_hwmon_cache_destroy(void *data)
 
 	sensors->active = false;
 	mutex_destroy(&sensors->lock);
-	kfree(sensors->obj);
+	kfree(sensors->buffer);
 }
 
 static struct hwmon_channel_info *dell_wmi_ddv_channel_init(struct wmi_device *wdev,
@@ -750,7 +770,7 @@ static void dell_wmi_battery_invalidate(struct dell_wmi_ddv_data *data,
 static ssize_t eppid_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct dell_wmi_ddv_data *data = container_of(attr, struct dell_wmi_ddv_data, eppid_attr);
-	union acpi_object *obj;
+	ssize_t count;
 	u32 index;
 	int ret;
 
@@ -758,19 +778,19 @@ static ssize_t eppid_show(struct device *dev, struct device_attribute *attr, cha
 	if (ret < 0)
 		return ret;
 
-	ret = dell_wmi_ddv_query_string(data->wdev, DELL_DDV_BATTERY_EPPID, index, &obj);
-	if (ret < 0)
-		return ret;
-
-	if (obj->string.length != DELL_EPPID_LENGTH && obj->string.length != DELL_EPPID_EXT_LENGTH)
-		dev_info_once(&data->wdev->dev, FW_INFO "Suspicious ePPID length (%d)\n",
-			      obj->string.length);
+	count = dell_wmi_ddv_query_string(data->wdev, DELL_DDV_BATTERY_EPPID, index, buf,
+					  PAGE_SIZE);
+	if (count < 0)
+		return count;
 
-	ret = sysfs_emit(buf, "%s\n", obj->string.pointer);
+	if (count != DELL_EPPID_LENGTH && count != DELL_EPPID_EXT_LENGTH)
+		dev_info_once(&data->wdev->dev, FW_INFO "Suspicious ePPID length (%zd)\n", count);
 
-	kfree(obj);
+	ret = sysfs_emit_at(buf, count, "\n");
+	if (ret < 0)
+		return ret;
 
-	return ret;
+	return count + ret;
 }
 
 static int dell_wmi_ddv_get_health(struct dell_wmi_ddv_data *data, u32 index,
@@ -994,19 +1014,15 @@ static int dell_wmi_ddv_buffer_read(struct seq_file *seq, enum dell_ddv_method m
 {
 	struct device *dev = seq->private;
 	struct dell_wmi_ddv_data *data = dev_get_drvdata(dev);
-	union acpi_object *obj;
-	u64 size;
-	u8 *buf;
+	struct dell_wmi_buffer *buffer;
 	int ret;
 
-	ret = dell_wmi_ddv_query_buffer(data->wdev, method, 0, &obj);
+	ret = dell_wmi_ddv_query_buffer(data->wdev, method, 0, &buffer);
 	if (ret < 0)
 		return ret;
 
-	size = obj->package.elements[0].integer.value;
-	buf = obj->package.elements[1].buffer.pointer;
-	ret = seq_write(seq, buf, size);
-	kfree(obj);
+	ret = seq_write(seq, buffer->raw_data, le32_to_cpu(buffer->raw_size));
+	kfree(buffer);
 
 	return ret;
 }
-- 
2.39.5

[PATCH v3 6/9] hwmon: (dell-smm) Use new buffer-based WMI API

[Armin Wolf]

Use the new buffer-based WMI API to also support ACPI firmware
implementations that do not use ACPI buffers for returning the
results of a SMM call.

Acked-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/hwmon/dell-smm-hwmon.c | 47 ++++++++++++----------------------
 1 file changed, 16 insertions(+), 31 deletions(-)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 038edffc1ac7..82aa626fc4b8 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -12,8 +12,9 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/acpi.h>
+#include <linux/align.h>
 #include <linux/capability.h>
+#include <linux/compiler_attributes.h>
 #include <linux/cpu.h>
 #include <linux/ctype.h>
 #include <linux/delay.h>
@@ -36,10 +37,10 @@
 #include <linux/thermal.h>
 #include <linux/types.h>
 #include <linux/uaccess.h>
+#include <linux/unaligned.h>
 #include <linux/wmi.h>
 
 #include <linux/i8k.h>
-#include <linux/unaligned.h>
 
 #define I8K_SMM_FN_STATUS	0x0025
 #define I8K_SMM_POWER_STATUS	0x0069
@@ -232,7 +233,7 @@ static const struct dell_smm_ops i8k_smm_ops = {
 /*
  * Call the System Management Mode BIOS over WMI.
  */
-static ssize_t wmi_parse_register(u8 *buffer, u32 length, unsigned int *reg)
+static ssize_t wmi_parse_register(u8 *buffer, size_t length, unsigned int *reg)
 {
 	__le32 value;
 	u32 reg_size;
@@ -253,7 +254,7 @@ static ssize_t wmi_parse_register(u8 *buffer, u32 length, unsigned int *reg)
 	return reg_size + sizeof(reg_size);
 }
 
-static int wmi_parse_response(u8 *buffer, u32 length, struct smm_regs *regs)
+static int wmi_parse_response(u8 *buffer, size_t length, struct smm_regs *regs)
 {
 	unsigned int *registers[] = {
 		&regs->eax,
@@ -261,7 +262,7 @@ static int wmi_parse_response(u8 *buffer, u32 length, struct smm_regs *regs)
 		&regs->ecx,
 		&regs->edx
 	};
-	u32 offset = 0;
+	size_t offset = 0;
 	ssize_t ret;
 	int i;
 
@@ -273,19 +274,16 @@ static int wmi_parse_response(u8 *buffer, u32 length, struct smm_regs *regs)
 		if (ret < 0)
 			return ret;
 
-		offset += ret;
+		/* WMI aligns u32 integers on a 4 byte boundary */
+		offset = ALIGN(offset + ret, 4);
 	}
 
-	if (offset != length)
-		return -ENOMSG;
-
 	return 0;
 }
 
 static int wmi_smm_call(struct device *dev, struct smm_regs *regs)
 {
 	struct wmi_device *wdev = container_of(dev, struct wmi_device, dev);
-	struct acpi_buffer out = { ACPI_ALLOCATE_BUFFER, NULL };
 	u32 wmi_payload[] = {
 		sizeof(regs->eax),
 		regs->eax,
@@ -296,32 +294,19 @@ static int wmi_smm_call(struct device *dev, struct smm_regs *regs)
 		sizeof(regs->edx),
 		regs->edx
 	};
-	const struct acpi_buffer in = {
+	const struct wmi_buffer in = {
 		.length = sizeof(wmi_payload),
-		.pointer = &wmi_payload,
+		.data = &wmi_payload,
 	};
-	union acpi_object *obj;
-	acpi_status status;
+	struct wmi_buffer out;
 	int ret;
 
-	status = wmidev_evaluate_method(wdev, 0x0, DELL_SMM_LEGACY_EXECUTE, &in, &out);
-	if (ACPI_FAILURE(status))
-		return -EIO;
-
-	obj = out.pointer;
-	if (!obj)
-		return -ENODATA;
-
-	if (obj->type != ACPI_TYPE_BUFFER) {
-		ret = -ENOMSG;
-
-		goto err_free;
-	}
-
-	ret = wmi_parse_response(obj->buffer.pointer, obj->buffer.length, regs);
+	ret = wmidev_invoke_method(wdev, 0x0, DELL_SMM_LEGACY_EXECUTE, &in, &out);
+	if (ret < 0)
+		return ret;
 
-err_free:
-	kfree(obj);
+	ret = wmi_parse_response(out.data, out.length,  regs);
+	kfree(out.data);
 
 	return ret;
 }
-- 
2.39.5

[PATCH v3 7/9] platform/wmi: Make wmi_bus_class const

[Armin Wolf]

The functions class_register()/_unregister() and device_create()
both support taking a const pointer to the class struct. Use this
to mark wmi_bus_class as const so that it can be placed into
read-only memory for better security.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/wmi/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/wmi/core.c b/drivers/platform/wmi/core.c
index b8e6b9a421c6..082c85625878 100644
--- a/drivers/platform/wmi/core.c
+++ b/drivers/platform/wmi/core.c
@@ -1069,7 +1069,7 @@ static void wmi_dev_shutdown(struct device *dev)
 	}
 }
 
-static struct class wmi_bus_class = {
+static const struct class wmi_bus_class = {
 	.name = "wmi_bus",
 };
 
-- 
2.39.5

[PATCH v3 8/9] platform/wmi: Make sysfs attributes const

[Armin Wolf]

The sysfs core supports const attributes. Use this to mark all
sysfs attributes as const so that they can be placed into read-only
memory for better security.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/wmi/core.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/wmi/core.c b/drivers/platform/wmi/core.c
index 082c85625878..1b5bb3410252 100644
--- a/drivers/platform/wmi/core.c
+++ b/drivers/platform/wmi/core.c
@@ -812,7 +812,8 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
 
 	return sysfs_emit(buf, "wmi:%pUL\n", &wblock->gblock.guid);
 }
-static DEVICE_ATTR_RO(modalias);
+
+static const DEVICE_ATTR_RO(modalias);
 
 static ssize_t guid_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
@@ -821,7 +822,8 @@ static ssize_t guid_show(struct device *dev, struct device_attribute *attr,
 
 	return sysfs_emit(buf, "%pUL\n", &wblock->gblock.guid);
 }
-static DEVICE_ATTR_RO(guid);
+
+static const DEVICE_ATTR_RO(guid);
 
 static ssize_t instance_count_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
@@ -830,7 +832,8 @@ static ssize_t instance_count_show(struct device *dev,
 
 	return sysfs_emit(buf, "%d\n", (int)wblock->gblock.instance_count);
 }
-static DEVICE_ATTR_RO(instance_count);
+
+static const DEVICE_ATTR_RO(instance_count);
 
 static ssize_t expensive_show(struct device *dev,
 			      struct device_attribute *attr, char *buf)
@@ -840,7 +843,8 @@ static ssize_t expensive_show(struct device *dev,
 	return sysfs_emit(buf, "%d\n",
 			  (wblock->gblock.flags & ACPI_WMI_EXPENSIVE) != 0);
 }
-static DEVICE_ATTR_RO(expensive);
+
+static const DEVICE_ATTR_RO(expensive);
 
 static ssize_t driver_override_show(struct device *dev, struct device_attribute *attr,
 				    char *buf)
@@ -867,9 +871,10 @@ static ssize_t driver_override_store(struct device *dev, struct device_attribute
 
 	return count;
 }
-static DEVICE_ATTR_RW(driver_override);
 
-static struct attribute *wmi_attrs[] = {
+static const DEVICE_ATTR_RW(driver_override);
+
+static const struct attribute * const wmi_attrs[] = {
 	&dev_attr_modalias.attr,
 	&dev_attr_guid.attr,
 	&dev_attr_instance_count.attr,
@@ -886,9 +891,10 @@ static ssize_t notify_id_show(struct device *dev, struct device_attribute *attr,
 
 	return sysfs_emit(buf, "%02X\n", (unsigned int)wblock->gblock.notify_id);
 }
-static DEVICE_ATTR_RO(notify_id);
 
-static struct attribute *wmi_event_attrs[] = {
+static const DEVICE_ATTR_RO(notify_id);
+
+static const struct attribute * const wmi_event_attrs[] = {
 	&dev_attr_notify_id.attr,
 	NULL
 };
@@ -902,7 +908,8 @@ static ssize_t object_id_show(struct device *dev, struct device_attribute *attr,
 	return sysfs_emit(buf, "%c%c\n", wblock->gblock.object_id[0],
 			  wblock->gblock.object_id[1]);
 }
-static DEVICE_ATTR_RO(object_id);
+
+static const DEVICE_ATTR_RO(object_id);
 
 static ssize_t setable_show(struct device *dev, struct device_attribute *attr,
 			    char *buf)
@@ -911,16 +918,17 @@ static ssize_t setable_show(struct device *dev, struct device_attribute *attr,
 
 	return sysfs_emit(buf, "%d\n", (int)wdev->setable);
 }
-static DEVICE_ATTR_RO(setable);
 
-static struct attribute *wmi_data_attrs[] = {
+static const DEVICE_ATTR_RO(setable);
+
+static const struct attribute * const wmi_data_attrs[] = {
 	&dev_attr_object_id.attr,
 	&dev_attr_setable.attr,
 	NULL
 };
 ATTRIBUTE_GROUPS(wmi_data);
 
-static struct attribute *wmi_method_attrs[] = {
+static const struct attribute * const wmi_method_attrs[] = {
 	&dev_attr_object_id.attr,
 	NULL
 };
-- 
2.39.5

[PATCH v3 9/9] modpost: Handle malformed WMI GUID strings

[Armin Wolf]

Some WMI GUIDs found inside binary MOF files contain both
uppercase and lowercase characters. Blindly copying such
GUIDs will prevent the associated WMI driver from loading
automatically because the WMI GUID found inside WMI device ids
always contains uppercase characters.

Avoid this issue by always converting WMI GUID strings to
uppercase. Also verify that the WMI GUID string actually looks
like a valid GUID.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 .../wmi/driver-development-guide.rst          |  2 +-
 scripts/mod/file2alias.c                      | 28 ++++++++++++++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/Documentation/wmi/driver-development-guide.rst b/Documentation/wmi/driver-development-guide.rst
index fbc2d9b12fe9..74bb156ad9cc 100644
--- a/Documentation/wmi/driver-development-guide.rst
+++ b/Documentation/wmi/driver-development-guide.rst
@@ -54,7 +54,7 @@ to matching WMI devices using a struct wmi_device_id table:
 ::
 
   static const struct wmi_device_id foo_id_table[] = {
-         /* Only use uppercase letters! */
+         /* Using only uppercase letters is recommended */
          { "936DA01F-9ABD-4D9D-80C7-02AF85C822A8", NULL },
          { }
   };
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 4e99393a35f1..20e542a888c4 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1253,6 +1253,8 @@ static void do_tee_entry(struct module *mod, void *symval)
 static void do_wmi_entry(struct module *mod, void *symval)
 {
 	DEF_FIELD_ADDR(symval, wmi_device_id, guid_string);
+	char result[sizeof(*guid_string)];
+	int i;
 
 	if (strlen(*guid_string) != UUID_STRING_LEN) {
 		warn("Invalid WMI device id 'wmi:%s' in '%s'\n",
@@ -1260,7 +1262,31 @@ static void do_wmi_entry(struct module *mod, void *symval)
 		return;
 	}
 
-	module_alias_printf(mod, false, WMI_MODULE_PREFIX "%s", *guid_string);
+	for (i = 0; i < UUID_STRING_LEN; i++) {
+		char value = (*guid_string)[i];
+		bool valid = false;
+
+		if (i == 8 || i == 13 || i == 18 || i == 23) {
+			if (value == '-')
+				valid = true;
+		} else {
+			if (isxdigit(value))
+				valid = true;
+		}
+
+		if (!valid) {
+			warn("Invalid character %c inside WMI GUID string '%s' in '%s'\n",
+			     value, *guid_string, mod->name);
+			return;
+		}
+
+		/* Some GUIDs from BMOF definitions contain lowercase characters */
+		result[i] = toupper(value);
+	}
+
+	result[i] = '\0';
+
+	module_alias_printf(mod, false, WMI_MODULE_PREFIX "%s", result);
 }
 
 /* Looks like: mhi:S */
-- 
2.39.5

Re: [PATCH v3 0/9] Convert most Dell WMI drivers to use the new buffer-based API

[Mario Limonciello]

On 3/14/26 12:52, Armin Wolf wrote:
> The new buffer-based WMI API improves the compatibility between
> different ACPI firmware implementations by performing marshalling/
> unmarshalling of WMI buffers like the original Windows driver.
> 
> Convert most Dell WMI drivers to use this new API. This also
> removes all ACPI-related code from those drivers because the
> new buffer-based WMI API uses abstract WMI buffer objects instead
> of ACPI objects.
> 
> All drivers have been tested on a Dell Inspiron 3505 and appear
> to work normally.
> 
> The last three patches contain some misc. cleanups for the WMI
> driver core itself. The most important change is a fix for modprobe
> to verify any WMI GUID strings from WMI drivers and convert them
> to uppercase if necessary. This should fix autoloading for WMI
> drivers that use WMI GUID strings with lowercase letters.
> 
> Changes since v2:
> - add Acked-by tag from Guenter
> - rework buffer parsing in dell-wmi-descriptor as suggested by Gergo
> 
> Changes since v1:
> - do not perform pointer arithmetic on void pointers in dell-smm-hwmon
> 
> Armin Wolf (9):
>    platform/x86: dell-descriptor: Use new buffer-based WMI API
>    platform/x86: dell-privacy: Use new buffer-based WMI API
>    platform/x86: dell-smbios-wmi: Use new buffer-based WMI API
>    platform/x86: dell-wmi-base: Use new buffer-based WMI API
>    platform/x86: dell-ddv: Use new buffer-based WMI API
>    hwmon: (dell-smm) Use new buffer-based WMI API
>    platform/wmi: Make wmi_bus_class const
>    platform/wmi: Make sysfs attributes const
>    modpost: Handle malformed WMI GUID strings
> 
>   .../wmi/driver-development-guide.rst          |   2 +-
>   drivers/hwmon/dell-smm-hwmon.c                |  47 ++---
>   drivers/platform/wmi/core.c                   |  34 +--
>   drivers/platform/x86/dell/dell-smbios-wmi.c   |  46 +++--
>   drivers/platform/x86/dell/dell-wmi-base.c     |  68 +++---
>   drivers/platform/x86/dell/dell-wmi-ddv.c      | 194 ++++++++++--------
>   .../platform/x86/dell/dell-wmi-descriptor.c   |  94 ++++-----
>   drivers/platform/x86/dell/dell-wmi-privacy.c  |  78 ++++---
>   scripts/mod/file2alias.c                      |  28 ++-
>   9 files changed, 315 insertions(+), 276 deletions(-)
> 

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

Re: [PATCH v3 0/9] Convert most Dell WMI drivers to use the new buffer-based API

[Guenter Roeck]

On 3/14/26 10:52, Armin Wolf wrote:
> The new buffer-based WMI API improves the compatibility between
> different ACPI firmware implementations by performing marshalling/
> unmarshalling of WMI buffers like the original Windows driver.
> 
> Convert most Dell WMI drivers to use this new API. This also
> removes all ACPI-related code from those drivers because the
> new buffer-based WMI API uses abstract WMI buffer objects instead
> of ACPI objects.
> 
> All drivers have been tested on a Dell Inspiron 3505 and appear
> to work normally.
> 
> The last three patches contain some misc. cleanups for the WMI
> driver core itself. The most important change is a fix for modprobe
> to verify any WMI GUID strings from WMI drivers and convert them
> to uppercase if necessary. This should fix autoloading for WMI
> drivers that use WMI GUID strings with lowercase letters.
> 

The series doesn't give hints to maintainers if the patches should be
applied on a per-subsystem basis (i.e., are independent of each other)
or if they need to be applied together. I Acked the hwmon patch, assuming
that it will be applied through a common branch. If the patches are
supposed to be applied in subsystem branches, please let me know.

Thanks,
Guenter

Re: [PATCH v3 0/9] Convert most Dell WMI drivers to use the new buffer-based API

[Armin Wolf]

Am 17.03.26 um 16:52 schrieb Guenter Roeck:

> On 3/14/26 10:52, Armin Wolf wrote:
>> The new buffer-based WMI API improves the compatibility between
>> different ACPI firmware implementations by performing marshalling/
>> unmarshalling of WMI buffers like the original Windows driver.
>>
>> Convert most Dell WMI drivers to use this new API. This also
>> removes all ACPI-related code from those drivers because the
>> new buffer-based WMI API uses abstract WMI buffer objects instead
>> of ACPI objects.
>>
>> All drivers have been tested on a Dell Inspiron 3505 and appear
>> to work normally.
>>
>> The last three patches contain some misc. cleanups for the WMI
>> driver core itself. The most important change is a fix for modprobe
>> to verify any WMI GUID strings from WMI drivers and convert them
>> to uppercase if necessary. This should fix autoloading for WMI
>> drivers that use WMI GUID strings with lowercase letters.
>>
>
> The series doesn't give hints to maintainers if the patches should be
> applied on a per-subsystem basis (i.e., are independent of each other)
> or if they need to be applied together. I Acked the hwmon patch, assuming
> that it will be applied through a common branch. If the patches are
> supposed to be applied in subsystem branches, please let me know.
>
> Thanks,
> Guenter 

I would prefer if the whole series was applied together on the platform drivers tree.

Thanks,
Armin Wolf

Re: [PATCH v3 0/9] Convert most Dell WMI drivers to use the new buffer-based API

[Guenter Roeck]

On 3/17/26 13:11, Armin Wolf wrote:
> Am 17.03.26 um 16:52 schrieb Guenter Roeck:
> 
>> On 3/14/26 10:52, Armin Wolf wrote:
>>> The new buffer-based WMI API improves the compatibility between
>>> different ACPI firmware implementations by performing marshalling/
>>> unmarshalling of WMI buffers like the original Windows driver.
>>>
>>> Convert most Dell WMI drivers to use this new API. This also
>>> removes all ACPI-related code from those drivers because the
>>> new buffer-based WMI API uses abstract WMI buffer objects instead
>>> of ACPI objects.
>>>
>>> All drivers have been tested on a Dell Inspiron 3505 and appear
>>> to work normally.
>>>
>>> The last three patches contain some misc. cleanups for the WMI
>>> driver core itself. The most important change is a fix for modprobe
>>> to verify any WMI GUID strings from WMI drivers and convert them
>>> to uppercase if necessary. This should fix autoloading for WMI
>>> drivers that use WMI GUID strings with lowercase letters.
>>>
>>
>> The series doesn't give hints to maintainers if the patches should be
>> applied on a per-subsystem basis (i.e., are independent of each other)
>> or if they need to be applied together. I Acked the hwmon patch, assuming
>> that it will be applied through a common branch. If the patches are
>> supposed to be applied in subsystem branches, please let me know.
>>
>> Thanks,
>> Guenter 
> 
> I would prefer if the whole series was applied together on the platform drivers tree.
> 

Thanks for the clarification.

Guenter

Re: [PATCH v3 0/9] Convert most Dell WMI drivers to use the new buffer-based API

[Armin Wolf]

Am 17.03.26 um 21:22 schrieb Guenter Roeck:
> On 3/17/26 13:11, Armin Wolf wrote:
>> Am 17.03.26 um 16:52 schrieb Guenter Roeck:
>>
>>> On 3/14/26 10:52, Armin Wolf wrote:
>>>> The new buffer-based WMI API improves the compatibility between
>>>> different ACPI firmware implementations by performing marshalling/
>>>> unmarshalling of WMI buffers like the original Windows driver.
>>>>
>>>> Convert most Dell WMI drivers to use this new API. This also
>>>> removes all ACPI-related code from those drivers because the
>>>> new buffer-based WMI API uses abstract WMI buffer objects instead
>>>> of ACPI objects.
>>>>
>>>> All drivers have been tested on a Dell Inspiron 3505 and appear
>>>> to work normally.
>>>>
>>>> The last three patches contain some misc. cleanups for the WMI
>>>> driver core itself. The most important change is a fix for modprobe
>>>> to verify any WMI GUID strings from WMI drivers and convert them
>>>> to uppercase if necessary. This should fix autoloading for WMI
>>>> drivers that use WMI GUID strings with lowercase letters.
>>>>
>>>
>>> The series doesn't give hints to maintainers if the patches should be
>>> applied on a per-subsystem basis (i.e., are independent of each other)
>>> or if they need to be applied together. I Acked the hwmon patch, 
>>> assuming
>>> that it will be applied through a common branch. If the patches are
>>> supposed to be applied in subsystem branches, please let me know.
>>>
>>> Thanks,
>>> Guenter 
>>
>> I would prefer if the whole series was applied together on the 
>> platform drivers tree.
>>
> 
> Thanks for the clarification.
> 
> Guenter
>

Is there still something to do for getting this series merged?

Thanks,
Armin Wolf

Re: [PATCH v3 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API

[Ilpo Järvinen]

On Sat, 14 Mar 2026, Armin Wolf wrote:

> Use the new buffer-based WMI API to also support ACPI firmware
> implementations that do not use ACPI buffers for the descriptor.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  .../platform/x86/dell/dell-wmi-descriptor.c   | 94 +++++++++----------
>  1 file changed, 47 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-wmi-descriptor.c b/drivers/platform/x86/dell/dell-wmi-descriptor.c
> index c2a180202719..fe42eb8bbd79 100644
> --- a/drivers/platform/x86/dell/dell-wmi-descriptor.c
> +++ b/drivers/platform/x86/dell/dell-wmi-descriptor.c
> @@ -7,7 +7,7 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> -#include <linux/acpi.h>
> +#include <linux/compiler_attributes.h>
>  #include <linux/list.h>
>  #include <linux/module.h>
>  #include <linux/wmi.h>
> @@ -15,6 +15,24 @@
>  
>  #define DELL_WMI_DESCRIPTOR_GUID "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
>  
> +/*
> + * Descriptor buffer is 128 byte long and contains:
> + *
> + *       Name             Offset  Length  Value
> + * Vendor Signature          0       4    "DELL"
> + * Object Signature          4       4    " WMI"
> + * WMI Interface Version     8       4    <version>
> + * WMI buffer length        12       4    <length>
> + * WMI hotfix number        16       4    <hotfix>
> + */
> +struct descriptor {
> +	char vendor_signature[4];
> +	char object_signature[4];
> +	__le32 interface_version;
> +	__le32 buffer_length;
> +	__le32 hotfix_number;
> +} __packed;
> +
>  struct descriptor_priv {
>  	struct list_head list;
>  	u32 interface_version;
> @@ -88,76 +106,58 @@ bool dell_wmi_get_hotfix(u32 *hotfix)
>  }
>  EXPORT_SYMBOL_GPL(dell_wmi_get_hotfix);
>  
> -/*
> - * Descriptor buffer is 128 byte long and contains:
> - *
> - *       Name             Offset  Length  Value
> - * Vendor Signature          0       4    "DELL"
> - * Object Signature          4       4    " WMI"
> - * WMI Interface Version     8       4    <version>
> - * WMI buffer length        12       4    <length>
> - * WMI hotfix number        16       4    <hotfix>
> - */
> -static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
> -				     const void *context)
> +static int dell_wmi_descriptor_probe(struct wmi_device *wdev, const void *context)
>  {
> -	union acpi_object *obj = NULL;
>  	struct descriptor_priv *priv;
> -	u32 *buffer;
> +	struct wmi_buffer buffer;
> +	struct descriptor *desc;
>  	int ret;
>  
> -	obj = wmidev_block_query(wdev, 0);
> -	if (!obj) {
> -		dev_err(&wdev->dev, "failed to read Dell WMI descriptor\n");
> -		ret = -EIO;
> -		goto out;
> -	}
> +	ret = wmidev_query_block(wdev, 0, &buffer);
> +	if (ret < 0)
> +		return ret;
>  
> -	if (obj->type != ACPI_TYPE_BUFFER) {
> -		dev_err(&wdev->dev, "Dell descriptor has wrong type\n");
> +	if (buffer.length < sizeof(*desc)) {
> +		dev_err(&wdev->dev,
> +			"Dell descriptor buffer contains not enough data (%zu)\n",

This word order sounds odd to me. I'd say:

Dell descriptor buffer does not contain ...

> +			buffer.length);
>  		ret = -EINVAL;
>  		descriptor_valid = ret;
>  		goto out;
>  	}
>  
> -	/* Although it's not technically a failure, this would lead to
> -	 * unexpected behavior
> -	 */
> -	if (obj->buffer.length != 128) {
> -		dev_err(&wdev->dev,
> -			"Dell descriptor buffer has unexpected length (%d)\n",
> -			obj->buffer.length);
> -		ret = -EINVAL;
> +	desc = buffer.data;

I think these should be moving towards using __free() so we could remove 
those gotos to kfree(). This applies to many changes in this series. Some 
may need a local variable to be introduced (here we've 'desc' already so 
it's very easy in this case).

I'm fine if you want to do __free() changes in a patch separate from 
this wmi conversion but we should aim to make these follow best practices 
so that others pick the preferred patterns more easily.

> +	if (strncmp(desc->vendor_signature, "DELL", sizeof(desc->vendor_signature))) {
> +		dev_err(&wdev->dev, "Dell descriptor buffer has invalid vendor signature (%4ph)\n",
> +			desc->vendor_signature);
> +		ret = -ENOMSG;
>  		descriptor_valid = ret;
>  		goto out;
>  	}
>  
> -	buffer = (u32 *)obj->buffer.pointer;
> -
> -	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0) {
> -		dev_err(&wdev->dev, "Dell descriptor buffer has invalid signature (%8ph)\n",
> -			buffer);
> -		ret = -EINVAL;
> +	if (strncmp(desc->object_signature, " WMI", sizeof(desc->vendor_signature))) {
> +		dev_err(&wdev->dev, "Dell descriptor buffer has invalid object signature (%4ph)\n",
> +			desc->object_signature);
> +		ret = -ENOMSG;
>  		descriptor_valid = ret;
>  		goto out;
>  	}
>  	descriptor_valid = 0;
>  
> -	if (buffer[2] != 0 && buffer[2] != 1)
> -		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%lu)\n",
> -			(unsigned long) buffer[2]);
> -
> -	priv = devm_kzalloc(&wdev->dev, sizeof(struct descriptor_priv),
> -	GFP_KERNEL);
> +	if (le32_to_cpu(desc->interface_version) > 2)
> +		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%u)\n",
> +			 le32_to_cpu(desc->interface_version));
>  
> +	priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
>  	if (!priv) {
>  		ret = -ENOMEM;
>  		goto out;
>  	}
>  
> -	priv->interface_version = buffer[2];
> -	priv->size = buffer[3];
> -	priv->hotfix = buffer[4];
> +	priv->interface_version = le32_to_cpu(desc->interface_version);
> +	priv->size = le32_to_cpu(desc->buffer_length);
> +	priv->hotfix = le32_to_cpu(desc->hotfix_number);
>  	ret = 0;
>  	dev_set_drvdata(&wdev->dev, priv);
>  	mutex_lock(&list_mutex);
> @@ -170,7 +170,7 @@ static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
>  		(unsigned long) priv->hotfix);
>  
>  out:
> -	kfree(obj);
> +	kfree(buffer.data);
>  	return ret;
>  }
>  
> 

-- 
 i.

Re: [PATCH v3 2/9] platform/x86: dell-privacy: Use new buffer-based WMI API

[Ilpo Järvinen]

On Sat, 14 Mar 2026, Armin Wolf wrote:

> Use the new buffer-based WMI API to also support ACPI firmware
> implementations that do not use ACPI buffers for the device state.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/platform/x86/dell/dell-wmi-privacy.c | 78 ++++++++++----------
>  1 file changed, 38 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-wmi-privacy.c b/drivers/platform/x86/dell/dell-wmi-privacy.c
> index ed099a431ea4..470273cc2fd2 100644
> --- a/drivers/platform/x86/dell/dell-wmi-privacy.c
> +++ b/drivers/platform/x86/dell/dell-wmi-privacy.c
> @@ -9,6 +9,7 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/bitops.h>
> +#include <linux/compiler_attributes.h>
>  #include <linux/input.h>
>  #include <linux/input/sparse-keymap.h>
>  #include <linux/list.h>
> @@ -25,6 +26,26 @@
>  #define DELL_PRIVACY_CAMERA_EVENT 0x2
>  #define led_to_priv(c)       container_of(c, struct privacy_wmi_data, cdev)
>  
> +/*
> + * Describes the Device State class exposed by BIOS which can be consumed by
> + * various applications interested in knowing the Privacy feature capabilities.
> + * class DeviceState
> + * {
> + *  [key, read] string InstanceName;
> + *  [read] boolean ReadOnly;
> + *
> + *  [WmiDataId(1), read] uint32 DevicesSupported;
> + *   0 - None; 0x1 - Microphone; 0x2 - Camera; 0x4 - ePrivacy  Screen
> + *
> + *  [WmiDataId(2), read] uint32 CurrentState;
> + *   0 - Off; 1 - On; Bit0 - Microphone; Bit1 - Camera; Bit2 - ePrivacyScreen
> + * };
> + */
> +struct device_state {
> +	__le32 devices_supported;
> +	__le32 current_state;
> +} __packed;
> +
>  /*
>   * The wmi_list is used to store the privacy_priv struct with mutex protecting
>   */
> @@ -185,59 +206,36 @@ static struct attribute *privacy_attrs[] = {
>  };
>  ATTRIBUTE_GROUPS(privacy);
>  
> -/*
> - * Describes the Device State class exposed by BIOS which can be consumed by
> - * various applications interested in knowing the Privacy feature capabilities.
> - * class DeviceState
> - * {
> - *  [key, read] string InstanceName;
> - *  [read] boolean ReadOnly;
> - *
> - *  [WmiDataId(1), read] uint32 DevicesSupported;
> - *   0 - None; 0x1 - Microphone; 0x2 - Camera; 0x4 - ePrivacy  Screen
> - *
> - *  [WmiDataId(2), read] uint32 CurrentState;
> - *   0 - Off; 1 - On; Bit0 - Microphone; Bit1 - Camera; Bit2 - ePrivacyScreen
> - * };
> - */
>  static int get_current_status(struct wmi_device *wdev)
>  {
>  	struct privacy_wmi_data *priv = dev_get_drvdata(&wdev->dev);
> -	union acpi_object *obj_present;
> -	u32 *buffer;
> -	int ret = 0;
> +	struct device_state *state;
> +	struct wmi_buffer buffer;
> +	int ret;
>  
>  	if (!priv) {
>  		dev_err(&wdev->dev, "dell privacy priv is NULL\n");
>  		return -EINVAL;
>  	}
> +
>  	/* check privacy support features and device states */
> -	obj_present = wmidev_block_query(wdev, 0);
> -	if (!obj_present) {
> -		dev_err(&wdev->dev, "failed to read Binary MOF\n");
> -		return -EIO;
> -	}
> +	ret = wmidev_query_block(wdev, 0, &buffer);
> +	if (ret < 0)
> +		return ret;
>  
> -	if (obj_present->type != ACPI_TYPE_BUFFER) {
> -		dev_err(&wdev->dev, "Binary MOF is not a buffer!\n");
> -		ret = -EIO;
> -		goto obj_free;
> -	}
> -	/*  Although it's not technically a failure, this would lead to
> -	 *  unexpected behavior
> -	 */
> -	if (obj_present->buffer.length != 8) {
> -		dev_err(&wdev->dev, "Dell privacy buffer has unexpected length (%d)!\n",
> -				obj_present->buffer.length);
> +	if (buffer.length < sizeof(*state)) {
> +		dev_err(&wdev->dev, "Dell privacy buffer contains not enough data (%zu)!\n",

Same comment here as with the other patch, perhaps in other patches as 
well if it's copy pasted so please check the rest, I won't comment on 
this with the other patches.

> +			buffer.length);
>  		ret = -EINVAL;
> -		goto obj_free;
> +		goto buffer_free;
>  	}
> -	buffer = (u32 *)obj_present->buffer.pointer;
> -	priv->features_present = buffer[0];
> -	priv->last_status = buffer[1];
>  
> -obj_free:
> -	kfree(obj_present);
> +	state = buffer.data;
> +	priv->features_present = le32_to_cpu(state->devices_supported);
> +	priv->last_status = le32_to_cpu(state->current_state);
> +
> +buffer_free:
> +	kfree(buffer.data);
>  	return ret;
>  }
>  
> 

-- 
 i.

Re: [PATCH v3 4/9] platform/x86: dell-wmi-base: Use new buffer-based WMI API

[Ilpo Järvinen]

On Sat, 14 Mar 2026, Armin Wolf wrote:

> Use the new buffer-based WMI API to also support ACPI firmware
> implementations that do not use ACPI buffers for the event data.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/platform/x86/dell/dell-wmi-base.c | 68 ++++++++++++-----------
>  1 file changed, 35 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-wmi-base.c b/drivers/platform/x86/dell/dell-wmi-base.c
> index 4eefbade2f5e..4a7ab9fb3f81 100644
> --- a/drivers/platform/x86/dell/dell-wmi-base.c
> +++ b/drivers/platform/x86/dell/dell-wmi-base.c
> @@ -13,6 +13,7 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> +#include <linux/compiler_attributes.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
> @@ -408,7 +409,8 @@ static void dell_wmi_switch_event(struct input_dev **subdev,
>  	input_sync(*subdev);
>  }
>  
> -static int dell_wmi_process_key(struct wmi_device *wdev, int type, int code, u16 *buffer, int remaining)
> +static int dell_wmi_process_key(struct wmi_device *wdev, int type, int code, __le16 *buffer,
> +				int remaining)
>  {
>  	struct dell_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
>  	const struct key_entry *key;
> @@ -440,15 +442,15 @@ static int dell_wmi_process_key(struct wmi_device *wdev, int type, int code, u16
>  	} else if (type == 0x0011 && code == 0xe070 && remaining > 0) {
>  		dell_wmi_switch_event(&priv->tabletswitch_dev,
>  				      "Dell tablet mode switch",
> -				      SW_TABLET_MODE, !buffer[0]);
> +				      SW_TABLET_MODE, !le16_to_cpu(buffer[0]));

__le* comes from types.h which this file already has (but the files in 
some of the other patches might be lacking it so please check those).

Should there be some header added for le16_to_cpu()? Probably relevant to 
some other patches as well.

-- 
 i.

>  		return 1;
>  	} else if (type == 0x0012 && code == 0x000c && remaining > 0) {
>  		/* Eprivacy toggle, switch to "on" key entry for on events */
> -		if (buffer[0] == 2)
> +		if (le16_to_cpu(buffer[0]) == 2)
>  			key++;
>  		used = 1;
>  	} else if (type == 0x0012 && code == 0x000d && remaining > 0) {
> -		value = (buffer[2] == 2);
> +		value = (le16_to_cpu(buffer[2]) == 2);
>  		used = 1;
>  	}
>  
> @@ -457,24 +459,17 @@ static int dell_wmi_process_key(struct wmi_device *wdev, int type, int code, u16
>  	return used;
>  }
>  
> -static void dell_wmi_notify(struct wmi_device *wdev,
> -			    union acpi_object *obj)
> +static void dell_wmi_notify(struct wmi_device *wdev, const struct wmi_buffer *buffer)
>  {
>  	struct dell_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
> -	u16 *buffer_entry, *buffer_end;
> -	acpi_size buffer_size;
> +	__le16 *buffer_entry, *buffer_end;
> +	size_t buffer_size;
>  	int len, i;
>  
> -	if (obj->type != ACPI_TYPE_BUFFER) {
> -		pr_warn("bad response type %x\n", obj->type);
> -		return;
> -	}
> +	pr_debug("Received WMI event (%*ph)\n", (int)buffer->length, buffer->data);
>  
> -	pr_debug("Received WMI event (%*ph)\n",
> -		obj->buffer.length, obj->buffer.pointer);
> -
> -	buffer_entry = (u16 *)obj->buffer.pointer;
> -	buffer_size = obj->buffer.length/2;
> +	buffer_entry = buffer->data;
> +	buffer_size = buffer->length / 2;
>  	buffer_end = buffer_entry + buffer_size;
>  
>  	/*
> @@ -490,12 +485,12 @@ static void dell_wmi_notify(struct wmi_device *wdev,
>  	 * one event on devices with WMI interface version 0.
>  	 */
>  	if (priv->interface_version == 0 && buffer_entry < buffer_end)
> -		if (buffer_end > buffer_entry + buffer_entry[0] + 1)
> -			buffer_end = buffer_entry + buffer_entry[0] + 1;
> +		if (buffer_end > buffer_entry + le16_to_cpu(buffer_entry[0]) + 1)
> +			buffer_end = buffer_entry + le16_to_cpu(buffer_entry[0]) + 1;
>  
>  	while (buffer_entry < buffer_end) {
>  
> -		len = buffer_entry[0];
> +		len = le16_to_cpu(buffer_entry[0]);
>  		if (len == 0)
>  			break;
>  
> @@ -508,11 +503,11 @@ static void dell_wmi_notify(struct wmi_device *wdev,
>  
>  		pr_debug("Process buffer (%*ph)\n", len*2, buffer_entry);
>  
> -		switch (buffer_entry[1]) {
> +		switch (le16_to_cpu(buffer_entry[1])) {
>  		case 0x0000: /* One key pressed or event occurred */
>  			if (len > 2)
> -				dell_wmi_process_key(wdev, buffer_entry[1],
> -						     buffer_entry[2],
> +				dell_wmi_process_key(wdev, le16_to_cpu(buffer_entry[1]),
> +						     le16_to_cpu(buffer_entry[2]),
>  						     buffer_entry + 3,
>  						     len - 3);
>  			/* Extended data is currently ignored */
> @@ -520,22 +515,29 @@ static void dell_wmi_notify(struct wmi_device *wdev,
>  		case 0x0010: /* Sequence of keys pressed */
>  		case 0x0011: /* Sequence of events occurred */
>  			for (i = 2; i < len; ++i)
> -				i += dell_wmi_process_key(wdev, buffer_entry[1],
> -							  buffer_entry[i],
> +				i += dell_wmi_process_key(wdev, le16_to_cpu(buffer_entry[1]),
> +							  le16_to_cpu(buffer_entry[i]),
>  							  buffer_entry + i,
>  							  len - i - 1);
>  			break;
>  		case 0x0012:
> -			if ((len > 4) && dell_privacy_process_event(buffer_entry[1], buffer_entry[3],
> -								    buffer_entry[4]))
> -				/* dell_privacy_process_event has handled the event */;
> -			else if (len > 2)
> -				dell_wmi_process_key(wdev, buffer_entry[1], buffer_entry[2],
> +			if (len > 4) {
> +				if (dell_privacy_process_event(le16_to_cpu(buffer_entry[1]),
> +							       le16_to_cpu(buffer_entry[3]),
> +							       le16_to_cpu(buffer_entry[4])))
> +					break;
> +			}
> +
> +			/* dell_privacy_process_event has not handled the event */
> +
> +			if (len > 2)
> +				dell_wmi_process_key(wdev, le16_to_cpu(buffer_entry[1]),
> +						     le16_to_cpu(buffer_entry[2]),
>  						     buffer_entry + 3, len - 3);
> +
>  			break;
>  		default: /* Unknown event */
> -			pr_info("Unknown WMI event type 0x%x\n",
> -				(int)buffer_entry[1]);
> +			pr_info("Unknown WMI event type 0x%x\n", le16_to_cpu(buffer_entry[1]));
>  			break;
>  		}
>  
> @@ -821,7 +823,7 @@ static struct wmi_driver dell_wmi_driver = {
>  	.id_table = dell_wmi_id_table,
>  	.probe = dell_wmi_probe,
>  	.remove = dell_wmi_remove,
> -	.notify = dell_wmi_notify,
> +	.notify_new = dell_wmi_notify,
>  };
>  
>  static int __init dell_wmi_init(void)
> 

Re: [PATCH v3 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API

[Ilpo Järvinen]

On Sat, 14 Mar 2026, Armin Wolf wrote:

> Use the new buffer-based WMI API to also support ACPI firmware
> implementations that do not use ACPI buffers for the descriptor.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  .../platform/x86/dell/dell-wmi-descriptor.c   | 94 +++++++++----------
>  1 file changed, 47 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-wmi-descriptor.c b/drivers/platform/x86/dell/dell-wmi-descriptor.c
> index c2a180202719..fe42eb8bbd79 100644
> --- a/drivers/platform/x86/dell/dell-wmi-descriptor.c
> +++ b/drivers/platform/x86/dell/dell-wmi-descriptor.c
> @@ -7,7 +7,7 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> -#include <linux/acpi.h>
> +#include <linux/compiler_attributes.h>
>  #include <linux/list.h>
>  #include <linux/module.h>
>  #include <linux/wmi.h>
> @@ -15,6 +15,24 @@
>  
>  #define DELL_WMI_DESCRIPTOR_GUID "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
>  
> +/*
> + * Descriptor buffer is 128 byte long and contains:
> + *
> + *       Name             Offset  Length  Value
> + * Vendor Signature          0       4    "DELL"
> + * Object Signature          4       4    " WMI"
> + * WMI Interface Version     8       4    <version>
> + * WMI buffer length        12       4    <length>
> + * WMI hotfix number        16       4    <hotfix>
> + */
> +struct descriptor {
> +	char vendor_signature[4];
> +	char object_signature[4];
> +	__le32 interface_version;
> +	__le32 buffer_length;
> +	__le32 hotfix_number;
> +} __packed;
> +
>  struct descriptor_priv {
>  	struct list_head list;
>  	u32 interface_version;
> @@ -88,76 +106,58 @@ bool dell_wmi_get_hotfix(u32 *hotfix)
>  }
>  EXPORT_SYMBOL_GPL(dell_wmi_get_hotfix);
>  
> -/*
> - * Descriptor buffer is 128 byte long and contains:
> - *
> - *       Name             Offset  Length  Value
> - * Vendor Signature          0       4    "DELL"
> - * Object Signature          4       4    " WMI"
> - * WMI Interface Version     8       4    <version>
> - * WMI buffer length        12       4    <length>
> - * WMI hotfix number        16       4    <hotfix>
> - */
> -static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
> -				     const void *context)
> +static int dell_wmi_descriptor_probe(struct wmi_device *wdev, const void *context)
>  {
> -	union acpi_object *obj = NULL;
>  	struct descriptor_priv *priv;
> -	u32 *buffer;
> +	struct wmi_buffer buffer;
> +	struct descriptor *desc;
>  	int ret;
>  
> -	obj = wmidev_block_query(wdev, 0);
> -	if (!obj) {
> -		dev_err(&wdev->dev, "failed to read Dell WMI descriptor\n");
> -		ret = -EIO;
> -		goto out;
> -	}
> +	ret = wmidev_query_block(wdev, 0, &buffer);
> +	if (ret < 0)
> +		return ret;
>  
> -	if (obj->type != ACPI_TYPE_BUFFER) {
> -		dev_err(&wdev->dev, "Dell descriptor has wrong type\n");
> +	if (buffer.length < sizeof(*desc)) {

Hi again,

Thinking some more of this...

Since many of these drivers will need to do this size check and assign 
into some variable that is different from the one passed to 
wmidev_query_block(), would it make sense to move those into generic code? 
It probably requires a macro so you'd have access to type information.

-- 
 i.

> +		dev_err(&wdev->dev,
> +			"Dell descriptor buffer contains not enough data (%zu)\n",
> +			buffer.length);
>  		ret = -EINVAL;
>  		descriptor_valid = ret;
>  		goto out;
>  	}
>  
> -	/* Although it's not technically a failure, this would lead to
> -	 * unexpected behavior
> -	 */
> -	if (obj->buffer.length != 128) {
> -		dev_err(&wdev->dev,
> -			"Dell descriptor buffer has unexpected length (%d)\n",
> -			obj->buffer.length);
> -		ret = -EINVAL;
> +	desc = buffer.data;
> +
> +	if (strncmp(desc->vendor_signature, "DELL", sizeof(desc->vendor_signature))) {
> +		dev_err(&wdev->dev, "Dell descriptor buffer has invalid vendor signature (%4ph)\n",
> +			desc->vendor_signature);
> +		ret = -ENOMSG;
>  		descriptor_valid = ret;
>  		goto out;
>  	}
>  
> -	buffer = (u32 *)obj->buffer.pointer;
> -
> -	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0) {
> -		dev_err(&wdev->dev, "Dell descriptor buffer has invalid signature (%8ph)\n",
> -			buffer);
> -		ret = -EINVAL;
> +	if (strncmp(desc->object_signature, " WMI", sizeof(desc->vendor_signature))) {
> +		dev_err(&wdev->dev, "Dell descriptor buffer has invalid object signature (%4ph)\n",
> +			desc->object_signature);
> +		ret = -ENOMSG;
>  		descriptor_valid = ret;
>  		goto out;
>  	}
>  	descriptor_valid = 0;
>  
> -	if (buffer[2] != 0 && buffer[2] != 1)
> -		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%lu)\n",
> -			(unsigned long) buffer[2]);
> -
> -	priv = devm_kzalloc(&wdev->dev, sizeof(struct descriptor_priv),
> -	GFP_KERNEL);
> +	if (le32_to_cpu(desc->interface_version) > 2)
> +		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%u)\n",
> +			 le32_to_cpu(desc->interface_version));
>  
> +	priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
>  	if (!priv) {
>  		ret = -ENOMEM;
>  		goto out;
>  	}
>  
> -	priv->interface_version = buffer[2];
> -	priv->size = buffer[3];
> -	priv->hotfix = buffer[4];
> +	priv->interface_version = le32_to_cpu(desc->interface_version);
> +	priv->size = le32_to_cpu(desc->buffer_length);
> +	priv->hotfix = le32_to_cpu(desc->hotfix_number);
>  	ret = 0;
>  	dev_set_drvdata(&wdev->dev, priv);
>  	mutex_lock(&list_mutex);
> @@ -170,7 +170,7 @@ static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
>  		(unsigned long) priv->hotfix);
>  
>  out:
> -	kfree(obj);
> +	kfree(buffer.data);
>  	return ret;
>  }
>  
> 

Re: [PATCH v3 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API

[Armin Wolf]

Am 31.03.26 um 11:00 schrieb Ilpo Järvinen:

> On Sat, 14 Mar 2026, Armin Wolf wrote:
>
>> Use the new buffer-based WMI API to also support ACPI firmware
>> implementations that do not use ACPI buffers for the descriptor.
>>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   .../platform/x86/dell/dell-wmi-descriptor.c   | 94 +++++++++----------
>>   1 file changed, 47 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/platform/x86/dell/dell-wmi-descriptor.c b/drivers/platform/x86/dell/dell-wmi-descriptor.c
>> index c2a180202719..fe42eb8bbd79 100644
>> --- a/drivers/platform/x86/dell/dell-wmi-descriptor.c
>> +++ b/drivers/platform/x86/dell/dell-wmi-descriptor.c
>> @@ -7,7 +7,7 @@
>>   
>>   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>   
>> -#include <linux/acpi.h>
>> +#include <linux/compiler_attributes.h>
>>   #include <linux/list.h>
>>   #include <linux/module.h>
>>   #include <linux/wmi.h>
>> @@ -15,6 +15,24 @@
>>   
>>   #define DELL_WMI_DESCRIPTOR_GUID "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
>>   
>> +/*
>> + * Descriptor buffer is 128 byte long and contains:
>> + *
>> + *       Name             Offset  Length  Value
>> + * Vendor Signature          0       4    "DELL"
>> + * Object Signature          4       4    " WMI"
>> + * WMI Interface Version     8       4    <version>
>> + * WMI buffer length        12       4    <length>
>> + * WMI hotfix number        16       4    <hotfix>
>> + */
>> +struct descriptor {
>> +	char vendor_signature[4];
>> +	char object_signature[4];
>> +	__le32 interface_version;
>> +	__le32 buffer_length;
>> +	__le32 hotfix_number;
>> +} __packed;
>> +
>>   struct descriptor_priv {
>>   	struct list_head list;
>>   	u32 interface_version;
>> @@ -88,76 +106,58 @@ bool dell_wmi_get_hotfix(u32 *hotfix)
>>   }
>>   EXPORT_SYMBOL_GPL(dell_wmi_get_hotfix);
>>   
>> -/*
>> - * Descriptor buffer is 128 byte long and contains:
>> - *
>> - *       Name             Offset  Length  Value
>> - * Vendor Signature          0       4    "DELL"
>> - * Object Signature          4       4    " WMI"
>> - * WMI Interface Version     8       4    <version>
>> - * WMI buffer length        12       4    <length>
>> - * WMI hotfix number        16       4    <hotfix>
>> - */
>> -static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
>> -				     const void *context)
>> +static int dell_wmi_descriptor_probe(struct wmi_device *wdev, const void *context)
>>   {
>> -	union acpi_object *obj = NULL;
>>   	struct descriptor_priv *priv;
>> -	u32 *buffer;
>> +	struct wmi_buffer buffer;
>> +	struct descriptor *desc;
>>   	int ret;
>>   
>> -	obj = wmidev_block_query(wdev, 0);
>> -	if (!obj) {
>> -		dev_err(&wdev->dev, "failed to read Dell WMI descriptor\n");
>> -		ret = -EIO;
>> -		goto out;
>> -	}
>> +	ret = wmidev_query_block(wdev, 0, &buffer);
>> +	if (ret < 0)
>> +		return ret;
>>   
>> -	if (obj->type != ACPI_TYPE_BUFFER) {
>> -		dev_err(&wdev->dev, "Dell descriptor has wrong type\n");
>> +	if (buffer.length < sizeof(*desc)) {
>> +		dev_err(&wdev->dev,
>> +			"Dell descriptor buffer contains not enough data (%zu)\n",
> This word order sounds odd to me. I'd say:
>
> Dell descriptor buffer does not contain ...

Ok.

>> +			buffer.length);
>>   		ret = -EINVAL;
>>   		descriptor_valid = ret;
>>   		goto out;
>>   	}
>>   
>> -	/* Although it's not technically a failure, this would lead to
>> -	 * unexpected behavior
>> -	 */
>> -	if (obj->buffer.length != 128) {
>> -		dev_err(&wdev->dev,
>> -			"Dell descriptor buffer has unexpected length (%d)\n",
>> -			obj->buffer.length);
>> -		ret = -EINVAL;
>> +	desc = buffer.data;
> I think these should be moving towards using __free() so we could remove
> those gotos to kfree(). This applies to many changes in this series. Some
> may need a local variable to be introduced (here we've 'desc' already so
> it's very easy in this case).
>
> I'm fine if you want to do __free() changes in a patch separate from
> this wmi conversion but we should aim to make these follow best practices
> so that others pick the preferred patterns more easily.

Ok, i will resend the series with the necessary changes.

Thanks,
Armin Wolf

>> +	if (strncmp(desc->vendor_signature, "DELL", sizeof(desc->vendor_signature))) {
>> +		dev_err(&wdev->dev, "Dell descriptor buffer has invalid vendor signature (%4ph)\n",
>> +			desc->vendor_signature);
>> +		ret = -ENOMSG;
>>   		descriptor_valid = ret;
>>   		goto out;
>>   	}
>>   
>> -	buffer = (u32 *)obj->buffer.pointer;
>> -
>> -	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0) {
>> -		dev_err(&wdev->dev, "Dell descriptor buffer has invalid signature (%8ph)\n",
>> -			buffer);
>> -		ret = -EINVAL;
>> +	if (strncmp(desc->object_signature, " WMI", sizeof(desc->vendor_signature))) {
>> +		dev_err(&wdev->dev, "Dell descriptor buffer has invalid object signature (%4ph)\n",
>> +			desc->object_signature);
>> +		ret = -ENOMSG;
>>   		descriptor_valid = ret;
>>   		goto out;
>>   	}
>>   	descriptor_valid = 0;
>>   
>> -	if (buffer[2] != 0 && buffer[2] != 1)
>> -		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%lu)\n",
>> -			(unsigned long) buffer[2]);
>> -
>> -	priv = devm_kzalloc(&wdev->dev, sizeof(struct descriptor_priv),
>> -	GFP_KERNEL);
>> +	if (le32_to_cpu(desc->interface_version) > 2)
>> +		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%u)\n",
>> +			 le32_to_cpu(desc->interface_version));
>>   
>> +	priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
>>   	if (!priv) {
>>   		ret = -ENOMEM;
>>   		goto out;
>>   	}
>>   
>> -	priv->interface_version = buffer[2];
>> -	priv->size = buffer[3];
>> -	priv->hotfix = buffer[4];
>> +	priv->interface_version = le32_to_cpu(desc->interface_version);
>> +	priv->size = le32_to_cpu(desc->buffer_length);
>> +	priv->hotfix = le32_to_cpu(desc->hotfix_number);
>>   	ret = 0;
>>   	dev_set_drvdata(&wdev->dev, priv);
>>   	mutex_lock(&list_mutex);
>> @@ -170,7 +170,7 @@ static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
>>   		(unsigned long) priv->hotfix);
>>   
>>   out:
>> -	kfree(obj);
>> +	kfree(buffer.data);
>>   	return ret;
>>   }
>>   
>>

Re: [PATCH v3 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API

[Armin Wolf]

Am 31.03.26 um 11:44 schrieb Ilpo Järvinen:

> On Sat, 14 Mar 2026, Armin Wolf wrote:
>
>> Use the new buffer-based WMI API to also support ACPI firmware
>> implementations that do not use ACPI buffers for the descriptor.
>>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   .../platform/x86/dell/dell-wmi-descriptor.c   | 94 +++++++++----------
>>   1 file changed, 47 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/platform/x86/dell/dell-wmi-descriptor.c b/drivers/platform/x86/dell/dell-wmi-descriptor.c
>> index c2a180202719..fe42eb8bbd79 100644
>> --- a/drivers/platform/x86/dell/dell-wmi-descriptor.c
>> +++ b/drivers/platform/x86/dell/dell-wmi-descriptor.c
>> @@ -7,7 +7,7 @@
>>   
>>   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>   
>> -#include <linux/acpi.h>
>> +#include <linux/compiler_attributes.h>
>>   #include <linux/list.h>
>>   #include <linux/module.h>
>>   #include <linux/wmi.h>
>> @@ -15,6 +15,24 @@
>>   
>>   #define DELL_WMI_DESCRIPTOR_GUID "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
>>   
>> +/*
>> + * Descriptor buffer is 128 byte long and contains:
>> + *
>> + *       Name             Offset  Length  Value
>> + * Vendor Signature          0       4    "DELL"
>> + * Object Signature          4       4    " WMI"
>> + * WMI Interface Version     8       4    <version>
>> + * WMI buffer length        12       4    <length>
>> + * WMI hotfix number        16       4    <hotfix>
>> + */
>> +struct descriptor {
>> +	char vendor_signature[4];
>> +	char object_signature[4];
>> +	__le32 interface_version;
>> +	__le32 buffer_length;
>> +	__le32 hotfix_number;
>> +} __packed;
>> +
>>   struct descriptor_priv {
>>   	struct list_head list;
>>   	u32 interface_version;
>> @@ -88,76 +106,58 @@ bool dell_wmi_get_hotfix(u32 *hotfix)
>>   }
>>   EXPORT_SYMBOL_GPL(dell_wmi_get_hotfix);
>>   
>> -/*
>> - * Descriptor buffer is 128 byte long and contains:
>> - *
>> - *       Name             Offset  Length  Value
>> - * Vendor Signature          0       4    "DELL"
>> - * Object Signature          4       4    " WMI"
>> - * WMI Interface Version     8       4    <version>
>> - * WMI buffer length        12       4    <length>
>> - * WMI hotfix number        16       4    <hotfix>
>> - */
>> -static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
>> -				     const void *context)
>> +static int dell_wmi_descriptor_probe(struct wmi_device *wdev, const void *context)
>>   {
>> -	union acpi_object *obj = NULL;
>>   	struct descriptor_priv *priv;
>> -	u32 *buffer;
>> +	struct wmi_buffer buffer;
>> +	struct descriptor *desc;
>>   	int ret;
>>   
>> -	obj = wmidev_block_query(wdev, 0);
>> -	if (!obj) {
>> -		dev_err(&wdev->dev, "failed to read Dell WMI descriptor\n");
>> -		ret = -EIO;
>> -		goto out;
>> -	}
>> +	ret = wmidev_query_block(wdev, 0, &buffer);
>> +	if (ret < 0)
>> +		return ret;
>>   
>> -	if (obj->type != ACPI_TYPE_BUFFER) {
>> -		dev_err(&wdev->dev, "Dell descriptor has wrong type\n");
>> +	if (buffer.length < sizeof(*desc)) {
> Hi again,
>
> Thinking some more of this...
>
> Since many of these drivers will need to do this size check and assign
> into some variable that is different from the one passed to
> wmidev_query_block(), would it make sense to move those into generic code?
> It probably requires a macro so you'd have access to type information.

You mean something like wmidev_block_query_sized(wdev, instance, size)?

Personally i see little benefit in having the assignment handled by a macro.

Thanks,
Armin Wolf

Re: [PATCH v3 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 4142 bytes --]

On Tue, 31 Mar 2026, Armin Wolf wrote:

> Am 31.03.26 um 11:44 schrieb Ilpo Järvinen:
> 
> > On Sat, 14 Mar 2026, Armin Wolf wrote:
> > 
> > > Use the new buffer-based WMI API to also support ACPI firmware
> > > implementations that do not use ACPI buffers for the descriptor.
> > > 
> > > Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> > > ---
> > >   .../platform/x86/dell/dell-wmi-descriptor.c   | 94 +++++++++----------
> > >   1 file changed, 47 insertions(+), 47 deletions(-)
> > > 
> > > diff --git a/drivers/platform/x86/dell/dell-wmi-descriptor.c
> > > b/drivers/platform/x86/dell/dell-wmi-descriptor.c
> > > index c2a180202719..fe42eb8bbd79 100644
> > > --- a/drivers/platform/x86/dell/dell-wmi-descriptor.c
> > > +++ b/drivers/platform/x86/dell/dell-wmi-descriptor.c
> > > @@ -7,7 +7,7 @@
> > >     #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > >   -#include <linux/acpi.h>
> > > +#include <linux/compiler_attributes.h>
> > >   #include <linux/list.h>
> > >   #include <linux/module.h>
> > >   #include <linux/wmi.h>
> > > @@ -15,6 +15,24 @@
> > >     #define DELL_WMI_DESCRIPTOR_GUID
> > > "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
> > >   +/*
> > > + * Descriptor buffer is 128 byte long and contains:
> > > + *
> > > + *       Name             Offset  Length  Value
> > > + * Vendor Signature          0       4    "DELL"
> > > + * Object Signature          4       4    " WMI"
> > > + * WMI Interface Version     8       4    <version>
> > > + * WMI buffer length        12       4    <length>
> > > + * WMI hotfix number        16       4    <hotfix>
> > > + */
> > > +struct descriptor {
> > > +	char vendor_signature[4];
> > > +	char object_signature[4];
> > > +	__le32 interface_version;
> > > +	__le32 buffer_length;
> > > +	__le32 hotfix_number;
> > > +} __packed;
> > > +
> > >   struct descriptor_priv {
> > >   	struct list_head list;
> > >   	u32 interface_version;
> > > @@ -88,76 +106,58 @@ bool dell_wmi_get_hotfix(u32 *hotfix)
> > >   }
> > >   EXPORT_SYMBOL_GPL(dell_wmi_get_hotfix);
> > >   -/*
> > > - * Descriptor buffer is 128 byte long and contains:
> > > - *
> > > - *       Name             Offset  Length  Value
> > > - * Vendor Signature          0       4    "DELL"
> > > - * Object Signature          4       4    " WMI"
> > > - * WMI Interface Version     8       4    <version>
> > > - * WMI buffer length        12       4    <length>
> > > - * WMI hotfix number        16       4    <hotfix>
> > > - */
> > > -static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
> > > -				     const void *context)
> > > +static int dell_wmi_descriptor_probe(struct wmi_device *wdev, const void
> > > *context)
> > >   {
> > > -	union acpi_object *obj = NULL;
> > >   	struct descriptor_priv *priv;
> > > -	u32 *buffer;
> > > +	struct wmi_buffer buffer;
> > > +	struct descriptor *desc;
> > >   	int ret;
> > >   -	obj = wmidev_block_query(wdev, 0);
> > > -	if (!obj) {
> > > -		dev_err(&wdev->dev, "failed to read Dell WMI descriptor\n");
> > > -		ret = -EIO;
> > > -		goto out;
> > > -	}
> > > +	ret = wmidev_query_block(wdev, 0, &buffer);
> > > +	if (ret < 0)
> > > +		return ret;
> > >   -	if (obj->type != ACPI_TYPE_BUFFER) {
> > > -		dev_err(&wdev->dev, "Dell descriptor has wrong type\n");
> > > +	if (buffer.length < sizeof(*desc)) {
> > Hi again,
> > 
> > Thinking some more of this...
> > 
> > Since many of these drivers will need to do this size check and assign
> > into some variable that is different from the one passed to
> > wmidev_query_block(), would it make sense to move those into generic code?
> > It probably requires a macro so you'd have access to type information.
> 
> You mean something like wmidev_block_query_sized(wdev, instance, size)?

So this would return ERR_PTR() as I see no out param given to it?

> Personally i see little benefit in having the assignment handled by a macro.

I was not so interested in assignment itself but getting the size 
information from the output type directly but if you prefer writing 
sizeof(*xx), not a big deal to me.

-- 
 i.

Re: [PATCH v3 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API

[Armin Wolf]

Am 01.04.26 um 12:06 schrieb Ilpo Järvinen:

> On Tue, 31 Mar 2026, Armin Wolf wrote:
>
>> Am 31.03.26 um 11:44 schrieb Ilpo Järvinen:
>>
>>> On Sat, 14 Mar 2026, Armin Wolf wrote:
>>>
>>>> Use the new buffer-based WMI API to also support ACPI firmware
>>>> implementations that do not use ACPI buffers for the descriptor.
>>>>
>>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>>>> ---
>>>>    .../platform/x86/dell/dell-wmi-descriptor.c   | 94 +++++++++----------
>>>>    1 file changed, 47 insertions(+), 47 deletions(-)
>>>>
>>>> diff --git a/drivers/platform/x86/dell/dell-wmi-descriptor.c
>>>> b/drivers/platform/x86/dell/dell-wmi-descriptor.c
>>>> index c2a180202719..fe42eb8bbd79 100644
>>>> --- a/drivers/platform/x86/dell/dell-wmi-descriptor.c
>>>> +++ b/drivers/platform/x86/dell/dell-wmi-descriptor.c
>>>> @@ -7,7 +7,7 @@
>>>>      #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>>>    -#include <linux/acpi.h>
>>>> +#include <linux/compiler_attributes.h>
>>>>    #include <linux/list.h>
>>>>    #include <linux/module.h>
>>>>    #include <linux/wmi.h>
>>>> @@ -15,6 +15,24 @@
>>>>      #define DELL_WMI_DESCRIPTOR_GUID
>>>> "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
>>>>    +/*
>>>> + * Descriptor buffer is 128 byte long and contains:
>>>> + *
>>>> + *       Name             Offset  Length  Value
>>>> + * Vendor Signature          0       4    "DELL"
>>>> + * Object Signature          4       4    " WMI"
>>>> + * WMI Interface Version     8       4    <version>
>>>> + * WMI buffer length        12       4    <length>
>>>> + * WMI hotfix number        16       4    <hotfix>
>>>> + */
>>>> +struct descriptor {
>>>> +	char vendor_signature[4];
>>>> +	char object_signature[4];
>>>> +	__le32 interface_version;
>>>> +	__le32 buffer_length;
>>>> +	__le32 hotfix_number;
>>>> +} __packed;
>>>> +
>>>>    struct descriptor_priv {
>>>>    	struct list_head list;
>>>>    	u32 interface_version;
>>>> @@ -88,76 +106,58 @@ bool dell_wmi_get_hotfix(u32 *hotfix)
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(dell_wmi_get_hotfix);
>>>>    -/*
>>>> - * Descriptor buffer is 128 byte long and contains:
>>>> - *
>>>> - *       Name             Offset  Length  Value
>>>> - * Vendor Signature          0       4    "DELL"
>>>> - * Object Signature          4       4    " WMI"
>>>> - * WMI Interface Version     8       4    <version>
>>>> - * WMI buffer length        12       4    <length>
>>>> - * WMI hotfix number        16       4    <hotfix>
>>>> - */
>>>> -static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
>>>> -				     const void *context)
>>>> +static int dell_wmi_descriptor_probe(struct wmi_device *wdev, const void
>>>> *context)
>>>>    {
>>>> -	union acpi_object *obj = NULL;
>>>>    	struct descriptor_priv *priv;
>>>> -	u32 *buffer;
>>>> +	struct wmi_buffer buffer;
>>>> +	struct descriptor *desc;
>>>>    	int ret;
>>>>    -	obj = wmidev_block_query(wdev, 0);
>>>> -	if (!obj) {
>>>> -		dev_err(&wdev->dev, "failed to read Dell WMI descriptor\n");
>>>> -		ret = -EIO;
>>>> -		goto out;
>>>> -	}
>>>> +	ret = wmidev_query_block(wdev, 0, &buffer);
>>>> +	if (ret < 0)
>>>> +		return ret;
>>>>    -	if (obj->type != ACPI_TYPE_BUFFER) {
>>>> -		dev_err(&wdev->dev, "Dell descriptor has wrong type\n");
>>>> +	if (buffer.length < sizeof(*desc)) {
>>> Hi again,
>>>
>>> Thinking some more of this...
>>>
>>> Since many of these drivers will need to do this size check and assign
>>> into some variable that is different from the one passed to
>>> wmidev_query_block(), would it make sense to move those into generic code?
>>> It probably requires a macro so you'd have access to type information.
>> You mean something like wmidev_block_query_sized(wdev, instance, size)?
> So this would return ERR_PTR() as I see no out param given to it?

My mistake, i meant something like wmidev_block_query_sized(wdev, instance, buffer, size).
This has the same purpose as acpi_evaluate_object_typed(), where the error handling can
often be simplified.

>
>> Personally i see little benefit in having the assignment handled by a macro.
> I was not so interested in assignment itself but getting the size
> information from the output type directly but if you prefer writing
> sizeof(*xx), not a big deal to me.

I would prefer writing sizeof(*var). I will resend this series with the new *_sized()
WMI helper functions and the __free()-related changes.

Thanks,
Armin Wolf