Issue with capturing phandles in a property

[aisri]

NOTE
please use uploaded patch for replicating the issue:
0001-issue-with-capturing-phandles-in-a-parent-s-property.patch

Describe the bug
build fails when capturing phandles of nodes.
in the below device node definition, capturing phandles of d1, d2 and d3 in devs fails

{
        test_dev: test_dev@deadbeef {
            status = "okay";
            compatible = "test,device";
            reg = <0xdeadbeef 0x100>;

            fsms = <&a_fsm>, <&b_fsm>;
            devs = <&d1>, <&d2>, <&d3>;

            #address-cells = <1>;
            #size-cells = <0>;

            edge: arrow {
                compatible = "test,edge";
                #edge-cells = <2>;
            };

            vertex: point {
                compatible = "test,node";
                #edge-cells = <1>;
            };

            a_fsm: aa_fsm {
                compatible = "test,fsm";
                edges = <&vertex ST_B &edge ST_A 2>,
                        <&vertex ST_B &edge ST_A 11>,
                        <&vertex ST_C &edge ST_A 14>,
                        <&vertex ST_A &edge ST_C 12 &edge ST_B 7 &edge ST_B 1>;
            };

            b_fsm: bb_fsm {
                compatible = "test,fsm";
                edges = <&vertex ST_B &edge ST_A 3>,
                        <&vertex ST_C &edge ST_A 5>,
                        <&vertex ST_B &edge ST_A 1>,
                        <&vertex ST_A &edge ST_C 4 &edge ST_B 10 &edge ST_B 0>;
            };

            d1: dev@200 {
                reg = <0x200>;
                parent = <&d1>;
                profile = <&a_fsm>;
            };
            d2: dev@400 {
                reg = <0x400>;
                parent = <&d1>;
                profile = <&b_fsm>;
            };
            d3: dev@600 {
                reg = <0x600>;
                parent = <&d1>;
                profile = <&b_fsm>;
            };
        };
    };

Platform independent fails in c-preprocessing expansion

To Reproduce
Apply the patch above
and run the following command:

west build -b 96b_carbon samples/hello_world/ -DEXTRA_CFLAGS=-save-temps=obj --pristine

Expected behavior
Build succeeds

Impact
Found a workaround.

Logs and console output
error:

/Users/$HOME/projects/zephyr/zephyr/include/zephyr/device.h:975:1: note: in expansion of macro 'DT_FOREACH_STATUS_OKAY_NODE'
  975 | DT_FOREACH_STATUS_OKAY_NODE(Z_MAYBE_DEVICE_DECLARE_INTERNAL)
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/$HOME/projects/zephyr/zephyr/include/zephyr/device.h:95:45: error: pasting "dts_ord_" and "-" does not give a valid preprocessing token
   95 | #define Z_DEVICE_DT_DEV_ID(node_id) _CONCAT(dts_ord_, DT_DEP_ORD(node_id))
      |                                             ^~~~~~~~
/Users/sreekanth/projects/zephyr/zephyr/include/zephyr/toolchain/common.h:132:26: note: in definition of macro '_DO_CONCAT'
  132 | #define _DO_CONCAT(x, y) x ## y
      |                          ^

Environment (please complete the following information):

• OS: MacOS
• Toolchain: e.g Zephyr SDK
• Commit SHA or Version used
  • commit b97a4a52a8e957dc41b7a7c2bf67a877b35f96b3 (origin/main, origin/HEAD)

0001-issue-with-capturing-phandles-in-a-parent-s-property.patch

[github-actions]

Hi @aisri! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

[mbolivar-nordic]

From Discord, this looks like a bug in the DT python tooling. Some nodes appear not to be getting dependency ordinals assigned. Root cause still TBD.

[57300]

Some nodes appear not to be getting dependency ordinals assigned. Root cause still TBD.

The ordinals are unassigned, because the nodes in question have circular dependencies. This is a known limitation; see #57708 for more information.

What you could do now is delete the fsms and devs properties from your test_dev node, which would remove the circular dependencies and allow the build to succeed. Those properties look redundant anyway, because test_dev shouldn't need to reference its own child nodes using phandles, and it should be possible to tell whether a child node is an "fsm" or a "dev" based on its compatible.

[mbolivar-ampere]

The ordinals are unassigned, because the nodes in question have circular dependencies. This is a known limitation; see #57708 for more information.

If that is the only error, then the SCC algorithm should have detected the cycle, though, I think.

[57300]

If that is the only error, then the SCC algorithm should have detected the cycle, though, I think.

Good point. So, there does appear to be a bug, because cycles can be detected but ignored.

What I've observed today is that a cycle in edtlib's dependency graph can be ignored by the SCC algorithm if it contains DT leaf nodes. In that case, none of those leaves are recognized as roots by this function:

zephyr/scripts/dts/python-devicetree/src/devicetree/grutils.py

Lines 41 to 44 in 518bbc1

	def roots(self):
	"""
	Return the set of nodes calculated to be roots (i.e., those
	that have no incoming edges).

By construction, the only nodes which can become roots are DT leaf nodes:

zephyr/scripts/dts/python-devicetree/src/devicetree/edtlib.py

Lines 2052 to 2055 in 518bbc1

	for node in self.nodes:
	# A Node always depends on its parent.
	for child in node.children.values():
	self._graph.add_edge(child, node)

If there is no root connected to the cycle, then it is never traversed by the algorithm, hence it is never reported and its constituent nodes are left with unassigned ordinals.

It seems like there needs to be a leaf which is connected to the cycle, but not part of it, in order for it to be caught:

&a_fsm {
        leaf {};
};

Result:

Exception: cycle in devicetree involving /soc/test_dev@deadbeef/dev@600, /soc/test_dev@deadbeef/dev@400, /soc/test_dev@deadbeef/dev@200, /soc/test_dev@deadbeef/point, /soc/test_dev@deadbeef/arrow, /soc/test_dev@deadbeef/bb_fsm, /soc/test_dev@deadbeef, /soc/test_dev@deadbeef/aa_fsm
CMake Error at /home/gms/ncs/zephyr/cmake/modules/dts.cmake:276 (message):
  gen_defines.py failed with return code: 1

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.