phandle-array doesn't allow array of just phandles

[joelspadin]

Describe the bug

zephyr/dts/binding-template.yaml

Lines 157 to 160 in 0a0cb52

	# - 'type: phandle-array' is for properties that take a list of phandles and
	# (possibly) 32-bit numbers, like
	#
	# pwms = <&ctrl-1 1 2 &ctrl-2 3 4>;

indicates that in a value of type phandle-array, numbers are optional, so <&foo &bar> should be valid.

However,

zephyr/scripts/dts/edtlib.py

Lines 1225 to 1230 in 0a0cb52

	if prop_type == "phandle-array":
	# This type is a bit high-level for dtlib as it involves
	# information from bindings and *-names properties, so there's no
	# to_phandle_array() in dtlib. Do the type check ourselves.
	if prop.type not in (TYPE_PHANDLE, TYPE_PHANDLES_AND_NUMS):
	_err("expected property '{0}' in {1} in {2} to be assigned "

appears to only allow TYPE_PHANDLE (a single phandle) and TYPE_PHANDLES_AND_NUMS. Is that supposed to be TYPE_PHANDLES instead of TYPE_PHANDLE?

To Reproduce
Steps to reproduce the behavior:

1. Set up the ZMK keyboard firmware project.
2. Edit app/boards/shields/romac/romac.keymap and change one of the bindings arrays to contain only values such as &reset and &trans so there are no numbers in the array.
3. Build with

cd app
west build -b proton_c -- -DSHIELD=romac

This will fail with an error like

devicetree error: expected property 'bindings' in /keymap/nav_layer in proton_c.dts.pre.tmp to be assigned with 'bindings = < &foo 1 2 ... &bar 3 4 ... >' (a mix of phandles and numbers), not 'bindings = < &reset &trans &trans &trans &trans &trans &trans &trans &trans &trans &trans &trans >;'

Expected behavior
As far as I can tell, an array of just phandles should be valid for a phandle-array.

Impact
Annoyance. Can be worked around by adding something that uses numbers somewhere in the array.

Environment (please complete the following information):

• OS: Windows 10
• Toolchain: GNU Arm Embedded
• Commit SHA or Version used: https://github.com/petejohanson/zephyr/tree/zmk-v2.3.0-with-fixes

[henrikbrixandersen]

How about <&foo>, <&bar>?

[joelspadin]

Same error (tested in a different keymap with a slightly different array):

devicetree error: expected property 'bindings' in /keymap/daw_layer in nice_nano.dts.pre.tmp to be assigned with 'bindings = < &foo 1 2 ... &bar 3 4 ... >' (a mix of phandles and numbers), not 'bindings = < &none >, < &none >, < &none >, < &none >, < &none >, < &none >, < &none >, < &none >, < &trans >, < &none >, < &none >, < &none >, < &none >, < &none >, < &none >, < &none >;'

[b0661]

zephyr/dts/binding-template.yaml

Lines 157 to 160 in 0a0cb52

	# - 'type: phandle-array' is for properties that take a list of phandles and
	# (possibly) 32-bit numbers, like
	#
	# pwms = <&ctrl-1 1 2 &ctrl-2 3 4>;

the ´(possibly)´ relates to ´32-bit´ (and not to numbers). phandle-array is always a combination of phandle and associated number(s). The numbers may be up to 32-bit size. In case you only have phandles just use ´phandles´ in the binding.

´(possibly)´ should possibly replaced or removed.

[joelspadin]

phandle-array is always a combination of phandle and associated number(s).

I'll admit I am still very unfamiliar with DT, but I'm confused as to why edtlib.py allows phandle-array to be either TYPE_PHANDLE or TYPE_PHANDLES_AND_NUMS then. Assuming TYPE_PHANDLE is not a typo intended to be TYPE_PHANDLES, then what would it mean for a value of type phandle-array to be set to a single phandle?

Having phandles with no associated numbers seems to work as long as there is at least one number somewhere in the array. If phandle-array allows any element to be a phandle with no associated number, why is it not valid for the entire array to be made of such elements? If that is not valid, then should phandle-array require every phandle to have at least one associated number?

I don't know if DT was intended to be used like this, but one of the ways it is used in ZMK is to define a mapping between keyboard keys and the behaviors to use when each key is pressed. Some behaviors such as &kp KEY and &lt LAYER KEY have one or more associated numbers. Other behaviors such as &reset and &none don't have any associated numbers. ZMK allows you to mix both types of behaviors in a keymap.

[b0661]

Having phandles with no associated numbers seems to work as long as there is at least one number somewhere in the array. If phandle-array allows any element to be a phandle with no associated number, why is it not valid for the entire array to be made of such elements?

You are right, the error check seems too strict for the degenerated case that you describe.

zephyr/scripts/dts/edtlib.py

Line 1229 in adb62ea

if prop.type not in (TYPE_PHANDLE, TYPE_PHANDLES_AND_NUMS):

I could not find behaviors.dtsi which defines the behavior nodes. The reset and none nodes should have properties like xxx-cells = 0; that define the count of values to be 0. With this given the phandles list as a degenerated phandles-array is a valid construct.

In my view TYPE_PHANDLES should be added to the allowed dtslib types for phandle-array.

[joelspadin]

I could not find behaviors.dtsi which defines the behavior nodes. The reset and none nodes should have properties like xxx-cells = 0; that define the count of values to be 0.

Yep. See https://github.com/zmkfirmware/zmk/blob/main/app/dts/behaviors/none.dtsi for example.

Should TYPE_PHANDLE be kept and TYPE_PHANDLES added, or should that be replaced with TYPE_PHANDLES?

[b0661]

Should TYPE_PHANDLE be kept and TYPE_PHANDLES added

Yes. Looking at

zephyr/scripts/dts/dtlib.py

Lines 1353 to 1357 in 1342d12

	foo = <&l>; | dtlib.TYPE_PHANDLE
	foo = <&l1 &l2 &l3>; | dtlib.TYPE_PHANDLES
	foo = <&l1 &l2>, <&l3>; | dtlib.TYPE_PHANDLES
	foo = <&l1 1 2 &l2 3 4>; | dtlib.TYPE_PHANDLES_AND_NUMS
	foo = <&l1 1 2>, <&l2 3 4>; | dtlib.TYPE_PHANDLES_AND_NUMS

suggests that TYPE_PHANDLE and TYPE_PHANDLES are valid types.

[mbolivar-nordic]

TYPE_PHANDLE and TYPE_PHANDLES are valid types.

Agreed; I'll send a PR