devicetree: allow interating devicetree nodes with identical zephyr,device-type properties

[henrikbrixandersen]

This PR add a base zephyr,device-type property for indicating the Zephyr-specific type of device represented by a given binding. Knowing the type of device will allow iterating through a devicetree nodes with the same type at build-time, an often requested feature.

As examples, the base ADC controller and base GPIO controller devicetree bindings have been updated to make use of this new property. If this is accepted, all devicetree base bindings should have this added.

Apart from the tests cases, the list of hardcoded compatibles in the ADC shell module has been replaced with a one-liner, iterating all ADC controller devicetree nodes. This not only saves us having to update this list, but it also makes the ADC shell work with out-of-tree ADC controller bindings/drivers.

This functionality can be used many places in the tree.

[gmarull]

I have to say that I'm not convinced by this approach. This is yet another DT property that has nothing to do with hardware description. Zephyr's flavor of DT should soon deserve its own name (or standard?), as it has deviated so much from Linux DT. If we need to iterate over devices based on their class, what I think we need are device class specific instantiation macros, so that we can store devices in multiple lists (using iterable sections). This would likely allow to also have N class instances of a single DT node, provided we have getter macros that are class specific, ie, DEVICE_GPIO_DT_GET() -> __device_gpio_dts_ord_N.

[henrikbrixandersen]

@gmarull Yeah, I know that this does have a slight hacky feel to it. Thanks for looking through it.

It was an idea I had while working in a different issue, and I couldn't really let it go without giving it a try.

I'd be interested in hearing other opinions too.

[bjarki-andreasen]

The last time I proposed something similar, namely adding an api property to the devicetree, it was argued that the API is not a hardware property. I know this PR only mentions device class, but I'm guessing we expect the API to match the device type as well given the use from shells. In any case, I pointed to the deprecated device_type property (according to DT spec v0.4) as an argument for having the type in the devicetree, which was not exactly met with open arms :D (very much no to that).

I like the approach of the macros like SENSOR_DEVICE_DT_DEFINE which are quite simple, and I may want to do the same for RTCs (to be able to automatically get time using standard time functions like mktime, although zephyr,rtc = &rtc may be better for that).

RTCs are actually a good example of where the API does not match the device type, given that some RTCs in-tree can implement either the counter API or the RTC API depending on if CONFIG_COUNTER=y or CONFIG_RTC=y, in this case, the sensor type macro will more accurately tag the device as an RTC or counter than the zephyr,devicetype property. I know its an outlier, but it's notable.

That said, how about the required properties used for gpio-controller and interrupt-controller? These seem to be able to be used in place of the zephyr,devicetype? should we add this to ADCs, like adc-controller?

[nordicjm]

If we need to iterate over devices based on their class, what I think we need are device class specific instantiation macros, so that we can store devices in multiple lists (using iterable sections).

The device specific class macro idea sounds like a lot of work for someone creating their own drivers classes out of tree, they need to edit/add linker files (so basically have to fork zephyr), add device instance macros that presumably would need to reference some unique ID - and how would that ID remain unique? If they then updated to the next zephyr release which adds e.g. 2 extra driver classes in tree, they'd then have a collision? Or would there be no IDs at all and just names, but then how would you iterate over all devices (ignoring types)?

[henrikbrixandersen]

That said, how about the required properties used for gpio-controller and interrupt-controller? These seem to be able to be used in place of the zephyr,devicetype? should we add this to ADCs, like adc-controller?

This was actually my initial approach, as I already use the gpio-controller property to iterate all GPIO controller nodes in the GPIO hogs driver:

zephyr/drivers/gpio/gpio_hogs.c

Line 38 in 574e641

#define GPIO_HOGS_NODE_IS_GPIO_CTLR(node_id) \

But in order to rely on this, every device base binding would have to invent their own variant of adc-controller, gpio-controller, interrupt-controller and it would be difficult to scale this solution - not to mention support out-of-tree driver classes.

[henrikbrixandersen]

The device specific class macro idea sounds like a lot of work for someone creating their own drivers classes out of tree, they need to edit/add linker files (so basically have to fork zephyr)

I share that concern. While I agree that the introduced zephyr,device-type property doesn't really describe hardware (although it kind of does, similar to the gpio-controller property), the approach here supports out-of-tree driver classes without anything else needed than three lines in an out-of-tree YAML file.

By using default and const in base bindings, the property will only be part of the generated zephyr.dts and should never have to appear in user-written DTS files, which somewhat encapsulates this.

[Laczen]

This proposal looks like a re-introduction of device_type that has been deprecated.

The compatible construction has the same functionality: create an umbrella compatible and use this to walk. The device_type would just replicate this construct with a lot of source for discussion: what device_types will be allowed, should multiple device types be supported, ...

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[bjarki-andreasen]

I made an attempt at making sorting by devicetype at build time using macros like dts a while back #58112 It's a bit more complicated as we are generating headers at different build stages, but the underlying idea was the same :)

[pdgendt]

When a compatible describing the device-type is added to the dts-node it is possible to just add the linker sections based upon #define DT_DRV_COMPAT xxx and DT_INST_FOREACH_STATUS_OKAY or in the driver code validate the existence of the "device-type compatible" and add it to allow run-time validation.

This would create an additional array/structure, whereas placing the existing API structs into a linker section has no additional ROM overhead.

[Laczen]

When a compatible describing the device-type is added to the dts-node it is possible to just add the linker sections based upon #define DT_DRV_COMPAT xxx and DT_INST_FOREACH_STATUS_OKAY or in the driver code validate the existence of the "device-type compatible" and add it to allow run-time validation.

This would create an additional array/structure, whereas placing the existing API structs into a linker section has no additional ROM overhead.

Adding them directly into a linker section could also be done, the compatible is describing the name of the linker section, a "registration" to this linker section is possible with a construct like the one utilized in registering static settings in the settings subsystem.

[fabiobaltieri]

@Laczen just to be clear: your suggestion is to use multiple compatibles, basically adding one for the device type, and then iterate over those, right?

[Laczen]

@Laczen just to be clear: your suggestion is to use multiple compatibles, basically adding one for the device type, and then iterate over those, right?

Yes, use a compatible to describe the device-type(e.g. XXX).

It is then possible to drive the creation of the linker section based upon DT_HAS_XXX_ENABLED and have the driver register to the appropiate linker section (the node has the xxx-compatible). I don't think any (extra) iteration is needed.

But I am unsure if adding to a linker section will work if the API is not constantly sized.

[fabiobaltieri]

Yeah my point is that it would be nice to use the api struct information instead rather than specifying it manually, that way the system would just work.

[Laczen]

Yeah my point is that it would be nice to use the api struct information instead rather than specifying it manually, that way the system would just work.

Using the api struct information could be done by modifying the DEVICE_DT_INST_DEFINE() macro, not registering a pointer but instead the api. This will take some macrobatics to support compile optimizations to reduce the api struct (some routines maybe missing depending on compile options). This would require quite some work.

Modifying DEVICE_DT_INST_DEFINE() to take a specific linker section (for storing the pointer to the devices api struct) should be quite simple and this would also allow checking if the dev belongs to a specific type (as suggested by @pdgendt). Maybe we can even just sort the devices based upon the specified compatible and retrieve the start and end of these devices types automatically. It would not be as optimal as directly registering the api structure but it are just a number of pointers to the api structs.

[henrikbrixandersen]

It's great you have all sorts of ideas how this could be done in different ways. I urge you to create PRs implementing this functionality as alternatives to this. This PR has been open for three months now without anybody stepping up with another proposed solution that does incur a run-time overhead in one way or another (ROM usage, CPU cycles, ...).

The suggestion about having a common device type be part of the compatibles list is not as easy as it sounds, though. All DTS(I) files would need to be modified for this solution, whereas the solution implemented here only requires three lines added to each base devicetree binding file.

[pdgendt]

It's great you have all sorts of ideas how this could be done in different ways. I urge you to create PRs implementing this functionality as alternatives to this. This PR has been open for three months now without anybody stepping up with another proposed solution that does incur a run-time overhead in one way or another (ROM usage, CPU cycles, ...).

I think this PR is a nice addition, it solves the problem of listing devices at compile time, whereas the linker idea is useful for runtime checks.

[Laczen]

It's great you have all sorts of ideas how this could be done in different ways. I urge you to create PRs implementing this functionality as alternatives to this. This PR has been open for three months now without anybody stepping up with another proposed solution that does incur a run-time overhead in one way or another (ROM usage, CPU cycles, ...).

The suggestion about having a common device type be part of the compatibles list is not as easy as it sounds, though. All DTS(I) files would need to be modified for this solution, whereas the solution implemented here only requires three lines added to each base devicetree binding file.

@henrikbrixandersen, all ideas that have been formulated where about extending the proposal. And these extensions could be done based upon a device-type or a compatible.

Regarding the use of compatibles versus device-type: The devicetree in linux used to have something like a device-type, but it has been deprecated so I find it strange that zephyr would need it. In zephyr we are already "grouping" items based upon compatibles, e.g. "fixed-partitions", "soc-nv-flash", so why take a different approach for devices ?

Regarding the need to modify all (DTS)I files or not, this is correct, the PR needs less work in (DTS)I files.

[de-nordic]

I like the idea, it very much reminds me of PCI classes for devices, which kinda represents subsystems that will use the device in the end.

Regarding the DTS usage: I think that the is the right place, even though it seems to be software configuration I do not think it actually is; notice that with PCI you could ask device for a class it serves, so basically you would ask it what subsystem it will be attached to - that is the same thing - but here DTS is what we have and we want to know at compile time.

Yes, you could probably do that with _HAS_ Kconfigs, but these should be reserved for advertisement that device driver is available rather then the device, we already know what devices we have from tree (if it is status "okay").

As far as I also understand: this is very automatic; once you derive your device binding from specific class it automatically advertises itself as a device of a given class. This is nice.

I think that this is nicely done and addresses the issue correctly; I would prefer "device-class" over "device-type", but I can live with the later.

Anyway, I like how this is done.

[henrikbrixandersen]

I think that this is nicely done and addresses the issue correctly; I would prefer "device-class" over "device-type", but I can live with the later.

I started out with zephyr,device-class actually, and I wouldn't be opposed to reverting back to that.

[de-nordic]

It's great you have all sorts of ideas how this could be done in different ways. I urge you to create PRs implementing this functionality as alternatives to this. This PR has been open for three months now without anybody stepping up with another proposed solution that does incur a run-time overhead in one way or another (ROM usage, CPU cycles, ...).
The suggestion about having a common device type be part of the compatibles list is not as easy as it sounds, though. All DTS(I) files would need to be modified for this solution, whereas the solution implemented here only requires three lines added to each base devicetree binding file.

@henrikbrixandersen, all ideas that have been formulated where about extending the proposal. And these extensions could be done based upon a device-type or a compatible.

Compatible has been designated for identification of device driver that can serve it, sometimes one or more, because you can have a generic driver, vendor specific or rebrand driver (e.g. Realtek as Dell).
Paragraph 2.3.1 https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.4

Regarding the use of compatibles versus device-type: The devicetree in linux used to have something like a device-type, but it has been deprecated so I find it strange that zephyr would need it. In zephyr we are already "grouping" items based upon compatibles, e.g. "fixed-partitions", "soc-nv-flash", so why take a different approach for devices ?

So we made mistakes, the "fixed-partitions" is a device actually, if we use Linux in argumentation:
https://github.com/torvalds/linux/blob/71b1543c83d65af8215d7558d70fc2ecbee77dcf/drivers/mtd/mtdpart.c#L416

And "soc-nv-flash" is another example of a device that has the wrong type, as it is basically "flash-controller", and causes problems when you want to have more than one of it in a system.

[de-nordic]

I think that this is nicely done and addresses the issue correctly; I would prefer "device-class" over "device-type", but I can live with the later.

I started out with zephyr,device-class actually, and I wouldn't be opposed to reverting back to that.

Do not bother till the approach is well understood, naming should be like the last concern in anything.

[decsny]

@henrikbrixandersen this is just a loose idea I have from glancing over this thread and the PR a bit, but what do you think of the idea of something like: instead of putting this as a DT property (or using linker scripts, etc), just add to the DT binding schema, and put this information in the binding definitions, but not as a property, therefore not in the actual DTS

[gmarull]

I'm sorry, but to me, using devicetree to add yet more software properties is a -1. Usage of zephyr, has become a mess to a point where we should likely rename devicetree to something else, maybe ztree or something. This solution also doesn't account for device nodes that can have >1 device type, which even if not supported now, they should if we ever pretend to fully align with Linux.

[decsny]

adding that my suggestion just above would theoretically be able to handle multiple types

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[henrikbrixandersen]

Closing this as we can now iterate devices of a given type at runtime (see #71773). I may pick it up again later if the need for build-time iteration arises.