* [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1015: Add label property
@ 2026-09-02 17:22 Flaviu Nistor
2026-09-02 17:22 ` [PATCH v2 2/2] iio: adc: ti-ads1015: Add support for label Flaviu Nistor
2026-09-02 18:31 ` [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1015: Add label property David Lechner
0 siblings, 2 replies; 11+ messages in thread
From: Flaviu Nistor @ 2026-09-02 17:22 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sa, Andy Shevchenko,
Krzysztof Kozlowski, Rob Herring, Conor Dooley, Daniel Baluta,
Antoniu Miclaus
Cc: Flaviu Nistor, devicetree, linux-kernel, linux-iio
Add support for an optional label property per channel similar to other adc
chips. This allows assigning distinct names for each channel which can
match the schematic signal name.
Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
---
Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
index 718f633c6e04..777a75c21d7f 100644
--- a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
@@ -66,6 +66,11 @@ patternProperties:
- minimum: 0
maximum: 7
+ label:
+ $ref: /schemas/types.yaml#/definitions/string
+ description:
+ A descriptive name for this channel, like "vcc_ram" or "CH3".
+
ti,gain:
$ref: /schemas/types.yaml#/definitions/uint32
minimum: 0
@@ -125,6 +130,7 @@ examples:
reg = <4>;
ti,gain = <3>;
ti,datarate = <5>;
+ label = "CH3";
};
};
};
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] iio: adc: ti-ads1015: Add support for label
2026-09-02 17:22 [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1015: Add label property Flaviu Nistor
@ 2026-09-02 17:22 ` Flaviu Nistor
2026-09-02 17:40 ` sashiko-bot
2026-09-02 18:37 ` David Lechner
2026-09-02 18:31 ` [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1015: Add label property David Lechner
1 sibling, 2 replies; 11+ messages in thread
From: Flaviu Nistor @ 2026-09-02 17:22 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sa, Andy Shevchenko,
Krzysztof Kozlowski, Rob Herring, Conor Dooley, Daniel Baluta,
Antoniu Miclaus
Cc: Flaviu Nistor, devicetree, linux-kernel, linux-iio
Add support for label sysfs attribute similar to other adc devices. This is
particularly useful because the label can match the schematic signal name,
identifying channel voltages easier if label is defined via device tree.
Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
---
Changes in v2:
- Implement changes suggested by Andy Shevchenko.
- Link to v1: https://lore.kernel.org/all/20260901184302.8127-2-flaviu.nistor@gmail.com/
drivers/iio/adc/ti-ads1015.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 0fbfa4e499aa..8be1f0c32ca9 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -233,6 +233,7 @@ static const struct iio_event_spec ads1015_events[] = {
struct ads1015_channel_data {
unsigned int pga;
unsigned int data_rate;
+ const char *label;
};
struct ads1015_thresh_data {
@@ -586,6 +587,15 @@ static int ads1015_read_raw(struct iio_dev *indio_dev,
}
}
+static int ads1015_read_label(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, char *label)
+{
+ struct ads1015_data *data = iio_priv(indio_dev);
+ const char *name = data->channel_data[chan->address].label;
+
+ return sysfs_emit(label, "%s\n", name);
+}
+
static int ads1015_write_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int val,
int val2, long mask)
@@ -843,6 +853,7 @@ static const struct iio_buffer_setup_ops ads1015_buffer_setup_ops = {
static const struct iio_info ads1015_info = {
.read_avail = ads1015_read_avail,
.read_raw = ads1015_read_raw,
+ .read_label = ads1015_read_label,
.write_raw = ads1015_write_raw,
.read_event_value = ads1015_read_event,
.write_event_value = ads1015_write_event,
@@ -853,6 +864,7 @@ static const struct iio_info ads1015_info = {
static const struct iio_info tla2024_info = {
.read_avail = ads1015_read_avail,
.read_raw = ads1015_read_raw,
+ .read_label = ads1015_read_label,
.write_raw = ads1015_write_raw,
};
@@ -861,7 +873,9 @@ static int ads1015_client_get_channels_config(struct i2c_client *client)
struct iio_dev *indio_dev = i2c_get_clientdata(client);
struct ads1015_data *data = iio_priv(indio_dev);
struct device *dev = &client->dev;
+ const char *label;
int i = -1;
+ int ret;
device_for_each_child_node_scoped(dev, node) {
u32 pval;
@@ -897,6 +911,17 @@ static int ads1015_client_get_channels_config(struct i2c_client *client)
}
}
+ if (fwnode_property_present(node, "label")) {
+ ret = fwnode_property_read_string(node, "label", &label);
+ if (ret) {
+ dev_err(dev, "invalid label on %pfw\n", node);
+ return ret;
+ }
+ data->channel_data[channel].label = label;
+ } else {
+ data->channel_data[channel].label = "";
+ }
+
data->channel_data[channel].pga = pga;
data->channel_data[channel].data_rate = data_rate;
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] iio: adc: ti-ads1015: Add support for label
2026-09-02 17:22 ` [PATCH v2 2/2] iio: adc: ti-ads1015: Add support for label Flaviu Nistor
@ 2026-09-02 17:40 ` sashiko-bot
2026-09-02 18:37 ` David Lechner
1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-09-02 17:40 UTC (permalink / raw)
To: Flaviu Nistor; +Cc: robh, devicetree, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `ads1015_read_label()` function passes a potentially NULL pointer to `sysfs_emit()`, causing userspace to read `"(null)\n"` for unconfigured channels.
--
commit 9084b6f4b1c391e596d3456f4a84598bb430a6e6
Author: Flaviu Nistor <flaviu.nistor@gmail.com>
Subject: iio: adc: ti-ads1015: Add support for label
Add support for label sysfs attribute similar to other adc devices. This is
particularly useful because the label can match the schematic signal name,
identifying channel voltages easier if label is defined via device tree.
Link: https://lore.kernel.org/all/20260901184302.8127-2-flaviu.nistor@gmail.com/
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index 0fbfa4e499aa8..8be1f0c32ca9d 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
[ ... ]
> @@ -586,6 +587,15 @@ static int ads1015_read_raw(struct iio_dev *indio_dev,
> }
> }
>
> +static int ads1015_read_label(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, char *label)
> +{
> + struct ads1015_data *data = iio_priv(indio_dev);
> + const char *name = data->channel_data[chan->address].label;
> +
> + return sysfs_emit(label, "%s\n", name);
[Severity: Medium]
Will this return "(null)\n" to userspace for channels without a configured
label?
If a channel is not explicitly defined in the firmware nodes, or if the driver
falls back to the default configuration via ads1015_get_channels_config(),
the label field will remain NULL. Passing a NULL pointer to sysfs_emit()
results in an incorrect literal "(null)\n" output at the UAPI boundary.
> +}
> +
> static int ads1015_write_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan, int val,
> int val2, long mask)
[ ... ]
> @@ -897,6 +911,17 @@ static int ads1015_client_get_channels_config(struct i2c_client *client)
> }
> }
>
> + if (fwnode_property_present(node, "label")) {
> + ret = fwnode_property_read_string(node, "label", &label);
> + if (ret) {
> + dev_err(dev, "invalid label on %pfw\n", node);
> + return ret;
> + }
> + data->channel_data[channel].label = label;
> + } else {
> + data->channel_data[channel].label = "";
> + }
> +
[Severity: Medium]
Are there cases where some channels might be skipped in this loop?
Since this initialization is contained within the child node iteration loop,
it only covers channels that are explicitly defined in the firmware nodes.
Channels omitted from the device tree will remain zero-initialized, leaving
their label field as NULL.
> data->channel_data[channel].pga = pga;
> data->channel_data[channel].data_rate = data_rate;
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902172230.5234-1-flaviu.nistor@gmail.com?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1015: Add label property
2026-09-02 17:22 [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1015: Add label property Flaviu Nistor
2026-09-02 17:22 ` [PATCH v2 2/2] iio: adc: ti-ads1015: Add support for label Flaviu Nistor
@ 2026-09-02 18:31 ` David Lechner
2026-09-03 15:19 ` Flaviu Nistor
1 sibling, 1 reply; 11+ messages in thread
From: David Lechner @ 2026-09-02 18:31 UTC (permalink / raw)
To: Flaviu Nistor, Jonathan Cameron, Nuno Sa, Andy Shevchenko,
Krzysztof Kozlowski, Rob Herring, Conor Dooley, Daniel Baluta,
Antoniu Miclaus
Cc: devicetree, linux-kernel, linux-iio
On 9/2/26 12:22 PM, Flaviu Nistor wrote:
> Add support for an optional label property per channel similar to other adc
> chips. This allows assigning distinct names for each channel which can
> match the schematic signal name.
>
> Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
> ---
> Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
> index 718f633c6e04..777a75c21d7f 100644
> --- a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
> @@ -66,6 +66,11 @@ patternProperties:
> - minimum: 0
> maximum: 7
>
> + label:
> + $ref: /schemas/types.yaml#/definitions/string
> + description:
> + A descriptive name for this channel, like "vcc_ram" or "CH3".
> +
Apparently this chip has quite old dt-bindings. Normally, we would have
included adc.yaml to get this and other channel properties.
And we would have used diff-channels and single-channel from that to
describe which pins are associated with which channel rather than having
a magic number for reg. And the ti,datarate looks suspicious as sampling
frequency is usually we control at runtime.
Anyway, none of that is really relevant to this patch. We'll see what
Jonathan says about using adc.yaml here vs. adding a label property.
> ti,gain:
> $ref: /schemas/types.yaml#/definitions/uint32
> minimum: 0
> @@ -125,6 +130,7 @@ examples:
> reg = <4>;
> ti,gain = <3>;
> ti,datarate = <5>;
> + label = "CH3";
> };
> };
> };
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] iio: adc: ti-ads1015: Add support for label
2026-09-02 17:22 ` [PATCH v2 2/2] iio: adc: ti-ads1015: Add support for label Flaviu Nistor
2026-09-02 17:40 ` sashiko-bot
@ 2026-09-02 18:37 ` David Lechner
2026-09-03 6:18 ` Andy Shevchenko
1 sibling, 1 reply; 11+ messages in thread
From: David Lechner @ 2026-09-02 18:37 UTC (permalink / raw)
To: Flaviu Nistor, Jonathan Cameron, Nuno Sa, Andy Shevchenko,
Krzysztof Kozlowski, Rob Herring, Conor Dooley, Daniel Baluta,
Antoniu Miclaus
Cc: devicetree, linux-kernel, linux-iio
On 9/2/26 12:22 PM, Flaviu Nistor wrote:
> Add support for label sysfs attribute similar to other adc devices. This is
> particularly useful because the label can match the schematic signal name,
> identifying channel voltages easier if label is defined via device tree.
>
> Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
> ---
> Changes in v2:
> - Implement changes suggested by Andy Shevchenko.
> - Link to v1: https://lore.kernel.org/all/20260901184302.8127-2-flaviu.nistor@gmail.com/
>
> drivers/iio/adc/ti-ads1015.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index 0fbfa4e499aa..8be1f0c32ca9 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -233,6 +233,7 @@ static const struct iio_event_spec ads1015_events[] = {
> struct ads1015_channel_data {
> unsigned int pga;
> unsigned int data_rate;
> + const char *label;
> };
>
> struct ads1015_thresh_data {
> @@ -586,6 +587,15 @@ static int ads1015_read_raw(struct iio_dev *indio_dev,
> }
> }
>
> +static int ads1015_read_label(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, char *label)
> +{
> + struct ads1015_data *data = iio_priv(indio_dev);
> + const char *name = data->channel_data[chan->address].label;
> +
> + return sysfs_emit(label, "%s\n", name);
> +}
> +
> static int ads1015_write_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan, int val,
> int val2, long mask)
> @@ -843,6 +853,7 @@ static const struct iio_buffer_setup_ops ads1015_buffer_setup_ops = {
> static const struct iio_info ads1015_info = {
> .read_avail = ads1015_read_avail,
> .read_raw = ads1015_read_raw,
> + .read_label = ads1015_read_label,
> .write_raw = ads1015_write_raw,
> .read_event_value = ads1015_read_event,
> .write_event_value = ads1015_write_event,
> @@ -853,6 +864,7 @@ static const struct iio_info ads1015_info = {
> static const struct iio_info tla2024_info = {
> .read_avail = ads1015_read_avail,
> .read_raw = ads1015_read_raw,
> + .read_label = ads1015_read_label,
> .write_raw = ads1015_write_raw,
> };
>
> @@ -861,7 +873,9 @@ static int ads1015_client_get_channels_config(struct i2c_client *client)
> struct iio_dev *indio_dev = i2c_get_clientdata(client);
> struct ads1015_data *data = iio_priv(indio_dev);
> struct device *dev = &client->dev;
> + const char *label;
> int i = -1;
> + int ret;
>
> device_for_each_child_node_scoped(dev, node) {
> u32 pval;
> @@ -897,6 +911,17 @@ static int ads1015_client_get_channels_config(struct i2c_client *client)
> }
> }
>
> + if (fwnode_property_present(node, "label")) {
> + ret = fwnode_property_read_string(node, "label", &label);
> + if (ret) {
> + dev_err(dev, "invalid label on %pfw\n", node);
> + return ret;
> + }
> + data->channel_data[channel].label = label;
> + } else {
> + data->channel_data[channel].label = "";
I think the normal way to do this is to return -EINVAL on channels with no
label rather than an empty string.
> + }
> +
> data->channel_data[channel].pga = pga;
> data->channel_data[channel].data_rate = data_rate;
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] iio: adc: ti-ads1015: Add support for label
2026-09-02 18:37 ` David Lechner
@ 2026-09-03 6:18 ` Andy Shevchenko
2026-09-03 15:16 ` Flaviu Nistor
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2026-09-03 6:18 UTC (permalink / raw)
To: David Lechner
Cc: Flaviu Nistor, Jonathan Cameron, Nuno Sa, Andy Shevchenko,
Krzysztof Kozlowski, Rob Herring, Conor Dooley, Daniel Baluta,
Antoniu Miclaus, devicetree, linux-kernel, linux-iio
On Wed, Sep 02, 2026 at 01:37:45PM -0500, David Lechner wrote:
> On 9/2/26 12:22 PM, Flaviu Nistor wrote:
...
> > + if (fwnode_property_present(node, "label")) {
> > + ret = fwnode_property_read_string(node, "label", &label);
> > + if (ret) {
> > + dev_err(dev, "invalid label on %pfw\n", node);
> > + return ret;
return dev_err_probe(...);
> > + }
> > + data->channel_data[channel].label = label;
> > + } else {
> > + data->channel_data[channel].label = "";
>
> I think the normal way to do this is to return -EINVAL on channels with no
> label rather than an empty string.
In this case the whole dance with property_present is not required.
(What you are saying sounds like label is mandatory property, but
how the old DT will work that have no such property?)
> > + }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] iio: adc: ti-ads1015: Add support for label
2026-09-03 6:18 ` Andy Shevchenko
@ 2026-09-03 15:16 ` Flaviu Nistor
2026-09-03 15:28 ` David Lechner
2026-09-03 19:03 ` Andy Shevchenko
0 siblings, 2 replies; 11+ messages in thread
From: Flaviu Nistor @ 2026-09-03 15:16 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, David Lechner, Nuno Sa, Krzysztof Kozlowski,
Rob Herring, Conor Dooley, Daniel Baluta, Antoniu Miclaus,
Flaviu Nistor, devicetree, linux-kernel, linux-iio
On Thu, Sep 3, 2026 at 9:18 AM, Andy Shevchenko wrote:
>On Wed, Sep 02, 2026 at 01:37:45PM -0500, David Lechner wrote:
>> On 9/2/26 12:22 PM, Flaviu Nistor wrote:
>
>...
>
>> > + if (fwnode_property_present(node, "label")) {
>> > + ret = fwnode_property_read_string(node, "label", &label);
>> > + if (ret) {
>> > + dev_err(dev, "invalid label on %pfw\n", node);
>> > + return ret;
>
> return dev_err_probe(...);
>
>> > + }
>> > + data->channel_data[channel].label = label;
>> > + } else {
>> > + data->channel_data[channel].label = "";
>>
>> I think the normal way to do this is to return -EINVAL on channels with no
>> label rather than an empty string.
>
>
>In this case the whole dance with property_present is not required.
>(What you are saying sounds like label is mandatory property, but
> how the old DT will work that have no such property?)
>
I also consider that since label is not a mandatory property it should
not return -EINVAL just be simply not being used in the device tree for
that channel. Since the iio core exposes in sysfs the in_volatagex_label
for all channels, there are 3 situations:
1. The channel is defined in the dts and has the optional label property
assigned, in which case a string is returned in the console in user space.
2. The channel is defined in the dts but has no optional label property
defined in the dts, in which case it an empty string "" (a default) is
returned.
3. The channel is not defined in the dts (in_voltagex_label is still visible
in the sysfs), and reading the label will cause "(null)\n" to be printed in
the console (as indicated also by Sashiko boot), since the default "" value
is not assigned during device_for_each_child_node_scoped(). I think this is
not an issue since is just the way sysfs_emit() handles an NULL string.
My initial version (v1) handeled all 3 cases, for case 2 and 3, an empty string
"" was printed out.
What would you prefer? Any further suggestions?
Best regards,
Flaviu Nistor
>> > + }
>
>--
>With Best Regards,
>Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1015: Add label property
2026-09-02 18:31 ` [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1015: Add label property David Lechner
@ 2026-09-03 15:19 ` Flaviu Nistor
2026-09-03 15:30 ` David Lechner
0 siblings, 1 reply; 11+ messages in thread
From: Flaviu Nistor @ 2026-09-03 15:19 UTC (permalink / raw)
To: David Lechner
Cc: Andy Shevchenko, Jonathan Cameron, Nuno Sa, Krzysztof Kozlowski,
Rob Herring, Conor Dooley, Daniel Baluta, Antoniu Miclaus,
Flaviu Nistor, devicetree, linux-kernel, linux-iio
On Wed, Sep 2, 2026 at 9:31 PM, David Lechner wrote:
>On 9/2/26 12:22 PM, Flaviu Nistor wrote:
>> Add support for an optional label property per channel similar to other adc
>> chips. This allows assigning distinct names for each channel which can
>> match the schematic signal name.
>>
>> Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
>> ---
>> Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
>> index 718f633c6e04..777a75c21d7f 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
>> @@ -66,6 +66,11 @@ patternProperties:
>> - minimum: 0
>> maximum: 7
>>
>> + label:
>> + $ref: /schemas/types.yaml#/definitions/string
>> + description:
>> + A descriptive name for this channel, like "vcc_ram" or "CH3".
>> +
>
>Apparently this chip has quite old dt-bindings. Normally, we would have
>included adc.yaml to get this and other channel properties.
>
>And we would have used diff-channels and single-channel from that to
>describe which pins are associated with which channel rather than having
>a magic number for reg. And the ti,datarate looks suspicious as sampling
>frequency is usually we control at runtime.
>
>Anyway, none of that is really relevant to this patch. We'll see what
>Jonathan says about using adc.yaml here vs. adding a label property.
>
Thanks for your feedback. I can also use:
$ref: adc.yaml
unevaluatedProperties: false
and move additionalProperties: false at the end of patternProperties block.
Any other idea?
Best regards,
Flaviu Nistor
>> ti,gain:
>> $ref: /schemas/types.yaml#/definitions/uint32
>> minimum: 0
>> @@ -125,6 +130,7 @@ examples:
>> reg = <4>;
>> ti,gain = <3>;
>> ti,datarate = <5>;
>> + label = "CH3";
>> };
>> };
>> };
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] iio: adc: ti-ads1015: Add support for label
2026-09-03 15:16 ` Flaviu Nistor
@ 2026-09-03 15:28 ` David Lechner
2026-09-03 19:03 ` Andy Shevchenko
1 sibling, 0 replies; 11+ messages in thread
From: David Lechner @ 2026-09-03 15:28 UTC (permalink / raw)
To: Flaviu Nistor, Andy Shevchenko
Cc: Jonathan Cameron, Nuno Sa, Krzysztof Kozlowski, Rob Herring,
Conor Dooley, Daniel Baluta, Antoniu Miclaus, devicetree,
linux-kernel, linux-iio
On 9/3/26 10:16 AM, Flaviu Nistor wrote:
> On Thu, Sep 3, 2026 at 9:18 AM, Andy Shevchenko wrote:
>> On Wed, Sep 02, 2026 at 01:37:45PM -0500, David Lechner wrote:
>>> On 9/2/26 12:22 PM, Flaviu Nistor wrote:
>>
>> ...
>>
>>>> + if (fwnode_property_present(node, "label")) {
>>>> + ret = fwnode_property_read_string(node, "label", &label);
>>>> + if (ret) {
>>>> + dev_err(dev, "invalid label on %pfw\n", node);
>>>> + return ret;
>>
>> return dev_err_probe(...);
>>
>>>> + }
>>>> + data->channel_data[channel].label = label;
>>>> + } else {
>>>> + data->channel_data[channel].label = "";
>>>
>>> I think the normal way to do this is to return -EINVAL on channels with no
>>> label rather than an empty string.
>>
>>
>> In this case the whole dance with property_present is not required.
>> (What you are saying sounds like label is mandatory property, but
>> how the old DT will work that have no such property?)
>>
>
> I also consider that since label is not a mandatory property it should
> not return -EINVAL just be simply not being used in the device tree for
> that channel. Since the iio core exposes in sysfs the in_volatagex_label
> for all channels, there are 3 situations:
> 1. The channel is defined in the dts and has the optional label property
> assigned, in which case a string is returned in the console in user space.
> 2. The channel is defined in the dts but has no optional label property
> defined in the dts, in which case it an empty string "" (a default) is
> returned.
> 3. The channel is not defined in the dts (in_voltagex_label is still visible
> in the sysfs), and reading the label will cause "(null)\n" to be printed in
> the console (as indicated also by Sashiko boot), since the default "" value
> is not assigned during device_for_each_child_node_scoped(). I think this is
> not an issue since is just the way sysfs_emit() handles an NULL string.
>
> My initial version (v1) handeled all 3 cases, for case 2 and 3, an empty string
> "" was printed out.
>
> What would you prefer? Any further suggestions?
We should implement it like all of the other drivers that implement labels.
I did not see any that have an empty string, so we should not do that. The
sysfs attribute returns -EINVAL when read if there is not a label.
(I think my previous comment was ambiguous. I was not suggesting that we
return -EINVAL from probe.)
>
> Best regards,
> Flaviu Nistor
>
>>>> + }
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1015: Add label property
2026-09-03 15:19 ` Flaviu Nistor
@ 2026-09-03 15:30 ` David Lechner
0 siblings, 0 replies; 11+ messages in thread
From: David Lechner @ 2026-09-03 15:30 UTC (permalink / raw)
To: Flaviu Nistor
Cc: Andy Shevchenko, Jonathan Cameron, Nuno Sa, Krzysztof Kozlowski,
Rob Herring, Conor Dooley, Daniel Baluta, Antoniu Miclaus,
devicetree, linux-kernel, linux-iio
On 9/3/26 10:19 AM, Flaviu Nistor wrote:
> On Wed, Sep 2, 2026 at 9:31 PM, David Lechner wrote:
>> On 9/2/26 12:22 PM, Flaviu Nistor wrote:
>>> Add support for an optional label property per channel similar to other adc
>>> chips. This allows assigning distinct names for each channel which can
>>> match the schematic signal name.
>>>
>>> Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
>>> ---
>>> Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
>>> index 718f633c6e04..777a75c21d7f 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
>>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
>>> @@ -66,6 +66,11 @@ patternProperties:
>>> - minimum: 0
>>> maximum: 7
>>>
>>> + label:
>>> + $ref: /schemas/types.yaml#/definitions/string
>>> + description:
>>> + A descriptive name for this channel, like "vcc_ram" or "CH3".
>>> +
>>
>> Apparently this chip has quite old dt-bindings. Normally, we would have
>> included adc.yaml to get this and other channel properties.
>>
>> And we would have used diff-channels and single-channel from that to
>> describe which pins are associated with which channel rather than having
>> a magic number for reg. And the ti,datarate looks suspicious as sampling
>> frequency is usually we control at runtime.
>>
>> Anyway, none of that is really relevant to this patch. We'll see what
>> Jonathan says about using adc.yaml here vs. adding a label property.
>>
>
> Thanks for your feedback. I can also use:
>
> $ref: adc.yaml
> unevaluatedProperties: false
>
> and move additionalProperties: false at the end of patternProperties block.
It can't have both unevaluatedProperties and additionalProperties (hopefully
the dt_binding_check would catch that).
> Any other idea?
>
> Best regards,
> Flaviu Nistor
>
>>> ti,gain:
>>> $ref: /schemas/types.yaml#/definitions/uint32
>>> minimum: 0
>>> @@ -125,6 +130,7 @@ examples:
>>> reg = <4>;
>>> ti,gain = <3>;
>>> ti,datarate = <5>;
>>> + label = "CH3";
>>> };
>>> };
>>> };
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] iio: adc: ti-ads1015: Add support for label
2026-09-03 15:16 ` Flaviu Nistor
2026-09-03 15:28 ` David Lechner
@ 2026-09-03 19:03 ` Andy Shevchenko
1 sibling, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2026-09-03 19:03 UTC (permalink / raw)
To: Flaviu Nistor
Cc: Andy Shevchenko, Jonathan Cameron, David Lechner, Nuno Sa,
Krzysztof Kozlowski, Rob Herring, Conor Dooley, Daniel Baluta,
Antoniu Miclaus, devicetree, linux-kernel, linux-iio
On Thu, Sep 03, 2026 at 06:16:27PM +0300, Flaviu Nistor wrote:
> On Thu, Sep 3, 2026 at 9:18 AM, Andy Shevchenko wrote:
...
> Since the iio core exposes in sysfs the in_volatagex_label
> for all channels, there are 3 situations:
> 3. The channel is not defined in the dts (in_voltagex_label is still visible
> in the sysfs), and reading the label will cause "(null)\n" to be printed in
> the console (as indicated also by Sashiko boot), since the default "" value
> is not assigned during device_for_each_child_node_scoped(). I think this is
> not an issue since is just the way sysfs_emit() handles an NULL string.
Is this current state of affairs?
If so, the patch must not change that — it's an ABI now and (null) is valid
response from the user space point of view.
> What would you prefer? Any further suggestions?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-09-03 19:04 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02 17:22 [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1015: Add label property Flaviu Nistor
2026-09-02 17:22 ` [PATCH v2 2/2] iio: adc: ti-ads1015: Add support for label Flaviu Nistor
2026-09-02 17:40 ` sashiko-bot
2026-09-02 18:37 ` David Lechner
2026-09-03 6:18 ` Andy Shevchenko
2026-09-03 15:16 ` Flaviu Nistor
2026-09-03 15:28 ` David Lechner
2026-09-03 19:03 ` Andy Shevchenko
2026-09-02 18:31 ` [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1015: Add label property David Lechner
2026-09-03 15:19 ` Flaviu Nistor
2026-09-03 15:30 ` David Lechner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox