Exempt 'ports' from the rule which asserts that nodes with single child
node having reg = 0, should not have the '#size-cells' and
'#address-cells' properties.
Ports of certain hardware do need to be described as only having a
single child node 'port@0', especially when hardware has multiple ports,
and the other ports 'port@x' are planned to be added subsequently. In
such cases, just using 'port', would be an inaccurate hardware
description.
For example, Texas Instruments' DSS (display-subsystem), which has 2 or
4 video ports depending on the SoC. Describing the first video port with
just 'port' under ports would be inaccurate and even slightly
misleading. Simply using port@0 (when other ports are not added)
produces the following warning, while making dtbs with W=1 flag set[0].
code-block ::
Warning (graph_child_address): /bus@100000/dss@4a00000/ports:
graph node has single child node 'port@0',
#address-cells/#size-cells are not necessary
Signed-off-by: Aradhya Bhatia <[email protected]>
[0]: https://lore.kernel.org/all/[email protected]/
---
scripts/dtc/checks.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/scripts/dtc/checks.c b/scripts/dtc/checks.c
index 9f31d2607182..705aa0fbcfa2 100644
--- a/scripts/dtc/checks.c
+++ b/scripts/dtc/checks.c
@@ -1797,9 +1797,18 @@ static void check_graph_child_address(struct check *c, struct dt_info *dti,
cnt++;
}
- if (cnt == 1 && node->addr_cells != -1)
+ if (cnt == 1 && node->addr_cells != -1) {
+ /*
+ * The graph node "ports" are exempt from this rule, because
+ * certain hardware do need to be described as only having a
+ * signle port with reg = 0.
+ */
+ if (!strcmp(node->name, "ports"))
+ return;
+
FAIL(c, dti, node, "graph node has single child node '%s', #address-cells/#size-cells are not necessary",
node->children->name);
+ }
}
WARNING(graph_child_address, check_graph_child_address, NULL, &graph_nodes);
base-commit: e3b18f7200f45d66f7141136c25554ac1e82009b
--
2.40.1
On Fri, Oct 13, 2023 at 02:24:24PM +0530, Aradhya Bhatia wrote:
> Exempt 'ports' from the rule which asserts that nodes with single child
> node having reg = 0, should not have the '#size-cells' and
> '#address-cells' properties.
>
> Ports of certain hardware do need to be described as only having a
> single child node 'port@0', especially when hardware has multiple ports,
> and the other ports 'port@x' are planned to be added subsequently. In
> such cases, just using 'port', would be an inaccurate hardware
> description.
>
> For example, Texas Instruments' DSS (display-subsystem), which has 2 or
> 4 video ports depending on the SoC. Describing the first video port with
> just 'port' under ports would be inaccurate and even slightly
> misleading. Simply using port@0 (when other ports are not added)
> produces the following warning, while making dtbs with W=1 flag set[0].
There's a reason this is behind W=1.
In general, if you only have a single 'port' it should be just 'port'
which is equivalent to port 0. There's exceptions to that, so the
warning is off by default.
> code-block ::
>
> Warning (graph_child_address): /bus@100000/dss@4a00000/ports:
> graph node has single child node 'port@0',
> #address-cells/#size-cells are not necessary
>
> Signed-off-by: Aradhya Bhatia <[email protected]>
>
> [0]: https://lore.kernel.org/all/[email protected]/
> ---
> scripts/dtc/checks.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
This is a copy of upstream dtc. We don't take patches for it (except in
emergency). Look at the commit history.
Rob
On 26-Oct-23 01:01, Rob Herring wrote:
> On Fri, Oct 13, 2023 at 02:24:24PM +0530, Aradhya Bhatia wrote:
>> Exempt 'ports' from the rule which asserts that nodes with single child
>> node having reg = 0, should not have the '#size-cells' and
>> '#address-cells' properties.
>>
>> Ports of certain hardware do need to be described as only having a
>> single child node 'port@0', especially when hardware has multiple ports,
>> and the other ports 'port@x' are planned to be added subsequently. In
>> such cases, just using 'port', would be an inaccurate hardware
>> description.
>>
>> For example, Texas Instruments' DSS (display-subsystem), which has 2 or
>> 4 video ports depending on the SoC. Describing the first video port with
>> just 'port' under ports would be inaccurate and even slightly
>> misleading. Simply using port@0 (when other ports are not added)
>> produces the following warning, while making dtbs with W=1 flag set[0].
>
> There's a reason this is behind W=1.
>
> In general, if you only have a single 'port' it should be just 'port'
> which is equivalent to port 0. There's exceptions to that, so the
> warning is off by default.
Thank you for reviewing the patch, Rob!
I had a discussion offline, and I agree that the patch may not be needed
after all.
Moreover, upon looking at the tests provided in upstream dtc tree, I
also realized that the check is exclusively limited to port@0, and does
not include any random 'child@0'. This makes the patch make a lot less
sense too.
Regards
Aradhya
>
>> code-block ::
>>
>> Warning (graph_child_address): /bus@100000/dss@4a00000/ports:
>> graph node has single child node 'port@0',
>> #address-cells/#size-cells are not necessary
>>
>> Signed-off-by: Aradhya Bhatia <[email protected]>
>>
>> [0]: https://lore.kernel.org/all/[email protected]/
>> ---
>> scripts/dtc/checks.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> This is a copy of upstream dtc. We don't take patches for it (except in
> emergency). Look at the commit history.
>
> Rob