[RFC] Deferred device initialization #67335

edersondisouza · 2024-01-08T18:27:08Z

Following discussions at #64869, this PR implements Option 1 from #39896.

With this PR, a new devicetree property, zephyr,deferred-init can be used to make a device not be initialised during boot. The device can be initialised by calling device_init() from the application.

Under the hood, this PR achieves that by grouping the deferred devices intialisation code in a different ELF section. This way, there's no overhead during normal initialisation: neither memory nor processing penalty. Deferred initialisation however loops over the deferred devices and initialises the matching one, incurring some processing penalty, which is hopefully acceptable.

A new test helps showcase the new feature.

Opens

Dependencies are not handled. If a non-deferred device depends on a deferred one, its initialisation will simply fail. We could A) issue warnings/errors during build or B) forcefully make all dependencies also be deferred.
Is the processing penalty to initialise a deferred device acceptable? If not, one thing I can think is to expose the initialiser function as non-static and providing a macro to get it and call it directly (think of DEVICE_INIT(DT_PATH(some_node))), but this seems way too ugly. Any ideas?
This PR doesn't check if a deferred device is initialised more than once - it will simply re-run the code. My reasoning is that Zephyr currently marks a device as initialised if initialisation was attempted, not if it succeeded. This behaviour is kept for deferred devices for the sake of consistency. But if someone wants to retry initialise some device, checking for attempted initialisation would prevent that.
This PR doesn't attempt to tackle the "device de-init" problem - only selective device init.

gmarull · 2024-01-10T11:19:40Z

include/zephyr/device.h

@@ -149,6 +149,8 @@ typedef int16_t device_handle_t;
 #define DEVICE_DT_NAME(node_id)                                                \
 	DT_PROP_OR(node_id, label, DT_NODE_FULL_NAME(node_id))

+#define DEVICE_DT_DEFER(node_id)                                               \
+	DT_PROP_OR(node_id, zephyr_deferred_init, false)


DT_PROP(node_id, zephyr_deferred_init) (boolean prop, defaults to 0 if not defined)

gmarull · 2024-01-10T11:20:38Z

kernel/init.c

+	for (entry = __deferred_init_start; entry <= __deferred_init_end; entry++) {
+		if (entry->dev == dev) {
+			return do_device_init(entry);
+		}
+	}


please use iterable sections

Yes - I can't remember the exact macro off the top of my head ITERABLE_SECTION_FOR_EACH() maybe? There is likely an ALT version that should allow for pointer aliasing if necessary

andyross

Clever and useful. Dropping a +1 even though it'll probably respin for the macrobatic review comments.

edersondisouza · 2024-01-10T20:56:39Z

v2:

Use STRUCT_SECTION_FOREACH_ALTERNATE to loop over init entries;
DT_PROP instead of DT_PROP_OR;
A bit more docs and style fixes.

nashif · 2024-01-10T21:24:16Z

include/zephyr/device.h

@@ -149,6 +149,16 @@ typedef int16_t device_handle_t;
 #define DEVICE_DT_NAME(node_id)                                                \
 	DT_PROP_OR(node_id, label, DT_NODE_FULL_NAME(node_id))

+/**
+ * @brief Deterimine if a devicetree node initialization should be deferred.


tbursztyka · 2024-04-05T06:28:52Z

@tbursztyka, @cfriedt do you have more comment about this approach vs #67877? Did you have a chance to take a look at the more recent discussion at #39896?

I still think it's not the right way. Whether it's a callback and/or a dts property (which is worse than the cb imo), because of the dependencies, because of shortcomings in the device driver model. But I won't block, there seems to be a consensus already.

pillo79 · 2024-04-08T12:18:20Z

This proposal is pretty close to what I had mocked up for the Arduino-on-Zephyr PoC last year, but properly done - so it definitely gets my preference! 👍

While I also appreciate the extra runtime flexibilty given by the alternative PR, I think the subtle issues arising with such an early state callback (as nicely detailed in this comment's second point), are pretty difficult to convey to users.

dts/bindings/base/base.yaml

include/zephyr/device.h

tests/kernel/device/boards/hifive_unmatched.overlay

carlescufi · 2024-04-09T15:21:04Z

Architecture WG:

Consensus is moving into the direction of using this approach
No current objections to this approach

Currently, all devices are initialized at boot time (following their level and priority order). This patch introduces deferred initialization: by setting the property `zephyr,deferred-init` on a device on the devicetree, Zephyr will not initialized the device. To initialize such devices, one has to call `device_init()`. Deferred initialization is done by grouping all deferred devices on a different ELF section. In this way, there's no need to consume more memory to keep track of deferred devices. When `device_init()` is called, Zephyr will scan the deferred devices section and call the initialization function for the matching device. As this scanning is done only during deferred device initialization, its cost should be bearable. Signed-off-by: Ederson de Souza <[email protected]>

Ensure that devices are not ready before calling `device_init()`, but are after the call. Signed-off-by: Ederson de Souza <[email protected]>

edersondisouza · 2024-04-10T16:19:53Z

v4:

Fix typo;
Fix documentation quotes;
Rebased.

gmarull

Not entirely happy with the result, but let's move on.

edersondisouza force-pushed the defer-init branch 3 times, most recently from 4d9ed48 to cea07b7 Compare January 9, 2024 18:02

edersondisouza marked this pull request as ready for review January 10, 2024 04:52

zephyrbot added area: Linker Scripts area: Device Model area: Devicetree Binding PR modifies or adds a Device Tree binding area: Kernel labels Jan 10, 2024

zephyrbot requested review from andyross, ceolin, cfriedt, dcpleung, galak, gmarull, nashif, npitre, peter-mitsis and tbursztyka January 10, 2024 04:53

zephyrbot assigned gmarull Jan 10, 2024

gmarull requested changes Jan 10, 2024

View reviewed changes

edersondisouza force-pushed the defer-init branch from cea07b7 to 81fea91 Compare January 10, 2024 19:37

andyross previously approved these changes Jan 10, 2024

View reviewed changes

edersondisouza requested a review from gmarull January 10, 2024 20:56

npitre previously approved these changes Jan 10, 2024

View reviewed changes

nashif reviewed Jan 10, 2024

View reviewed changes

edersondisouza dismissed stale reviews from npitre and andyross via 7361202 January 11, 2024 00:09

edersondisouza force-pushed the defer-init branch from 81fea91 to 7361202 Compare January 11, 2024 00:09

edersondisouza requested a review from nashif January 11, 2024 00:22

jhedberg previously approved these changes Apr 5, 2024

View reviewed changes

gmarull previously approved these changes Apr 9, 2024

View reviewed changes

nashif added DNM This PR should not be merged (Do Not Merge) and removed DNM This PR should not be merged (Do Not Merge) Architecture Review Discussion in the Architecture WG required labels Apr 9, 2024

henrikbrixandersen requested changes Apr 9, 2024

View reviewed changes

dts/bindings/base/base.yaml Outdated Show resolved Hide resolved

include/zephyr/device.h Outdated Show resolved Hide resolved

tests/kernel/device/boards/hifive_unmatched.overlay Outdated Show resolved Hide resolved

edersondisouza added 2 commits April 10, 2024 08:11

tests/kernel/device: Add tests for device deferred init

ae1af21

Ensure that devices are not ready before calling `device_init()`, but are after the call. Signed-off-by: Ederson de Souza <[email protected]>

edersondisouza dismissed stale reviews from gmarull, jhedberg, nashif, cfriedt, and npitre via ae1af21 April 10, 2024 16:19

edersondisouza force-pushed the defer-init branch from 3e054b7 to ae1af21 Compare April 10, 2024 16:19

zephyrbot requested a review from TaiJuWu April 10, 2024 16:19

nashif requested a review from henrikbrixandersen April 10, 2024 16:20

nashif approved these changes Apr 10, 2024

View reviewed changes

edersondisouza requested review from npitre, jhedberg, cfriedt and gmarull April 10, 2024 16:25

gmarull approved these changes Apr 11, 2024

View reviewed changes

henrikbrixandersen approved these changes Apr 11, 2024

View reviewed changes

nashif merged commit 2febacc into zephyrproject-rtos:main Apr 11, 2024
30 checks passed

fabiobaltieri mentioned this pull request Apr 22, 2024

GNSS: minor improvements #71774

Merged

pdgendt mentioned this pull request May 7, 2024

cmake: linker_script: Fix using deferred init with CMAKE_LINKER_GENERATOR #72369

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] Deferred device initialization #67335

[RFC] Deferred device initialization #67335

edersondisouza commented Jan 8, 2024

gmarull Jan 10, 2024

gmarull Jan 10, 2024

cfriedt Jan 10, 2024

andyross left a comment

edersondisouza commented Jan 10, 2024

nashif Jan 10, 2024

tbursztyka commented Apr 5, 2024

pillo79 commented Apr 8, 2024

carlescufi commented Apr 9, 2024

edersondisouza commented Apr 10, 2024

gmarull left a comment

[RFC] Deferred device initialization #67335

[RFC] Deferred device initialization #67335

Conversation

edersondisouza commented Jan 8, 2024

Opens

gmarull Jan 10, 2024

Choose a reason for hiding this comment

gmarull Jan 10, 2024

Choose a reason for hiding this comment

cfriedt Jan 10, 2024

Choose a reason for hiding this comment

andyross left a comment

Choose a reason for hiding this comment

edersondisouza commented Jan 10, 2024

nashif Jan 10, 2024

Choose a reason for hiding this comment

tbursztyka commented Apr 5, 2024

pillo79 commented Apr 8, 2024

carlescufi commented Apr 9, 2024

edersondisouza commented Apr 10, 2024

gmarull left a comment

Choose a reason for hiding this comment