Hi,
On 9/12/22 13:53, Armin Wolf wrote:
> The dell-wmi-ddv driver adds support for reading
> the current temperature and ePPID of ACPI batteries
> on supported Dell machines.
>
> Since the WMI interface used by this driver does not
> do any input validation and thus cannot be used for probing,
> the driver depends on the ACPI battery extension machanism
> to discover batteries.
>
> The driver also supports a debugfs interface for retrieving
> buffers containing fan and thermal sensor information.
> Since the meaing of the content of those buffers is currently
> unknown, the interface is meant for reverse-engineering and
> will likely be replaced with an hwmon interface once the
> meaning has been understood.
>
> The driver was tested on a Dell Inspiron 3505.
>
> Signed-off-by: Armin Wolf <W_Armin@xxxxxx>
> ---
> .../ABI/testing/debugfs-dell-wmi-ddv | 21 +
> .../ABI/testing/sysfs-platform-dell-wmi-ddv | 16 +
> MAINTAINERS | 7 +
> drivers/platform/x86/dell/Kconfig | 13 +
> drivers/platform/x86/dell/Makefile | 1 +
> drivers/platform/x86/dell/dell-wmi-ddv.c | 365 ++++++++++++++++++
> drivers/platform/x86/wmi.c | 1 +
> 7 files changed, 424 insertions(+)
> create mode 100644 Documentation/ABI/testing/debugfs-dell-wmi-ddv
> create mode 100644 Documentation/ABI/testing/sysfs-platform-dell-wmi-ddv
> create mode 100644 drivers/platform/x86/dell/dell-wmi-ddv.c
>
> diff --git a/Documentation/ABI/testing/debugfs-dell-wmi-ddv b/Documentation/ABI/testing/debugfs-dell-wmi-ddv
> new file mode 100644
> index 000000000000..fbcc5d6f7388
> --- /dev/null
> +++ b/Documentation/ABI/testing/debugfs-dell-wmi-ddv
> @@ -0,0 +1,21 @@
> +What: /sys/kernel/debug/dell-wmi-ddv-<wmi_device_name>/fan_sensor_information
> +Date: September 2022
> +KernelVersion: 6.1
> +Contact: Armin Wolf <W_Armin@xxxxxx>
> +Description:
> + This file contains the contents of the fan sensor information buffer,
> + which contains fan sensor entries and a terminating character (0xFF).
> +
> + Each fan sensor entry consists of three bytes with an unknown meaning,
> + interested people may use this file for reverse-engineering.
> +
> +What: /sys/kernel/debug/dell-wmi-ddv-<wmi_device_name>/thermal_sensor_information
> +Date: September 2022
> +KernelVersion: 6.1
> +Contact: Armin Wolf <W_Armin@xxxxxx>
> +Description:
> + This file contains the contents of the thermal sensor information buffer,
> + which contains thermal sensor entries and a terminating character (0xFF).
> +
> + Each thermal sensor entry consists of five bytes with an unknown meaning,
> + interested people may use this file for reverse-engineering.
> diff --git a/Documentation/ABI/testing/sysfs-platform-dell-wmi-ddv b/Documentation/ABI/testing/sysfs-platform-dell-wmi-ddv
> new file mode 100644
> index 000000000000..98e0b8399d13
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform-dell-wmi-ddv
> @@ -0,0 +1,16 @@
> +What: /sys/class/power_supply/<battery_name>/temp
> +Date: September 2022
> +KernelVersion: 6.1
> +Contact: Armin Wolf <W_Armin@xxxxxx>
> +Description:
> + Reports the current ACPI battery temperature on supported Dell machines.
> +
> + Values are represented in 1/10 Degrees Celsius.
This is a standard power_supply class attribute which is already documented in:
Documentation/ABI/testing/sysfs-class-power
so no need to document this here, please drop it.
Other then that this patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Regards,
Hans
> +
> +What: /sys/class/power_supply/<battery_name>/eppid
> +Date: September 2022
> +KernelVersion: 6.1
> +Contact: Armin Wolf <W_Armin@xxxxxx>
> +Description:
> + Reports the Dell ePPID (electronic Dell Piece Part Identification)
> + of the ACPI battery.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6bb894ea4a77..d9fd4c9eebbc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5821,6 +5821,13 @@ L: Dell.Client.Kernel@xxxxxxxx
> S: Maintained
> F: drivers/platform/x86/dell/dell-wmi-descriptor.c
>
> +DELL WMI DDV DRIVER
> +M: Armin Wolf <W_Armin@xxxxxx>
> +S: Maintained
> +F: Documentation/ABI/testing/debugfs-dell-wmi-ddv
> +F: Documentation/ABI/testing/sysfs-platform-dell-wmi-ddv
> +F: drivers/platform/x86/dell/dell-wmi-ddv.c
> +
> DELL WMI SYSMAN DRIVER
> M: Divya Bharathi <divya.bharathi@xxxxxxxx>
> M: Prasanth Ksr <prasanth.ksr@xxxxxxxx>
> diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
> index 25421e061c47..209e63e347e2 100644
> --- a/drivers/platform/x86/dell/Kconfig
> +++ b/drivers/platform/x86/dell/Kconfig
> @@ -189,6 +189,19 @@ config DELL_WMI_DESCRIPTOR
> default n
> depends on ACPI_WMI
>
> +config DELL_WMI_DDV
> + tristate "Dell WMI sensors Support"
> + default m
> + depends on ACPI_BATTERY
> + depends on ACPI_WMI
> + help
> + This option adds support for WMI-based sensors like
> + battery temperature sensors found on some Dell notebooks.
> + It also supports reading of the batteries ePPID.
> +
> + To compile this drivers as a module, choose M here: the module will
> + be called dell-wmi-ddv.
> +
> config DELL_WMI_LED
> tristate "External LED on Dell Business Netbooks"
> default m
> diff --git a/drivers/platform/x86/dell/Makefile b/drivers/platform/x86/dell/Makefile
> index ddba1df71e80..1b8942426622 100644
> --- a/drivers/platform/x86/dell/Makefile
> +++ b/drivers/platform/x86/dell/Makefile
> @@ -19,5 +19,6 @@ dell-wmi-objs := dell-wmi-base.o
> dell-wmi-$(CONFIG_DELL_WMI_PRIVACY) += dell-wmi-privacy.o
> obj-$(CONFIG_DELL_WMI_AIO) += dell-wmi-aio.o
> obj-$(CONFIG_DELL_WMI_DESCRIPTOR) += dell-wmi-descriptor.o
> +obj-$(CONFIG_DELL_WMI_DDV) += dell-wmi-ddv.o
> obj-$(CONFIG_DELL_WMI_LED) += dell-wmi-led.o
> obj-$(CONFIG_DELL_WMI_SYSMAN) += dell-wmi-sysman/
> diff --git a/drivers/platform/x86/dell/dell-wmi-ddv.c b/drivers/platform/x86/dell/dell-wmi-ddv.c
> new file mode 100644
> index 000000000000..6eef298f13eb
> --- /dev/null
> +++ b/drivers/platform/x86/dell/dell-wmi-ddv.c
> @@ -0,0 +1,365 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * dell-wmi-ddv.c -- Linux driver for WMI sensor information on Dell notebooks.
> + *
> + * Copyright (C) 2022 Armin Wolf <W_Armin@xxxxxx>
> + */
> +
> +#define pr_format(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <acpi/battery.h>
> +#include <linux/acpi.h>
> +#include <linux/debugfs.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/kstrtox.h>
> +#include <linux/math.h>
> +#include <linux/module.h>
> +#include <linux/limits.h>
> +#include <linux/power_supply.h>
> +#include <linux/seq_file.h>
> +#include <linux/sysfs.h>
> +#include <linux/wmi.h>
> +
> +#define DRIVER_NAME "dell-wmi-ddv"
> +
> +#define DELL_DDV_SUPPORTED_INTERFACE 2
> +#define DELL_DDV_GUID "8A42EA14-4F2A-FD45-6422-0087F7A7E608"
> +
> +enum dell_ddv_method {
> + DELL_DDV_BATTERY_DESIGN_CAPACITY = 0x01,
> + DELL_DDV_BATTERY_FULL_CHARGE_CAPACITY = 0x02,
> + DELL_DDV_BATTERY_MANUFACTURE_NAME = 0x03,
> + DELL_DDV_BATTERY_MANUFACTURE_DATE = 0x04,
> + DELL_DDV_BATTERY_SERIAL_NUMBER = 0x05,
> + DELL_DDV_BATTERY_CHEMISTRY_VALUE = 0x06,
> + DELL_DDV_BATTERY_TEMPERATURE = 0x07,
> + DELL_DDV_BATTERY_CURRENT = 0x08,
> + DELL_DDV_BATTERY_VOLTAGE = 0x09,
> + DELL_DDV_BATTERY_MANUFACTURER_ACCESS = 0x0A,
> + DELL_DDV_BATTERY_RELATIVE_CHARGE_STATE = 0x0B,
> + DELL_DDV_BATTERY_CYCLE_COUNT = 0x0C,
> + DELL_DDV_BATTERY_EPPID = 0x0D,
> + DELL_DDV_BATTERY_RAW_ANALYTICS_START = 0x0E,
> + DELL_DDV_BATTERY_RAW_ANALYTICS = 0x0F,
> + DELL_DDV_BATTERY_DESIGN_VOLTAGE = 0x10,
> +
> + DELL_DDV_INTERFACE_VERSION = 0x12,
> +
> + DELL_DDV_FAN_SENSOR_INFORMATION = 0x20,
> + DELL_DDV_THERMAL_SENSOR_INFORMATION = 0x22,
> +};
> +
> +struct dell_wmi_ddv_data {
> + struct device_attribute temp_attr, eppid_attr;
> + struct wmi_device *wdev;
> +};
> +
> +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)
> +{
> + struct acpi_buffer out = { ACPI_ALLOCATE_BUFFER, NULL };
> + const struct acpi_buffer in = {
> + .length = sizeof(arg),
> + .pointer = &arg,
> + };
> + 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 -EIO;
> + }
> +
> + *result = obj;
> +
> + return 0;
> +}
> +
> +static int dell_wmi_ddv_query_integer(struct wmi_device *wdev, enum dell_ddv_method method,
> + u32 arg, u32 *res)
> +{
> + union acpi_object *obj;
> + int ret;
> +
> + ret = dell_wmi_ddv_query_type(wdev, method, arg, &obj, ACPI_TYPE_INTEGER);
> + if (ret < 0)
> + return ret;
> +
> + if (obj->integer.value <= U32_MAX)
> + *res = (u32)obj->integer.value;
> + else
> + ret = -ERANGE;
> +
> + kfree(obj);
> +
> + return ret;
> +}
> +
> +static int dell_wmi_ddv_query_buffer(struct wmi_device *wdev, enum dell_ddv_method method,
> + u32 arg, union acpi_object **result)
> +{
> + union acpi_object *obj;
> + u64 buffer_size;
> + int ret;
> +
> + ret = dell_wmi_ddv_query_type(wdev, method, arg, &obj, ACPI_TYPE_PACKAGE);
> + if (ret < 0)
> + return ret;
> +
> + if (obj->package.count != 2)
> + goto err_free;
> +
> + if (obj->package.elements[0].type != ACPI_TYPE_INTEGER)
> + goto err_free;
> +
> + buffer_size = obj->package.elements[0].integer.value;
> +
> + if (obj->package.elements[1].type != ACPI_TYPE_BUFFER)
> + goto err_free;
> +
> + if (buffer_size != obj->package.elements[1].buffer.length) {
> + dev_warn(&wdev->dev,
> + FW_WARN "ACPI buffer size (%llu) does not match WMI buffer size (%d)\n",
> + buffer_size, obj->package.elements[1].buffer.length);
> +
> + goto err_free;
> + }
> +
> + *result = obj;
> +
> + return 0;
> +
> +err_free:
> + kfree(obj);
> +
> + return -EIO;
> +}
> +
> +static int dell_wmi_ddv_query_string(struct wmi_device *wdev, enum dell_ddv_method method,
> + u32 arg, union acpi_object **result)
> +{
> + return dell_wmi_ddv_query_type(wdev, method, arg, result, ACPI_TYPE_STRING);
> +}
> +
> +static int dell_wmi_ddv_battery_index(struct acpi_device *acpi_dev, u32 *index)
> +{
> + const char *uid_str = acpi_device_uid(acpi_dev);
> +
> + if (!uid_str)
> + return -ENODEV;
> +
> + return kstrtou32(uid_str, 10, index);
> +}
> +
> +static ssize_t temp_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct dell_wmi_ddv_data *data = container_of(attr, struct dell_wmi_ddv_data, temp_attr);
> + u32 index, value;
> + int ret;
> +
> + ret = dell_wmi_ddv_battery_index(to_acpi_device(dev->parent), &index);
> + if (ret < 0)
> + return ret;
> +
> + ret = dell_wmi_ddv_query_integer(data->wdev, DELL_DDV_BATTERY_TEMPERATURE, index, &value);
> + if (ret < 0)
> + return ret;
> +
> + return sysfs_emit(buf, "%d\n", DIV_ROUND_CLOSEST(value, 10));
> +}
> +
> +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;
> + u32 index;
> + int ret;
> +
> + ret = dell_wmi_ddv_battery_index(to_acpi_device(dev->parent), &index);
> + if (ret < 0)
> + return ret;
> +
> + ret = dell_wmi_ddv_query_string(data->wdev, DELL_DDV_BATTERY_EPPID, index, &obj);
> + if (ret < 0)
> + return ret;
> +
> + ret = sysfs_emit(buf, "%s\n", obj->string.pointer);
> +
> + kfree(obj);
> +
> + return ret;
> +}
> +
> +static int dell_wmi_ddv_add_battery(void *drvdata, struct power_supply *battery)
> +{
> + struct dell_wmi_ddv_data *data = drvdata;
> + u32 index;
> + int ret;
> +
> + ret = dell_wmi_ddv_battery_index(to_acpi_device(battery->dev.parent), &index);
> + if (ret < 0)
> + return ret;
> +
> + ret = device_create_file(&battery->dev, &data->temp_attr);
> + if (ret < 0)
> + return ret;
> +
> + ret = device_create_file(&battery->dev, &data->eppid_attr);
> + if (ret < 0) {
> + device_remove_file(&battery->dev, &data->temp_attr);
> +
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int dell_wmi_ddv_remove_battery(void *drvdata, struct power_supply *battery)
> +{
> + struct dell_wmi_ddv_data *data = drvdata;
> +
> + device_remove_file(&battery->dev, &data->temp_attr);
> + device_remove_file(&battery->dev, &data->eppid_attr);
> +
> + return 0;
> +}
> +
> +static const struct acpi_battery_hook_ops dell_wmi_ddv_battery_hook_ops = {
> + .add_battery = dell_wmi_ddv_add_battery,
> + .remove_battery = dell_wmi_ddv_remove_battery,
> +};
> +
> +static void dell_wmi_ddv_battery_remove(void *data)
> +{
> + struct acpi_battery_hook *hook = data;
> +
> + battery_hook_unregister(hook);
> +}
> +
> +static int dell_wmi_ddv_battery_add(struct dell_wmi_ddv_data *data)
> +{
> + struct acpi_battery_hook *hook;
> +
> + sysfs_attr_init(&data->temp_attr.attr);
> + data->temp_attr.attr.name = "temp";
> + data->temp_attr.attr.mode = 0444;
> + data->temp_attr.show = temp_show;
> +
> + sysfs_attr_init(&data->eppid_attr.attr);
> + data->eppid_attr.attr.name = "eppid";
> + data->eppid_attr.attr.mode = 0444;
> + data->eppid_attr.show = eppid_show;
> +
> + hook = battery_hook_register("DELL Battery Extension", data,
> + &dell_wmi_ddv_battery_hook_ops);
> + if (IS_ERR(hook))
> + return PTR_ERR(hook);
> +
> + return devm_add_action_or_reset(&data->wdev->dev, dell_wmi_ddv_battery_remove, hook);
> +}
> +
> +static int dell_wmi_ddv_buffer_read(struct seq_file *seq, enum dell_ddv_method method)
> +{
> + struct device *dev = seq->private;
> + struct dell_wmi_ddv_data *data = dev_get_drvdata(dev);
> + union acpi_object *obj;
> + union acpi_object buf;
> + int ret;
> +
> + ret = dell_wmi_ddv_query_buffer(data->wdev, method, 0, &obj);
> + if (ret < 0)
> + return ret;
> +
> + buf = obj->package.elements[1];
> + ret = seq_write(seq, buf.buffer.pointer, buf.buffer.length);
> + kfree(obj);
> +
> + return ret;
> +}
> +
> +static int dell_wmi_ddv_fan_read(struct seq_file *seq, void *offset)
> +{
> + return dell_wmi_ddv_buffer_read(seq, DELL_DDV_FAN_SENSOR_INFORMATION);
> +}
> +
> +static int dell_wmi_ddv_temp_read(struct seq_file *seq, void *offset)
> +{
> + return dell_wmi_ddv_buffer_read(seq, DELL_DDV_THERMAL_SENSOR_INFORMATION);
> +}
> +
> +static void dell_wmi_ddv_debugfs_remove(void *data)
> +{
> + struct dentry *entry = data;
> +
> + debugfs_remove(entry);
> +}
> +
> +static void dell_wmi_ddv_debugfs_init(struct wmi_device *wdev)
> +{
> + struct dentry *entry;
> + char name[64];
> +
> + scnprintf(name, ARRAY_SIZE(name), "%s-%s", DRIVER_NAME, dev_name(&wdev->dev));
> + entry = debugfs_create_dir(name, NULL);
> +
> + debugfs_create_devm_seqfile(&wdev->dev, "fan_sensor_information", entry,
> + dell_wmi_ddv_fan_read);
> + debugfs_create_devm_seqfile(&wdev->dev, "thermal_sensor_information", entry,
> + dell_wmi_ddv_temp_read);
> +
> + devm_add_action_or_reset(&wdev->dev, dell_wmi_ddv_debugfs_remove, entry);
> +}
> +
> +static int dell_wmi_ddv_probe(struct wmi_device *wdev, const void *context)
> +{
> + struct dell_wmi_ddv_data *data;
> + u32 version;
> + int ret;
> +
> + ret = dell_wmi_ddv_query_integer(wdev, DELL_DDV_INTERFACE_VERSION, 0, &version);
> + if (ret < 0)
> + return ret;
> +
> + dev_dbg(&wdev->dev, "WMI interface version: %d\n", version);
> + if (version != DELL_DDV_SUPPORTED_INTERFACE)
> + return -ENODEV;
> +
> + data = devm_kzalloc(&wdev->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + dev_set_drvdata(&wdev->dev, data);
> + data->wdev = wdev;
> +
> + dell_wmi_ddv_debugfs_init(wdev);
> +
> + return dell_wmi_ddv_battery_add(data);
> +}
> +
> +static const struct wmi_device_id dell_wmi_ddv_id_table[] = {
> + { DELL_DDV_GUID, NULL },
> + { }
> +};
> +MODULE_DEVICE_TABLE(wmi, dell_wmi_ddv_id_table);
> +
> +static struct wmi_driver dell_wmi_ddv_driver = {
> + .driver = {
> + .name = DRIVER_NAME,
> + },
> + .id_table = dell_wmi_ddv_id_table,
> + .probe = dell_wmi_ddv_probe,
> +};
> +module_wmi_driver(dell_wmi_ddv_driver);
> +
> +MODULE_AUTHOR("Armin Wolf <W_Armin@xxxxxx>");
> +MODULE_DESCRIPTION("Dell WMI sensor driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index aff23309b5d3..f307d8c5c6c3 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -108,6 +108,7 @@ MODULE_DEVICE_TABLE(acpi, wmi_device_ids);
> /* allow duplicate GUIDs as these device drivers use struct wmi_driver */
> static const char * const allow_duplicates[] = {
> "05901221-D566-11D1-B2F0-00A0C9062910", /* wmi-bmof */
> + "8A42EA14-4F2A-FD45-6422-0087F7A7E608", /* dell-wmi-ddv */
> NULL
> };
>
> --
> 2.30.2
>
