Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

devicetree-based device definitions and dependency representations reboot #32127

Merged
merged 5 commits into from
Feb 19, 2021

Conversation

pabigot
Copy link
Collaborator

@pabigot pabigot commented Feb 9, 2021

This PR resurrects the features of #29644 that were reverted in #31548 with a few changes:

@pabigot
Copy link
Collaborator Author

pabigot commented Feb 10, 2021

Added to dev-review to re-introduce to the community and solicit reviews.

@galak galak removed the dev-review To be discussed in dev-review meeting label Feb 11, 2021
@pabigot pabigot force-pushed the nordic/20210124a branch 2 times, most recently from 3c3a9d5 to 48291ac Compare February 16, 2021 13:57
Copy link
Collaborator Author

@pabigot pabigot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erwango Thanks, comments addressed.

include/device.h Outdated Show resolved Hide resolved
@pabigot
Copy link
Collaborator Author

pabigot commented Feb 19, 2021

As I am curious, I ran the new shell functionality, and got the following:
This seems fully correct. But as a side note, this shows the interest of node labels properties.

Yes, they're useful for diagnostics and shell identification. However, if we're going to do that, then we should reverse the in-progress course of removing label properties from nodes. @galak

The label property should still not be marked required, and shell should accept tokens like [0x20000294] as a device identifier (assuming the address resolves to a device object). The latter is not in scope for this PR.

Copy link
Member

@nashif nashif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one comment, otherwise LGTM

include/device.h Outdated
* pointer if @p dh does not provide dependency information.
*/
static inline const device_handle_t *
device_get_requires_handles(const struct device *dev,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about device_dependency_get(..)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that when we support dynamic dependencies this API won't work, because we won't have all devices in a contiguous array. We'll need a callback-based API to iterate over both static and dynamic dependencies.

But I can make this device_requires_handles_get() which would handle the static fetch with naming closer to the proposed standard.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nashif Changed to device_required_handles_get(). Turns out when this was reverted the test code was inadvertently left in tree because it had been patched subsequent to the original merge.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the description of the API is Get the set of devices on which a given device depends, so dependency_get would have reflected that.

I see you changed that to device_required_handles_get, this is fine I guess (better than device_requires_handles_get).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This had to be rebased to resolve a massive conflict with #31880 so I changed the description to reflect that it's limited to devicetree required dependencies. With #31880 in I can now add API to record dynamic dependencies and add a solution for "device_dependency_get()"

Generate arrays of dependency information for each device.  If a
device definition is being constructed from devicetree these come from
the devicetree dependency information.  Additional dependencies may be
passed through using the DT_ macros.

Define flag values for device handles so we can partition the
dependency array into distinct sets, which include things it requires,
things it supports (may not be needed), and child nodes (not
implemented, may not be needed).

Signed-off-by: Peter Bigot <[email protected]>
Following the idiom used for system calls, add script support to read
the initial application binary to identify which devices are defined,
and to use their offset in the device array as their unique handle
rather than the externally-defined ordinal from devicetree.  The
device dependency arrays are updated to use these handles.

Signed-off-by: Peter Bigot <[email protected]>
Verify that device dependencies are encoded into and retrievable from
the device structure.

Signed-off-by: Peter Bigot <[email protected]>
Refactor the output of device list to use standard API to retrieve the
list of devices, and to always display a status rather than hiding
disabled/failed devices.

Add API to associate a distinct identifier with any "device" that does
not have a name.

Where a device has requires dependencies display the devices on which
it depends.

Signed-off-by: Peter Bigot <[email protected]>
Fix the order so that it reflects the actual initialization order,
rather than putting PRE_KERNEL initializations after APPLICATION.

Add SMP in the proper location.

Use the helper function to provide unique identifiers for "devices"
that don't have a device pointer (so don't have a name).

Signed-off-by: Peter Bigot <[email protected]>
@pabigot
Copy link
Collaborator Author

pabigot commented Feb 19, 2021

Recent build failures are still due to #32344.

@nashif nashif merged commit 1fa3447 into zephyrproject-rtos:master Feb 19, 2021
@pabigot pabigot deleted the nordic/20210124a branch February 22, 2021 13:32
tejlmand added a commit to tejlmand/zephyr that referenced this pull request Mar 15, 2021
The script gen_handles.py was introduce in zephyrproject-rtos#32127 but relies on
ZEPHYR_BASE being set in environment.

However, it is not a requirement to set Zephyr base in the users
environment, therefore this is changed to an argument `-z` or
`--zephyr-base` which will be passed from the build system to the
script.

If `-z` or `--zephyr-base` is not provided, the environment will be
checked for a ZEPHYR_BASE setting there.

Signed-off-by: Torsten Rasmussen <[email protected]>
nashif pushed a commit that referenced this pull request Mar 15, 2021
The script gen_handles.py was introduce in #32127 but relies on
ZEPHYR_BASE being set in environment.

However, it is not a requirement to set Zephyr base in the users
environment, therefore this is changed to an argument `-z` or
`--zephyr-base` which will be passed from the build system to the
script.

If `-z` or `--zephyr-base` is not provided, the environment will be
checked for a ZEPHYR_BASE setting there.

Signed-off-by: Torsten Rasmussen <[email protected]>
ryanjh pushed a commit to ryanjh/zephyr that referenced this pull request Mar 23, 2021
... argument

The script gen_handles.py was introduce in zephyrproject-rtos#32127 but relies on
ZEPHYR_BASE being set in environment.

However, it is not a requirement to set Zephyr base in the users
environment, therefore this is changed to an argument `-z` or
`--zephyr-base` which will be passed from the build system to the
script.

If `-z` or `--zephyr-base` is not provided, the environment will be
checked for a ZEPHYR_BASE setting there.

Signed-off-by: Torsten Rasmussen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants