If a label is defined in the device tree for this channel add that
to the channel specific attributes. This is useful for userspace to
be able to identify an individual channel.
Signed-off-by: Cristian Pop <[email protected]>
---
drivers/iio/industrialio-core.c | 40 +++++++++++++++++++++++++++++++++
include/linux/iio/iio.h | 6 +++++
2 files changed, 46 insertions(+)
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 6e388293c828..b577fff35641 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -669,6 +669,19 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
}
EXPORT_SYMBOL_GPL(iio_format_value);
+static ssize_t iio_read_channel_label(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+
+ if (!indio_dev->info->read_label)
+ return -EINVAL;
+
+ return indio_dev->info->read_label(indio_dev, this_attr->c, buf);
+}
+
static ssize_t iio_read_channel_info(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -1137,6 +1150,28 @@ int __iio_add_chan_devattr(const char *postfix,
return ret;
}
+static int iio_device_add_channel_label(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan)
+{
+ int ret;
+
+ if (!indio_dev->info->read_label)
+ return 0;
+
+ ret = __iio_add_chan_devattr("label",
+ chan,
+ &iio_read_channel_label,
+ NULL,
+ 0,
+ IIO_SEPARATE,
+ &indio_dev->dev,
+ &indio_dev->channel_attr_list);
+ if (ret < 0)
+ return ret;
+
+ return 1;
+}
+
static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
enum iio_shared_by shared_by,
@@ -1270,6 +1305,11 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
return ret;
attrcount += ret;
+ ret = iio_device_add_channel_label(indio_dev, chan);
+ if (ret < 0)
+ return ret;
+ attrcount += ret;
+
if (chan->ext_info) {
unsigned int i = 0;
for (ext_info = chan->ext_info; ext_info->name; ext_info++) {
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 2e45b3ceafa7..9a3cf4815148 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -362,6 +362,8 @@ struct iio_trigger; /* forward declaration */
* and max. For lists, all possible values are enumerated.
* @write_raw: function to write a value to the device.
* Parameters are the same as for read_raw.
+ * @read_label: function to request label name for a specified label,
+ * for better channel identification.
* @write_raw_get_fmt: callback function to query the expected
* format/precision. If not set by the driver, write_raw
* returns IIO_VAL_INT_PLUS_MICRO.
@@ -420,6 +422,10 @@ struct iio_info {
int val2,
long mask);
+ int (*read_label)(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ char *label);
+
int (*write_raw_get_fmt)(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
long mask);
--
2.17.1
For better identification of the channels.
Signed-off-by: Cristian Pop <[email protected]>
---
drivers/iio/adc/ad7768-1.c | 41 ++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index 0e93b0766eb4..5c0cbee03230 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -161,6 +161,7 @@ struct ad7768_state {
struct completion completion;
struct iio_trigger *trig;
struct gpio_desc *gpio_sync_in;
+ const char *labels[ARRAY_SIZE(ad7768_channels)];
/*
* DMA (thus cache coherency maintenance) requires the
* transfer buffers to live in their own cache lines.
@@ -407,6 +408,14 @@ static int ad7768_write_raw(struct iio_dev *indio_dev,
}
}
+static int ad7768_read_label(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan, char *label)
+{
+ struct ad7768_state *st = iio_priv(indio_dev);
+
+ return sprintf(label, "%s\n", st->labels[chan->channel]);
+}
+
static struct attribute *ad7768_attributes[] = {
&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
NULL
@@ -420,6 +429,7 @@ static const struct iio_info ad7768_info = {
.attrs = &ad7768_group,
.read_raw = &ad7768_read_raw,
.write_raw = &ad7768_write_raw,
+ .read_label = ad7768_read_label,
.debugfs_reg_access = &ad7768_reg_access,
};
@@ -532,6 +542,33 @@ static void ad7768_clk_disable(void *data)
clk_disable_unprepare(st->mclk);
}
+static int ad7768_set_channel_label(struct iio_dev *indio_dev,
+ int num_channels)
+{
+ struct ad7768_state *st = iio_priv(indio_dev);
+ struct device *device = indio_dev->dev.parent;
+ struct fwnode_handle *fwnode;
+ struct fwnode_handle *child;
+ const char *label;
+ int crt_ch = 0;
+
+ fwnode = dev_fwnode(device);
+ fwnode_for_each_child_node(fwnode, child) {
+ if (fwnode_property_read_u32(child, "reg", &crt_ch))
+ continue;
+
+ if (crt_ch >= num_channels)
+ continue;
+
+ if (fwnode_property_read_string(child, "label", &label))
+ continue;
+
+ st->labels[crt_ch] = label;
+ }
+
+ return 0;
+}
+
static int ad7768_probe(struct spi_device *spi)
{
struct ad7768_state *st;
@@ -604,6 +641,10 @@ static int ad7768_probe(struct spi_device *spi)
init_completion(&st->completion);
+ ret = ad7768_set_channel_label(indio_dev, ARRAY_SIZE(ad7768_channels));
+ if (ret)
+ return ret;
+
ret = devm_request_irq(&spi->dev, spi->irq,
&ad7768_interrupt,
IRQF_TRIGGER_RISING | IRQF_ONESHOT,
--
2.17.1
Optional attribute for better identification of the channels.
Signed-off-by: Cristian Pop <[email protected]>
---
Changes in v7:
- Add "additionalProperties: false" for channel child nodes.
- Fix "reg" spelling.
.../bindings/iio/adc/adi,ad7768-1.yaml | 32 +++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
index d3733ad8785a..6be43bf5c1e0 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
@@ -29,6 +29,12 @@ properties:
interrupts:
maxItems: 1
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
vref-supply:
description:
ADC reference voltage supply
@@ -61,6 +67,24 @@ required:
- spi-cpha
- adi,sync-in-gpios
+patternProperties:
+ "^channel@([0-9]|1[0-5])$":
+ type: object
+ description: |
+ Represents the external channels which are connected to the device.
+
+ properties:
+ reg:
+ description: |
+ The channel number.
+
+ label:
+ description: |
+ Unique name to identify which channel this is.
+ required:
+ - reg
+ additionalProperties: false
+
additionalProperties: false
examples:
@@ -84,6 +108,14 @@ examples:
reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
clocks = <&ad7768_mclk>;
clock-names = "mclk";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@0 {
+ reg = <0>;
+ label = "channel_0";
+ };
};
};
...
--
2.17.1
Optional attribute for better identification of the channels.
Signed-off-by: Cristian Pop <[email protected]>
---
Changes in v7:
- Add this extra commit
Documentation/devicetree/bindings/iio/adc/adc.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/adc.txt b/Documentation/devicetree/bindings/iio/adc/adc.txt
index 5bbaa330a250..4b37575bbddc 100644
--- a/Documentation/devicetree/bindings/iio/adc/adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/adc.txt
@@ -5,18 +5,24 @@ Optional properties for child nodes:
- diff-channels : Differential channels muxed for this ADC. The first value
specifies the positive input pin, the second value the negative
input pin.
+- reg: The channel number.
+- label: Unique name to identify which channel this is.
Example:
adc@0 {
compatible = "some,adc";
...
channel@0 {
+ reg = <0>;
+ label = "channel_0_name";
bipolar;
diff-channels = <0 1>;
...
};
channel@1 {
+ reg = <1>;
+ label = "channel_1_name";
diff-channels = <2 3>;
...
};
--
2.17.1
If a label is defined for this channel add that to the channel specific
attributes. This is useful for userspace to be able to identify an
individual channel.
Signed-off-by: Cristian Pop <[email protected]>
---
Changes in v7:
- Specify affected only channels which we are implementing in this patch
set.
Documentation/ABI/testing/sysfs-bus-iio | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index a9d51810a3ba..4a72ee43dee8 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1715,6 +1715,16 @@ Description:
Raw counter device counters direction for channel Y.
+What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_label
+What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_label
+KernelVersion: 5.8
+Contact: [email protected]
+Description:
+ Optional symbolic label to a device channel.
+ If a label is defined for this channel add that to the channel
+ specific attributes. This is useful for userspace to be able to
+ better identify an individual channel.
+
What: /sys/bus/iio/devices/iio:deviceX/in_phaseY_raw
KernelVersion: 4.18
Contact: [email protected]
--
2.17.1
On Mon, 28 Sep 2020 12:09:59 +0300
Cristian Pop <[email protected]> wrote:
> Optional attribute for better identification of the channels.
>
> Signed-off-by: Cristian Pop <[email protected]>
This one is on my list of binding files to convert fairly soon
so I'm fine with this going in whilst it's still in txt form.
The addition of reg here shows up a quirk of this binding. We
originally introduced it to specify differential pairs, hence
the @ can refer to the first value of diff-channels instead
of reg. Hopefully I'll remember to clarify that when doing the
yaml conversion.
The series looks good to me, but given timing it won't make the
merge window for the coming cycle.
+CC Rob and the dt list.
> ---
> Changes in v7:
> - Add this extra commit
> Documentation/devicetree/bindings/iio/adc/adc.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adc.txt b/Documentation/devicetree/bindings/iio/adc/adc.txt
> index 5bbaa330a250..4b37575bbddc 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/adc.txt
> @@ -5,18 +5,24 @@ Optional properties for child nodes:
> - diff-channels : Differential channels muxed for this ADC. The first value
> specifies the positive input pin, the second value the negative
> input pin.
> +- reg: The channel number.
> +- label: Unique name to identify which channel this is.
>
> Example:
> adc@0 {
> compatible = "some,adc";
> ...
> channel@0 {
> + reg = <0>;
> + label = "channel_0_name";
> bipolar;
> diff-channels = <0 1>;
> ...
> };
>
> channel@1 {
> + reg = <1>;
> + label = "channel_1_name";
> diff-channels = <2 3>;
> ...
> };
On Mon, 28 Sep 2020 12:09:58 +0300
Cristian Pop <[email protected]> wrote:
> Optional attribute for better identification of the channels.
>
> Signed-off-by: Cristian Pop <[email protected]>
Dt-bindings should be cc'd to Rob + the dt list.
+CC
> ---
> Changes in v7:
> - Add "additionalProperties: false" for channel child nodes.
> - Fix "reg" spelling.
> .../bindings/iio/adc/adi,ad7768-1.yaml | 32 +++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> index d3733ad8785a..6be43bf5c1e0 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> @@ -29,6 +29,12 @@ properties:
> interrupts:
> maxItems: 1
>
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> vref-supply:
> description:
> ADC reference voltage supply
> @@ -61,6 +67,24 @@ required:
> - spi-cpha
> - adi,sync-in-gpios
>
> +patternProperties:
> + "^channel@([0-9]|1[0-5])$":
> + type: object
> + description: |
> + Represents the external channels which are connected to the device.
> +
> + properties:
> + reg:
> + description: |
> + The channel number.
> +
> + label:
> + description: |
> + Unique name to identify which channel this is.
> + required:
> + - reg
> + additionalProperties: false
> +
> additionalProperties: false
>
> examples:
> @@ -84,6 +108,14 @@ examples:
> reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
> clocks = <&ad7768_mclk>;
> clock-names = "mclk";
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + channel@0 {
> + reg = <0>;
> + label = "channel_0";
> + };
> };
> };
> ...
On Mon, 28 Sep 2020 12:09:55 +0300
Cristian Pop <[email protected]> wrote:
> If a label is defined in the device tree for this channel add that
> to the channel specific attributes. This is useful for userspace to
> be able to identify an individual channel.
>
> Signed-off-by: Cristian Pop <[email protected]>
Hi Cristian,
I think the DT binding part of this has gotten lost now, so please
repost the whole series making sure to include Rob H and DT list.
I have proposed a conversion of the adc.txt file now. I'll deal
with merging your changes into there if necessary though.
May be worth mentioning that in the cover letter for the resend.
Thanks,
Jonathan
> ---
> drivers/iio/industrialio-core.c | 40 +++++++++++++++++++++++++++++++++
> include/linux/iio/iio.h | 6 +++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 6e388293c828..b577fff35641 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -669,6 +669,19 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
> }
> EXPORT_SYMBOL_GPL(iio_format_value);
>
> +static ssize_t iio_read_channel_label(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> +
> + if (!indio_dev->info->read_label)
> + return -EINVAL;
> +
> + return indio_dev->info->read_label(indio_dev, this_attr->c, buf);
> +}
> +
> static ssize_t iio_read_channel_info(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> @@ -1137,6 +1150,28 @@ int __iio_add_chan_devattr(const char *postfix,
> return ret;
> }
>
> +static int iio_device_add_channel_label(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan)
> +{
> + int ret;
> +
> + if (!indio_dev->info->read_label)
> + return 0;
> +
> + ret = __iio_add_chan_devattr("label",
> + chan,
> + &iio_read_channel_label,
> + NULL,
> + 0,
> + IIO_SEPARATE,
> + &indio_dev->dev,
> + &indio_dev->channel_attr_list);
> + if (ret < 0)
> + return ret;
> +
> + return 1;
> +}
> +
> static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> enum iio_shared_by shared_by,
> @@ -1270,6 +1305,11 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
> return ret;
> attrcount += ret;
>
> + ret = iio_device_add_channel_label(indio_dev, chan);
> + if (ret < 0)
> + return ret;
> + attrcount += ret;
> +
> if (chan->ext_info) {
> unsigned int i = 0;
> for (ext_info = chan->ext_info; ext_info->name; ext_info++) {
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 2e45b3ceafa7..9a3cf4815148 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -362,6 +362,8 @@ struct iio_trigger; /* forward declaration */
> * and max. For lists, all possible values are enumerated.
> * @write_raw: function to write a value to the device.
> * Parameters are the same as for read_raw.
> + * @read_label: function to request label name for a specified label,
> + * for better channel identification.
> * @write_raw_get_fmt: callback function to query the expected
> * format/precision. If not set by the driver, write_raw
> * returns IIO_VAL_INT_PLUS_MICRO.
> @@ -420,6 +422,10 @@ struct iio_info {
> int val2,
> long mask);
>
> + int (*read_label)(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + char *label);
> +
> int (*write_raw_get_fmt)(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> long mask);
On Sun, 1 Nov 2020 15:38:03 +0000
Jonathan Cameron <[email protected]> wrote:
> On Mon, 28 Sep 2020 12:09:55 +0300
> Cristian Pop <[email protected]> wrote:
>
> > If a label is defined in the device tree for this channel add that
> > to the channel specific attributes. This is useful for userspace to
> > be able to identify an individual channel.
> >
> > Signed-off-by: Cristian Pop <[email protected]>
>
> Hi Cristian,
>
> I think the DT binding part of this has gotten lost now, so please
> repost the whole series making sure to include Rob H and DT list.
>
> I have proposed a conversion of the adc.txt file now. I'll deal
> with merging your changes into there if necessary though.
> May be worth mentioning that in the cover letter for the resend.
Rob found it and acked, so all good on that front.
It did manage to cross with some of the work to take large parts
of iio_dev opaque so I've had to modify it a tiny bit.
Please check the result in the testing branch of iio.git.
Thanks,
Jonathan
>
> Thanks,
>
> Jonathan
>
> > ---
> > drivers/iio/industrialio-core.c | 40 +++++++++++++++++++++++++++++++++
> > include/linux/iio/iio.h | 6 +++++
> > 2 files changed, 46 insertions(+)
> >
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > index 6e388293c828..b577fff35641 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -669,6 +669,19 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
> > }
> > EXPORT_SYMBOL_GPL(iio_format_value);
> >
> > +static ssize_t iio_read_channel_label(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> > +
> > + if (!indio_dev->info->read_label)
> > + return -EINVAL;
> > +
> > + return indio_dev->info->read_label(indio_dev, this_attr->c, buf);
> > +}
> > +
> > static ssize_t iio_read_channel_info(struct device *dev,
> > struct device_attribute *attr,
> > char *buf)
> > @@ -1137,6 +1150,28 @@ int __iio_add_chan_devattr(const char *postfix,
> > return ret;
> > }
> >
> > +static int iio_device_add_channel_label(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan)
> > +{
> > + int ret;
> > +
> > + if (!indio_dev->info->read_label)
> > + return 0;
> > +
> > + ret = __iio_add_chan_devattr("label",
> > + chan,
> > + &iio_read_channel_label,
> > + NULL,
> > + 0,
> > + IIO_SEPARATE,
> > + &indio_dev->dev,
> > + &indio_dev->channel_attr_list);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return 1;
> > +}
> > +
> > static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
> > struct iio_chan_spec const *chan,
> > enum iio_shared_by shared_by,
> > @@ -1270,6 +1305,11 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
> > return ret;
> > attrcount += ret;
> >
> > + ret = iio_device_add_channel_label(indio_dev, chan);
> > + if (ret < 0)
> > + return ret;
> > + attrcount += ret;
> > +
> > if (chan->ext_info) {
> > unsigned int i = 0;
> > for (ext_info = chan->ext_info; ext_info->name; ext_info++) {
> > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> > index 2e45b3ceafa7..9a3cf4815148 100644
> > --- a/include/linux/iio/iio.h
> > +++ b/include/linux/iio/iio.h
> > @@ -362,6 +362,8 @@ struct iio_trigger; /* forward declaration */
> > * and max. For lists, all possible values are enumerated.
> > * @write_raw: function to write a value to the device.
> > * Parameters are the same as for read_raw.
> > + * @read_label: function to request label name for a specified label,
> > + * for better channel identification.
> > * @write_raw_get_fmt: callback function to query the expected
> > * format/precision. If not set by the driver, write_raw
> > * returns IIO_VAL_INT_PLUS_MICRO.
> > @@ -420,6 +422,10 @@ struct iio_info {
> > int val2,
> > long mask);
> >
> > + int (*read_label)(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + char *label);
> > +
> > int (*write_raw_get_fmt)(struct iio_dev *indio_dev,
> > struct iio_chan_spec const *chan,
> > long mask);
>