Skip to contentSkip to conversation

battery: Add the battery hooking API

PATCH Superseded Ognjen Galic 2 participants 3 messages Dec 23, 2017

Subsystems (3)
ACPI x86 platform drivers ACPI component architecture (acpica)

v10 of 13 Superseded Latest is v13 →

lore
Share & tools
lore

Ognjen Galic

author
#0 lore raw

PATCH v10 1/4 battery: Add the battery hooking API

This is a patch that implements a generic hooking API for the generic ACPI battery driver.

With this new generic API, drivers can expose platform specific behaviour via sysfs attributes in /sys/class/power_supply/BATn/ in a generic way.

A perfect example of the need for this API are Lenovo ThinkPads.

Lenovo ThinkPads have a ACPI extension that allows the setting of start and stop charge thresholds in the EC and battery firmware via ACPI. The thinkpad_acpi module can use this API to expose sysfs attributes that it controls inside the ACPI battery driver sysfs tree, under /sys/class/power_supply/BATN/.

The file drivers/acpi/battery.h has been moved to include/acpi/battery.h and the includes inside ac.c, sbs.c, and battery.c have been adjusted to reflect that.

When drivers hooks into the API, the API calls add_battery() for each battery in the system that passes it a acpi_battery struct. Then, the drivers can use device_create_file() to create new sysfs attributes with that struct and identify the batteries for per-battery attributes.
---
v7:
* Implemented mutual exclusion for hooking methods and battery callbacks
* Fixed a BUG where errors in other modules would occur when the modules that depend on battery get unloaded

v8:
* Use list_for_each_safe instead of list_for_each for the module exit function where deletion of nodes occurs

v9:
* No changes in this patch in v9

v10:
* Fix compiler warnings in Intel's 0-day CI

Signed-off-by Ognjen Galic <[email protected]>
---
 drivers/acpi/ac.c      |   2 +-
 drivers/acpi/battery.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++-
 drivers/acpi/battery.h |  11 ---
 drivers/acpi/sbs.c     |   2 +-
 include/acpi/battery.h |  21 ++++++
 5 files changed, 199 insertions(+), 16 deletions(-)
 delete mode 100644 drivers/acpi/battery.h
 create mode 100644 include/acpi/battery.h
drivers/acpi/ac.c +1 -1
index 47a7ed5..2d8de2f 100644
@@ -33,7 +33,7 @@
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
 #include <linux/acpi.h>
-#include "battery.h"
+#include <acpi/battery.h>
 
 #define PREFIX "ACPI: "
drivers/acpi/battery.c +176 -3

228 lines not on this page · raw message

drivers/acpi/battery.h deleted +0 -11
deleted file mode 100644
index 225f493..0000000
@@ -1,11 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __ACPI_BATTERY_H
-#define __ACPI_BATTERY_H
-
-#define ACPI_BATTERY_CLASS "battery"
-
-#define ACPI_BATTERY_NOTIFY_STATUS	0x80
-#define ACPI_BATTERY_NOTIFY_INFO	0x81
-#define ACPI_BATTERY_NOTIFY_THRESHOLD   0x82
-
-#endif
drivers/acpi/sbs.c +1 -1
index a2428e9..295b592 100644
@@ -32,9 +32,9 @@
 #include <linux/delay.h>
 #include <linux/power_supply.h>
 #include <linux/platform_data/x86/apple.h>
+#include <acpi/battery.h>
 
 #include "sbshc.h"
-#include "battery.h"
 
 #define PREFIX "ACPI: "
include/acpi/battery.h new file +21 -0
new file mode 100644
index 0000000..5d8f5d9
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ACPI_BATTERY_H
+#define __ACPI_BATTERY_H
+
+#define ACPI_BATTERY_CLASS "battery"
+
+#define ACPI_BATTERY_NOTIFY_STATUS	0x80
+#define ACPI_BATTERY_NOTIFY_INFO	0x81
+#define ACPI_BATTERY_NOTIFY_THRESHOLD   0x82
+
+struct acpi_battery_hook {
+	const char *name;
+	int (*add_battery)(struct power_supply *battery);
+	int (*remove_battery)(struct power_supply *battery);
+	struct list_head list;
+};
+
+void battery_hook_register(struct acpi_battery_hook *hook);
+void battery_hook_unregister(struct acpi_battery_hook *hook);
+
+#endif
Signature · 1 line
2.7.4
#1 lore raw
Quoted 35 lines · Ognjen Galic

This is a patch that implements a generic hooking API for the generic ACPI battery driver.

With this new generic API, drivers can expose platform specific behaviour via sysfs attributes in /sys/class/power_supply/BATn/ in a generic way.

A perfect example of the need for this API are Lenovo ThinkPads.

Lenovo ThinkPads have a ACPI extension that allows the setting of start and stop charge thresholds in the EC and battery firmware via ACPI. The thinkpad_acpi module can use this API to expose sysfs attributes that it controls inside the ACPI battery driver sysfs tree, under /sys/class/power_supply/BATN/.

The file drivers/acpi/battery.h has been moved to include/acpi/battery.h and the includes inside ac.c, sbs.c, and battery.c have been adjusted to reflect that.

When drivers hooks into the API, the API calls add_battery() for each battery in the system that passes it a acpi_battery struct. Then, the drivers can use device_create_file() to create new sysfs attributes with that struct and identify the batteries for per-battery attributes.
---
v7:
* Implemented mutual exclusion for hooking methods and battery callbacks
* Fixed a BUG where errors in other modules would occur when the modules that depend on battery get unloaded

v8:
* Use list_for_each_safe instead of list_for_each for the module exit function where deletion of nodes occurs

Quoted 5 lines

v9:
* No changes in this patch in v9

v10:
* Fix compiler warnings in Intel's 0-day CI

You're sending new revisions too fast for me to review them. :-/

Please stop doing this for a while until you get a response from me for the v10.

Thanks,
Rafael

author
#2 lore raw
Quoted 36 lines · Rafael J. Wysocki
Quoted 35 lines · Ognjen Galic

This is a patch that implements a generic hooking API for the generic ACPI battery driver.

With this new generic API, drivers can expose platform specific behaviour via sysfs attributes in /sys/class/power_supply/BATn/ in a generic way.

A perfect example of the need for this API are Lenovo ThinkPads.

Lenovo ThinkPads have a ACPI extension that allows the setting of start and stop charge thresholds in the EC and battery firmware via ACPI. The thinkpad_acpi module can use this API to expose sysfs attributes that it controls inside the ACPI battery driver sysfs tree, under /sys/class/power_supply/BATN/.

The file drivers/acpi/battery.h has been moved to include/acpi/battery.h and the includes inside ac.c, sbs.c, and battery.c have been adjusted to reflect that.

When drivers hooks into the API, the API calls add_battery() for each battery in the system that passes it a acpi_battery struct. Then, the drivers can use device_create_file() to create new sysfs attributes with that struct and identify the batteries for per-battery attributes.
---
v7:
* Implemented mutual exclusion for hooking methods and battery callbacks
* Fixed a BUG where errors in other modules would occur when the modules that depend on battery get unloaded

v8:
* Use list_for_each_safe instead of list_for_each for the module exit function where deletion of nodes occurs

Quoted 7 lines
Quoted 5 lines

v9:
* No changes in this patch in v9

v10:
* Fix compiler warnings in Intel's 0-day CI

You're sending new revisions too fast for me to review them. :-/

I'm sorry for the flooding, but it's really just small changes here and there, mainly syntax and style changes and some compiler warnings on Intel's 0-day CI. Nothing major.

Quoted 2 lines

Please stop doing this for a while until you get a response from me for the v10.

There are no major issues with v10 that I can see, so I will wait for your review.

Quoted 2 lines

Thanks,

Thank YOU for your time!

Quoted 1 line

Rafael