Re: [PATCH v2] checks: Remove check for graph child addresses

David Gibson <[email protected]> Sat, 16 Aug 2025 14:45:58 +1000
Newsgroups	org.kernel.vger.devicetree-compiler,org.kernel.vger.linux-renesas-soc
Message-ID	<aKANBt7QzzMWhIaJ@zatzit>
On Tue, Aug 12, 2025 at 06:51:22PM +0200, Niklas Söderlund wrote:
> The dtc graph_child_address check can't distinguish between bindings
> where there can only be a single endpoint, and cases where there can be
> multiple endpoints.
> 
> In cases where the bindings allow for multiple endpoints but only one is
> described false warnings about unnecessary #address-cells/#size-cells
> can be generated, but only if the endpoint described have an address of
> 0 (A), for single endpoints with a non-zero address (B) no warnings are
> generated.
> 
> A)
>     ports {
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 
> 	port@0 {
> 	    #address-cells = <1>;
> 	    #size-cells = <0>;
> 
> 	    sourceA: endpoint@0 {
> 		reg = <0>
> 	    };
> 	};
>     };
> 
> B)
>     ports {
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 
> 	port@0 {
> 	    #address-cells = <1>;
> 	    #size-cells = <0>;
> 
> 	    sourceB: endpoint@1 {
> 		reg = <1>
> 	    };
> 	};
>     };
> 
> Remove the check as it is somewhat redundant now that we can use schemas
> to validate the full node.
> 
> Signed-off-by: Niklas Söderlund
> <[email protected]>

This causes test suite failures.  You'll need to also remove the
explicit graph_child_address test from the testsuite.

Note that this patch does technically cause an incompatible change:
when I suggested removing this I hadn't realised this was a single
check, rather than a piece of a larger check.  That means that anyone
configuring this test in their scripts will be broken by removing it.
This is probably an obscure enough case that I'm prepared to accept
it, though.

> ---
> Changes since v2
> - Rebase after 6.17-rc1 merge window closed.
> ---
>  checks.c | 27 +--------------------------
>  1 file changed, 1 insertion(+), 26 deletions(-)
> 
> diff --git a/checks.c b/checks.c
> index 7e3fed5005b3..2072e1ea82dc 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -1894,31 +1894,6 @@ static void check_graph_endpoint(struct check *c, struct dt_info *dti,
>  }
>  WARNING(graph_endpoint, check_graph_endpoint, NULL, &graph_nodes);
>  
> -static void check_graph_child_address(struct check *c, struct dt_info *dti,
> -				      struct node *node)
> -{
> -	int cnt = 0;
> -	struct node *child;
> -
> -	if (node->bus != &graph_ports_bus && node->bus != &graph_port_bus)
> -		return;
> -
> -	for_each_child(node, child) {
> -		struct property *prop = get_property(child, "reg");
> -
> -		/* No error if we have any non-zero unit address */
> -                if (prop && propval_cell(prop) != 0 )
> -			return;
> -
> -		cnt++;
> -	}
> -
> -	if (cnt == 1 && node->addr_cells != -1)
> -		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, &graph_port, &graph_endpoint);
> -
>  static struct check *check_table[] = {
>  	&duplicate_node_names, &duplicate_property_names,
>  	&node_name_chars, &node_name_format, &property_name_chars,
> @@ -2005,7 +1980,7 @@ static struct check *check_table[] = {
>  
>  	&alias_paths,
>  
> -	&graph_nodes, &graph_child_address, &graph_port, &graph_endpoint,
> +	&graph_nodes, &graph_port, &graph_endpoint,
>  
>  	&always_fail,
>  };

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson
signature.asc (application/pgp-signature, 833 B)