This patch series adds a new driver for a WMI interface found in
many newer Dell machines. This interface allows to read battery
properties like temperature and the ePPID (Dell-specific), while
also providing fan and thermal sensor information.
The interface does support multiple batteries which are indetified
by an "index", which appears to be the batteries ACPI UID. Since
the interface also appears to omit any bounts checking of the
index, the ACPI battery hook mechanism is used to discover batteries.
Since the information returned when querying fan/thermal sensor
information is currently unknown, a debugfs entry is created to
allow for easier reverse engineering. The interface is likely
to be replaced by a proper hwmon interface in the future.
Since the driver can potentially be instantiated mutplie times,
the ACPI battery hook mechanism had to be extended.
The first two patches allow that battery hooks are not unloaded
if they return an error when a battery was added, so that they
can safely return -ENODEV.
The next two patches extend the battery hook mechanism to better
support drivers which can be instantiated mupltible times.
The last patch finally adds the new driver. It was called
dell-wmi-ddv since the interface is called "DDV" by Dell software,
likely meaning "Dell Data Vault".
The driver was tested, together with the changes made to the
ACPI battery driver, on a Dell Inspiron 3505. Other drivers
already using the battery hook mechanism where changed as well,
but could only be compile-tested due to missing hardware.
Armin Wolf (5):
ACPI: battery: Do not unload battery hooks on single error
ACPI: battery: Simplify battery_hook_unregister()
ACPI: battery: Allow battery hooks to be registered multiple times.
ACPI: battery: Allow for passing data to battery hooks.
platform/x86: dell: Add new dell-wmi-ddv driver
.../ABI/testing/debugfs-dell-wmi-ddv | 21 +
.../ABI/testing/sysfs-platform-dell-wmi-ddv | 16 +
MAINTAINERS | 7 +
drivers/acpi/battery.c | 59 ++-
drivers/platform/x86/asus-wmi.c | 20 +-
drivers/platform/x86/dell/Kconfig | 13 +
drivers/platform/x86/dell/Makefile | 1 +
drivers/platform/x86/dell/dell-wmi-ddv.c | 365 ++++++++++++++++++
drivers/platform/x86/huawei-wmi.c | 15 +-
drivers/platform/x86/lg-laptop.c | 14 +-
drivers/platform/x86/system76_acpi.c | 22 +-
drivers/platform/x86/thinkpad_acpi.c | 15 +-
drivers/platform/x86/wmi.c | 1 +
include/acpi/battery.h | 12 +-
14 files changed, 504 insertions(+), 77 deletions(-)
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
--
2.30.2
Since a driver may register multiple instances of a
battery hook, passing data to each instance is
convenient.
Signed-off-by: Armin Wolf <[email protected]>
---
drivers/acpi/battery.c | 11 ++++++-----
drivers/platform/x86/asus-wmi.c | 7 ++++---
drivers/platform/x86/huawei-wmi.c | 6 +++---
drivers/platform/x86/lg-laptop.c | 6 +++---
drivers/platform/x86/system76_acpi.c | 6 +++---
drivers/platform/x86/thinkpad_acpi.c | 6 +++---
include/acpi/battery.h | 7 ++++---
7 files changed, 26 insertions(+), 23 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 15bb5d868a56..396a7324216c 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -696,7 +696,7 @@ void battery_hook_unregister(struct acpi_battery_hook *hook)
mutex_lock(&hook_mutex);
list_for_each_entry(battery, &acpi_battery_list, list) {
- hook->ops->remove_battery(battery->bat);
+ hook->ops->remove_battery(hook->data, battery->bat);
}
list_del(&hook->list);
@@ -706,7 +706,7 @@ void battery_hook_unregister(struct acpi_battery_hook *hook)
}
EXPORT_SYMBOL_GPL(battery_hook_unregister);
-struct acpi_battery_hook *battery_hook_register(const char *name,
+struct acpi_battery_hook *battery_hook_register(const char *name, void *data,
const struct acpi_battery_hook_ops *ops)
{
struct acpi_battery_hook *hook = kzalloc(sizeof(*hook), GFP_KERNEL);
@@ -716,6 +716,7 @@ struct acpi_battery_hook *battery_hook_register(const char *name,
return ERR_PTR(-ENOMEM);
hook->name = name;
+ hook->data = data;
hook->ops = ops;
mutex_lock(&hook_mutex);
@@ -728,7 +729,7 @@ struct acpi_battery_hook *battery_hook_register(const char *name,
* its attributes.
*/
list_for_each_entry(battery, &acpi_battery_list, list) {
- hook->ops->add_battery(battery->bat);
+ hook->ops->add_battery(hook->data, battery->bat);
}
pr_info("new extension: %s\n", hook->name);
@@ -758,7 +759,7 @@ static void battery_hook_add_battery(struct acpi_battery *battery)
* during the battery module initialization.
*/
list_for_each_entry_safe(hook_node, tmp, &battery_hook_list, list) {
- hook_node->ops->add_battery(battery->bat);
+ hook_node->ops->add_battery(hook_node->data, battery->bat);
}
mutex_unlock(&hook_mutex);
}
@@ -773,7 +774,7 @@ static void battery_hook_remove_battery(struct acpi_battery *battery)
* custom attributes from the battery.
*/
list_for_each_entry(hook, &battery_hook_list, list) {
- hook->ops->remove_battery(battery->bat);
+ hook->ops->remove_battery(hook->data, battery->bat);
}
/* Then, just remove the battery from the list */
list_del(&battery->list);
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 37d8649418f4..18afcf38931f 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -882,7 +882,7 @@ static ssize_t charge_control_end_threshold_show(struct device *device,
static DEVICE_ATTR_RW(charge_control_end_threshold);
-static int asus_wmi_battery_add(struct power_supply *battery)
+static int asus_wmi_battery_add(void *data, struct power_supply *battery)
{
/* The WMI method does not provide a way to specific a battery, so we
* just assume it is the first battery.
@@ -909,7 +909,7 @@ static int asus_wmi_battery_add(struct power_supply *battery)
return 0;
}
-static int asus_wmi_battery_remove(struct power_supply *battery)
+static int asus_wmi_battery_remove(void *data, struct power_supply *battery)
{
device_remove_file(&battery->dev,
&dev_attr_charge_control_end_threshold);
@@ -924,7 +924,8 @@ static const struct acpi_battery_hook_ops battery_hook_ops = {
static void asus_wmi_battery_init(struct asus_wmi *asus)
{
if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_RSOC)) {
- asus->hook = battery_hook_register("ASUS Battery Extension", &battery_hook_ops);
+ asus->hook = battery_hook_register("ASUS Battery Extension", NULL,
+ &battery_hook_ops);
}
}
diff --git a/drivers/platform/x86/huawei-wmi.c b/drivers/platform/x86/huawei-wmi.c
index 6fd02b25a97b..f23806299c1a 100644
--- a/drivers/platform/x86/huawei-wmi.c
+++ b/drivers/platform/x86/huawei-wmi.c
@@ -469,7 +469,7 @@ static DEVICE_ATTR_RW(charge_control_start_threshold);
static DEVICE_ATTR_RW(charge_control_end_threshold);
static DEVICE_ATTR_RW(charge_control_thresholds);
-static int huawei_wmi_battery_add(struct power_supply *battery)
+static int huawei_wmi_battery_add(void *data, struct power_supply *battery)
{
int err = 0;
@@ -484,7 +484,7 @@ static int huawei_wmi_battery_add(struct power_supply *battery)
return err;
}
-static int huawei_wmi_battery_remove(struct power_supply *battery)
+static int huawei_wmi_battery_remove(void *data, struct power_supply *battery)
{
device_remove_file(&battery->dev, &dev_attr_charge_control_start_threshold);
device_remove_file(&battery->dev, &dev_attr_charge_control_end_threshold);
@@ -507,7 +507,7 @@ static void huawei_wmi_battery_setup(struct device *dev)
return;
}
- huawei->hook = battery_hook_register("Huawei Battery Extension",
+ huawei->hook = battery_hook_register("Huawei Battery Extension", NULL,
&huawei_wmi_battery_hook_ops);
device_create_file(dev, &dev_attr_charge_control_thresholds);
}
diff --git a/drivers/platform/x86/lg-laptop.c b/drivers/platform/x86/lg-laptop.c
index d8a61a07313e..f1abb1924150 100644
--- a/drivers/platform/x86/lg-laptop.c
+++ b/drivers/platform/x86/lg-laptop.c
@@ -547,7 +547,7 @@ static DEVICE_ATTR_RW(fn_lock);
static DEVICE_ATTR_RW(charge_control_end_threshold);
static DEVICE_ATTR_RW(battery_care_limit);
-static int lg_battery_add(struct power_supply *battery)
+static int lg_battery_add(void *data, struct power_supply *battery)
{
if (device_create_file(&battery->dev,
&dev_attr_charge_control_end_threshold))
@@ -556,7 +556,7 @@ static int lg_battery_add(struct power_supply *battery)
return 0;
}
-static int lg_battery_remove(struct power_supply *battery)
+static int lg_battery_remove(void *data, struct power_supply *battery)
{
device_remove_file(&battery->dev,
&dev_attr_charge_control_end_threshold);
@@ -750,7 +750,7 @@ static int acpi_add(struct acpi_device *device)
led_classdev_register(&pf_device->dev, &tpad_led);
wmi_input_setup();
- hook = battery_hook_register("LG Battery Extension", &battery_hook_ops);
+ hook = battery_hook_register("LG Battery Extension", NULL, &battery_hook_ops);
return 0;
diff --git a/drivers/platform/x86/system76_acpi.c b/drivers/platform/x86/system76_acpi.c
index 1ec22db32bd0..9414b9491806 100644
--- a/drivers/platform/x86/system76_acpi.c
+++ b/drivers/platform/x86/system76_acpi.c
@@ -255,7 +255,7 @@ static struct attribute *system76_battery_attrs[] = {
ATTRIBUTE_GROUPS(system76_battery);
-static int system76_battery_add(struct power_supply *battery)
+static int system76_battery_add(void *data, struct power_supply *battery)
{
// System76 EC only supports 1 battery
if (strcmp(battery->desc->name, "BAT0") != 0)
@@ -267,7 +267,7 @@ static int system76_battery_add(struct power_supply *battery)
return 0;
}
-static int system76_battery_remove(struct power_supply *battery)
+static int system76_battery_remove(void *data, struct power_supply *battery)
{
device_remove_groups(&battery->dev, system76_battery_groups);
return 0;
@@ -280,7 +280,7 @@ static const struct acpi_battery_hook_ops system76_battery_hook_ops = {
static void system76_battery_init(struct system76_data *data)
{
- data->hook = battery_hook_register("System76 Battery Extension",
+ data->hook = battery_hook_register("System76 Battery Extension", NULL,
&system76_battery_hook_ops);
}
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 8adafc3c31fa..6008d88e0727 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9898,7 +9898,7 @@ ATTRIBUTE_GROUPS(tpacpi_battery);
/* ACPI battery hooking */
-static int tpacpi_battery_add(struct power_supply *battery)
+static int tpacpi_battery_add(void *data, struct power_supply *battery)
{
int batteryid = tpacpi_battery_get_id(battery->desc->name);
@@ -9909,7 +9909,7 @@ static int tpacpi_battery_add(struct power_supply *battery)
return 0;
}
-static int tpacpi_battery_remove(struct power_supply *battery)
+static int tpacpi_battery_remove(void *data, struct power_supply *battery)
{
device_remove_groups(&battery->dev, tpacpi_battery_groups);
return 0;
@@ -9943,7 +9943,7 @@ static int __init tpacpi_battery_init(struct ibm_init_struct *ibm)
battery_quirk_table,
ARRAY_SIZE(battery_quirk_table));
- battery_info.hook = battery_hook_register("ThinkPad Battery Extension",
+ battery_info.hook = battery_hook_register("ThinkPad Battery Extension", NULL,
&battery_hook_ops);
return 0;
diff --git a/include/acpi/battery.h b/include/acpi/battery.h
index b3c81abada1e..cca401b793b2 100644
--- a/include/acpi/battery.h
+++ b/include/acpi/battery.h
@@ -11,17 +11,18 @@
#define ACPI_BATTERY_NOTIFY_THRESHOLD 0x82
struct acpi_battery_hook_ops {
- int (*add_battery)(struct power_supply *battery);
- int (*remove_battery)(struct power_supply *battery);
+ int (*add_battery)(void *data, struct power_supply *battery);
+ int (*remove_battery)(void *data, struct power_supply *battery);
};
struct acpi_battery_hook {
const char *name;
const struct acpi_battery_hook_ops *ops;
+ void *data;
struct list_head list;
};
-struct acpi_battery_hook *battery_hook_register(const char *name,
+struct acpi_battery_hook *battery_hook_register(const char *name, void *data,
const struct acpi_battery_hook_ops *hook);
void battery_hook_unregister(struct acpi_battery_hook *hook);
--
2.30.2
Registering multiple instances of a battery hook is beneficial
for drivers which can be instantiated multiple times. Until now,
this would mean that such a driver would have to implement some
logic to manage battery hooks.
Extend the battery hook handling instead.
Signed-off-by: Armin Wolf <[email protected]>
---
drivers/acpi/battery.c | 21 ++++++++++++++++-----
drivers/platform/x86/asus-wmi.c | 15 ++++++---------
drivers/platform/x86/huawei-wmi.c | 11 +++++++----
drivers/platform/x86/lg-laptop.c | 10 ++++++----
drivers/platform/x86/system76_acpi.c | 18 ++++++++++--------
drivers/platform/x86/thinkpad_acpi.c | 11 +++++++----
include/acpi/battery.h | 11 ++++++++---
7 files changed, 60 insertions(+), 37 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 4aea65f3d8c3..15bb5d868a56 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -696,19 +696,28 @@ void battery_hook_unregister(struct acpi_battery_hook *hook)
mutex_lock(&hook_mutex);
list_for_each_entry(battery, &acpi_battery_list, list) {
- hook->remove_battery(battery->bat);
+ hook->ops->remove_battery(battery->bat);
}
list_del(&hook->list);
mutex_unlock(&hook_mutex);
pr_info("extension unregistered: %s\n", hook->name);
+ kfree(hook);
}
EXPORT_SYMBOL_GPL(battery_hook_unregister);
-void battery_hook_register(struct acpi_battery_hook *hook)
+struct acpi_battery_hook *battery_hook_register(const char *name,
+ const struct acpi_battery_hook_ops *ops)
{
+ struct acpi_battery_hook *hook = kzalloc(sizeof(*hook), GFP_KERNEL);
struct acpi_battery *battery;
+ if (!hook)
+ return ERR_PTR(-ENOMEM);
+
+ hook->name = name;
+ hook->ops = ops;
+
mutex_lock(&hook_mutex);
INIT_LIST_HEAD(&hook->list);
list_add(&hook->list, &battery_hook_list);
@@ -719,11 +728,13 @@ void battery_hook_register(struct acpi_battery_hook *hook)
* its attributes.
*/
list_for_each_entry(battery, &acpi_battery_list, list) {
- hook->add_battery(battery->bat);
+ hook->ops->add_battery(battery->bat);
}
pr_info("new extension: %s\n", hook->name);
mutex_unlock(&hook_mutex);
+
+ return hook;
}
EXPORT_SYMBOL_GPL(battery_hook_register);
@@ -747,7 +758,7 @@ static void battery_hook_add_battery(struct acpi_battery *battery)
* during the battery module initialization.
*/
list_for_each_entry_safe(hook_node, tmp, &battery_hook_list, list) {
- hook_node->add_battery(battery->bat);
+ hook_node->ops->add_battery(battery->bat);
}
mutex_unlock(&hook_mutex);
}
@@ -762,7 +773,7 @@ static void battery_hook_remove_battery(struct acpi_battery *battery)
* custom attributes from the battery.
*/
list_for_each_entry(hook, &battery_hook_list, list) {
- hook->remove_battery(battery->bat);
+ hook->ops->remove_battery(battery->bat);
}
/* Then, just remove the battery from the list */
list_del(&battery->list);
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index d95170b7dba0..37d8649418f4 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -252,8 +252,8 @@ struct asus_wmi {
struct platform_profile_handler platform_profile_handler;
bool platform_profile_support;
- // The RSOC controls the maximum charging percentage.
- bool battery_rsoc_available;
+ // The RSOC battery hook controls the maximum charging percentage.
+ struct acpi_battery_hook *hook;
bool panel_overdrive_available;
@@ -916,25 +916,22 @@ static int asus_wmi_battery_remove(struct power_supply *battery)
return 0;
}
-static struct acpi_battery_hook battery_hook = {
+static const struct acpi_battery_hook_ops battery_hook_ops = {
.add_battery = asus_wmi_battery_add,
.remove_battery = asus_wmi_battery_remove,
- .name = "ASUS Battery Extension",
};
static void asus_wmi_battery_init(struct asus_wmi *asus)
{
- asus->battery_rsoc_available = false;
if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_RSOC)) {
- asus->battery_rsoc_available = true;
- battery_hook_register(&battery_hook);
+ asus->hook = battery_hook_register("ASUS Battery Extension", &battery_hook_ops);
}
}
static void asus_wmi_battery_exit(struct asus_wmi *asus)
{
- if (asus->battery_rsoc_available)
- battery_hook_unregister(&battery_hook);
+ if (!IS_ERR_OR_NULL(asus->hook))
+ battery_hook_unregister(asus->hook);
}
/* LEDs ***********************************************************************/
diff --git a/drivers/platform/x86/huawei-wmi.c b/drivers/platform/x86/huawei-wmi.c
index eac3e6b4ea11..6fd02b25a97b 100644
--- a/drivers/platform/x86/huawei-wmi.c
+++ b/drivers/platform/x86/huawei-wmi.c
@@ -62,6 +62,7 @@ struct huawei_wmi {
bool battery_available;
bool fn_lock_available;
+ struct acpi_battery_hook *hook;
struct huawei_wmi_debug debug;
struct input_dev *idev[2];
struct led_classdev cdev;
@@ -491,10 +492,9 @@ static int huawei_wmi_battery_remove(struct power_supply *battery)
return 0;
}
-static struct acpi_battery_hook huawei_wmi_battery_hook = {
+static const struct acpi_battery_hook_ops huawei_wmi_battery_hook_ops = {
.add_battery = huawei_wmi_battery_add,
.remove_battery = huawei_wmi_battery_remove,
- .name = "Huawei Battery Extension"
};
static void huawei_wmi_battery_setup(struct device *dev)
@@ -507,7 +507,8 @@ static void huawei_wmi_battery_setup(struct device *dev)
return;
}
- battery_hook_register(&huawei_wmi_battery_hook);
+ huawei->hook = battery_hook_register("Huawei Battery Extension",
+ &huawei_wmi_battery_hook_ops);
device_create_file(dev, &dev_attr_charge_control_thresholds);
}
@@ -516,7 +517,9 @@ static void huawei_wmi_battery_exit(struct device *dev)
struct huawei_wmi *huawei = dev_get_drvdata(dev);
if (huawei->battery_available) {
- battery_hook_unregister(&huawei_wmi_battery_hook);
+ if (!IS_ERR(huawei->hook))
+ battery_hook_unregister(huawei->hook);
+
device_remove_file(dev, &dev_attr_charge_control_thresholds);
}
}
diff --git a/drivers/platform/x86/lg-laptop.c b/drivers/platform/x86/lg-laptop.c
index 332868b140ed..d8a61a07313e 100644
--- a/drivers/platform/x86/lg-laptop.c
+++ b/drivers/platform/x86/lg-laptop.c
@@ -73,6 +73,7 @@ static u32 inited;
#define INIT_SPARSE_KEYMAP 0x80
static int battery_limit_use_wmbb;
+static struct acpi_battery_hook *hook;
static struct led_classdev kbd_backlight;
static enum led_brightness get_kbd_backlight_level(void);
@@ -562,10 +563,9 @@ static int lg_battery_remove(struct power_supply *battery)
return 0;
}
-static struct acpi_battery_hook battery_hook = {
+static const struct acpi_battery_hook_ops battery_hook_ops = {
.add_battery = lg_battery_add,
.remove_battery = lg_battery_remove,
- .name = "LG Battery Extension",
};
static struct attribute *dev_attributes[] = {
@@ -750,7 +750,7 @@ static int acpi_add(struct acpi_device *device)
led_classdev_register(&pf_device->dev, &tpad_led);
wmi_input_setup();
- battery_hook_register(&battery_hook);
+ hook = battery_hook_register("LG Battery Extension", &battery_hook_ops);
return 0;
@@ -768,7 +768,9 @@ static int acpi_remove(struct acpi_device *device)
led_classdev_unregister(&tpad_led);
led_classdev_unregister(&kbd_backlight);
- battery_hook_unregister(&battery_hook);
+ if (!IS_ERR(hook))
+ battery_hook_unregister(hook);
+
wmi_input_destroy();
platform_device_unregister(pf_device);
pf_device = NULL;
diff --git a/drivers/platform/x86/system76_acpi.c b/drivers/platform/x86/system76_acpi.c
index 958df41ad509..1ec22db32bd0 100644
--- a/drivers/platform/x86/system76_acpi.c
+++ b/drivers/platform/x86/system76_acpi.c
@@ -26,6 +26,7 @@
struct system76_data {
struct acpi_device *acpi_dev;
+ struct acpi_battery_hook *hook;
struct led_classdev ap_led;
struct led_classdev kb_led;
enum led_brightness kb_brightness;
@@ -272,20 +273,21 @@ static int system76_battery_remove(struct power_supply *battery)
return 0;
}
-static struct acpi_battery_hook system76_battery_hook = {
+static const struct acpi_battery_hook_ops system76_battery_hook_ops = {
.add_battery = system76_battery_add,
.remove_battery = system76_battery_remove,
- .name = "System76 Battery Extension",
};
-static void system76_battery_init(void)
+static void system76_battery_init(struct system76_data *data)
{
- battery_hook_register(&system76_battery_hook);
+ data->hook = battery_hook_register("System76 Battery Extension",
+ &system76_battery_hook_ops);
}
-static void system76_battery_exit(void)
+static void system76_battery_exit(struct system76_data *data)
{
- battery_hook_unregister(&system76_battery_hook);
+ if (!IS_ERR(data->hook))
+ battery_hook_unregister(data->hook);
}
// Get the airplane mode LED brightness
@@ -730,7 +732,7 @@ static int system76_add(struct acpi_device *acpi_dev)
if (err)
goto error;
- system76_battery_init();
+ system76_battery_init(data);
}
return 0;
@@ -751,7 +753,7 @@ static int system76_remove(struct acpi_device *acpi_dev)
data = acpi_driver_data(acpi_dev);
if (data->has_open_ec) {
- system76_battery_exit();
+ system76_battery_exit(data);
kfree(data->nfan);
kfree(data->ntmp);
}
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 8fbe21ebcc52..8adafc3c31fa 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9407,6 +9407,7 @@ struct tpacpi_battery_data {
struct tpacpi_battery_driver_data {
struct tpacpi_battery_data batteries[3];
+ struct acpi_battery_hook *hook;
int individual_addressing;
};
@@ -9914,10 +9915,9 @@ static int tpacpi_battery_remove(struct power_supply *battery)
return 0;
}
-static struct acpi_battery_hook battery_hook = {
+static const struct acpi_battery_hook_ops battery_hook_ops = {
.add_battery = tpacpi_battery_add,
.remove_battery = tpacpi_battery_remove,
- .name = "ThinkPad Battery Extension",
};
/* Subdriver init/exit */
@@ -9943,13 +9943,16 @@ static int __init tpacpi_battery_init(struct ibm_init_struct *ibm)
battery_quirk_table,
ARRAY_SIZE(battery_quirk_table));
- battery_hook_register(&battery_hook);
+ battery_info.hook = battery_hook_register("ThinkPad Battery Extension",
+ &battery_hook_ops);
+
return 0;
}
static void tpacpi_battery_exit(void)
{
- battery_hook_unregister(&battery_hook);
+ if (!IS_ERR(battery_info.hook))
+ battery_hook_unregister(battery_info.hook);
}
static struct ibm_struct battery_driver_data = {
diff --git a/include/acpi/battery.h b/include/acpi/battery.h
index b8d56b702c7a..b3c81abada1e 100644
--- a/include/acpi/battery.h
+++ b/include/acpi/battery.h
@@ -10,14 +10,19 @@
#define ACPI_BATTERY_NOTIFY_INFO 0x81
#define ACPI_BATTERY_NOTIFY_THRESHOLD 0x82
-struct acpi_battery_hook {
- const char *name;
+struct acpi_battery_hook_ops {
int (*add_battery)(struct power_supply *battery);
int (*remove_battery)(struct power_supply *battery);
+};
+
+struct acpi_battery_hook {
+ const char *name;
+ const struct acpi_battery_hook_ops *ops;
struct list_head list;
};
-void battery_hook_register(struct acpi_battery_hook *hook);
+struct acpi_battery_hook *battery_hook_register(const char *name,
+ const struct acpi_battery_hook_ops *hook);
void battery_hook_unregister(struct acpi_battery_hook *hook);
#endif
--
2.30.2
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 <[email protected]>
---
.../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 <[email protected]>
+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 <[email protected]>
+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 <[email protected]>
+Description:
+ Reports the current ACPI battery temperature on supported Dell machines.
+
+ Values are represented in 1/10 Degrees Celsius.
+
+What: /sys/class/power_supply/<battery_name>/eppid
+Date: September 2022
+KernelVersion: 6.1
+Contact: Armin Wolf <[email protected]>
+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: [email protected]
S: Maintained
F: drivers/platform/x86/dell/dell-wmi-descriptor.c
+DELL WMI DDV DRIVER
+M: Armin Wolf <[email protected]>
+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 <[email protected]>
M: Prasanth Ksr <[email protected]>
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 <[email protected]>
+ */
+
+#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 <[email protected]>");
+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
Hi
2022. szeptember 12., hétfő 14:53 keltezéssel, Armin Wolf írta:
> Registering multiple instances of a battery hook is beneficial
> for drivers which can be instantiated multiple times. Until now,
> this would mean that such a driver would have to implement some
> logic to manage battery hooks.
>
> Extend the battery hook handling instead.
I think this is already possible by embedding the acpi_battery_hook
object inside the driver's device specific data object, no?
Regards,
Barnabás Pőcze
> [...]
Am 12.09.22 um 18:42 schrieb Barnabás Pőcze:
> Hi
>
> 2022. szeptember 12., hétfő 14:53 keltezéssel, Armin Wolf írta:
>
>> Registering multiple instances of a battery hook is beneficial
>> for drivers which can be instantiated multiple times. Until now,
>> this would mean that such a driver would have to implement some
>> logic to manage battery hooks.
>>
>> Extend the battery hook handling instead.
> I think this is already possible by embedding the acpi_battery_hook
> object inside the driver's device specific data object, no?
>
> Regards,
> Barnabás Pőcze
>
>
Yes, it indeed is. However afaik it is not possible to pass instance-specific
data to such an embedded battery hook. It could be possible by passing the
battery hook as an argument to add_battery()/remove_battery() and using container_of(),
but in my opinion this would be too much of a quick hack.
>> [...]
Hi--
On 9/12/22 05:53, Armin Wolf wrote:
> 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
You should (try to) justify default m, otherwise just
don't have a default for it.
> + 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.
--
~Randy
Am 12.09.22 um 23:56 schrieb Randy Dunlap:
> Hi--
>
> On 9/12/22 05:53, Armin Wolf wrote:
>> 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
> You should (try to) justify default m, otherwise just
> don't have a default for it.
I have chosen default m since many other Dell platform drivers are being
default m. Since this driver is not essential for normal operation,
i will drop default m then.
Armin Wolf
>> + 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.
[Public]
> -----Original Message-----
> From: Armin Wolf <[email protected]>
> Sent: Tuesday, September 13, 2022 09:41
> To: Randy Dunlap <[email protected]>; [email protected];
> [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH 5/5] platform/x86: dell: Add new dell-wmi-ddv driver
>
> Am 12.09.22 um 23:56 schrieb Randy Dunlap:
>
> > Hi--
> >
> > On 9/12/22 05:53, Armin Wolf wrote:
> >> 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
> > You should (try to) justify default m, otherwise just
> > don't have a default for it.
>
> I have chosen default m since many other Dell platform drivers are being
> default m. Since this driver is not essential for normal operation,
> i will drop default m then.
Actually Dell drivers directory are a bit unique in this regard. There is a special
top level boolean. I would suggest to keep it as is.
Take a look at:
menuconfig X86_PLATFORM_DRIVERS_DELL
>
> Armin Wolf
>
> >> + 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.
Am 13.09.22 um 18:08 schrieb Limonciello, Mario:
> [Public]
>
>
>
>> -----Original Message-----
>> From: Armin Wolf <[email protected]>
>> Sent: Tuesday, September 13, 2022 09:41
>> To: Randy Dunlap <[email protected]>; [email protected];
>> [email protected]
>> Cc: [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; linux-
>> [email protected]; [email protected]
>> Subject: Re: [PATCH 5/5] platform/x86: dell: Add new dell-wmi-ddv driver
>>
>> Am 12.09.22 um 23:56 schrieb Randy Dunlap:
>>
>>> Hi--
>>>
>>> On 9/12/22 05:53, Armin Wolf wrote:
>>>> 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
>>> You should (try to) justify default m, otherwise just
>>> don't have a default for it.
>> I have chosen default m since many other Dell platform drivers are being
>> default m. Since this driver is not essential for normal operation,
>> i will drop default m then.
> Actually Dell drivers directory are a bit unique in this regard. There is a special
> top level boolean. I would suggest to keep it as is.
>
> Take a look at:
> menuconfig X86_PLATFORM_DRIVERS_DELL
Ok.
Armin Wolf
>> Armin Wolf
>>
>>>> + 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.
On 9/13/22 09:08, Limonciello, Mario wrote:
> [Public]
>
>
>
>> -----Original Message-----
>> From: Armin Wolf <[email protected]>
>> Sent: Tuesday, September 13, 2022 09:41
>> To: Randy Dunlap <[email protected]>; [email protected];
>> [email protected]
>> Cc: [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; linux-
>> [email protected]; [email protected]
>> Subject: Re: [PATCH 5/5] platform/x86: dell: Add new dell-wmi-ddv driver
>>
>> Am 12.09.22 um 23:56 schrieb Randy Dunlap:
>>
>>> Hi--
>>>
>>> On 9/12/22 05:53, Armin Wolf wrote:
>>>> 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
>>> You should (try to) justify default m, otherwise just
>>> don't have a default for it.
>>
>> I have chosen default m since many other Dell platform drivers are being
>> default m. Since this driver is not essential for normal operation,
>> i will drop default m then.
>
> Actually Dell drivers directory are a bit unique in this regard. There is a special
> top level boolean. I would suggest to keep it as is.
>
> Take a look at:
> menuconfig X86_PLATFORM_DRIVERS_DELL
>
So all of those "default m" and "default y" drivers are *needed*
as opposed to desirable?
>>
>> Armin Wolf
>>
>>>> + 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.
thanks.
--
~Randy
On 9/13/22 11:30, Limonciello, Mario wrote:
> On 9/13/2022 13:27, Randy Dunlap wrote:
>>
>>
>> On 9/13/22 09:08, Limonciello, Mario wrote:
>>> [Public]
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Armin Wolf <[email protected]>
>>>> Sent: Tuesday, September 13, 2022 09:41
>>>> To: Randy Dunlap <[email protected]>; [email protected];
>>>> [email protected]
>>>> Cc: [email protected]; [email protected]; [email protected];
>>>> [email protected]; [email protected]; [email protected];
>>>> [email protected]; [email protected]; linux-
>>>> [email protected]; [email protected]
>>>> Subject: Re: [PATCH 5/5] platform/x86: dell: Add new dell-wmi-ddv driver
>>>>
>>>> Am 12.09.22 um 23:56 schrieb Randy Dunlap:
>>>>
>>>>> Hi--
>>>>>
>>>>> On 9/12/22 05:53, Armin Wolf wrote:
>>>>>> 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
>>>>> You should (try to) justify default m, otherwise just
>>>>> don't have a default for it.
>>>>
>>>> I have chosen default m since many other Dell platform drivers are being
>>>> default m. Since this driver is not essential for normal operation,
>>>> i will drop default m then.
>>>
>>> Actually Dell drivers directory are a bit unique in this regard. There is a special
>>> top level boolean. I would suggest to keep it as is.
>>>
>>> Take a look at:
>>> menuconfig X86_PLATFORM_DRIVERS_DELL
>>>
>>
>> So all of those "default m" and "default y" drivers are *needed*
>> as opposed to desirable?
>>
>
> It was supposed to be a convenience option, it's first introduced in f1e1ea516721d1.
>
> So if you have a Dell laptop you set the one option and then get defaults for all those modules.
oh well. whatever.
thanks.
>>>>
>>>> Armin Wolf
>>>>
>>>>>> + 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.
>>
>> thanks.
>
--
~Randy
On 9/13/2022 13:27, Randy Dunlap wrote:
>
>
> On 9/13/22 09:08, Limonciello, Mario wrote:
>> [Public]
>>
>>
>>
>>> -----Original Message-----
>>> From: Armin Wolf <[email protected]>
>>> Sent: Tuesday, September 13, 2022 09:41
>>> To: Randy Dunlap <[email protected]>; [email protected];
>>> [email protected]
>>> Cc: [email protected]; [email protected]; [email protected];
>>> [email protected]; [email protected]; [email protected];
>>> [email protected]; [email protected]; linux-
>>> [email protected]; [email protected]
>>> Subject: Re: [PATCH 5/5] platform/x86: dell: Add new dell-wmi-ddv driver
>>>
>>> Am 12.09.22 um 23:56 schrieb Randy Dunlap:
>>>
>>>> Hi--
>>>>
>>>> On 9/12/22 05:53, Armin Wolf wrote:
>>>>> 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
>>>> You should (try to) justify default m, otherwise just
>>>> don't have a default for it.
>>>
>>> I have chosen default m since many other Dell platform drivers are being
>>> default m. Since this driver is not essential for normal operation,
>>> i will drop default m then.
>>
>> Actually Dell drivers directory are a bit unique in this regard. There is a special
>> top level boolean. I would suggest to keep it as is.
>>
>> Take a look at:
>> menuconfig X86_PLATFORM_DRIVERS_DELL
>>
>
> So all of those "default m" and "default y" drivers are *needed*
> as opposed to desirable?
>
It was supposed to be a convenience option, it's first introduced in
f1e1ea516721d1.
So if you have a Dell laptop you set the one option and then get
defaults for all those modules.
>>>
>>> Armin Wolf
>>>
>>>>> + 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.
>
> thanks.
Hi,
On 9/12/22 18:29, Armin Wolf wrote:
> Am 12.09.22 um 18:42 schrieb Barnabás Pőcze:
>
>> Hi
>>
>> 2022. szeptember 12., hétfő 14:53 keltezéssel, Armin Wolf írta:
>>
>>> Registering multiple instances of a battery hook is beneficial
>>> for drivers which can be instantiated multiple times. Until now,
>>> this would mean that such a driver would have to implement some
>>> logic to manage battery hooks.
>>>
>>> Extend the battery hook handling instead.
>> I think this is already possible by embedding the acpi_battery_hook
>> object inside the driver's device specific data object, no?
>>
>> Regards,
>> Barnabás Pőcze
>>
>>
> Yes, it indeed is. However afaik it is not possible to pass instance-specific
> data to such an embedded battery hook. It could be possible by passing the
> battery hook as an argument to add_battery()/remove_battery() and using container_of(),
> but in my opinion this would be too much of a quick hack.
Actually thinking more about this (after my reviewed-by of 4/5) I believe
that leaving the lifetime management of the struct acpi_battery_hook hook
in the caller and then modifying 4/4 to pass the hook to the callback,
so that the callback can indeed do container_of to get its top-level
driver-data struct would be a better/cleaner solution then this patch +
patch 4/5 .
Doing things this way is quite normal in the kernel and doing a single
large alloc is better then a bunch of small allocs. In this case it does
not really matter, but if we do things like this over all drivers
eventually all the small mallocs add up.
Doing things this way would also reduce the amount of churn in this
series a bit.
Regards,
Hans
Hi,
On 9/12/22 13:53, Armin Wolf wrote:
> Since a driver may register multiple instances of a
> battery hook, passing data to each instance is
> convenient.
>
> Signed-off-by: Armin Wolf <[email protected]>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <[email protected]>
Regards,
Hans
> ---
> drivers/acpi/battery.c | 11 ++++++-----
> drivers/platform/x86/asus-wmi.c | 7 ++++---
> drivers/platform/x86/huawei-wmi.c | 6 +++---
> drivers/platform/x86/lg-laptop.c | 6 +++---
> drivers/platform/x86/system76_acpi.c | 6 +++---
> drivers/platform/x86/thinkpad_acpi.c | 6 +++---
> include/acpi/battery.h | 7 ++++---
> 7 files changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 15bb5d868a56..396a7324216c 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -696,7 +696,7 @@ void battery_hook_unregister(struct acpi_battery_hook *hook)
> mutex_lock(&hook_mutex);
>
> list_for_each_entry(battery, &acpi_battery_list, list) {
> - hook->ops->remove_battery(battery->bat);
> + hook->ops->remove_battery(hook->data, battery->bat);
> }
> list_del(&hook->list);
>
> @@ -706,7 +706,7 @@ void battery_hook_unregister(struct acpi_battery_hook *hook)
> }
> EXPORT_SYMBOL_GPL(battery_hook_unregister);
>
> -struct acpi_battery_hook *battery_hook_register(const char *name,
> +struct acpi_battery_hook *battery_hook_register(const char *name, void *data,
> const struct acpi_battery_hook_ops *ops)
> {
> struct acpi_battery_hook *hook = kzalloc(sizeof(*hook), GFP_KERNEL);
> @@ -716,6 +716,7 @@ struct acpi_battery_hook *battery_hook_register(const char *name,
> return ERR_PTR(-ENOMEM);
>
> hook->name = name;
> + hook->data = data;
> hook->ops = ops;
>
> mutex_lock(&hook_mutex);
> @@ -728,7 +729,7 @@ struct acpi_battery_hook *battery_hook_register(const char *name,
> * its attributes.
> */
> list_for_each_entry(battery, &acpi_battery_list, list) {
> - hook->ops->add_battery(battery->bat);
> + hook->ops->add_battery(hook->data, battery->bat);
> }
> pr_info("new extension: %s\n", hook->name);
>
> @@ -758,7 +759,7 @@ static void battery_hook_add_battery(struct acpi_battery *battery)
> * during the battery module initialization.
> */
> list_for_each_entry_safe(hook_node, tmp, &battery_hook_list, list) {
> - hook_node->ops->add_battery(battery->bat);
> + hook_node->ops->add_battery(hook_node->data, battery->bat);
> }
> mutex_unlock(&hook_mutex);
> }
> @@ -773,7 +774,7 @@ static void battery_hook_remove_battery(struct acpi_battery *battery)
> * custom attributes from the battery.
> */
> list_for_each_entry(hook, &battery_hook_list, list) {
> - hook->ops->remove_battery(battery->bat);
> + hook->ops->remove_battery(hook->data, battery->bat);
> }
> /* Then, just remove the battery from the list */
> list_del(&battery->list);
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 37d8649418f4..18afcf38931f 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -882,7 +882,7 @@ static ssize_t charge_control_end_threshold_show(struct device *device,
>
> static DEVICE_ATTR_RW(charge_control_end_threshold);
>
> -static int asus_wmi_battery_add(struct power_supply *battery)
> +static int asus_wmi_battery_add(void *data, struct power_supply *battery)
> {
> /* The WMI method does not provide a way to specific a battery, so we
> * just assume it is the first battery.
> @@ -909,7 +909,7 @@ static int asus_wmi_battery_add(struct power_supply *battery)
> return 0;
> }
>
> -static int asus_wmi_battery_remove(struct power_supply *battery)
> +static int asus_wmi_battery_remove(void *data, struct power_supply *battery)
> {
> device_remove_file(&battery->dev,
> &dev_attr_charge_control_end_threshold);
> @@ -924,7 +924,8 @@ static const struct acpi_battery_hook_ops battery_hook_ops = {
> static void asus_wmi_battery_init(struct asus_wmi *asus)
> {
> if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_RSOC)) {
> - asus->hook = battery_hook_register("ASUS Battery Extension", &battery_hook_ops);
> + asus->hook = battery_hook_register("ASUS Battery Extension", NULL,
> + &battery_hook_ops);
> }
> }
>
> diff --git a/drivers/platform/x86/huawei-wmi.c b/drivers/platform/x86/huawei-wmi.c
> index 6fd02b25a97b..f23806299c1a 100644
> --- a/drivers/platform/x86/huawei-wmi.c
> +++ b/drivers/platform/x86/huawei-wmi.c
> @@ -469,7 +469,7 @@ static DEVICE_ATTR_RW(charge_control_start_threshold);
> static DEVICE_ATTR_RW(charge_control_end_threshold);
> static DEVICE_ATTR_RW(charge_control_thresholds);
>
> -static int huawei_wmi_battery_add(struct power_supply *battery)
> +static int huawei_wmi_battery_add(void *data, struct power_supply *battery)
> {
> int err = 0;
>
> @@ -484,7 +484,7 @@ static int huawei_wmi_battery_add(struct power_supply *battery)
> return err;
> }
>
> -static int huawei_wmi_battery_remove(struct power_supply *battery)
> +static int huawei_wmi_battery_remove(void *data, struct power_supply *battery)
> {
> device_remove_file(&battery->dev, &dev_attr_charge_control_start_threshold);
> device_remove_file(&battery->dev, &dev_attr_charge_control_end_threshold);
> @@ -507,7 +507,7 @@ static void huawei_wmi_battery_setup(struct device *dev)
> return;
> }
>
> - huawei->hook = battery_hook_register("Huawei Battery Extension",
> + huawei->hook = battery_hook_register("Huawei Battery Extension", NULL,
> &huawei_wmi_battery_hook_ops);
> device_create_file(dev, &dev_attr_charge_control_thresholds);
> }
> diff --git a/drivers/platform/x86/lg-laptop.c b/drivers/platform/x86/lg-laptop.c
> index d8a61a07313e..f1abb1924150 100644
> --- a/drivers/platform/x86/lg-laptop.c
> +++ b/drivers/platform/x86/lg-laptop.c
> @@ -547,7 +547,7 @@ static DEVICE_ATTR_RW(fn_lock);
> static DEVICE_ATTR_RW(charge_control_end_threshold);
> static DEVICE_ATTR_RW(battery_care_limit);
>
> -static int lg_battery_add(struct power_supply *battery)
> +static int lg_battery_add(void *data, struct power_supply *battery)
> {
> if (device_create_file(&battery->dev,
> &dev_attr_charge_control_end_threshold))
> @@ -556,7 +556,7 @@ static int lg_battery_add(struct power_supply *battery)
> return 0;
> }
>
> -static int lg_battery_remove(struct power_supply *battery)
> +static int lg_battery_remove(void *data, struct power_supply *battery)
> {
> device_remove_file(&battery->dev,
> &dev_attr_charge_control_end_threshold);
> @@ -750,7 +750,7 @@ static int acpi_add(struct acpi_device *device)
> led_classdev_register(&pf_device->dev, &tpad_led);
>
> wmi_input_setup();
> - hook = battery_hook_register("LG Battery Extension", &battery_hook_ops);
> + hook = battery_hook_register("LG Battery Extension", NULL, &battery_hook_ops);
>
> return 0;
>
> diff --git a/drivers/platform/x86/system76_acpi.c b/drivers/platform/x86/system76_acpi.c
> index 1ec22db32bd0..9414b9491806 100644
> --- a/drivers/platform/x86/system76_acpi.c
> +++ b/drivers/platform/x86/system76_acpi.c
> @@ -255,7 +255,7 @@ static struct attribute *system76_battery_attrs[] = {
>
> ATTRIBUTE_GROUPS(system76_battery);
>
> -static int system76_battery_add(struct power_supply *battery)
> +static int system76_battery_add(void *data, struct power_supply *battery)
> {
> // System76 EC only supports 1 battery
> if (strcmp(battery->desc->name, "BAT0") != 0)
> @@ -267,7 +267,7 @@ static int system76_battery_add(struct power_supply *battery)
> return 0;
> }
>
> -static int system76_battery_remove(struct power_supply *battery)
> +static int system76_battery_remove(void *data, struct power_supply *battery)
> {
> device_remove_groups(&battery->dev, system76_battery_groups);
> return 0;
> @@ -280,7 +280,7 @@ static const struct acpi_battery_hook_ops system76_battery_hook_ops = {
>
> static void system76_battery_init(struct system76_data *data)
> {
> - data->hook = battery_hook_register("System76 Battery Extension",
> + data->hook = battery_hook_register("System76 Battery Extension", NULL,
> &system76_battery_hook_ops);
> }
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 8adafc3c31fa..6008d88e0727 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -9898,7 +9898,7 @@ ATTRIBUTE_GROUPS(tpacpi_battery);
>
> /* ACPI battery hooking */
>
> -static int tpacpi_battery_add(struct power_supply *battery)
> +static int tpacpi_battery_add(void *data, struct power_supply *battery)
> {
> int batteryid = tpacpi_battery_get_id(battery->desc->name);
>
> @@ -9909,7 +9909,7 @@ static int tpacpi_battery_add(struct power_supply *battery)
> return 0;
> }
>
> -static int tpacpi_battery_remove(struct power_supply *battery)
> +static int tpacpi_battery_remove(void *data, struct power_supply *battery)
> {
> device_remove_groups(&battery->dev, tpacpi_battery_groups);
> return 0;
> @@ -9943,7 +9943,7 @@ static int __init tpacpi_battery_init(struct ibm_init_struct *ibm)
> battery_quirk_table,
> ARRAY_SIZE(battery_quirk_table));
>
> - battery_info.hook = battery_hook_register("ThinkPad Battery Extension",
> + battery_info.hook = battery_hook_register("ThinkPad Battery Extension", NULL,
> &battery_hook_ops);
>
> return 0;
> diff --git a/include/acpi/battery.h b/include/acpi/battery.h
> index b3c81abada1e..cca401b793b2 100644
> --- a/include/acpi/battery.h
> +++ b/include/acpi/battery.h
> @@ -11,17 +11,18 @@
> #define ACPI_BATTERY_NOTIFY_THRESHOLD 0x82
>
> struct acpi_battery_hook_ops {
> - int (*add_battery)(struct power_supply *battery);
> - int (*remove_battery)(struct power_supply *battery);
> + int (*add_battery)(void *data, struct power_supply *battery);
> + int (*remove_battery)(void *data, struct power_supply *battery);
> };
>
> struct acpi_battery_hook {
> const char *name;
> const struct acpi_battery_hook_ops *ops;
> + void *data;
> struct list_head list;
> };
>
> -struct acpi_battery_hook *battery_hook_register(const char *name,
> +struct acpi_battery_hook *battery_hook_register(const char *name, void *data,
> const struct acpi_battery_hook_ops *hook);
> void battery_hook_unregister(struct acpi_battery_hook *hook);
>
> --
> 2.30.2
>
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 <[email protected]>
> ---
> .../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 <[email protected]>
> +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 <[email protected]>
> +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 <[email protected]>
> +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 <[email protected]>
Regards,
Hans
> +
> +What: /sys/class/power_supply/<battery_name>/eppid
> +Date: September 2022
> +KernelVersion: 6.1
> +Contact: Armin Wolf <[email protected]>
> +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: [email protected]
> S: Maintained
> F: drivers/platform/x86/dell/dell-wmi-descriptor.c
>
> +DELL WMI DDV DRIVER
> +M: Armin Wolf <[email protected]>
> +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 <[email protected]>
> M: Prasanth Ksr <[email protected]>
> 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 <[email protected]>
> + */
> +
> +#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 <[email protected]>");
> +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
>
Hi
2022. szeptember 12., hétfő 19:29 keltezéssel, Armin Wolf <[email protected]> írta:
> Am 12.09.22 um 18:42 schrieb Barnabás Pőcze:
>
> > Hi
> >
> > 2022. szeptember 12., hétfő 14:53 keltezéssel, Armin Wolf írta:
> >
> > > Registering multiple instances of a battery hook is beneficial
> > > for drivers which can be instantiated multiple times. Until now,
> > > this would mean that such a driver would have to implement some
> > > logic to manage battery hooks.
> > >
> > > Extend the battery hook handling instead.
> > > I think this is already possible by embedding the acpi_battery_hook
> > > object inside the driver's device specific data object, no?
> >
> > Regards,
> > Barnabás Pőcze
>
> Yes, it indeed is. However afaik it is not possible to pass instance-specific
> data to such an embedded battery hook. It could be possible by passing the
> battery hook as an argument to add_battery()/remove_battery() and using container_of(),
> but in my opinion this would be too much of a quick hack.
Good point about the instance-specific data. However, regarding the second point,
I am with Hans. I do not really think it is that big of a hack. It is inheritance.
Regards,
Barnabás Pőcze