[PATCH v2] checks: Relax avoid_unnecessary_addr_size check to allow child ranges properties

[PATCH v2] checks: Relax avoid_unnecessary_addr_size check to allow child ranges properties

[Philipp Zabel]

Do not fail the unnecessary #address-cells/#size-cells check if any
children of the node have a "ranges" property.

Suggested-by: Rob Herring <robh@kernel.org>
Link: https://lore.kernel.org/all/CAL_JsqKebRL454poAYZ9i=sCsHqGzmocLy0psQcng-79UWJB-A@mail.gmail.com/
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes in v2:
 - Add comment to child 'reg'/'ranges' check.
 - Add child 'ranges' to failure message.
 - Link to v1: https://lore.kernel.org/all/20241025161307.3629901-1-p.zabel@pengutronix.de/
---
 checks.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/checks.c b/checks.c
index 6e06aeab5503..9e6a7b6a3e97 100644
--- a/checks.c
+++ b/checks.c
@@ -1217,9 +1217,7 @@ WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
 static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *dti,
 					      struct node *node)
 {
-	struct property *prop;
 	struct node *child;
-	bool has_reg = false;
 
 	if (!node->parent || node->addr_cells < 0 || node->size_cells < 0)
 		return;
@@ -1228,13 +1226,18 @@ static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *d
 		return;
 
 	for_each_child(node, child) {
-		prop = get_property(child, "reg");
-		if (prop)
-			has_reg = true;
+		/*
+		 * Even if the child devices' address space is not mapped into
+		 * the parent bus (no 'ranges' property on node), children can
+		 * still have registers on a local bus, or map local addresses
+		 * to another subordinate address space. The properties on the
+		 * child nodes then make #address-cells/#size-cells necessary:
+		 */
+		if (get_property(child, "reg") || get_property(child, "ranges"))
+			return;
 	}
 
-	if (!has_reg)
-		FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" property");
+	FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" or \"ranges\" property");
 }
 WARNING(avoid_unnecessary_addr_size, check_avoid_unnecessary_addr_size, NULL, &avoid_default_addr_size);
 
-- 
2.39.5

Re: [PATCH v2] checks: Relax avoid_unnecessary_addr_size check to allow child ranges properties

[David Gibson]

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

On Wed, Nov 06, 2024 at 02:01:08PM +0100, Philipp Zabel wrote:
> Do not fail the unnecessary #address-cells/#size-cells check if any
> children of the node have a "ranges" property.
> 
> Suggested-by: Rob Herring <robh@kernel.org>
> Link: https://lore.kernel.org/all/CAL_JsqKebRL454poAYZ9i=sCsHqGzmocLy0psQcng-79UWJB-A@mail.gmail.com/
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

Applied, thanks.

Sorry it's taken so long.

> ---
> Changes in v2:
>  - Add comment to child 'reg'/'ranges' check.
>  - Add child 'ranges' to failure message.
>  - Link to v1: https://lore.kernel.org/all/20241025161307.3629901-1-p.zabel@pengutronix.de/
> ---
>  checks.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/checks.c b/checks.c
> index 6e06aeab5503..9e6a7b6a3e97 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -1217,9 +1217,7 @@ WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
>  static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *dti,
>  					      struct node *node)
>  {
> -	struct property *prop;
>  	struct node *child;
> -	bool has_reg = false;
>  
>  	if (!node->parent || node->addr_cells < 0 || node->size_cells < 0)
>  		return;
> @@ -1228,13 +1226,18 @@ static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *d
>  		return;
>  
>  	for_each_child(node, child) {
> -		prop = get_property(child, "reg");
> -		if (prop)
> -			has_reg = true;
> +		/*
> +		 * Even if the child devices' address space is not mapped into
> +		 * the parent bus (no 'ranges' property on node), children can
> +		 * still have registers on a local bus, or map local addresses
> +		 * to another subordinate address space. The properties on the
> +		 * child nodes then make #address-cells/#size-cells necessary:
> +		 */
> +		if (get_property(child, "reg") || get_property(child, "ranges"))
> +			return;
>  	}
>  
> -	if (!has_reg)
> -		FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" property");
> +	FAIL(c, dti, node, "unnecessary #address-cells/#size-cells without \"ranges\", \"dma-ranges\" or child \"reg\" or \"ranges\" property");
>  }
>  WARNING(avoid_unnecessary_addr_size, check_avoid_unnecessary_addr_size, NULL, &avoid_default_addr_size);
>  

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