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

device: supported device API #40418

Merged
merged 3 commits into from
Nov 23, 2021
Merged

device: supported device API #40418

merged 3 commits into from
Nov 23, 2021

Conversation

JordanYates
Copy link
Collaborator

Resurrection of #37836, which was reverted in #38834, due to build system fixes in #39959

Implements an API to iterate over devicetree devices that an arbitrary device supports.
Follows the conventions setup by the dependent devices API.
In addition to the original functionality, this PR now forwards supports when an intermediate device is not present in the final build (For example flash partitions under a flash device, with an intermediate "fixed-partitions" node).

Implements #37793.

Jordan Yates added 3 commits November 17, 2021 19:07
Add supported device information to the device `handles` array. This
enables API's to iterate over supported devices for power management
purposes.

Signed-off-by: Jordan Yates <[email protected]>
Adds an API to query and visit supported devices. Follows the example
set by the required devices API.

Implements #37793.

Signed-off-by: Jordan Yates <[email protected]>
Add tests for the supported devices API.

Signed-off-by: Jordan Yates <[email protected]>
@github-actions github-actions bot added the area: API Changes to public APIs label Nov 17, 2021
@JordanYates
Copy link
Collaborator Author

@github-actions github-actions bot added area: Kernel area: Tests Issues related to a particular existing or missing test labels Nov 17, 2021
@carlescufi carlescufi requested review from gmarull and ceolin November 17, 2021 11:25
* pointer if @p dh does not provide dependency information.
*/
static inline const device_handle_t *
device_supported_handles_get(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.

Can we rethink the name? "supported" is really not very explanatory here. What exactly are we returning here, from the point of view of the Devicetree?

Copy link
Member

Choose a reason for hiding this comment

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

Could we use "children" here instead perhaps?

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 existing API usage is SUPPORTS_ORDS, and as this is the exact same information just in a different forms, I don't believe the naming should be different. Whether the existing API should be changed I have no strong feelings on.

/**
* @brief Get a list of dependency ordinals of what depends directly on a node
*
* There is a comma after each ordinal in the expansion, **including**
* the last one:
*
* DT_SUPPORTS_DEP_ORDS(my_node) // supported_ord_1, ..., supported_ord_n,
*
* DT_SUPPORTS_DEP_ORDS() may expand to nothing. This happens when @p node_id
* refers to a leaf node that nothing else depends on.
*
* @param node_id Node identifier
* @return a list of dependency ordinals, with each ordinal followed
* by a comma (<tt>,</tt>), or an empty expansion
*/
#define DT_SUPPORTS_DEP_ORDS(node_id) DT_CAT(node_id, _SUPPORTS_ORDS)

Copy link
Member

Choose a reason for hiding this comment

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

OK, no objections in that case @JordanYates

Copy link
Member

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

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

This is now in good shape. Thanks @JordanYates.

@carlescufi
Copy link
Member

@tbursztyka @mbolivar-nordic @gmarull @ceolin feedback very welcome on this PR.

Comment on lines +503 to +504
* The set of supported devices is inferred from devicetree, and does not
* include any software constructs that may depend on the device.
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at this again a second time, is there any reason for the asymmetry here?

We have dependencies both from devicetree and declared by the build system as "injected" dependencies. So if a device D1 which is not created from devicetree requires device D2 that is created from devicetree, we still know that D2 supports D1 at gen_handles.py time. Why not include it here?

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 comment is perhaps a little bit misleading for the supports direction, as a software construct CANNOT inject any dependencies. This is because DEVICE_DEFINE does not expose the required arguments to Z_DEVICE_DEFINE.

To be honest I'm not entirely sure of the purpose of the injected dependencies in the current implementation. Any ordinals passed to the constructor are not transformed by gen_handles.py, and ordinals are generally useless. If a dependency has an ordinal it implies that it is from devicetree, so why is the dependency not expressed in devicetree in the first place? As already noted, a software device cannot express dependencies.

@carlescufi carlescufi merged commit eec7c53 into zephyrproject-rtos:main Nov 23, 2021
@JordanYates JordanYates deleted the 211117_supported_devices branch November 23, 2021 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Device Model area: Devicetree area: Kernel area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants