Skip to contentSkip to conversation

standardized attributes for powersupply charge behaviour

RFC In review Thomas Weißschuh 3 participants 4 messages Nov 8, 2021 – Nov 12, 2021

Subsystems (2)
x86 platform drivers Power supply
lore
Share & tools
lore

Thomas Weißschuh

author
#0 lore raw

RFC v2 1/1 standardized attributes for powersupply charge behaviour

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/[email protected]/

---
 Documentation/ABI/testing/sysfs-class-power | 14 ++++++++++++++
 drivers/power/supply/power_supply_sysfs.c   |  1 +
 include/linux/power_supply.h                |  7 +++++++
 3 files changed, 22 insertions(+)
Documentation/ABI/testing/sysfs-class-power +14 -0
index ca830c6cd809..2f58cfc91420 100644
@@ -455,6 +455,20 @@ Description:
 			      "Unknown", "Charging", "Discharging",
 			      "Not charging", "Full"
 
+What:		/sys/class/power_supply/<supply_name>/charge_behaviour
+Date:		November 2021
+Contact:	[email protected]
+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:	[email protected]
drivers/power/supply/power_supply_sysfs.c +1 -0
index c3d7cbcd4fad..26c60587dca1 100644
@@ -172,6 +172,7 @@ static struct power_supply_attr power_supply_attrs[] = {
 	POWER_SUPPLY_ATTR(CHARGE_CONTROL_LIMIT_MAX),
 	POWER_SUPPLY_ATTR(CHARGE_CONTROL_START_THRESHOLD),
 	POWER_SUPPLY_ATTR(CHARGE_CONTROL_END_THRESHOLD),
+	POWER_SUPPLY_ENUM_ATTR(CHARGE_BEHAVIOUR),
 	POWER_SUPPLY_ATTR(INPUT_CURRENT_LIMIT),
 	POWER_SUPPLY_ATTR(INPUT_VOLTAGE_LIMIT),
 	POWER_SUPPLY_ATTR(INPUT_POWER_LIMIT),
include/linux/power_supply.h +7 -0
index 9ca1f120a211..70c333e86293 100644
@@ -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,
 };

base-commit: 6880fa6c56601bb8ed59df6c30fd390cc5f6dd8f

Signature · 1 line
2.33.1
Reviewed-by on v2 1/1
#1 lore raw

Hi,

Quoted 8 lines · Thomas Weißschuh

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/[email protected]/

Thanks, patch looks good to me:

Reviewed-by Hans de Goede <[email protected]>

Regards,

Hans

Quoted 71 lines
---
 Documentation/ABI/testing/sysfs-class-power | 14 ++++++++++++++
 drivers/power/supply/power_supply_sysfs.c   |  1 +
 include/linux/power_supply.h                |  7 +++++++
 3 files changed, 22 insertions(+)
Documentation/ABI/testing/sysfs-class-power +14 -0
index ca830c6cd809..2f58cfc91420 100644
@@ -455,6 +455,20 @@ Description:
 			      "Unknown", "Charging", "Discharging",
 			      "Not charging", "Full"
 
+What:		/sys/class/power_supply/<supply_name>/charge_behaviour
+Date:		November 2021
+Contact:	[email protected]
+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:	[email protected]
drivers/power/supply/power_supply_sysfs.c +1 -0
index c3d7cbcd4fad..26c60587dca1 100644
@@ -172,6 +172,7 @@ static struct power_supply_attr power_supply_attrs[] = {
 	POWER_SUPPLY_ATTR(CHARGE_CONTROL_LIMIT_MAX),
 	POWER_SUPPLY_ATTR(CHARGE_CONTROL_START_THRESHOLD),
 	POWER_SUPPLY_ATTR(CHARGE_CONTROL_END_THRESHOLD),
+	POWER_SUPPLY_ENUM_ATTR(CHARGE_BEHAVIOUR),
 	POWER_SUPPLY_ATTR(INPUT_CURRENT_LIMIT),
 	POWER_SUPPLY_ATTR(INPUT_VOLTAGE_LIMIT),
 	POWER_SUPPLY_ATTR(INPUT_POWER_LIMIT),
include/linux/power_supply.h +7 -0
index 9ca1f120a211..70c333e86293 100644
@@ -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,
 };

base-commit: 6880fa6c56601bb8ed59df6c30fd390cc5f6dd8f

#2 lore raw
In reply to Thomas Weißschuh

Hi,

Quoted 44 lines · Thomas Weißschuh

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/[email protected]/

---
 Documentation/ABI/testing/sysfs-class-power | 14 ++++++++++++++
 drivers/power/supply/power_supply_sysfs.c   |  1 +
 include/linux/power_supply.h                |  7 +++++++
 3 files changed, 22 insertions(+)
Documentation/ABI/testing/sysfs-class-power +14 -0
index ca830c6cd809..2f58cfc91420 100644
@@ -455,6 +455,20 @@ Description:
 			      "Unknown", "Charging", "Discharging",
 			      "Not charging", "Full"
 
+What:		/sys/class/power_supply/<supply_name>/charge_behaviour
+Date:		November 2021
+Contact:	[email protected]
+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:	[email protected]
drivers/power/supply/power_supply_sysfs.c +0 -0
index c3d7cbcd4fad..26c60587dca1 100644

drivers/power/supply/power_supply_sysfs.c static struct power_supply_attr power_supply_attrs[] = { · line 178

Quoted 7 lines
drivers/power/supply/power_supply_sysfs.c +1 -0
@@ -172,6 +172,7 @@ static struct power_supply_attr power_supply_attrs[] = {
 	POWER_SUPPLY_ATTR(CHARGE_CONTROL_LIMIT_MAX),
 	POWER_SUPPLY_ATTR(CHARGE_CONTROL_START_THRESHOLD),
 	POWER_SUPPLY_ATTR(CHARGE_CONTROL_END_THRESHOLD),
+	POWER_SUPPLY_ENUM_ATTR(CHARGE_BEHAVIOUR),
 	POWER_SUPPLY_ATTR(INPUT_CURRENT_LIMIT),
 	POWER_SUPPLY_ATTR(INPUT_VOLTAGE_LIMIT),
 	POWER_SUPPLY_ATTR(INPUT_POWER_LIMIT),

this is missing (and should not compile without it):

static const char * const POWER_SUPPLY_CHARGE_BEHAVIOUR_TEXT[] = { ... };

Otherwise LGTM. But you need to send API changes with an API user (i.e. the patch updating acpi battery driver using this).

-- Sebastian

Quoted 30 lines
include/linux/power_supply.h +7 -0
index 9ca1f120a211..70c333e86293 100644
@@ -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,
 };

base-commit: 6880fa6c56601bb8ed59df6c30fd390cc5f6dd8f

Signature · 1 line
2.33.1
author
#3 lore raw

Hi,

Quoted 55 lines · Sebastian Reichel
Quoted 44 lines · Thomas Weißschuh

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/[email protected]/

---
 Documentation/ABI/testing/sysfs-class-power | 14 ++++++++++++++
 drivers/power/supply/power_supply_sysfs.c   |  1 +
 include/linux/power_supply.h                |  7 +++++++
 3 files changed, 22 insertions(+)
Documentation/ABI/testing/sysfs-class-power +14 -0
index ca830c6cd809..2f58cfc91420 100644
@@ -455,6 +455,20 @@ Description:
 			      "Unknown", "Charging", "Discharging",
 			      "Not charging", "Full"
 
+What:		/sys/class/power_supply/<supply_name>/charge_behaviour
+Date:		November 2021
+Contact:	[email protected]
+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:	[email protected]
drivers/power/supply/power_supply_sysfs.c +0 -0
index c3d7cbcd4fad..26c60587dca1 100644

drivers/power/supply/power_supply_sysfs.c static struct power_supply_attr power_supply_attrs[] = { · line 178

Quoted 7 lines
drivers/power/supply/power_supply_sysfs.c +1 -0
@@ -172,6 +172,7 @@ static struct power_supply_attr power_supply_attrs[] = {
 	POWER_SUPPLY_ATTR(CHARGE_CONTROL_LIMIT_MAX),
 	POWER_SUPPLY_ATTR(CHARGE_CONTROL_START_THRESHOLD),
 	POWER_SUPPLY_ATTR(CHARGE_CONTROL_END_THRESHOLD),
+	POWER_SUPPLY_ENUM_ATTR(CHARGE_BEHAVIOUR),
 	POWER_SUPPLY_ATTR(INPUT_CURRENT_LIMIT),
 	POWER_SUPPLY_ATTR(INPUT_VOLTAGE_LIMIT),
 	POWER_SUPPLY_ATTR(INPUT_POWER_LIMIT),

this is missing (and should not compile without it):

Quoted 1 line

static const char * const POWER_SUPPLY_CHARGE_BEHAVIOUR_TEXT[] = { ... };

The non-RFC version will have that.

Quoted 2 lines

Otherwise LGTM. But you need to send API changes with an API user (i.e. the patch updating acpi battery driver using this).

I have an implementation for the thinkpad_acpi driver.

Is it fine if I export helper functions from power_supply_sysfs.c?

The problem is that thinkpad_acpi is using the acpi battery hooks which do not allow other drivers to extend existing ACPI batteries with proper powersupply properties but only plain sysfs attributes.

Currently I keep the parsing and formatting of the sysfs file inside power_supply_sysfs.c, because that is where it belongs but export it to other modules to use for their custom sysfs attributes.

I'm just not sure if this is acceptable, because no other function from power_supply_sysfs.c is exported yet.

If it's not clear enough, we can also discuss this on the real patchset which I'll probably submit tomorrow.

Thanks,
Thomas