[PATCH v5 0/1] power: supply: Add new "charge_types" property

[PATCH v5 0/1] power: supply: Add new "charge_types" property

[Hans de Goede]

Here is v5 of my "charge_types" property series, most of this series
has already been merged, leaving only the dell-laptop patch.

Changes in v5:
- Drop patches 1-3 (already merged)
- dell-laptop: Return ENOENT instead of EIO in charge_types_store() when
  the requested mode was accepted by power_supply_charge_types_parse() but
  for some reason is not found in the battery_modes[] array

As already discussed since the dependencies are merged into
linux-power-supply/for-next this patch should also be merged through
linux-power-supply/for-next.

Ilpo, this new version addresses your review comment, can you please
provide your Acked-by for merging this through linux-power-supply/for-next?

Sebastian, can you merge this once acked by Ilpo?

Regards,

Hans


Hans de Goede (1):
  platform/x86: dell-laptop: Use power_supply_charge_types_show/_parse()
    helpers

 drivers/platform/x86/dell/dell-laptop.c | 54 ++++++++++++-------------
 1 file changed, 25 insertions(+), 29 deletions(-)

-- 
2.47.1

[PATCH v5 1/1] platform/x86: dell-laptop: Use power_supply_charge_types_show/_parse() helpers

[Hans de Goede]

Make battery_modes a map between tokens and enum power_supply_charge_type
values instead of between tokens and strings and use the new
power_supply_charge_types_show/_parse() helpers for show()/store()
to ensure that things are handled in the same way as in other drivers.

This also changes battery_supported_modes to be a bitmap of charge-types
(enum power_supply_charge_type values) rather then a bitmap of indices
into battery_modes[].

Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v5:
- Return ENOENT instead of EIO in charge_types_store() when the requested
  mode was accepted by power_supply_charge_types_parse() but for some
  reason is not found in the battery_modes[] array
---
 drivers/platform/x86/dell/dell-laptop.c | 54 ++++++++++++-------------
 1 file changed, 25 insertions(+), 29 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c
index 5671bd0deee7..e6da468daf83 100644
--- a/drivers/platform/x86/dell/dell-laptop.c
+++ b/drivers/platform/x86/dell/dell-laptop.c
@@ -103,15 +103,15 @@ static bool mute_led_registered;
 
 struct battery_mode_info {
 	int token;
-	const char *label;
+	enum power_supply_charge_type charge_type;
 };
 
 static const struct battery_mode_info battery_modes[] = {
-	{ BAT_PRI_AC_MODE_TOKEN,   "Trickle" },
-	{ BAT_EXPRESS_MODE_TOKEN,  "Fast" },
-	{ BAT_STANDARD_MODE_TOKEN, "Standard" },
-	{ BAT_ADAPTIVE_MODE_TOKEN, "Adaptive" },
-	{ BAT_CUSTOM_MODE_TOKEN,   "Custom" },
+	{ BAT_PRI_AC_MODE_TOKEN,   POWER_SUPPLY_CHARGE_TYPE_TRICKLE },
+	{ BAT_EXPRESS_MODE_TOKEN,  POWER_SUPPLY_CHARGE_TYPE_FAST },
+	{ BAT_STANDARD_MODE_TOKEN, POWER_SUPPLY_CHARGE_TYPE_STANDARD },
+	{ BAT_ADAPTIVE_MODE_TOKEN, POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE },
+	{ BAT_CUSTOM_MODE_TOKEN,   POWER_SUPPLY_CHARGE_TYPE_CUSTOM },
 };
 static u32 battery_supported_modes;
 
@@ -2261,46 +2261,42 @@ static ssize_t charge_types_show(struct device *dev,
 		struct device_attribute *attr,
 		char *buf)
 {
-	ssize_t count = 0;
+	enum power_supply_charge_type charge_type;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(battery_modes); i++) {
-		bool active;
+		charge_type = battery_modes[i].charge_type;
 
-		if (!(battery_supported_modes & BIT(i)))
+		if (!(battery_supported_modes & BIT(charge_type)))
 			continue;
 
-		active = dell_battery_mode_is_active(battery_modes[i].token);
-		count += sysfs_emit_at(buf, count, active ? "[%s] " : "%s ",
-				battery_modes[i].label);
+		if (!dell_battery_mode_is_active(battery_modes[i].token))
+			continue;
+
+		return power_supply_charge_types_show(dev, battery_supported_modes,
+						      charge_type, buf);
 	}
 
-	/* convert the last space to a newline */
-	if (count > 0)
-		count--;
-	count += sysfs_emit_at(buf, count, "\n");
-
-	return count;
+	/* No active mode found */
+	return -EIO;
 }
 
 static ssize_t charge_types_store(struct device *dev,
 		struct device_attribute *attr,
 		const char *buf, size_t size)
 {
-	bool matched = false;
-	int err, i;
+	int charge_type, err, i;
+
+	charge_type = power_supply_charge_types_parse(battery_supported_modes, buf);
+	if (charge_type < 0)
+		return charge_type;
 
 	for (i = 0; i < ARRAY_SIZE(battery_modes); i++) {
-		if (!(battery_supported_modes & BIT(i)))
-			continue;
-
-		if (sysfs_streq(battery_modes[i].label, buf)) {
-			matched = true;
+		if (battery_modes[i].charge_type == charge_type)
 			break;
-		}
 	}
-	if (!matched)
-		return -EINVAL;
+	if (i == ARRAY_SIZE(battery_modes))
+		return -ENOENT;
 
 	err = dell_battery_set_mode(battery_modes[i].token);
 	if (err)
@@ -2430,7 +2426,7 @@ static u32 __init battery_get_supported_modes(void)
 
 	for (i = 0; i < ARRAY_SIZE(battery_modes); i++) {
 		if (dell_smbios_find_token(battery_modes[i].token))
-			modes |= BIT(i);
+			modes |= BIT(battery_modes[i].charge_type);
 	}
 
 	return modes;
-- 
2.47.1

Re: [PATCH v5 1/1] platform/x86: dell-laptop: Use power_supply_charge_types_show/_parse() helpers

[Ilpo Järvinen]

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

On Sat, 21 Dec 2024, Hans de Goede wrote:

> Make battery_modes a map between tokens and enum power_supply_charge_type
> values instead of between tokens and strings and use the new
> power_supply_charge_types_show/_parse() helpers for show()/store()
> to ensure that things are handled in the same way as in other drivers.
> 
> This also changes battery_supported_modes to be a bitmap of charge-types
> (enum power_supply_charge_type values) rather then a bitmap of indices
> into battery_modes[].
> 
> Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

> ---
> Changes in v5:
> - Return ENOENT instead of EIO in charge_types_store() when the requested
>   mode was accepted by power_supply_charge_types_parse() but for some
>   reason is not found in the battery_modes[] array
> ---
>  drivers/platform/x86/dell/dell-laptop.c | 54 ++++++++++++-------------
>  1 file changed, 25 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c
> index 5671bd0deee7..e6da468daf83 100644
> --- a/drivers/platform/x86/dell/dell-laptop.c
> +++ b/drivers/platform/x86/dell/dell-laptop.c
> @@ -103,15 +103,15 @@ static bool mute_led_registered;
>  
>  struct battery_mode_info {
>  	int token;
> -	const char *label;
> +	enum power_supply_charge_type charge_type;
>  };
>  
>  static const struct battery_mode_info battery_modes[] = {
> -	{ BAT_PRI_AC_MODE_TOKEN,   "Trickle" },
> -	{ BAT_EXPRESS_MODE_TOKEN,  "Fast" },
> -	{ BAT_STANDARD_MODE_TOKEN, "Standard" },
> -	{ BAT_ADAPTIVE_MODE_TOKEN, "Adaptive" },
> -	{ BAT_CUSTOM_MODE_TOKEN,   "Custom" },
> +	{ BAT_PRI_AC_MODE_TOKEN,   POWER_SUPPLY_CHARGE_TYPE_TRICKLE },
> +	{ BAT_EXPRESS_MODE_TOKEN,  POWER_SUPPLY_CHARGE_TYPE_FAST },
> +	{ BAT_STANDARD_MODE_TOKEN, POWER_SUPPLY_CHARGE_TYPE_STANDARD },
> +	{ BAT_ADAPTIVE_MODE_TOKEN, POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE },
> +	{ BAT_CUSTOM_MODE_TOKEN,   POWER_SUPPLY_CHARGE_TYPE_CUSTOM },
>  };
>  static u32 battery_supported_modes;
>  
> @@ -2261,46 +2261,42 @@ static ssize_t charge_types_show(struct device *dev,
>  		struct device_attribute *attr,
>  		char *buf)
>  {
> -	ssize_t count = 0;
> +	enum power_supply_charge_type charge_type;
>  	int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(battery_modes); i++) {
> -		bool active;
> +		charge_type = battery_modes[i].charge_type;
>  
> -		if (!(battery_supported_modes & BIT(i)))
> +		if (!(battery_supported_modes & BIT(charge_type)))
>  			continue;
>  
> -		active = dell_battery_mode_is_active(battery_modes[i].token);
> -		count += sysfs_emit_at(buf, count, active ? "[%s] " : "%s ",
> -				battery_modes[i].label);
> +		if (!dell_battery_mode_is_active(battery_modes[i].token))
> +			continue;
> +
> +		return power_supply_charge_types_show(dev, battery_supported_modes,
> +						      charge_type, buf);
>  	}
>  
> -	/* convert the last space to a newline */
> -	if (count > 0)
> -		count--;
> -	count += sysfs_emit_at(buf, count, "\n");
> -
> -	return count;
> +	/* No active mode found */
> +	return -EIO;
>  }
>  
>  static ssize_t charge_types_store(struct device *dev,
>  		struct device_attribute *attr,
>  		const char *buf, size_t size)
>  {
> -	bool matched = false;
> -	int err, i;
> +	int charge_type, err, i;
> +
> +	charge_type = power_supply_charge_types_parse(battery_supported_modes, buf);
> +	if (charge_type < 0)
> +		return charge_type;
>  
>  	for (i = 0; i < ARRAY_SIZE(battery_modes); i++) {
> -		if (!(battery_supported_modes & BIT(i)))
> -			continue;
> -
> -		if (sysfs_streq(battery_modes[i].label, buf)) {
> -			matched = true;
> +		if (battery_modes[i].charge_type == charge_type)
>  			break;
> -		}
>  	}
> -	if (!matched)
> -		return -EINVAL;
> +	if (i == ARRAY_SIZE(battery_modes))
> +		return -ENOENT;
>  
>  	err = dell_battery_set_mode(battery_modes[i].token);
>  	if (err)
> @@ -2430,7 +2426,7 @@ static u32 __init battery_get_supported_modes(void)
>  
>  	for (i = 0; i < ARRAY_SIZE(battery_modes); i++) {
>  		if (dell_smbios_find_token(battery_modes[i].token))
> -			modes |= BIT(i);
> +			modes |= BIT(battery_modes[i].charge_type);
>  	}
>  
>  	return modes;
> 

Re: [PATCH v5 0/1] power: supply: Add new "charge_types" property

[Sebastian Reichel]

On Sat, 21 Dec 2024 13:51:39 +0100, Hans de Goede wrote:
> Here is v5 of my "charge_types" property series, most of this series
> has already been merged, leaving only the dell-laptop patch.
> 
> Changes in v5:
> - Drop patches 1-3 (already merged)
> - dell-laptop: Return ENOENT instead of EIO in charge_types_store() when
>   the requested mode was accepted by power_supply_charge_types_parse() but
>   for some reason is not found in the battery_modes[] array
> 
> [...]

Applied, thanks!

[1/1] platform/x86: dell-laptop: Use power_supply_charge_types_show/_parse() helpers
      commit: a3a8799165ff83bb764fd800c6559c3cba0ddac3

Best regards,
-- 
Sebastian Reichel <sebastian.reichel@collabora.com>