[RFC] DT: emit ordinals #22292

mbolivar · 2020-01-29T01:38:57Z

Emit dependency ordinals for devices in devicetree.

The idea is we can start using these for hierarchical device management, so also extend the bindings language to express when nodes correspond to Zephyr devices. This lets us emit structure initializers that let devices track their dependencies in a ROM friendly way.

Going this way will let us use __device_<ORDINAL> as the identifier for each struct device with a DT-aware device init macro in the same style as #20253, but with dependency info implicitly embedded in the number. It remains to be seen how that would best fit into the existing init levels and priorities.

This also lets us resolve devices at build time if their ordinals are known constants. It would require a look up table to get a device from its ordinal in the general case.

Setting 'is-device: true' at the top level of a binding YAML file will cause the build system to treat the resulting node specially. In particular, it will be considered to correspond to a struct device. This will enable us to emit dependency ordinals for devices and the other devices they depend on from device tree bindings. This in turn will allow us to make the Zephyr device model more aware of DT. Signed-off-by: Martí Bolívar <[email protected]>

Emit macros for: - a node's device tree dependency ordinal - the dependency ordinals of the devices and nodes it depends on - the dependency ordinals of the devices and nodes that depend on it This enables the Zephyr build system to track this information for dynamic binary device power management based on usage. Signed-off-by: Martí Bolívar <[email protected]>

mbolivar · 2020-01-29T01:39:36Z

scripts/dts/edtlib.py

+    @property
+    def is_device(self):
+        "See the class docstring"
+        return self._binding and self._binding.get('is-device', False)


Maybe this should default to True. As long as we have a matching-compat, we're likely to be a struct device.

Defaulting to True will break #22255. #22255 introduces default bindings for special nodes. These special nodes would be recognized as devices, but are none.

ulfalizer

Some nits

ulfalizer · 2020-01-29T04:15:41Z

dts/binding-template.yaml

 compatible: "manufacturer,device"

+# Set this to a truthy value if this binding is describing a node that
+# has a struct device representation within the Zephyr device


Could make it "... has a 'struct device' representation ..."

Was wondering what kind of device a "struct device" is.

ulfalizer · 2020-01-29T04:16:31Z

scripts/dts/edtlib.py

+    @property
+    def is_device(self):
+        "See the class docstring"
+        return self._binding and self._binding.get('is-device', False)


Just a personal quirk, but make it "is-device" instead of 'is-device' for consistency with the rest of the code.

ulfalizer · 2020-01-29T04:25:19Z

scripts/dts/gen_defines.py

+                  dep_ords([n for n in node.depends_on if n.is_device]))
+    out_node_init(node, 'SUPPORTS_NODES', dep_ords(node.required_by))
+    out_node_init(node, 'SUPPORTS_DEVS',
+                  dep_ords([n for n in node.required_by if n.is_device]))


Use double quotes for strings here too, for consistency.

ulfalizer · 2020-01-29T04:26:05Z

scripts/dts/gen_defines.py

+    out_node(node, 'ORDINAL', node.dep_ordinal)
+    out_node_init(node, 'REQUIRES_NODES', dep_ords(node.depends_on))
+    out_node_init(node, 'REQUIRES_DEVS',
+                  dep_ords([n for n in node.depends_on if n.is_device]))


Maybe dep or the like instead of n. Not sure what n stands for.

On second thought, this isn't enough to capture parameterized dependencies; I'll rethink this. I guess we'll need to special-case by type of dependency (clock subsystem, interrupt controller, GPIO spec, etc.)

ulfalizer · 2020-01-29T04:26:45Z

scripts/dts/edtlib.py


+    is_device:
+      True if the node's binding signals that it corresponds to a Zephyr
+      device model struct device. False otherwise.


'struct device'

b0661 · 2020-01-29T06:46:24Z

This adds a Zephyr software feature "usable for Zephyr device structure" to the bindings that should be hardware specific only.

At least from the naming it should be clear that this for Zephyr (aka "zephyr,is-device", "is_zephyr_device").

Why not add a zephyr specific property to base.yaml:

properties:
    'zephyr,is-device':
        type: bool
        required: false
        description: node corresponds to Zephyr device
        default: false

By this you can specify and get the same information, but the Zephyr specific is only in the Zephyr specific base.yaml. No changes needed for edtlib.py.

In the DTS of the device you would set 'zephyr,is-device' as any other device property.

ulfalizer · 2020-01-29T07:07:24Z

@b0661
I think we should treat the bindings more as glue between the devicetree and the code. I wouldn't mind putting things like output options in there either.

b0661 · 2020-01-29T20:07:54Z

@ulfalizer

I think we should treat the bindings more as glue between the devicetree and the code. I wouldn't mind putting things like output options in there either.

Ok, I see. It's just the dream to have edtlib.py as a generic tool, not a specific tool for Zephyr.
IMHO 'is-device' should classify a device in the view of a device tree (aka. hardware) not in the view of Zephyr. It should also be a device in the view of FreeRTOS, uCos, VxWorks, ...

Reducing the view to Zephyr could create the situation that Zepyhr has an umbrella device driver where only the umbrella device gets the 'is-device' property. All the devices under the umbrella are no devices in this view as no device structure has to be created. Out of tree driver developers with another view on what is a device will get a problem (which can be solved by providing binding file overwrite in edtlib.py, see #22255).

Maybe I'm too peculiar and anyway there is a workaround on the way.

mbolivar · 2020-01-29T20:18:20Z

Ok, I see. It's just the dream to have edtlib.py as a generic tool, not a specific tool for Zephyr.

I was unaware that anybody intended to use edtlib outside of Zephyr. Is this true?

I thought dtlib was the generic part, and edtlib was where we were supposed to put all the special sauce.

IMHO 'is-device' should classify a device in the view of a device tree (aka. hardware) not in the view of Zephyr. It should also be a device in the view of FreeRTOS, uCos, VxWorks, ...

I don't follow. I think whether or not a node in DT corresponds to a struct device in a particular driver implementation is really up to the zephyr driver source code, which means it isn't logically a DT property.

In this case, what I ultimately want to do in zephyr is to obtain the direct struct device dependencies of each struct device in the system for automatic power management purposes. Leveraging DT for this makes sense, since it's the only place where these dependencies are recorded. But not all DT nodes have a corresponding struct device, so the full set of ordinals contains redundant information I want to filter out.

This filtering is up to the drivers to decide, and I think of the zephyr specific DT bindings for a driver as being logically part of that driver.

TL;DR: I'm ultimately trying to let drivers signal to edtlib that some nodes are relevant parts of this dependency hierarchy, while other nodes are not. Then gen_defines can in turn tell me what I need to know about it.

I don't get why FreeRTOS is relevant here.

What am I missing?

mbolivar · 2020-01-29T20:20:30Z

@ulfalizer thank you for the review. I'll address the nits if this RFC seems reasonable and we want to move forward with an ordinal-based approach for device numbering.

carlescufi · 2020-01-29T20:28:23Z

It's just the dream to have edtlib.py as a generic tool, not a specific tool for Zephyr.

This has been mentioned before by @galak, he might want to weigh in.

b0661 · 2020-01-29T21:19:59Z

I was unaware that anybody intended to use edtlib outside of Zephyr. Is this true?

Yes me - somehow. edtlib.py feeds the inline code generator (not gen_defines.py). The inline code generator is used for a Zephyr based project but shall/will also be used for a project that does not run on Zephyr.

I thought dtlib was the generic part, and edtlib was where we were supposed to put all the special sauce.

I thought gen_defines.py is where the Zephyr special sauce is cooked and edtlib extends the DTS for ease of use but does not create new info.

I don't follow. I think whether or not a node in DT corresponds to a struct device in a particular driver implementation is really up to the zephyr driver source code, which means it isn't logically a DT property.

I don´t follow either. I though you want to make the creation of a device struct dependent on the 'is-device' property in the binding.

I don't get why FreeRTOS is relevant here.

It is just an example for another RTOS that might use the bindings and edtlib.

What am I missing?

Nothing. We just have a different understanding of the functionality that edtlib shall provide and what shall be the functionality that is outside of edtlib and the bindings.

As already mentioned:

Maybe I'm too peculiar and anyway there is a workaround on the way.

I just expressed my opinion. I do NOT vote against this PR.

pabigot · 2020-01-30T03:43:37Z

I'm getting a bit lost in understanding the domain model here, and wonder whether everybody commenting shares an understanding.

I believe what we want to know, for a given devicetree node with a compatible, is whether that compatible identifies something that has a driver, and consequently that the node describes an instance of a device supported by that driver, for which dependency information must be provided. That is, compatibles correspond to drivers; devicetree nodes correspond to device instances associated with a driver.

(Is there any case where we'd have two nodes with the same compatible, one of which represents a device and another does not? I hope not, but if so we need to understand how that could happen.)

As such I don't think an is-device property associated with a devicetree node is the right model (and I don't think that's what this PR implements, though I think it was suggested as an alternative). Rather, I think we want a has-driver key associated with the binding. Based on seeing self._binding.get('is-device', False) in the code I believe the association is correct; I just don't think the key is optimally named. I would expect to see something like has-driver: true as a YAML property in the binding top-level dictionary.

Since what we're trying to identify is devicetree nodes that correspond to device instances associated with a driver in Zephyr, I agree that the name should be marked with zephyr somehow so that the binding and tooling can be shared with other systems. So make that zephyr,has-driver: true if you can have commas in keys in YAML, otherwise some other extensible syntax like has-driver: [zephyr, "free-rtos"].

mbolivar · 2020-01-30T17:03:19Z

Yes me - somehow. edtlib.py feeds the inline code generator (not gen_defines.py). The inline code generator is used for a Zephyr based project but shall/will also be used for a project that does not run on Zephyr.

This was discussed today at the dev meeting. @galak agreed that edtlib should be OS agnostic wherever possible. The consensus was also that ordinal-based power management is the clear way forward, so we will need to emit this information in some form TBD.

I think that makes this RFC a success. I'll think of another way to do this and take it forward in another PR. Thanks for the comments!

mbolivar-nordic added 2 commits January 28, 2020 17:33

mbolivar added the area: Devicetree label Jan 29, 2020

mbolivar requested review from galak, pabigot and ulfalizer January 29, 2020 01:38

mbolivar commented Jan 29, 2020

View reviewed changes

ulfalizer reviewed Jan 29, 2020

View reviewed changes

galak added area: Devicetree Tooling PR modifies or adds a Device Tree tooling and removed area: Devicetree labels Jan 30, 2020

mbolivar closed this Jan 30, 2020

[RFC] DT: emit ordinals #22292

[RFC] DT: emit ordinals #22292

Uh oh!

Conversation

mbolivar commented Jan 29, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ulfalizer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

b0661 commented Jan 29, 2020

Uh oh!

ulfalizer commented Jan 29, 2020

Uh oh!

b0661 commented Jan 29, 2020

Uh oh!

mbolivar commented Jan 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbolivar commented Jan 29, 2020

Uh oh!

carlescufi commented Jan 29, 2020

Uh oh!

b0661 commented Jan 29, 2020

Uh oh!

pabigot commented Jan 30, 2020

Uh oh!

mbolivar commented Jan 30, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mbolivar commented Jan 29, 2020 •

edited

Loading