Re: [PATCH v2 2/2] iio: adc: ti-ads1015: Add support for label

[David Lechner]

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