new devicetree.h API #23245

mbolivar · 2020-03-04T06:55:24Z

Add a new devicetree.h API. This makes the current macros deprecated but keeps them around. The new API uses macros which are not generated, hiding the details of the generated macros from the user.

This is inspired by #22964 (comment) but "reverses" the capitalization: the names from devicetree remain lowercase. See the documentation changes for what I mean.

Fixes: #22964
Fixes: #22555
Fixes: #22554

Per @galak and @pabigot:

Fixes: #21369
Fixes: #19285

TODO list:

Out of scope for this PR, to be addressed later:

flash partitions

zephyrbot · 2020-03-04T07:05:43Z

All checks are passing now.

checkpatch (informational only, not a failure)

-:3036: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string "vnd,soc-i2c" appears un-documented -- check ./dts/bindings/
#3036: FILE: doc/guides/dts/main-example.dts:22:
+			compatible = "vnd,soc-i2c";

-:5320: WARNING:LONG_LINE_COMMENT: line over 80 characters
#5320: FILE: include/devicetree.h:1113:
+ * @brief Get a DT_DRV_COMPAT instance's phandle-array specifier value at an index

-:5399: WARNING:LONG_LINE: line over 80 characters
#5399: FILE: include/devicetree.h:1192:
+#define DT_INST_REG_ADDR_BY_IDX(inst, idx) DT_REG_ADDR_BY_IDX(DT_DRV_INST(inst), idx)

-:5590: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#5590: FILE: include/devicetree.h:1383:
+#define DT_INST_ON_BUS_OR(inst, bus) DT_ON_BUS(DT_DRV_INST(inst), bus) ||

-:5720: WARNING:LONG_LINE: line over 80 characters
#5720: FILE: include/devicetree/adc.h:122:
+#define DT_INST_IO_CHANNELS_LABEL(inst) DT_INST_IO_CHANNELS_LABEL_BY_IDX(inst, 0)

-:5785: WARNING:LONG_LINE: line over 80 characters
#5785: FILE: include/devicetree/adc.h:187:
+#define DT_INST_IO_CHANNELS_INPUT(inst) DT_INST_IO_CHANNELS_INPUT_BY_IDX(inst, 0)

-:9171: WARNING:LONG_LINE_STRING: line over 80 characters
#9171: FILE: tests/lib/devicetree/src/main.c:395:
+	zassert_equal(DT_IRQ_BY_IDX(TEST_SPI_BUS_0, 0, priority), 3, "irq 0 pri");

-:9172: WARNING:LONG_LINE_STRING: line over 80 characters
#9172: FILE: tests/lib/devicetree/src/main.c:396:
+	zassert_equal(DT_IRQ_BY_IDX(TEST_SPI_BUS_0, 1, priority), 0, "irq 1 pri");

-:9173: WARNING:LONG_LINE_STRING: line over 80 characters
#9173: FILE: tests/lib/devicetree/src/main.c:397:
+	zassert_equal(DT_IRQ_BY_IDX(TEST_SPI_BUS_0, 2, priority), 1, "irq 2 pri");

-:9176: WARNING:LONG_LINE_STRING: line over 80 characters
#9176: FILE: tests/lib/devicetree/src/main.c:400:
+	zassert_equal(DT_IRQ_BY_NAME(TEST_I2C_BUS, status, irq), 6, "irq status");

-:9226: WARNING:LONG_LINE_STRING: line over 80 characters
#9226: FILE: tests/lib/devicetree/src/main.c:450:
+	zassert_equal(DT_INST_IRQ_BY_IDX(0, 0, priority), 3, "inst 0 irq 0 pri");

-:9227: WARNING:LONG_LINE_STRING: line over 80 characters
#9227: FILE: tests/lib/devicetree/src/main.c:451:
+	zassert_equal(DT_INST_IRQ_BY_IDX(0, 1, priority), 5, "inst 0 irq 1 pri");

-:9228: WARNING:LONG_LINE_STRING: line over 80 characters
#9228: FILE: tests/lib/devicetree/src/main.c:452:
+	zassert_equal(DT_INST_IRQ_BY_IDX(0, 2, priority), 7, "inst 0 irq 2 pri");

-:9232: WARNING:LONG_LINE_STRING: line over 80 characters
#9232: FILE: tests/lib/devicetree/src/main.c:456:
+	zassert_equal(DT_INST_IRQ_BY_NAME(0, stat, irq), 40, "inst 0 irq status");

-:9233: WARNING:LONG_LINE_STRING: line over 80 characters
#9233: FILE: tests/lib/devicetree/src/main.c:457:
+	zassert_equal(DT_INST_IRQ_BY_NAME(0, done, irq), 60, "inst 0 done error");

-:9538: WARNING:LONG_LINE_STRING: line over 80 characters
#9538: FILE: tests/lib/devicetree/src/main.c:762:
+	zassert_equal(DT_GPIO_PIN_BY_IDX(TEST_PH, gpios, 0), 10, "gpio 0 pin idx");

-:9539: WARNING:LONG_LINE_STRING: line over 80 characters
#9539: FILE: tests/lib/devicetree/src/main.c:763:
+	zassert_equal(DT_GPIO_PIN_BY_IDX(TEST_PH, gpios, 1), 30, "gpio 1 pin idx");

-:9633: WARNING:LONG_LINE_STRING: line over 80 characters
#9633: FILE: tests/lib/devicetree/src/main.c:857:
+	zassert_equal(DT_INST_IO_CHANNELS_INPUT_BY_NAME(0, ch1), 10, "input ch1");

-:9634: WARNING:LONG_LINE_STRING: line over 80 characters
#9634: FILE: tests/lib/devicetree/src/main.c:858:
+	zassert_equal(DT_INST_IO_CHANNELS_INPUT_BY_NAME(0, ch2), 20, "input ch2");

- total: 0 errors, 19 warnings, 9450 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

ulfalizer

Random comments from skimming.

scripts/dts/gen_defines.py

mbolivar · 2020-03-07T00:58:00Z

Thanks @ulfalizer; comments addressed.

I've done a couple of test conversions of nRF drivers as well.

mbolivar · 2020-03-07T01:06:08Z

-:2712: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string "vnd,soc-i2c" appears un-documented -- check ./dts/bindings/
#2712: FILE: doc/guides/dts/main-example.dts:16:
	compatible = "vnd,soc-i2c";
-:2712: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string vendor "vnd" appears un-documented -- check ./dts/bindings/vendor-prefixes.txt
#2712: FILE: doc/guides/dts/main-example.dts:16:
	compatible = "vnd,soc-i2c";

@ulfalizer do you know if we can whitelist "vnd," for examples here?

mnkp · 2020-03-09T23:45:46Z

When we use the new macros such as DT_INST(), DT_PROP() in the device drivers we should be fine. Even if the macros fail to expand correctly the person dealing with the error will be the author of the device driver. I'm however concerned about the usage of DT_ALIAS() within the application code. Here, if the macro fails to expand to a known symbol the person dealing with the error message will be a random user who doesn't have to be very familiar with Zephyr.

I propose to convert the samples/basic/button sample app (it's a bit more complex than samples/basic/blinky but still fairly straightforward) to use DT_ALIAS() and then try to compile it on a board that doesn't have aliases node. It's something a random, new Zephyr user is likely to do.

Then we should make a judgment, trying to asses if the error message produced by the compiler is something this user is able to deal with.

I run some tests locally, but it looks like we are missing macros to access properties with type phandle-array, i.e. extract pin value from gpios property.

led0: led_0 {
	gpios = <&gpio0 13 GPIO_ACTIVE_LOW>;
	label = "Green LED 0";
};

so I'm not really sure what the end result would be.

Zephyr's logging subsystem is also using macros extensively and the error messages produced are bulky and complex, difficult to interpret.

pabigot · 2020-03-23T14:12:25Z

I would like restoration of the ability to reference initializer lists for phandle array elements ASAP. So I can do:

This was originally present but was removed when we moved towards generating node identifiers in place of phandles.

OK, that's something to discuss then, because a phandle+specifier is a very useful thing to be able to treat as a single unit, for GPIO, IO channels, probably everything. LPF as a name to capture the GPIO phandle+specifier seems opaque. For now I'll do my own within each module where I need it.

galak · 2020-03-23T14:43:57Z

I would like restoration of the ability to reference initializer lists for phandle array elements ASAP. So I can do:

This was originally present but was removed when we moved towards generating node identifiers in place of phandles.

OK, that's something to discuss then, because a phandle+specifier is a very useful thing to be able to treat as a single unit, for GPIO, IO channels, probably everything. LPF as a name to capture the GPIO phandle+specifier seems opaque. For now I'll do my own within each module where I need it.

@pabigot - Do we have an issue open for this?

pabigot · 2020-03-23T14:47:43Z

@pabigot - Do we have an issue open for this?

No, because there's a working solution for the old API. The new API doesn't have a replacement.

mbolivar · 2020-03-23T14:48:31Z

OK, that's something to discuss then, because a phandle+specifier is a very useful thing to be able to treat as a single unit, for GPIO, IO channels, probably everything.

I agree, but I think the correct solution is for the initializer generated for a phandle+specifier to replace the phandle with a pointer to the struct device, which we can't do yet.

Edit: I think that is the next step, just to be explicit.

pabigot · 2020-03-23T14:50:17Z

I agree, but I think the correct solution is for the initializer generated for a phandle+specifier to replace the phandle with a pointer to the struct device, which we can't do yet.

In the final configuration, yes. For now, I need something to replace the existing uses of initializer-lists that include the name of the device along with the specifier. I will deal with that on a case-by-case basis.

This is useful for devicetree documentation, examples, and tests, where we need to put something for the vendor but we can't use an actual piece of hardware for some reason. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

This returns the entire logical {name: Node} dictionary which is currently being accessed element by element via chosen_node(name). It will be used in a new gen_defines.py for moving the handling of chosen nodes into C from Python. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

This rename is mostly to easy git managment and review so any changes or the addition of the new gen_defines.py doesn't look like a diff against the old code if you look at just that commit. We keep changes to a minimum to just keep things building with the renamed gen_legacy_defines.py. Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

If the #address-cells property for a register is 0 than we set the addr value of the reg to None. Similar, if #size-cells is 0 than we set the size value to None for the reg. Fixup kconfigfunctions.py to handle reg.size and reg.addr being None. Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

This is joint work with Kumar Gala (see signed-off-by). This supports a new devicetree macro syntax coming. It's not really worth mixing up the old and the new generation scripts into one file, because: - we aim to remove support for the old macros at some point, so it will be cleaner to start fresh with a new script based on the old one that only generates the new syntax - it will avoid regressions to leave the existing code alone while we're moving users to the new names Keep the existing script by moving it to gen_legacy_defines.py and changing a few comments and strings around. It's responsible for generating: - devicetree.conf: only needed by deprecated kconfigfunctions - devicetree_legacy_unfixed.h: "old" devicetree_unfixed.h macros Put a new gen_defines.py in its place. It generates: - zephyr.dts - devicetree_unfixed.h in the new syntax Include devicetree_legacy_unfixed.h from devicetree.h so no DT users are affected by this change. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no> Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

This is joint work with Kumar Gala (see signed-off-by). Add helper macros which abstract the "true names" of each of the four types of node identifier we intend to support (e.g. DT_ALIAS(), DT_INST()). These can be passed to a new DT_PROP() macro which can be used to read the value of a devicetree property given a node identifier from one of these four other macros, and the as-a-c-token name of the property. Add other accessor macros and tests as well. Add some convenience APIs for writing device drivers based on instance numbers as well. Drivers can "#define DT_DRV_COMPAT driver_compatible" at the top of the file, then utilize these DT_INST_* macros to access various property defines. For example, the uart_sifive driver can do: #define DT_DRV_COMPAT sifive_uart0 Then use DT_INST macros like: .port = DT_INST_REG_ADDR(0), .sys_clk_freq = DT_INST_PROP(0, clock_frequency), For convenience working with specific hardware, also add: <devicetree/gpio.h> <devicetree/adc.h> <devicetree/spi.h> Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no> Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

This is joint work with Kumar Gala (see signed-off-by). Document the changes to the generated node macros in macros.bnf, moving the old file to legacy-macros.bnf and putting it in its own section. The actual generated macros are now a low-level detail, so rewrite the foregoing sections as examples in terms of the new <devicetree.h> APIs. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no> Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

This doesn't sacrifice any readability when compiled for boards that don't support this alias. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

Use the new devicetree.h API instead of the legacy macros. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

This allows us to start using DT_NODELABEL() to access SPIMs that way, instead of via an alias. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

Use the new devicetree.h API instead of the legacy macros. Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

Mention the new devicetree.h API and link to relevant resources. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

Update maintainers for code and docs. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

galak · 2020-03-23T14:53:13Z

I agree, but I think the correct solution is for the initializer generated for a phandle+specifier to replace the phandle with a pointer to the struct device, which we can't do yet.

In the final configuration, yes. For now, I need something to replace the existing uses of initializer-lists that include the name of the device along with the specifier. I will deal with that on a case-by-case basis.

How do you feel about a macro per subsystem/class? Or do you see the need for this for phandle/phandle-array out of tree?

mbolivar-nordic · 2020-03-23T15:09:48Z

How do you feel about a macro per subsystem/class? Or do you see the need for this for phandle/phandle-array out of tree?

Can we just do this as needed for now since it's temporary? Any other cases besides GPIO?

I really want to be able to turn a phandle into a struct device* sooner rather than later, so I am hoping we don't have to spend too much review / discussion bandwidth solving an interim problem...

pabigot · 2020-03-23T15:13:51Z

There are other cases, yes, ADC in particular. But external workarounds are fine for now. #23691 can be resolved in the context of generating a device handle instead of the name; that's preferable anyway.

mbolivar requested review from ulfalizer, nashif, pabigot, henrikbrixandersen, MaureenHelm, carlescufi, galak and mnkp March 4, 2020 06:55

mbolivar requested review from dbkinder and SebastianBoe as code owners March 4, 2020 06:55

mbolivar requested review from erwango and b0661 March 4, 2020 06:55

mbolivar force-pushed the dt-macros-ng branch from e34ce5c to caeb40b Compare March 4, 2020 07:02

zephyrbot added area: API Changes to public APIs area: Tests Issues related to a particular existing or missing test area: Documentation area: Build System labels Mar 4, 2020

mbolivar force-pushed the dt-macros-ng branch 4 times, most recently from 66a327f to e3c55ba Compare March 4, 2020 14:48

dbkinder removed their request for review March 4, 2020 22:30

ulfalizer reviewed Mar 6, 2020

View reviewed changes

mbolivar force-pushed the dt-macros-ng branch from e3c55ba to df91bd3 Compare March 7, 2020 00:57

mbolivar requested a review from tbursztyka as a code owner March 7, 2020 00:57

zephyrbot added the area: SPI SPI bus label Mar 7, 2020

mbolivar force-pushed the dt-macros-ng branch from cfdaf1d to 1b0e185 Compare March 23, 2020 14:46

mbolivar-nordic and others added 14 commits March 23, 2020 07:51

dts: bindings: add fake 'vnd' vendor

daba5b4

This is useful for devicetree documentation, examples, and tests, where we need to put something for the vendor but we can't use an actual piece of hardware for some reason. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

samples: convert blinky to new DT API

eba7e67

This doesn't sacrifice any readability when compiled for boards that don't support this alias. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

drivers: serial: convert uart_nrfx_uart to new DT API

f9e379b

Use the new devicetree.h API instead of the legacy macros. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

drivers: spi: convert spi_nrfx_spim.c to new DT API

0befecf

This allows us to start using DT_NODELABEL() to access SPIMs that way, instead of via an alias. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

drivers: serial: uart_sifive: convert to new DT API

6d276ac

Use the new devicetree.h API instead of the legacy macros. Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

drivers: sensor: fxos8700: convert to new DT API

44211a2

Use the new devicetree.h API instead of the legacy macros. Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

doc: release-notes-2.3: add devicetree changes

947c63f

Mention the new devicetree.h API and link to relevant resources. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

CODEOWNERS: devicetree updates

4655f56

Update maintainers for code and docs. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

mbolivar force-pushed the dt-macros-ng branch from 1b0e185 to 4655f56 Compare March 23, 2020 14:52

pabigot mentioned this pull request Mar 23, 2020

add support to store devicetree phandle+specifier as "foo_dt_spec" structures #23691

Open

mbolivar-nordic mentioned this pull request Mar 23, 2020

[RFC][DNM]DTS service nodes #23394

Closed

jfischer-no approved these changes Mar 24, 2020

View reviewed changes

galak merged commit 29887ec into zephyrproject-rtos:master Mar 24, 2020

mbolivar-nordic mentioned this pull request Mar 24, 2020

Devicetree: Macro to initialize driver for every device instance #23416

Merged

pabigot mentioned this pull request Mar 26, 2020

devicetree: Add helper macros for GPIO element/array initialization #23795

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new devicetree.h API #23245

new devicetree.h API #23245

mbolivar commented Mar 4, 2020 •

edited by mbolivar-nordic

Loading

zephyrbot commented Mar 4, 2020 •

edited

Loading

ulfalizer left a comment

mbolivar commented Mar 7, 2020

mbolivar commented Mar 7, 2020

mnkp commented Mar 9, 2020

pabigot commented Mar 23, 2020

galak commented Mar 23, 2020

pabigot commented Mar 23, 2020

mbolivar commented Mar 23, 2020 •

edited

Loading

pabigot commented Mar 23, 2020

galak commented Mar 23, 2020

mbolivar-nordic commented Mar 23, 2020

pabigot commented Mar 23, 2020

new devicetree.h API #23245

new devicetree.h API #23245

Conversation

mbolivar commented Mar 4, 2020 • edited by mbolivar-nordic Loading

zephyrbot commented Mar 4, 2020 • edited Loading

checkpatch (informational only, not a failure)

ulfalizer left a comment

Choose a reason for hiding this comment

mbolivar commented Mar 7, 2020

mbolivar commented Mar 7, 2020

mnkp commented Mar 9, 2020

pabigot commented Mar 23, 2020

galak commented Mar 23, 2020

pabigot commented Mar 23, 2020

mbolivar commented Mar 23, 2020 • edited Loading

pabigot commented Mar 23, 2020

galak commented Mar 23, 2020

mbolivar-nordic commented Mar 23, 2020

pabigot commented Mar 23, 2020

mbolivar commented Mar 4, 2020 •

edited by mbolivar-nordic

Loading

zephyrbot commented Mar 4, 2020 •

edited

Loading

mbolivar commented Mar 23, 2020 •

edited

Loading