[PATCH 1/5] test: self: printf: log skipped tests

[PATCH 1/5] test: self: printf: log skipped tests

[Ahmad Fatoum]

Number of skipped tests is reported, but let's report the reason as
well.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 test/self/printf.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/test/self/printf.c b/test/self/printf.c
index 52fe6ac0faf9..ec74df32ee4b 100644
--- a/test/self/printf.c
+++ b/test/self/printf.c
@@ -247,8 +247,11 @@ uuid(void)
 	const char uuid[16] = {0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
 			       0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf};
 
-	if (!IS_ENABLED(CONFIG_PRINTF_UUID))
+	if (!IS_ENABLED(CONFIG_PRINTF_UUID)) {
+		pr_info("skipping UUID tests: disabled in config\n");
+		skipped_tests += 4;
 		return;
+	}
 
 	test("00010203-0405-0607-0809-0a0b0c0d0e0f", "%pUb", uuid);
 	test("00010203-0405-0607-0809-0A0B0C0D0E0F", "%pUB", uuid);
@@ -265,8 +268,11 @@ errptr(void)
 	/* Check that %pe with a non-ERR_PTR gets treated as ordinary %p. */
 	BUILD_BUG_ON(IS_ERR(PTR));
 
-	if (!IS_ENABLED(CONFIG_ERRNO_MESSAGES))
+	if (!IS_ENABLED(CONFIG_ERRNO_MESSAGES)) {
+		pr_info("skipping errno messages tests: disabled in config\n");
+		skipped_tests += 2;
 		return;
+	}
 	test("(Operation not permitted)", "(%pe)", ERR_PTR(-EPERM));
 	test("Requested probe deferral", "%pe", ERR_PTR(-EPROBE_DEFER));
 }
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/5] test: self: printf: add tests for %*ph prints

[Ahmad Fatoum]

This modifier was recently added. Turn the description from its commit
message into a test.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 test/self/printf.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/test/self/printf.c b/test/self/printf.c
index ec74df32ee4b..7a74660868c2 100644
--- a/test/self/printf.c
+++ b/test/self/printf.c
@@ -277,6 +277,23 @@ errptr(void)
 	test("Requested probe deferral", "%pe", ERR_PTR(-EPROBE_DEFER));
 }
 
+static void __init
+test_hexstr(void)
+{
+	u8 buf[4] = { 0, 1, 2, 3 };
+
+	if (!IS_ENABLED(CONFIG_PRINTF_HEXSTR)) {
+		pr_info("skipping hexstr tests: disabled in config\n");
+		skipped_tests += 4;
+		return;
+	}
+
+	test("[00 01 02 03]", "[%*ph]",  (int)sizeof(buf), buf);
+	test("[00:01:02:03]", "[%*phC]", (int)sizeof(buf), buf);
+	test("[00-01-02-03]", "[%*phD]", (int)sizeof(buf), buf);
+	test("[00010203]",    "[%*phN]", (int)sizeof(buf), buf);
+}
+
 static void __init
 test_pointer(void)
 {
@@ -299,6 +316,7 @@ static void __init test_printf(void)
 	test_number();
 	test_string();
 	test_pointer();
+	test_hexstr();
 
 	free(alloced_buffer);
 }
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/5] of: report whether of_diff found differences in return code

[Ahmad Fatoum]

Tests may want to leverage of_diff to verify that fixups proceeded as
expected. of_diff lends itself nicely to that by being silent in case of
success and just reporting diff on error. Add a return code to make it
usable in follow-up tests.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 commands/of_diff.c |  4 +---
 drivers/of/base.c  | 13 ++++++++++---
 include/of.h       |  2 +-
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/commands/of_diff.c b/commands/of_diff.c
index fa99fcd641da..fcae367a8105 100644
--- a/commands/of_diff.c
+++ b/commands/of_diff.c
@@ -77,9 +77,7 @@ static int do_of_diff(int argc, char *argv[])
 		goto out;
 	}
 
-	of_diff(a, b, 0);
-
-	ret = 0;
+	ret = of_diff(a, b, 0) ? COMMAND_SUCCESS : COMMAND_ERROR;
 out:
 	if (a && a != root)
 		of_delete_node(a);
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 80465d6d5062..99be24254203 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2077,15 +2077,16 @@ static void of_print_close(struct device_node *node, int *printed)
  * This function compares two device trees against each other and prints
  * a diff-like result.
  */
-void of_diff(struct device_node *a, struct device_node *b, int indent)
+int of_diff(struct device_node *a, struct device_node *b, int indent)
 {
 	struct property *ap, *bp;
 	struct device_node *ca, *cb;
-	int printed = 0;
+	int printed = 0, diff = 0;
 
 	list_for_each_entry(ap, &a->properties, list) {
 		bp = of_find_property(b, ap->name, NULL);
 		if (!bp) {
+			diff++;
 			of_print_parents(a, &printed);
 			printf("- ");
 			__of_print_property(ap, indent);
@@ -2093,6 +2094,7 @@ void of_diff(struct device_node *a, struct device_node *b, int indent)
 		}
 
 		if (ap->length != bp->length || memcmp(of_property_get_value(ap), of_property_get_value(bp), bp->length)) {
+			diff++;
 			of_print_parents(a, &printed);
 			printf("- ");
 			__of_print_property(ap, indent);
@@ -2104,6 +2106,7 @@ void of_diff(struct device_node *a, struct device_node *b, int indent)
 	list_for_each_entry(bp, &b->properties, list) {
 		ap = of_find_property(a, bp->name, NULL);
 		if (!ap) {
+			diff++;
 			of_print_parents(a, &printed);
 			printf("+ ");
 			__of_print_property(bp, indent);
@@ -2113,8 +2116,9 @@ void of_diff(struct device_node *a, struct device_node *b, int indent)
 	for_each_child_of_node(a, ca) {
 		cb = of_get_child_by_name(b, ca->name);
 		if (cb) {
-			of_diff(ca, cb, indent + 1);
+			diff += of_diff(ca, cb, indent + 1);
 		} else {
+			diff++;
 			of_print_parents(a, &printed);
 			__of_print_nodes(ca, indent, "- ");
 		}
@@ -2122,12 +2126,15 @@ void of_diff(struct device_node *a, struct device_node *b, int indent)
 
 	for_each_child_of_node(b, cb) {
 		if (!of_get_child_by_name(a, cb->name)) {
+			diff++;
 			of_print_parents(a, &printed);
 			__of_print_nodes(cb, indent, "+ ");
 		}
 	}
 
 	of_print_close(a, &printed);
+
+	return diff;
 }
 
 struct device_node *of_new_node(struct device_node *parent, const char *name)
diff --git a/include/of.h b/include/of.h
index 216d0ee76368..b449d10ec774 100644
--- a/include/of.h
+++ b/include/of.h
@@ -107,7 +107,7 @@ void of_print_cmdline(struct device_node *root);
 
 void of_print_nodes(struct device_node *node, int indent);
 void of_print_properties(struct device_node *node);
-void of_diff(struct device_node *a, struct device_node *b, int indent);
+int of_diff(struct device_node *a, struct device_node *b, int indent);
 int of_probe(void);
 int of_parse_dtb(struct fdt_header *fdt);
 struct device_node *of_unflatten_dtb(const void *fdt, int size);
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/5] of: silence of_diff output for negative indents

[Ahmad Fatoum]

Negative indents just led to strange behavior so far. Repurpose them to
mean that the caller is not interested in console output. This makes
them useful for negative tests (Sanity check that two different nodes
are indeed different).

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/of/base.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 99be24254203..ffa79785f80e 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2087,6 +2087,8 @@ int of_diff(struct device_node *a, struct device_node *b, int indent)
 		bp = of_find_property(b, ap->name, NULL);
 		if (!bp) {
 			diff++;
+			if (indent < 0)
+				continue;
 			of_print_parents(a, &printed);
 			printf("- ");
 			__of_print_property(ap, indent);
@@ -2095,6 +2097,8 @@ int of_diff(struct device_node *a, struct device_node *b, int indent)
 
 		if (ap->length != bp->length || memcmp(of_property_get_value(ap), of_property_get_value(bp), bp->length)) {
 			diff++;
+			if (indent < 0)
+				continue;
 			of_print_parents(a, &printed);
 			printf("- ");
 			__of_print_property(ap, indent);
@@ -2107,6 +2111,8 @@ int of_diff(struct device_node *a, struct device_node *b, int indent)
 		ap = of_find_property(a, bp->name, NULL);
 		if (!ap) {
 			diff++;
+			if (indent < 0)
+				continue;
 			of_print_parents(a, &printed);
 			printf("+ ");
 			__of_print_property(bp, indent);
@@ -2116,9 +2122,11 @@ int of_diff(struct device_node *a, struct device_node *b, int indent)
 	for_each_child_of_node(a, ca) {
 		cb = of_get_child_by_name(b, ca->name);
 		if (cb) {
-			diff += of_diff(ca, cb, indent + 1);
+			diff += of_diff(ca, cb, indent >= 0 ? indent + 1 : indent);
 		} else {
 			diff++;
+			if (indent < 0)
+				continue;
 			of_print_parents(a, &printed);
 			__of_print_nodes(ca, indent, "- ");
 		}
@@ -2127,12 +2135,15 @@ int of_diff(struct device_node *a, struct device_node *b, int indent)
 	for_each_child_of_node(b, cb) {
 		if (!of_get_child_by_name(a, cb->name)) {
 			diff++;
+			if (indent < 0)
+				continue;
 			of_print_parents(a, &printed);
 			__of_print_nodes(cb, indent, "+ ");
 		}
 	}
 
-	of_print_close(a, &printed);
+	if (indent >= 0)
+		of_print_close(a, &printed);
 
 	return diff;
 }
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 4/5] of: silence of_diff output for negative indents

[Sascha Hauer]

On Mon, Feb 07, 2022 at 08:56:20AM +0100, Ahmad Fatoum wrote:
> Negative indents just led to strange behavior so far. Repurpose them to
> mean that the caller is not interested in console output. This makes
> them useful for negative tests (Sanity check that two different nodes
> are indeed different).
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/of/base.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 99be24254203..ffa79785f80e 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2087,6 +2087,8 @@ int of_diff(struct device_node *a, struct device_node *b, int indent)

Can we add a

	bool do_print = indent >= 0;

variable and use it for the tests below? I think this makes the code a
bit clearer.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 4/5] of: silence of_diff output for negative indents

[Sascha Hauer]

On Mon, Feb 07, 2022 at 09:17:30AM +0100, Sascha Hauer wrote:
> On Mon, Feb 07, 2022 at 08:56:20AM +0100, Ahmad Fatoum wrote:
> > Negative indents just led to strange behavior so far. Repurpose them to
> > mean that the caller is not interested in console output. This makes
> > them useful for negative tests (Sanity check that two different nodes
> > are indeed different).
> > 
> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > ---
> >  drivers/of/base.c | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > index 99be24254203..ffa79785f80e 100644
> > --- a/drivers/of/base.c
> > +++ b/drivers/of/base.c
> > @@ -2087,6 +2087,8 @@ int of_diff(struct device_node *a, struct device_node *b, int indent)
> 
> Can we add a
> 
> 	bool do_print = indent >= 0;
> 
> variable and use it for the tests below? I think this makes the code a
> bit clearer.

Changed while applying, so when you agree with this we are good to go.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 4/5] of: silence of_diff output for negative indents

[Ahmad Fatoum]

On 07.02.22 09:22, Sascha Hauer wrote:
> On Mon, Feb 07, 2022 at 09:17:30AM +0100, Sascha Hauer wrote:
>> On Mon, Feb 07, 2022 at 08:56:20AM +0100, Ahmad Fatoum wrote:
>>> Negative indents just led to strange behavior so far. Repurpose them to
>>> mean that the caller is not interested in console output. This makes
>>> them useful for negative tests (Sanity check that two different nodes
>>> are indeed different).
>>>
>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>> ---
>>>  drivers/of/base.c | 15 +++++++++++++--
>>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>>> index 99be24254203..ffa79785f80e 100644
>>> --- a/drivers/of/base.c
>>> +++ b/drivers/of/base.c
>>> @@ -2087,6 +2087,8 @@ int of_diff(struct device_node *a, struct device_node *b, int indent)
>>
>> Can we add a
>>
>> 	bool do_print = indent >= 0;
>>
>> variable and use it for the tests below? I think this makes the code a
>> bit clearer.
> 
> Changed while applying, so when you agree with this we are good to go.

I'd have called it silent to avoid having negative conditions. I did the
change locally as well as fixed two typos. I will send a v2.

> 
> Sascha
> 
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 5/5] test: self: add device tree manipulation test

[Ahmad Fatoum]

We had recently gained support for two extra helpers:

of_property_sprintf and of_property_write_string. Add tests for them as
well as for general OF creation and unflattening.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 test/self/Kconfig             |   6 ++
 test/self/Makefile            |   1 +
 test/self/of_manipulation.c   | 111 ++++++++++++++++++++++++++++++++++
 test/self/of_manipulation.dts |  30 +++++++++
 4 files changed, 148 insertions(+)
 create mode 100644 test/self/of_manipulation.c
 create mode 100644 test/self/of_manipulation.dts

diff --git a/test/self/Kconfig b/test/self/Kconfig
index dfaa32dda009..3340a9146ee2 100644
--- a/test/self/Kconfig
+++ b/test/self/Kconfig
@@ -37,6 +37,12 @@ config SELFTEST_PRINTF
 	help
 	  Tests barebox vsnprintf() functionality
 
+config SELFTEST_OF_MANIPULATION
+	bool "OF manipulation selftest"
+	select OFTREE
+	help
+	  Tests barebox device tree manipulation functionality
+
 config SELFTEST_PROGRESS_NOTIFIER
 	bool "progress notifier selftest"
 
diff --git a/test/self/Makefile b/test/self/Makefile
index e78ccc3cfb90..05a2a6a236ec 100644
--- a/test/self/Makefile
+++ b/test/self/Makefile
@@ -3,3 +3,4 @@
 obj-$(CONFIG_SELFTEST) += core.o
 obj-$(CONFIG_SELFTEST_PRINTF) += printf.o
 obj-$(CONFIG_SELFTEST_PROGRESS_NOTIFIER) += progress-notifier.o
+obj-$(CONFIG_SELFTEST_OF_MANIPULATION) += of_manipulation.o of_manipulation.dtb.o
diff --git a/test/self/of_manipulation.c b/test/self/of_manipulation.c
new file mode 100644
index 000000000000..1bcd593c8628
--- /dev/null
+++ b/test/self/of_manipulation.c
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <common.h>
+#include <bselftest.h>
+#include <linux/kernel.h>
+#include <module.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <linux/string.h>
+#include <errno.h>
+#include <of.h>
+
+BSELFTEST_GLOBALS();
+
+static void assert_different(struct device_node *a, struct device_node *b, int expect)
+{
+	int ret;
+
+	total_tests++;
+
+	ret = of_diff(a, b, -1);
+	if (ret == expect)
+		return;
+
+	pr_warn("comparison of %s and %s failed: %u differences expected, %u found.\n",
+		a->full_name, b->full_name, expect, ret);
+	of_diff(a, b, 1);
+	failed_tests++;
+}
+
+#define assert_equal(a, b) assert_different(a, b, 0)
+
+static void test_of_basics(struct device_node *root)
+{
+	struct device_node *node1, *node2, *node21;
+
+	node1 = of_new_node(root, "node1");
+	node2 = of_new_node(root, "node2");
+
+	assert_equal(node1, node2);
+
+	of_property_write_bool(node2, "property1", true);
+
+	assert_different(node1, node2, 1);
+
+	node21 = of_new_node(node2, "node21");
+
+	assert_different(node1, node2, 2);
+	assert_equal(node1, node21);
+
+	of_new_node(node1, "node21");
+
+	assert_different(node1, node2, 1);
+
+	of_property_write_bool(node1, "property1", true);
+
+	assert_equal(node1, node2);
+}
+
+static void test_of_property_strings(struct device_node *root)
+{
+	struct device_node *np1, *np2, *np3;
+	char properties[] = "ayy\0bee\0sea";
+
+	np1 = of_new_node(root, "np1");
+	np2 = of_new_node(root, "np2");
+	np3 = of_new_node(root, "np3");
+
+	of_property_sprintf(np1, "property-single", "%c%c%c", 'a', 'y', 'y');
+
+	of_property_write_string(np2, "property-single", "ayy");
+
+	assert_equal(np1, np2);
+
+	of_set_property(np2, "property-multi", properties, sizeof(properties), 1);
+
+	of_property_write_strings(np3, "property-single", "ayy", NULL);
+	of_property_write_strings(np3, "property-multi",
+				  "ayy", "bee", "sea", NULL);
+
+	assert_equal(np2, np3);
+
+	of_set_property(np1, "property-multi", properties, sizeof(properties), 1);
+
+	assert_equal(np1, np2);
+
+	of_set_property(np1, "property-multi", properties, sizeof(properties) - 1, 0);
+
+	assert_different(np1, np2, 1);
+}
+
+static void __init test_of_manipulation(void)
+{
+	extern char __dtb_of_manipulation_start[], __dtb_of_manipulation_end[];
+	struct device_node *root = of_new_node(NULL, NULL);
+	struct device_node *expected;
+
+	test_of_basics(root);
+	test_of_property_strings(root);
+
+	expected = of_unflatten_dtb(__dtb_of_manipulation_start,
+				    __dtb_of_manipulation_end - __dtb_of_manipulation_start);
+
+	assert_equal(root, expected);
+
+	of_delete_node(root);
+	of_delete_node(expected);
+}
+bselftest(core, test_of_manipulation);
diff --git a/test/self/of_manipulation.dts b/test/self/of_manipulation.dts
new file mode 100644
index 000000000000..3b690bb7f0fe
--- /dev/null
+++ b/test/self/of_manipulation.dts
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+/dts-v1/;
+
+/ {
+	node1 {
+		property1;
+		node21 { };
+	};
+
+	node2 {
+		property1;
+		node21 { };
+	};
+
+	np1 {
+		property-single = "ayy";
+		property-multi = [61 79 79 00 62 65 65 00 73 65 61];
+	};
+
+	np2 {
+		property-single = "ayy";
+		property-multi = "ayy", "bee", "sea";
+	};
+
+	np3 {
+		property-single = "ayy";
+		property-multi = "ayy", "bee", "sea";
+	};
+};
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox