From 63d5529a0d08e14f91997d0ec29e3e49431a45c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=AD=20Bol=C3=ADvar?= Date: Mon, 6 Apr 2020 15:13:53 -0700 Subject: [PATCH] devicetree: re-work DT_INST_FOREACH() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Due to the use of UTIL_EVAL*() macros, the UTIL_LISTIFY() macro used by DT_INST_FOREACH(foo) can cause long build errors when there is a build error in the expansion for "foo". More than a thousand lines of build error output have been observed for an error in a single line of faulty C. To improve the situation, re-work the implementation details so the errors are a bit shorter and easier to read. The use of COND_CODE_1 still makes the error messages quite long, due to GCC generating notes for various intermediate expansions (__DEBRACKET, __GET_ARG_2_DEBRACKET, __COND_CODE, Z_COND_CODE_1, COND_CODE1), but it's better than the long list of UTIL_EVAL notes. Signed-off-by: Martí Bolívar --- doc/guides/dts/macros.bnf | 4 ++++ include/devicetree.h | 5 ++--- scripts/dts/gen_defines.py | 19 +++++++++++++++++-- tests/lib/devicetree/src/main.c | 20 ++++++++++++++++++++ 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/doc/guides/dts/macros.bnf b/doc/guides/dts/macros.bnf index 23b51d49717..744eec1f386 100644 --- a/doc/guides/dts/macros.bnf +++ b/doc/guides/dts/macros.bnf @@ -125,6 +125,10 @@ prop-suf = 1*( "_" gen-name ["_" dt-name] ) other-macro = %s"DT_N_" alternate-id ; Total count of enabled instances of a compatible. other-macro =/ %s"DT_N_INST_" dt-name %s"_NUM" +; These are used internally by DT_INST_FOREACH, which iterates over +; each enabled instance of a compatible. +other-macro =/ %s"DT_FOREACH_INST_" dt-name +other-macro =/ %s"DT_FOREACH_IMPL_" DIGIT ; E.g.: #define DT_CHOSEN_zephyr_flash other-macro =/ %s"DT_CHOSEN_" dt-name ; Declares that a compatible has at least one node on a bus. diff --git a/include/devicetree.h b/include/devicetree.h index 2513790ebd0..8c00e50a8c3 100644 --- a/include/devicetree.h +++ b/include/devicetree.h @@ -1389,7 +1389,8 @@ * Has to accept instance_number as only parameter. */ #define DT_INST_FOREACH(inst_expr) \ - UTIL_LISTIFY(DT_NUM_INST(DT_DRV_COMPAT), DT_CALL_WITH_ARG, inst_expr) + COND_CODE_1(DT_HAS_COMPAT(DT_DRV_COMPAT), \ + (UTIL_CAT(DT_FOREACH_INST_, DT_DRV_COMPAT)(inst_expr)), ()) /** * @brief Does a DT_DRV_COMPAT instance have a property? @@ -1489,7 +1490,5 @@ #define DT_DASH(...) MACRO_MAP_CAT(DT_DASH_PREFIX, __VA_ARGS__) /** @internal helper for DT_DASH(): prepends _ to a name */ #define DT_DASH_PREFIX(name) _##name -/** @internal DT_INST_FOREACH helper */ -#define DT_CALL_WITH_ARG(arg, expr) expr(arg); #endif /* DEVICETREE_H */ diff --git a/scripts/dts/gen_defines.py b/scripts/dts/gen_defines.py index cf112f77e90..13f2759cf7f 100755 --- a/scripts/dts/gen_defines.py +++ b/scripts/dts/gen_defines.py @@ -525,9 +525,24 @@ def write_global_compat_info(edt): if bus is not None and bus not in compat2buses[compat]: compat2buses[compat].append(bus) - out_comment("Number of enabled instances of each compatible\n") + out_comment("Helpers for calling a macro/function a fixed number of times" + "\n") + for numinsts in range(1, max(compat2numinst.values(), default=0) + 1): + out_define(f"DT_FOREACH_IMPL_{numinsts}(fn)", + " ".join([f"fn({i});" for i in range(numinsts)])) + + out_comment("Macros for enabled instances of each compatible\n") + n_inst_macros = {} + for_each_macros = {} for compat, numinst in compat2numinst.items(): - out_define(f"DT_N_INST_{str2ident(compat)}_NUM", numinst) + ident = str2ident(compat) + n_inst_macros[f"DT_N_INST_{ident}_NUM"] = numinst + for_each_macros[f"DT_FOREACH_INST_{ident}(fn)"] = \ + f"DT_FOREACH_IMPL_{numinst}(fn)" + for macro, value in n_inst_macros.items(): + out_define(macro, value) + for macro, value in for_each_macros.items(): + out_define(macro, value) out_comment("Bus information for enabled nodes of each compatible\n") for compat, buses in compat2buses.items(): diff --git a/tests/lib/devicetree/src/main.c b/tests/lib/devicetree/src/main.c index 53a84957cbb..20385a153c5 100644 --- a/tests/lib/devicetree/src/main.c +++ b/tests/lib/devicetree/src/main.c @@ -1022,6 +1022,7 @@ static void test_devices(void) struct device *dev0; struct device *dev1; struct device *dev_abcd; + unsigned int val; zassert_true(DT_HAS_NODE(INST(0)), "inst 0 device"); zassert_true(DT_HAS_NODE(INST(1)), "inst 1 device"); @@ -1044,6 +1045,25 @@ static void test_devices(void) zassert_not_null(dev_abcd, "abcd"); zassert_equal(to_info(dev_abcd)->reg_addr, 0xabcd1234, "abcd addr"); zassert_equal(to_info(dev_abcd)->reg_len, 0x500, "abcd len"); + + /* + * Make sure DT_INST_FOREACH can be called from functions + * using macros with side effects in the current scope. + */ +#undef SET_BIT +#define SET_BIT(i) do { unsigned int bit = BIT(i); val |= bit; } while (0) + val = 0; + DT_INST_FOREACH(SET_BIT); + zassert_equal(val, 0x3, "foreach vnd_gpio"); + + /* + * Make sure DT_INST_FOREACH works with 0 instances, and does + * not expand its argument at all. + */ +#undef DT_DRV_COMPAT +#define DT_DRV_COMPAT xxxx +#define BUILD_BUG_ON_EXPANSION (there is a bug in devicetree.h) + DT_INST_FOREACH(BUILD_BUG_ON_EXPANSION); } static void test_cs_gpios(void)