* [PATCH v2 1/4] power: supply: add charge_behaviour attributes
2021-11-23 23:27 [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge) Thomas Weißschuh
@ 2021-11-23 23:27 ` Thomas Weißschuh
2021-11-23 23:27 ` [PATCH v2 2/4] power: supply: add helpers for charge_behaviour sysfs Thomas Weißschuh
` (5 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Thomas Weißschuh @ 2021-11-23 23:27 UTC (permalink / raw)
To: linux-pm, Sebastian Reichel, ibm-acpi-devel, platform-driver-x86,
Mark Gross, Hans de Goede, Henrique de Moraes Holschuh
Cc: Thomas Weißschuh, linux-kernel, linrunner, bberg, hadess,
markpearson, nicolopiazzalunga, njoshi1, smclt30p
This a revised version of
"[RFC] add standardized attributes for force_discharge and inhibit_charge" [0],
incorporating discussion results.
The biggest change is the switch from two boolean attributes to a single
enum attribute.
[0] https://lore.kernel.org/platform-driver-x86/21569a89-8303-8573-05fb-c2fec29983d1@gmail.com/
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Documentation/ABI/testing/sysfs-class-power | 14 ++++++++++++++
include/linux/power_supply.h | 7 +++++++
2 files changed, 21 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
index f7904efc4cfa..cece094764f8 100644
--- a/Documentation/ABI/testing/sysfs-class-power
+++ b/Documentation/ABI/testing/sysfs-class-power
@@ -455,6 +455,20 @@ Description:
"Unknown", "Charging", "Discharging",
"Not charging", "Full"
+What: /sys/class/power_supply/<supply_name>/charge_behaviour
+Date: November 2021
+Contact: linux-pm@vger.kernel.org
+Description:
+ Represents the charging behaviour.
+
+ Access: Read, Write
+
+ Valid values:
+ ================ ====================================
+ auto: Charge normally, respect thresholds
+ inhibit-charge: Do not charge while AC is attached
+ force-discharge: Force discharge while AC is attached
+
What: /sys/class/power_supply/<supply_name>/technology
Date: May 2007
Contact: linux-pm@vger.kernel.org
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 9ca1f120a211..70c333e86293 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -132,6 +132,7 @@ enum power_supply_property {
POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX,
POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD, /* in percents! */
POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD, /* in percents! */
+ POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR,
POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT,
POWER_SUPPLY_PROP_INPUT_POWER_LIMIT,
@@ -202,6 +203,12 @@ enum power_supply_usb_type {
POWER_SUPPLY_USB_TYPE_APPLE_BRICK_ID, /* Apple Charging Method */
};
+enum power_supply_charge_behaviour {
+ POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO = 0,
+ POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE,
+ POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE,
+};
+
enum power_supply_notifier_events {
PSY_EVENT_PROP_CHANGED,
};
--
2.34.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 2/4] power: supply: add helpers for charge_behaviour sysfs
2021-11-23 23:27 [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge) Thomas Weißschuh
2021-11-23 23:27 ` [PATCH v2 1/4] power: supply: add charge_behaviour attributes Thomas Weißschuh
@ 2021-11-23 23:27 ` Thomas Weißschuh
2021-11-23 23:27 ` [PATCH v2 3/4] platform/x86: thinkpad_acpi: support force-discharge Thomas Weißschuh
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Thomas Weißschuh @ 2021-11-23 23:27 UTC (permalink / raw)
To: linux-pm, Sebastian Reichel, ibm-acpi-devel, platform-driver-x86,
Mark Gross, Hans de Goede, Henrique de Moraes Holschuh
Cc: Thomas Weißschuh, linux-kernel, linrunner, bberg, hadess,
markpearson, nicolopiazzalunga, njoshi1, smclt30p
These helper functions can be used by drivers to implement their own
sysfs-attributes.
This is useful for ACPI-drivers extending the default ACPI-battery with
their own charge_behaviour attributes.
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
drivers/power/supply/power_supply_sysfs.c | 55 +++++++++++++++++++++++
include/linux/power_supply.h | 9 ++++
2 files changed, 64 insertions(+)
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index c3d7cbcd4fad..5e3b8c15ddbe 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -133,6 +133,12 @@ static const char * const POWER_SUPPLY_SCOPE_TEXT[] = {
[POWER_SUPPLY_SCOPE_DEVICE] = "Device",
};
+static const char * const POWER_SUPPLY_CHARGE_BEHAVIOUR_TEXT[] = {
+ [POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO] = "auto",
+ [POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE] = "inhibit-charge",
+ [POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE] = "force-discharge",
+};
+
static struct power_supply_attr power_supply_attrs[] = {
/* Properties of type `int' */
POWER_SUPPLY_ENUM_ATTR(STATUS),
@@ -484,3 +490,52 @@ int power_supply_uevent(struct device *dev, struct kobj_uevent_env *env)
return ret;
}
+
+ssize_t power_supply_charge_behaviour_show(struct device *dev,
+ unsigned int available_behaviours,
+ enum power_supply_charge_behaviour current_behaviour,
+ char *buf)
+{
+ bool match = false, available, active;
+ ssize_t count = 0;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(POWER_SUPPLY_CHARGE_BEHAVIOUR_TEXT); i++) {
+ available = available_behaviours & BIT(i);
+ active = i == current_behaviour;
+
+ if (available && active) {
+ count += sysfs_emit_at(buf, count, "[%s] ",
+ POWER_SUPPLY_CHARGE_BEHAVIOUR_TEXT[i]);
+ match = true;
+ } else if (available) {
+ count += sysfs_emit_at(buf, count, "%s ",
+ POWER_SUPPLY_CHARGE_BEHAVIOUR_TEXT[i]);
+ }
+ }
+
+ if (!match) {
+ dev_warn(dev, "driver reporting unsupported charge behaviour\n");
+ return -EINVAL;
+ }
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+EXPORT_SYMBOL_GPL(power_supply_charge_behaviour_show);
+
+int power_supply_charge_behaviour_parse(unsigned int available_behaviours, const char *buf)
+{
+ int i = sysfs_match_string(POWER_SUPPLY_CHARGE_BEHAVIOUR_TEXT, buf);
+
+ if (i < 0)
+ return i;
+
+ if (available_behaviours & BIT(i))
+ return i;
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(power_supply_charge_behaviour_parse);
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 70c333e86293..71f0379c2af8 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -546,4 +546,13 @@ static inline
void power_supply_remove_hwmon_sysfs(struct power_supply *psy) {}
#endif
+#ifdef CONFIG_SYSFS
+ssize_t power_supply_charge_behaviour_show(struct device *dev,
+ unsigned int available_behaviours,
+ enum power_supply_charge_behaviour behaviour,
+ char *buf);
+
+int power_supply_charge_behaviour_parse(unsigned int available_behaviours, const char *buf);
+#endif
+
#endif /* __LINUX_POWER_SUPPLY_H__ */
--
2.34.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 3/4] platform/x86: thinkpad_acpi: support force-discharge
2021-11-23 23:27 [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge) Thomas Weißschuh
2021-11-23 23:27 ` [PATCH v2 1/4] power: supply: add charge_behaviour attributes Thomas Weißschuh
2021-11-23 23:27 ` [PATCH v2 2/4] power: supply: add helpers for charge_behaviour sysfs Thomas Weißschuh
@ 2021-11-23 23:27 ` Thomas Weißschuh
2021-11-23 23:27 ` [PATCH v2 4/4] platform/x86: thinkpad_acpi: support inhibit-charge Thomas Weißschuh
` (3 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Thomas Weißschuh @ 2021-11-23 23:27 UTC (permalink / raw)
To: linux-pm, Sebastian Reichel, ibm-acpi-devel, platform-driver-x86,
Mark Gross, Hans de Goede, Henrique de Moraes Holschuh
Cc: Thomas Weißschuh, linux-kernel, linrunner, bberg, hadess,
markpearson, nicolopiazzalunga, njoshi1, smclt30p
This adds support for the force-discharge charge_behaviour through the
embedded controller of ThinkPads.
Co-developed-by: Thomas Koch <linrunner@gmx.net>
Signed-off-by: Thomas Koch <linrunner@gmx.net>
Co-developed-by: Nicolò Piazzalunga <nicolopiazzalunga@gmail.com>
Signed-off-by: Nicolò Piazzalunga <nicolopiazzalunga@gmail.com>
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
This patch is based on https://lore.kernel.org/platform-driver-x86/c2504700-06e9-e7d8-80f7-de90b0b6dfb5@gmail.com/
---
drivers/platform/x86/thinkpad_acpi.c | 131 ++++++++++++++++++++++++++-
1 file changed, 127 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 9c632df734bb..e3567cc686fa 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9319,6 +9319,8 @@ static struct ibm_struct mute_led_driver_data = {
#define SET_START "BCCS"
#define GET_STOP "BCSG"
#define SET_STOP "BCSS"
+#define GET_DISCHARGE "BDSG"
+#define SET_DISCHARGE "BDSS"
enum {
BAT_ANY = 0,
@@ -9335,6 +9337,7 @@ enum {
/* This is used in the get/set helpers */
THRESHOLD_START,
THRESHOLD_STOP,
+ FORCE_DISCHARGE,
};
struct tpacpi_battery_data {
@@ -9342,6 +9345,7 @@ struct tpacpi_battery_data {
int start_support;
int charge_stop;
int stop_support;
+ unsigned int charge_behaviours;
};
struct tpacpi_battery_driver_data {
@@ -9399,6 +9403,12 @@ static int tpacpi_battery_get(int what, int battery, int *ret)
if (*ret == 0)
*ret = 100;
return 0;
+ case FORCE_DISCHARGE:
+ if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_DISCHARGE, ret, battery))
+ return -ENODEV;
+ /* The force discharge status is in bit 0 */
+ *ret = *ret & 0x01;
+ return 0;
default:
pr_crit("wrong parameter: %d", what);
return -EINVAL;
@@ -9427,12 +9437,49 @@ static int tpacpi_battery_set(int what, int battery, int value)
return -ENODEV;
}
return 0;
+ case FORCE_DISCHARGE:
+ /* Force discharge is in bit 0,
+ * break on AC attach is in bit 1 (won't work on some ThinkPads),
+ * battery ID is in bits 8-9, 2 bits.
+ */
+ if (ACPI_FAILURE(tpacpi_battery_acpi_eval(SET_DISCHARGE, &ret, param))) {
+ pr_err("failed to set force discharge on %d", battery);
+ return -ENODEV;
+ }
+ return 0;
default:
pr_crit("wrong parameter: %d", what);
return -EINVAL;
}
}
+static int tpacpi_battery_set_validate(int what, int battery, int value)
+{
+ int ret, v;
+
+ ret = tpacpi_battery_set(what, battery, value);
+ if (ret < 0)
+ return ret;
+
+ ret = tpacpi_battery_get(what, battery, &v);
+ if (ret < 0)
+ return ret;
+
+ if (v == value)
+ return 0;
+
+ msleep(500);
+
+ ret = tpacpi_battery_get(what, battery, &v);
+ if (ret < 0)
+ return ret;
+
+ if (v == value)
+ return 0;
+
+ return -EIO;
+}
+
static int tpacpi_battery_probe(int battery)
{
int ret = 0;
@@ -9445,6 +9492,8 @@ static int tpacpi_battery_probe(int battery)
* 2) Check for support
* 3) Get the current stop threshold
* 4) Check for support
+ * 5) Get the current force discharge status
+ * 6) Check for support
*/
if (acpi_has_method(hkey_handle, GET_START)) {
if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_START, &ret, battery)) {
@@ -9481,10 +9530,25 @@ static int tpacpi_battery_probe(int 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);
+ if (acpi_has_method(hkey_handle, GET_DISCHARGE)) {
+ if (ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_DISCHARGE, &ret, battery))) {
+ pr_err("Error probing battery discharge; %d\n", battery);
+ return -ENODEV;
+ }
+ /* Support is marked in bit 8 */
+ if (ret & BIT(8))
+ battery_info.batteries[battery].charge_behaviours |=
+ BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE);
+ }
+
+ battery_info.batteries[battery].charge_behaviours |=
+ BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO);
+
+ pr_info("battery %d registered (start %d, stop %d, behaviours: 0x%x)\n",
+ battery,
+ battery_info.batteries[battery].charge_start,
+ battery_info.batteries[battery].charge_stop,
+ battery_info.batteries[battery].charge_behaviours);
return 0;
}
@@ -9619,6 +9683,28 @@ static ssize_t charge_control_end_threshold_show(struct device *device,
return tpacpi_battery_show(THRESHOLD_STOP, device, buf);
}
+static ssize_t charge_behaviour_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ enum power_supply_charge_behaviour active = POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO;
+ struct power_supply *supply = to_power_supply(dev);
+ unsigned int available;
+ int ret, battery;
+
+ battery = tpacpi_battery_get_id(supply->desc->name);
+ available = battery_info.batteries[battery].charge_behaviours;
+
+ if (available & BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE)) {
+ if (tpacpi_battery_get(FORCE_DISCHARGE, battery, &ret))
+ return -ENODEV;
+ if (ret)
+ active = POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE;
+ }
+
+ return power_supply_charge_behaviour_show(dev, available, active, buf);
+}
+
static ssize_t charge_control_start_threshold_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
@@ -9633,8 +9719,44 @@ static ssize_t charge_control_end_threshold_store(struct device *dev,
return tpacpi_battery_store(THRESHOLD_STOP, dev, buf, count);
}
+static ssize_t charge_behaviour_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct power_supply *supply = to_power_supply(dev);
+ int selected, battery, ret = 0;
+ unsigned int available;
+
+ battery = tpacpi_battery_get_id(supply->desc->name);
+ available = battery_info.batteries[battery].charge_behaviours;
+ selected = power_supply_charge_behaviour_parse(available, buf);
+
+ if (selected < 0)
+ return selected;
+
+ switch (selected) {
+ case POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO:
+ if (available & BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE))
+ ret = tpacpi_battery_set_validate(FORCE_DISCHARGE, battery, 0);
+ if (ret < 0)
+ return ret;
+ break;
+ case POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE:
+ ret = tpacpi_battery_set_validate(FORCE_DISCHARGE, battery, 1);
+ if (ret < 0)
+ return ret;
+ break;
+ default:
+ dev_err(dev, "Unexpected charge behaviour: %d\n", selected);
+ return -EINVAL;
+ }
+
+ return count;
+}
+
static DEVICE_ATTR_RW(charge_control_start_threshold);
static DEVICE_ATTR_RW(charge_control_end_threshold);
+static DEVICE_ATTR_RW(charge_behaviour);
static struct device_attribute dev_attr_charge_start_threshold = __ATTR(
charge_start_threshold,
0644,
@@ -9653,6 +9775,7 @@ static struct attribute *tpacpi_battery_attrs[] = {
&dev_attr_charge_control_end_threshold.attr,
&dev_attr_charge_start_threshold.attr,
&dev_attr_charge_stop_threshold.attr,
+ &dev_attr_charge_behaviour.attr,
NULL,
};
--
2.34.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 4/4] platform/x86: thinkpad_acpi: support inhibit-charge
2021-11-23 23:27 [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge) Thomas Weißschuh
` (2 preceding siblings ...)
2021-11-23 23:27 ` [PATCH v2 3/4] platform/x86: thinkpad_acpi: support force-discharge Thomas Weißschuh
@ 2021-11-23 23:27 ` Thomas Weißschuh
2021-11-25 18:04 ` [ibm-acpi-devel] [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge) Kevin Locke
` (2 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Thomas Weißschuh @ 2021-11-23 23:27 UTC (permalink / raw)
To: linux-pm, Sebastian Reichel, ibm-acpi-devel, platform-driver-x86,
Mark Gross, Hans de Goede, Henrique de Moraes Holschuh
Cc: Thomas Weißschuh, linux-kernel, linrunner, bberg, hadess,
markpearson, nicolopiazzalunga, njoshi1, smclt30p
This adds support for the inhibit-charge charge_behaviour through the
embedded controller of ThinkPads.
Co-developed-by: Thomas Koch <linrunner@gmx.net>
Signed-off-by: Thomas Koch <linrunner@gmx.net>
Co-developed-by: Nicolò Piazzalunga <nicolopiazzalunga@gmail.com>
Signed-off-by: Nicolò Piazzalunga <nicolopiazzalunga@gmail.com>
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
This patch is based on https://lore.kernel.org/platform-driver-x86/d2808930-5840-6ffb-3a59-d235cdb1fe16@gmail.com/
---
drivers/platform/x86/thinkpad_acpi.c | 64 +++++++++++++++++++++++++++-
1 file changed, 62 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index e3567cc686fa..7f2f46c71482 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9321,6 +9321,8 @@ static struct ibm_struct mute_led_driver_data = {
#define SET_STOP "BCSS"
#define GET_DISCHARGE "BDSG"
#define SET_DISCHARGE "BDSS"
+#define GET_INHIBIT "BICG"
+#define SET_INHIBIT "BICS"
enum {
BAT_ANY = 0,
@@ -9338,6 +9340,7 @@ enum {
THRESHOLD_START,
THRESHOLD_STOP,
FORCE_DISCHARGE,
+ INHIBIT_CHARGE,
};
struct tpacpi_battery_data {
@@ -9409,6 +9412,12 @@ static int tpacpi_battery_get(int what, int battery, int *ret)
/* The force discharge status is in bit 0 */
*ret = *ret & 0x01;
return 0;
+ case INHIBIT_CHARGE:
+ if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_INHIBIT, ret, battery))
+ return -ENODEV;
+ /* The inhibit charge status is in bit 0 */
+ *ret = *ret & 0x01;
+ return 0;
default:
pr_crit("wrong parameter: %d", what);
return -EINVAL;
@@ -9447,6 +9456,22 @@ static int tpacpi_battery_set(int what, int battery, int value)
return -ENODEV;
}
return 0;
+ case INHIBIT_CHARGE:
+ /* When setting inhibit charge, we set a default value of
+ * always breaking on AC detach and the effective time is set to
+ * be permanent.
+ * The battery ID is in bits 4-5, 2 bits,
+ * the effective time is in bits 8-23, 2 bytes.
+ * A time of FFFF indicates forever.
+ */
+ param = value;
+ param |= battery << 4;
+ param |= 0xFFFF << 8;
+ if (ACPI_FAILURE(tpacpi_battery_acpi_eval(SET_INHIBIT, &ret, param))) {
+ pr_err("failed to set inhibit charge on %d", battery);
+ return -ENODEV;
+ }
+ return 0;
default:
pr_crit("wrong parameter: %d", what);
return -EINVAL;
@@ -9494,6 +9519,8 @@ static int tpacpi_battery_probe(int battery)
* 4) Check for support
* 5) Get the current force discharge status
* 6) Check for support
+ * 7) Get the current inhibit charge status
+ * 8) Check for support
*/
if (acpi_has_method(hkey_handle, GET_START)) {
if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_START, &ret, battery)) {
@@ -9540,6 +9567,16 @@ static int tpacpi_battery_probe(int battery)
battery_info.batteries[battery].charge_behaviours |=
BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE);
}
+ if (acpi_has_method(hkey_handle, GET_INHIBIT)) {
+ if (ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_INHIBIT, &ret, battery))) {
+ pr_err("Error probing battery inhibit charge; %d\n", battery);
+ return -ENODEV;
+ }
+ /* Support is marked in bit 5 */
+ if (ret & BIT(5))
+ battery_info.batteries[battery].charge_behaviours |=
+ BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE);
+ }
battery_info.batteries[battery].charge_behaviours |=
BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO);
@@ -9698,10 +9735,22 @@ static ssize_t charge_behaviour_show(struct device *dev,
if (available & BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE)) {
if (tpacpi_battery_get(FORCE_DISCHARGE, battery, &ret))
return -ENODEV;
- if (ret)
+ if (ret) {
active = POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE;
+ goto out;
+ }
+ }
+
+ if (available & BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE)) {
+ if (tpacpi_battery_get(INHIBIT_CHARGE, battery, &ret))
+ return -ENODEV;
+ if (ret) {
+ active = POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE;
+ goto out;
+ }
}
+out:
return power_supply_charge_behaviour_show(dev, available, active, buf);
}
@@ -9738,11 +9787,22 @@ static ssize_t charge_behaviour_store(struct device *dev,
case POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO:
if (available & BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE))
ret = tpacpi_battery_set_validate(FORCE_DISCHARGE, battery, 0);
+ if (available & BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE))
+ ret = min(ret, tpacpi_battery_set_validate(INHIBIT_CHARGE, battery, 0));
if (ret < 0)
return ret;
break;
case POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE:
- ret = tpacpi_battery_set_validate(FORCE_DISCHARGE, battery, 1);
+ if (available & BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE))
+ ret = tpacpi_battery_set_validate(INHIBIT_CHARGE, battery, 0);
+ ret = min(ret, tpacpi_battery_set_validate(FORCE_DISCHARGE, battery, 1));
+ if (ret < 0)
+ return ret;
+ break;
+ case POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE:
+ if (available & BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE))
+ ret = tpacpi_battery_set_validate(FORCE_DISCHARGE, battery, 0);
+ ret = min(ret, tpacpi_battery_set_validate(INHIBIT_CHARGE, battery, 1));
if (ret < 0)
return ret;
break;
--
2.34.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [ibm-acpi-devel] [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge)
2021-11-23 23:27 [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge) Thomas Weißschuh
` (3 preceding siblings ...)
2021-11-23 23:27 ` [PATCH v2 4/4] platform/x86: thinkpad_acpi: support inhibit-charge Thomas Weißschuh
@ 2021-11-25 18:04 ` Kevin Locke
2021-12-03 21:33 ` Sebastian Reichel
2021-12-04 16:04 ` Thomas Koch
6 siblings, 0 replies; 15+ messages in thread
From: Kevin Locke @ 2021-11-25 18:04 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: linux-pm, ibm-acpi-devel, platform-driver-x86, linux-kernel
On Wed, 2021-11-24 at 00:27 +0100, Thomas Weißschuh wrote:
> this series adds support for the charge_behaviour property to the power
> subsystem and thinkpad_acpi driver.
Wonderful! Thanks for working on this.
I can confirm inhibit-charge and force-discharge states work with
patch v2 on v5.16-rc2 on a T430 (2342-CTO) with BIOS G1ETC2WW (2.82 ),
EC G1HT36WW and a single battery. Most behavior is as expected:
- With force-discharge, status becomes "Discharging" and energy_now
drops over time while AC remains connected.
- With inhibit-charge, status becomes "Unknown" and energy_now is
stable over time, even when not fully charged.
- With auto, status becomes "Charging" and energy_now rises over time.
- charge_behaviour takes precedence over
charge_control_{start,end}_threshold: status remains
Discharging/Unknown when below the start threshold, either due to
discharge or threshold change.
- charge_behaviour is preserved over soft reboot.
- inhibit-charge/auto are preserved across battery removal and
reinsertion.
- inhibit-charge/auto are preserved across s2ram (S3).
- With force-discharge, if the battery is removed, the machine
immediately powers off.
Some behavior is a little surprising:
- charge_behaviour can not be set to force-discharge if AC is
disconnected (EIO). If charge_behaviour is force-discharge when AC
is disconnected, it changes to auto, unlike inhibit-charge.
- charge_behavior force-discharge is not preserved across s2ram (S3),
unlike inhibit-charge.
- charge_behaviour is not preserved across hard reset (unlike charge
thresholds). Interestingly, it appears that inhibit-charge is
preserved across power-off (no charging is observed while powered
off) but not power-on, even though it is preserved across soft
reboot, as noted above.
I assume the behavior is under the control of the EC, so these aren't
criticisms of the patch. Just some observations.
Tested-by: Kevin Locke <kevin@kevinlocke.name>
Thanks again,
Kevin
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge)
2021-11-23 23:27 [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge) Thomas Weißschuh
` (4 preceding siblings ...)
2021-11-25 18:04 ` [ibm-acpi-devel] [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge) Kevin Locke
@ 2021-12-03 21:33 ` Sebastian Reichel
2021-12-04 13:56 ` Hans de Goede
` (2 more replies)
2021-12-04 16:04 ` Thomas Koch
6 siblings, 3 replies; 15+ messages in thread
From: Sebastian Reichel @ 2021-12-03 21:33 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: linux-pm, ibm-acpi-devel, platform-driver-x86, Mark Gross,
Hans de Goede, Henrique de Moraes Holschuh, linux-kernel,
linrunner, bberg, hadess, markpearson, nicolopiazzalunga,
njoshi1, smclt30p
[-- Attachment #1: Type: text/plain, Size: 3087 bytes --]
Hi,
On Wed, Nov 24, 2021 at 12:27:00AM +0100, Thomas Weißschuh wrote:
> This series adds support for the charge_behaviour property to the power
> subsystem and thinkpad_acpi driver.
>
> As thinkpad_acpi has to use the 'struct power_supply' created by the generic
> ACPI driver it has to rely on custom sysfs attributes instead of proper
> power_supply properties to implement this property.
>
> Patch 1: Adds the power_supply documentation and basic public API
> Patch 2: Adds helpers to power_supply core to help drivers implement the
> charge_behaviour attribute
> Patch 3: Adds support for force-discharge to thinkpad_acpi.
> Patch 4: Adds support for inhibit-discharge to thinkpad_acpi.
>
> Patch 3 and 4 are largely taken from other patches and adapted to the new API.
> (Links are in the patch trailer)
>
> Ognjen Galic:
>
> Your S-o-b is on the original inhibit_charge and force_discharge patches.
> I would like to add you as Co-developed-by but to do that it will also require
> your S-o-b. Could you give your sign-offs for the new patches, so you can be
> properly attributed?
>
> Sebastian Reichel:
>
> Currently the series does not actually support the property as a proper
> powersupply property handled fully by power_supply_sysfs.c because there would
> be no user for this property.
I'm not too happy how the acpi-battery hooks work, but that's not
your fault and this patchset does not really make the situation
worse. So:
Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
-- Sebastian
> Previous discussions about the API:
>
> https://lore.kernel.org/platform-driver-x86/20211108192852.357473-1-linux@weissschuh.net/
> https://lore.kernel.org/platform-driver-x86/21569a89-8303-8573-05fb-c2fec29983d1@gmail.com/
>
> v1: https://lore.kernel.org/lkml/20211113104225.141333-1-linux@weissschuh.net/
> v1 -> v2:
>
> * Use sysfs_emit-APIs instead of plain sprintf
> * More cecks for actual feature availability
> * Validation of the written values
> * Read inhibit-charge via BICG instead of PSSG (peak shift state)
> * Don't mangle error numbers in charge_behaviour_store()
>
> Open points:
>
> Thomas Koch has observed that on a T450s with two batteries
> inhibit-charge on BAT0 will affect both batteries and for BAT1 it is ignored
> entirely, this seems to be a bug in the EC.
> On my T460s with two batteries it works correctly.
>
> Thomas Weißschuh (4):
> power: supply: add charge_behaviour attributes
> power: supply: add helpers for charge_behaviour sysfs
> platform/x86: thinkpad_acpi: support force-discharge
> platform/x86: thinkpad_acpi: support inhibit-charge
>
> Documentation/ABI/testing/sysfs-class-power | 14 ++
> drivers/platform/x86/thinkpad_acpi.c | 191 +++++++++++++++++++-
> drivers/power/supply/power_supply_sysfs.c | 51 ++++++
> include/linux/power_supply.h | 16 ++
> 4 files changed, 268 insertions(+), 4 deletions(-)
>
>
> base-commit: 66f4beaa6c1d28161f534471484b2daa2de1dce0
> --
> 2.34.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge)
2021-12-03 21:33 ` Sebastian Reichel
@ 2021-12-04 13:56 ` Hans de Goede
2021-12-16 15:41 ` Hans de Goede
2021-12-21 16:06 ` Hans de Goede
2 siblings, 0 replies; 15+ messages in thread
From: Hans de Goede @ 2021-12-04 13:56 UTC (permalink / raw)
To: Sebastian Reichel, Thomas Weißschuh
Cc: linux-pm, ibm-acpi-devel, platform-driver-x86, Mark Gross,
Henrique de Moraes Holschuh, linux-kernel, linrunner, bberg,
hadess, markpearson, nicolopiazzalunga, njoshi1, smclt30p
Hi,
On 12/3/21 22:33, Sebastian Reichel wrote:
> Hi,
>
> On Wed, Nov 24, 2021 at 12:27:00AM +0100, Thomas Weißschuh wrote:
>> This series adds support for the charge_behaviour property to the power
>> subsystem and thinkpad_acpi driver.
>>
>> As thinkpad_acpi has to use the 'struct power_supply' created by the generic
>> ACPI driver it has to rely on custom sysfs attributes instead of proper
>> power_supply properties to implement this property.
>>
>> Patch 1: Adds the power_supply documentation and basic public API
>> Patch 2: Adds helpers to power_supply core to help drivers implement the
>> charge_behaviour attribute
>> Patch 3: Adds support for force-discharge to thinkpad_acpi.
>> Patch 4: Adds support for inhibit-discharge to thinkpad_acpi.
>>
>> Patch 3 and 4 are largely taken from other patches and adapted to the new API.
>> (Links are in the patch trailer)
>>
>> Ognjen Galic:
>>
>> Your S-o-b is on the original inhibit_charge and force_discharge patches.
>> I would like to add you as Co-developed-by but to do that it will also require
>> your S-o-b. Could you give your sign-offs for the new patches, so you can be
>> properly attributed?
>>
>> Sebastian Reichel:
>>
>> Currently the series does not actually support the property as a proper
>> powersupply property handled fully by power_supply_sysfs.c because there would
>> be no user for this property.
>
> I'm not too happy how the acpi-battery hooks work, but that's not
> your fault and this patchset does not really make the situation
> worse. So:
>
> Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
I haven't looked at the thinkpad_apci.c bits closely yet (for this new version),
but assuming those are ready for merging too, we need to discuss about how
to merge this.
The thinkpad_acpi code has already seen quite a lot of changes in -next,
so I would like the thinkpad_acpi changes to go upstream through the
platform-drivers-x86.git tree to avoid conflicts.
As such I think it is best if you (Sebastian) can prepare an immutable
branch with patch 1 + 2 for me to merge. Then even if patch 3 + 4 need
more work, Thomas can just respin those on top of the immutable branch.
Alternatively I can take the entire series upstream through the
platform-drivers-x86.git tree if that is ok with you (Sebastian).
Either way please let me know how you want to proceed with this.
Regards,
Hans
>> Previous discussions about the API:
>>
>> https://lore.kernel.org/platform-driver-x86/20211108192852.357473-1-linux@weissschuh.net/
>> https://lore.kernel.org/platform-driver-x86/21569a89-8303-8573-05fb-c2fec29983d1@gmail.com/
>>
>> v1: https://lore.kernel.org/lkml/20211113104225.141333-1-linux@weissschuh.net/
>> v1 -> v2:
>>
>> * Use sysfs_emit-APIs instead of plain sprintf
>> * More cecks for actual feature availability
>> * Validation of the written values
>> * Read inhibit-charge via BICG instead of PSSG (peak shift state)
>> * Don't mangle error numbers in charge_behaviour_store()
>>
>> Open points:
>>
>> Thomas Koch has observed that on a T450s with two batteries
>> inhibit-charge on BAT0 will affect both batteries and for BAT1 it is ignored
>> entirely, this seems to be a bug in the EC.
>> On my T460s with two batteries it works correctly.
>>
>> Thomas Weißschuh (4):
>> power: supply: add charge_behaviour attributes
>> power: supply: add helpers for charge_behaviour sysfs
>> platform/x86: thinkpad_acpi: support force-discharge
>> platform/x86: thinkpad_acpi: support inhibit-charge
>>
>> Documentation/ABI/testing/sysfs-class-power | 14 ++
>> drivers/platform/x86/thinkpad_acpi.c | 191 +++++++++++++++++++-
>> drivers/power/supply/power_supply_sysfs.c | 51 ++++++
>> include/linux/power_supply.h | 16 ++
>> 4 files changed, 268 insertions(+), 4 deletions(-)
>>
>>
>> base-commit: 66f4beaa6c1d28161f534471484b2daa2de1dce0
>> --
>> 2.34.0
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge)
2021-12-03 21:33 ` Sebastian Reichel
2021-12-04 13:56 ` Hans de Goede
@ 2021-12-16 15:41 ` Hans de Goede
2021-12-21 16:06 ` Hans de Goede
2 siblings, 0 replies; 15+ messages in thread
From: Hans de Goede @ 2021-12-16 15:41 UTC (permalink / raw)
To: Sebastian Reichel, Thomas Weißschuh
Cc: linux-pm, ibm-acpi-devel, platform-driver-x86, Mark Gross,
Henrique de Moraes Holschuh, linux-kernel, linrunner, bberg,
hadess, markpearson, nicolopiazzalunga, njoshi1, smclt30p
Hi,
On 12/3/21 22:33, Sebastian Reichel wrote:
> Hi,
>
> On Wed, Nov 24, 2021 at 12:27:00AM +0100, Thomas Weißschuh wrote:
>> This series adds support for the charge_behaviour property to the power
>> subsystem and thinkpad_acpi driver.
>>
>> As thinkpad_acpi has to use the 'struct power_supply' created by the generic
>> ACPI driver it has to rely on custom sysfs attributes instead of proper
>> power_supply properties to implement this property.
>>
>> Patch 1: Adds the power_supply documentation and basic public API
>> Patch 2: Adds helpers to power_supply core to help drivers implement the
>> charge_behaviour attribute
>> Patch 3: Adds support for force-discharge to thinkpad_acpi.
>> Patch 4: Adds support for inhibit-discharge to thinkpad_acpi.
>>
>> Patch 3 and 4 are largely taken from other patches and adapted to the new API.
>> (Links are in the patch trailer)
>>
>> Ognjen Galic:
>>
>> Your S-o-b is on the original inhibit_charge and force_discharge patches.
>> I would like to add you as Co-developed-by but to do that it will also require
>> your S-o-b. Could you give your sign-offs for the new patches, so you can be
>> properly attributed?
>>
>> Sebastian Reichel:
>>
>> Currently the series does not actually support the property as a proper
>> powersupply property handled fully by power_supply_sysfs.c because there would
>> be no user for this property.
>
> I'm not too happy how the acpi-battery hooks work, but that's not
> your fault and this patchset does not really make the situation
> worse. So:
>
> Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Sebastian, what is the plan for taking this upstream ? Does your ack mean that
you are ok with me taking the entire series upstream through the pdx86 tree?
Or do you plan to apply patches 1-2 through linux-power-supply.git; and in that
case can you provide an inmmutable branch with those patches for me to merge
into pdx86/for-next so that I can then apply patches 3 + 4 there ?
Note merging everything through the linux-power-supply.git tree is non ideal
in this case because the thinkpad_acpi.c code already has a lot of changes
in pdx86/for-next.
Regards,
Hans
>> Previous discussions about the API:
>>
>> https://lore.kernel.org/platform-driver-x86/20211108192852.357473-1-linux@weissschuh.net/
>> https://lore.kernel.org/platform-driver-x86/21569a89-8303-8573-05fb-c2fec29983d1@gmail.com/
>>
>> v1: https://lore.kernel.org/lkml/20211113104225.141333-1-linux@weissschuh.net/
>> v1 -> v2:
>>
>> * Use sysfs_emit-APIs instead of plain sprintf
>> * More cecks for actual feature availability
>> * Validation of the written values
>> * Read inhibit-charge via BICG instead of PSSG (peak shift state)
>> * Don't mangle error numbers in charge_behaviour_store()
>>
>> Open points:
>>
>> Thomas Koch has observed that on a T450s with two batteries
>> inhibit-charge on BAT0 will affect both batteries and for BAT1 it is ignored
>> entirely, this seems to be a bug in the EC.
>> On my T460s with two batteries it works correctly.
>>
>> Thomas Weißschuh (4):
>> power: supply: add charge_behaviour attributes
>> power: supply: add helpers for charge_behaviour sysfs
>> platform/x86: thinkpad_acpi: support force-discharge
>> platform/x86: thinkpad_acpi: support inhibit-charge
>>
>> Documentation/ABI/testing/sysfs-class-power | 14 ++
>> drivers/platform/x86/thinkpad_acpi.c | 191 +++++++++++++++++++-
>> drivers/power/supply/power_supply_sysfs.c | 51 ++++++
>> include/linux/power_supply.h | 16 ++
>> 4 files changed, 268 insertions(+), 4 deletions(-)
>>
>>
>> base-commit: 66f4beaa6c1d28161f534471484b2daa2de1dce0
>> --
>> 2.34.0
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge)
2021-12-03 21:33 ` Sebastian Reichel
2021-12-04 13:56 ` Hans de Goede
2021-12-16 15:41 ` Hans de Goede
@ 2021-12-21 16:06 ` Hans de Goede
2 siblings, 0 replies; 15+ messages in thread
From: Hans de Goede @ 2021-12-21 16:06 UTC (permalink / raw)
To: Sebastian Reichel, Thomas Weißschuh
Cc: linux-pm, ibm-acpi-devel, platform-driver-x86, Mark Gross,
Henrique de Moraes Holschuh, linux-kernel, linrunner, bberg,
hadess, markpearson, nicolopiazzalunga, njoshi1, smclt30p
Hi,
On 12/3/21 22:33, Sebastian Reichel wrote:
> Hi,
>
> On Wed, Nov 24, 2021 at 12:27:00AM +0100, Thomas Weißschuh wrote:
>> This series adds support for the charge_behaviour property to the power
>> subsystem and thinkpad_acpi driver.
>>
>> As thinkpad_acpi has to use the 'struct power_supply' created by the generic
>> ACPI driver it has to rely on custom sysfs attributes instead of proper
>> power_supply properties to implement this property.
>>
>> Patch 1: Adds the power_supply documentation and basic public API
>> Patch 2: Adds helpers to power_supply core to help drivers implement the
>> charge_behaviour attribute
>> Patch 3: Adds support for force-discharge to thinkpad_acpi.
>> Patch 4: Adds support for inhibit-discharge to thinkpad_acpi.
>>
>> Patch 3 and 4 are largely taken from other patches and adapted to the new API.
>> (Links are in the patch trailer)
>>
>> Ognjen Galic:
>>
>> Your S-o-b is on the original inhibit_charge and force_discharge patches.
>> I would like to add you as Co-developed-by but to do that it will also require
>> your S-o-b. Could you give your sign-offs for the new patches, so you can be
>> properly attributed?
>>
>> Sebastian Reichel:
>>
>> Currently the series does not actually support the property as a proper
>> powersupply property handled fully by power_supply_sysfs.c because there would
>> be no user for this property.
>
> I'm not too happy how the acpi-battery hooks work, but that's not
> your fault and this patchset does not really make the situation
> worse. So:
>
> Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Sebastian, I have taken the liberty to assume that this means that you are
ok with merging the entire series through the pdx86 tree (I've done a test-merge
with linux-power-supply/for-next and there are no conflicts).
Thomas, Thank you for your patch-series, I've applied the series
to my review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
>
> -- Sebastian
>
>> Previous discussions about the API:
>>
>> https://lore.kernel.org/platform-driver-x86/20211108192852.357473-1-linux@weissschuh.net/
>> https://lore.kernel.org/platform-driver-x86/21569a89-8303-8573-05fb-c2fec29983d1@gmail.com/
>>
>> v1: https://lore.kernel.org/lkml/20211113104225.141333-1-linux@weissschuh.net/
>> v1 -> v2:
>>
>> * Use sysfs_emit-APIs instead of plain sprintf
>> * More cecks for actual feature availability
>> * Validation of the written values
>> * Read inhibit-charge via BICG instead of PSSG (peak shift state)
>> * Don't mangle error numbers in charge_behaviour_store()
>>
>> Open points:
>>
>> Thomas Koch has observed that on a T450s with two batteries
>> inhibit-charge on BAT0 will affect both batteries and for BAT1 it is ignored
>> entirely, this seems to be a bug in the EC.
>> On my T460s with two batteries it works correctly.
>>
>> Thomas Weißschuh (4):
>> power: supply: add charge_behaviour attributes
>> power: supply: add helpers for charge_behaviour sysfs
>> platform/x86: thinkpad_acpi: support force-discharge
>> platform/x86: thinkpad_acpi: support inhibit-charge
>>
>> Documentation/ABI/testing/sysfs-class-power | 14 ++
>> drivers/platform/x86/thinkpad_acpi.c | 191 +++++++++++++++++++-
>> drivers/power/supply/power_supply_sysfs.c | 51 ++++++
>> include/linux/power_supply.h | 16 ++
>> 4 files changed, 268 insertions(+), 4 deletions(-)
>>
>>
>> base-commit: 66f4beaa6c1d28161f534471484b2daa2de1dce0
>> --
>> 2.34.0
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge)
2021-11-23 23:27 [PATCH 0/4] power: supply: add charge_behaviour property (force-discharge, inhibit-charge) Thomas Weißschuh
` (5 preceding siblings ...)
2021-12-03 21:33 ` Sebastian Reichel
@ 2021-12-04 16:04 ` Thomas Koch
6 siblings, 0 replies; 15+ messages in thread
From: Thomas Koch @ 2021-12-04 16:04 UTC (permalink / raw)
To: Thomas Weißschuh, linux-pm, Sebastian Reichel,
ibm-acpi-devel, platform-driver-x86, Mark Gross, Hans de Goede,
Henrique de Moraes Holschuh
Cc: linux-kernel, bberg, hadess, markpearson, nicolopiazzalunga,
njoshi1, smclt30p
Hi Thomas,
On 24.11.21 00:27, Thomas Weißschuh wrote:
> Hi,
>
> this series adds support for the charge_behaviour property to the power
> subsystem and thinkpad_acpi driver.
>
> As thinkpad_acpi has to use the 'struct power_supply' created by the generic
> ACPI driver it has to rely on custom sysfs attributes instead of proper
> power_supply properties to implement this property.
>
> Patch 1: Adds the power_supply documentation and basic public API
> Patch 2: Adds helpers to power_supply core to help drivers implement the
> charge_behaviour attribute
> Patch 3: Adds support for force-discharge to thinkpad_acpi.
> Patch 4: Adds support for inhibit-discharge to thinkpad_acpi.
>
> Patch 3 and 4 are largely taken from other patches and adapted to the new API.
> (Links are in the patch trailer)
>
> Ognjen Galic:
>
> Your S-o-b is on the original inhibit_charge and force_discharge patches.
> I would like to add you as Co-developed-by but to do that it will also require
> your S-o-b. Could you give your sign-offs for the new patches, so you can be
> properly attributed?
>
> Sebastian Reichel:
>
> Currently the series does not actually support the property as a proper
> powersupply property handled fully by power_supply_sysfs.c because there would
> be no user for this property.
>
> Previous discussions about the API:
>
> https://lore.kernel.org/platform-driver-x86/20211108192852.357473-1-linux@weissschuh.net/
> https://lore.kernel.org/platform-driver-x86/21569a89-8303-8573-05fb-c2fec29983d1@gmail.com/
>
> v1: https://lore.kernel.org/lkml/20211113104225.141333-1-linux@weissschuh.net/
> v1 -> v2:
>
> * Use sysfs_emit-APIs instead of plain sprintf
> * More cecks for actual feature availability
> * Validation of the written values
> * Read inhibit-charge via BICG instead of PSSG (peak shift state)
> * Don't mangle error numbers in charge_behaviour_store()
>
> Open points:
>
> Thomas Koch has observed that on a T450s with two batteries
> inhibit-charge on BAT0 will affect both batteries and for BAT1 it is ignored
> entirely, this seems to be a bug in the EC.
> On my T460s with two batteries it works correctly.
>
> Thomas Weißschuh (4):
> power: supply: add charge_behaviour attributes
> power: supply: add helpers for charge_behaviour sysfs
> platform/x86: thinkpad_acpi: support force-discharge
> platform/x86: thinkpad_acpi: support inhibit-charge
>
> Documentation/ABI/testing/sysfs-class-power | 14 ++
> drivers/platform/x86/thinkpad_acpi.c | 191 +++++++++++++++++++-
> drivers/power/supply/power_supply_sysfs.c | 51 ++++++
> include/linux/power_supply.h | 16 ++
> 4 files changed, 268 insertions(+), 4 deletions(-)
>
>
> base-commit: 66f4beaa6c1d28161f534471484b2daa2de1dce0
>
Reviewed-by
: Thomas Koch <linrunner@gmx.net>
Tested-by: Thomas Koch <linrunner@gmx.net>
Works well on ThinkPad X220, T450s, X1C6 with the exception mentioned above.
The new API is included in TLP already [1].
[1]
https://github.com/linrunner/TLP/commit/f0bf18f847470ae495a68f9f0e30130b96348936
--
Freundliche Grüße / Kind regards,
Thomas Koch
Mail : linrunner@gmx.net
Web : https://linrunner.de/tlp
^ permalink raw reply [flat|nested] 15+ messages in thread