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: add API to iterate over devices and check their validity #26342

Merged
merged 3 commits into from
Jun 23, 2020

Conversation

pabigot
Copy link
Collaborator

@pabigot pabigot commented Jun 22, 2020

The power management infrastructure added a generic API to access the static device list, but it is not generally available. Provide an internal standard API for this.

Replace all the shell infrastructure that iterated over devices with a helper that implements it once. This incidently fixes bugs related to using a pointer before checking it for null, and matching a prefix wherever it occurred in the device name.

Finally add standard API to determine whether a device is OK to use, so we can replace the nulling of the API pointer with something else.

Fixes #24750

Device objects in Zephyr are currently placed into an array by linker
scripts, making it easy to iterate over all devices if the array
address and size can be obtained.  This has applications in device
power management, but the existing API for this was available only
when that feature was enabled.  It also uses a signed type to hold the
device count.

Provide a new API that is generally available, but marked as internal
since normally applications should not iterate over all devices.  Mark
the PM API approach deprecated.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@zephyrbot zephyrbot added area: Sensors Sensors area: I2C area: Shell Shell subsystem area: API Changes to public APIs area: Tests Issues related to a particular existing or missing test area: Kernel area: Documentation labels Jun 22, 2020
@pabigot pabigot force-pushed the nordic/20200622b branch from 9182054 to 3077c14 Compare June 22, 2020 15:37
Several shell modules use cloned code to iterate over all devices and
identify the nth instance that meets some criteria.  The code was
repetitive and included various errors.  Abstract to a helper function
that performs the check consistently.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
include/device.h Outdated
*
* @return true if and only if the device is available for use.
*/
static inline bool z_device_ok(const struct device *dev)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a similar patch for device const-ify stuff (name is different and obviously it does not check on driver_api attribute, plus other details). Since that one is not very relevant for that PR, could your remove that commit?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wondered if that might conflict, but figured that outside the potential name difference it should be easy to adapt to either. Removed anyway.

I do think we should be consistent in naming, i.e. agree whether the check for validity and fetch of static device pointers should be device_* or z_device_* functions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can't close #24750 without that patch, though.

Copy link
Collaborator

@tbursztyka tbursztyka Jun 22, 2020

Choose a reason for hiding this comment

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

indeed, I can pull my patch out for it then
[edit] just a sec

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hum it's inside a patch that revise the initialization status part.

Well, let's cut the pear in half: call it device_ready() and that will be fine for me then

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll do that.

But if this is device_ready then perhaps z_device_get_all_static() should be device_get_all_static(). I don't see applications having a use for device_ready(), as they can't get a device pointer unless it's ready. (In a future Zephyr when they can, e.g. when devices can be made unready, we might have to add more API.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's back, but I kept the internal-use prefix (which I may be mis-using, not sure). This path allows device_ready() to remain available for future situations where somebody might get a device pointer without going through a validating API like device_get_binding(), or where a device may be made unready through power management.

@pabigot pabigot force-pushed the nordic/20200622b branch from 3077c14 to bf3c1e5 Compare June 22, 2020 16:55
Currently this is useful only for some internal applications that
iterate over the device table, since applications can't get access to
a device that isn't ready, and devices can't be made unready.  So it's
introduced as internal API that may be exposed as device_ready() when
those conditions change.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@carlescufi carlescufi merged commit d8b86cb into zephyrproject-rtos:master Jun 23, 2020
@pabigot pabigot deleted the nordic/20200622b branch July 6, 2020 13:31
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: Documentation area: I2C area: Kernel area: Sensors Sensors area: Shell Shell subsystem area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

need API to get list of succeed initialization device or add initialization status flag in struct device
6 participants