Re: [PATCH 1/2] hwmon: (ltc2947) Use the energy64 attribute type to report the energy

["Nuno Sá" <noname.nuno@gmail.com>]

On Sun, 2025-10-12 at 14:16 -0700, Guenter Roeck wrote:
> Use the energy64 attribute type instead of a locally defined sysfs
> attribute to report the accumulated energy.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/hwmon/ltc2947-core.c | 60 ++++++++++++++++--------------------
>  1 file changed, 26 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/hwmon/ltc2947-core.c b/drivers/hwmon/ltc2947-core.c
> index 244839167e51..90f70f732b41 100644
> --- a/drivers/hwmon/ltc2947-core.c
> +++ b/drivers/hwmon/ltc2947-core.c
> @@ -9,8 +9,8 @@
>  #include <linux/clk.h>
>  #include <linux/device.h>
>  #include <linux/hwmon.h>
> -#include <linux/hwmon-sysfs.h>
>  #include <linux/module.h>
> +#include <linux/math64.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/property.h>
>  #include <linux/regmap.h>
> @@ -319,24 +319,6 @@ static int ltc2947_alarm_read(struct ltc2947_data *st,
> const u8 reg,
>  	return ret;
>  }
>  
> -static ssize_t ltc2947_show_value(struct device *dev,
> -				  struct device_attribute *da, char *buf)
> -{
> -	struct ltc2947_data *st = dev_get_drvdata(dev);
> -	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> -	int ret;
> -	s64 val = 0;
> -
> -	ret = ltc2947_val_read(st, attr->index, LTC2947_PAGE0, 6, &val);
> -	if (ret)
> -		return ret;
> -
> -	/* value in microJoule. st->lsb_energy was multiplied by 10E9 */
> -	val = div_s64(val * st->lsb_energy, 1000);
> -
> -	return sprintf(buf, "%lld\n", val);
> -}
> -
>  static int ltc2947_read_temp(struct device *dev, const u32 attr, long *val,
>  			     const int channel)
>  {
> @@ -588,6 +570,23 @@ static int ltc2947_read_in(struct device *dev, const u32
> attr, long *val,
>  	return 0;
>  }
>  
> +static int ltc2947_read_energy(struct device *dev, s64 *val, const int
> channel)
> +{
> +	int reg = channel ? LTC2947_REG_ENERGY2 : LTC2947_REG_ENERGY1;
> +	struct ltc2947_data *st = dev_get_drvdata(dev);
> +	s64 __val = 0;
> +	int ret;
> +
> +	ret = ltc2947_val_read(st, reg, LTC2947_PAGE0, 6, &__val);
> +	if (ret)
> +		return ret;
> +
> +	/* value in microJoule. st->lsb_energy was multiplied by 10E9 */
> +	*val = DIV_S64_ROUND_CLOSEST(__val * st->lsb_energy, 1000);
> +
> +	return 0;
> +}
> +
>  static int ltc2947_read(struct device *dev, enum hwmon_sensor_types type,
>  			u32 attr, int channel, long *val)
>  {
> @@ -600,6 +599,8 @@ static int ltc2947_read(struct device *dev, enum
> hwmon_sensor_types type,
>  		return ltc2947_read_power(dev, attr, val);
>  	case hwmon_temp:
>  		return ltc2947_read_temp(dev, attr, val, channel);
> +	case hwmon_energy64:
> +		return ltc2947_read_energy(dev, (s64 *)val, channel);
>  	default:
>  		return -ENOTSUPP;
>  	}
> @@ -897,6 +898,8 @@ static umode_t ltc2947_is_visible(const void *data,
>  		return ltc2947_power_is_visible(attr);
>  	case hwmon_temp:
>  		return ltc2947_temp_is_visible(attr);
> +	case hwmon_energy64:
> +		return 0444;
>  	default:
>  		return 0;
>  	}
> @@ -929,6 +932,9 @@ static const struct hwmon_channel_info * const
> ltc2947_info[] = {
>  			   HWMON_T_LABEL,
>  			   HWMON_T_MAX_ALARM | HWMON_T_MIN_ALARM |
> HWMON_T_MAX |
>  			   HWMON_T_MIN | HWMON_T_LABEL),
> +	HWMON_CHANNEL_INFO(energy64,
> +			   HWMON_E_INPUT,
> +			   HWMON_E_INPUT),
>  	NULL
>  };
>  
> @@ -944,19 +950,6 @@ static const struct hwmon_chip_info ltc2947_chip_info = {
>  	.info = ltc2947_info,
>  };
>  
> -/* energy attributes are 6bytes wide so we need u64 */
> -static SENSOR_DEVICE_ATTR(energy1_input, 0444, ltc2947_show_value, NULL,
> -			  LTC2947_REG_ENERGY1);
> -static SENSOR_DEVICE_ATTR(energy2_input, 0444, ltc2947_show_value, NULL,
> -			  LTC2947_REG_ENERGY2);
> -
> -static struct attribute *ltc2947_attrs[] = {
> -	&sensor_dev_attr_energy1_input.dev_attr.attr,
> -	&sensor_dev_attr_energy2_input.dev_attr.attr,
> -	NULL,
> -};
> -ATTRIBUTE_GROUPS(ltc2947);
> -
>  static int ltc2947_setup(struct ltc2947_data *st)
>  {
>  	int ret;
> @@ -1114,8 +1107,7 @@ int ltc2947_core_probe(struct regmap *map, const char
> *name)
>  		return ret;
>  
>  	hwmon = devm_hwmon_device_register_with_info(dev, name, st,
> -						     &ltc2947_chip_info,
> -						     ltc2947_groups);
> +						     &ltc2947_chip_info,
> NULL);
>  	return PTR_ERR_OR_ZERO(hwmon);
>  }
>  EXPORT_SYMBOL_GPL(ltc2947_core_probe);