[PATCH 0/1] support charge_types in the extensions API

[PATCH 0/1] support charge_types in the extensions API

[Jelle van der Waa]

From: Jelle van der Waa <jvanderwaa@redhat.com>

The goal of this patch is to make it possible for the ideapad-laptop
driver to move from its custom conservation_mode sysfs attribute to the
standardised charge_types sysfs attribute with a power_supply extension.
Along with other laptop models which support an end charge threshold
which limits it to a fixed value such as some Toshiba and LG models.

User space, in this case UPower would then be detect 'Long Life' as
charge_types option and set it.

This patch should also allow the "dell-laptop" driver to move over to
the extensions API.

Jelle van der Waa (1):
  power: supply: support charge_types in extensions

 drivers/power/supply/power_supply_sysfs.c | 23 ++++++++++++++++++++++-
 drivers/power/supply/test_power.c         | 20 ++++++++++++++++++--
 include/linux/power_supply.h              |  1 +
 3 files changed, 41 insertions(+), 3 deletions(-)

-- 
2.49.0

[PATCH 1/1] power: supply: support charge_types in extensions

[Jelle van der Waa]

From: Jelle van der Waa <jvanderwaa@redhat.com>

Similar to charge_behaviour, charge_types is an enum option with
multiple possible values. To be able to use it with a power_supply
extension its known values have to be exposed.

Signed-off-by: Jelle van der Waa <jvanderwaa@redhat.com>
---
 drivers/power/supply/power_supply_sysfs.c | 23 ++++++++++++++++++++++-
 drivers/power/supply/test_power.c         | 20 ++++++++++++++++++--
 include/linux/power_supply.h              |  1 +
 3 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index edb058c19c9c..6d80640511b5 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -321,6 +321,27 @@ static ssize_t power_supply_show_charge_behaviour(struct device *dev,
 						  value->intval, buf);
 }
 
+static ssize_t power_supply_show_charge_types(struct device *dev,
+					      struct power_supply *psy,
+					      enum power_supply_charge_type current_type,
+					      char *buf)
+{
+	struct power_supply_ext_registration *reg;
+
+	scoped_guard(rwsem_read, &psy->extensions_sem) {
+		power_supply_for_each_extension(reg, psy) {
+			if (power_supply_ext_has_property(reg->ext,
+							  POWER_SUPPLY_PROP_CHARGE_TYPES))
+				return power_supply_charge_types_show(dev,
+						reg->ext->charge_types,
+						current_type, buf);
+		}
+	}
+
+	return power_supply_charge_types_show(dev, psy->desc->charge_types,
+						  current_type, buf);
+}
+
 static ssize_t power_supply_format_property(struct device *dev,
 					    bool uevent,
 					    struct device_attribute *attr,
@@ -365,7 +386,7 @@ static ssize_t power_supply_format_property(struct device *dev,
 	case POWER_SUPPLY_PROP_CHARGE_TYPES:
 		if (uevent) /* no possible values in uevents */
 			goto default_format;
-		ret = power_supply_charge_types_show(dev, psy->desc->charge_types,
+		ret = power_supply_show_charge_types(dev, psy,
 						     value.intval, buf);
 		break;
 	case POWER_SUPPLY_PROP_MODEL_NAME ... POWER_SUPPLY_PROP_SERIAL_NUMBER:
diff --git a/drivers/power/supply/test_power.c b/drivers/power/supply/test_power.c
index 2a975a110f48..0d9cc0c5613e 100644
--- a/drivers/power/supply/test_power.c
+++ b/drivers/power/supply/test_power.c
@@ -37,6 +37,8 @@ static int battery_charge_counter	= -1000;
 static int battery_current		= -1600;
 static enum power_supply_charge_behaviour battery_charge_behaviour =
 	POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO;
+static enum power_supply_charge_type battery_charge_types =
+	POWER_SUPPLY_CHARGE_TYPE_NONE;
 static bool battery_extension;
 
 static bool module_initialized;
@@ -87,7 +89,7 @@ static int test_power_get_battery_property(struct power_supply *psy,
 		val->intval = battery_status;
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_TYPE:
-		val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
+		val->intval = battery_charge_types;
 		break;
 	case POWER_SUPPLY_PROP_HEALTH:
 		val->intval = battery_health;
@@ -129,6 +131,9 @@ static int test_power_get_battery_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR:
 		val->intval = battery_charge_behaviour;
 		break;
+	case POWER_SUPPLY_PROP_CHARGE_TYPES:
+		val->intval = battery_charge_types;
+		break;
 	default:
 		pr_info("%s: some properties deliberately report errors.\n",
 			__func__);
@@ -140,7 +145,7 @@ static int test_power_get_battery_property(struct power_supply *psy,
 static int test_power_battery_property_is_writeable(struct power_supply *psy,
 						    enum power_supply_property psp)
 {
-	return psp == POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR;
+	return psp == POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR | POWER_SUPPLY_PROP_CHARGE_TYPES;
 }
 
 static int test_power_set_battery_property(struct power_supply *psy,
@@ -156,6 +161,14 @@ static int test_power_set_battery_property(struct power_supply *psy,
 		}
 		battery_charge_behaviour = val->intval;
 		break;
+	case POWER_SUPPLY_PROP_CHARGE_TYPES:
+		if (val->intval < 0 ||
+		    val->intval >= BITS_PER_TYPE(typeof(psy->desc->charge_types)) ||
+		    !(BIT(val->intval) & psy->desc->charge_types)) {
+			return -EINVAL;
+		}
+		battery_charge_types = val->intval;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -188,6 +201,7 @@ static enum power_supply_property test_power_battery_props[] = {
 	POWER_SUPPLY_PROP_CURRENT_AVG,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
 	POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR,
+	POWER_SUPPLY_PROP_CHARGE_TYPES,
 };
 
 static char *test_power_ac_supplied_to[] = {
@@ -215,6 +229,8 @@ static const struct power_supply_desc test_power_desc[] = {
 		.charge_behaviours = BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO)
 				   | BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE)
 				   | BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE),
+		.charge_types = BIT(POWER_SUPPLY_CHARGE_TYPE_NONE)
+				   | BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)
 	},
 	[TEST_USB] = {
 		.name = "test_usb",
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 888824592953..c4cb854971f5 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -288,6 +288,7 @@ struct power_supply_desc {
 struct power_supply_ext {
 	const char *const name;
 	u8 charge_behaviours;
+	u32 charge_types;
 	const enum power_supply_property *properties;
 	size_t num_properties;
 
-- 
2.49.0

Re: [PATCH 1/1] power: supply: support charge_types in extensions

[Hans de Goede]

Hi Jelle,

Thank you for your patch.

On 7-Apr-25 10:18 PM, Jelle van der Waa wrote:
> From: Jelle van der Waa <jvanderwaa@redhat.com>
> 
> Similar to charge_behaviour, charge_types is an enum option with
> multiple possible values. To be able to use it with a power_supply
> extension its known values have to be exposed.

"multiple possible values" sounds like the actual setting
is a bitmask where multiple options can be enabled at once.

And "its known values" sounds like this is about the currently
active value(s).

Instead I suggest a text like this:

"""
Similar to charge_behaviour, charge_types is an enum option where reading
the property shows the supported values, with the active value surrounded
by brackets. To be able to use it with a power_supply extension a bitmask
with the supported charge_Types values has to be added to power_supply_ext.
"""

Feel free to re-use this verbatim, or just the bits you like.

> Signed-off-by: Jelle van der Waa <jvanderwaa@redhat.com>
> ---
>  drivers/power/supply/power_supply_sysfs.c | 23 ++++++++++++++++++++++-
>  drivers/power/supply/test_power.c         | 20 ++++++++++++++++++--
>  include/linux/power_supply.h              |  1 +
>  3 files changed, 41 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index edb058c19c9c..6d80640511b5 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -321,6 +321,27 @@ static ssize_t power_supply_show_charge_behaviour(struct device *dev,
>  						  value->intval, buf);
>  }
>  
> +static ssize_t power_supply_show_charge_types(struct device *dev,
> +					      struct power_supply *psy,
> +					      enum power_supply_charge_type current_type,
> +					      char *buf)
> +{
> +	struct power_supply_ext_registration *reg;
> +
> +	scoped_guard(rwsem_read, &psy->extensions_sem) {
> +		power_supply_for_each_extension(reg, psy) {
> +			if (power_supply_ext_has_property(reg->ext,
> +							  POWER_SUPPLY_PROP_CHARGE_TYPES))
> +				return power_supply_charge_types_show(dev,
> +						reg->ext->charge_types,
> +						current_type, buf);
> +		}
> +	}
> +
> +	return power_supply_charge_types_show(dev, psy->desc->charge_types,
> +						  current_type, buf);
> +}
> +

Once the last external user of power_supply_charge_types_show() (dell-laptop)
is ported to become a power-supply extension, we should stop exporting
power_supply_charge_types_show(). The question then becomes do we just stop
exporting it; or do we remove it entirely ? My vote would go to remove it
entirely. This could be done if the above function were to directly call
power_supply_show_enum_with_available() instead of using the helper, e.g.:

static ssize_t power_supply_show_charge_types(struct device *dev,
					      struct power_supply *psy,
					      enum power_supply_charge_type current_type,
					      char *buf)
{
	struct power_supply_ext_registration *reg;

	scoped_guard(rwsem_read, &psy->extensions_sem) {
		power_supply_for_each_extension(reg, psy) {
			if (power_supply_ext_has_property(reg->ext,
							  POWER_SUPPLY_PROP_CHARGE_TYPES))
				return power_supply_show_enum_with_available(dev,
						POWER_SUPPLY_CHARGE_TYPE_TEXT,
						ARRAY_SIZE(POWER_SUPPLY_CHARGE_TYPE_TEXT),
						reg->ext->charge_types,
						current_type, buf);
		}
	}

	return power_supply_show_enum_with_available(dev, POWER_SUPPLY_CHARGE_TYPE_TEXT,
						     ARRAY_SIZE(POWER_SUPPLY_CHARGE_TYPE_TEXT),
						     psy->desc->charge_types, current_type, buf);
}

Since we already know we want to eventually remove power_supply_charge_types_show()
I think it would be better to write this as above right away.

Sebastian, Thomas any opinions on this ?

>  static ssize_t power_supply_format_property(struct device *dev,
>  					    bool uevent,
>  					    struct device_attribute *attr,
> @@ -365,7 +386,7 @@ static ssize_t power_supply_format_property(struct device *dev,
>  	case POWER_SUPPLY_PROP_CHARGE_TYPES:
>  		if (uevent) /* no possible values in uevents */
>  			goto default_format;
> -		ret = power_supply_charge_types_show(dev, psy->desc->charge_types,
> +		ret = power_supply_show_charge_types(dev, psy,
>  						     value.intval, buf);
>  		break;
>  	case POWER_SUPPLY_PROP_MODEL_NAME ... POWER_SUPPLY_PROP_SERIAL_NUMBER:
> diff --git a/drivers/power/supply/test_power.c b/drivers/power/supply/test_power.c
> index 2a975a110f48..0d9cc0c5613e 100644
> --- a/drivers/power/supply/test_power.c
> +++ b/drivers/power/supply/test_power.c
> @@ -37,6 +37,8 @@ static int battery_charge_counter	= -1000;
>  static int battery_current		= -1600;
>  static enum power_supply_charge_behaviour battery_charge_behaviour =
>  	POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO;
> +static enum power_supply_charge_type battery_charge_types =
> +	POWER_SUPPLY_CHARGE_TYPE_NONE;
>  static bool battery_extension;
>  
>  static bool module_initialized;
> @@ -87,7 +89,7 @@ static int test_power_get_battery_property(struct power_supply *psy,
>  		val->intval = battery_status;
>  		break;
>  	case POWER_SUPPLY_PROP_CHARGE_TYPE:
> -		val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
> +		val->intval = battery_charge_types;
>  		break;
>  	case POWER_SUPPLY_PROP_HEALTH:
>  		val->intval = battery_health;
> @@ -129,6 +131,9 @@ static int test_power_get_battery_property(struct power_supply *psy,
>  	case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR:
>  		val->intval = battery_charge_behaviour;
>  		break;
> +	case POWER_SUPPLY_PROP_CHARGE_TYPES:
> +		val->intval = battery_charge_types;
> +		break;
>  	default:
>  		pr_info("%s: some properties deliberately report errors.\n",
>  			__func__);
> @@ -140,7 +145,7 @@ static int test_power_get_battery_property(struct power_supply *psy,
>  static int test_power_battery_property_is_writeable(struct power_supply *psy,
>  						    enum power_supply_property psp)
>  {
> -	return psp == POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR;
> +	return psp == POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR | POWER_SUPPLY_PROP_CHARGE_TYPES;
>  }
>  
>  static int test_power_set_battery_property(struct power_supply *psy,
> @@ -156,6 +161,14 @@ static int test_power_set_battery_property(struct power_supply *psy,
>  		}
>  		battery_charge_behaviour = val->intval;
>  		break;
> +	case POWER_SUPPLY_PROP_CHARGE_TYPES:
> +		if (val->intval < 0 ||
> +		    val->intval >= BITS_PER_TYPE(typeof(psy->desc->charge_types)) ||
> +		    !(BIT(val->intval) & psy->desc->charge_types)) {
> +			return -EINVAL;
> +		}
> +		battery_charge_types = val->intval;
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -188,6 +201,7 @@ static enum power_supply_property test_power_battery_props[] = {
>  	POWER_SUPPLY_PROP_CURRENT_AVG,
>  	POWER_SUPPLY_PROP_CURRENT_NOW,
>  	POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR,
> +	POWER_SUPPLY_PROP_CHARGE_TYPES,
>  };
>  
>  static char *test_power_ac_supplied_to[] = {
> @@ -215,6 +229,8 @@ static const struct power_supply_desc test_power_desc[] = {
>  		.charge_behaviours = BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO)
>  				   | BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE)
>  				   | BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE),
> +		.charge_types = BIT(POWER_SUPPLY_CHARGE_TYPE_NONE)
> +				   | BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)
>  	},
>  	[TEST_USB] = {
>  		.name = "test_usb",
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index 888824592953..c4cb854971f5 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -288,6 +288,7 @@ struct power_supply_desc {
>  struct power_supply_ext {
>  	const char *const name;
>  	u8 charge_behaviours;
> +	u32 charge_types;
>  	const enum power_supply_property *properties;
>  	size_t num_properties;
>  

Otherwise this patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

Re: [PATCH 1/1] power: supply: support charge_types in extensions

[kernel test robot]

Hi Jelle,

kernel test robot noticed the following build warnings:

[auto build test WARNING on sre-power-supply/for-next]
[also build test WARNING on linus/master v6.15-rc1 next-20250408]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jelle-van-der-Waa/power-supply-support-charge_types-in-extensions/20250408-045501
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
patch link:    https://lore.kernel.org/r/20250407201845.332348-2-jvanderw%40redhat.com
patch subject: [PATCH 1/1] power: supply: support charge_types in extensions
config: i386-buildonly-randconfig-004-20250409 (https://download.01.org/0day-ci/archive/20250409/202504090646.w9GLxe17-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250409/202504090646.w9GLxe17-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504090646.w9GLxe17-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/power/supply/test_power.c:148:51: warning: | has lower precedence than ==; == will be evaluated first [-Wparentheses]
     148 |         return psp == POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR | POWER_SUPPLY_PROP_CHARGE_TYPES;
         |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
   drivers/power/supply/test_power.c:148:51: note: place parentheses around the '==' expression to silence this warning
     148 |         return psp == POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR | POWER_SUPPLY_PROP_CHARGE_TYPES;
         |                                                          ^
         |                (                                        )
   drivers/power/supply/test_power.c:148:51: note: place parentheses around the | expression to evaluate it first
     148 |         return psp == POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR | POWER_SUPPLY_PROP_CHARGE_TYPES;
         |                                                          ^                               
         |                       (                                                                  )
   1 warning generated.


vim +148 drivers/power/supply/test_power.c

   144	
   145	static int test_power_battery_property_is_writeable(struct power_supply *psy,
   146							    enum power_supply_property psp)
   147	{
 > 148		return psp == POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR | POWER_SUPPLY_PROP_CHARGE_TYPES;
   149	}
   150	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/1] power: supply: support charge_types in extensions

[kernel test robot]

Hi Jelle,

kernel test robot noticed the following build warnings:

[auto build test WARNING on sre-power-supply/for-next]
[also build test WARNING on linus/master v6.15-rc1 next-20250408]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jelle-van-der-Waa/power-supply-support-charge_types-in-extensions/20250408-045501
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
patch link:    https://lore.kernel.org/r/20250407201845.332348-2-jvanderw%40redhat.com
patch subject: [PATCH 1/1] power: supply: support charge_types in extensions
config: csky-randconfig-001-20250409 (https://download.01.org/0day-ci/archive/20250409/202504090859.yXF0zkoj-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250409/202504090859.yXF0zkoj-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504090859.yXF0zkoj-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/power/supply/test_power.c: In function 'test_power_battery_property_is_writeable':
>> drivers/power/supply/test_power.c:148:20: warning: suggest parentheses around comparison in operand of '|' [-Wparentheses]
     148 |         return psp == POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR | POWER_SUPPLY_PROP_CHARGE_TYPES;
         |                ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +148 drivers/power/supply/test_power.c

   144	
   145	static int test_power_battery_property_is_writeable(struct power_supply *psy,
   146							    enum power_supply_property psp)
   147	{
 > 148		return psp == POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR | POWER_SUPPLY_PROP_CHARGE_TYPES;
   149	}
   150	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki