Zephyr: devicetree: fixed non-alias reference to specific nodes

[pabigot]

This issue steps back from a solution-oriented #18986 and poses more generally the question of how to get access to devicetree node data from application and driver sources, given that code does not have the ability to follow a node path.

For background, three cases have been identified:

• Accessing a specific node requires a devicetree alias foo = &label, which produces node properties with the prefix DT_ALIAS_FOO_. This closely models standard devicetree use and is fine, but it requires that nodes be given a (node) label (not label property) that's referenced in the alias section.
• Initializing state for multiple instances within a driver, such as multiple I2C sensors with different addresses. The existing solution provides grouping through a prefix DT_INST_#_COMPATIBLE_ where # is a zero-based ordinal. The association between a given ordinal and a specific instance is not fixed, because new instances may be added through devicetree overlays. Here the solution is fine because user code will access specific devices through the instance identifier (label property value) and the driver doesn't care which one is which.
• Accessing values of properties associated with a specific node with a short-hand name that has a a fixed relation to the node (DT_INST_#_ does not work) and use of an alias (DT_ALIAS_) is annoying. Examples are accessing node properties using the name provided in the hardware description, such as vendor SOC documentation or board schematics.

This issue addresses the last. The most recent motivating use case was in #18937 where many aliases were added solely to have a fixed name for specific instances. An earlier proposal was #12546 which was closed at a time where the previous spelling of DT_ALIAS_ was judged adequate.

Current proposed solutions circle around a string property that provides the name. soc-label has been proposed, as has zephyr,id. Let's pick a property name, decide once and for all that aliases are the right approach, or pick from other unknown solution approaches.

[pabigot]

In #20115 I discovered that the IMX GPIO driver enumerates ports from 1 to 7, corresponding to the datasheet names. I also discovered that udoo_neo_full_m4 only uses two (I think 4 and 5), which broke the assumption I had that DT_INST_n_SOMETHING_FOO corresponded to DT_GPIO_IMX_PORT_(n+1)_FOO.

I'm making this high because we need a solution for this that does not involve massive soc/series-specific dts_fixup.h files that have to be adjusted every time somebody adds a new property to the binding.

[erwango]

About DT_INST_ macros, there are 2 main concerns on my side:

• Need to have DT_ macros that follow closely SoC nomenclature, since IP enumeration do not always follow a continuous scheme [1]
• Use of build dependent instance <> IP relationship does not work, as for some IPs, driver needs to know the exact ID of the IP. This could not be achieved if instance depend of the number of the IP depends on its number in the list of enabled IPs at each build [2]

[1] Similarly to following observation:

In #20115 I discovered that the IMX GPIO driver enumerates ports from 1 to 7, corresponding to the datasheet names. I also discovered that udoo_neo_full_m4 only uses two (I think 4 and 5), which broke the assumption I had that DT_INST_n_SOMETHING_FOO corresponded to DT_GPIO_IMX_PORT_(n+1)_FOO.

On STM32 SoCs, this case is frequent as well. For instance:

• TIM1, 5 6 on STM32F410
• USART1,2 6 on STM32F411

[2] Also in some particular cases, we need instance specific code in drivers, due to specialties of one instance (cf https://github.com/zephyrproject-rtos/zephyr/blob/4f1626851b8296a86edb67442d56f4ed5f17dc47/drivers/can/can_stm32.c#L432). and we should be able to have a IP related macro that would be fixed whether other IPs are enabled or not.

[erwango]

Side note on the 'non-alias' nature of the solution:

I think aliases are highly valuable in case of providing a hook between hardware abstracted application and a particular target. For instance the use of led0 or sw0, that provides a handy application<>hardware matching. We can even find more value in these, such as the ability to know of specific if a board support such or such feature in a cross-vendor scheme.
Using alias in the only goal of providing a HW specific reference to a single node would be wasting this potential.

[pabigot]

I agree with @erwango about the value of aliases for cross-target naming of common capability like LEDs and buttons. But for that to be robust we might need to control the namespace, so people know that uart2 refers to a secondary UART, and not simply the second hardware instance which might not even be present on a specific SOC. The existing (undocumented?) distinction between uart2 (label) and uart-2 (alias) is also pretty subtle.

I also find the two-part definition for alias to be inconvenient: because DT_ALIAS_foo is the only option we have for short-hand identification you need a label on the node and, far away, a property bound to that label in the aliases node, solely for the purpose of providing an anchor to identify the node.

At this time I'm leaning toward adding DT_NODE_foo generated wherever foo is a label attached to a node (not the label property). Node labels must be unique within the tree, and absent a representation of the entire path this seems a cleaner solution than defining a special property like zephyr,id or soc-label.

In the case of IMX GPIO for example the fifth GPIO node is defined by:

aliases {
    gpio-5 = &gpio5;
};

soc {
    gpio5:gpio@420ac000 {
    compatible = "nxp,imx-gpio";
    reg = <0x420ac000 0x4000>;
    interrupts = <74 0>, <75 0>;
    rdc = <(RDC_DOMAIN_PERM(A9_DOMAIN_ID,\
        RDC_DOMAIN_PERM_RW)|\
        RDC_DOMAIN_PERM(M4_DOMAIN_ID,\
        RDC_DOMAIN_PERM_RW))>;
    label = "GPIO_5";
    gpio-controller;
    #gpio-cells = <2>;
    status = "disabled";
};

and currently tooling generates:

DT_NXP_IMX_GPIO_420AC000_property   // from compatible+reg? not necessarily unique
DT_ALIAS_GPIO_5_property            // from alias
DT_NXP_IMX_GPIO_GPIO_5_property     // from compatible + label_property
DT_INST_1_NXP_IMX_GPIO_property     // from enumerated compatible, build-specific

I propose removing the alias from the devicetree source, since it's not useful for cross-target resource identification, and generating:

#define DT_NODE_GPIO5_property              // from node label

which is unambiguous within SOC-specific code like a peripheral driver, assuming the SOC devicetree includes are consistent.

I'd also be interested in considering:

#define DT_PATH_SOC_GPIO_420AC000_property   // from path + reg

to solve the "how do I find things by path" problem.

[erwango]

DT_NXP_IMX_GPIO_GPIO_5_property // from compatible + alias? why?

Could it be: compatible + label (prop) ?

I propose removing the alias from the devicetree source, since it's not useful for cross-target resource identification, and generating:
#define DT_NODE_GPIO5_property // from node label

+1

[pabigot]

Could it be: compatible + label (prop) ?

That's probably it and makes more sense than alias (though still not particularly useful). Thanks, updated comment.

[galak]

Covered discussion of this in Dev Review meeting on Feb 6. Opened these two issues for enhancements related to this discussion: #22554 & #22555. @pabigot can you look at these 2 issues and add any comments there.

[erwango]

As part of this work, I think we should settle on current usage of aliases.
A recurrent usage is to look for aliases availability as filter for test compatibility.
cf tests/drivers/pwm/pwm_api/testcase.yaml:

tests:
  drivers.pwm:
    tags: drivers pwm
    filter: dt_alias_exists("pwm-0") or
            dt_alias_exists("pwm-1") or
            dt_alias_exists("pwm-2") or
            dt_alias_exists("pwm-3")
    depends_on: pwm

A (non exhaustive) list of occurrences could be given by following link:
https://github.com/zephyrproject-rtos/zephyr/commit/4cc91fdd0033e5fbadd4a464b8998a70d6df0ad0

Following #22555 implementation, next step would be removal of pleonastic pwm-0 = &pwm0 alias definition.
Then should we define new set of aliases (using a specific namespace?) to express the idea that these are meant to provide genericity and be used with application ?

[mbolivar]

@pabigot as discussed internally at Nordic, @anangl has identified at least one place where this is an invalid assumption:

Here the solution [to use DT_INST_X] is fine because user code will access specific devices through the instance identifier (label property value) and the driver doesn't care which one is which.

In particular, the Nordic HAL exposes some ISR routines for SPI peripherals through names that correspond to their SoC IDs, namely nrfx_spim_X_irq_handler:

https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/spi/spi_nrfx_spim.c#L396

I.e. the driver does care which one is which in some cases, even when drivers have identical compats, so drivers should be able to use NODELABEL in time of need.

It also seems to me there's no disagreement to #22555 (generate NODELABEL node identifiers) in principle, there's just desire to settle on a more robust naming convention ( #22964) for DT macros before adding more names.

I will follow up with another comment summarizing the discussion and making a proposal for how we can close this.

[pabigot]

In particular, the Nordic HAL exposes some ISR routines for SPI peripherals through names that correspond to their SoC IDs, namely nrfx_spim_X_irq_handler:

That likely could be finessed through indexing cast initializer lists, but I have no objections to using NODELABEL for cases where something special is needed. I think it's @galak that particularly wants things to be instance-based.

[mbolivar]

@pabigot @erwango @galak @ulfalizer @carlescufi @mnkp @anangl @jfischer-phytec-iot :

Edit: this comment is now a working consensus, following discussion at 27 February 2020 dev-review metting.

I propose these answers to the questions raised by this issue:

• drivers should refer to devices by INST if possible, especially if those devices can be generically driven by their compatible
• drivers may use SoC specific NODELABEL identifiers to refer to devices when they become available (#22555), e.g. if HAL constraints prevent use of INST (see example in previous comment)
• drivers may use PATH (#22554) node identifiers when they become available, but there should be a clear reason why INST or NODELABEL is not suitable in each case
• drivers shall stop using ALIAS once NODELABEL is available, and we should enforce this as a rule in upstream Zephyr the future. Note the documentation says "aliases nodes are used for [...] mapping certain generic, commonly used names to specific hardware resources", i.e. users can expect to be able to change the node pointed to by an ALIAS around to totally different hardware and expect the code to work, because a different driver will be selected. This makes ALIAS unsuitable for use within a driver IMO.
• in-tree samples should refer to devices by ALIAS if possible, to allow generic sample code to be overridden with overlays for different hardware
• in-tree sample applications should avoid referring to devices by INST and NODELABEL unless they are specifically designed to be device specific -- but generic samples which support overlays are preferred
• in-tree samples may refer to nodes by PATH, e.g. to get compile-time overrides for constant data snuck in via special-purpose nodes (a use-case mentioned by @pabigot at dev-review meeting last week)
• out-of-tree applications may use INST, NODELABEL, ALIAS, PATH, or whatever else works for them
• We do not yet have a stable naming convention for node macros yet (as was requested by pabigot in #21369), but that's covered by another issue, #22964.
• in-tree tests should prefer NODELABEL to ALIAS
• /aliases properties which exist only as a workaround for missing NODELABEL macros (the "pleonastic" nodes erwango mentioned earlier) should be removed once they are unused

[erwango]

@mbolivar

This seems quite fine to me. Though, I'm not clear on the difference you make between in-tree and built-in. Can you clarify this ?

Finally, about legit aliases, should we work on defining naming rules, or keep as is for now and clarify when need arises?

[mbolivar]

Though, I'm not clear on the difference you make between _in-tree_ and _built-in_. Can you clarify this ?

No difference was intended. Sorry to be confusing. I've updated the comment to use in-tree consistently.

Finally, about legit aliases, should we work on defining naming rules, or keep as is for now and clarify when need arises?

I don't know myself, but I think that might be a separate issue. It seems like this issue is mainly about what types of node identifiers should be used, and when, rather than how to name each alias identifier.

[erwango]

Though, I'm not clear on the difference you make between _in-tree_ and _built-in_. Can you clarify this ?

No difference was intended. Sorry to be confusing. I've updated the comment to use in-tree consistently.

Perfect thanks.

Finally, about legit aliases, should we work on defining naming rules, or keep as is for now and clarify when need arises?

I don't know myself, but I think that might be a separate issue. It seems like this issue is mainly about what _types_ of node identifiers should be used, and when, rather than how to name each alias identifier.

Ok, thanks.