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

Improve out-of-tree driver experience #27048

Closed
gmarull opened this issue Jul 22, 2020 · 11 comments
Closed

Improve out-of-tree driver experience #27048

gmarull opened this issue Jul 22, 2020 · 11 comments
Assignees
Labels
area: Build System Enhancement Changes/Updates/Additions to existing features

Comments

@gmarull
Copy link
Member

gmarull commented Jul 22, 2020

Is your enhancement proposal related to a problem? Please describe.

It is not always possible to add an out-of-tree driver for an existing subsystem using the same CMake macros used upstream. The reason for that is that the library created with zephyr_library() macro gets no sources if no upstream driver is enabled (e.g. for kscan). In fact, this is not always true as some subsystems like sensors use zephyr_library() on each driver subdirectory.

Describe the solution you'd like

I'd like to be able to add a new out-of-tree driver for an existing subsystem using equal or similar CMake macros used upstream. That is, assuming the following layout:

$APP_DIR
...
├── drivers
│   ├── CMakeLists.txt
│   ├── Kconfig
│   └── kscan
│       ├── CMakeLists.txt
│       ├── Kconfig
│       ├── Kconfig.newdriver
│       └── kscan_newdriver.c
├── dts
│   └── bindings
│       └── kscan
│           └── vendor,newdriver.yaml
├── Kconfig
├── prj.conf
├── src
...

with the following content in $APP_DIR/drivers/kscan/CMakeLists.txt:

zephyr_library()
zephyr_library_sources_ifdef(CONFIG_KSCAN_NEWDRIVER kscan_newdriver.c)

I think that there should be clear and concise documentation on how to write out-of-tree drivers.

Describe alternatives you've considered

I have tried the following to solve this problem:

  1. Create a new macro, zephyr_ext_library() with:
macro(zephyr_ext_library)
  zephyr_library_get_current_dir_lib_name(${CMAKE_SOURCE_DIR} lib_name)
  set(ZEPHYR_CURRENT_LIBRARY ${lib_name})
endmacro()

Then you have the following content in $APP_DIR/drivers/kscan/CMakeLists.txt:

zephyr_ext_library()
zephyr_library_sources_ifdef(CONFIG_KSCAN_NEWDRIVER kscan_newdriver.c)
  1. Add sources to the target created by zephyr_library() (knowing that library name is built using its path with underscores). You end up with the following content in $APP_DIR/drivers/kscan/CMakeLists.txt:
target_sources(drivers__kscan PRIVATE ${CMAKE_CURRENT_LIST_DIR}/kscan_newdriver.c)

Additional context

N/A

@gmarull gmarull added the Enhancement Changes/Updates/Additions to existing features label Jul 22, 2020
@tejlmand
Copy link
Collaborator

tejlmand commented Aug 6, 2020

Is the existing macro zephyr_library_amend not sufficient ?

https://github.com/zephyrproject-rtos/zephyr/blob/master/cmake/extensions.cmake#L394-L405

@tejlmand
Copy link
Collaborator

tejlmand commented Aug 6, 2020

An example on how this is used for an out-of-tree entropy driver.
https://github.com/nrfconnect/sdk-nrf/blob/master/drivers/entropy/CMakeLists.txt#L6-L7

@gmarull
Copy link
Member Author

gmarull commented Aug 6, 2020

@tejlmand zephyr_library_amend is only available for Zephyr modules, I get:

  Function only available for Zephyr modules.

when used in a structure such as the one listed above.

@tejlmand
Copy link
Collaborator

@tejlmand zephyr_library_amend is only available for Zephyr modules, I get:

  Function only available for Zephyr modules.

when used in a structure such as the one listed above.

right.
That was originally because Zephyr modules should be described inside the Zephyr CMake build tree as part of the Zephyr boilerplate.

That is because there are some code related to processing order that will not work as expected for Zephyr modules defined after find_package(Zephyr).
One example is the --whole-archive that will only include libs defined as part of the boilerplate execution.

Before removing this safeguard in zephyr_library_amend, I would need a little time to go through the usage of Zephyr libs as to ensure there will be no unintended side effects on such change.

Until then, you may define you APP_DIR as a Zephyr module, just for the driver:
https://docs.zephyrproject.org/latest/guides/modules.html
Create ${APP_DIR}/zephyr/module.yml with following content:

build:
  cmake: drivers
  kconfig: drivers/Kconfig

and then remove add_subdirectory(drivers) from ${APP_DIR}/CMakeLists.txt.
If ${APP_DIR} is part of west.yml the drivers folder will automatically be added to the Zephyr build.

Else you may add set(ZEPHYR_EXTRA_MODULES ${CMAKE_CURRENT_LIST_DIR}) to ${APP_DIR}/CMakeLists.txt

@gmarull
Copy link
Member Author

gmarull commented May 17, 2021

Closing this issue, I think example-application covers how to deal with oot drivers.

@gmarull gmarull closed this as completed May 17, 2021
@saacifuentesmu-titoma
Copy link

@gmarull Thank you for the discussion, but the thread seems unconcluded to me.

As of today I still can't use the zephyr_library_amend with my driver.
I have the exact same layout as described by you in the beginning.
Also, using the zds example as reference did not work.
I'm trying to integrate a custom LCD driver to use with the zephyr's display driver.
I always get the No SOURCES given to Zephyr library: drivers__display
Excluding target from build.

Any help is appreciated.

@merethan
Copy link

merethan commented Jan 8, 2024

@saacifuentesmu-titoma the Zephyr build system got very complicated over time. It tries to make everything "just work™" in every situation. To achieve this, a lot of "boilerplate"-code (meaning repetitive, common, standard stuff needed everywhere) has been abstracted away. This convenience however has now became a barrier to people new to the project, as one must first understand this "boilerplate" code, to understand what it is you really get by calling it. Which to a degree defeats its purpose: It is no longer that convenient. (It is to the people that developed it, but not to people that were not around whilst it was made.)

One such example is zephyr_library() being used in each drivers subdirectory: It does a lot of magic behind the scenes, and thus declutters a CMakeLists.txt to a great extend if you write a driver.

What's not immediately obvious is the fact that zephyr_library() is meant to be used inside the Zephyr tree only. Essentially it is called on all of the Zephyr tree once your own CMakeLists.txt hits find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}), and is of little to no use after that point.

So, if you copy an in-tree directory as a guide to go by whilst developing your driver, it will not work outside of the Zephyr tree. If you run it in your own tree (outside of the Zephyr tree) the zephyr_library() is hit after find_package(Zephyr) has completed, and no longer has effect on it: Whatever you want built using zephyr_library() not gets linked and none of its dependencies will be considered because it no longer affects the result of find_package(Zephyr)` and all it magically sets up for you (which is everything).

The solution:
Rather than trying to use zephyr_library() you want to have your driver code build using a Zephyr-thing called "modules": https://docs.zephyrproject.org/latest/develop/modules.html
Modules are Zephyr-lingo for code that's supplied externally, and in this case your local driver could be considered such. The Zephyr build-system has a handy-dandy function for building, including, linking such code:

Not much is required for that. Instead of doing add_subdirectory(drivers) in your root CMakeLists.txt, you do list(APPEND ZEPHYR_EXTRA_MODULES ${CMAKE_CURRENT_SOURCE_DIR}/drivers).
Then, remove zephyr_library() from the CMakeLists.txt files of each driver you are trying to build.
In drivers/CMakeLists.txt, all one needs is add_subdirectory_ifdef(CONFIG_MYDRIVER mydriver) for each driver you have.
Lastly, you create the file drivers/zephyr/module.yaml, which only needs the following:

build:
  cmake: .
  kconfig: Kconfig

And that's all there's to it. By dropping zephyr_library() and having drivers/zephyr/module.yaml with list(APPEND ZEPHYR_EXTRA_MODULES ${CMAKE_CURRENT_SOURCE_DIR}/drivers) instead, to build drivers/mydriver/CMakeLists.txt, drivers/myotherdriver/CMakeLists.txt, drivers/yeatanotherdriver/CMakeLists.txt etc. some other piece of magic "It just works™" is invoked and then your driver code is properly part of the build again.

@gmarull
Copy link
Member Author

gmarull commented Jan 8, 2024

What's not immediately obvious is the fact that zephyr_library() is meant to be used inside the Zephyr tree only.

I'm afraid you're wrong: https://github.com/zephyrproject-rtos/example-application/blob/main/lib/custom_lib/CMakeLists.txt#L4

@merethan
Copy link

merethan commented Jan 8, 2024

Then I don't know why it not worked for me. Neither do I know why what I describe above does work for me. And above all I not know where to read up on it other than "self-documenting" examples.

In summary: I know nothing. The above is best I could make of it, with help from Discord user thetruebell.

@dman82499
Copy link

+1 on this, would be nice to have some documentation explaining all this, the existing examples don't explain how things work to the extent that this thread at least attempted to explain.

@htcrane
Copy link

htcrane commented Apr 18, 2024

@merethan Thank you for your helpful explanation of how to add external out of tree modules! For others wanting to get more info I am going to link the discussion #43650 and the example repo dedicated to out of tree module support for both zephyr and non-zephyr targets.

Updates

  • If you are using Zephyr >= 3.4.0, ZEPHYR_EXTRA_MODULES is replaced by EXTRA_ZEPHYR_MODULES per the release documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Build System Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

No branches or pull requests

6 participants