On 07/15/2016 02:59 AM, Quentin Schulz wrote:
Currently, iio_hwmon only exposes values of the IIO channels it can read
but no label by channel is exposed.
This adds exposition of sysfs files containing label for IIO channels it
can read based on extended_name field of the iio_chan_spec of the channel.
If the extended_name field is empty, the sysfs file is not created by
iio_hwmon.
Signed-off-by: Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxx>
---
patch added in v2
drivers/hwmon/iio_hwmon.c | 77 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 68 insertions(+), 9 deletions(-)
diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
index c0da4d9..28d15b2 100644
--- a/drivers/hwmon/iio_hwmon.c
+++ b/drivers/hwmon/iio_hwmon.c
@@ -16,6 +16,7 @@
#include <linux/of.h>
#include <linux/hwmon-sysfs.h>
#include <linux/iio/consumer.h>
+#include <linux/iio/iio.h>
#include <linux/iio/types.h>
/**
@@ -57,12 +58,26 @@ static ssize_t iio_hwmon_read_val(struct device *dev,
return sprintf(buf, "%d\n", result);
}
+static ssize_t iio_hwmon_read_label(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
+ struct iio_hwmon_state *state = dev_get_drvdata(dev);
+ const char *label = state->channels[sattr->index].channel->extend_name;
+
+ if (label)
+ return sprintf(buf, "%s\n", label);
+
Can the name disappear on the fly, or be changed on the fly ?
Then this is unusable. We should and can only provide labels
if a name exists and is permanent. Otherwise all we do is
to confuse user space.
+ return 0;
+}
+
static int iio_hwmon_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct iio_hwmon_state *st;
- struct sensor_device_attribute *a;
- int ret, i;
+ struct sensor_device_attribute *a, *b;
+ int ret, i, j = 0;
int in_i = 1, temp_i = 1, curr_i = 1, humidity_i = 1;
enum iio_chan_type type;
struct iio_channel *channels;
@@ -92,7 +107,8 @@ static int iio_hwmon_probe(struct platform_device *pdev)
st->num_channels++;
st->attrs = devm_kzalloc(dev,
- sizeof(*st->attrs) * (st->num_channels + 1),
+ sizeof(*st->attrs) *
+ (2 * st->num_channels + 1),
GFP_KERNEL);
if (st->attrs == NULL) {
ret = -ENOMEM;
@@ -107,6 +123,18 @@ static int iio_hwmon_probe(struct platform_device *pdev)
}
sysfs_attr_init(&a->dev_attr.attr);
+
+ b = NULL;
+ if (st->channels[i].channel->extend_name) {
+ b = devm_kzalloc(dev, sizeof(*b), GFP_KERNEL);
+ if (b == NULL) {
+ ret = -ENOMEM;
+ goto error_release_channels;
+ }
+
+ sysfs_attr_init(&b->dev_attr.attr);
Why is this initialization here and not with the rest of the initialization
of this attribute ?
+ }
+
ret = iio_get_channel_type(&st->channels[i], &type);
if (ret < 0)
goto error_release_channels;
@@ -115,35 +143,66 @@ static int iio_hwmon_probe(struct platform_device *pdev)
case IIO_VOLTAGE:
a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
"in%d_input",
- in_i++);
+ in_i);
+ if (b)
+ b->dev_attr.attr.name = kasprintf(GFP_KERNEL,
+ "in%d_label",
+ in_i);
+ in_i++;
break;
case IIO_TEMP:
a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
"temp%d_input",
- temp_i++);
+ temp_i);
+
+ if (b)
+ b->dev_attr.attr.name = kasprintf(GFP_KERNEL,
+ "temp%d_label",
+ temp_i);
+ temp_i++;
break;
case IIO_CURRENT:
a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
"curr%d_input",
- curr_i++);
+ curr_i);
+
+ if (b)
+ b->dev_attr.attr.name = kasprintf(GFP_KERNEL,
+ "curr%d_label",
+ curr_i);
+ curr_i++;
break;
case IIO_HUMIDITYRELATIVE:
a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
"humidity%d_input",
- humidity_i++);
+ humidity_i);
+
+ if (b)
+ b->dev_attr.attr.name = kasprintf(GFP_KERNEL,
+ "humidity%d_label",
+ humidity_i);
+ humidity_i++;
break;
default:
ret = -EINVAL;
goto error_release_channels;
}
- if (a->dev_attr.attr.name == NULL) {
+ if (a->dev_attr.attr.name == NULL ||
+ (b && b->dev_attr.attr.name == NULL)) {
ret = -ENOMEM;
goto error_release_channels;
}
Just realized that we have a memory leak here. The 'name' memory is never released.
a->dev_attr.show = iio_hwmon_read_val;
a->dev_attr.attr.mode = S_IRUGO;
a->index = i;
- st->attrs[i] = &a->dev_attr.attr;
+ st->attrs[j++] = &a->dev_attr.attr;
+
+ if (b) {
+ b->dev_attr.show = iio_hwmon_read_label;
+ b->dev_attr.attr.mode = S_IRUGO;
+ b->index = i;
+ st->attrs[j++] = &b->dev_attr.attr;
+ }
}
st->attr_group.attrs = st->attrs;
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
