linux-kernel - Re: [PATCH] hwmon: ina2xx: Add per-sensor label

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56D91CCE.4070408@roeck-us.net>
Date:	Thu, 3 Mar 2016 21:27:42 -0800
From:	Guenter Roeck <linux@...ck-us.net>
To:	Anatol Pomozov <anatol.pomozov@...il.com>, mtitinger@...libre.com
Cc:	linux-kernel@...r.kernel.org, Anatol Pomazau <anatol@...gle.com>
Subject: Re: [PATCH] hwmon: ina2xx: Add per-sensor label

On 03/03/2016 01:02 PM, Anatol Pomozov wrote:
> From: Anatol Pomazau <anatol@...gle.com>
>
> Some systems have a lot of power sensors and having an way to label sensor
> makes easier to use sensor information.
>
> Add dts property 'label' that is readable via device sysfs file 'label'.
>
> Signed-off-by: Anatol Pomozov <anatol.pomozov@...il.com>

Labels can be attached via /etc/sensors3.conf. I don't really like
the idea of bypassing that mechanism, and the devicetree maintainers
don't like it either. The idea of labels is for those to be provided
by the driver if an association to a specific meaning (eg VCC)
is known.

Any reason why sensors3.conf would not be feasible ?

Thanks,
Guenter

> ---
>   Documentation/devicetree/bindings/hwmon/ina2xx.txt |  3 +++
>   drivers/hwmon/ina2xx.c                             | 19 +++++++++++++++++++
>   2 files changed, 22 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/ina2xx.txt b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
> index 9bcd5e8..76315e2 100644
> --- a/Documentation/devicetree/bindings/hwmon/ina2xx.txt
> +++ b/Documentation/devicetree/bindings/hwmon/ina2xx.txt
> @@ -11,6 +11,9 @@ Required properties:
>
>   Optional properties:
>
> +- label
> +	Human readable identifier for the sensor. Readable via sysfs file 'label'.
> +
>   - shunt-resistor
>   	Shunt resistor value in micro-Ohm
>
> diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
> index b24f1d3..6e15115 100644
> --- a/drivers/hwmon/ina2xx.c
> +++ b/drivers/hwmon/ina2xx.c
> @@ -108,6 +108,7 @@ struct ina2xx_data {
>   	long rshunt;
>   	struct mutex config_lock;
>   	struct regmap *regmap;
> +	const char *label;
>
>   	const struct attribute_group *groups[INA2XX_MAX_ATTRIBUTE_GROUPS];
>   };
> @@ -368,6 +369,21 @@ static ssize_t ina226_show_interval(struct device *dev,
>   	return snprintf(buf, PAGE_SIZE, "%d\n", ina226_reg_to_interval(regval));
>   }
>
> +static ssize_t ina2xx_show_label(struct device *dev,
> +			struct device_attribute *da, char *buf)
> +{
> +	struct ina2xx_data *data = dev_get_drvdata(dev);
> +	ssize_t ret = 0;
> +
> +	if (data->label)
> +		ret = scnprintf(buf, PAGE_SIZE, "%s\n", data->label);
> +
> +	return ret;
> +}
> +
> +/* label */
> +static SENSOR_DEVICE_ATTR(label, S_IRUGO, ina2xx_show_label, NULL,  0);
> +
>   /* shunt voltage */
>   static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, ina2xx_show_value, NULL,
>   			  INA2XX_SHUNT_VOLTAGE);
> @@ -395,6 +411,7 @@ static SENSOR_DEVICE_ATTR(update_interval, S_IRUGO | S_IWUSR,
>
>   /* pointers to created device attributes */
>   static struct attribute *ina2xx_attrs[] = {
> +	&sensor_dev_attr_label.dev_attr.attr,
>   	&sensor_dev_attr_in0_input.dev_attr.attr,
>   	&sensor_dev_attr_in1_input.dev_attr.attr,
>   	&sensor_dev_attr_curr1_input.dev_attr.attr,
> @@ -432,6 +449,8 @@ static int ina2xx_probe(struct i2c_client *client,
>   	/* set the device type */
>   	data->config = &ina2xx_config[id->driver_data];
>
> +	of_property_read_string(dev->of_node, "label", &data->label);
> +
>   	if (of_property_read_u32(dev->of_node, "shunt-resistor", &val) < 0) {
>   		struct ina2xx_platform_data *pdata = dev_get_platdata(dev);
>
>