Zephyr: [RFC]: moving to eliminate dts_fixup files

[pabigot]

DTS fixup files exist, as far as I can reconstruct, to bridge between names formerly used to specify device hardware resource assignments in Kconfig with names that are generated when the resource relationships are defined in device tree source files. They are ugly, verbose, and fragile; in short, a source of errors and maintenance pain.

Work like #11707 gives a hint for how we can get rid of fixup files, but there are impacts in naming that propagate to platform-independent driver code and to application code.

The purpose of this issue is to test the Zephyr community's tolerance for the consequences of this proposal.

As an example let's use samples/sensor/ccs811. The sensor exists on the nrf52_pca20020, and driver code was converted to using DT_ prefix instead of CONFIG_ prefix in #11180. As a result, the existing driver requires the following fixups:

• DT_CCS811_NAME used in DEVICE_AND_API_INIT (and preferably instead of "CCS811" in the application call to device_get_binding());
• DT_CCS811_I2C_MASTER_DEV_NAME used in the driver
• DT_CCS811_I2C_ADDR used in the driver

The board DTS file contains this:

&i2c0 {
        ccs811: ccs811@5a {
                compatible = "ams,ccs811";
                reg = <0x5a>;
                label = "CCS811";
        };
};

In the absence of any dts_fixup.h entries we end up with the following in generated_dts_board.h:

// block 1
#define DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_BASE_ADDRESS 0x5a
#define DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_BUS_NAME     "I2C_0"
#define DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_LABEL        "CCS811"
// block 2
#define DT_AMS_CCS811_I2C_0_AMS_CCS811_5A_BASE_ADDRESS DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_BASE_ADDRESS
#define DT_AMS_CCS811_I2C_0_AMS_CCS811_5A_BUS_NAME     DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_BUS_NAME
#define DT_AMS_CCS811_I2C_0_AMS_CCS811_5A_LABEL        DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_LABEL
#define DT_AMS_CCS811_I2C_0_AMS_CCS811_5A_SIZE         DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_SIZE
// block 3
#define I2C_0_AMS_CCS811_5A_BASE_ADDRESS               DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_BASE_ADDRESS
#define I2C_0_AMS_CCS811_5A_BUS_NAME                   DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_BUS_NAME
#define I2C_0_AMS_CCS811_5A_LABEL                      DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_LABEL
#define I2C_0_AMS_CCS811_5A_SIZE                       DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_SIZE

Block 1 comes from the device tree directly, block 2 is synthesized by extract_dts_includes using a new
alias name incorporating the compatible property, and block 3 comes from the defaulted --old-alias-names. (I think the _SIZE generations are a bug in the extractor as there is no such symbol.)

None of these can be used directly in the sensor driver because the board-specific I2C bus and address are part of the identifier.

We can add the following alias to the DTS file:

/ {
        aliases {
                ccs811 = &ccs811;
        };
};

As a result we automatically get these added to generated_dts_board.h (excluding the _SIZE aliases):

// block 1
#define DT_AMS_CCS811_CCS811_BASE_ADDRESS DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_BASE_ADDRESS
#define DT_AMS_CCS811_CCS811_BUS_NAME     DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_BUS_NAME
#define DT_AMS_CCS811_CCS811_LABEL        DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_LABEL
// block 2
#define CCS811_BASE_ADDRESS               DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_BASE_ADDRESS
#define CCS811_BUS_NAME                   DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_BUS_NAME
#define CCS811_LABEL                      DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_LABEL

where extract_dts_includes has produced both old-style and new-style names.

IMO --old-alias-names should be deprecated because the symbols aren't prefixed with DT_, but the new aliases are decoupled from any board-specific wiring and are suitable for use in cross-platform drivers.

So if we're willing to make global name changes like the following:

• Replace DT_CCS811_NAME with DT_AMS_CCS811_CCS811_LABEL
• Replace DT_CCS811_I2C_MASTER_DEV_NAME with DT_AMS_CCS811_CCS811_BUS_NAME
• Replace DT_CCS811_I2C_ADDR with DT_AMS_CCS811_CCS811_BASE_ADDRESS

and more generally accept the names synthesized from the device tree as canonical instead of whatever was used in the past, I think we can entirely get rid of dts_fixup.h files. But take into account what is visible in application code: e.g. the parameter to device_get_binding will change for samples that didn't hard-code the label. Within this development cycle it's already changed (CONFIG_FXOS8700_NAME to DT_FXOS8700_NAME) so a second permanent change (... to DT_NXP_FXOS8700_FXOS8700_LABEL) doesn't seem too harsh.

We could also consider this comment which would change the generated symbols for aliases to:

#define DT_CCS811_BASE_ADDRESS DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_BASE_ADDRESS
#define DT_CCS811_BUS_NAME     DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_BUS_NAME
#define DT_CCS811_LABEL        DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_LABEL

This is closer to the pre-device-tree CONFIG_foo naming scheme, and is probably safe as it'd be the responsibility of the device tree overlay that defines the alias to make sure that it identifies a node with the expected compatibility. It would eliminate the oddity of names like DT_AMS_CCS811_CCS811_LABEL with its duplicated CCS811 component.

[diegosueiro]

@pabigot,

One question came up in my mind.
Why do we even need to explicitly add an alias to generate a more "friendly" #define?
Maybe this could be generated automatically.

In my understanding, we should be using the alias to create meaningful names. For the FXOS8700 we could have an alias like accel = &fxos8700;, for example.

[pabigot]

@diegosueiro I think you're proposing adding or substituting more generic aliases, instead of only an alias that conforms to the device name. I don't have an objection to the adding, though if we do it should probably be from a specific set of standard aliases: e.g. always use accelerometer not sometimes accel.

Consider this:

/ {
    accelerometer = &fxos8700;
    fxos8700 = &fxos8700;
}

Here we specify the device by function, but also by the model field of the corresponding compatible property. You'd use the first in application code that uses the generic sensor API, but the second if your code used features present in the fxos8700 but not all other accelerometer drivers. The driver would use the second.

I'm motivated to the specific name because the existing sensor API is too generic for complete functionality. Devices may or may not support certain types of trigger. The CCS811 indoor air quality sensor simply cannot be used correctly without additional API that can't be supported by the existing generic sensor API. Once some more basic questions are resolved I plan to issue proposals to address that.

Or a device might be a magnetometer and/or gyroscope as well. I don't think you'd want generic aliases that all refer to the same device because the operations used might conflict if all specified on the same device.

I think most non-trivial applications are written for cases where the developer knows the underlying hardware and intends to take advantage of it. So providing a way to clearly indicate the intended target hardware by using its name in the source is a good thing.

If your point is that the extraction tools should automatically be generating an "alias" based on the model component of a compatible property, that's an approach but it's starting to get complex. I expect to see what's available by looking at device tree source files. If magic in preprocessors is changing or adding stuff, it's going to be very difficult to figure out what's going on.

[diegosueiro]

@pabigot,

If we always want to have #defines like #defines DT_AMS_CCS811_LABEL DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_LABEL, I think that this should be performed automatically, without needing to add an alias in the device tree.

Then the alias could be used by the user if he/she wants to generate a more meaningful/generic names.

[sdrsdr]

Aliases are super useful if you have two sensors with the same driver but on different buses or addresses. Also aliases isolates you somewhat from schematics changes (from bus to bus) all you have to do is adjust the device tree and your C/C++ code remains the same. Auto generated aliases will just shorting some defines but might get you in trouble if you're having same chips on different buses (it is not common but is possible)

[erwango]

Fixup files are temporary solution waiting to be able to generate device driver code from device tree.

Requirements for this are here:

• https://github.com/zephyrproject-rtos/zephyr/issues/10904

Prototypes solutions:

• https://github.com/zephyrproject-rtos/zephyr/pull/10957
• https://github.com/zephyrproject-rtos/zephyr/pull/8561

Hopefully, once available, we'll get rid of (most of) the fixups.

[pabigot]

10904 outlines design goals, not requirements (how would you verify "Code generation provided should be easy to maintain"?) and half the text describes a proposed solution using template-based code generation.

I was sorta hoping for something to explain why we aren't just walking a device tree blob at startup (using existing practice from Linux), or generating an immutable data structure with pre-populated driver configuration objects at compile time and walking that at startup (more suitable for embedded applications with fixed resources). Or maybe that's fundamentally what all that stuff with databases or jinja code generation accomplishes. I haven't read it closely but going in I'm scared.

So we have two competing proposals for a solution? How is the selection to be made? And when?

What I propose doesn't cover multiple instances of the same device on different busses/addresses, but then neither does what we have now (at least for any sensor I've seen). For #11545 I'm currently looking at something like this in a cross-platform application dts_fixup.h:

#if defined(CONFIG_HAS_DTS_I2C)

#ifndef DT_SHT3XD_NAME

#ifdef DT_NORDIC_NRF_I2C_40003000_SENSIRION_SHT3XD_44_LABEL

#define DT_SHT3XD_NAME          DT_NORDIC_NRF_I2C_40003000_SENSIRION_SHT3XD_44_LABEL
#define DT_SHT3XD_I2C_ADDR      DT_NORDIC_NRF_I2C_40003000_SENSIRION_SHT3XD_44_BASE_ADDRESS
#define DT_SHT3XD_I2C_MASTER_DEV_NAME   DT_NORDIC_NRF_I2C_40003000_SENSIRION_SHT3XD_44_BUS_NAME

#ifdef DT_NORDIC_NRF_I2C_40003000_SENSIRION_SHT3XD_44_ALERT_GPIOS_CONTROLLER
#define DT_SHT3XD_GPIO_ALERT_DEV_NAME   DT_NORDIC_NRF_I2C_40003000_SENSIRION_SHT3XD_44_ALERT_GPIOS_CONTROLLER
#define DT_SHT3XD_GPIO_ALERT_PIN    DT_NORDIC_NRF_I2C_40003000_SENSIRION_SHT3XD_44_ALERT_GPIOS_PIN
#endif /* GPIO_ALERT */

#else /* board selection */
#error No device tree bindings for SHT3XD
#endif /* board selection */

#endif /* DT_SHT3XD_NAME */

#endif /* CONFIG_HAS_DTS_I2C */

and I'd really like to do something better.

[erwango]

I was sorta hoping for something to explain why we aren't just walking a device tree blob at startup (using existing practice from Linux)

Well, we want to keep footprint as small a possible. And the only blob might just fill the memory space for some of the devices supported. Besides, the parsing code would occupy memory as well. This is why we're looking for a solution that would do the parsing before build time.

[pabigot]

I was sorta hoping for something to explain why we aren't just walking a device tree blob at startup (using existing practice from Linux)

Well, we want to keep footprint as small a possible. And the only blob might just fill the memory space for some of the devices supported. Besides, the parsing code would occupy memory as well. This is why we're looking for a solution that would do the parsing before build time.

Thus the next part you didn't quote:

or generating an immutable data structure with pre-populated driver configuration objects at compile time

which may or may not be what those proposed solutions are doing.

More importantly it seems there's disagreement on whether aliases are the right solution. Alternatives of generating DT_foo symbols based on node label properties as suggested by @diegosueiro are a possible approach that would require small enhancements to existing tooling and would produce identifiers consistent with the actual device name.

I'd be willing to do a PR to add that feature if there are members who would be willing to approve it in principle.

[carlescufi]

@pabigot I think this requires additional discussion based on your proposal and what we have been considering so far. We should wait for @galak to be able to take a look at this first. @erwango has already commented on the current proposal to get rid of the fixup files.

[pabigot]

Agreed, input from @galak is important. My own perspective is still evolving as I get deeper; I'm now less a fan of aliases and also of label. A dedicated property, possibly even marked as specific to zephyr (zephyr,binding-name = "UART0") seems a better approach.

I also believe that the overall solution should include an option to place the device tree blob in the application so it can process the information it needs without having to muck about with tooling that may be ignorant of certain properties.

[mnkp]

I would like to repeat here my comment from #11180 to close the other discussion. The PR has been merged and has too many participants who are likely not interested in the issue.

Another good alternative method to generate DTS defines is to use node label (not to be confused with label property) and phase out defines based on a unit address.

According to the specification a DTS node is defined as

[label:] node-name[@unit-address] {
    [properties definitions]
    [child nodes]
}

e.g.

usart0: usart@40010000 { /* USART0 */
        [...]
};

where usart0 is a node label. In that case

#define DT_SILABS_GECKO_USART_40010000_BASE_ADDRESS           0x40010000

would become

#define DT_SILABS_GECKO_USART_USART0_BASE_ADDRESS             0x40010000

Such defines could be used directly by the device drivers without the need for dts_fixup files or creating redundant alias entries.

Depending on a context within dts source the node label is expanded either to the node's phandle or its full path. As such node label, by definition, is always unique. This is important since by generating defines we flatten DTS hierarchy.

Current method of using unit-address may lead to conflicts. Having board DTS file instantiate two identical I2C sensors with the same I2C address placed on two different buses (not that unusual scenario) would generate overlapping defines.

As implemented currently in Zephyr's DTS files node label typically corresponds to the module instance name as specified by the datasheet. The name is vendor specific, e.g. uart0, usart3, sercom0, leuart2 for serial devices, i2c0, twihs3 for I2C modules. This makes them perfect to use by the device drivers. The nomenclature is vendor specific and easy to use.

I would like also to note that node label needn't be related to the argument passed to device_get_binding(). It should merely be considered as a way to flatten DTS hierarchy in a safe way where every instance is assigned a meaningful logical name.

[pabigot]

I want to step back, because I think my original proposal for this issue was premature. I no longer believe there's consensus on or common understanding of the overall approach for using device tree in Zephyr. I want to instead probe the boundaries of this question:

How should information defined in device tree sources be exposed to SOC and peripheral driver and application code?

We need context. Assume a system with GPIO and I2C support and an HTS221 temperature/humidity sensor that will be monitored by an application. Below is a standalone device tree that demonstrates how some of the information Zephyr requires might be represented:

/*
gcc -x assembler-with-cpp -nostdinc -P -E \
  -DGPIO_ACTIVE_LOW="(1 << 4)" \
  skel.dts \
| dtc -I dts -O dtb -o skel.dtb
dtc -I dtb -O dts skel.dtb > skel.pdts
*/

/dts-v1/;

/ {
  soc {
    #address-cells = <1>;
    #size-cells = <1>;
    gpio0label: gpio@80001000 {
      #gpio-cells = <2>;
      compatible = "vendor,gpio";
      reg = <0x80001000 0x1000>;
      zephyr,binding-name = "GPIO_0";
      // other properties
    };
    i2c0label: i2c@80003000 {
      #address-cells = <1>;
      #size-cells = <0>;
      compatible = "vendor,i2c";
      reg = <0x80003000 0x1000>;
      zephyr,binding-name = "I2C_0";
      // other properties
      hts221@5f {
        compatible = "st,hts221";
    reg = <0x5f>;
    drdy-gpios = <&gpio0label 13 GPIO_ACTIVE_LOW>;
    zephyr,binding-name = "HTS221_0";
    // other properties
      };
    };
  };
};

(Here I'm assuming Zephyr will continue to use string device names at the application level and I'm specifying those names with zephyr,binding-name as the property. The standard label property we're currently using has a different purpose and IMO we should not take it over.)

The pure device tree processed content (normally provided as a binary blob) is very similar, replacing labels with paths or phandles:

/dts-v1/;

/ {

    soc {
        #address-cells = <0x1>;
        #size-cells = <0x1>;

        gpio@80001000 {
            #gpio-cells = <0x2>;
            compatible = "vendor,gpio";
            reg = <0x80001000 0x1000>;
            zephyr,binding-name = "GPIO_0";
            phandle = <0x1>;
        };

        i2c@80003000 {
            #address-cells = <0x1>;
            #size-cells = <0x0>;
            compatible = "vendor,i2c";
            reg = <0x80003000 0x1000>;
            zephyr,binding-name = "I2C_0";

            hts221@5f {
                compatible = "st,hts221";
                reg = <0x5f>;
                drdy-gpios = <0x1 0xd 0x10>;
                zephyr,binding-name = "HTS221_0";
            };
        };
    };
};

Firmware using all these resources must:

• Configure an instance named "GPIO_0" of the vendor specific GPIO driver using the properties at "/soc/gpio@80001000".
• Configure an instance named "I2C_0" of the vendor specific I2C driver using the properties at "/soc/i2c@80003000".
• Configure an instance named "HTS221_0" of the ST HTS221 driver using the "I2C_0" driver through bus address 0x77 with an active-low data-ready signal controlled through the "GPIO_0" driver at pin 13. Note that this driver is not vendor-specific.

The application must then use driver_get_binding("HTS221_0") to fetch the device and use the sensor API to get information from it.

With native use of binding trees the blob representing the hierarchy would be walked to create the necessary devices by reading each node, identifying the corresponding driver through compatible, and initializing an instance of the driver dynamically from the node property values, using node parentage and phandle references to resolve dependencies. Zephyr could do this, but the blob is big (557 bytes for the one above, more like 5 KiBy for a real system) and the code to dynamically create things also large.

Alternatives are to build more compact data structures so driver configuration can be specified at compile-time. Currently this is done by:

• converting node path and property names to macro names where the macro expands to the property value;
• using dts_fixup files to provide macro names where a short component replaces SOC-specific full node path components;
• using the macros to hard-code the configuration in the compiled driver modules.

The shortened names may derive from a device tree alias like gpio0 in the following:

/ {
  aliases {
    gpio0 = &gpio0label; //  references "/soc/gpio0@80001000"
  };
};

or the value of a zephyr,binding-name property (label in the current approach), or a device tree label like gpio0label (which normally would be discarded). I don't see agreement on which way to go.

This macro-based approach does not scale to supporting multiple instances of generic drivers, as the source code requires unique macro names for each configuration and the bus driver, address, and pins are currently all hard-coded via the macro value.

Could @galak, @erwango, @evenl, and others involved in each of the in-progress solutions covered by issue #10904 describe how those solutions will support device initialization and driver lookup, using the above system as a specific example and also describing how the solution would support multiple HTS221 devices?

[erwango]

@pabigot, sorry I don't have much time right now to implement changes to illustrate you request.
One thing to consider is that having multiple HTS221 on one I2C bus can't work, as HTS221 has a hardcoded address. Only way to have 2 HTS221 is to have them on 2 different I2C buses.
This being said, in #8561, you could see an example of a sensor (LSM6DSL) instantiated both on SPI and I2C buses (building the 96b_argonkey sample).

[mnkp]

My understanding was that the main point of this issue is to find an intermediary solution to get rid of dts_fixup files until the proper, long term approach using instance generation is implemented. Which is a separate topic being worked on in #10904. We probably shouldn't duplicate here the discussion related to instance generation. Or have I mixed up something?

[pabigot]

@mnkp you have a good point. The original goal of the issue was removing dts_fixup; however, I now believe we can't do that until we agree on how to get the information from DTS to the code without using dts_fixup. Which depends in part on how we represent the information in DTS, and how existing tooling needs to be updated to transfer the information in a standard way.

That certainly contributes to instance generation in #10904, but I think it can and should be separated. On the whole I'm concerned #10904 (which seems to incorporate #9876) is a too-complex solution path that doesn't have adequately defined top-level requirements or acceptance criteria and probably hasn't gotten as much community visibility and acceptance as it should, given how big a change it appears to be.

Maybe that should be a TSC topic; or at least some discussion about what we should expect for the status of device tree in 1.14. We're two months from feature freeze.

[mnkp]

I believe we should split the following question

How should information defined in device tree sources be exposed to SOC and peripheral driver and application code?

into two

• How should information defined in device tree sources be exposed to SOC and peripheral driver?
• How should information defined in device tree sources be exposed to the application code?"

and focus here on the first one. The second question is best treated as a completely independent one to be answered elsewhere, another time. This will result in a cleaner design. I'll try to explain why.

DTS describes hierarchical structure of the hardware where every node corresponds to a distinct hardware module. This structure is described by the board schematic and SoC datasheet. We should make sure that all DTS nodes can be easily identified by consulting the documentation. Let's take a SoC that has uart0 to uart3, usart0, usart1, leuart0, leuart1. These are vendor specific names which precisely identify the hardware module and its instance. DTS should reflect that.

The question of how user in their application code should get access to these instances is a separate one. Let's say board DTS will enable uart2, uart3 and leuart0 modules as the only serial devices. Should user get access to these modules by using UART_2, UART_3, LEUART_0 names or rather UART_0, UART_1, UART_2? Or maybe TTY_0, TTY_1, COM_0? It's an important question but can be answered another time. We should have this extra mapping layer and possibility to connect application level name to the underlying hardware module that implements it.

To sum up, defines used by SOC and peripheral driver code should reflect schematic/datasheet names, not application names. This is in fact how it is mostly done today.

[pabigot]

I think that's almost right. How about three:

• How should information defined in device tree sources be exposed to SOC drivers?
• How should information defined in device tree sources be exposed to peripheral drivers?
• How should information defined in device tree sources be exposed to applications?

For SOC drivers like gpio, serial, i2c, I agree with you: use compatibility property values and node names that match the vendor documented names. If it's convenient to define an alias that also matches the vendor name, that's appropriate and consistent with existing use of device tree in other systems. Drivers are vendor-specific, so any unique encoding of the node path to a macro name will be fine. We're probably mostly covered for this.

For peripheral drivers like an I2C sensor it becomes more complex. The properties in the peripheral node should match the datasheet: if the data ready signal is named DRDY it should be drdy-gpios not int-gpios. There should be convention on whether a RESETn signal should be resetn-gpios or reset-gpios using GPIO_ACTIVE_LOW (I prefer the latter as it supports the case where the vendor documentation spells RESET with an overbar, and it's what most Linux bindings use).

But the peripheral node is a child of a vendor-specific bus node, and the peripheral driver code is vendor-agnostic: it needs to be provided a reference to a device implementing zephyr,i2c (denoting the common Zephyr I2C driver interface) without having to worry whether the vendor spells the node name i2c-0 or TWI0. It may need reference to a device implementing zephyr,gpio (multiple devices, if multiple signals are needed). I don't know how to do that without mapping the vendor-specific node path to a vendor-independent name, and we don't have a convention for doing that. (It could be avoided if every driver took its parameters from a config structure that's initialized with values that can use vendor-specific names, which is probably the right way and may be the intent behind #10904 though nobody's shown me how that's going to work. If that isn't going to happen in the next two months we need to support the current solution of using macros to hard-code constants in the driver source.)

Conveying to the application the value to be passed to device_get_binding() to access a specific UART is somewhat independent, but presumably the device tree specified the driver instance identifier, so it too has to be exposed through a vendor-independent identifier. It's basically the same problem as we have with peripheral driver reference to SOC resources, but harder because we can't (shouldn't?) generate the application code with the board-specific name substituted in.

It might help to consider the problem from the context of samples/sensors: we're showing how to use a capability in a way that's supposed to work on any board to which somebody's wired up the device, even if we need to provide a DTS overlay for the board in the sample application source directory to describe the wiring. In fact this is where it affects me right now.

[mnkp]

But the peripheral node is a child of a vendor-specific bus node, and the peripheral driver code is vendor-agnostic: it needs to be provided a reference to a device implementing zephyr,i2c (denoting the common Zephyr I2C driver interface) without having to worry whether the vendor spells the node name i2c-0 or TWI0.

Again, I believe this issue is not related to how we should get rid of the fixup files. If the board level DTS exists it contains already information on which sensor is connected to which i2c bus. The problem here is that this information is currently not exported from DTS database. The sensor driver needs to know the handle to the i2c bus instance it is connected to, i.e. the parameter to device_get_binding() function. This is currently the _LABEL property. The driver knows its own _LABEL, but it also needs a _PARENT_LABEL which would contain the _LABEL of the parent. This property is currently not exported. The question of what exactly _PARENT_LABEL property should contain: vendor name or some general name can be in this case ignored.

DTS is using phandles to track dependencies between nodes. The _PARENT_LABEL define is Zephyr specific and used here to illustrate the problem.

[pabigot]

@mnkp Thanks, that provides more information. In the last day I've learned that we have to keep the label property even though it's being misused, but that it's ok because it's going to disappear when tooling is complete. I've also learned that a design goal is to not have Zephyr-specific information in the device-tree source: it's supposed to come from the yaml files (which are very Zephyr-specific).

I've also tentatively concluded that there's no written and maintained design plan for how this is all supposed to work when the tooling is complete. My current best guess is that, when I want to learn how a particular DT_FOO symbol got into a generated header or why a configuration value was set the way it was, I'll need to locate whatever under scripts/dts produced it and decode the Python enough to understand that it noticed a particular compatible value in the DTS file, dug up a yaml file associated with that value, and extracted something from that file that told it to do whatever it did.

I cringe at the thought of how frustrating that's going to be, and how many support questions it'll generate.

For now it does appears the label value is the one we need to manage. The existing tooling should be able to detect node parentage and synthesize "generic" names based on a label path that traces the node path.

But if the future is aliases, maybe we should work towards using them instead.

But they're not generic so can't be used in the drivers as currently implemented.

At this point I really have no idea what to do.

[pabigot]

@galak was good enough to take the time to run through the high level plans and overall concepts so I have a better handle on things. It would be good if those driving this work would come up with some "here's what an application developer would do"; "here's how you'd add a new sensor device" use case walk-throughs so people not deeply involved can see and become more comfortable with what's coming.

In the mean time he's come up with a fourth solution, based on generating a generic prefix using the compatible property value for anything that's a child of a SPI or I2C bus node:

#define DT_AMS_CCS811_LABEL                 DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_LABEL
#define DT_AMS_CCS811_I2C_MASTER_DEV_NAME   DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_BUS_NAME
#define DT_AMS_CCS811_I2C_ADDR              DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_BASE_ADDRESS
#define DT_AMS_CCS811_GPIO_INT_DEV_NAME     DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_INT_GPIOS_CONTROLLER
#define DT_AMS_CCS811_GPIO_INT_PIN          DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_INT_GPIOS_PIN
#define DT_AMS_CCS811_GPIO_RESET_DEV_NAME   DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_RESET_GPIOS_CONTROLLER
#define DT_AMS_CCS811_GPIO_RESET_PIN        DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_RESET_GPIOS_PIN
#define DT_AMS_CCS811_GPIO_WAKE_DEV_NAME    DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_WAKE_GPIOS_CONTROLLER
#define DT_AMS_CCS811_GPIO_WAKE_PIN         DT_NORDIC_NRF_I2C_40003000_AMS_CCS811_5A_WAKE_GPIOS_PIN

I like that in principle, and plan to prototype a solution using that approach rather than aliases.

[galak]

I pushed PR #11886 to update the generation script to generate defines for I2C & SPI devices that we can use in the drivers. This is a temp solution for LTS to help remove dts_fixup.h defines since all the I2C & SPI drivers only support a single instance of a sensor, etc right now.

We'll clean this up post 1.14 when we support fuller device tree code generation.

[mnkp]

I really think we should reconsider the way we use aliases. #11886 relies on the fact that aliases are prefixed with a 'compatible' string which, I believe, is a questionable approach. In DTS aliases are used as a convenient way to create a mapping between logical name and a node. However the generated #defines decorate this name with an extra string. That goes against the very idea of an aliases node. It also complicates things, e.g. it's not possible any more to use defines generated from aliases for tests as a define referring to _LABEL property depends now on the name of the driver that provides it.

Nordic is the first vendor where we define aliases on the SoC and not any more on a board level. Looking at an excerpt from dts/arm/nordic/nrf51822.dtsi.

        aliases {
                i2c-0 = &i2c0;
                i2c-1 = &i2c1;
                spi-0 = &spi0;
                spi-1 = &spi1;
                uart-0 = &uart0;
                adc-0 = &adc;
                gpio-0 = &gpio0;
                gpiote-0 = &gpiote;
                wdt-0 = &wdt;
                qdec-0 = &qdec;
        };

It's pretty clear that we do something wrong here. We create a logical name for a node that already has a logical name. Both names refer to a vendor specific module instance as specified in the datasheet and therefore relay the same information. This is unnecessary.

IMHO the better approach, which I have argued for already, would be to generate defines for the alias nodes in the same way like it was done before the #11180 was merged (the only difference would be adding the DT_ prefix). And to use node labels to generate defines which can be used directly by device drivers.

[pabigot]

+1: I agree the macros generated for aliases should be exactly as spelled by the DTS maintainer, rather than being augmented annotated with elements derived from compatible: the DTS maintainer has to make sure they're unique and I don't see what the annotation adds. I don't believe the augmentation is necessary for what #11886 is trying to accomplish as the Python scripts have access to the compatible values for any node and can use that information as necessary. I do believe aliases are the right way to provide a standard generic name for SOC driver source code to use when the full node path includes board-specific variations like unit address.

-0.9: I do not agree that labels should be used to generate anything (if you mean the label associated with a node name, rather than the label property). In part because I don't see what they add if we standardize on using aliases, and in part because I'd rather not introduce a dependency on something that's lost if the device tree content passes through a blob transformation. However, this is personal opinion and isn't a rejection if somebody can come up with a good reason why aliases done right aren't good enough.

I also think a lot of this discussion is exploring a rabbit hole that we could ignore if there was a clear description of where this whole process is going to end up. AFAICT all this naming is supposed to be removed when the tooling is complete, and the only thing we have to deal with in the short term is trying to reduce the amount of dts_fixup noise.

[mnkp]

Couldn't we replace #11886 by requiring the board or overlay file to define an alias, e.g.

/ {
    aliases {
        ccs811-0 = &ccs811_0;
    };
};

The current DTS scripts generate the following defines (values from generated_dts_board.conf as they are easier to interpret)

DT_AMS_CCS811_CCS811_0_BASE_ADDRESS=0x44
DT_AMS_CCS811_CCS811_0_BUS_NAME="I2C_0"
DT_AMS_CCS811_CCS811_0_LABEL="CCS811"
DT_AMS_CCS811_CCS811_0_SIZE=None

And yes, the alias name would have to follow strict convention, e.g. <_module_name_>-<_instance no._>. But that's easy to document and straightforward to follow.

This approach would work for an arbitrary number of sensors attached to an arbitrary number of buses as long as the total instance number would not exceed number of instances defined in the sensor driver. It would be also consistent with usage in SoC drivers. If we adopt the 'aliases' approach.

[pabigot]

@mnkp so basically the proposal that introduced this issue, but adding an instance number to the alias node name. Yes, that seems to be the best remaining short-term option. It will break out-of-tree application code, but no more than has already been broken by changes since 1.13. We'll have to see what pushback shows up once it starts making it into pull requests. I'm working on one now.

[Mierunski]

I've ran into problems of accessing information from dts few months ago, at first I wanted to automatically add new generation option for defines, as current names are unusable in any way, so #9157 was created where defines in form of 'compatible-name_number_property' were generated depending on peripheral status, i.e. if only I2C_1 was enabled we got something like this:

#define NORDIC_NRF5_I2C_COUNT 1

#define NORDIC_NRF5_I2C_0_LABEL "I2C_1"
#define NORDIC_NRF5_I2C_0_PERIPHERAL_NUMBER 1

Which allowed to use macros for instantiating variable number of peripheral instances.
This was blocked as codegen is going to solve all the problems, and aliases can be used in similar way.

So second thing I tested aliases with #9943 and proceeded to replace Nordic dts.fixup files with aliases #10377
There it was stated that using only alias name is a too 'weak' binding between dts and driver. Through compromise with @galak current solution was developed #11180 where defines from aliases contain compatibles, it is far from perfect but still allows for minimizing dts.fixup files.

IMHO Binding between driver code and dts should depend on compatible property, but it should not be a part of alias. We should have defines that contain compatible, instance number and property generated directly from dts without any needs for aliases and keep alias defines simple as just alias name and property.

At this point however from my experience most of such ideas or discussions is blocked as 'codegen is preferred solution' even if work on it is ongoing for over a year and there is still no sight of ending it soon while most drivers don't need such complicated mechanism for extracting information from dts so defines would be sufficient.

[mnkp]

Binding between driver code and dts should depend on compatible property, but it should not be a part of alias. We should have defines that contain compatible, instance number and property generated directly from dts without any needs for aliases and keep alias defines simple as just alias name and property.

I second that opinion and agree very much with the rest of the post. One solution which would cover all these requirements is to use node labels (but not label property) to generate defines. Node labels provide logical names that correspond to data sheet / schematic naming convention, they are guaranteed to be unique and therefore perfectly suited to flatten the DTS hierarchy when extracting properties.

[b0661]

codegen ... even if work on it is ongoing for over a year

Codegen is not such old !-)
A year ago I proposed a solution with C macros and defines. Only later switched to code generation by Python (the macros were not acceptable).

Within the original proposal I had something similar to the following. It only uses aliases to define the prefix of the device tree property defines. The rest can be easily done by the C preprocessor.

/* aliases for controllers */
#define DT_CONTROLLER_I2C_0     DT_NORDIC_NRF_I2C_0
#define DT_CONTROLLER_I2C_1 ...

/* standard properties */
#define DT_NORDIC_NRF_I2C_0_LABEL "I2C_0"
#define DT_NORDIC_NRF_I2C_0_...       ....

#include <misc/util.h>
i2c_0_label = UTIL_CAT(DT_CONTROLLER_I2C_0, _LABEL); 

[pabigot]

Responding to this comment from @chris-makaio in this issue since the other is a PR with focused technical changes and the question added opens some wider issues better addressed here:

• impact of multiple compatible values on generated symbols
• selecting periph or periph-0 as the alias name

I'm also away from a development environment now so any statements of fact I make are unchecked, but a couple high-level comments:

• Pick a generated name that includes the DT_ prefix, not the old alias names that encroach on arbitrary namespaces. Until this PR is resolved the only one you have available is the one with the "compatible" tag embedded. That may cause a problem because of the next point.
• I'm pretty sure you're going to need something like;
  compatible = "maxim,max17043", "maxim,max170xx";
  since you have a specific chip that (presumably) has a generic interface satisfiable by the same driver as others in the family. I don't know what the existing tooling does when presented with multiple values in compatible property.
• You'll want the base alias name max170xx to match the driver module name so the generated names are fixed. For that I like your edited version.
• Finally, you're touching on a border case by going with an alias max170xx rather than max170xx-0 which allows for multiple instances. Is there any conceivable system that would have multiple instances? E.g. multiple batteries, one supporting the MCU and another supporting a high-voltage peripheral? If so, give the alias an instance identifier: max170xx-0.

[chris-makaio]

• Pick a generated name that includes the DT_ prefix, not the old alias names that encroach on arbitrary namespaces. Until this PR is resolved the only one you have available is the one with the "compatible" tag embedded. That may cause a problem because of the next point.

you mean using
#ifdef DT_MAX170XX_MAX170XX_BUS_NAME

instead of
#ifdef MAX170XX_BUS_NAME

?

• I'm pretty sure you're going to need something like;
  compatible = "maxim,max17043", "maxim,max170xx";
  since you have a specific chip that (presumably) has a generic interface satisfiable by the same driver as others in the family. I don't know what the existing tooling does when presented with multiple values in compatible property.

I thought that the selection of the specific chip would still be done in Kconfig, which would generate for example
#define CONFIG_MAX170XX_MAX17043

• Finally, you're touching on a border case by going with an alias max170xx rather than max170xx-0 which allows for multiple instances. Is there _any_ conceivable system that would have multiple instances? E.g. multiple batteries, one supporting the MCU and another supporting a high-voltage peripheral? If so, give the alias an instance identifier: max170xx-0.

I think this could work:

Example

&i2c1 {
    ...
    max17043:max170xx{
       ...
    };
};

/ {
    aliases {
        max170xx-0 = &max17043;
    };
};

Generated dts:

...
#define DT_MAX170XX_MAX170XX_0_BUS_NAME                     DT_NORDIC_NRF_I2C_40003000_MAX170XX_40003000_MAX170XX_BUS_NAME
#define DT_MAX170XX_MAX170XX_0_LABEL                        DT_NORDIC_NRF_I2C_40003000_MAX170XX_40003000_MAX170XX_LABEL
...
#define DT_MAX170XX_MAX170XX_1_BUS_NAME                     DT_NORDIC_NRF_I2C_40004000_MAX170XX_40004000_MAX170XX_BUS_NAME
#define DT_MAX170XX_MAX170XX_1_LABEL                        DT_NORDIC_NRF_I2C_40004000_MAX170XX_40004000_MAX170XX_LABEL
...

But also twice (once for each instance):
#define DT_MAX170XX_BUS_I2C 1

Furthermore, would be nice to be able to get the instance count somehow

[chris-makaio]

doing some preprocessor magic, something crazy like this could be used for multi-instance driver initialization:

#define MAX170XX_DEVICE_INIT(instance)  \
    static struct max170xx_data _CONCAT(max170xx_data_, instance) = { .idx = instance }; \
    DEVICE_AND_API_INIT( \
        _CONCAT(max170xx_, instance), _CONCAT(_CONCAT(DT_MAX170XX_MAX170XX_, instance), _LABEL), \
        &max170xx_init, \
        &_CONCAT(max170xx_data_, instance), \
        NULL, POST_KERNEL, \
        CONFIG_POWER_INIT_PRIORITY, &max170xx_api \
    );


#ifdef DT_MAX170XX_MAX170XX_0_LABEL
MAX170XX_DEVICE_INIT(0);
#endif

#ifdef DT_MAX170XX_MAX170XX_1_LABEL
MAX170XX_DEVICE_INIT(1);
#endif

#ifdef DT_MAX170XX_MAX170XX_2_LABEL
MAX170XX_DEVICE_INIT(2);
#endif

And for init:

int max170xx_init(struct device *dev)
{
    struct max170xx_data *drv_data = dev->driver_data;
    LOG_DBG("Initializing instance #%u", drv_data->idx);
        return 0;
}

uh this is so crazy...

Any way to avoid ifdef'ing all possible instances? I tried creating a "triple_concat" macro but it didn't work

[pabigot]

Regarding selection of specific chip in Kconfig: why? The board knows what chip's there; allowing the application configuration to specify/override it would only open the possibility of getting it wrong. Plus the driver can/should probably figure out what chip's attached when it's initialized.

Regarding instance generation: that's the responsibility of the future tooling. For now, #ifdef as here is quite adequate (that is, in fact, the only instance in Zephyr I could find of multiple instances of the same device, and it exists solely to provide an I2C test application).

Multiple instances will become more common as Zephyr gains visibility, and quite possibly exist in out-of-tree systems. IMO it's important to have the instance in the alias so that when a second instance is added to the driver we don't need to redefine the alias for the previous one to DT_DRIVER_0_LABEL, or introduce a gratuitous inconsistency/confusion by having DT_DRIVER_LABEL and DT_DRIVER_1_LABEL.

[chris-makaio]

@pabigot your assumption that the board knows what chip is there might be true for the standard boards used around here. But I really would decouple "peripheral settings" from the board.
Just imagine someone taking an Arduino (okay, pretty uncommon, but take the Adafruit Feather M0 instead) as a base. They'll start with the plain board (which except some buttons and leds has no peripherals) and add shields (= peripherals in my terminology). The board will know nothing about these.
Actual case for us is similar: we plan to take a Particle Xenon (nRF52840 SoC) as some sort of hardware boiler-plate for our hardware projects. But since it's basically just the SoC with some buttons and leds, we'll create project-specific boards with further devices. To dive into details: Particle Xenon's sibling, Particle Boron, has a MAX17043 which is a fuel gauge only reading state-of-charge and battery voltage. That's enough for most projects, but we'll run a project which will be solar-powered. For that, we'll use MAX17055 which adds the capability to read charge / discharge rate which is pretty important for locating the solar panel.

Sorry for my spamming here, I hope you get my point, though :)

About the instance generation: comparing with your linked solution, is this way really better? It's pretty much duplicating everything for both instances

[chris-makaio]

pinging @galak because he just replied to my mail related to this topic, #12088 and #11977

[chris-makaio]

Here's my proposal for a new DEVICE macro:

#define DEVICE_INSTANCE_AND_DATA_INIT(dev_name, drv_name, instance, init_fn, data_struct, cfg_info,  \
                level, prio, api) \
            static struct data_struct _CONCAT(_CONCAT(dev_name, _data_), instance) = { \
                .idx = instance, \
                .bus_name = _CONCAT(_CONCAT(_CONCAT(drv_name, _), instance), _BUS_NAME), \
            }; \
            DEVICE_AND_API_INIT( \
                _CONCAT(_CONCAT(dev_name, _), instance), _CONCAT(_CONCAT(_CONCAT(drv_name, _), instance), _LABEL), \
                init_fn, \
                &_CONCAT(_CONCAT(dev_name, _data_), instance), \
                cfg_info, level, \
                prio, api \
            );

usage for drivers supporting multiple instances:

#define MAX170XX_INSTANCE_INIT(instance)  \
    DEVICE_INSTANCE_AND_DATA_INIT( \
        max170xx, DT_MAXIM_MAX170XX_MAX170XX, \
        instance, &max170xx_init, \
        max170xx_data, \
        NULL, POST_KERNEL, \
        CONFIG_POWER_INIT_PRIORITY, &max170xx_api \
    );

What I currently don't like with this is the requirement of passing the substring "DT_MAXIM_MAX170XX_MAX170XX" - how would a developer know what he/she should change it to for their own drivers?
This needs some explanation:

_How do I know which drv_name DEFINE to use?_
Look at your reference board .dts, for example:

&i2c1 {
    status = "ok";
    sda-pin = <24>;
    scl-pin = <41>;

    max17043_0:max170xx {
        label = "max17043";
        compatible = "maxim,max170xx";
        status = "ok";
    };
};

/ {
    aliases {
        max170xx-0 = &max17043_0;
    };
};

Node _max17043_0:max170xx_ defines instance #0 of a device based on max170xx IC family, compatibility enforced by _compatible = "maxim,max170xx";_ where "maxim" is the manufacturer and max170xx the device family.
The compatible-Attribute is the first part transferred to the board's generated DTS header.
_max170xx-0 = &max17043_0;_ inside the aliases-Section exposes it in an accessible manner to the board's generated DTS header and the alias should consist of -.
By following this suggestion, the defines in generated DTS header file will be prepended with

DT_ (for DTS compatibility)
_
_
(where DEVICE_ALIAS should be split in and , separated with "_")

or in one line:
<DEVICE_MANUFACTURER>_<DEVICE_FAMILY>_<DEVICE_FAMILY>_<DEVICE_INSTANCE>

Generated example based on above dts example:
DT_MAXIM_MAX170XX_MAX170XX_0
(prefix for all device-related defines)

#define DT_MAXIM_MAX170XX_MAX170XX_0_BUS_NAME   DT_NORDIC_NRF_I2C_40003000_MAXIM_MAX170XX_40003000_MAX170XX_BUS_NAME
#define DT_MAXIM_MAX170XX_MAX170XX_0_LABEL   DT_NORDIC_NRF_I2C_40003000_MAXIM_MAX170XX_40003000_MAX170XX_LABEL

(actual defines in generated_dts_board.h)

So, DT_MAXIM_MAX170XX_MAX170XX is our reference for use in driver implementation. DT_MAXIM_MAX170XX_MAX170XX_0 on the other hand already includes the instance number and probably won't be used during implementation since we're trying to write code (hopefully) independent from an actual instance.

A macro for defining instances of our driver could now look like this:

#define MAX170XX_INSTANCE_INIT(instance)  \
    DEVICE_INSTANCE_AND_DATA_INIT( \
        max170xx, DT_MAXIM_MAX170XX_MAX170XX, \   
        instance, &max170xx_init, \
        max170xx_data, \
        NULL, POST_KERNEL, \
        CONFIG_POWER_INIT_PRIORITY, &max170xx_api \
    );

Notice the DT_MAXIM_MAX170XX_MAX170XX in line 3 - this is how we're referencing to the device in generated_dts_board.h

[pabigot]

Actual case for us is similar: we plan to take a Particle Xenon (nRF52840 SoC) as some sort of hardware boiler-plate for our hardware projects. But since it's basically just the SoC with some buttons and leds, we'll create project-specific boards with further devices. [...]
Sorry for my spamming here, I hope you get my point, though :)

I do, but the normal way of dealing with differences like you describe would be to create a project/application-specific overlay that extends the base system device tree with whatever hardware is added to support the application. There are several examples of this in the samples directory. You may, of course, do what you want with your own applications, but getting Kconfig involved seems unnecessary.

About the instance generation: comparing with your linked solution, is this way really better? It's pretty much duplicating everything for both instances

The role of this issue is to reach consensus on a way to reduce or eliminate the need for dts_fixup.h. Doing that doesn't require changing the way device instances are defined. It may be worth discussing an alternative approach not related to the code-generator infrastructure plans, but I don't think it belongs here.

[chris-makaio]

How would a driver be included then? By checking for DT_ in CMake?

[pabigot]

After going around with aliases versus labels to produce unique prefixes this was ultimately resolved by #12426 and the decision to use a prefix derived from an instance-annotated compatible property value. Work is ongoing to convert drivers to the new standard.