device: allow lookup by name without requiring device be ready

[pabigot]

device_get_binding() combines looking up a device structure by name
and verifying that the device is ready for use (originally that it had
been successfully initialized).

With a move to DEVICE_DT_GET() supporting retrieval of the device
reference at build-time we've added a separate function to verify that
the device is ready to be used.

Formerly the ready-to-use check was incorrect in that it indicated
readiness before the device had been initialized, and initialization
could fail. This has been fixed, but it means that drivers that had
successfully retrieved a device by name before initialization but
didn't try to use it right away are now are getting a null device
pointer.

Support these capabilities by adding new API that matches
DEVICE_DT_GET() in simply retrieving the device object pointer without
validation.

Signed-off-by: Peter Bigot [email protected]

[pabigot]

This enhancement plus:

--- a/subsys/shell/shell_uart.c
+++ b/subsys/shell/shell_uart.c
@@ -290,7 +290,7 @@ static int enable_shell_uart(const struct device *arg)
 {
        ARG_UNUSED(arg);
        const struct device *dev =
-                       device_get_binding(CONFIG_UART_SHELL_ON_DEV_NAME);
+                       device_from_name(CONFIG_UART_SHELL_ON_DEV_NAME);
        bool log_backend = CONFIG_SHELL_BACKEND_SERIAL_LOG_LEVEL > 0;
        uint32_t level =
                (CONFIG_SHELL_BACKEND_SERIAL_LOG_LEVEL > LOG_LEVEL_DBG) ?

"fixes" #32581.

To the degree putting a device into a power-down mode makes it unready other API that might retrieve an instance by name would need the non-checked API in this.

Draft to allow people to get used to this idea, mediate it against the remove of label properties which makes this feature unuseful, and to invite opportunities to pick paint colors.

[mbolivar-nordic]

Looks good. One non-blocking suggestion for a docstring.

[mbolivar-nordic]

This paragraph is starting to feel like it's doing too many things. The antecedent for "this" in "this means" was unclear to me after the discussion of all the different APIs that "this" function (which is the antecedent in "this can be used" earlier) is related to. How about splitting it up:

Suggested change

	* device_get_binding(). At minimum this means that the device has been
	* successfully initialized, but it may take on further conditions (e.g. is
	* device_get_binding().
	*
	* At minimum, "ready" means that the device has been successfully initialized,
	* but it may take on further conditions (e.g. is

[pabigot]

done, thanks.

[eanderlind]

Replacing device_get_binding() with a function returning an error code (like Linux match() ) would be useful, so can distinguish "not ready" and "doesn't exist". Otherwise unclear what a good idiom is for a caller to wait for a device to initialize. Encountered this (v2.4) where main tread early on calls device_get_binding() for a board LED that is driven by PWM driver that wasn't yet fully initialized - got intermittent board startup failures. Alternatively, could deprecate this function, forcing to always use DEVICE_DT_GET and ensure all driver api calls return an -EREADY or similar code so caller can retry. Deciding how long to wait e.g for a sensor that is dependent on other drivers is also tricky.

[pabigot]

Rebased to resolve merge conflict.

[nashif]

where is this being used?

[pabigot]

This is provided so driver init functions and other code already known to be running in privileged mode doesn't have to go through a system call.

[anangl]

But it looks like there is no implementation of this function. I think you need to add something like:

const struct device *z_device_from_name(const char *name)
{
	return device_lookup_by_name(name, false);
}

in device.c.

[pabigot]

Yes, you're right.

[nashif]

device_lookup_by_name is better suited to be the syscall and device_from_name the static function.

[pabigot]

But we already have device_from_handle, so it's more consistent to do it this way.

[pabigot]

This isn't complete, and since I'm not going to use it I think it's best to not keep hacking at it but rather leave it orphaned and let somebody who thinks this functionality has a role in future Zephyr finish it up, or drop it.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.