Re: [PATCH] checks: Document possible false warning for graph child addresses

David Gibson <[email protected]> Tue, 8 Jul 2025 13:07:12 +1000
Newsgroups	org.kernel.vger.devicetree-compiler,org.kernel.vger.linux-renesas-soc
Message-ID	<aGyLYBoJ0c0UcN-8@zatzit>
On Sun, Jul 06, 2025 at 02:26:38PM +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>
> 	    };
> 	};
>     };
> 
> Add a comment in the check to document this.

Hm.  I don't know the graph bindings at all well, so I'll take your
word for it on what's happening here.  But simply documenting this
within the code doesn't seem particularly useful.  Someone running dtc
will still see the bogus error, and they'd have a pretty long way to
go to find this explanation.

Probably better to simply remove the check (and maybe comment that it
would be nice to check further, but we can't adequately it from a
valid case).

> 
> Signed-off-by: Niklas Söderlund <[email protected]>
> ---
> Hello,
> 
> This was previously part of a patch posted to [email protected]
> [1], but as Rob's points out in that thread it should have been posted
> separately to [email protected]. Sorry for not
> realising that dtc changes go to thru a separate tree.
> 
> 1.  https://lore.kernel.org/all/20250702085008.689727-1-niklas.soderlund%[email protected]/
> ---
>  checks.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/checks.c b/checks.c
> index 123f2eb425f4..52d09fcf8d3a 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -1913,6 +1913,11 @@ static void check_graph_child_address(struct check *c, struct dt_info *dti,
>  		cnt++;
>  	}
>  
> +	/*
> +	 * This check can produce false warnings if the bindings allow for more
> +	 * then one endpoint in the node but only one is present and it has a
> +	 * unit address of zero.
> +	 */
>  	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);

-- 
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)