| Message ID | 20211108192852.357473-1-linux@weissschuh.net (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | [RFC,v2] standardized attributes for powersupply charge behaviour | expand |
Hi, On 11/8/21 20:28, Thomas Weißschuh wrote: > 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/ Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > 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(+) > > diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power > index ca830c6cd809..2f58cfc91420 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/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c > index c3d7cbcd4fad..26c60587dca1 100644 > --- a/drivers/power/supply/power_supply_sysfs.c > +++ b/drivers/power/supply/power_supply_sysfs.c > @@ -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), > 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, > }; > > base-commit: 6880fa6c56601bb8ed59df6c30fd390cc5f6dd8f >
Hi, On Mon, Nov 08, 2021 at 08:28:52PM +0100, Thomas Weißschuh wrote: > 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/ > --- > 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(+) > > diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power > index ca830c6cd809..2f58cfc91420 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/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c > index c3d7cbcd4fad..26c60587dca1 100644 > --- a/drivers/power/supply/power_supply_sysfs.c > +++ b/drivers/power/supply/power_supply_sysfs.c > @@ -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 > 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, > }; > > base-commit: 6880fa6c56601bb8ed59df6c30fd390cc5f6dd8f > -- > 2.33.1 >
Hi, On 2021-11-12 19:10+0100, Sebastian Reichel wrote: > On Mon, Nov 08, 2021 at 08:28:52PM +0100, Thomas Weißschuh wrote: > > 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/ > > --- > > 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(+) > > > > diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power > > index ca830c6cd809..2f58cfc91420 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/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c > > index c3d7cbcd4fad..26c60587dca1 100644 > > --- a/drivers/power/supply/power_supply_sysfs.c > > +++ b/drivers/power/supply/power_supply_sysfs.c > > @@ -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[] = { ... }; The non-RFC version will have that. > 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
diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power index ca830c6cd809..2f58cfc91420 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/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c index c3d7cbcd4fad..26c60587dca1 100644 --- a/drivers/power/supply/power_supply_sysfs.c +++ b/drivers/power/supply/power_supply_sysfs.c @@ -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), 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, };