Skip to contentSkip to conversation

thinkpad_acpi: add support for force_discharge

PATCH In review Ognjen Galic 3 participants 4 messages May 13, 2018 – Jun 10, 2018 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 2/3 thinkpad_acpi: add support for force_discharge

Lenovo ThinkPad systems have a feature that lets you force the battery to discharge regardless if AC is attached or not.

This patch implements that feature and exposes it via the generic ACPI battery driver.

---
 drivers/platform/x86/thinkpad_acpi.c | 56 ++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 3 deletions(-)
drivers/platform/x86/thinkpad_acpi.c +53 -3
index b8b74889..28a87640 100644
@@ -9236,7 +9236,11 @@ static struct ibm_struct mute_led_driver_data = {
 #define SET_INHIBIT	"BICS"
 #define GET_INHIBIT	"BICG"
 
+#define GET_DISCHARGE	"BDSG"
+#define SET_DISCHARGE	"PSBS"
+
 #define INHIBIT_ATTR	"inhibit_charge"
+#define DISCHARGE_ATTR	"force_discharge"
 
 #define START_ATTR "charge_start_threshold"
 #define STOP_ATTR  "charge_stop_threshold"
@@ -9256,7 +9260,8 @@ enum {
 	/* This is used in the get/set helpers */
 	THRESHOLD_START,
 	THRESHOLD_STOP,
-	INHIBIT_CHARGE
+	INHIBIT_CHARGE,
+	FORCE_DISCHARGE
 };
 
 struct tpacpi_battery_data {
@@ -9265,6 +9270,7 @@ struct tpacpi_battery_data {
 	int charge_stop;
 	int stop_support;
 	int inhibit_support;
+	int discharge_support;
 };
 
 struct tpacpi_battery_driver_data {
@@ -9329,6 +9335,12 @@ static int tpacpi_battery_get(int what, int battery, int *ret)
 		/* The inhibit charge status is in the first bit */
 		*ret = *ret & 0x01;
 		return 0;
+	case FORCE_DISCHARGE:
+		if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_DISCHARGE, ret, battery))
+			return -ENODEV;
+		/* The force discharge status is in the first bit */
+		*ret = *ret & 0x01;
+		return 0;
 	default:
 		pr_crit("wrong parameter: %d", what);
 		return -EINVAL;
@@ -9372,6 +9384,14 @@ static int tpacpi_battery_set(int what, int battery, int value)
 			return -ENODEV;
 		}
 		return 0;
+	case FORCE_DISCHARGE:
+		param = battery;
+		param |= value << 8;
+		if ACPI_FAILURE(tpacpi_battery_acpi_eval(SET_DISCHARGE, &ret, param)) {
+			pr_err("failed to set force dischrage on %d", battery);
+			return -ENODEV;
+		}
+		return 0;
 	default:
 		pr_crit("wrong parameter: %d", what);
 		return -EINVAL;
@@ -9431,11 +9451,16 @@ static int tpacpi_battery_probe(int battery)
 			/* Support is marked in bit 5 */
 			battery_info.batteries[battery].inhibit_support = ret & BIT(5);
 
-	pr_info("battery %d registered (start %d, stop %d, inhibit: %d)",
+	if (acpi_has_method(hkey_handle, GET_DISCHARGE))
+		if (!ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_DISCHARGE, &ret, battery)))
+			battery_info.batteries[battery].discharge_support = ret & BIT(8);
+
+	pr_info("battery %d registered (start %d, stop %d, inhibit: %d, force: %d)",
 			battery,
 			battery_info.batteries[battery].charge_start,
 			battery_info.batteries[battery].charge_stop,
-			battery_info.batteries[battery].inhibit_support);
+			battery_info.batteries[battery].inhibit_support,
+			battery_info.batteries[battery].discharge_support);
 
 	return 0;
 }
@@ -9530,6 +9555,15 @@ static ssize_t tpacpi_battery_store(int what,
 		if (tpacpi_battery_set(INHIBIT_CHARGE, battery, value))
 			return -ENODEV;
 		return count;
+	case FORCE_DISCHARGE:
+		if (!battery_info.batteries[battery].discharge_support)
+			return -ENODEV;
+		/* The only valid values are 1 and 0 */
+		if (value != 0 && value != 1)
+			return -EINVAL;
+		if (tpacpi_battery_set(FORCE_DISCHARGE, battery, value))
+			return -ENODEV;
+		return count;
 	default:
 		pr_crit("Wrong parameter: %d", what);
 		return -EINVAL;
@@ -9606,14 +9640,30 @@ static ssize_t inhibit_charge_show(struct device *device,
 	return tpacpi_battery_show(INHIBIT_CHARGE, device, buf);
 }
 
+static ssize_t force_discharge_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	return tpacpi_battery_store(FORCE_DISCHARGE, dev, buf, count);
+}
+
+static ssize_t force_discharge_show(struct device *device,
+				struct device_attribute *attr,
+				char *buf)
+{
+	return tpacpi_battery_show(FORCE_DISCHARGE, device, buf);
+}
+
 static DEVICE_ATTR_RW(charge_start_threshold);
 static DEVICE_ATTR_RW(charge_stop_threshold);
 static DEVICE_ATTR_RW(inhibit_charge);
+static DEVICE_ATTR_RW(force_discharge);
 
 static struct attribute *tpacpi_battery_attrs[] = {
 	&dev_attr_charge_start_threshold.attr,
 	&dev_attr_charge_stop_threshold.attr,
 	&dev_attr_inhibit_charge.attr,
+	&dev_attr_force_discharge.attr,
 	NULL,
 };
 
Signature · 6 lines
2.17.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Tested-by on v1 2/3
#1 lore raw
Quoted 6 lines · Ognjen Galic

Lenovo ThinkPad systems have a feature that lets you force the battery to discharge regardless if AC is attached or not.

This patch implements that feature and exposes it via the generic ACPI battery driver.

On a T430 (2342-CTO) I can confirm that both force_discharge and inhibit_charge behave as expected, both when the battery is above and below charge_start_threshold. Input validation also works as-expected. The only oddity I noticed is that force_discharge has a delay taking effect (<1 sec) transitioning from 0 to 1 (but not 1 to 0).

Thanks for working on this!
Kevin

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot

#2 lore raw
Quoted 3 lines · Kevin Locke

as-expected. The only oddity I noticed is that force_discharge has a delay taking effect (<1 sec) transitioning from 0 to 1 (but not 1 to 0).

The EC can take its sweet time to obey any such requests ;-)

Signature · 5 lines
  Henrique Holschuh

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

A message that is not in the archive (sent off-list or not yet received): 20180524113555.wrwpdun64ai4cqwj-ZGHd14iZgfaRjzvQDGKj+xxZW9W5cXbT@public.gmane.org

#4 lore raw
Quoted 7 lines · Henrique de Moraes Holschuh
Quoted 3 lines · Kevin Locke

as-expected. The only oddity I noticed is that force_discharge has a delay taking effect (<1 sec) transitioning from 0 to 1 (but not 1 to 0).

The EC can take its sweet time to obey any such requests ;-)

What is the status on this patch? Do I need some revisional work or?

Quoted 2 lines
Signature · 1 line
  Henrique Holschuh

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot