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

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

[Niklas Söderlund]

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 <niklas.soderlund+renesas@ragnatech.se>
---
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,
 };
-- 
2.50.1

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

[Rob Herring]

On Tue, Aug 12, 2025 at 11:51 AM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> 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 <niklas.soderlund+renesas@ragnatech.se>
> ---
> Changes since v2
> - Rebase after 6.17-rc1 merge window closed.
> ---
>  checks.c | 27 +--------------------------
>  1 file changed, 1 insertion(+), 26 deletions(-)

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3596 bytes --]

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
> <niklas.soderlund+renesas@ragnatech.se>

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Niklas Söderlund]

Hi David,

Thanks for your feedback.

On 2025-08-16 14:45:58 +1000, David Gibson wrote:
> 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
> > <niklas.soderlund+renesas@ragnatech.se>
> 
> This causes test suite failures.  You'll need to also remove the
> explicit graph_child_address test from the testsuite.

Thanks for pointing this out. I'm no expert on DTC, this is the 
references to it in tests/run_tests.sh? Is there any more locations I 
should check?

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



-- 
Kind Regards,
Niklas Söderlund

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4646 bytes --]

On Sun, Aug 17, 2025 at 08:42:59AM +0200, Niklas Söderlund wrote:
> Hi David,
> 
> Thanks for your feedback.
> 
> On 2025-08-16 14:45:58 +1000, David Gibson wrote:
> > 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
> > > <niklas.soderlund+renesas@ragnatech.se>
> > 
> > This causes test suite failures.  You'll need to also remove the
> > explicit graph_child_address test from the testsuite.
> 
> Thanks for pointing this out. I'm no expert on DTC, this is the 
> references to it in tests/run_tests.sh? Is there any more locations I 
> should check?

No, that's it.  "make check" should run the full suite for you; good
practice before sending patches.  If you want you can use the github
MR mechanism which will also run the tests as part of its CI.
Old-school patches on the mailing list are fine too, though.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Niklas Söderlund]

On 2025-08-17 18:19:46 +1000, David Gibson wrote:
> On Sun, Aug 17, 2025 at 08:42:59AM +0200, Niklas Söderlund wrote:
> > Hi David,
> > 
> > Thanks for your feedback.
> > 
> > On 2025-08-16 14:45:58 +1000, David Gibson wrote:
> > > 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
> > > > <niklas.soderlund+renesas@ragnatech.se>
> > > 
> > > This causes test suite failures.  You'll need to also remove the
> > > explicit graph_child_address test from the testsuite.
> > 
> > Thanks for pointing this out. I'm no expert on DTC, this is the 
> > references to it in tests/run_tests.sh? Is there any more locations I 
> > should check?
> 
> No, that's it.  "make check" should run the full suite for you; good
> practice before sending patches.  If you want you can use the github
> MR mechanism which will also run the tests as part of its CI.
> Old-school patches on the mailing list are fine too, though.

Thanks, indeed 'make check' catches the issue. I did not know about 
that, will fix for next version.

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



-- 
Kind Regards,
Niklas Söderlund