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

llext: export DT devices to extensions #78508

Merged
merged 5 commits into from
Sep 25, 2024

Conversation

pillo79
Copy link
Collaborator

@pillo79 pillo79 commented Sep 16, 2024

This PR adds support for automatic export of all Zephyr devices to extensions via a new Kconfig option, CONFIG_LLEXT_EXPORT_DEVICES. When this option is enabled, all Devicetree macros in zephyr/device.h can be used in extensions as well to inspect and interact with the devices configured in the current build.

Calling EXPORT_SYMBOL with anything more complicated than a simple identifier resulted in macro expansion issues. This has been fixed by adding a level of indirection, moving the actual symbol definition code to private Z_ macros. A number of related comments have been updated.

New test code has been added to properly verify the new support. The test retrieves the device object associated with the Zephyr console via DT_CHOSEN macros and verifies it matches with the one obtained at runtime by calling device_get_binding(). This check is currently unsupported on Xtensa since the qemu_xtensa platform defines no usable DT devices.

Also fixes 2 minor issues in device.h:

  • The error message for Z_DEVICE_NAME_CHECK included a call to DEVICE_NAME_GET, but the argument was already a string. This resulted in a confusing error message being displayed when the check failed.
  • There was a misleading comment that implied the name returned by Z_DEVICE_DT_DEV_ID was used in device_get_binding() instead of the DT path, and must thus be limited to Z_DEVICE_MAX_NAME_LEN chars.
    This is not the case (anymore?): device_get_binding() matches neither paths nor identifiers, but either DT labels or DT full names as returned by the DEVICE_DT_NAME macro here.

The error message for Z_DEVICE_NAME_CHECK was needlessly calling
DEVICE_NAME_GET() on the "name" string, resulting in a confusing message
being displayed.

Remove the DEVICE_NAME_GET call so that the error message shown by the
compiler contains the actual name.

Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
@pillo79 pillo79 force-pushed the pr-llext-export-dev branch 2 times, most recently from 031a747 to 0d1c9a1 Compare September 17, 2024 07:22
@pillo79 pillo79 marked this pull request as ready for review September 17, 2024 08:36
@zephyrbot zephyrbot added area: llext Linkable Loadable Extensions area: Device Model labels Sep 17, 2024
The comment for Z_DEVICE_DT_DEV_ID() is misleading: device_get_binding()
compares its inputs with values generated by the DEVICE_DT_NAME() macro,
which expands to the node's DT label or to the node's full name.

This patch updates the comment to reflect the actual behavior of
Z_DEVICE_DT_DEV_ID().

Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
@fabiobaltieri
Copy link
Member

@gmarull can you take a look?

include/zephyr/device.h Show resolved Hide resolved
@@ -197,7 +207,8 @@ typedef int16_t device_handle_t;
DEVICE_DT_NAME(node_id), init_fn, pm, data, config, \
level, prio, api, \
&Z_DEVICE_STATE_NAME(Z_DEVICE_DT_DEV_ID(node_id)), \
__VA_ARGS__)
__VA_ARGS__); \
Z_DEVICE_EXPORT(node_id)
Copy link
Member

Choose a reason for hiding this comment

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

you can use IF_ENABLED(CONFIG_LLEXT_EXPORT_DEVICES, (EXPORT_SYMBOL(DEVICE_DT_NAME_GET(node_id))))

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 could, but the separate macro is easier to extend in the (short) future; I have a draft of a more complex proposal (#77799) where there are two different implementations for Z_DEVICE_EXPORT.
For this reason I'm proposing to keep the separate macro, but included within an IF_ENABLED in the parent.

include/zephyr/device.h Outdated Show resolved Hide resolved
Comment on lines 103 to 110
#if defined(CONFIG_LLEXT_EXPORT_DEVICES)
/* Export device identifiers using the builtin name */
#define Z_DEVICE_EXPORT(node_id) EXPORT_SYMBOL(DEVICE_DT_NAME_GET(node_id))
#else
/* Do not export device identifiers */
#define Z_DEVICE_EXPORT(node_id)
#endif

Copy link
Member

Choose a reason for hiding this comment

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

see my other comment

include/zephyr/device.h Outdated Show resolved Hide resolved
@@ -197,7 +207,8 @@ typedef int16_t device_handle_t;
DEVICE_DT_NAME(node_id), init_fn, pm, data, config, \
level, prio, api, \
&Z_DEVICE_STATE_NAME(Z_DEVICE_DT_DEV_ID(node_id)), \
__VA_ARGS__)
__VA_ARGS__); \
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't trailing ; be conditioned to the next statement? if not, move export above.

This patch adds an indirection level moving the current code from inside
the EXPORT_SYMBOL and LL_EXTENSION_SYMBOL macros to private Z_* macros.
This is done to allow for the official APIs to properly expand more
complex arguments, such as the DT function-like macros.

Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
This change adds a new configuration option, LLEXT_EXPORT_DEVICES, which
enables exporting all devices defined in the device tree to llext
extensions. When enabled, all devices are made available to extensions
via the standard DT_ / DEVICE_* macros.

Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
Add a new test to the simple llext test suite that verifies that DT
devices are properly exported to extensions. This is done by obtaining a
device handle from the DT and then trying to get the same handle at
runtime using the device_get_binding() API.

This test is optional because the current qemu_xtensa platform does not
have any usable DT devices to test with.

The LLEXT heap size is also increased to 64 kbytes to avoid heap
exhaustion when running the new test on mps2_an385.

Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
@pillo79
Copy link
Collaborator Author

pillo79 commented Sep 24, 2024

v2:

  • new proposal for Z_DEVICE_EXPORT
  • implemented all other notes by @gmarull
  • renamed new private macros from macro_IMPL to Z_macro to keep in line with Zephyr internals

Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

ack device changes

@pillo79 pillo79 deleted the pr-llext-export-dev branch October 2, 2024 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Device Model area: llext Linkable Loadable Extensions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants