2016-03-03 21:03:06

by Anatol Pomozov

[permalink] [raw]
Subject: [PATCH] hwmon: ina2xx: Add per-sensor label

From: Anatol Pomazau <[email protected]>

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 <[email protected]>
---
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);

--
2.7.0.rc3.207.g0ac5344


2016-03-04 05:27:46

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH] hwmon: ina2xx: Add per-sensor label

On 03/03/2016 01:02 PM, Anatol Pomozov wrote:
> From: Anatol Pomazau <[email protected]>
>
> 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 <[email protected]>

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);
>
>

2016-03-07 09:14:40

by Marc Titinger

[permalink] [raw]
Subject: Re: [PATCH] hwmon: ina2xx: Add per-sensor label



On 03/03/2016 22:02, Anatol Pomozov wrote:
> From: Anatol Pomazau <[email protected]>
>
> 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'.

Hi Anatol and all,

I haven't looked how to do that, but if this label was to extend the
chip name instead, could this benefit to any existing client app already ?

Cheers,
Marc.

>
> Signed-off-by: Anatol Pomozov <[email protected]>
> ---
> 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);
>