cmake: dts: import devicetree symbols into CMake #37624

tejlmand · 2021-08-11T19:32:36Z

This commit introduces devicetree API in CMake so that devicetree
properties and register block can be used in the CMake build system.

The script scripts/dts/dts_cmake.py processes the edt.pickle and
generates a corresponding devicetree property map in a devicetree_target
that is then used in CMake.

The following devicetree API has been made available in Zephyr CMake:

- dt_nodelabel(<var> LABEL <label>)
- dt_node_exists(<var> NODE <node>)
- dt_node_has_status(<var> NODE <node> STATUS <status>)
- dt_prop(<var> NODE <node> PROPERTY <prop>)
- dt_prop(<var> NODE <node> INDEX <idx> PROPERTY <prop>)
- dt_num_regs(<var> NODE <node>)
- dt_reg_addr(<var> NODE <node> [INDEX <idx>])
- dt_reg_size(<var> NODE <node> [INDEX <idx>])
- dt_has_chosen(<var> PROPERTY <prop>)
- dt_chosen(<var> PROPERTY <prop>)

Signed-off-by: Marti Bolivar [email protected]
Signed-off-by: Torsten Rasmussen [email protected]

This is needed by: #36140

galak · 2021-08-11T22:38:04Z

cmake/extensions.cmake

@@ -2261,3 +2263,369 @@ function(target_byproducts)
                     COMMENT "Logical command for additional byproducts on target: ${TB_TARGET}"
  )
 endfunction()
+
+########################################################


can we split this out into its own file.

we could, but are we sure that we want to do that ?

Today, whenever one sees a CMake function or macro in use inside Zephyr build system that is not vanilla CMake, then there is one place to look, and that is cmake/extensions.cmake.

That makes it easy to lookup extension functions.

Only exception are areas where functions are used internally, such as: cmake/flash/CMakeLists.txt but those functions are not use outside that file, afaik.

We could of course establish a new scheme, like:

cmake/extensions/dt-extensions.cmake

cmake/extensions/library-extensions.cmake

for library related extensions, like zephyr_library(), zephyr_library_sources, zephyr_sources, ...

cmake/extensions/generic-extensions.cmake

for generic CMake functions with a twist, like target_sources_ifdef(), add_subdirectory_ifdef().

cmake/extensions/misc-extensions.cmake

There always needs to be a misc.

cmake/extensions/<...>-extensions.cmake

but doing such split (which would be outside the scope of this PR) will sometimes make it harder to know where a given function belongs (most functions will be easy, but not all).

Having only devicetree functions in a separate file just makes it inconsistent where functions are placed, imho.

If it is because you believe this feature could be reusable outside Zephyr scope, like with linker generator, then I see the point and a use-case.

the thought was that this could be usable outside of zephyr.

the thought was that this could be usable outside of zephyr.

makes sense, but those functions are useless without the generated <build>/zephyr/dts.cmake file which is created here https://github.com/zephyrproject-rtos/zephyr/blob/aecb7fc1b488ac3d017447d324b1e78e793f7620/scripts/dts/dts_cmake.py based on the edt.pickle file, https://github.com/zephyrproject-rtos/zephyr/blob/aecb7fc1b488ac3d017447d324b1e78e793f7620/scripts/dts/dts_cmake.py#L34

The edt.pickle file is created in the gen_defines.py script here:

zephyr/scripts/dts/gen_defines.py

Line 929 in 33d5be8

pickle.dump(edt, f, protocol=4)

so just placing those dt_* functions in its own CMake file doesn't really make them reusable.

If we really want to make it easier to reuse the devicetree --> CMake infrastructure outside Zephyr, then perhaps we should see if this could be streamlined with reusability in mind for all the parts involved:

<zephyr>/cmake/dts.cmake

<zephyr>/scripts/dts/dts_cmake.py

<zephyr>/scripts/dts/gen_defines.py

<zephyr>/cmake/extensions.cmake, devicetree part only.

That sounds very reasonable to me, but I think it would be outside the scope of this PR.

I agree with @tejlmand . If someone wants to use it outside of Zephyr they can pick up the patch at that point and make it do what they need. I'm not sure it's really necessary for this PR.

galak · 2021-08-13T16:50:49Z

How do we feel about use of the to select files to build?

For example something like:

drivers/gpio/Kconfig.psoc6 that defines GPIO_PSOC6

and drivers/gpio/CMakeLists.txt could go from:

zephyr_library_sources_ifdef(CONFIG_GPIO_PSOC6 gpio_psoc6.c)

to

zephyr_library_sources_dt_compat_enabled("cypress,psoc6-gpio" gpio_psoc6.c)

galak · 2021-08-13T16:56:57Z

Is this used for anything other than zephyr,dtcm and zephyr,itcm in the linker PR?

tejlmand · 2021-08-13T19:26:55Z

Is this used for anything other than zephyr,dtcm and zephyr,itcm in the linker PR?

also here in linker PR:
https://github.com/zephyrproject-rtos/zephyr/blob/aa4f08e1220b1c5f53ad5b8bff87ae89730286e7/cmake/extensions.cmake#L2818-L2836
which is then used here:
https://github.com/zephyrproject-rtos/zephyr/blob/aa4f08e1220b1c5f53ad5b8bff87ae89730286e7/cmake/linker_script/arm/linker.cmake#L40-L49

but besides that, there have previous been requests regarding devicetree symbols in CMake.

tejlmand · 2021-08-13T19:34:16Z

How do we feel about use of the to select files to build?

Personal I think that would cleanup the code not having the intermediate Kconfig setting for enabling the driver, but when looking at the Kconfig:

zephyr/drivers/gpio/Kconfig.psoc6

Lines 7 to 9 in 33d5be8

    
           config GPIO_PSOC6 
        
           	bool "Cypress PSoC-6 GPIO driver" 
        
           	default y if $(dt_compat_enabled,$(DT_COMPAT_CYPRESS_PSOC6_GPIO))

then the symbol does have a prompt, although with default y

So today it is possible to have dt_compat_enabled for cypress,psoc6-gpio and CONFIG_GPIO_PSOC6=n which would disable the sw-driver.
I don't know the exact usecase for that, but it does allow users to provide their own driver which would not be possible if we include the driver based directly on devicetree properties.

mbolivar-nordic · 2021-08-16T21:23:49Z

I don't know the exact usecase for that, but it does allow users to provide their own driver which would not be possible if we include the driver based directly on devicetree properties.

This is important to users; see e.g. #10621 (comment) from @henrikbrixandersen

mbolivar-nordic

As some background, the devicetree.h API in C uses "node identifier" as a precise term for the magic macros we use to refer to nodes, but we only have those because we cannot refer to a node by its path.

However, in CMake, we can refer to nodes by their paths, so I don't think the term "node identifier" should be used in this context. I would rather preserve the term "node identifier" for just the magic macro you see in C. That will be less ambiguous. In CMake, we can just take a path or alias when the user wants to tell us what node they want instead.

mbolivar-nordic · 2021-08-16T21:38:03Z

How do we feel about use of the to select files to build?

Deciding whether or not to enable an entire driver should be left user-settable via Kconfig as I mentioned earlier.

However there are use cases, like here:

zephyr/drivers/sensor/bme280/bme280_i2c.c

Line 15 in 190532b

#if BME280_BUS_I2C

When CONFIG_BME280=y, I still only want to build bme280_i2c.c if there is an instance of the compatible on an I2C bus.

So if I could do something like this:

if (CONFIG_BME280)
  dt_compat_on_bus_status_okay(result COMPATIBLE bosch,bme280 BUS i2c)
  if (result)
    zephyr_library_sources(bme280_i2c.c)
  endif()
  dt_compat_on_bus_status_okay(result COMPATIBLE bosch,bme280 BUS i2c)
  if (result)
    zephyr_library_sources(bme280_i2c.c)
  endif()
endif()

Then I think that would be a little cleaner than the current ifdef in the driver source.

Edit: or even something like this:

zephyr_library_sources_if_compat_on_bus(bme280_i2c.c COMPATIBLE bosch,bme280 BUS i2c)
zephyr_library_sources_if_compat_on_bus(bme280_spi.c COMPATIBLE bosch,bme280 BUS spi)

tejlmand · 2021-08-18T10:02:20Z

However, in CMake, we can refer to nodes by their paths, so I don't think the term "node identifier" should be used in this context. I would rather preserve the term "node identifier" for just the magic macro you see in C. That will be less ambiguous. In CMake, we can just take a path or alias when the user wants to tell us what node they want instead.

Good point, changed node identifier --> node path.

tejlmand · 2021-08-18T10:13:07Z

Then I think that would be a little cleaner than the current ifdef in the driver source.

Edit: or even something like this:

Agree, let's look into the cleanest way to do this after the basic support for devicetree has been merged (This PR).
Initial feeling is that a 40 chars long function call is a bit long, and because you would end-up line wrapping anyway, then this might be just as clean, imho:

dt_compat_on_bus_status_okay(result COMPATIBLE bosch,bme280 BUS i2c)
zephyr_library_sources_ifdef(result bme280_i2c.c)

Remember, we already have the zephyr_library_sources_ifdef() function.

cmake/extensions.cmake

galak

See inline comments:

General comments:

cleanup docs to clarify return values for true/false cases as well as on "errors" (ie path doesn't exist, prop doesn't exist, etc)
replace NODE with PATH

mbolivar-nordic

I generally agree with Kumar's cleanup suggestions, with a couple of other ideas and additional comments inline. Thanks.

cmake/dts.cmake

cmake/extensions.cmake

mbolivar-nordic · 2021-08-19T19:10:24Z

cmake/extensions.cmake

+endfunction()
+
+# Usage:
+#   dt_node_exists(<var> NODE <node>)


I think dt_node_exists is a good name for the function. It should take PATH <path> for now, but we can extend it later to also accept ALIAS <alias>, NODELABEL <label>, etc. without changing the name. I think that would be nicer than having separate dt_alias_exists ,dt_nodelabel_exists, etc.

cmake/extensions.cmake

scripts/dts/dts_cmake.py

cmake/dts.cmake

mbolivar-nordic · 2021-08-25T02:21:50Z

@tejlmand I pushed a fixup branch here:

https://github.com/mbolivar-nordic/zephyr/tree/dts_cmake

Please fetch, review, and squash if it looks OK with you. See commit logs for justifications.

If we're on the same page about all of those, I'm +1 to get this in.

mbolivar-nordic · 2021-08-25T02:39:58Z

If we're on the same page about all of those, I'm +1 to get this in.

And if I've made any terrible mistakes, please let me know 😄

tejlmand · 2021-08-25T07:34:25Z

If we're on the same page about all of those, I'm +1 to get this in.

And if I've made any terrible mistakes, please let me know 😄

Not a mistake, but placed a comment here: mbolivar-nordic@d3b44e2#r55395442
which allows us to handle the edge case without a lot of extra code.

tejlmand · 2021-08-25T08:02:28Z

Please fetch, review, and squash if it looks OK with you. See commit logs for justifications.

Thanks a lot, I have taken in your changes here: https://github.com/zephyrproject-rtos/zephyr/compare/f0cae1d28edafda85345fe24de6e37dc64c0063c..736dc0b1edc516de74568881ca44c7f484e0a79c

and then made a small update according to my comments here:
https://github.com/zephyrproject-rtos/zephyr/compare/7dbfc0094d73a90822d69041454792573cce6e0a..55ac7d859a1380cbc66b2379aa1a895863cc8937

mbolivar-nordic

Looks good now, thanks.

mbolivar-nordic · 2021-08-25T15:51:45Z

I think we should dismiss @galak's review as "addressed" to get this in for feature freeze.

This commit introduces devicetree API in CMake so that devicetree properties and register block can be used in the CMake build system. The script scripts/dts/gen_dts_cmake.py processes the edt.pickle and generates a corresponding devicetree property map in a devicetree_target that is then used in CMake. The following devicetree API has been made available in Zephyr CMake: - dt_nodelabel(<var> NODELABEL <label>) - dt_node_exists(<var> PATH <path>) - dt_node_has_status(<var> PATH <path> STATUS <status>) - dt_prop(<var> PATH <path> PROPERTY <prop>) - dt_prop(<var> PATH <path> INDEX <idx> PROPERTY <prop>) - dt_num_regs(<var> PATH <path>) - dt_reg_addr(<var> PATH <path> [INDEX <idx>]) - dt_reg_size(<var> PATH <path> [INDEX <idx>]) - dt_has_chosen(<var> PROPERTY <prop>) - dt_chosen(<var> PROPERTY <prop>) Signed-off-by: Martí Bolívar <[email protected]> Signed-off-by: Torsten Rasmussen <[email protected]>

tejlmand requested review from hakonfam, carlescufi, galak and mbolivar-nordic August 11, 2021 19:32

tejlmand requested a review from nashif as a code owner August 11, 2021 19:32

github-actions bot added area: Build System area: Devicetree Tooling PR modifies or adds a Device Tree tooling labels Aug 11, 2021

tejlmand force-pushed the dts_cmake branch 2 times, most recently from 8916a8d to 2ab3b86 Compare August 11, 2021 21:54

galak reviewed Aug 11, 2021

View reviewed changes

tejlmand force-pushed the dts_cmake branch from 2ab3b86 to 271b43b Compare August 12, 2021 13:10

galak self-assigned this Aug 12, 2021

tejlmand force-pushed the dts_cmake branch from 271b43b to aecb7fc Compare August 13, 2021 10:24

tejlmand mentioned this pull request Aug 13, 2021

Armclang / armlink support in Zephyr - Zephyr CMake linker functions #36140

Merged

7 tasks

mbolivar-nordic reviewed Aug 16, 2021

View reviewed changes

tejlmand force-pushed the dts_cmake branch from aecb7fc to a3400ce Compare August 18, 2021 10:01

tejlmand force-pushed the dts_cmake branch from a3400ce to 8d9ce4b Compare August 19, 2021 09:22

github-actions bot added area: Modules manifest west manifest-lvgl DNM This PR should not be merged (Do Not Merge) labels Aug 19, 2021

galak reviewed Aug 19, 2021

View reviewed changes

cmake/extensions.cmake Outdated Show resolved Hide resolved

galak reviewed Aug 19, 2021

View reviewed changes

cmake/extensions.cmake Show resolved Hide resolved

galak reviewed Aug 19, 2021

View reviewed changes

cmake/extensions.cmake Show resolved Hide resolved

galak requested changes Aug 19, 2021

View reviewed changes

mbolivar-nordic suggested changes Aug 19, 2021

View reviewed changes

tejlmand force-pushed the dts_cmake branch 2 times, most recently from af2b91a to 6cfd1ad Compare August 20, 2021 09:22

tejlmand mentioned this pull request Aug 20, 2021

cmake: Add support for variable offset and slot size nrfconnect/sdk-nrf#5331

Closed

tejlmand requested review from stephanosio, abrodkin and evgeniy-paltsev August 23, 2021 15:26

tejlmand force-pushed the dts_cmake branch from 6cfd1ad to 2390752 Compare August 24, 2021 22:29

mbolivar-nordic reviewed Aug 24, 2021

View reviewed changes

cmake/dts.cmake Outdated Show resolved Hide resolved

cmake/dts.cmake Outdated Show resolved Hide resolved

tejlmand force-pushed the dts_cmake branch from 2390752 to f0cae1d Compare August 24, 2021 22:33

tejlmand force-pushed the dts_cmake branch 2 times, most recently from 736dc0b to 7dbfc00 Compare August 25, 2021 08:00

tejlmand force-pushed the dts_cmake branch 2 times, most recently from 55ac7d8 to f42386d Compare August 25, 2021 15:12

mbolivar-nordic approved these changes Aug 25, 2021

View reviewed changes

mbolivar-nordic requested a review from galak August 25, 2021 15:51

tejlmand force-pushed the dts_cmake branch from f42386d to d195783 Compare August 25, 2021 19:43

mbolivar-nordic requested a review from trond-snekvik August 25, 2021 23:13

nashif approved these changes Aug 26, 2021

View reviewed changes

nashif merged commit fcf7209 into zephyrproject-rtos:main Aug 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmake: dts: import devicetree symbols into CMake #37624

cmake: dts: import devicetree symbols into CMake #37624

tejlmand commented Aug 11, 2021 •

edited

Loading

galak Aug 11, 2021

tejlmand Aug 12, 2021 •

edited

Loading

galak Aug 12, 2021

tejlmand Aug 13, 2021

mbolivar-nordic Aug 16, 2021

galak commented Aug 13, 2021

galak commented Aug 13, 2021

tejlmand commented Aug 13, 2021

tejlmand commented Aug 13, 2021

mbolivar-nordic commented Aug 16, 2021 •

edited

Loading

mbolivar-nordic left a comment

mbolivar-nordic commented Aug 16, 2021 •

edited

Loading

tejlmand commented Aug 18, 2021

tejlmand commented Aug 18, 2021

galak left a comment

mbolivar-nordic left a comment

mbolivar-nordic Aug 19, 2021

mbolivar-nordic commented Aug 25, 2021 •

edited

Loading

mbolivar-nordic commented Aug 25, 2021

tejlmand commented Aug 25, 2021

tejlmand commented Aug 25, 2021 •

edited

Loading

mbolivar-nordic left a comment

mbolivar-nordic commented Aug 25, 2021

cmake: dts: import devicetree symbols into CMake #37624

cmake: dts: import devicetree symbols into CMake #37624

Conversation

tejlmand commented Aug 11, 2021 • edited Loading

galak Aug 11, 2021

Choose a reason for hiding this comment

tejlmand Aug 12, 2021 • edited Loading

Choose a reason for hiding this comment

galak Aug 12, 2021

Choose a reason for hiding this comment

tejlmand Aug 13, 2021

Choose a reason for hiding this comment

mbolivar-nordic Aug 16, 2021

Choose a reason for hiding this comment

galak commented Aug 13, 2021

galak commented Aug 13, 2021

tejlmand commented Aug 13, 2021

tejlmand commented Aug 13, 2021

mbolivar-nordic commented Aug 16, 2021 • edited Loading

mbolivar-nordic left a comment

Choose a reason for hiding this comment

mbolivar-nordic commented Aug 16, 2021 • edited Loading

tejlmand commented Aug 18, 2021

tejlmand commented Aug 18, 2021

galak left a comment

Choose a reason for hiding this comment

mbolivar-nordic left a comment

Choose a reason for hiding this comment

mbolivar-nordic Aug 19, 2021

Choose a reason for hiding this comment

mbolivar-nordic commented Aug 25, 2021 • edited Loading

mbolivar-nordic commented Aug 25, 2021

tejlmand commented Aug 25, 2021

tejlmand commented Aug 25, 2021 • edited Loading

mbolivar-nordic left a comment

Choose a reason for hiding this comment

mbolivar-nordic commented Aug 25, 2021

tejlmand commented Aug 11, 2021 •

edited

Loading

tejlmand Aug 12, 2021 •

edited

Loading

mbolivar-nordic commented Aug 16, 2021 •

edited

Loading

mbolivar-nordic commented Aug 16, 2021 •

edited

Loading

mbolivar-nordic commented Aug 25, 2021 •

edited

Loading

tejlmand commented Aug 25, 2021 •

edited

Loading