lkml.org 
[lkml]   [2022]   [Feb]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [RFC PATCH] iio: core: provide a default value `label` property
From
On 2/22/22 18:36, Jonathan Cameron wrote:
> On Tue, 22 Feb 2022 09:42:12 +0200
> Nandor Han <nandor.han@vaisala.com> wrote:
>
>> On 2/20/22 15:18, Jonathan Cameron wrote:
>>> On Wed, 16 Feb 2022 15:56:04 +0200
>>> Nandor Han <nandor.han@vaisala.com> wrote:
>>>
>>
>> Thanks for reviewing the patch and provide feed back.
>>
>>>> The label property is used to correctly identify the same IIO device
>>>> over reboots. The implementation requires that a value will be provided
>>>> through device-tree. This sometime could requires many changes to
>>>> device-trees when multiple devices want to use the label property.
>>>> In order to prevent this, we could use the device-tree node
>>>> name as default value. The device-tree node name is unique and
>>>> also reflects the device which makes it a good choice as default value.
>>>> This change is backward compatible since doesn't affect the users that
>>>> do configure a label using the device-tree or the ones that are not
>>>> using the labels at all.
>>>>
>>>> Use the device-tree node name as a default value for `label` property,
>>>> in case there isn't one configured through device-tree.
>>>
>>> Interesting idea. However a few concerns come to mind.
>>> 1) If we start having a default for this, then it will get used as ABI
>>> and if a label is applied later to the DT then we will end up breaking
>>> userspace scripts.
>>
>> When a label is explicitly configured means that the userspace expects
>> to have that value available. Therefore, I don't see this as ABI change,
>> given that this affects the property label content and not for example
>> the property name.
>
> The potential issue is that with this userspace code may rely on the common
> option (matches device tree node name) and then get confused on it changing.
>
> If it wasn't there previously and appears (which is what happens when
> a label is added currently) userspace is unlikely to have in some fashion
> depended on it not being there...
>

If they rely on missing the label, it's true this will break that.

> If someone modifies an existing label they can reasonably expect to break
> compatibility because they made something 'go away'.
>
>>
>>> 2) If we do this it should be firmware agnostics (we need to fix
>>> the existing code to be such as well).
>>
>> Not sure I understand this. If you could explain a bit more I would
>> really appriciate.
>
> Typo in that didn't help. (agnostic). Anyhow, basically it has to work
> for ACPI as well.
>
>>
>>> 3) Is the node name always unique (think multiple accelerometers on
>>> different i2c masters)?
>>> 3) I'm fairly sure this information is readily available anyway.
>>> either via the of_node link for the iio\:deviceX
>>> So why not have your usespace use that instead of label?
>>> I'm not a fan of duplicating information that is readily available
>>> anyway - be it as name and reg in the of_node directory.
>>>
>>
>> The node name supposed to be unique AFAIK and you're right it is
>> available already in the userspace.
>
> It's not unique. As per https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Felinux.org%2FDevice_Tree_Usage&amp;data=04%7C01%7Cnandor.han%40vaisala.com%7Cdbfd5d178b484f5258fe08d9f6216e65%7C6d7393e041f54c2e9b124c2be5da5c57%7C0%7C0%7C637811445712047047%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=0y2Vx5Cqso5iYAvA58tXnfzI%2BJ9oUbhvDIIkC8XP6Ik%3D&amp;reserved=0,
> "sibling nodes are expected to be unique". If you have multiple
> i2c masters and the same device under each of them with the
> same i2c address they are not siblings (different parents) and
> will have the same node name.
>

Thanks for the info. Keep that in mind

>
>> My point with this patch is to provide a default value for the label
>> content and I'm open to suggestions related to content. The of_node name
>> was something that I thought that is unique and easy to use, but if
>> somebody has better suggestions I'm really open to these.
>
> I don't see why we want a default label. If it's not provided it's
> not there (no file) and userspace can go use something else for
> it's unique naming. Note that for older kernels they need to do
> that anyway because label never existed. So userspace will need
> to work with possibility of it being absent. As userspace is
> going to do that today, why add another option so we have:
>
> 1) No label attribute.
> 2) Label attribute == node name
> 3) Label attribute something else
>
> vs having to handle 2 cases.
>
> 1) No label attribute
> 2) Label attribute present.
>
> So adding a default makes userspace code more complex, not less.
>

Thanks for the explanation and review.

Given the above comments I guess we will drop this idea.



Thanks and Regards,
Nandor

\
 
 \ /
  Last update: 2022-02-23 10:27    [W:0.059 / U:0.364 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site