[PATCH v3] checks: add chosen node checks

[PATCH v3] checks: add chosen node checks

[Rob Herring]

Add some checks for /chosen node. These check that chosen is located at
the root level and that bootargs and stdout-path properties are strings.

Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
v3:
- Split into 3 separate checks
- Move next to check_obsolete_chosen_interrupt_controller

v2:
- New patch making an explicit chosen node test.

 checks.c             | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/bad-chosen.dts | 10 ++++++++++
 tests/run_tests.sh   |  3 +++
 3 files changed, 64 insertions(+)
 create mode 100644 tests/bad-chosen.dts

diff --git a/checks.c b/checks.c
index 66e5fd6405a5..1cded3658491 100644
--- a/checks.c
+++ b/checks.c
@@ -1052,6 +1052,56 @@ static void check_obsolete_chosen_interrupt_controller(struct check *c,
 WARNING(obsolete_chosen_interrupt_controller,
 	check_obsolete_chosen_interrupt_controller, NULL);
 
+static void check_chosen_node_is_root(struct check *c, struct dt_info *dti,
+				      struct node *node)
+{
+	if (!streq(node->name, "chosen"))
+		return;
+
+	if (node->parent != dti->dt)
+		FAIL(c, dti, "chosen node '%s' must be at root node",
+		     node->fullpath);
+}
+WARNING(chosen_node_is_root, check_chosen_node_is_root, NULL);
+
+static void check_chosen_node_bootargs(struct check *c, struct dt_info *dti,
+				       struct node *node)
+{
+	struct property *prop;
+
+	if (!streq(node->name, "chosen"))
+		return;
+
+	prop = get_property(node, "bootargs");
+	if (!prop)
+		return;
+
+	c->data = prop->name;
+	check_is_string(c, dti, node);
+}
+WARNING(chosen_node_bootargs, check_chosen_node_bootargs, NULL);
+
+static void check_chosen_node_stdout_path(struct check *c, struct dt_info *dti,
+					  struct node *node)
+{
+	struct property *prop;
+
+	if (!streq(node->name, "chosen"))
+		return;
+
+	prop = get_property(node, "stdout-path");
+	if (!prop) {
+		prop = get_property(node, "linux,stdout-path");
+		if (!prop)
+			return;
+		FAIL(c, dti, "Use 'stdout-path' instead of 'linux,stdout-path'");
+	}
+
+	c->data = prop->name;
+	check_is_string(c, dti, node);
+}
+WARNING(chosen_node_stdout_path, check_chosen_node_stdout_path, NULL);
+
 struct provider {
 	const char *prop_name;
 	const char *cell_name;
@@ -1354,6 +1404,7 @@ static struct check *check_table[] = {
 	&avoid_default_addr_size,
 	&avoid_unnecessary_addr_size,
 	&obsolete_chosen_interrupt_controller,
+	&chosen_node_is_root, &chosen_node_bootargs, &chosen_node_stdout_path,
 
 	&clocks_property,
 	&cooling_device_property,
diff --git a/tests/bad-chosen.dts b/tests/bad-chosen.dts
new file mode 100644
index 000000000000..d6f53c68ddd9
--- /dev/null
+++ b/tests/bad-chosen.dts
@@ -0,0 +1,10 @@
+/dts-v1/;
+
+/ {
+	node2 {
+		chosen {
+			bootargs = <0xdeadbeef>;
+			stdout-path = <1>;
+		};
+	};
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index d36dffbc4ef9..61c95f1fc899 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -547,6 +547,9 @@ dtc_tests () {
 
     check_tests bad-ncells.dts address_cells_is_cell size_cells_is_cell interrupt_cells_is_cell
     check_tests bad-string-props.dts device_type_is_string model_is_string status_is_string label_is_string compatible_is_string_list names_is_string_list
+    check_tests bad-chosen.dts chosen_node_is_root
+    check_tests bad-chosen.dts chosen_node_bootargs
+    check_tests bad-chosen.dts chosen_node_stdout_path
     check_tests bad-reg-ranges.dts reg_format ranges_format
     check_tests bad-empty-ranges.dts ranges_format
     check_tests reg-ranges-root.dts reg_format ranges_format
-- 
2.14.1

Re: [PATCH v3] checks: add chosen node checks

[David Gibson]

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

On Thu, Dec 14, 2017 at 04:40:01PM -0600, Rob Herring wrote:
> Add some checks for /chosen node. These check that chosen is located at
> the root level and that bootargs and stdout-path properties are strings.
> 
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Applied, thanks.

One possible follow-on...

[snip]
> +static void check_chosen_node_stdout_path(struct check *c, struct dt_info *dti,
> +					  struct node *node)
> +{
> +	struct property *prop;
> +
> +	if (!streq(node->name, "chosen"))
> +		return;
> +
> +	prop = get_property(node, "stdout-path");
> +	if (!prop) {
> +		prop = get_property(node, "linux,stdout-path");
> +		if (!prop)
> +			return;
> +		FAIL(c, dti, "Use 'stdout-path' instead of 'linux,stdout-path'");
> +	}
> +
> +	c->data = prop->name;
> +	check_is_string(c, dti, node);

.. you could also check that it's actually a valid path to a node,
couldn't you?

> +}
> +WARNING(chosen_node_stdout_path, check_chosen_node_stdout_path, NULL);
> +
>  struct provider {
>  	const char *prop_name;
>  	const char *cell_name;
> @@ -1354,6 +1404,7 @@ static struct check *check_table[] = {
>  	&avoid_default_addr_size,
>  	&avoid_unnecessary_addr_size,
>  	&obsolete_chosen_interrupt_controller,
> +	&chosen_node_is_root, &chosen_node_bootargs, &chosen_node_stdout_path,
>  
>  	&clocks_property,
>  	&cooling_device_property,
> diff --git a/tests/bad-chosen.dts b/tests/bad-chosen.dts
> new file mode 100644
> index 000000000000..d6f53c68ddd9
> --- /dev/null
> +++ b/tests/bad-chosen.dts
> @@ -0,0 +1,10 @@
> +/dts-v1/;
> +
> +/ {
> +	node2 {
> +		chosen {
> +			bootargs = <0xdeadbeef>;
> +			stdout-path = <1>;
> +		};
> +	};
> +};
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index d36dffbc4ef9..61c95f1fc899 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -547,6 +547,9 @@ dtc_tests () {
>  
>      check_tests bad-ncells.dts address_cells_is_cell size_cells_is_cell interrupt_cells_is_cell
>      check_tests bad-string-props.dts device_type_is_string model_is_string status_is_string label_is_string compatible_is_string_list names_is_string_list
> +    check_tests bad-chosen.dts chosen_node_is_root
> +    check_tests bad-chosen.dts chosen_node_bootargs
> +    check_tests bad-chosen.dts chosen_node_stdout_path
>      check_tests bad-reg-ranges.dts reg_format ranges_format
>      check_tests bad-empty-ranges.dts ranges_format
>      check_tests reg-ranges-root.dts reg_format ranges_format

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

Re: [PATCH v3] checks: add chosen node checks

[Rob Herring]

On Thu, Dec 14, 2017 at 8:27 PM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Thu, Dec 14, 2017 at 04:40:01PM -0600, Rob Herring wrote:
>> Add some checks for /chosen node. These check that chosen is located at
>> the root level and that bootargs and stdout-path properties are strings.
>>
>> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> Applied, thanks.
>
> One possible follow-on...
>
> [snip]
>> +static void check_chosen_node_stdout_path(struct check *c, struct dt_info *dti,
>> +                                       struct node *node)
>> +{
>> +     struct property *prop;
>> +
>> +     if (!streq(node->name, "chosen"))
>> +             return;
>> +
>> +     prop = get_property(node, "stdout-path");
>> +     if (!prop) {
>> +             prop = get_property(node, "linux,stdout-path");
>> +             if (!prop)
>> +                     return;
>> +             FAIL(c, dti, "Use 'stdout-path' instead of 'linux,stdout-path'");
>> +     }
>> +
>> +     c->data = prop->name;
>> +     check_is_string(c, dti, node);
>
> .. you could also check that it's actually a valid path to a node,
> couldn't you?

Yes I thought of that, but it's often an alias. I didn't look closely,
but I don't think that would work unless I add that additional lookup.

Rob

Re: [PATCH v3] checks: add chosen node checks

[David Gibson]

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

On Thu, Dec 14, 2017 at 09:11:52PM -0600, Rob Herring wrote:
> On Thu, Dec 14, 2017 at 8:27 PM, David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > On Thu, Dec 14, 2017 at 04:40:01PM -0600, Rob Herring wrote:
> >> Add some checks for /chosen node. These check that chosen is located at
> >> the root level and that bootargs and stdout-path properties are strings.
> >>
> >> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >
> > Applied, thanks.
> >
> > One possible follow-on...
> >
> > [snip]
> >> +static void check_chosen_node_stdout_path(struct check *c, struct dt_info *dti,
> >> +                                       struct node *node)
> >> +{
> >> +     struct property *prop;
> >> +
> >> +     if (!streq(node->name, "chosen"))
> >> +             return;
> >> +
> >> +     prop = get_property(node, "stdout-path");
> >> +     if (!prop) {
> >> +             prop = get_property(node, "linux,stdout-path");
> >> +             if (!prop)
> >> +                     return;
> >> +             FAIL(c, dti, "Use 'stdout-path' instead of 'linux,stdout-path'");
> >> +     }
> >> +
> >> +     c->data = prop->name;
> >> +     check_is_string(c, dti, node);
> >
> > .. you could also check that it's actually a valid path to a node,
> > couldn't you?
> 
> Yes I thought of that, but it's often an alias. I didn't look closely,
> but I don't think that would work unless I add that additional
> lookup.

Ah, good point, you would need to check against aliases as well.  We
probably should have a helper that looks up a path, including alias
resolution (so both "/absolute/path" and "alias/path/from/alias").
Both forms are traditionally allowed in most contexts that accept a
path in OF.

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