[PATCH v2 1/6] checks: add a string check for 'label' property

[PATCH v2 1/6] checks: add a string check for 'label' property

[Rob Herring]

Add a string property check for 'label' property. 'label' is a human
readable string typically used to identify connectors or ports on devices.

Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
v2:
- Move chosen node checks to patch 6.
- Add tests.

 checks.c                   | 2 ++
 tests/bad-string-props.dts | 1 +
 tests/run_tests.sh         | 2 +-
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/checks.c b/checks.c
index 334c2a4a0527..8d52d78c1dca 100644
--- a/checks.c
+++ b/checks.c
@@ -586,6 +586,7 @@ WARNING_IF_NOT_CELL(interrupt_cells_is_cell, "#interrupt-cells");
 WARNING_IF_NOT_STRING(device_type_is_string, "device_type");
 WARNING_IF_NOT_STRING(model_is_string, "model");
 WARNING_IF_NOT_STRING(status_is_string, "status");
+WARNING_IF_NOT_STRING(label_is_string, "label");
 
 static void fixup_addr_size_cells(struct check *c, struct dt_info *dti,
 				  struct node *node)
@@ -1236,6 +1237,7 @@ static struct check *check_table[] = {
 
 	&address_cells_is_cell, &size_cells_is_cell, &interrupt_cells_is_cell,
 	&device_type_is_string, &model_is_string, &status_is_string,
+	&label_is_string,
 
 	&property_name_chars_strict,
 	&node_name_chars_strict,
diff --git a/tests/bad-string-props.dts b/tests/bad-string-props.dts
index 396f82069cf7..04194da50636 100644
--- a/tests/bad-string-props.dts
+++ b/tests/bad-string-props.dts
@@ -4,4 +4,5 @@
 	device_type = <0xdeadbeef>;
 	model = <0xdeadbeef>;
 	status = <0xdeadbeef>;
+	label = <0xdeadbeef>;
 };
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 850bc165e757..e7505255043d 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -546,7 +546,7 @@ dtc_tests () {
     check_tests bad-name-property.dts name_properties
 
     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
+    check_tests bad-string-props.dts device_type_is_string model_is_string status_is_string label_is_string
     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

[PATCH v2 2/6] checks: add string list check

[Rob Herring]

Add a check for string list properties with compatible being the first
check.

Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
v2:
- Add tests

 checks.c                   | 34 ++++++++++++++++++++++++++++++++++
 tests/bad-string-props.dts |  5 +++++
 tests/run_tests.sh         |  2 +-
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/checks.c b/checks.c
index 8d52d78c1dca..cf670ddc4a5c 100644
--- a/checks.c
+++ b/checks.c
@@ -179,6 +179,36 @@ static void check_is_string(struct check *c, struct dt_info *dti,
 #define ERROR_IF_NOT_STRING(nm, propname) \
 	ERROR(nm, check_is_string, (propname))
 
+static void check_is_string_list(struct check *c, struct dt_info *dti,
+				 struct node *node)
+{
+	int rem, l;
+	struct property *prop;
+	char *propname = c->data;
+	char *str;
+
+	prop = get_property(node, propname);
+	if (!prop)
+		return; /* Not present, assumed ok */
+
+	str = prop->val.val;
+	rem = prop->val.len;
+	while (rem > 0) {
+		l = strnlen(str, rem);
+		if (l == rem) {
+			FAIL(c, dti, "\"%s\" property in %s is not a string list",
+			     propname, node->fullpath);
+			break;
+		}
+		rem -= l + 1;
+		str += l + 1;
+	}
+}
+#define WARNING_IF_NOT_STRING_LIST(nm, propname) \
+	WARNING(nm, check_is_string_list, (propname))
+#define ERROR_IF_NOT_STRING_LIST(nm, propname) \
+	ERROR(nm, check_is_string_list, (propname))
+
 static void check_is_cell(struct check *c, struct dt_info *dti,
 			  struct node *node)
 {
@@ -588,6 +618,8 @@ WARNING_IF_NOT_STRING(model_is_string, "model");
 WARNING_IF_NOT_STRING(status_is_string, "status");
 WARNING_IF_NOT_STRING(label_is_string, "label");
 
+WARNING_IF_NOT_STRING_LIST(compatible_is_string_list, "compatible");
+
 static void fixup_addr_size_cells(struct check *c, struct dt_info *dti,
 				  struct node *node)
 {
@@ -1239,6 +1271,8 @@ static struct check *check_table[] = {
 	&device_type_is_string, &model_is_string, &status_is_string,
 	&label_is_string,
 
+	&compatible_is_string_list,
+
 	&property_name_chars_strict,
 	&node_name_chars_strict,
 
diff --git a/tests/bad-string-props.dts b/tests/bad-string-props.dts
index 04194da50636..9362ee815965 100644
--- a/tests/bad-string-props.dts
+++ b/tests/bad-string-props.dts
@@ -5,4 +5,9 @@
 	model = <0xdeadbeef>;
 	status = <0xdeadbeef>;
 	label = <0xdeadbeef>;
+
+
+	node {
+		compatible = "good", <0xdeadbeef>;
+	};
 };
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index e7505255043d..910a71cafe16 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -546,7 +546,7 @@ dtc_tests () {
     check_tests bad-name-property.dts name_properties
 
     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
+    check_tests bad-string-props.dts device_type_is_string model_is_string status_is_string label_is_string compatible_is_string_list
     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

[PATCH v2 3/6] checks: add string list check for *-names properties

[Rob Herring]

Add a string list check for common properties ending in "-names" such as
reg-names or interrupt-names.

Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
v2:
- Ensure "-names" is at end of the prop name.
- Add tests

 checks.c                   | 18 +++++++++++++++++-
 tests/bad-string-props.dts |  1 +
 tests/run_tests.sh         |  2 +-
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/checks.c b/checks.c
index cf670ddc4a5c..e4f5cb9fd317 100644
--- a/checks.c
+++ b/checks.c
@@ -620,6 +620,22 @@ WARNING_IF_NOT_STRING(label_is_string, "label");
 
 WARNING_IF_NOT_STRING_LIST(compatible_is_string_list, "compatible");
 
+static void check_names_is_string_list(struct check *c, struct dt_info *dti,
+				       struct node *node)
+{
+	struct property *prop;
+
+	for_each_property(node, prop) {
+		const char *s = strrchr(prop->name, '-');
+		if (!s || !streq(s, "-names"))
+			continue;
+
+		c->data = prop->name;
+		check_is_string_list(c, dti, node);
+	}
+}
+WARNING(names_is_string_list, check_names_is_string_list, NULL);
+
 static void fixup_addr_size_cells(struct check *c, struct dt_info *dti,
 				  struct node *node)
 {
@@ -1271,7 +1287,7 @@ static struct check *check_table[] = {
 	&device_type_is_string, &model_is_string, &status_is_string,
 	&label_is_string,
 
-	&compatible_is_string_list,
+	&compatible_is_string_list, &names_is_string_list,
 
 	&property_name_chars_strict,
 	&node_name_chars_strict,
diff --git a/tests/bad-string-props.dts b/tests/bad-string-props.dts
index 9362ee815965..6694704a5fc2 100644
--- a/tests/bad-string-props.dts
+++ b/tests/bad-string-props.dts
@@ -6,6 +6,7 @@
 	status = <0xdeadbeef>;
 	label = <0xdeadbeef>;
 
+	foobar-names = "foo", <1>;
 
 	node {
 		compatible = "good", <0xdeadbeef>;
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 910a71cafe16..d36dffbc4ef9 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -546,7 +546,7 @@ dtc_tests () {
     check_tests bad-name-property.dts name_properties
 
     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
+    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-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

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

[Rob Herring]

Add a check for unnecessary "#{size,address}-cells" when there's neither
a 'ranges' property nor child nodes with a 'reg' property.

An exception may be an overlay that adds nodes, but this case would need
"#{size,address}-cells" in the overlay to properly compile already.

Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
v2:
- fix commit msg

 checks.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/checks.c b/checks.c
index e4f5cb9fd317..e0ec67be1b7a 100644
--- a/checks.c
+++ b/checks.c
@@ -981,6 +981,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_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;
+
+	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_unnecessary_addr_size, check_avoid_unnecessary_addr_size, NULL, &avoid_default_addr_size);
+
 static void check_obsolete_chosen_interrupt_controller(struct check *c,
 						       struct dt_info *dti,
 						       struct node *node)
@@ -1305,6 +1330,7 @@ static struct check *check_table[] = {
 	&simple_bus_reg,
 
 	&avoid_default_addr_size,
+	&avoid_unnecessary_addr_size,
 	&obsolete_chosen_interrupt_controller,
 
 	&clocks_property,
-- 
2.14.1

[PATCH v2 5/6] checks: add aliases node checks

[Rob Herring]

Add checks for aliases node that all properties follow alias naming
convention and the values are a valid path.

Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
v2:
- Drop known name check
- Add character set check for alias names

 checks.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/checks.c b/checks.c
index e0ec67be1b7a..66e5fd6405a5 100644
--- a/checks.c
+++ b/checks.c
@@ -636,6 +636,28 @@ 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);
+			continue;
+		}
+		if (strspn(prop->name, LOWERCASE DIGITS "-") != strlen(prop->name))
+			FAIL(c, dti, "aliases property name '%s' is not valid",
+			     prop->name);
+
+	}
+}
+WARNING(alias_paths, check_alias_paths, NULL);
+
 static void fixup_addr_size_cells(struct check *c, struct dt_info *dti,
 				  struct node *node)
 {
@@ -1354,6 +1376,8 @@ static struct check *check_table[] = {
 	&gpios_property,
 	&interrupts_property,
 
+	&alias_paths,
+
 	&always_fail,
 };
 
-- 
2.14.1

[PATCH v2 6/6] checks: add a chosen node check

[Rob Herring]

Add a check for /chosen node. This checks 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>
---
v2:
- New patch making an explicit chosen node test.

 checks.c                         | 32 ++++++++++++++++++++++++++++++++
 tests/bad-chosen-bootargs.dts    |  7 +++++++
 tests/bad-chosen-location.dts    |  8 ++++++++
 tests/bad-chosen-stdout-path.dts |  9 +++++++++
 tests/run_tests.sh               |  3 +++
 5 files changed, 59 insertions(+)
 create mode 100644 tests/bad-chosen-bootargs.dts
 create mode 100644 tests/bad-chosen-location.dts
 create mode 100644 tests/bad-chosen-stdout-path.dts

diff --git a/checks.c b/checks.c
index 66e5fd6405a5..234e97ffb905 100644
--- a/checks.c
+++ b/checks.c
@@ -636,6 +636,37 @@ 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_chosen_node(struct check *c, struct dt_info *dti,
+				    struct node *node)
+{
+	struct property *prop;
+
+	if (!streq(node->name, "chosen"))
+		return;
+
+	if (node->parent->parent) {
+		FAIL(c, dti, "chosen node must be at root node (%s)",
+		     node->parent->name);
+	}
+
+	prop = get_property(node, "bootargs");
+	if (prop) {
+		c->data = prop->name;
+		check_is_string(c, dti, node);
+	}
+
+	prop = get_property(node, "linux,stdout-path");
+	if (prop)
+		FAIL(c, dti, "Use 'stdout-path' instead of 'linux,stdout-path'");
+
+	prop = get_property(node, "stdout-path");
+	if (prop) {
+		c->data = prop->name;
+		check_is_string(c, dti, node);
+	}
+}
+WARNING(chosen_node, check_chosen_node, NULL);
+
 static void check_alias_paths(struct check *c, struct dt_info *dti,
 				    struct node *node)
 {
@@ -1377,6 +1408,7 @@ static struct check *check_table[] = {
 	&interrupts_property,
 
 	&alias_paths,
+	&chosen_node,
 
 	&always_fail,
 };
diff --git a/tests/bad-chosen-bootargs.dts b/tests/bad-chosen-bootargs.dts
new file mode 100644
index 000000000000..fb72a820b340
--- /dev/null
+++ b/tests/bad-chosen-bootargs.dts
@@ -0,0 +1,7 @@
+/dts-v1/;
+
+/ {
+	chosen {
+		bootargs = <0xdeadbeef>;
+	};
+};
diff --git a/tests/bad-chosen-location.dts b/tests/bad-chosen-location.dts
new file mode 100644
index 000000000000..0f6de5ed7ed3
--- /dev/null
+++ b/tests/bad-chosen-location.dts
@@ -0,0 +1,8 @@
+/dts-v1/;
+
+/ {
+	node2 {
+		chosen {
+		};
+	};
+};
diff --git a/tests/bad-chosen-stdout-path.dts b/tests/bad-chosen-stdout-path.dts
new file mode 100644
index 000000000000..a022e68e96ec
--- /dev/null
+++ b/tests/bad-chosen-stdout-path.dts
@@ -0,0 +1,9 @@
+/dts-v1/;
+
+/ {
+	chosen {
+		linux,stdout-path = "foo";
+		stdout-path = <1>;
+	};
+
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index d36dffbc4ef9..8b45b81b2e61 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-location.dts chosen_node
+    check_tests bad-chosen-bootargs.dts chosen_node
+    check_tests bad-chosen-stdout-path.dts chosen_node
     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 v2 1/6] checks: add a string check for 'label' property

[David Gibson]

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

On Tue, Dec 12, 2017 at 04:46:24PM -0600, Rob Herring wrote:
> Add a string property check for 'label' property. 'label' is a human
> readable string typically used to identify connectors or ports on devices.
> 
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Applied, thanks.

> ---
> v2:
> - Move chosen node checks to patch 6.
> - Add tests.
> 
>  checks.c                   | 2 ++
>  tests/bad-string-props.dts | 1 +
>  tests/run_tests.sh         | 2 +-
>  3 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/checks.c b/checks.c
> index 334c2a4a0527..8d52d78c1dca 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -586,6 +586,7 @@ WARNING_IF_NOT_CELL(interrupt_cells_is_cell, "#interrupt-cells");
>  WARNING_IF_NOT_STRING(device_type_is_string, "device_type");
>  WARNING_IF_NOT_STRING(model_is_string, "model");
>  WARNING_IF_NOT_STRING(status_is_string, "status");
> +WARNING_IF_NOT_STRING(label_is_string, "label");
>  
>  static void fixup_addr_size_cells(struct check *c, struct dt_info *dti,
>  				  struct node *node)
> @@ -1236,6 +1237,7 @@ static struct check *check_table[] = {
>  
>  	&address_cells_is_cell, &size_cells_is_cell, &interrupt_cells_is_cell,
>  	&device_type_is_string, &model_is_string, &status_is_string,
> +	&label_is_string,
>  
>  	&property_name_chars_strict,
>  	&node_name_chars_strict,
> diff --git a/tests/bad-string-props.dts b/tests/bad-string-props.dts
> index 396f82069cf7..04194da50636 100644
> --- a/tests/bad-string-props.dts
> +++ b/tests/bad-string-props.dts
> @@ -4,4 +4,5 @@
>  	device_type = <0xdeadbeef>;
>  	model = <0xdeadbeef>;
>  	status = <0xdeadbeef>;
> +	label = <0xdeadbeef>;
>  };
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 850bc165e757..e7505255043d 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -546,7 +546,7 @@ dtc_tests () {
>      check_tests bad-name-property.dts name_properties
>  
>      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
> +    check_tests bad-string-props.dts device_type_is_string model_is_string status_is_string label_is_string
>      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 v2 2/6] checks: add string list check

[David Gibson]

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

On Tue, Dec 12, 2017 at 04:46:25PM -0600, Rob Herring wrote:
> Add a check for string list properties with compatible being the first
> check.
> 
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Applied, thanks.

> ---
> v2:
> - Add tests
> 
>  checks.c                   | 34 ++++++++++++++++++++++++++++++++++
>  tests/bad-string-props.dts |  5 +++++
>  tests/run_tests.sh         |  2 +-
>  3 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/checks.c b/checks.c
> index 8d52d78c1dca..cf670ddc4a5c 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -179,6 +179,36 @@ static void check_is_string(struct check *c, struct dt_info *dti,
>  #define ERROR_IF_NOT_STRING(nm, propname) \
>  	ERROR(nm, check_is_string, (propname))
>  
> +static void check_is_string_list(struct check *c, struct dt_info *dti,
> +				 struct node *node)
> +{
> +	int rem, l;
> +	struct property *prop;
> +	char *propname = c->data;
> +	char *str;
> +
> +	prop = get_property(node, propname);
> +	if (!prop)
> +		return; /* Not present, assumed ok */
> +
> +	str = prop->val.val;
> +	rem = prop->val.len;
> +	while (rem > 0) {
> +		l = strnlen(str, rem);
> +		if (l == rem) {
> +			FAIL(c, dti, "\"%s\" property in %s is not a string list",
> +			     propname, node->fullpath);
> +			break;
> +		}
> +		rem -= l + 1;
> +		str += l + 1;
> +	}
> +}
> +#define WARNING_IF_NOT_STRING_LIST(nm, propname) \
> +	WARNING(nm, check_is_string_list, (propname))
> +#define ERROR_IF_NOT_STRING_LIST(nm, propname) \
> +	ERROR(nm, check_is_string_list, (propname))
> +
>  static void check_is_cell(struct check *c, struct dt_info *dti,
>  			  struct node *node)
>  {
> @@ -588,6 +618,8 @@ WARNING_IF_NOT_STRING(model_is_string, "model");
>  WARNING_IF_NOT_STRING(status_is_string, "status");
>  WARNING_IF_NOT_STRING(label_is_string, "label");
>  
> +WARNING_IF_NOT_STRING_LIST(compatible_is_string_list, "compatible");
> +
>  static void fixup_addr_size_cells(struct check *c, struct dt_info *dti,
>  				  struct node *node)
>  {
> @@ -1239,6 +1271,8 @@ static struct check *check_table[] = {
>  	&device_type_is_string, &model_is_string, &status_is_string,
>  	&label_is_string,
>  
> +	&compatible_is_string_list,
> +
>  	&property_name_chars_strict,
>  	&node_name_chars_strict,
>  
> diff --git a/tests/bad-string-props.dts b/tests/bad-string-props.dts
> index 04194da50636..9362ee815965 100644
> --- a/tests/bad-string-props.dts
> +++ b/tests/bad-string-props.dts
> @@ -5,4 +5,9 @@
>  	model = <0xdeadbeef>;
>  	status = <0xdeadbeef>;
>  	label = <0xdeadbeef>;
> +
> +
> +	node {
> +		compatible = "good", <0xdeadbeef>;
> +	};
>  };
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index e7505255043d..910a71cafe16 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -546,7 +546,7 @@ dtc_tests () {
>      check_tests bad-name-property.dts name_properties
>  
>      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
> +    check_tests bad-string-props.dts device_type_is_string model_is_string status_is_string label_is_string compatible_is_string_list
>      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 v2 3/6] checks: add string list check for *-names properties

[David Gibson]

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

On Tue, Dec 12, 2017 at 04:46:26PM -0600, Rob Herring wrote:
> Add a string list check for common properties ending in "-names" such as
> reg-names or interrupt-names.
> 
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Applied, thanks.

> ---
> v2:
> - Ensure "-names" is at end of the prop name.
> - Add tests
> 
>  checks.c                   | 18 +++++++++++++++++-
>  tests/bad-string-props.dts |  1 +
>  tests/run_tests.sh         |  2 +-
>  3 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/checks.c b/checks.c
> index cf670ddc4a5c..e4f5cb9fd317 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -620,6 +620,22 @@ WARNING_IF_NOT_STRING(label_is_string, "label");
>  
>  WARNING_IF_NOT_STRING_LIST(compatible_is_string_list, "compatible");
>  
> +static void check_names_is_string_list(struct check *c, struct dt_info *dti,
> +				       struct node *node)
> +{
> +	struct property *prop;
> +
> +	for_each_property(node, prop) {
> +		const char *s = strrchr(prop->name, '-');
> +		if (!s || !streq(s, "-names"))
> +			continue;
> +
> +		c->data = prop->name;
> +		check_is_string_list(c, dti, node);
> +	}
> +}
> +WARNING(names_is_string_list, check_names_is_string_list, NULL);
> +
>  static void fixup_addr_size_cells(struct check *c, struct dt_info *dti,
>  				  struct node *node)
>  {
> @@ -1271,7 +1287,7 @@ static struct check *check_table[] = {
>  	&device_type_is_string, &model_is_string, &status_is_string,
>  	&label_is_string,
>  
> -	&compatible_is_string_list,
> +	&compatible_is_string_list, &names_is_string_list,
>  
>  	&property_name_chars_strict,
>  	&node_name_chars_strict,
> diff --git a/tests/bad-string-props.dts b/tests/bad-string-props.dts
> index 9362ee815965..6694704a5fc2 100644
> --- a/tests/bad-string-props.dts
> +++ b/tests/bad-string-props.dts
> @@ -6,6 +6,7 @@
>  	status = <0xdeadbeef>;
>  	label = <0xdeadbeef>;
>  
> +	foobar-names = "foo", <1>;
>  
>  	node {
>  		compatible = "good", <0xdeadbeef>;
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 910a71cafe16..d36dffbc4ef9 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -546,7 +546,7 @@ dtc_tests () {
>      check_tests bad-name-property.dts name_properties
>  
>      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
> +    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-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 v2 4/6] checks: check for #{size,address}-cells without child nodes

[David Gibson]

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

On Tue, Dec 12, 2017 at 04:46:27PM -0600, Rob Herring wrote:
> Add a check for unnecessary "#{size,address}-cells" when there's neither
> a 'ranges' property nor child nodes with a 'reg' property.
> 
> An exception may be an overlay that adds nodes, but this case would need
> "#{size,address}-cells" in the overlay to properly compile already.
> 
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

I do worry slightly about the case of a bridge which has some special
subnodes with no reg, plus normal subnodes with reg, but where there
aren't any normal subnodes in the base tree.

Still, that's a fairly corner case, so I've applied the patch.

> ---
> v2:
> - fix commit msg
> 
>  checks.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/checks.c b/checks.c
> index e4f5cb9fd317..e0ec67be1b7a 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -981,6 +981,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_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;
> +
> +	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_unnecessary_addr_size, check_avoid_unnecessary_addr_size, NULL, &avoid_default_addr_size);
> +
>  static void check_obsolete_chosen_interrupt_controller(struct check *c,
>  						       struct dt_info *dti,
>  						       struct node *node)
> @@ -1305,6 +1330,7 @@ static struct check *check_table[] = {
>  	&simple_bus_reg,
>  
>  	&avoid_default_addr_size,
> +	&avoid_unnecessary_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 --]

Re: [PATCH v2 6/6] checks: add a chosen node check

[David Gibson]

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

On Tue, Dec 12, 2017 at 04:46:29PM -0600, Rob Herring wrote:
> Add a check for /chosen node. This checks 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>

I'd prefer to see the pieces of this as individual checks.  See
check_obsolete_interrupt_controller() for a similar existing example.

> ---
> v2:
> - New patch making an explicit chosen node test.
> 
>  checks.c                         | 32 ++++++++++++++++++++++++++++++++
>  tests/bad-chosen-bootargs.dts    |  7 +++++++
>  tests/bad-chosen-location.dts    |  8 ++++++++
>  tests/bad-chosen-stdout-path.dts |  9 +++++++++
>  tests/run_tests.sh               |  3 +++
>  5 files changed, 59 insertions(+)
>  create mode 100644 tests/bad-chosen-bootargs.dts
>  create mode 100644 tests/bad-chosen-location.dts
>  create mode 100644 tests/bad-chosen-stdout-path.dts
> 
> diff --git a/checks.c b/checks.c
> index 66e5fd6405a5..234e97ffb905 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -636,6 +636,37 @@ 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_chosen_node(struct check *c, struct dt_info *dti,
> +				    struct node *node)
> +{
> +	struct property *prop;
> +
> +	if (!streq(node->name, "chosen"))
> +		return;
> +
> +	if (node->parent->parent) {
> +		FAIL(c, dti, "chosen node must be at root node (%s)",
> +		     node->parent->name);
> +	}
> +
> +	prop = get_property(node, "bootargs");
> +	if (prop) {
> +		c->data = prop->name;
> +		check_is_string(c, dti, node);
> +	}
> +
> +	prop = get_property(node, "linux,stdout-path");
> +	if (prop)
> +		FAIL(c, dti, "Use 'stdout-path' instead of 'linux,stdout-path'");
> +
> +	prop = get_property(node, "stdout-path");
> +	if (prop) {
> +		c->data = prop->name;
> +		check_is_string(c, dti, node);
> +	}
> +}
> +WARNING(chosen_node, check_chosen_node, NULL);
> +
>  static void check_alias_paths(struct check *c, struct dt_info *dti,
>  				    struct node *node)
>  {
> @@ -1377,6 +1408,7 @@ static struct check *check_table[] = {
>  	&interrupts_property,
>  
>  	&alias_paths,
> +	&chosen_node,
>  
>  	&always_fail,
>  };
> diff --git a/tests/bad-chosen-bootargs.dts b/tests/bad-chosen-bootargs.dts
> new file mode 100644
> index 000000000000..fb72a820b340
> --- /dev/null
> +++ b/tests/bad-chosen-bootargs.dts
> @@ -0,0 +1,7 @@
> +/dts-v1/;
> +
> +/ {
> +	chosen {
> +		bootargs = <0xdeadbeef>;
> +	};
> +};
> diff --git a/tests/bad-chosen-location.dts b/tests/bad-chosen-location.dts
> new file mode 100644
> index 000000000000..0f6de5ed7ed3
> --- /dev/null
> +++ b/tests/bad-chosen-location.dts
> @@ -0,0 +1,8 @@
> +/dts-v1/;
> +
> +/ {
> +	node2 {
> +		chosen {
> +		};
> +	};
> +};
> diff --git a/tests/bad-chosen-stdout-path.dts b/tests/bad-chosen-stdout-path.dts
> new file mode 100644
> index 000000000000..a022e68e96ec
> --- /dev/null
> +++ b/tests/bad-chosen-stdout-path.dts
> @@ -0,0 +1,9 @@
> +/dts-v1/;
> +
> +/ {
> +	chosen {
> +		linux,stdout-path = "foo";
> +		stdout-path = <1>;
> +	};
> +
> +};
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index d36dffbc4ef9..8b45b81b2e61 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-location.dts chosen_node
> +    check_tests bad-chosen-bootargs.dts chosen_node
> +    check_tests bad-chosen-stdout-path.dts chosen_node
>      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 --]