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.

Sign up for GitHub

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

Jump to bottom

device: supported devices API #37836

Merged
merged 6 commits into from
Aug 25, 2021
Merged

device: supported devices API #37836

merged 6 commits into from
Aug 25, 2021

Conversation

JordanYates
Copy link
Collaborator

Implements an API to iterate over devicetree devices that an arbitrary device supports.
Follows the conventions setup by the dependent devices API.

Includes a minor optimization to the handles array (saves one handle per array), which offsets the cost of the additional DEVICE_HANDLE_SEP. Therefore the ROM increase is purely a function of the number of supported devices per device (2 bytes per).

Implements #37793.

@github-actions github-actions bot added area: API Changes to public APIs area: Kernel area: Tests Issues related to a particular existing or missing test labels Aug 20, 2021
@ceolin
Copy link
Member

ceolin commented Aug 20, 2021

cool, I have the same feature implemented in a different pr #37723. Lets me try my feature on top of your supported devices implementation. I'm prone to your implementation, it is looking cleaner than mine.

mbolivar-nordic
mbolivar-nordic reviewed Aug 20, 2021
View reviewed changes
Copy link
Contributor

@mbolivar-nordic mbolivar-nordic left a comment

Choose a reason for hiding this comment

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

Can you please split the functional changes from the whitespace changes?

@JordanYates
Copy link
Collaborator Author

Can you please split the functional changes from the whitespace changes?

Done

@JordanYates
Copy link
Collaborator Author

cool, I have the same feature implemented in a different pr #37723.

I hadn't realized there was already an implementation, or I wouldn't have bothered to try it myself.
I do think adding the handles to the existing array is a better solution than a new array.

@ceolin
Copy link
Member

ceolin commented Aug 23, 2021

cool, I have the same feature implemented in a different pr #37723.

I hadn't realized there was already an implementation, or I wouldn't have bothered to try it myself.
I do think adding the handles to the existing array is a better solution than a new array.

Agree, as soon as this pr gets in, I'll rebase that one.

@ceolin ceolin mentioned this pull request Aug 24, 2021
Closed
nashif
nashif requested changes Aug 24, 2021
View reviewed changes
include/device.h Outdated Show resolved Hide resolved
include/device.h Outdated Show resolved Hide resolved
include/device.h
* @return The number of devices that were visited if all visits succeed, or
* the negative value returned from the first visit that did not succeed.
*/
int device_supported_foreach(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.

I am wondering if "supported" is the right language we want to use here, maybe "connected"? Then the @brief would be "Visit every device that is directly connected to @p dev."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Supports is the terminology used in the existing API's. I do agree it makes for awkward documentation.

zephyr/include/devicetree/ordinals.h

Lines 47 to 61 in cc760c5

* @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.

Supports is the terminology used in the existing API's. I do agree it makes for awkward documentation.

yes it looks very awkward for someone not familiar with the DT APIs. Is this a zephyr thing or does this come from the devicetree spec? @mbolivar-nordic @galak

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a zephyr thing or does this come from the devicetree spec? @mbolivar-nordic @galak

It's a zephyr thing.

Jordan Yates added 5 commits August 25, 2021 08:42
device: whitespace cleanup
ee5809f 
Remove double spaces after full stops in doxygen comments.

Signed-off-by: Jordan Yates <[email protected]>
device: optimize handles array
a0ba89e 
Optimize the handles array by making the following observations:
 * The devicetree ordinal at index 0 in pass1 is discarded by
   gen_handles.py, and therefore does not appear in the pass2 array.
 * gen_handles.py does not need `DEVICE_HANDLE_ENDS` to determine the
   end of the handle array, as that information is present in the .elf.

Therefore, instead of replacing the devicetree ordinal with an
additional `DEVICE_HANDLE_ENDS` at the end (to preserve lengths), we
can simply move the ordinal to the end and have it be the original
`DEVICE_HANDLE_ENDS` symbol. This reduces the size of the array by
one handle per device (2 bytes).

Signed-off-by: Jordan Yates <[email protected]>
device: simplify structure of handles array
ff599d8 
Force the inclusion of a `DEVICE_HANDLE_SEP` at the end of the
devicetree dependency section of the array. This lets us simplify the
implementation of `device_required_handles_get`, as there is only one
symbol the section ends with.

This does not use any extra ROM as the array is padded out to the
original size with `DEVICE_HANDLE_ENDS` anyway.

Also adds a description of the array format where the array is
instantiated.

Signed-off-by: Jordan Yates <[email protected]>
device: iterable supported devices
3a7d327 
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]>
device: supported devices visitor API
7011219 
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]>
tests: devicetree: test supported devices API
352dd13 
Add tests for the supported devices API.

Signed-off-by: Jordan Yates <[email protected]>
@nashif nashif merged commit 4c32e21 into zephyrproject-rtos:main Aug 25, 2021
@JordanYates JordanYates deleted the 210820_dt_supported branch August 26, 2021 01:24
This was referenced Sep 24, 2021
Merged
Closed
@JordanYates JordanYates mentioned this pull request Nov 17, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nashif nashif nashif approved these changes

@ceolin ceolin ceolin approved these changes

@dcpleung dcpleung dcpleung approved these changes

@mbolivar-nordic mbolivar-nordic mbolivar-nordic approved these changes

@gmarull gmarull Awaiting requested review from gmarull

@andyross andyross Awaiting requested review from andyross

@tbursztyka tbursztyka Awaiting requested review from tbursztyka

@galak galak Awaiting requested review from galak

Assignees

@tbursztyka tbursztyka

Labels
area: API Changes to public APIs area: Build System area: Device Model area: Devicetree Tooling PR modifies or adds a Device Tree tooling area: Devicetree area: Kernel area: Tests Issues related to a particular existing or missing test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@JordanYates @ceolin @nashif @dcpleung @mbolivar-nordic @tbursztyka