Re: [PATCH 5/5] checks: add aliases node checks

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

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

On Fri, Nov 17, 2017 at 08:45:15AM -0600, Rob Herring wrote:
> Add checks for aliases node that properties are a valid path and that the
> alias property names are a known, standard name.
> 
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

The check for valid paths is definitely good.  I'm not convinced by
the restrictive list of alias names though.

It's true that in trees originating from dts files, aliases don't have
a lot of purpose, but in trees from real OF that's not so.  It's
pretty reasonable for those to define arbitrary aliases for user
convenience - and the names here aren't even the common ones for that
purpose: those are "net", "disk" and "cd" so you can do things like
"boot disk" from the user interface without having to enter a hideous
path giving PCI and SCSI addresses.

> ---
>  checks.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/checks.c b/checks.c
> index a785b81bea07..2c5b6c2eacb3 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -637,6 +637,48 @@ static void check_names_is_string_list(struct check *c, struct dt_info *dti,
>  }
>  WARNING(names_is_string_list, check_names_is_string_list, NULL);
>  
> +static void check_alias_paths(struct check *c, struct dt_info *dti,
> +				    struct node *node)
> +{
> +	struct property *prop;
> +
> +	if (!streq(node->name, "aliases"))
> +		return;
> +
> +	for_each_property(node, prop) {
> +		if (!prop->val.val || !get_node_by_path(dti->dt, prop->val.val))
> +			FAIL(c, dti, "aliases property '%s' is not a valid node (%s)",
> +			     prop->name, prop->val.val);
> +	}
> +}
> +WARNING(alias_paths, check_alias_paths, NULL);
> +
> +static void check_known_aliases(struct check *c, struct dt_info *dti,
> +				    struct node *node)
> +{
> +	int i;
> +	struct property *prop;
> +	static char *aliases_strings[] = {
> +		"ethernet", "gpio", "i2c", "rtc", "serial", "spi"
> +	};
> +
> +	if (!streq(node->name, "aliases"))
> +		return;
> +
> +	for_each_property(node, prop) {
> +		for (i = 0; i < ARRAY_SIZE(aliases_strings); i++) {
> +			if (strstarts(prop->name, aliases_strings[i]))
> +				break;
> +		}
> +
> +		if (i == ARRAY_SIZE(aliases_strings)) {
> +			FAIL(c, dti, "unknown alias name %s", prop->name);
> +			continue;
> +		}
> +	}
> +}
> +WARNING(known_aliases, check_known_aliases, NULL);
> +
>  static void fixup_addr_size_cells(struct check *c, struct dt_info *dti,
>  				  struct node *node)
>  {
> @@ -1355,6 +1397,8 @@ static struct check *check_table[] = {
>  	&gpios_property,
>  	&interrupts_property,
>  
> +	&known_aliases,	&alias_paths,
> +
>  	&always_fail,
>  };
>  

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