[PATCH 2/3 v8] thinkpad_acpi: Add support for battery thresholds

* [PATCH 2/3 v8] thinkpad_acpi: Add support for battery thresholds
@ 2017-12-21 10:00 Ognjen Galic
  2017-12-21 13:53   ` [Devel] " Andy Shevchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Ognjen Galic @ 2017-12-21 10:00 UTC (permalink / raw)
  To: Andy Shevchenko, Rafael J. Wysocki, Ognjen Galić,
	Rafael J. Wysocki, Len Brown, Robert Moore, Lv Zheng,
	ACPI Devel Maling List, devel, Darren Hart, Andy Shevchenko,
	Henrique de Moraes Holschuh, Sebastian Reichel, Platform Driver,
	ibm-acpi-devel, Linux PM, Christoph Böhmwalder, Kevin Locke

thinkpad_acpi registers two new attributes for each battery:

1) Charge start threshold
/sys/class/power_supply/BATN/charge_start_threshold

Valid values are [0, 99]. A value of 0 turns off the
start threshold wear control.

2) Charge stop threshold
/sys/class/power_supply/BATN/charge_stop_threshold

Valid values are [1, 100]. A value of 100 turns off
the stop threshold wear control. This must be
configured first.

This patch depends on the following patches:

"battery: Add the battery hooking API"

v2:
* Re-write the patch to make the changes in
battery.c generic as suggested by Rafael

v3:
* Fixed a bug where you could not set the stop
threshold to 100% when the start threshold is 0%
* Fixed the "Not Charging" quirk to match Lenovo's
BIOS Firmware specification

v4:
* Fixed a bug where you could not set the start
threshold to >stop if stop was 100%

v5:
* Migrated from symbol_get to native linking,
to fix module dependencies
* Fixed a bug where unloading the module would
cause a BUG inside battery
* Fixed a bug where you could unload battery
before unloading thinkpad_acpi

v6:
* Fixed all the style and naming issues pointed
out by Andy Shevchenko

v7:
* No changes in this patch in v7

v8:
* No changes in this patch in v8

Signed-off-by: Ognjen Galic <smclt30p@gmail.com>
---
 drivers/platform/x86/Kconfig         |   1 +
 drivers/platform/x86/thinkpad_acpi.c | 442 ++++++++++++++++++++++++++++++++++-
 include/linux/power_supply.h         |   2 +
 3 files changed, 444 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 2c745e8..d705c62 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -399,6 +399,7 @@ config SURFACE3_WMI
 config THINKPAD_ACPI
 	tristate "ThinkPad ACPI Laptop Extras"
 	depends on ACPI
+	depends on ACPI_BATTERY
 	depends on INPUT
 	depends on RFKILL || RFKILL = n
 	depends on ACPI_VIDEO || ACPI_VIDEO = n
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 117be48..06bcfbd 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -23,7 +23,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#define TPACPI_VERSION "0.25"
+#define TPACPI_VERSION "0.26"
 #define TPACPI_SYSFS_VERSION 0x030000
 
 /*
@@ -66,6 +66,7 @@
 #include <linux/seq_file.h>
 #include <linux/sysfs.h>
 #include <linux/backlight.h>
+#include <linux/bitops.h>
 #include <linux/fb.h>
 #include <linux/platform_device.h>
 #include <linux/hwmon.h>
@@ -78,13 +79,16 @@
 #include <linux/workqueue.h>
 #include <linux/acpi.h>
 #include <linux/pci_ids.h>
+#include <linux/power_supply.h>
 #include <linux/thinkpad_acpi.h>
 #include <sound/core.h>
 #include <sound/control.h>
 #include <sound/initval.h>
 #include <linux/uaccess.h>
+#include <acpi/battery.h>
 #include <acpi/video.h>
 
+
 /* ThinkPad CMOS commands */
 #define TP_CMOS_VOLUME_DOWN	0
 #define TP_CMOS_VOLUME_UP	1
@@ -331,6 +335,7 @@ static struct {
 	u32 sensors_pdev_attrs_registered:1;
 	u32 hotkey_poll_active:1;
 	u32 has_adaptive_kbd:1;
+	u32 battery:1;
 } tp_features;
 
 static struct {
@@ -9201,6 +9206,437 @@ static struct ibm_struct mute_led_driver_data = {
 	.resume = mute_led_resume,
 };
 
+/***************** Battery Wear Control Driver ******************/
+
+/* Metadata */
+
+#define GET_START	"BCTG"
+#define SET_START	"BCCS"
+#define GET_STOP	"BCSG"
+#define SET_STOP	"BCSS"
+
+#define START_ATTR "charge_start_threshold"
+#define STOP_ATTR  "charge_stop_threshold"
+
+enum {
+	BAT_ANY = 0,
+	BAT_PRIMARY = 1,
+	BAT_SECONDARY = 2
+};
+
+enum {
+	/* Error condition bit */
+	METHOD_ERR = (1 << 31),
+};
+
+enum {
+	/* This is used in the get/set helpers */
+	THRESHOLD_START,
+	THRESHOLD_STOP,
+};
+
+struct tpacpi_battery_data {
+	int charge_start;
+	int start_support;
+	int charge_stop;
+	int stop_support;
+};
+
+struct tpacpi_battery_driver_data {
+	struct tpacpi_battery_data batteries[3];
+	int individual_addressing;
+};
+
+static struct tpacpi_battery_driver_data battery_info;
+
+/* ACPI helpers/functions/probes */
+
+/**
+ * This evaluates a ACPI method call specific to the battery
+ * ACPI extension. The specifics are that an error is marked
+ * in the 32rd bit of the response, so we just check that here.
+ *
+ * Returns 0 on success
+ */
+static int tpacpi_battery_acpi_eval(char *method, int *ret, int param)
+{
+	if (!acpi_evalf(hkey_handle, ret, method, "dd", param)) {
+		acpi_handle_err(hkey_handle, "%s: evaluate failed", method);
+		return AE_ERROR;
+	}
+
+	if (*ret & METHOD_ERR) {
+		acpi_handle_err(hkey_handle,
+				"%s evaluated but flagged as error", method);
+		return AE_ERROR;
+	}
+
+	return AE_OK;
+}
+
+static int tpacpi_battery_get(int what, int battery, int *ret)
+{
+	switch (what) {
+
+	case THRESHOLD_START:
+
+		if (tpacpi_battery_acpi_eval(GET_START, ret, battery))
+			return -ENODEV;
+
+		/* The value is in the low 8 bits of the response */
+		*ret = *ret & 0xFF;
+		return 0;
+
+	case THRESHOLD_STOP:
+
+		if (tpacpi_battery_acpi_eval(GET_STOP, ret, battery))
+			return -ENODEV;
+
+		/* Value is in lower 8 bits */
+		*ret = *ret & 0xFF;
+
+		/*
+		 * On the stop value, if we return 0 that
+		 * does not make any sense. 0 means Default, which
+		 * means that charging stops at 100%, so we return
+		 * that.
+		 */
+		if (*ret == 0)
+			*ret = 100;
+
+		return 0;
+
+	default:
+		pr_crit("wrong parameter: %d", what);
+		return -EINVAL;
+	}
+
+}
+
+static int tpacpi_battery_set(int what, int battery, int value)
+{
+
+	int param = 0x0, ret = 0xFFFFFFFF;
+
+	/* The first 8 bits are the value of the threshold */
+	param = value;
+	/* The battery ID is in bits 8-9, 2 bits */
+	param |= battery << 8;
+
+	switch (what) {
+
+	case THRESHOLD_START:
+
+		if (tpacpi_battery_acpi_eval(SET_START, &ret, param)) {
+			pr_err("failed to set charge threshold on battery %d",
+					battery);
+			return -ENODEV;
+		}
+
+		return 0;
+
+	case THRESHOLD_STOP:
+
+		if (tpacpi_battery_acpi_eval(SET_STOP, &ret, param)) {
+			pr_err("failed to set charge stop threshold: %d", battery);
+			return -ENODEV;
+		}
+
+		return 0;
+
+	default:
+		pr_crit("wrong parameter: %d", what);
+		return -EINVAL;
+	}
+
+}
+
+static int tpacpi_battery_probe(int battery)
+{
+	int ret = 0;
+
+	/* Reset the struct */
+	memset(&battery_info, 0, sizeof(struct tpacpi_battery_driver_data));
+
+	/*
+	 * 1) Get the current start threshold
+	 * 2) Check for support
+	 * 3) Get the current stop threshold
+	 * 4) Check for support
+	 */
+
+	if (acpi_has_method(hkey_handle, GET_START)) {
+
+		if (tpacpi_battery_acpi_eval(GET_START, &ret, battery)) {
+			pr_err("Error probing battery %d\n", battery);
+			return -ENODEV;
+		}
+
+		/* Individual addressing is in bit 9 */
+		if (ret & BIT(9))
+			battery_info.individual_addressing = true;
+
+		/* Support is marked in bit 8 */
+		if (ret & BIT(8))
+			battery_info.batteries[battery].start_support = 1;
+		else
+			return -ENODEV;
+
+		if (tpacpi_battery_get(THRESHOLD_START, battery,
+			&battery_info.batteries[battery].charge_start)) {
+			pr_err("Error probing battery %d\n", battery);
+			return -ENODEV;
+		}
+
+	}
+
+	if (acpi_has_method(hkey_handle, GET_STOP)) {
+
+		if (tpacpi_battery_acpi_eval(GET_STOP, &ret, battery)) {
+			pr_err("Error probing battery stop; %d\n", battery);
+			return -ENODEV;
+		}
+
+		/* Support is marked in bit 8 */
+		if (ret & BIT(8))
+			battery_info.batteries[battery].stop_support = 1;
+		else
+			return -ENODEV;
+
+		if (tpacpi_battery_get(THRESHOLD_STOP, battery,
+			&battery_info.batteries[battery].charge_stop)) {
+			pr_err("Error probing battery stop: %d\n", battery);
+			return -ENODEV;
+		}
+	}
+
+	pr_info("battery %d registered (start %d, stop %d)",
+			battery,
+			battery_info.batteries[battery].charge_start,
+			battery_info.batteries[battery].charge_stop);
+
+	return 0;
+}
+
+/* General helper functions */
+
+static int tpacpi_battery_get_id(const char *battery_name)
+{
+
+	if (strcmp(battery_name, "BAT0") == 0)
+		return BAT_PRIMARY;
+	if (strcmp(battery_name, "BAT1") == 0)
+		return BAT_SECONDARY;
+
+	/*
+	 * If for some reason the battery is not BAT0 nor is it
+	 * BAT1, we will assume it's the default, first battery,
+	 * AKA primary.
+	 */
+	pr_warn("unknown battery %s, assuming primary", battery_name);
+	return BAT_PRIMARY;
+}
+
+static int tpacpi_battery_get_attr(struct device_attribute *attr)
+{
+	if (strcmp(START_ATTR, attr->attr.name) == 0)
+		return THRESHOLD_START;
+	if (strcmp(STOP_ATTR, attr->attr.name) == 0)
+		return THRESHOLD_STOP;
+
+	pr_crit("Invalid attribute: %s", attr->attr.name);
+	return -EINVAL;
+}
+
+/* sysfs interface */
+
+static ssize_t tpacpi_battery_store(struct device *dev,
+				   struct device_attribute *devattr,
+				   const char *buf, size_t count)
+{
+	int attr, battery;
+	unsigned long value;
+	struct power_supply *supply = to_power_supply(dev);
+
+	if (!supply) {
+		pr_err("Can't upcast to power_supply!");
+		return -ENODEV;
+	}
+
+	/*
+	 * Some systems have support for more than
+	 * one battery. If that is the case,
+	 * tpacpi_battery_probe marked that addressing
+	 * them individually is supported, so we do that
+	 * based on the device struct.
+	 *
+	 * On systems that are not supported, we assume
+	 * the primary as most of the ACPI calls fail
+	 * with "Any Battery" as the parameter.
+	 */
+	if (battery_info.individual_addressing)
+		/* BAT_PRIMARY or BAT_SECONDARY */
+		battery = tpacpi_battery_get_id(supply->desc->name);
+	else
+		battery = BAT_PRIMARY;
+
+	if (kstrtoul(buf, 10, &value))
+		return -EINVAL;
+
+	attr = tpacpi_battery_get_attr(devattr);
+
+	switch (attr) {
+
+	case THRESHOLD_START:
+
+		if (!battery_info.batteries[battery].start_support)
+			return -ENODEV;
+
+		/* valid values are [0, 99] */
+		if (value < 0 || value > 99)
+			return -EINVAL;
+
+		if (value > battery_info.batteries[battery].charge_stop)
+			return -EINVAL;
+
+		if (tpacpi_battery_set(THRESHOLD_START, battery, value))
+			return -ENODEV;
+
+		battery_info.batteries[battery].charge_stop = value;
+		return count;
+
+	case THRESHOLD_STOP:
+
+		if (!battery_info.batteries[battery].stop_support)
+			return -ENODEV;
+
+		/* valid values are [1, 100] */
+		if (value < 1 || value > 100)
+			return -EINVAL;
+
+
+		if (value < battery_info.batteries[battery].charge_start)
+			return -EINVAL;
+
+		battery_info.batteries[battery].charge_stop = value;
+
+		/*
+		 * When 100 is passed to stop, we need to flip
+		 * it to 0 as that the EC understands that as
+		 * "Default", which will charge to 100%
+		 */
+		if (value == 100)
+			value = 0;
+
+		if (tpacpi_battery_set(THRESHOLD_STOP, battery, value))
+			return -EINVAL;
+
+		return count;
+
+	default:
+		pr_crit("Wrong parameter: %d", attr);
+		return -EINVAL;
+	}
+
+	return count;
+}
+
+static ssize_t tpacpi_battery_show(struct device *dev,
+				  struct device_attribute *devattr,
+				  char *buf)
+{
+	int ret = 0x0, attr, battery;
+	struct power_supply *supply = to_power_supply(dev);
+
+	if (!supply) {
+		pr_crit("Can't upcast to power_supply!");
+		return -ENODEV;
+	}
+
+	/* THRESHOLD_START or THRESHOLD_STOP */
+	attr = tpacpi_battery_get_attr(devattr);
+
+	/*
+	 * Some systems have support for more than
+	 * one battery. If that is the case,
+	 * tpacpi_battery_probe marked that addressing
+	 * them individually is supported, so we do that
+	 * based on the device struct.
+	 *
+	 * On systems that are not supported, we assume
+	 * the primary as most of the ACPI calls fail
+	 * with "Any Battery" as the parameter.
+	 */
+	if (battery_info.individual_addressing)
+		/* BAT_PRIMARY or BAT_SECONDARY */
+		battery = tpacpi_battery_get_id(supply->desc->name);
+	else
+		battery = BAT_PRIMARY;
+
+	if (tpacpi_battery_get(attr, battery, &ret))
+		return -ENODEV;
+
+	return sprintf(buf, "%d\n", ret);
+}
+
+DEVICE_ATTR(charge_start_threshold, 0644,
+		tpacpi_battery_show, tpacpi_battery_store);
+DEVICE_ATTR(charge_stop_threshold, 0644,
+	tpacpi_battery_show, tpacpi_battery_store);
+
+static struct attribute *tpacpi_battery_attrs[] = {
+	&dev_attr_charge_start_threshold.attr,
+	&dev_attr_charge_stop_threshold.attr,
+	NULL,
+};
+
+ATTRIBUTE_GROUPS(tpacpi_battery);
+
+/* ACPI battery hooking */
+
+static int tpacpi_battery_add(struct power_supply *battery)
+{
+	int batteryid = tpacpi_battery_get_id(battery->desc->name);
+
+	if (tpacpi_battery_probe(batteryid))
+		return -ENODEV;
+
+	if (device_add_groups(&battery->dev, tpacpi_battery_groups))
+		return -ENODEV;
+
+	return 0;
+}
+
+static int tpacpi_battery_remove(struct power_supply *battery)
+{
+	device_remove_groups(&battery->dev, tpacpi_battery_groups);
+	return 0;
+}
+
+static struct acpi_battery_hook battery_hook = {
+	.add_battery = tpacpi_battery_add,
+	.remove_battery = tpacpi_battery_remove,
+	.name = "ThinkPad Battery Extension",
+};
+
+/* Subdriver init/exit */
+
+static int __init tpacpi_battery_init(struct ibm_init_struct *ibm)
+{
+	battery_hook_register(&battery_hook);
+	return 0;
+}
+
+static void tpacpi_battery_exit(void)
+{
+	battery_hook_unregister(&battery_hook);
+}
+
+static struct ibm_struct battery_driver_data = {
+	.name = "battery",
+	.exit = tpacpi_battery_exit,
+};
+
 /****************************************************************************
  ****************************************************************************
  *
@@ -9647,6 +10083,10 @@ static struct ibm_init_struct ibms_init[] __initdata = {
 		.init = mute_led_init,
 		.data = &mute_led_driver_data,
 	},
+	{
+		.init = tpacpi_battery_init,
+		.data = &battery_driver_data,
+	},
 };
 
 static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 79e90b3..f0139b4 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -371,6 +371,8 @@ devm_power_supply_register_no_ws(struct device *parent,
 extern void power_supply_unregister(struct power_supply *psy);
 extern int power_supply_powers(struct power_supply *psy, struct device *dev);
 
+#define to_power_supply(device) container_of(device, struct power_supply, dev)
+
 extern void *power_supply_get_drvdata(struct power_supply *psy);
 /* For APM emulation, think legacy userspace. */
 extern struct class *power_supply_class;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread