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.

Sign up for GitHub

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

cmake: dts: print binding dirs and board extension overlays #83533

Closed
wants to merge 2 commits into from
Closed

cmake: dts: print binding dirs and board extension overlays #83533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
99044ca
cmake: dts: print binding dirs and board extension overlays
jelledevleeschouwer Jan 3, 2025
b0899dc
doc: dts: add note about BOARD_EXTENSION_DIRS
jelledevleeschouwer Jan 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cmake/modules/dts.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,10 @@ endif()
# with them.
#

foreach(board_extension_dir ${BOARD_EXTENSION_DIRS})
message(STATUS "Found board extension dir: ${board_extension_dir}")
endforeach()

zephyr_file(CONF_FILES ${BOARD_EXTENSION_DIRS} DTS board_extension_dts_files)

set(dts_files
Expand Down Expand Up @@ -222,6 +226,7 @@ unset(DTS_ROOT_BINDINGS)
foreach(dts_root ${DTS_ROOT})
set(bindings_path ${dts_root}/dts/bindings)
if(EXISTS ${bindings_path})
message(STATUS "Found DTS bindings dir: ${bindings_path}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's always challenging to decide exactly what should be printed.
We try to limit the output CMake status output to just exact files or tools included in build as those are those are most important in order to quickly discover if a wrong file (conf, dts, etc.) or version of a given tool (compiler, python, etc) is picked up, as this quickly helps users to discover when something is wrong, like env variable, paths, and the like.

You may say paths can be in similar category, but there can be a lot of paths being searched and if we start to print all of those, including those being provided by Zephyr modules, then we quickly get a lot of lines that makes it harder to quickly spot the more important information mentioned above.

If you want to see which paths were actually considered for the build, then you should take a look at <build>/build_info.yml which contains a lot more information regarding both files and paths used during the CMake and build stages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I see. build_info.yml seems to be fairly recent (October 8, 2024) and doesn't seem to have made it into my downstream fork (nrfconnect/[email protected]) yet, so I haven't tried it. Currently, we're upstepping to nRF Connect 2.9.0, so then I can re-evaluate. For that reason, I will close this.

Most of my issues related to devicetree generation have been due to malformed file paths, forgetting the bindings or dts in dts/bindings, for example. I am a big proponent of the "The Rule of Silence" posix philosophy, but discarding devicetree entries because the binding for a node could not be found (in all the paths where it goes looking for them), I consider particularly noteworthy. So in that philosophy, that would be the ideal moment to print the binding paths. But I am also pragmatic, so without dwelling too much into the devicetree generation code, this would be, I thought, the next best opportunity.

My apologies for any inconvenience this PR may have caused and thanks a lot for your valuable feedback and time 🙂

Regards,
Jelle

Copy link
Collaborator

Choose a reason for hiding this comment

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

but discarding devicetree entries because the binding for a node could not be found

yes, this I agree too sounds like something which should probably be printed.
Discarding an entry without informing the user is in general never nice.

I'm unsure the reason for such silent discard, but I this PR (or an alternative) can be made which informs users when entries are discarded, then that sounds like a nice improvements.

Alternative, create an enhancement issue, as that can have other people providing feedback as to why this is silently ignored.

My apologies for any inconvenience this PR

The PR had no inconveniences, only the description of the PR wasn't exactly clear that entries was silently discarded.
You surely have found something worth improving 👍

list(APPEND
DTS_ROOT_BINDINGS
${bindings_path}
Expand Down
12 changes: 12 additions & 0 deletions doc/build/dts/howtos.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,18 @@ finds in the configuration phase, like this:

-- Found devicetree overlay: .../some/file.overlay

Additional directories containing devicetree overlay files can also be added
by appending to the ``BOARD_EXTENSION_DIRS`` CMake variable in the application
Copy link
Contributor

Choose a reason for hiding this comment

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

BOARD_EXTENSION_DIRS is an internal variable, so it shouldn't be mentioned here.
In case you haven't seen it, we have a documented way to add a second board directory:
https://docs.zephyrproject.org/4.0.0/hardware/porting/board_porting.html#board-extensions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I saw it when I was investigating the BOARD_EXTENSION_DIRS variable indeed. It seemed a bit overcomplicated to me, I don't want make any new variant with new qualifiers, I just want to extend a board with the same name. I also think it cannot do any harm by supporting BOARD_EXTENSION_DIRS more openly. It is still possible anyway, why not document it so people can use it correctly?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't want make any new variant with new qualifiers

I understand, but we require this because we don't want the original board description to be "quietly" replaced or overridden just by including an external module in the workspace. We also don't want potential conflicts from multiple modules trying to add their own overlays etc.

It is still possible anyway, why not document it so people can use it correctly?

In this case, it's because the variable is tied to the old board extension feature, which is deprecated and will be removed together with HWMv1.

But note that the use case you alluded to in this file - setting BOARD_EXTENSION_DIRS before find_package(Zephyr) - is also possible in ways that we openly support, e.g., by using another variable like EXTRA_DTC_OVERLAY_FILE, or by adding a project-specific boards/<normalized_board_target>.overlay.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The use case that I am dealing with is that I have the following project structure where I want to reuse dts/bindings and boards directory only from a directory outside of the application directory (which is in fact located inside a tests subdirectory of the app directory itself):

app/
├─── src/
│    └─── ...
├─── tests/
│    └─── stripped_app/
│         ├─── sysbuild/...
│         ├─── CMakeLists.txt
│         ├─── Kconfig
│         ├─── Kconfig.sysbuild
│         ├─── prj.conf
│         └─── sysbuild.conf
├─── boards/
│    ├─── redacted.conf
│    ├─── redacted.overlay
│    ├─── ...
│    ├─── nrf5340dk_nrf5340_cpuapp.conf
│    └─── nrf5340dk_nrf5340_cpuapp.overlay
├─── dts/bindings
│    └─── binding.yml
├─── sysbuild/...
├─── CMakeLists.txt
├─── Kconfig
├─── Kconfig.sysbuild
├─── prj.conf
├─── sysbuild.conf
└─── west.yml

Essentially, I want to be able to specify where that project-specific boards/ directory is and not be restricted to CMAKE_CURRENT_SOURCE_DIR. If I have to refactor to board extension format that I can tolerate but the former is where I mostly care about. Can I do that with BOARD_ROOT and board extensions?

Copy link
Contributor

Choose a reason for hiding this comment

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

It sounds like you could use APPLICATION_CONFIG_DIR here, and/or some combination of CONF_FILE, DTC_OVERLAY_FILE, DTS_ROOT, KCONFIG_ROOT. Extending the board should not be necessary.

:file:`CMakeLists.txt` file. Make sure to do so **before** pulling in the
Zephyr boilerplate with ``find_package(Zephyr ...)``.

.. note::

When specifying ``BOARD_EXTENSION_DIRS`` in a CMakeLists.txt, then an absolute path must
be provided, for example ``list(APPEND BOARD_EXTENSION_DIRS ${CMAKE_CURRENT_SOURCE_DIR}/<extension-dir>``.
When using ``-DBOARD_EXTENSION_DIRS=<extension-dir>`` both absolute and relative paths can be
used. Relative paths are treated relatively to the application directory.

.. _use-dt-overlays:

Use devicetree overlays
Expand Down
Loading