[PATCH 0/4] checks: clean up string parsing

[PATCH 0/4] checks: clean up string parsing

[Ilya Lipnitskiy]

Add a new function, strends(), as suggested in [0]. Replace existing
suffix logic with calls to strends().

Make nr-gpios exception stricter by aligning with the devicetree spec
and the logic in the Linux kernel.

Add a test case to cover correct non-deprecated GPIO properties.

[0]: https://www.spinics.net/lists/devicetree-compiler/msg03634.html

Ilya Lipnitskiy (4):
  checks: tigthen up nr-gpios prop exception
  dtc.h: add strends for suffix matching
  checks: replace strstr and strrchr with strends
  tests: add a positive gpio test case

 checks.c            | 25 +++++++------------------
 dtc.h               | 10 ++++++++++
 tests/good-gpio.dts | 12 ++++++++++++
 tests/run_tests.sh  |  2 ++
 4 files changed, 31 insertions(+), 18 deletions(-)
 create mode 100644 tests/good-gpio.dts

-- 
2.31.1

[PATCH 1/4] checks: tigthen up nr-gpios prop exception

[Ilya Lipnitskiy]

There are no instances of nr-gpio in the Linux kernel tree, only
"[<vendor>,]nr-gpios", so make the check stricter.

nr-gpios without a "vendor," prefix is also invalid, according to the DT
spec[0], and there are no DT files in the Linux kernel tree with
non-vendor nr-gpios. There are some drivers, but they are not DT spec
compliant, so don't suppress the check for them.

[0]:
Link: https://github.com/devicetree-org/dt-schema/blob/cb53a16a1eb3e2169ce170c071e47940845ec26e/schemas/gpio/gpio-consumer.yaml#L20

Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 checks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/checks.c b/checks.c
index c4207720ce8c..7e9d73af02a3 100644
--- a/checks.c
+++ b/checks.c
@@ -1494,7 +1494,7 @@ static bool prop_is_gpio(struct property *prop)
 	 * *-gpios and *-gpio can appear in property names,
 	 * so skip over any false matches (only one known ATM)
 	 */
-	if (strstr(prop->name, "nr-gpio"))
+	if (strstr(prop->name, ",nr-gpios"))
 		return false;
 
 	str = strrchr(prop->name, '-');
-- 
2.31.1

[PATCH 2/4] dtc.h: add strends for suffix matching

[Ilya Lipnitskiy]

Logic is similar to strcmp_suffix in <kernel>/drivers/of/property.c with
the exception that strends allows string length to equal suffix length.

Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 dtc.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/dtc.h b/dtc.h
index d3e82fb8e3db..62963617c79a 100644
--- a/dtc.h
+++ b/dtc.h
@@ -86,6 +86,16 @@ static inline uint64_t dtb_ld64(const void *p)
 #define streq(a, b)	(strcmp((a), (b)) == 0)
 #define strstarts(s, prefix)	(strncmp((s), (prefix), strlen(prefix)) == 0)
 #define strprefixeq(a, n, b)	(strlen(b) == (n) && (memcmp(a, b, n) == 0))
+static inline bool strends(const char *str, const char *suffix)
+{
+	unsigned int len, suffix_len;
+
+	len = strlen(str);
+	suffix_len = strlen(suffix);
+	if (len < suffix_len)
+		return false;
+	return streq(str + len - suffix_len, suffix);
+}
 
 #define ALIGN(x, a)	(((x) + (a) - 1) & ~((a) - 1))
 
-- 
2.31.1

[PATCH 3/4] checks: replace strstr and strrchr with strends

[Ilya Lipnitskiy]

Makes the logic more clear

Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 checks.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/checks.c b/checks.c
index 7e9d73af02a3..eb91c8ea22fa 100644
--- a/checks.c
+++ b/checks.c
@@ -687,8 +687,7 @@ static void check_names_is_string_list(struct check *c, struct dt_info *dti,
 	struct property *prop;
 
 	for_each_property(node, prop) {
-		const char *s = strrchr(prop->name, '-');
-		if (!s || !streq(s, "-names"))
+		if (!strends(prop->name, "-names"))
 			continue;
 
 		c->data = prop->name;
@@ -1488,24 +1487,17 @@ WARNING_PROPERTY_PHANDLE_CELLS(thermal_sensors, "thermal-sensors", "#thermal-sen
 
 static bool prop_is_gpio(struct property *prop)
 {
-	char *str;
-
 	/*
 	 * *-gpios and *-gpio can appear in property names,
 	 * so skip over any false matches (only one known ATM)
 	 */
-	if (strstr(prop->name, ",nr-gpios"))
+	if (strends(prop->name, ",nr-gpios"))
 		return false;
 
-	str = strrchr(prop->name, '-');
-	if (str)
-		str++;
-	else
-		str = prop->name;
-	if (!(streq(str, "gpios") || streq(str, "gpio")))
-		return false;
-
-	return true;
+	return strends(prop->name, "-gpios") ||
+		streq(prop->name, "gpios") ||
+		strends(prop->name, "-gpio") ||
+		streq(prop->name, "gpio");
 }
 
 static void check_gpios_property(struct check *c,
@@ -1540,13 +1532,10 @@ static void check_deprecated_gpio_property(struct check *c,
 	struct property *prop;
 
 	for_each_property(node, prop) {
-		char *str;
-
 		if (!prop_is_gpio(prop))
 			continue;
 
-		str = strstr(prop->name, "gpio");
-		if (!streq(str, "gpio"))
+		if (!strends(prop->name, "gpio"))
 			continue;
 
 		FAIL_PROP(c, dti, node, prop,
-- 
2.31.1

[PATCH 4/4] tests: add a positive gpio test case

[Ilya Lipnitskiy]

Ensure that properly named properties don't trigger warnings

Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 tests/good-gpio.dts | 12 ++++++++++++
 tests/run_tests.sh  |  2 ++
 2 files changed, 14 insertions(+)
 create mode 100644 tests/good-gpio.dts

diff --git a/tests/good-gpio.dts b/tests/good-gpio.dts
new file mode 100644
index 000000000000..65d1c177afa5
--- /dev/null
+++ b/tests/good-gpio.dts
@@ -0,0 +1,12 @@
+/dts-v1/;
+
+/ {
+	gpio: gpio-controller {
+		#gpio-cells = <3>;
+	};
+
+	node {
+		foo,nr-gpios = <1>;
+		foo-gpios = <&gpio 1 2 3>;
+	};
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 5c2b1e8dc29b..0e8ecdb40c2c 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -709,10 +709,12 @@ dtc_tests () {
     check_tests "$SRCDIR/unit-addr-unique.dts" unique_unit_address
     check_tests "$SRCDIR/bad-phandle-cells.dts" interrupts_extended_property
     check_tests "$SRCDIR/bad-gpio.dts" gpios_property
+    check_tests "$SRCDIR/good-gpio.dts" -n gpios_property
     check_tests "$SRCDIR/bad-graph.dts" graph_child_address
     check_tests "$SRCDIR/bad-graph.dts" graph_port
     check_tests "$SRCDIR/bad-graph.dts" graph_endpoint
     run_sh_test "$SRCDIR/dtc-checkfails.sh" deprecated_gpio_property -- -Wdeprecated_gpio_property -I dts -O dtb "$SRCDIR/bad-gpio.dts"
+    run_sh_test "$SRCDIR/dtc-checkfails.sh" -n deprecated_gpio_property -- -Wdeprecated_gpio_property -I dts -O dtb "$SRCDIR/good-gpio.dts"
     check_tests "$SRCDIR/bad-interrupt-cells.dts" interrupts_property
     check_tests "$SRCDIR/bad-interrupt-controller.dts" interrupt_provider
     run_sh_test "$SRCDIR/dtc-checkfails.sh" node_name_chars -- -I dtb -O dtb bad_node_char.dtb
-- 
2.31.1

Re: [PATCH 0/4] checks: clean up string parsing

[David Gibson]

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

On Mon, May 03, 2021 at 08:59:40PM -0700, Ilya Lipnitskiy wrote:
> Add a new function, strends(), as suggested in [0]. Replace existing
> suffix logic with calls to strends().
> 
> Make nr-gpios exception stricter by aligning with the devicetree spec
> and the logic in the Linux kernel.
> 
> Add a test case to cover correct non-deprecated GPIO properties.

Applied, thanks.

> 
> [0]: https://www.spinics.net/lists/devicetree-compiler/msg03634.html
> 
> Ilya Lipnitskiy (4):
>   checks: tigthen up nr-gpios prop exception
>   dtc.h: add strends for suffix matching
>   checks: replace strstr and strrchr with strends
>   tests: add a positive gpio test case
> 
>  checks.c            | 25 +++++++------------------
>  dtc.h               | 10 ++++++++++
>  tests/good-gpio.dts | 12 ++++++++++++
>  tests/run_tests.sh  |  2 ++
>  4 files changed, 31 insertions(+), 18 deletions(-)
>  create mode 100644 tests/good-gpio.dts
> 

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