[PATCH v2 0/2] HWMON compatibility layer for power supplies

[PATCH v2 0/2] HWMON compatibility layer for power supplies

[Andrey Smirnov]

Everyone:

This small series contains the code I wrote to expose various power
supply sensors via HWMON layer in order to be able to access all of
the sensors in the system with libsensors. Not sure if this is an
something that can be accepted upstream, so I am hoping to get some
quick feedback.

Thanks,
Andrey Smirnov


Changes since [v1]:

  - All multiplications converted to use check_mul_overflow()

  - All divisions converted to use DIV_ROUND_CLOSEST()

  - Places that were ignoring errors now don't

  - Alphabetized include list

Andrey Smirnov (2):
  power: supply: Add HWMON compatibility layer
  power: supply: ucs1002: Add HWMON interface

 drivers/power/supply/Kconfig              |  14 +
 drivers/power/supply/Makefile             |   1 +
 drivers/power/supply/power_supply_hwmon.c | 349 ++++++++++++++++++++++
 drivers/power/supply/ucs1002_power.c      |   6 +
 include/linux/power_supply.h              |   9 +
 5 files changed, 379 insertions(+)
 create mode 100644 drivers/power/supply/power_supply_hwmon.c

-- 
2.21.0

[PATCH v2 1/2] power: supply: Add HWMON compatibility layer

[Andrey Smirnov]

Add code implementing HWMON adapter/compatibility layer to allow
expositing various sensors present on power supply devices via HWMON
subsystem. This is done in order to allow userspace to use single
ABI/library(libsensors) to access/manipulate all of the sensors of the
system.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-pm@vger.kernel.org
---
 drivers/power/supply/Kconfig              |  14 +
 drivers/power/supply/Makefile             |   1 +
 drivers/power/supply/power_supply_hwmon.c | 349 ++++++++++++++++++++++
 include/linux/power_supply.h              |   9 +
 4 files changed, 373 insertions(+)
 create mode 100644 drivers/power/supply/power_supply_hwmon.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 26dacdab03cc..1f2252cb95fd 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -14,6 +14,20 @@ config POWER_SUPPLY_DEBUG
 	  Say Y here to enable debugging messages for power supply class
 	  and drivers.
 
+config POWER_SUPPLY_HWMON
+	bool
+	prompt "Expose power supply sensors as hwmon device"
+	depends on HWMON=y || HWMON=POWER_SUPPLY
+	default y
+	help
+	  This options enables API that allows sensors found on a
+	  power supply device (current, voltage, temperature) to be
+	  exposed as a hwmon device.
+
+	  Say 'Y' here if you want power supplies to
+	  have hwmon sysfs interface too.
+
+
 config PDA_POWER
 	tristate "Generic PDA/phone power driver"
 	depends on !S390
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index f208273f9686..c47e88ba16b9 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -6,6 +6,7 @@ power_supply-$(CONFIG_SYSFS)		+= power_supply_sysfs.o
 power_supply-$(CONFIG_LEDS_TRIGGERS)	+= power_supply_leds.o
 
 obj-$(CONFIG_POWER_SUPPLY)	+= power_supply.o
+obj-$(CONFIG_POWER_SUPPLY_HWMON) += power_supply_hwmon.o
 obj-$(CONFIG_GENERIC_ADC_BATTERY)	+= generic-adc-battery.o
 
 obj-$(CONFIG_PDA_POWER)		+= pda_power.o
diff --git a/drivers/power/supply/power_supply_hwmon.c b/drivers/power/supply/power_supply_hwmon.c
new file mode 100644
index 000000000000..3451b997367d
--- /dev/null
+++ b/drivers/power/supply/power_supply_hwmon.c
@@ -0,0 +1,349 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  power_supply_hwmon.c - power supply hwmon support.
+ */
+
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/power_supply.h>
+#include <linux/slab.h>
+
+struct power_supply_hwmon {
+	struct power_supply *psy;
+	unsigned long *props;
+};
+
+static int power_supply_hwmon_in_to_property(u32 attr)
+{
+	switch (attr) {
+	case hwmon_in_average:
+		return POWER_SUPPLY_PROP_VOLTAGE_AVG;
+	case hwmon_in_min:
+		return POWER_SUPPLY_PROP_VOLTAGE_MIN;
+	case hwmon_in_max:
+		return POWER_SUPPLY_PROP_VOLTAGE_MAX;
+	case hwmon_in_input:
+		return POWER_SUPPLY_PROP_VOLTAGE_NOW;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int power_supply_hwmon_curr_to_property(u32 attr)
+{
+	switch (attr) {
+	case hwmon_curr_average:
+		return POWER_SUPPLY_PROP_CURRENT_AVG;
+	case hwmon_curr_max:
+		return POWER_SUPPLY_PROP_CURRENT_MAX;
+	case hwmon_curr_input:
+		return POWER_SUPPLY_PROP_CURRENT_NOW;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int power_supply_hwmon_temp_to_property(u32 attr, int channel)
+{
+	if (channel) {
+		switch (attr) {
+		case hwmon_temp_input:
+			return POWER_SUPPLY_PROP_TEMP_AMBIENT;
+		case hwmon_temp_min_alarm:
+			return POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MIN;
+		case hwmon_temp_max_alarm:
+			return POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MAX;
+		default:
+			break;
+		}
+	} else {
+		switch (attr) {
+		case hwmon_temp_input:
+			return POWER_SUPPLY_PROP_TEMP;
+		case hwmon_temp_max:
+			return POWER_SUPPLY_PROP_TEMP_MAX;
+		case hwmon_temp_min:
+			return POWER_SUPPLY_PROP_TEMP_MIN;
+		case hwmon_temp_min_alarm:
+			return POWER_SUPPLY_PROP_TEMP_ALERT_MIN;
+		case hwmon_temp_max_alarm:
+			return POWER_SUPPLY_PROP_TEMP_ALERT_MAX;
+		default:
+			break;
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int
+power_supply_hwmon_to_property(enum hwmon_sensor_types type,
+			       u32 attr, int channel)
+{
+	switch (type) {
+	case hwmon_in:
+		return power_supply_hwmon_in_to_property(attr);
+	case hwmon_curr:
+		return power_supply_hwmon_curr_to_property(attr);
+	case hwmon_temp:
+		return power_supply_hwmon_temp_to_property(attr, channel);
+	default:
+		return -EINVAL;
+	}
+}
+
+static bool power_supply_hwmon_is_a_label(enum hwmon_sensor_types type,
+					   u32 attr)
+{
+	return type == hwmon_temp && attr == hwmon_temp_label;
+}
+
+static bool power_supply_hwmon_is_writable(enum hwmon_sensor_types type,
+					   u32 attr)
+{
+	switch (type) {
+	case hwmon_in:
+		return attr == hwmon_in_min ||
+		       attr == hwmon_in_max;
+	case hwmon_curr:
+		return attr == hwmon_curr_max;
+	case hwmon_temp:
+		return attr == hwmon_temp_max ||
+		       attr == hwmon_temp_min ||
+		       attr == hwmon_temp_min_alarm ||
+		       attr == hwmon_temp_max_alarm;
+	default:
+		return false;
+	}
+}
+
+static umode_t power_supply_hwmon_is_visible(const void *data,
+					     enum hwmon_sensor_types type,
+					     u32 attr, int channel)
+{
+	const struct power_supply_hwmon *psyhw = data;
+	int prop;
+
+
+	if (power_supply_hwmon_is_a_label(type, attr))
+		return 0444;
+
+	prop = power_supply_hwmon_to_property(type, attr, channel);
+	if (prop < 0 || !test_bit(prop, psyhw->props))
+		return 0;
+
+	if (power_supply_property_is_writeable(psyhw->psy, prop) > 0 &&
+	    power_supply_hwmon_is_writable(type, attr))
+		return 0644;
+
+	return 0444;
+}
+
+static int power_supply_hwmon_read_string(struct device *dev,
+					  enum hwmon_sensor_types type,
+					  u32 attr, int channel,
+					  const char **str)
+{
+	*str = channel ? "temp" : "temp ambient";
+	return 0;
+}
+
+static int
+power_supply_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+			u32 attr, int channel, long *val)
+{
+	struct power_supply_hwmon *psyhw = dev_get_drvdata(dev);
+	struct power_supply *psy = psyhw->psy;
+	union power_supply_propval pspval;
+	int ret, prop;
+
+	prop = power_supply_hwmon_to_property(type, attr, channel);
+	if (prop < 0)
+		return prop;
+
+	ret  = power_supply_get_property(psy, prop, &pspval);
+	if (ret)
+		return ret;
+
+	switch (type) {
+	/*
+	 * Both voltage and current is reported in units of
+	 * microvolts/microamps, so we need to adjust it to
+	 * milliamps(volts)
+	 */
+	case hwmon_curr:
+	case hwmon_in:
+		pspval.intval = DIV_ROUND_CLOSEST(pspval.intval, 1000);
+		break;
+	/*
+	 * Temp needs to be converted from 1/10 C to milli-C
+	 */
+	case hwmon_temp:
+		if (check_mul_overflow(pspval.intval, 100,
+				       &pspval.intval))
+			return -EOVERFLOW;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	*val = pspval.intval;
+
+	return 0;
+}
+
+static int
+power_supply_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
+			 u32 attr, int channel, long val)
+{
+	struct power_supply_hwmon *psyhw = dev_get_drvdata(dev);
+	struct power_supply *psy = psyhw->psy;
+	union power_supply_propval pspval;
+	int prop;
+
+	prop = power_supply_hwmon_to_property(type, attr, channel);
+	if (prop < 0)
+		return prop;
+
+	pspval.intval = val;
+
+	switch (type) {
+	/*
+	 * Both voltage and current is reported in units of
+	 * microvolts/microamps, so we need to adjust it to
+	 * milliamps(volts)
+	 */
+	case hwmon_curr:
+	case hwmon_in:
+		if (check_mul_overflow(pspval.intval, 1000,
+				       &pspval.intval))
+			return -EOVERFLOW;
+		break;
+	/*
+	 * Temp needs to be converted from 1/10 C to milli-C
+	 */
+	case hwmon_temp:
+		pspval.intval = DIV_ROUND_CLOSEST(pspval.intval, 100);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return power_supply_set_property(psy, prop, &pspval);
+}
+
+static const struct hwmon_ops power_supply_hwmon_ops = {
+	.is_visible	= power_supply_hwmon_is_visible,
+	.read		= power_supply_hwmon_read,
+	.write		= power_supply_hwmon_write,
+	.read_string	= power_supply_hwmon_read_string,
+};
+
+static const struct hwmon_channel_info *power_supply_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(temp,
+			   HWMON_T_LABEL     |
+			   HWMON_T_INPUT     |
+			   HWMON_T_MAX       |
+			   HWMON_T_MIN       |
+			   HWMON_T_MIN_ALARM |
+			   HWMON_T_MIN_ALARM,
+
+			   HWMON_T_LABEL     |
+			   HWMON_T_INPUT     |
+			   HWMON_T_MIN_ALARM |
+			   HWMON_T_LABEL     |
+			   HWMON_T_MAX_ALARM),
+
+	HWMON_CHANNEL_INFO(curr,
+			   HWMON_C_AVERAGE |
+			   HWMON_C_MAX     |
+			   HWMON_C_INPUT),
+
+	HWMON_CHANNEL_INFO(in,
+			   HWMON_I_AVERAGE |
+			   HWMON_I_MIN     |
+			   HWMON_I_MAX     |
+			   HWMON_I_INPUT),
+	NULL
+};
+
+static const struct hwmon_chip_info power_supply_hwmon_chip_info = {
+	.ops = &power_supply_hwmon_ops,
+	.info = power_supply_hwmon_info,
+};
+
+static void power_supply_hwmon_bitmap_free(void *data)
+{
+	bitmap_free(data);
+}
+
+int devm_power_supply_add_hwmon_sysfs(struct power_supply *psy)
+{
+	const struct power_supply_desc *desc = psy->desc;
+	struct power_supply_hwmon *psyhw;
+	struct device *dev = &psy->dev;
+	struct device *hwmon;
+	int ret, i;
+
+	if (!devres_open_group(dev, NULL, GFP_KERNEL))
+		return -ENOMEM;
+
+	psyhw = devm_kzalloc(dev, sizeof(*psyhw), GFP_KERNEL);
+	if (!psyhw) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	psyhw->psy = psy;
+	psyhw->props = bitmap_zalloc(POWER_SUPPLY_PROP_TIME_TO_FULL_AVG + 1,
+				     GFP_KERNEL);
+	if (!psyhw->props) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	ret = devm_add_action(dev, power_supply_hwmon_bitmap_free,
+			      psyhw->props);
+	if (ret)
+		goto error;
+
+	for (i = 0; i < desc->num_properties; i++) {
+		const enum power_supply_property prop = desc->properties[i];
+
+		switch (prop) {
+		case POWER_SUPPLY_PROP_CURRENT_AVG:
+		case POWER_SUPPLY_PROP_CURRENT_MAX:
+		case POWER_SUPPLY_PROP_CURRENT_NOW:
+		case POWER_SUPPLY_PROP_TEMP:
+		case POWER_SUPPLY_PROP_TEMP_MAX:
+		case POWER_SUPPLY_PROP_TEMP_MIN:
+		case POWER_SUPPLY_PROP_TEMP_ALERT_MIN:
+		case POWER_SUPPLY_PROP_TEMP_ALERT_MAX:
+		case POWER_SUPPLY_PROP_TEMP_AMBIENT:
+		case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MIN:
+		case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MAX:
+		case POWER_SUPPLY_PROP_VOLTAGE_AVG:
+		case POWER_SUPPLY_PROP_VOLTAGE_MIN:
+		case POWER_SUPPLY_PROP_VOLTAGE_MAX:
+		case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+			set_bit(prop, psyhw->props);
+			break;
+		default:
+			break;
+		}
+	}
+
+	hwmon = devm_hwmon_device_register_with_info(dev, psy->desc->name,
+						psyhw,
+						&power_supply_hwmon_chip_info,
+						NULL);
+	ret = PTR_ERR_OR_ZERO(hwmon);
+	if (ret)
+		goto error;
+
+	devres_remove_group(dev, NULL);
+	return 0;
+error:
+	devres_release_group(dev, NULL);
+	return ret;
+}
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index d9c0c094f8a0..e94cdfa797e3 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -481,4 +481,13 @@ static inline bool power_supply_is_watt_property(enum power_supply_property psp)
 	return 0;
 }
 
+#ifdef CONFIG_POWER_SUPPLY_HWMON
+int devm_power_supply_add_hwmon_sysfs(struct power_supply *psy);
+#else
+int devm_power_supply_add_hwmon_sysfs(struct power_supply *psy)
+{
+	return 0;
+}
+#endif
+
 #endif /* __LINUX_POWER_SUPPLY_H__ */
-- 
2.21.0

[PATCH v2 2/2] power: supply: ucs1002: Add HWMON interface

[Andrey Smirnov]

Expose current sensors found on UCS1002 via HWMON.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-pm@vger.kernel.org
---
 drivers/power/supply/ucs1002_power.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/power/supply/ucs1002_power.c b/drivers/power/supply/ucs1002_power.c
index 1c89d030c045..ebe84f1ca42f 100644
--- a/drivers/power/supply/ucs1002_power.c
+++ b/drivers/power/supply/ucs1002_power.c
@@ -571,6 +571,12 @@ static int ucs1002_probe(struct i2c_client *client,
 		return ret;
 	}
 
+	ret = devm_power_supply_add_hwmon_sysfs(info->charger);
+	if (ret) {
+		dev_err(dev, "Failed to add hmwon attributes: %d\n", ret);
+		return ret;
+	}
+
 	ret = regmap_read(info->regmap, UCS1002_REG_PIN_STATUS, &regval);
 	if (ret) {
 		dev_err(dev, "Failed to read pin status: %d\n", ret);
-- 
2.21.0

Re: [PATCH v2 1/2] power: supply: Add HWMON compatibility layer

[Guenter Roeck]

On Thu, May 30, 2019 at 06:16:19PM -0700, Andrey Smirnov wrote:
> Add code implementing HWMON adapter/compatibility layer to allow
> expositing various sensors present on power supply devices via HWMON
> subsystem. This is done in order to allow userspace to use single
> ABI/library(libsensors) to access/manipulate all of the sensors of the
> system.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-pm@vger.kernel.org
> ---
>  drivers/power/supply/Kconfig              |  14 +
>  drivers/power/supply/Makefile             |   1 +
>  drivers/power/supply/power_supply_hwmon.c | 349 ++++++++++++++++++++++
>  include/linux/power_supply.h              |   9 +
>  4 files changed, 373 insertions(+)
>  create mode 100644 drivers/power/supply/power_supply_hwmon.c
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 26dacdab03cc..1f2252cb95fd 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -14,6 +14,20 @@ config POWER_SUPPLY_DEBUG
>  	  Say Y here to enable debugging messages for power supply class
>  	  and drivers.
>  
> +config POWER_SUPPLY_HWMON
> +	bool
> +	prompt "Expose power supply sensors as hwmon device"
> +	depends on HWMON=y || HWMON=POWER_SUPPLY
> +	default y
> +	help
> +	  This options enables API that allows sensors found on a
> +	  power supply device (current, voltage, temperature) to be
> +	  exposed as a hwmon device.
> +
> +	  Say 'Y' here if you want power supplies to
> +	  have hwmon sysfs interface too.
> +
> +
>  config PDA_POWER
>  	tristate "Generic PDA/phone power driver"
>  	depends on !S390
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index f208273f9686..c47e88ba16b9 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -6,6 +6,7 @@ power_supply-$(CONFIG_SYSFS)		+= power_supply_sysfs.o
>  power_supply-$(CONFIG_LEDS_TRIGGERS)	+= power_supply_leds.o
>  
>  obj-$(CONFIG_POWER_SUPPLY)	+= power_supply.o
> +obj-$(CONFIG_POWER_SUPPLY_HWMON) += power_supply_hwmon.o
>  obj-$(CONFIG_GENERIC_ADC_BATTERY)	+= generic-adc-battery.o
>  
>  obj-$(CONFIG_PDA_POWER)		+= pda_power.o
> diff --git a/drivers/power/supply/power_supply_hwmon.c b/drivers/power/supply/power_supply_hwmon.c
> new file mode 100644
> index 000000000000..3451b997367d
> --- /dev/null
> +++ b/drivers/power/supply/power_supply_hwmon.c
> @@ -0,0 +1,349 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  power_supply_hwmon.c - power supply hwmon support.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/power_supply.h>
> +#include <linux/slab.h>
> +
> +struct power_supply_hwmon {
> +	struct power_supply *psy;
> +	unsigned long *props;
> +};
> +
> +static int power_supply_hwmon_in_to_property(u32 attr)
> +{
> +	switch (attr) {
> +	case hwmon_in_average:
> +		return POWER_SUPPLY_PROP_VOLTAGE_AVG;
> +	case hwmon_in_min:
> +		return POWER_SUPPLY_PROP_VOLTAGE_MIN;
> +	case hwmon_in_max:
> +		return POWER_SUPPLY_PROP_VOLTAGE_MAX;
> +	case hwmon_in_input:
> +		return POWER_SUPPLY_PROP_VOLTAGE_NOW;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int power_supply_hwmon_curr_to_property(u32 attr)
> +{
> +	switch (attr) {
> +	case hwmon_curr_average:
> +		return POWER_SUPPLY_PROP_CURRENT_AVG;
> +	case hwmon_curr_max:
> +		return POWER_SUPPLY_PROP_CURRENT_MAX;
> +	case hwmon_curr_input:
> +		return POWER_SUPPLY_PROP_CURRENT_NOW;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int power_supply_hwmon_temp_to_property(u32 attr, int channel)
> +{
> +	if (channel) {
> +		switch (attr) {
> +		case hwmon_temp_input:
> +			return POWER_SUPPLY_PROP_TEMP_AMBIENT;
> +		case hwmon_temp_min_alarm:
> +			return POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MIN;
> +		case hwmon_temp_max_alarm:
> +			return POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MAX;
> +		default:
> +			break;
> +		}
> +	} else {
> +		switch (attr) {
> +		case hwmon_temp_input:
> +			return POWER_SUPPLY_PROP_TEMP;
> +		case hwmon_temp_max:
> +			return POWER_SUPPLY_PROP_TEMP_MAX;
> +		case hwmon_temp_min:
> +			return POWER_SUPPLY_PROP_TEMP_MIN;
> +		case hwmon_temp_min_alarm:
> +			return POWER_SUPPLY_PROP_TEMP_ALERT_MIN;
> +		case hwmon_temp_max_alarm:
> +			return POWER_SUPPLY_PROP_TEMP_ALERT_MAX;
> +		default:
> +			break;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int
> +power_supply_hwmon_to_property(enum hwmon_sensor_types type,
> +			       u32 attr, int channel)
> +{
> +	switch (type) {
> +	case hwmon_in:
> +		return power_supply_hwmon_in_to_property(attr);
> +	case hwmon_curr:
> +		return power_supply_hwmon_curr_to_property(attr);
> +	case hwmon_temp:
> +		return power_supply_hwmon_temp_to_property(attr, channel);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static bool power_supply_hwmon_is_a_label(enum hwmon_sensor_types type,
> +					   u32 attr)
> +{
> +	return type == hwmon_temp && attr == hwmon_temp_label;
> +}
> +
> +static bool power_supply_hwmon_is_writable(enum hwmon_sensor_types type,
> +					   u32 attr)
> +{
> +	switch (type) {
> +	case hwmon_in:
> +		return attr == hwmon_in_min ||
> +		       attr == hwmon_in_max;
> +	case hwmon_curr:
> +		return attr == hwmon_curr_max;
> +	case hwmon_temp:
> +		return attr == hwmon_temp_max ||
> +		       attr == hwmon_temp_min ||
> +		       attr == hwmon_temp_min_alarm ||
> +		       attr == hwmon_temp_max_alarm;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static umode_t power_supply_hwmon_is_visible(const void *data,
> +					     enum hwmon_sensor_types type,
> +					     u32 attr, int channel)
> +{
> +	const struct power_supply_hwmon *psyhw = data;
> +	int prop;
> +
> +
> +	if (power_supply_hwmon_is_a_label(type, attr))
> +		return 0444;
> +
> +	prop = power_supply_hwmon_to_property(type, attr, channel);
> +	if (prop < 0 || !test_bit(prop, psyhw->props))
> +		return 0;
> +
> +	if (power_supply_property_is_writeable(psyhw->psy, prop) > 0 &&
> +	    power_supply_hwmon_is_writable(type, attr))
> +		return 0644;
> +
> +	return 0444;
> +}
> +
> +static int power_supply_hwmon_read_string(struct device *dev,
> +					  enum hwmon_sensor_types type,
> +					  u32 attr, int channel,
> +					  const char **str)
> +{
> +	*str = channel ? "temp" : "temp ambient";
> +	return 0;
> +}
> +
> +static int
> +power_supply_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> +			u32 attr, int channel, long *val)
> +{
> +	struct power_supply_hwmon *psyhw = dev_get_drvdata(dev);
> +	struct power_supply *psy = psyhw->psy;
> +	union power_supply_propval pspval;
> +	int ret, prop;
> +
> +	prop = power_supply_hwmon_to_property(type, attr, channel);
> +	if (prop < 0)
> +		return prop;
> +
> +	ret  = power_supply_get_property(psy, prop, &pspval);
> +	if (ret)
> +		return ret;
> +
> +	switch (type) {
> +	/*
> +	 * Both voltage and current is reported in units of
> +	 * microvolts/microamps, so we need to adjust it to
> +	 * milliamps(volts)
> +	 */
> +	case hwmon_curr:
> +	case hwmon_in:
> +		pspval.intval = DIV_ROUND_CLOSEST(pspval.intval, 1000);
> +		break;
> +	/*
> +	 * Temp needs to be converted from 1/10 C to milli-C
> +	 */
> +	case hwmon_temp:
> +		if (check_mul_overflow(pspval.intval, 100,
> +				       &pspval.intval))
> +			return -EOVERFLOW;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	*val = pspval.intval;
> +
> +	return 0;
> +}
> +
> +static int
> +power_supply_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> +			 u32 attr, int channel, long val)
> +{
> +	struct power_supply_hwmon *psyhw = dev_get_drvdata(dev);
> +	struct power_supply *psy = psyhw->psy;
> +	union power_supply_propval pspval;
> +	int prop;
> +
> +	prop = power_supply_hwmon_to_property(type, attr, channel);
> +	if (prop < 0)
> +		return prop;
> +
> +	pspval.intval = val;
> +
> +	switch (type) {
> +	/*
> +	 * Both voltage and current is reported in units of
> +	 * microvolts/microamps, so we need to adjust it to
> +	 * milliamps(volts)
> +	 */
> +	case hwmon_curr:
> +	case hwmon_in:
> +		if (check_mul_overflow(pspval.intval, 1000,
> +				       &pspval.intval))
> +			return -EOVERFLOW;
> +		break;
> +	/*
> +	 * Temp needs to be converted from 1/10 C to milli-C
> +	 */
> +	case hwmon_temp:
> +		pspval.intval = DIV_ROUND_CLOSEST(pspval.intval, 100);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return power_supply_set_property(psy, prop, &pspval);
> +}
> +
> +static const struct hwmon_ops power_supply_hwmon_ops = {
> +	.is_visible	= power_supply_hwmon_is_visible,
> +	.read		= power_supply_hwmon_read,
> +	.write		= power_supply_hwmon_write,
> +	.read_string	= power_supply_hwmon_read_string,
> +};
> +
> +static const struct hwmon_channel_info *power_supply_hwmon_info[] = {
> +	HWMON_CHANNEL_INFO(temp,
> +			   HWMON_T_LABEL     |
> +			   HWMON_T_INPUT     |
> +			   HWMON_T_MAX       |
> +			   HWMON_T_MIN       |
> +			   HWMON_T_MIN_ALARM |
> +			   HWMON_T_MIN_ALARM,
> +
> +			   HWMON_T_LABEL     |
> +			   HWMON_T_INPUT     |
> +			   HWMON_T_MIN_ALARM |
> +			   HWMON_T_LABEL     |
> +			   HWMON_T_MAX_ALARM),
> +
> +	HWMON_CHANNEL_INFO(curr,
> +			   HWMON_C_AVERAGE |
> +			   HWMON_C_MAX     |
> +			   HWMON_C_INPUT),
> +
> +	HWMON_CHANNEL_INFO(in,
> +			   HWMON_I_AVERAGE |
> +			   HWMON_I_MIN     |
> +			   HWMON_I_MAX     |
> +			   HWMON_I_INPUT),
> +	NULL
> +};
> +
> +static const struct hwmon_chip_info power_supply_hwmon_chip_info = {
> +	.ops = &power_supply_hwmon_ops,
> +	.info = power_supply_hwmon_info,
> +};
> +
> +static void power_supply_hwmon_bitmap_free(void *data)
> +{
> +	bitmap_free(data);
> +}
> +
> +int devm_power_supply_add_hwmon_sysfs(struct power_supply *psy)
> +{
> +	const struct power_supply_desc *desc = psy->desc;
> +	struct power_supply_hwmon *psyhw;
> +	struct device *dev = &psy->dev;
> +	struct device *hwmon;
> +	int ret, i;
> +
> +	if (!devres_open_group(dev, NULL, GFP_KERNEL))
> +		return -ENOMEM;
> +
> +	psyhw = devm_kzalloc(dev, sizeof(*psyhw), GFP_KERNEL);
> +	if (!psyhw) {
> +		ret = -ENOMEM;
> +		goto error;
> +	}
> +
> +	psyhw->psy = psy;
> +	psyhw->props = bitmap_zalloc(POWER_SUPPLY_PROP_TIME_TO_FULL_AVG + 1,
> +				     GFP_KERNEL);
> +	if (!psyhw->props) {
> +		ret = -ENOMEM;
> +		goto error;
> +	}
> +
> +	ret = devm_add_action(dev, power_supply_hwmon_bitmap_free,
> +			      psyhw->props);
> +	if (ret)
> +		goto error;
> +
> +	for (i = 0; i < desc->num_properties; i++) {
> +		const enum power_supply_property prop = desc->properties[i];
> +
> +		switch (prop) {
> +		case POWER_SUPPLY_PROP_CURRENT_AVG:
> +		case POWER_SUPPLY_PROP_CURRENT_MAX:
> +		case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		case POWER_SUPPLY_PROP_TEMP:
> +		case POWER_SUPPLY_PROP_TEMP_MAX:
> +		case POWER_SUPPLY_PROP_TEMP_MIN:
> +		case POWER_SUPPLY_PROP_TEMP_ALERT_MIN:
> +		case POWER_SUPPLY_PROP_TEMP_ALERT_MAX:
> +		case POWER_SUPPLY_PROP_TEMP_AMBIENT:
> +		case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MIN:
> +		case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MAX:
> +		case POWER_SUPPLY_PROP_VOLTAGE_AVG:
> +		case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> +		case POWER_SUPPLY_PROP_VOLTAGE_MAX:
> +		case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +			set_bit(prop, psyhw->props);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +
> +	hwmon = devm_hwmon_device_register_with_info(dev, psy->desc->name,
> +						psyhw,
> +						&power_supply_hwmon_chip_info,
> +						NULL);
> +	ret = PTR_ERR_OR_ZERO(hwmon);
> +	if (ret)
> +		goto error;
> +
> +	devres_remove_group(dev, NULL);
> +	return 0;
> +error:
> +	devres_release_group(dev, NULL);
> +	return ret;
> +}
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index d9c0c094f8a0..e94cdfa797e3 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -481,4 +481,13 @@ static inline bool power_supply_is_watt_property(enum power_supply_property psp)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_POWER_SUPPLY_HWMON
> +int devm_power_supply_add_hwmon_sysfs(struct power_supply *psy);
> +#else
> +int devm_power_supply_add_hwmon_sysfs(struct power_supply *psy)

That needs to be static inline int to avoid duplicate symbols.

Otherwise

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> +{
> +	return 0;
> +}
> +#endif
> +
>  #endif /* __LINUX_POWER_SUPPLY_H__ */
> -- 
> 2.21.0
> 

Re: [PATCH v2 2/2] power: supply: ucs1002: Add HWMON interface

[Guenter Roeck]

On Thu, May 30, 2019 at 06:16:20PM -0700, Andrey Smirnov wrote:
> Expose current sensors found on UCS1002 via HWMON.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-pm@vger.kernel.org

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/power/supply/ucs1002_power.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/power/supply/ucs1002_power.c b/drivers/power/supply/ucs1002_power.c
> index 1c89d030c045..ebe84f1ca42f 100644
> --- a/drivers/power/supply/ucs1002_power.c
> +++ b/drivers/power/supply/ucs1002_power.c
> @@ -571,6 +571,12 @@ static int ucs1002_probe(struct i2c_client *client,
>  		return ret;
>  	}
>  
> +	ret = devm_power_supply_add_hwmon_sysfs(info->charger);
> +	if (ret) {
> +		dev_err(dev, "Failed to add hmwon attributes: %d\n", ret);
> +		return ret;
> +	}
> +
>  	ret = regmap_read(info->regmap, UCS1002_REG_PIN_STATUS, &regval);
>  	if (ret) {
>  		dev_err(dev, "Failed to read pin status: %d\n", ret);
> -- 
> 2.21.0
>