devicetree: clearly define constraints on identifier/property name conflicts

[pabigot]

I would like to propose a rule for the devicetree scripting, to the effect that the generated identifiers should always end with the unmodified property or node name. This rule would excludes the use of suffixes like _INIT or _UNQUOTED for special cases.

It also means we need to deprecate the original generation of individual elements of array-valued properties with a suffix ordinal, e.g. DT_STUFF_ARRAY_0 and DT_STUFF_ARRAY_1. Now that the tooling generates an initializer list as the value of DT_STUFF_ARRAY these sub-property values are no longer necessary. By inspection of samples from the slack channel it seems having properties bar of type array and bar-1 of type string would create conflicting symbols.

I'm aware this deeply complicates solution to #21273. A solution might be supported by clearly defining a namespace within a prefix, e.g. DT_ZEPHYR_something_COMPAT_PROPERTY, or switching to a more powerful way of providing devicetree data to the code that needs it. We have partial solutions like assuming DT_INST_# will not conflict, but that's not robust.

Something to discuss.

[pabigot]

I've created this as a bug because the existing path seems to prevent binding authors from using certain property names because the tooling generates symbols that conflict with them. Fixing that doesn't seem like an enhancement.

[ulfalizer]

I think it would be best to step to step remove stuff that isn't used/needed to start with. It's hard to even get an overview now, and that makes it hard to redesign things.

@galak @pabigot
If you know of any identifiers that aren't used (didn't know the _<index> ones for arrays weren't), then please submit PRs to get rid of them. Getting rid of some of the inconsistency for when indices are generated would be nice too, if there'd be any of those left.

Once gen_defines.py is a bit cleaner and easier to read, it'll be easier to see how to improve it.

[ulfalizer]

The stuff I'm working on now will allow initializers to be generated for entire nodes by the way. That might clean stuff up some more eventually.

[pabigot]

I want this discussed as a policy before trying to make changes. There probably are uses of the _<index> identifiers, because until the initializer list support was added there wasn't anything else.

I'm aware of new work extending initializer support and concern about the preview I saw on slack is what motivated this issue.

[ulfalizer]

Do you prefer DT_..._INIT_<node> over DT_..._<node>_INIT? Or are you talking about not adding any tags at all? The problem with that at the moment is that it clashes with the node existence flags.

Another option would be to add a tag to the existence flags, but I didn't feel like going through all of the code to fix it while working on this. I think that might be nicer at least, if we don't want tags for at least some of the cases.

[ulfalizer]

Gotta have some way to differentiate things if we're gonna generate both "node X exists" and "initializer for node X" identifiers at least.

Maybe the second one could double for the first in some nicer-designed universe...

[pabigot]

I proposed that the name end with the devicetree path, to node or property. Anything else risks conflict with other paths. So distinguishing roles for multiple attributes of a node or property has to be done somewhere else, perhaps as a prefix as I suggested.

ATM I'm not convinced we need initializers for nodes. What's the use case, how do you deal with order of components, and with changing elements as properties are added? We need to be careful and take into account devicetree use cases we haven't been able to support yet.

The interpretation of the base identifier for a node should remain the presence flag, while for a property it should remain the value (expressed as an initializer for multi-element values like arrays). For phandle-arrays where cell names are provided those might reasonably be used as suffixes, as they are already.

[ulfalizer]

I proposed that the name end with the devicetree path, to node or property. Anything else risks conflict with other paths. So distinguishing roles for multiple attributes of a node or property has to be done somewhere else, perhaps as a prefix as I suggested.

Can't it clash if you have a prefix as well though? The macro stuff is brittle in that way in general.

_INIT just helps disambiguate it. Doesn't matter if it's a prefix or a suffix.

Haven't run into any issues that were caused by clashes between paths/names and "tag" parts of identifiers, so maybe it's not a super-urgent issue.

ATM I'm not convinced we need initializers for nodes. What's the use case, how do you deal with order of components, and with changing elements as properties are added? We need to be careful and take into account devicetree use cases we haven't been able to support yet.

It's for #21239. Feels like it might simplify some stuff there at least, and might make new ways of defining things in devicetree possible.

The order matches the order in devicetree. It's up to you to make sure it matches whatever struct you're initializing.

[ulfalizer]

If name/tag clashes start becoming a problem, I'd add a separator like __.

I agree that it's ugly that it can clash. Just wonder how urgent it is.

And yeah, nice if it's consistent.

[pabigot]

@carlescufi Related to this I think we need to start taking the devicetree infrastructure under management similar to other API with respect to changes to interface and behavior.

Binding descriptions specify what properties are present, which affects devicetree source and drivers. Adding a new property can require modifications to the test drivers/buildall/dts_fixup that are often forgotten. #21393 proposes a change that introduces a strong dependence between the order of properties in the binding and the code that uses the binding. The impact of things like this must be widely understood and accepted before they're adopted.

Devicetree sources are very likeout to be out-of-tree, so adding required: true to any property is a breaking change. Doesn't mean we can't or shouldn't do it, but it needs to be publicized and alternative approaches considered. It also means that the bindings should be carefully designed so that the names and constraints on properties do not need to change as a particular device evolves.

And specific to this issue, changes to the way the tooling presents devicetree property values to the application and driver code may require changes to drivers to adopt the new approach so old solutions can be removed, or may inadvertently prevent use of "compatible" devicetree source from non-Zephyr projects because the property names in the outside bindings conflict when processed by Zephyr's tooling.

We need to move toward a more managed evolution of the entire Zephyr devicetree infrastructure so that we pick solutions that can meet needs we are only starting to understand, without having to come up with new solutions every time a problem reappears from a new perspective (cf #19285).

[carlescufi]

@carlescufi Related to this I think we need to start taking the devicetree infrastructure under management similar to other API with respect to changes to interface and behavior.

I agree with this. I want to discuss this with @galak as part of the API meeting series. I will add this issue to the project and remind myself to add it for discussion.

[carlescufi]

@galak @pabigot @ulfalizer After the meeting yesterday, could we come to a conclusion on this particular item? It is blocking other DT-related progress that is regarded as important:

#21393
#21578

[carlescufi]

@jhedberg can we discuss the priority of this issue for 2.2? This has become a blocker for additional work and I'd like to propose raising this to medium

[jhedberg]

@jhedberg can we discuss the priority of this issue for 2.2? This has become a blocker for additional work and I'd like to propose raising this to medium

Updated to medium now

[jhedberg]

Back to low.. hasn't been updated for awhile (discussed in release meeting, @carlescufi approves)