[PATCH v2] power: supply: register HWMON devices with valid names

[PATCH v2] power: supply: register HWMON devices with valid names

[Romain Izard]

With the introduction of the HWMON compatibility layer to the power
supply framework in Linux 5.3, all power supply devices' names can be
used directly to create HWMON devices with the same names.

But HWMON has rules on allowable names that are different from those
used in the power supply framework. The dash character is forbidden, as
it is used by the libsensors library in userspace as a separator,
whereas this character is used in the device names in more than half of
the existing power supply drivers. This last case is consistent with the
typical naming usage with MFD and Device Tree.

This leads to warnings in the kernel log, with the format:

power_supply gpio-charger: hwmon: \
	'gpio-charger' is not a valid name attribute, please fix

Add a protection to power_supply_add_hwmon_sysfs() that replaces any
dash in the device name with an underscore when registering with the
HWMON framework. Other forbidden characters (star, slash, space, tab,
newline) are not replaced, as they are not in common use.

Fixes: e67d4dfc9ff1 ("power: supply: Add HWMON compatibility layer")
Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---

v2: Remove a superfluous cast

 drivers/power/supply/power_supply_hwmon.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/power_supply_hwmon.c b/drivers/power/supply/power_supply_hwmon.c
index 51fe60440d12..75cf861ba492 100644
--- a/drivers/power/supply/power_supply_hwmon.c
+++ b/drivers/power/supply/power_supply_hwmon.c
@@ -284,6 +284,7 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy)
 	struct device *dev = &psy->dev;
 	struct device *hwmon;
 	int ret, i;
+	const char *name;
 
 	if (!devres_open_group(dev, power_supply_add_hwmon_sysfs,
 			       GFP_KERNEL))
@@ -334,7 +335,19 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy)
 		}
 	}
 
-	hwmon = devm_hwmon_device_register_with_info(dev, psy->desc->name,
+	name = psy->desc->name;
+	if (strchr(name, '-')) {
+		char *new_name;
+
+		new_name = devm_kstrdup(dev, name, GFP_KERNEL);
+		if (!new_name) {
+			ret = -ENOMEM;
+			goto error;
+		}
+		strreplace(new_name, '-', '_');
+		name = new_name;
+	}
+	hwmon = devm_hwmon_device_register_with_info(dev, name,
 						psyhw,
 						&power_supply_hwmon_chip_info,
 						NULL);
-- 
2.17.1

Re: [PATCH v2] power: supply: register HWMON devices with valid names

[Sebastian Reichel]

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

Hi,

On Fri, Aug 30, 2019 at 03:15:56PM +0200, Romain Izard wrote:
> With the introduction of the HWMON compatibility layer to the power
> supply framework in Linux 5.3, all power supply devices' names can be
> used directly to create HWMON devices with the same names.
> 
> But HWMON has rules on allowable names that are different from those
> used in the power supply framework. The dash character is forbidden, as
> it is used by the libsensors library in userspace as a separator,
> whereas this character is used in the device names in more than half of
> the existing power supply drivers. This last case is consistent with the
> typical naming usage with MFD and Device Tree.
> 
> This leads to warnings in the kernel log, with the format:
> 
> power_supply gpio-charger: hwmon: \
> 	'gpio-charger' is not a valid name attribute, please fix
> 
> Add a protection to power_supply_add_hwmon_sysfs() that replaces any
> dash in the device name with an underscore when registering with the
> HWMON framework. Other forbidden characters (star, slash, space, tab,
> newline) are not replaced, as they are not in common use.
> 
> Fixes: e67d4dfc9ff1 ("power: supply: Add HWMON compatibility layer")
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---

Thanks, queued.

-- Sebastian

> 
> v2: Remove a superfluous cast
> 
>  drivers/power/supply/power_supply_hwmon.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/power_supply_hwmon.c b/drivers/power/supply/power_supply_hwmon.c
> index 51fe60440d12..75cf861ba492 100644
> --- a/drivers/power/supply/power_supply_hwmon.c
> +++ b/drivers/power/supply/power_supply_hwmon.c
> @@ -284,6 +284,7 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy)
>  	struct device *dev = &psy->dev;
>  	struct device *hwmon;
>  	int ret, i;
> +	const char *name;
>  
>  	if (!devres_open_group(dev, power_supply_add_hwmon_sysfs,
>  			       GFP_KERNEL))
> @@ -334,7 +335,19 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy)
>  		}
>  	}
>  
> -	hwmon = devm_hwmon_device_register_with_info(dev, psy->desc->name,
> +	name = psy->desc->name;
> +	if (strchr(name, '-')) {
> +		char *new_name;
> +
> +		new_name = devm_kstrdup(dev, name, GFP_KERNEL);
> +		if (!new_name) {
> +			ret = -ENOMEM;
> +			goto error;
> +		}
> +		strreplace(new_name, '-', '_');
> +		name = new_name;
> +	}
> +	hwmon = devm_hwmon_device_register_with_info(dev, name,
>  						psyhw,
>  						&power_supply_hwmon_chip_info,
>  						NULL);
> -- 
> 2.17.1
> 

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