Re: [PATCH 4/5] checks: check for #{size,address}-cells without child nodes

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

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

On Fri, Nov 17, 2017 at 08:45:14AM -0600, Rob Herring wrote:
> nodes with a 'reg' property nor a ranges property.
> 
> An exception may be an overlay that adds nodes, but this case would
> need

Sentence doesn't seem finished..

In any case, I'm not sure this is a good idea.  It's not uncommon to
have bus bridge nodes that ought to have a well defined #address and
#size cells, but just don't happen to have any plugged devices yet.
An overlay that adds nodes is one possibility, but a bus where the
children can be probed is another.

The check for 'ranges' will get some of those cases, but a bus bridge
which doesn't directly map the child address space into the parents
(e.g. indirect access) is perfectly legitimate still.

> 
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  checks.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/checks.c b/checks.c
> index 346b0256f9cb..a785b81bea07 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -982,6 +982,31 @@ static void check_avoid_default_addr_size(struct check *c, struct dt_info *dti,
>  WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
>  	&addr_size_cells);
>  
> +static void check_avoid_unecessary_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;
> +
> +	if (get_property(node, "ranges") || !node->children)
> +		return;
> +
> +	for_each_child(node, child) {
> +		prop = get_property(child, "reg");
> +		if (prop)
> +			has_reg = true;
> +	}
> +
> +	if (!has_reg)
> +		FAIL(c, dti, "unnecessary #address-cells/#size-cells without \"ranges\" or child \"reg\" property in %s",
> +		     node->fullpath);
> +}
> +WARNING(avoid_unecessary_addr_size, check_avoid_unecessary_addr_size, NULL, &avoid_default_addr_size);
> +
>  static void check_obsolete_chosen_interrupt_controller(struct check *c,
>  						       struct dt_info *dti,
>  						       struct node *node)
> @@ -1306,6 +1331,7 @@ static struct check *check_table[] = {
>  	&simple_bus_reg,
>  
>  	&avoid_default_addr_size,
> +	&avoid_unecessary_addr_size,
>  	&obsolete_chosen_interrupt_controller,
>  
>  	&clocks_property,

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