[PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[Ognjen Galic]

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.

Signed-off-by: Ognjen Galic <smclt30p@gmail.com>
---

Notes:
    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
    
    v9:
    * Use DEVICE_ATTR_RW instead of DEVICE_ATTR
    * Use bitopts.h instead of raw operators
    * Remove redundant whitespaces
    * Remove redundant comments
    * Move the power_supply changes to separate patch
    * Fix other various styling issues pointed out by
    Andy Shevchenko
    
    v10:
    * Fix a pasting error from v6 that prevents the setting
    of the start threshold with EINVAL
    
    v11:
    * Fix formatting of changelog
    
    v12:
    * Remove most whitespaces between ifs
    * Change int to acpi_status in tpacpi_battery_acpi_eval
    
    v13:
    * Change if (!tpacpi_... to if ACPI_FAILURE(...

 drivers/platform/x86/Kconfig         |   1 +
 drivers/platform/x86/thinkpad_acpi.c | 389 ++++++++++++++++++++++++++++++++++-
 2 files changed, 389 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 9a8f96465..0d13d30c1 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -425,6 +425,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 d5eaf3b1e..1c57ee2b6 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,11 +79,13 @@
 #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 */
@@ -335,6 +338,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 {
@@ -9209,6 +9213,385 @@ static struct ibm_struct mute_led_driver_data = {
 	.resume = mute_led_resume,
 };
 
+/*
+ * Battery Wear Control Driver
+ * Contact: Ognjen Galic <smclt30p@gmail.com>
+ */
+
+/* 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 = BIT(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.
+ */
+static acpi_status tpacpi_battery_acpi_eval(char *method, int *ret, int param)
+{
+	int response;
+
+	if (!acpi_evalf(hkey_handle, &response, method, "dd", param)) {
+		acpi_handle_err(hkey_handle, "%s: evaluate failed", method);
+		return AE_ERROR;
+	}
+	if (response & METHOD_ERR) {
+		acpi_handle_err(hkey_handle,
+				"%s evaluated but flagged as error", method);
+		return AE_ERROR;
+	}
+	*ret = response;
+	return AE_OK;
+}
+
+static int tpacpi_battery_get(int what, int battery, int *ret)
+{
+	switch (what) {
+	case THRESHOLD_START:
+		if ACPI_FAILURE(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 ACPI_FAILURE(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, ret;
+	/* 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 ACPI_FAILURE(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 ACPI_FAILURE(tpacpi_battery_acpi_eval(SET_STOP, &ret, param)) {
+			pr_err("failed to set 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;
+
+	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 ACPI_FAILURE(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 ACPI_FAILURE(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;
+}
+
+/* sysfs interface */
+
+static ssize_t tpacpi_battery_store(int what,
+				    struct device *dev,
+				    const char *buf, size_t count)
+{
+	struct power_supply *supply = to_power_supply(dev);
+	unsigned long value;
+	int battery, rval;
+	/*
+	 * 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;
+
+	rval = kstrtoul(buf, 10, &value);
+	if (rval)
+		return rval;
+
+	switch (what) {
+	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_start = 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", what);
+		return -EINVAL;
+	}
+	return count;
+}
+
+static ssize_t tpacpi_battery_show(int what,
+				   struct device *dev,
+				   char *buf)
+{
+	struct power_supply *supply = to_power_supply(dev);
+	int ret, battery;
+	/*
+	 * 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;
+	 * 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(what, battery, &ret))
+		return -ENODEV;
+	return sprintf(buf, "%d\n", ret);
+}
+
+static ssize_t charge_start_threshold_show(struct device *device,
+				struct device_attribute *attr,
+				char *buf)
+{
+	return tpacpi_battery_show(THRESHOLD_START, device, buf);
+}
+
+static ssize_t charge_stop_threshold_show(struct device *device,
+				struct device_attribute *attr,
+				char *buf)
+{
+	return tpacpi_battery_show(THRESHOLD_STOP, device, buf);
+}
+
+static ssize_t charge_start_threshold_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	return tpacpi_battery_store(THRESHOLD_START, dev, buf, count);
+}
+
+static ssize_t charge_stop_threshold_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	return tpacpi_battery_store(THRESHOLD_STOP, dev, buf, count);
+}
+
+static DEVICE_ATTR_RW(charge_start_threshold);
+static DEVICE_ATTR_RW(charge_stop_threshold);
+
+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,
+};
+
 /****************************************************************************
  ****************************************************************************
  *
@@ -9655,6 +10038,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)
-- 
2.14.1

Re: [PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[Rafael J. Wysocki]

On Wed, Feb 7, 2018 at 3:58 PM, Ognjen Galic <smclt30p@gmail.com> wrote:
> 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.
>
> Signed-off-by: Ognjen Galic <smclt30p@gmail.com>

Andy, Darren, Henrique,

I'm assuming no concerns about this you from you, please let me know
if that's not correct.

> ---
>
> Notes:
>     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
>
>     v9:
>     * Use DEVICE_ATTR_RW instead of DEVICE_ATTR
>     * Use bitopts.h instead of raw operators
>     * Remove redundant whitespaces
>     * Remove redundant comments
>     * Move the power_supply changes to separate patch
>     * Fix other various styling issues pointed out by
>     Andy Shevchenko
>
>     v10:
>     * Fix a pasting error from v6 that prevents the setting
>     of the start threshold with EINVAL
>
>     v11:
>     * Fix formatting of changelog
>
>     v12:
>     * Remove most whitespaces between ifs
>     * Change int to acpi_status in tpacpi_battery_acpi_eval
>
>     v13:
>     * Change if (!tpacpi_... to if ACPI_FAILURE(...
>
>  drivers/platform/x86/Kconfig         |   1 +
>  drivers/platform/x86/thinkpad_acpi.c | 389 ++++++++++++++++++++++++++++++++++-
>  2 files changed, 389 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 9a8f96465..0d13d30c1 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -425,6 +425,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 d5eaf3b1e..1c57ee2b6 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,11 +79,13 @@
>  #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 */
> @@ -335,6 +338,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 {
> @@ -9209,6 +9213,385 @@ static struct ibm_struct mute_led_driver_data = {
>         .resume = mute_led_resume,
>  };
>
> +/*
> + * Battery Wear Control Driver
> + * Contact: Ognjen Galic <smclt30p@gmail.com>
> + */
> +
> +/* 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 = BIT(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.
> + */
> +static acpi_status tpacpi_battery_acpi_eval(char *method, int *ret, int param)
> +{
> +       int response;
> +
> +       if (!acpi_evalf(hkey_handle, &response, method, "dd", param)) {
> +               acpi_handle_err(hkey_handle, "%s: evaluate failed", method);
> +               return AE_ERROR;
> +       }
> +       if (response & METHOD_ERR) {
> +               acpi_handle_err(hkey_handle,
> +                               "%s evaluated but flagged as error", method);
> +               return AE_ERROR;
> +       }
> +       *ret = response;
> +       return AE_OK;
> +}
> +
> +static int tpacpi_battery_get(int what, int battery, int *ret)
> +{
> +       switch (what) {
> +       case THRESHOLD_START:
> +               if ACPI_FAILURE(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 ACPI_FAILURE(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, ret;
> +       /* 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 ACPI_FAILURE(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 ACPI_FAILURE(tpacpi_battery_acpi_eval(SET_STOP, &ret, param)) {
> +                       pr_err("failed to set 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;
> +
> +       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 ACPI_FAILURE(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 ACPI_FAILURE(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;
> +}
> +
> +/* sysfs interface */
> +
> +static ssize_t tpacpi_battery_store(int what,
> +                                   struct device *dev,
> +                                   const char *buf, size_t count)
> +{
> +       struct power_supply *supply = to_power_supply(dev);
> +       unsigned long value;
> +       int battery, rval;
> +       /*
> +        * 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;
> +
> +       rval = kstrtoul(buf, 10, &value);
> +       if (rval)
> +               return rval;
> +
> +       switch (what) {
> +       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_start = 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", what);
> +               return -EINVAL;
> +       }
> +       return count;
> +}
> +
> +static ssize_t tpacpi_battery_show(int what,
> +                                  struct device *dev,
> +                                  char *buf)
> +{
> +       struct power_supply *supply = to_power_supply(dev);
> +       int ret, battery;
> +       /*
> +        * 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;
> +        * 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(what, battery, &ret))
> +               return -ENODEV;
> +       return sprintf(buf, "%d\n", ret);
> +}
> +
> +static ssize_t charge_start_threshold_show(struct device *device,
> +                               struct device_attribute *attr,
> +                               char *buf)
> +{
> +       return tpacpi_battery_show(THRESHOLD_START, device, buf);
> +}
> +
> +static ssize_t charge_stop_threshold_show(struct device *device,
> +                               struct device_attribute *attr,
> +                               char *buf)
> +{
> +       return tpacpi_battery_show(THRESHOLD_STOP, device, buf);
> +}
> +
> +static ssize_t charge_start_threshold_store(struct device *dev,
> +                               struct device_attribute *attr,
> +                               const char *buf, size_t count)
> +{
> +       return tpacpi_battery_store(THRESHOLD_START, dev, buf, count);
> +}
> +
> +static ssize_t charge_stop_threshold_store(struct device *dev,
> +                               struct device_attribute *attr,
> +                               const char *buf, size_t count)
> +{
> +       return tpacpi_battery_store(THRESHOLD_STOP, dev, buf, count);
> +}
> +
> +static DEVICE_ATTR_RW(charge_start_threshold);
> +static DEVICE_ATTR_RW(charge_stop_threshold);
> +
> +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,
> +};
> +
>  /****************************************************************************
>   ****************************************************************************
>   *
> @@ -9655,6 +10038,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)
> --
> 2.14.1
>

Re: [PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 16070 bytes --]

Hi,

On Wed, Feb 07, 2018 at 03:58:44PM +0100, Ognjen Galic wrote:
> 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 is a new sysfs file, that should be documented. Also this
looks pretty generic. Just introduce new POWER_SUPPLY_PROP_
entries for start/stop charging threshold and use them.

-- Sebastian

> Signed-off-by: Ognjen Galic <smclt30p@gmail.com>
> ---
> 
> Notes:
>     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
>     
>     v9:
>     * Use DEVICE_ATTR_RW instead of DEVICE_ATTR
>     * Use bitopts.h instead of raw operators
>     * Remove redundant whitespaces
>     * Remove redundant comments
>     * Move the power_supply changes to separate patch
>     * Fix other various styling issues pointed out by
>     Andy Shevchenko
>     
>     v10:
>     * Fix a pasting error from v6 that prevents the setting
>     of the start threshold with EINVAL
>     
>     v11:
>     * Fix formatting of changelog
>     
>     v12:
>     * Remove most whitespaces between ifs
>     * Change int to acpi_status in tpacpi_battery_acpi_eval
>     
>     v13:
>     * Change if (!tpacpi_... to if ACPI_FAILURE(...
> 
>  drivers/platform/x86/Kconfig         |   1 +
>  drivers/platform/x86/thinkpad_acpi.c | 389 ++++++++++++++++++++++++++++++++++-
>  2 files changed, 389 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 9a8f96465..0d13d30c1 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -425,6 +425,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 d5eaf3b1e..1c57ee2b6 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,11 +79,13 @@
>  #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 */
> @@ -335,6 +338,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 {
> @@ -9209,6 +9213,385 @@ static struct ibm_struct mute_led_driver_data = {
>  	.resume = mute_led_resume,
>  };
>  
> +/*
> + * Battery Wear Control Driver
> + * Contact: Ognjen Galic <smclt30p@gmail.com>
> + */
> +
> +/* 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 = BIT(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.
> + */
> +static acpi_status tpacpi_battery_acpi_eval(char *method, int *ret, int param)
> +{
> +	int response;
> +
> +	if (!acpi_evalf(hkey_handle, &response, method, "dd", param)) {
> +		acpi_handle_err(hkey_handle, "%s: evaluate failed", method);
> +		return AE_ERROR;
> +	}
> +	if (response & METHOD_ERR) {
> +		acpi_handle_err(hkey_handle,
> +				"%s evaluated but flagged as error", method);
> +		return AE_ERROR;
> +	}
> +	*ret = response;
> +	return AE_OK;
> +}
> +
> +static int tpacpi_battery_get(int what, int battery, int *ret)
> +{
> +	switch (what) {
> +	case THRESHOLD_START:
> +		if ACPI_FAILURE(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 ACPI_FAILURE(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, ret;
> +	/* 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 ACPI_FAILURE(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 ACPI_FAILURE(tpacpi_battery_acpi_eval(SET_STOP, &ret, param)) {
> +			pr_err("failed to set 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;
> +
> +	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 ACPI_FAILURE(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 ACPI_FAILURE(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;
> +}
> +
> +/* sysfs interface */
> +
> +static ssize_t tpacpi_battery_store(int what,
> +				    struct device *dev,
> +				    const char *buf, size_t count)
> +{
> +	struct power_supply *supply = to_power_supply(dev);
> +	unsigned long value;
> +	int battery, rval;
> +	/*
> +	 * 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;
> +
> +	rval = kstrtoul(buf, 10, &value);
> +	if (rval)
> +		return rval;
> +
> +	switch (what) {
> +	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_start = 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", what);
> +		return -EINVAL;
> +	}
> +	return count;
> +}
> +
> +static ssize_t tpacpi_battery_show(int what,
> +				   struct device *dev,
> +				   char *buf)
> +{
> +	struct power_supply *supply = to_power_supply(dev);
> +	int ret, battery;
> +	/*
> +	 * 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;
> +	 * 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(what, battery, &ret))
> +		return -ENODEV;
> +	return sprintf(buf, "%d\n", ret);
> +}
> +
> +static ssize_t charge_start_threshold_show(struct device *device,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	return tpacpi_battery_show(THRESHOLD_START, device, buf);
> +}
> +
> +static ssize_t charge_stop_threshold_show(struct device *device,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	return tpacpi_battery_show(THRESHOLD_STOP, device, buf);
> +}
> +
> +static ssize_t charge_start_threshold_store(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t count)
> +{
> +	return tpacpi_battery_store(THRESHOLD_START, dev, buf, count);
> +}
> +
> +static ssize_t charge_stop_threshold_store(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t count)
> +{
> +	return tpacpi_battery_store(THRESHOLD_STOP, dev, buf, count);
> +}
> +
> +static DEVICE_ATTR_RW(charge_start_threshold);
> +static DEVICE_ATTR_RW(charge_stop_threshold);
> +
> +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,
> +};
> +
>  /****************************************************************************
>   ****************************************************************************
>   *
> @@ -9655,6 +10038,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)
> -- 
> 2.14.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[Rafael J. Wysocki]

On Thu, Feb 8, 2018 at 11:03 PM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Wed, Feb 07, 2018 at 03:58:44PM +0100, Ognjen Galic wrote:
>> 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 is a new sysfs file, that should be documented.

Right, I should have remembered about that, sorry.

> Also this looks pretty generic. Just introduce new POWER_SUPPLY_PROP_
> entries for start/stop charging threshold and use them.

What about doing this as a follow-up?

Re: [PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[Henrique de Moraes Holschuh]

Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

-- 
  Henrique Holschuh

Re: [PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 998 bytes --]

Hi,

On Fri, Feb 09, 2018 at 10:22:54AM +0100, Rafael J. Wysocki wrote:
> On Thu, Feb 8, 2018 at 11:03 PM, Sebastian Reichel <sre@kernel.org> wrote:
> > Hi,
> >
> > On Wed, Feb 07, 2018 at 03:58:44PM +0100, Ognjen Galic wrote:
> >> 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 is a new sysfs file, that should be documented.
> 
> Right, I should have remembered about that, sorry.
> 
> > Also this looks pretty generic. Just introduce new POWER_SUPPLY_PROP_
> > entries for start/stop charging threshold and use them.
> 
> What about doing this as a follow-up?

Fine with me.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[Rafael J. Wysocki]

On Fri, Feb 9, 2018 at 11:39 AM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Fri, Feb 09, 2018 at 10:22:54AM +0100, Rafael J. Wysocki wrote:
>> On Thu, Feb 8, 2018 at 11:03 PM, Sebastian Reichel <sre@kernel.org> wrote:
>> > Hi,
>> >
>> > On Wed, Feb 07, 2018 at 03:58:44PM +0100, Ognjen Galic wrote:
>> >> 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 is a new sysfs file, that should be documented.
>>
>> Right, I should have remembered about that, sorry.
>>
>> > Also this looks pretty generic. Just introduce new POWER_SUPPLY_PROP_
>> > entries for start/stop charging threshold and use them.
>>
>> What about doing this as a follow-up?
>
> Fine with me.

OK

Actually, I don't see any documentation whatever for ACPI battery and
AC power supply properties, so I guess that needs to be added in
general and I don't think it would be fair to ask Ognjen to do that in
order to get the extension in.

Why don't we pencil this in as work to do?

Re: [PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]

Hi,

On Fri, Feb 09, 2018 at 11:49:40AM +0100, Rafael J. Wysocki wrote:
> On Fri, Feb 9, 2018 at 11:39 AM, Sebastian Reichel <sre@kernel.org> wrote:
> > Hi,
> >
> > On Fri, Feb 09, 2018 at 10:22:54AM +0100, Rafael J. Wysocki wrote:
> >> On Thu, Feb 8, 2018 at 11:03 PM, Sebastian Reichel <sre@kernel.org> wrote:
> >> > Hi,
> >> >
> >> > On Wed, Feb 07, 2018 at 03:58:44PM +0100, Ognjen Galic wrote:
> >> >> 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 is a new sysfs file, that should be documented.
> >>
> >> Right, I should have remembered about that, sorry.
> >>
> >> > Also this looks pretty generic. Just introduce new POWER_SUPPLY_PROP_
> >> > entries for start/stop charging threshold and use them.
> >>
> >> What about doing this as a follow-up?
> >
> > Fine with me.
> 
> OK
> 
> Actually, I don't see any documentation whatever for ACPI battery and
> AC power supply properties, so I guess that needs to be added in
> general and I don't think it would be fair to ask Ognjen to do that in
> order to get the extension in.
> 
> Why don't we pencil this in as work to do?

The generic ones are documented here:

Documentation/power/power_supply_class.txt

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[Ognjen Galić]

On Fri, Feb 09, 2018 at 01:34:14PM +0100, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, Feb 09, 2018 at 11:49:40AM +0100, Rafael J. Wysocki wrote:
> > On Fri, Feb 9, 2018 at 11:39 AM, Sebastian Reichel <sre@kernel.org> wrote:
> > > Hi,
> > >
> > > On Fri, Feb 09, 2018 at 10:22:54AM +0100, Rafael J. Wysocki wrote:
> > >> On Thu, Feb 8, 2018 at 11:03 PM, Sebastian Reichel <sre@kernel.org> wrote:
> > >> > Hi,
> > >> >
> > >> > On Wed, Feb 07, 2018 at 03:58:44PM +0100, Ognjen Galic wrote:
> > >> >> 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 is a new sysfs file, that should be documented.
> > >>
> > >> Right, I should have remembered about that, sorry.
> > >>
> > >> > Also this looks pretty generic. Just introduce new POWER_SUPPLY_PROP_
> > >> > entries for start/stop charging threshold and use them.
> > >>
> > >> What about doing this as a follow-up?
> > >
> > > Fine with me.
> > 
> > OK
> > 
> > Actually, I don't see any documentation whatever for ACPI battery and
> > AC power supply properties, so I guess that needs to be added in
> > general and I don't think it would be fair to ask Ognjen to do that in
> > order to get the extension in.
> > 
> > Why don't we pencil this in as work to do?
> 
> The generic ones are documented here:

Do you guys want me to send in another revision of the patch with some
documentation on the sysfs API?

> 
> Documentation/power/power_supply_class.txt
> 
> -- Sebastian

Re: [PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[Andy Shevchenko]

On Sat, Feb 10, 2018 at 10:48 AM, Ognjen Galić <smclt30p@gmail.com> wrote:

> Do you guys want me to send in another revision of the patch with some
> documentation on the sysfs API?

I noticed I didn't apply it because of some pending changes discussed,
perhaps this one above.
So, definitely please send a new version which addresses comments.

-- 
With Best Regards,
Andy Shevchenko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Re: [PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[Rafael J. Wysocki]

On Fri, Feb 23, 2018 at 6:21 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sat, Feb 10, 2018 at 10:48 AM, Ognjen Galić <smclt30p@gmail.com> wrote:
>
>> Do you guys want me to send in another revision of the patch with some
>> documentation on the sysfs API?
>
> I noticed I didn't apply it because of some pending changes discussed,
> perhaps this one above.
>
> So, definitely please send a new version which addresses comments.

No, it actually has been applied already.

What is needed is a follow-up patch, but I'll write on that later.

Thanks,
Rafael

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Re: [PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[Ognjen Galić]

On Sat, Feb 24, 2018 at 10:07:20AM +0100, Rafael J. Wysocki wrote:
> On Fri, Feb 23, 2018 at 6:21 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Sat, Feb 10, 2018 at 10:48 AM, Ognjen Galić <smclt30p@gmail.com> wrote:
> >
> >> Do you guys want me to send in another revision of the patch with some
> >> documentation on the sysfs API?
> >
> > I noticed I didn't apply it because of some pending changes discussed,
> > perhaps this one above.
> >
> > So, definitely please send a new version which addresses comments.
> 
> No, it actually has been applied already.

So it has been queued up for 4.17?

If it has, that is awesome! Thanks!

> 
> What is needed is a follow-up patch, but I'll write on that later.
> 
> Thanks,
> Rafael

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Re: [PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[Ognjen Galić]

On Sat, Feb 24, 2018 at 10:07:20AM +0100, Rafael J. Wysocki wrote:
> On Fri, Feb 23, 2018 at 6:21 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Sat, Feb 10, 2018 at 10:48 AM, Ognjen Galić <smclt30p@gmail.com> wrote:
> >
> >> Do you guys want me to send in another revision of the patch with some
> >> documentation on the sysfs API?
> >
> > I noticed I didn't apply it because of some pending changes discussed,
> > perhaps this one above.
> >
> > So, definitely please send a new version which addresses comments.
> 
> No, it actually has been applied already.

And also, how is it applied if it does not apply to 4.16-rc2 anymore? 
Do you need a new revision that solves the apply conflicts?
Do you need a new revision that addresses the documentation?
Did you apply it earlier than 4.16-rc2? 
When did you apply it? 
Did you apply it at all?

> 
> What is needed is a follow-up patch, but I'll write on that later.
> 
> Thanks,
> Rafael

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Re: [PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

[Rafael J. Wysocki]

On Sat, Feb 24, 2018 at 6:32 PM, Ognjen Galić <smclt30p@gmail.com> wrote:
> On Sat, Feb 24, 2018 at 10:07:20AM +0100, Rafael J. Wysocki wrote:
>> On Fri, Feb 23, 2018 at 6:21 PM, Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
>> > On Sat, Feb 10, 2018 at 10:48 AM, Ognjen Galić <smclt30p@gmail.com> wrote:
>> >
>> >> Do you guys want me to send in another revision of the patch with some
>> >> documentation on the sysfs API?
>> >
>> > I noticed I didn't apply it because of some pending changes discussed,
>> > perhaps this one above.
>> >
>> > So, definitely please send a new version which addresses comments.
>>
>> No, it actually has been applied already.
>
> And also, how is it applied if it does not apply to 4.16-rc2 anymore?
> Do you need a new revision that solves the apply conflicts?
> Do you need a new revision that addresses the documentation?
> Did you apply it earlier than 4.16-rc2?
> When did you apply it?
> Did you apply it at all?

Yes, I did, on top of 4.16-rc2, and I might have rebased it a bit.

Please see linux-next and I will expose the acpi-battery branch
containing this series later today and tomorrow.

>> What is needed is a follow-up patch, but I'll write on that later.

Please see my previous message in this thread regarding the above.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel