Skip to content

Commit e291676

Browse files
committed
doc: sca: dtdoctor: the specifier diagnosis is not gpio-specific
The page listed the specifier spaces as "gpios, interrupts or reg", which undersells it and reads as a limitation that is not there: the handler runs on any phandle-array property, and Zephyr's in-tree bindings declare several hundred distinct ones -- reset-gpios, int-gpios, clocks, pwms, dmas and so on. Plain 'gpios' is a handful of them. Only 'interrupts' and 'reg' are genuinely a fixed pair, since those hang off the node rather than off a property. Every cell test used 'gpios' too, so nothing would have caught a refactor that accidentally keyed off it. Give the fixture a second phandle-array with a different specifier space and cell count, and assert on it. Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NS18GhZ19Qeb67QoRkCu7K
1 parent fe475da commit e291676

5 files changed

Lines changed: 52 additions & 5 deletions

File tree

doc/develop/sca/dtdoctor.rst

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,12 @@ An instance that does not exist
4040
Reports how many instances of the compatible the build has and which of them are
4141
enabled, or, when nothing declares that compatible at all, suggests the closest ones.
4242

43-
A bad index, name or cell in a ``gpios``, ``interrupts`` or ``reg`` specifier
44-
Names the controller the entry points at and lists the cells it defines together with
45-
this node's values, or lists the entries that do exist. Cell names come from the
46-
controller's binding rather than from the node using it, which is what makes these
47-
awkward to track down by hand.
43+
A bad index, name or cell in a specifier
44+
Covers any ``phandle-array`` property -- ``gpios``, ``pwms``, ``clocks``, ``dmas`` and
45+
the rest -- as well as ``interrupts`` and ``reg``. Names the controller the entry points
46+
at and lists the cells it defines together with this node's values, or lists the entries
47+
that do exist. Cell names come from the controller's binding rather than from the node
48+
using it, which is what makes these awkward to track down by hand.
4849

4950
Devicetree names are written differently in C than in DTS: lowercased, with ``-``, ``,``,
5051
``.``, ``@``, ``/`` and ``+`` all becoming ``_``. ``dtdoctor`` spells out both forms

scripts/tests/dtdoctor/conftest.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,9 @@
149149
# entries are named and whose cells come from a controller's binding, an interrupt whose
150150
# entries are *not* named, and a single register. The controllers carry the cell names, so
151151
# 'pin'/'flags' and 'irq'/'priority' are what a diagnosis has to be able to find.
152+
#
153+
# 'pwms' is a second phandle-array, with a different specifier space and cell count, so the
154+
# diagnoses stay pinned as working on any phandle-array rather than just on 'gpios'.
152155
DTS_SPECIFIERS = """
153156
/dts-v1/;
154157
@@ -162,6 +165,11 @@
162165
#gpio-cells = <2>;
163166
};
164167
168+
pwm0: pwm@40003000 {
169+
compatible = "vnd,pwm-ctrl";
170+
#pwm-cells = <2>;
171+
};
172+
165173
irq0: interrupt-controller@e000e100 {
166174
compatible = "vnd,irq-ctrl";
167175
interrupt-controller;
@@ -173,6 +181,7 @@
173181
reg = <0x40002000 0x1000>;
174182
gpios = <&gpio0 13 1>, <&gpio0 14 0>;
175183
gpio-names = "red", "green";
184+
pwms = <&pwm0 3 5000>;
176185
interrupt-parent = <&irq0>;
177186
interrupts = <5 2>;
178187
};
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# SPDX-FileCopyrightText: Copyright The Zephyr Project Contributors
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
description: |
5+
Test PWM controller. A second specifier space, with a different cell count from
6+
the GPIO one, so the diagnoses are pinned as working on any phandle-array rather
7+
than just on 'gpios'.
8+
9+
compatible: "vnd,pwm-ctrl"
10+
11+
pwm-cells:
12+
- channel
13+
- period
14+
15+
properties:
16+
"#pwm-cells":
17+
type: int
18+
required: true

scripts/tests/dtdoctor/fixture/bindings/vnd,specifier-consumer.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ properties:
1212
type: phandle-array
1313
gpio-names:
1414
type: string-array
15+
pwms:
16+
type: phandle-array
1517
interrupts:
1618
type: array
1719
interrupt-names:

scripts/tests/dtdoctor/test_dtdoctor_cells.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,23 @@ def test_gpio_name_not_found(make_edt):
7878
assert " - red" in out
7979

8080

81+
def test_cells_are_not_gpio_specific(make_edt):
82+
# Nothing here keys off 'gpios': the handler runs on any phandle-array property, of
83+
# which Zephyr's in-tree bindings declare several hundred distinct names
84+
edt, _ = make_edt(DTS_SPECIFIERS)
85+
out = diagnose(edt, "_P_pwms_IDX_0_VAL_chanel")
86+
assert "has no 'chanel' cell in entry 0 of the 'pwms' property." in out
87+
assert "Entry 0 is controlled by 'pwm0: /pwm@40003000'" in out
88+
assert " - channel (currently 3)" in out
89+
assert " - period (currently 5000)" in out
90+
91+
92+
def test_non_gpio_specifier_index_out_of_range(make_edt):
93+
edt, _ = make_edt(DTS_SPECIFIERS)
94+
out = diagnose(edt, "_P_pwms_IDX_2_VAL_channel")
95+
assert "has no entry 2 in the 'pwms' property -- there is only 1." in out
96+
97+
8198
def test_unknown_interrupt_cell(make_edt):
8299
edt, _ = make_edt(DTS_SPECIFIERS)
83100
out = diagnose(edt, "_IRQ_IDX_0_VAL_priorty")

0 commit comments

Comments
 (0)