platform-driver-x86.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC v2] standardized attributes for powersupply charge behaviour
@ 2021-11-08 19:28 Thomas Weißschuh
  2021-11-11  9:53 ` Hans de Goede
  2021-11-12 18:10 ` Sebastian Reichel
  0 siblings, 2 replies; 4+ messages in thread
From: Thomas Weißschuh @ 2021-11-08 19:28 UTC (permalink / raw)
  To: linux-pm
  Cc: nicolopiazzalunga, linrunner, platform-driver-x86, smclt30p, sre,
	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/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),
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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC v2] standardized attributes for powersupply charge behaviour
  2021-11-08 19:28 [RFC v2] standardized attributes for powersupply charge behaviour Thomas Weißschuh
@ 2021-11-11  9:53 ` Hans de Goede
  2021-11-12 18:10 ` Sebastian Reichel
  1 sibling, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2021-11-11  9:53 UTC (permalink / raw)
  To: Thomas Weißschuh, linux-pm
  Cc: nicolopiazzalunga, linrunner, platform-driver-x86, smclt30p, sre

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
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC v2] standardized attributes for powersupply charge behaviour
  2021-11-08 19:28 [RFC v2] standardized attributes for powersupply charge behaviour Thomas Weißschuh
  2021-11-11  9:53 ` Hans de Goede
@ 2021-11-12 18:10 ` Sebastian Reichel
  2021-11-12 18:26   ` Thomas Weißschuh
  1 sibling, 1 reply; 4+ messages in thread
From: Sebastian Reichel @ 2021-11-12 18:10 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: linux-pm, nicolopiazzalunga, linrunner, platform-driver-x86, smclt30p

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

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
> 

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC v2] standardized attributes for powersupply charge behaviour
  2021-11-12 18:10 ` Sebastian Reichel
@ 2021-11-12 18:26   ` Thomas Weißschuh
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Weißschuh @ 2021-11-12 18:26 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: linux-pm, nicolopiazzalunga, linrunner, platform-driver-x86, smclt30p

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-11-12 18:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-08 19:28 [RFC v2] standardized attributes for powersupply charge behaviour Thomas Weißschuh
2021-11-11  9:53 ` Hans de Goede
2021-11-12 18:10 ` Sebastian Reichel
2021-11-12 18:26   ` Thomas Weißschuh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).