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

Ilpo Järvinen
2026-03-31T09:03:42Z

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.