[PATCH v2 00/13] of: overlay/unittest: Miscellaneous fixes and improvements

[PATCH v2 00/13] of: overlay/unittest: Miscellaneous fixes and improvements

[Geert Uytterhoeven]

	Hi all,

This patch series contains miscellaneous fixes and improvements for
dynamic DT overlays and the related unit tests.

The first two patches are fixes for a lock-up and a crash.
The remaining patches are smaller fixes, enhancements and cleanups for
the overlay tests, including one new test.

I ran into the crash when accidentally loading the wrong overlay (using
the out-of-tree DT overlay configfs[1]), and removing it afterwards.
As this case was not yet covered by the unittests, I added a test.
I enhanced the tests to clean up partial state after a failed
overlay apply attempt, which triggered the lock-up.

Changes compared to v1[2]:
  - Correct fixes tag and update description.
  - Merge differently, as requested by Rob.

Thanks for your comments!

[1] https://elinux.org/R-Car/DT-Overlays
[2] https://lore.kernel.org/r/cover.1689776064.git.geert+renesas@glider.be

Geert Uytterhoeven (13):
  of: dynamic: Do not use "%pOF" while holding devtree_lock
  of: overlay: Call of_changeset_init() early
  of: unittest: Fix overlay type in apply/revert check
  of: unittest: Restore indentation in overlay_bad_add_dup_prop test
  of: unittest: Improve messages and comments in apply/revert checks
  of: unittest: Merge of_unittest_apply{,_revert}_overlay_check()
  of: unittest: Cleanup partially-applied overlays
  of: unittest: Add separators to of_unittest_overlay_high_level()
  of: overlay: unittest: Add test for unresolved symbol
  of: unittest-data: Convert remaining overlay DTS files to sugar syntax
  of: unittest-data: Fix whitespace - blank lines
  of: unittest-data: Fix whitespace - indentation
  of: unittest-data: Fix whitespace - angular brackets

 drivers/of/dynamic.c                          |  12 +-
 drivers/of/overlay.c                          |   3 +-
 drivers/of/unittest-data/Makefile             |   3 +-
 drivers/of/unittest-data/overlay.dtso         |  32 ++-
 drivers/of/unittest-data/overlay_0.dtso       |  11 +-
 drivers/of/unittest-data/overlay_1.dtso       |  11 +-
 drivers/of/unittest-data/overlay_11.dtso      |   1 -
 drivers/of/unittest-data/overlay_12.dtso      |  11 +-
 drivers/of/unittest-data/overlay_13.dtso      |  11 +-
 drivers/of/unittest-data/overlay_15.dtso      |   1 +
 drivers/of/unittest-data/overlay_4.dtso       |   1 -
 .../overlay_bad_add_dup_node.dtso             |   9 +-
 .../overlay_bad_add_dup_prop.dtso             |   9 +-
 .../of/unittest-data/overlay_bad_phandle.dtso |   5 +-
 .../of/unittest-data/overlay_bad_symbol.dtso  |   5 +-
 .../unittest-data/overlay_bad_unresolved.dtso |   7 +
 drivers/of/unittest-data/overlay_common.dtsi  |  36 ++-
 drivers/of/unittest-data/overlay_gpio_01.dtso |   1 +
 .../of/unittest-data/overlay_gpio_02a.dtso    |   1 +
 .../of/unittest-data/overlay_gpio_02b.dtso    |   1 +
 drivers/of/unittest-data/overlay_gpio_03.dtso |   1 +
 .../of/unittest-data/overlay_gpio_04a.dtso    |   1 +
 .../of/unittest-data/overlay_gpio_04b.dtso    |   1 +
 .../of/unittest-data/testcases_common.dtsi    |   1 +
 .../of/unittest-data/tests-interrupts.dtsi    |   1 +
 drivers/of/unittest-data/tests-overlay.dtsi   |   1 -
 drivers/of/unittest-data/tests-phandle.dtsi   |   2 +
 drivers/of/unittest.c                         | 218 +++++++++++-------
 28 files changed, 224 insertions(+), 173 deletions(-)
 create mode 100644 drivers/of/unittest-data/overlay_bad_unresolved.dtso

-- 
2.34.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH v2 01/13] of: dynamic: Do not use "%pOF" while holding devtree_lock

[Geert Uytterhoeven]

While originally it was fine to format strings using "%pOF" while
holding devtree_lock, this now causes a deadlock.  Lockdep reports:

    of_get_parent from of_fwnode_get_parent+0x18/0x24
    ^^^^^^^^^^^^^
    of_fwnode_get_parent from fwnode_count_parents+0xc/0x28
    fwnode_count_parents from fwnode_full_name_string+0x18/0xac
    fwnode_full_name_string from device_node_string+0x1a0/0x404
    device_node_string from pointer+0x3c0/0x534
    pointer from vsnprintf+0x248/0x36c
    vsnprintf from vprintk_store+0x130/0x3b4

Fix this by making the locking cover only the parts that really need it.

Fixes: a92eb7621b9fb2c2 ("lib/vsprintf: Make use of fwnode API to obtain node names and separators")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Correct fixes tag and update description.
---
 drivers/of/dynamic.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index e311d406b1705306..eae45a1c673ee05f 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -601,13 +601,16 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce)
 
 	__of_changeset_entry_dump(ce);
 
-	raw_spin_lock_irqsave(&devtree_lock, flags);
 	switch (ce->action) {
 	case OF_RECONFIG_ATTACH_NODE:
+		raw_spin_lock_irqsave(&devtree_lock, flags);
 		__of_attach_node(ce->np);
+		raw_spin_unlock_irqrestore(&devtree_lock, flags);
 		break;
 	case OF_RECONFIG_DETACH_NODE:
+		raw_spin_lock_irqsave(&devtree_lock, flags);
 		__of_detach_node(ce->np);
+		raw_spin_unlock_irqrestore(&devtree_lock, flags);
 		break;
 	case OF_RECONFIG_ADD_PROPERTY:
 		/* If the property is in deadprops then it must be removed */
@@ -619,7 +622,9 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce)
 			}
 		}
 
+		raw_spin_lock_irqsave(&devtree_lock, flags);
 		ret = __of_add_property(ce->np, ce->prop);
+		raw_spin_unlock_irqrestore(&devtree_lock, flags);
 		if (ret) {
 			pr_err("changeset: add_property failed @%pOF/%s\n",
 				ce->np,
@@ -628,7 +633,9 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce)
 		}
 		break;
 	case OF_RECONFIG_REMOVE_PROPERTY:
+		raw_spin_lock_irqsave(&devtree_lock, flags);
 		ret = __of_remove_property(ce->np, ce->prop);
+		raw_spin_unlock_irqrestore(&devtree_lock, flags);
 		if (ret) {
 			pr_err("changeset: remove_property failed @%pOF/%s\n",
 				ce->np,
@@ -647,7 +654,9 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce)
 			}
 		}
 
+		raw_spin_lock_irqsave(&devtree_lock, flags);
 		ret = __of_update_property(ce->np, ce->prop, &old_prop);
+		raw_spin_unlock_irqrestore(&devtree_lock, flags);
 		if (ret) {
 			pr_err("changeset: update_property failed @%pOF/%s\n",
 				ce->np,
@@ -658,7 +667,6 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce)
 	default:
 		ret = -EINVAL;
 	}
-	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 
 	if (ret)
 		return ret;
-- 
2.34.1

[PATCH v2 02/13] of: overlay: Call of_changeset_init() early

[Geert Uytterhoeven]

When of_overlay_fdt_apply() fails, the changeset may be partially
applied, and the caller is still expected to call of_overlay_remove() to
clean up this partial state.

However, of_overlay_apply() calls of_resolve_phandles() before
init_overlay_changeset().  Hence if the overlay fails to apply due to an
unresolved symbol, the overlay_changeset.cset.entries list is still
uninitialized, and cleanup will crash with a NULL-pointer dereference in
overlay_removal_is_ok().

Fix this by moving the call to of_changeset_init() from
init_overlay_changeset() to of_overlay_fdt_apply(), where all other
early initialization is done.

Fixes: f948d6d8b792bb90 ("of: overlay: avoid race condition between applying multiple overlays")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - No changes.
---
 drivers/of/overlay.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 2eb831bf906d70f3..abfbe3f5f1256957 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -797,8 +797,6 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs)
 	if (!of_node_is_root(ovcs->overlay_root))
 		pr_debug("%s() ovcs->overlay_root is not root\n", __func__);
 
-	of_changeset_init(&ovcs->cset);
-
 	cnt = 0;
 
 	/* fragment nodes */
@@ -1160,6 +1158,7 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
 
 	INIT_LIST_HEAD(&ovcs->ovcs_list);
 	list_add_tail(&ovcs->ovcs_list, &ovcs_list);
+	of_changeset_init(&ovcs->cset);
 
 	/*
 	 * Must create permanent copy of FDT because of_fdt_unflatten_tree()
-- 
2.34.1

[PATCH v2 03/13] of: unittest: Fix overlay type in apply/revert check

[Geert Uytterhoeven]

The removal check in of_unittest_apply_revert_overlay_check()
always uses the platform device overlay type, while it should use the
actual overlay type, as passed as a parameter to the function.

This has no impact on any current test, as all tests calling
of_unittest_apply_revert_overlay_check() use the platform device overlay
type.

Fixes: d5e75500ca401d31 ("of: unitest: Add I2C overlay unit tests.")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - No changes.
---
 drivers/of/unittest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index a406a12eb20804d6..840f26477f77f622 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -2180,7 +2180,7 @@ static int __init of_unittest_apply_revert_overlay_check(int overlay_nr,
 	of_unittest_untrack_overlay(save_ovcs_id);
 
 	/* unittest device must be again in before state */
-	if (of_unittest_device_exists(unittest_nr, PDEV_OVERLAY) != before) {
+	if (of_unittest_device_exists(unittest_nr, ovtype) != before) {
 		unittest(0, "%s with device @\"%s\" %s\n",
 				overlay_name_from_nr(overlay_nr),
 				unittest_path(unittest_nr, ovtype),
-- 
2.34.1

[PATCH v2 04/13] of: unittest: Restore indentation in overlay_bad_add_dup_prop test

[Geert Uytterhoeven]

When updating the expected error messages for the
overlay_bad_add_dup_prop test, indentation of the EXPECT_END()
statements was accidentally changed.

Fixes: 29acfb65598f9167 ("of: unittest: kmemleak in duplicate property update")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - No changes.
---
 drivers/of/unittest.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 840f26477f77f622..bb7e2ec05da59070 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -3687,11 +3687,11 @@ static __init void of_unittest_overlay_high_level(void)
 		 "Adding overlay 'overlay_bad_add_dup_prop' failed\n");
 
 	EXPECT_END(KERN_ERR,
-		     "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/name");
+		   "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/name");
 	EXPECT_END(KERN_ERR,
-		     "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/rpm_avail");
+		   "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/rpm_avail");
 	EXPECT_END(KERN_ERR,
-		     "OF: overlay: ERROR: multiple fragments add and/or delete node /testcase-data-2/substation@100/motor-1/electric");
+		   "OF: overlay: ERROR: multiple fragments add and/or delete node /testcase-data-2/substation@100/motor-1/electric");
 
 	unittest(overlay_data_apply("overlay_bad_phandle", NULL),
 		 "Adding overlay 'overlay_bad_phandle' failed\n");
-- 
2.34.1

[PATCH v2 05/13] of: unittest: Improve messages and comments in apply/revert checks

[Geert Uytterhoeven]

Miscellaneous improvements for the apply and apply/revert checks,
making them more similar:
  - Fix inverted comment for before state check,
  - Add more comments to improve symmetry,
  - Fix grammar s/must be to set to/must be in/,
  - Avoid saying "create" in messages, as the actual operation depends
    on the value of the before/after parameters.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - No changes.
---
 drivers/of/unittest.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index bb7e2ec05da59070..42abfcd0cdffa4af 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -2108,7 +2108,7 @@ static int __init of_unittest_apply_overlay_check(int overlay_nr,
 {
 	int ret, ovcs_id;
 
-	/* unittest device must not be in before state */
+	/* unittest device must be in before state */
 	if (of_unittest_device_exists(unittest_nr, ovtype) != before) {
 		unittest(0, "%s with device @\"%s\" %s\n",
 				overlay_name_from_nr(overlay_nr),
@@ -2117,6 +2117,7 @@ static int __init of_unittest_apply_overlay_check(int overlay_nr,
 		return -EINVAL;
 	}
 
+	/* apply the overlay */
 	ovcs_id = 0;
 	ret = of_unittest_apply_overlay(overlay_nr, &ovcs_id);
 	if (ret != 0) {
@@ -2124,9 +2125,9 @@ static int __init of_unittest_apply_overlay_check(int overlay_nr,
 		return ret;
 	}
 
-	/* unittest device must be to set to after state */
+	/* unittest device must be in after state */
 	if (of_unittest_device_exists(unittest_nr, ovtype) != after) {
-		unittest(0, "%s failed to create @\"%s\" %s\n",
+		unittest(0, "%s with device @\"%s\" %s\n",
 				overlay_name_from_nr(overlay_nr),
 				unittest_path(unittest_nr, ovtype),
 				!after ? "enabled" : "disabled");
@@ -2162,13 +2163,14 @@ static int __init of_unittest_apply_revert_overlay_check(int overlay_nr,
 
 	/* unittest device must be in after state */
 	if (of_unittest_device_exists(unittest_nr, ovtype) != after) {
-		unittest(0, "%s failed to create @\"%s\" %s\n",
+		unittest(0, "%s with device @\"%s\" %s\n",
 				overlay_name_from_nr(overlay_nr),
 				unittest_path(unittest_nr, ovtype),
 				!after ? "enabled" : "disabled");
 		return -EINVAL;
 	}
 
+	/* remove the overlay */
 	save_ovcs_id = ovcs_id;
 	ret = of_overlay_remove(&ovcs_id);
 	if (ret != 0) {
-- 
2.34.1

[PATCH v2 06/13] of: unittest: Merge of_unittest_apply{,_revert}_overlay_check()

[Geert Uytterhoeven]

of_unittest_apply_overlay_check() and the first part of
of_unittest_apply_revert_overlay_check() are identical.
Reduce code duplication by replacing them by two wrappers around a
common helper.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Merge differently, as requested by Rob.
---
 drivers/of/unittest.c | 45 +++++++++++++++++--------------------------
 1 file changed, 18 insertions(+), 27 deletions(-)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 42abfcd0cdffa4af..b23a44de091bd044 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -2101,8 +2101,7 @@ static int __init of_unittest_apply_overlay(int overlay_nr, int *ovcs_id)
 	return 0;
 }
 
-/* apply an overlay while checking before and after states */
-static int __init of_unittest_apply_overlay_check(int overlay_nr,
+static int __init __of_unittest_apply_overlay_check(int overlay_nr,
 		int unittest_nr, int before, int after,
 		enum overlay_type ovtype)
 {
@@ -2134,6 +2133,19 @@ static int __init of_unittest_apply_overlay_check(int overlay_nr,
 		return -EINVAL;
 	}
 
+	return ovcs_id;
+}
+
+/* apply an overlay while checking before and after states */
+static int __init of_unittest_apply_overlay_check(int overlay_nr,
+		int unittest_nr, int before, int after,
+		enum overlay_type ovtype)
+{
+	int ovcs_id = __of_unittest_apply_overlay_check(overlay_nr,
+				unittest_nr, before, after, ovtype);
+	if (ovcs_id < 0)
+		return ovcs_id;
+
 	return 0;
 }
 
@@ -2144,31 +2156,10 @@ static int __init of_unittest_apply_revert_overlay_check(int overlay_nr,
 {
 	int ret, ovcs_id, save_ovcs_id;
 
-	/* unittest device must be in before state */
-	if (of_unittest_device_exists(unittest_nr, ovtype) != before) {
-		unittest(0, "%s with device @\"%s\" %s\n",
-				overlay_name_from_nr(overlay_nr),
-				unittest_path(unittest_nr, ovtype),
-				!before ? "enabled" : "disabled");
-		return -EINVAL;
-	}
-
-	/* apply the overlay */
-	ovcs_id = 0;
-	ret = of_unittest_apply_overlay(overlay_nr, &ovcs_id);
-	if (ret != 0) {
-		/* of_unittest_apply_overlay already called unittest() */
-		return ret;
-	}
-
-	/* unittest device must be in after state */
-	if (of_unittest_device_exists(unittest_nr, ovtype) != after) {
-		unittest(0, "%s with device @\"%s\" %s\n",
-				overlay_name_from_nr(overlay_nr),
-				unittest_path(unittest_nr, ovtype),
-				!after ? "enabled" : "disabled");
-		return -EINVAL;
-	}
+	ovcs_id = __of_unittest_apply_overlay_check(overlay_nr, unittest_nr,
+						    before, after, ovtype);
+	if (ovcs_id < 0)
+		return ovcs_id;
 
 	/* remove the overlay */
 	save_ovcs_id = ovcs_id;
-- 
2.34.1

[PATCH v2 07/13] of: unittest: Cleanup partially-applied overlays

[Geert Uytterhoeven]

When of_overlay_fdt_apply() fails, the changeset may be partially
applied, and the caller is still expected to call of_overlay_remove() to
clean up this partial state.  However, overlay_17 is the only test that
takes care of cleaning up after an (expected) failure.

Instead of adding cleanup code to each individual test, extend
overlay_info with the optional expected return value of
of_overlay_remove(), and handle cleanup in the overlay_data_apply()
helper.  While at it, simplify the end marker in the overlay_info table.

Update the expected error output for errors during the newly cleanup.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - No changes.
---
 drivers/of/unittest.c | 130 +++++++++++++++++++++++++++---------------
 1 file changed, 83 insertions(+), 47 deletions(-)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index b23a44de091bd044..f9c09d5787362601 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -2964,12 +2964,6 @@ static void __init of_unittest_overlay_notify(void)
 
 	unittest(ovcs_id, "ovcs_id not created for overlay_17\n");
 
-	if (ovcs_id) {
-		ret = of_overlay_remove(&ovcs_id);
-		unittest(!ret,
-			"overlay_17 of_overlay_remove(), ret = %d\n", ret);
-	}
-
 	/* ---  overlay 18  --- */
 
 	unittest(overlay_data_apply("overlay_18", &ovcs_id),
@@ -3257,17 +3251,19 @@ static void __init of_unittest_lifecycle(void)
 	extern uint8_t __dtbo_##overlay_name##_begin[]; \
 	extern uint8_t __dtbo_##overlay_name##_end[]
 
-#define OVERLAY_INFO(overlay_name, expected) \
-{	.dtbo_begin       = __dtbo_##overlay_name##_begin, \
-	.dtbo_end         = __dtbo_##overlay_name##_end, \
-	.expected_result = expected, \
-	.name            = #overlay_name, \
+#define OVERLAY_INFO(overlay_name, expected, expected_remove) \
+{	.dtbo_begin		= __dtbo_##overlay_name##_begin, \
+	.dtbo_end		= __dtbo_##overlay_name##_end, \
+	.expected_result	= expected, \
+	.expected_result_remove	= expected_remove, \
+	.name			= #overlay_name, \
 }
 
 struct overlay_info {
 	uint8_t		*dtbo_begin;
 	uint8_t		*dtbo_end;
 	int		expected_result;
+	int		expected_result_remove;	/* if apply failed */
 	int		ovcs_id;
 	char		*name;
 };
@@ -3307,40 +3303,40 @@ OVERLAY_INFO_EXTERN(overlay_bad_symbol);
 
 /* entries found by name */
 static struct overlay_info overlays[] = {
-	OVERLAY_INFO(overlay_base, -9999),
-	OVERLAY_INFO(overlay, 0),
-	OVERLAY_INFO(overlay_0, 0),
-	OVERLAY_INFO(overlay_1, 0),
-	OVERLAY_INFO(overlay_2, 0),
-	OVERLAY_INFO(overlay_3, 0),
-	OVERLAY_INFO(overlay_4, 0),
-	OVERLAY_INFO(overlay_5, 0),
-	OVERLAY_INFO(overlay_6, 0),
-	OVERLAY_INFO(overlay_7, 0),
-	OVERLAY_INFO(overlay_8, 0),
-	OVERLAY_INFO(overlay_9, 0),
-	OVERLAY_INFO(overlay_10, 0),
-	OVERLAY_INFO(overlay_11, 0),
-	OVERLAY_INFO(overlay_12, 0),
-	OVERLAY_INFO(overlay_13, 0),
-	OVERLAY_INFO(overlay_15, 0),
-	OVERLAY_INFO(overlay_16, -EBUSY),
-	OVERLAY_INFO(overlay_17, -EEXIST),
-	OVERLAY_INFO(overlay_18, 0),
-	OVERLAY_INFO(overlay_19, 0),
-	OVERLAY_INFO(overlay_20, 0),
-	OVERLAY_INFO(overlay_gpio_01, 0),
-	OVERLAY_INFO(overlay_gpio_02a, 0),
-	OVERLAY_INFO(overlay_gpio_02b, 0),
-	OVERLAY_INFO(overlay_gpio_03, 0),
-	OVERLAY_INFO(overlay_gpio_04a, 0),
-	OVERLAY_INFO(overlay_gpio_04b, 0),
-	OVERLAY_INFO(overlay_bad_add_dup_node, -EINVAL),
-	OVERLAY_INFO(overlay_bad_add_dup_prop, -EINVAL),
-	OVERLAY_INFO(overlay_bad_phandle, -EINVAL),
-	OVERLAY_INFO(overlay_bad_symbol, -EINVAL),
+	OVERLAY_INFO(overlay_base, -9999, 0),
+	OVERLAY_INFO(overlay, 0, 0),
+	OVERLAY_INFO(overlay_0, 0, 0),
+	OVERLAY_INFO(overlay_1, 0, 0),
+	OVERLAY_INFO(overlay_2, 0, 0),
+	OVERLAY_INFO(overlay_3, 0, 0),
+	OVERLAY_INFO(overlay_4, 0, 0),
+	OVERLAY_INFO(overlay_5, 0, 0),
+	OVERLAY_INFO(overlay_6, 0, 0),
+	OVERLAY_INFO(overlay_7, 0, 0),
+	OVERLAY_INFO(overlay_8, 0, 0),
+	OVERLAY_INFO(overlay_9, 0, 0),
+	OVERLAY_INFO(overlay_10, 0, 0),
+	OVERLAY_INFO(overlay_11, 0, 0),
+	OVERLAY_INFO(overlay_12, 0, 0),
+	OVERLAY_INFO(overlay_13, 0, 0),
+	OVERLAY_INFO(overlay_15, 0, 0),
+	OVERLAY_INFO(overlay_16, -EBUSY, 0),
+	OVERLAY_INFO(overlay_17, -EEXIST, 0),
+	OVERLAY_INFO(overlay_18, 0, 0),
+	OVERLAY_INFO(overlay_19, 0, 0),
+	OVERLAY_INFO(overlay_20, 0, 0),
+	OVERLAY_INFO(overlay_gpio_01, 0, 0),
+	OVERLAY_INFO(overlay_gpio_02a, 0, 0),
+	OVERLAY_INFO(overlay_gpio_02b, 0, 0),
+	OVERLAY_INFO(overlay_gpio_03, 0, 0),
+	OVERLAY_INFO(overlay_gpio_04a, 0, 0),
+	OVERLAY_INFO(overlay_gpio_04b, 0, 0),
+	OVERLAY_INFO(overlay_bad_add_dup_node, -EINVAL, -ENODEV),
+	OVERLAY_INFO(overlay_bad_add_dup_prop, -EINVAL, -ENODEV),
+	OVERLAY_INFO(overlay_bad_phandle, -EINVAL, 0),
+	OVERLAY_INFO(overlay_bad_symbol, -EINVAL, -ENODEV),
 	/* end marker */
-	{.dtbo_begin = NULL, .dtbo_end = NULL, .expected_result = 0, .name = NULL}
+	{ }
 };
 
 static struct device_node *overlay_base_root;
@@ -3435,8 +3431,9 @@ void __init unittest_unflatten_overlay_base(void)
 static int __init overlay_data_apply(const char *overlay_name, int *ovcs_id)
 {
 	struct overlay_info *info;
+	int passed = 1;
 	int found = 0;
-	int ret;
+	int ret, ret2;
 	u32 size;
 
 	for (info = overlays; info && info->name; info++) {
@@ -3463,11 +3460,24 @@ static int __init overlay_data_apply(const char *overlay_name, int *ovcs_id)
 	pr_debug("%s applied\n", overlay_name);
 
 out:
-	if (ret != info->expected_result)
+	if (ret != info->expected_result) {
 		pr_err("of_overlay_fdt_apply() expected %d, ret=%d, %s\n",
 		       info->expected_result, ret, overlay_name);
+		passed = 0;
+	}
+
+	if (ret < 0) {
+		/* changeset may be partially applied */
+		ret2 = of_overlay_remove(&info->ovcs_id);
+		if (ret2 != info->expected_result_remove) {
+			pr_err("of_overlay_remove() expected %d, ret=%d, %s\n",
+			       info->expected_result_remove, ret2,
+			       overlay_name);
+			passed = 0;
+		}
+	}
 
-	return (ret == info->expected_result);
+	return passed;
 }
 
 /*
@@ -3660,10 +3670,18 @@ static __init void of_unittest_overlay_high_level(void)
 		     "OF: overlay: ERROR: multiple fragments add and/or delete node /testcase-data-2/substation@100/motor-1/controller");
 	EXPECT_BEGIN(KERN_ERR,
 		     "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/controller/name");
+	EXPECT_BEGIN(KERN_ERR,
+		     "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/controller/name");
+	EXPECT_BEGIN(KERN_ERR,
+		     "OF: Error reverting changeset (-19)");
 
 	unittest(overlay_data_apply("overlay_bad_add_dup_node", NULL),
 		 "Adding overlay 'overlay_bad_add_dup_node' failed\n");
 
+	EXPECT_END(KERN_ERR,
+		   "OF: Error reverting changeset (-19)");
+	EXPECT_END(KERN_ERR,
+		   "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/controller/name");
 	EXPECT_END(KERN_ERR,
 		   "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/controller/name");
 	EXPECT_END(KERN_ERR,
@@ -3675,10 +3693,18 @@ static __init void of_unittest_overlay_high_level(void)
 		     "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/rpm_avail");
 	EXPECT_BEGIN(KERN_ERR,
 		     "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/name");
+	EXPECT_BEGIN(KERN_ERR,
+		     "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/electric/name");
+	EXPECT_BEGIN(KERN_ERR,
+		     "OF: Error reverting changeset (-19)");
 
 	unittest(overlay_data_apply("overlay_bad_add_dup_prop", NULL),
 		 "Adding overlay 'overlay_bad_add_dup_prop' failed\n");
 
+	EXPECT_END(KERN_ERR,
+		   "OF: Error reverting changeset (-19)");
+	EXPECT_END(KERN_ERR,
+		   "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/electric/name");
 	EXPECT_END(KERN_ERR,
 		   "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/name");
 	EXPECT_END(KERN_ERR,
@@ -3689,9 +3715,19 @@ static __init void of_unittest_overlay_high_level(void)
 	unittest(overlay_data_apply("overlay_bad_phandle", NULL),
 		 "Adding overlay 'overlay_bad_phandle' failed\n");
 
+	EXPECT_BEGIN(KERN_ERR,
+		     "OF: changeset: remove_property failed @/testcase-data-2/substation@100/hvac-medium-2/name");
+	EXPECT_BEGIN(KERN_ERR,
+		     "OF: resolver: overlay phandle fixup failed: -22");
+
 	unittest(overlay_data_apply("overlay_bad_symbol", NULL),
 		 "Adding overlay 'overlay_bad_symbol' failed\n");
 
+	EXPECT_END(KERN_ERR,
+		   "OF: resolver: overlay phandle fixup failed: -22");
+	EXPECT_END(KERN_ERR,
+		   "OF: changeset: remove_property failed @/testcase-data-2/substation@100/hvac-medium-2/name");
+
 	return;
 
 err_unlock:
-- 
2.34.1

[PATCH v2 08/13] of: unittest: Add separators to of_unittest_overlay_high_level()

[Geert Uytterhoeven]

It is hard to see the start and end of each individual test in
of_unittest_overlay_high_level().  Add visual cues in the form of
separator comments, like was done in of_unittest_overlay_notify().

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - No changes.
---
 drivers/of/unittest.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index f9c09d5787362601..f0e97cfb31573696 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -3616,6 +3616,8 @@ static __init void of_unittest_overlay_high_level(void)
 
 	/* now do the normal overlay usage test */
 
+	/* ---  overlay  --- */
+
 	EXPECT_BEGIN(KERN_ERR,
 		     "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/substation@100/status");
 	EXPECT_BEGIN(KERN_ERR,
@@ -3666,6 +3668,8 @@ static __init void of_unittest_overlay_high_level(void)
 
 	unittest(ret, "Adding overlay 'overlay' failed\n");
 
+	/* ---  overlay_bad_add_dup_node  --- */
+
 	EXPECT_BEGIN(KERN_ERR,
 		     "OF: overlay: ERROR: multiple fragments add and/or delete node /testcase-data-2/substation@100/motor-1/controller");
 	EXPECT_BEGIN(KERN_ERR,
@@ -3687,6 +3691,8 @@ static __init void of_unittest_overlay_high_level(void)
 	EXPECT_END(KERN_ERR,
 		   "OF: overlay: ERROR: multiple fragments add and/or delete node /testcase-data-2/substation@100/motor-1/controller");
 
+	/* ---  overlay_bad_add_dup_prop  --- */
+
 	EXPECT_BEGIN(KERN_ERR,
 		     "OF: overlay: ERROR: multiple fragments add and/or delete node /testcase-data-2/substation@100/motor-1/electric");
 	EXPECT_BEGIN(KERN_ERR,
@@ -3712,9 +3718,13 @@ static __init void of_unittest_overlay_high_level(void)
 	EXPECT_END(KERN_ERR,
 		   "OF: overlay: ERROR: multiple fragments add and/or delete node /testcase-data-2/substation@100/motor-1/electric");
 
+	/* ---  overlay_bad_phandle  --- */
+
 	unittest(overlay_data_apply("overlay_bad_phandle", NULL),
 		 "Adding overlay 'overlay_bad_phandle' failed\n");
 
+	/* ---  overlay_bad_symbol  --- */
+
 	EXPECT_BEGIN(KERN_ERR,
 		     "OF: changeset: remove_property failed @/testcase-data-2/substation@100/hvac-medium-2/name");
 	EXPECT_BEGIN(KERN_ERR,
-- 
2.34.1

[PATCH v2 09/13] of: overlay: unittest: Add test for unresolved symbol

[Geert Uytterhoeven]

Add a test to exercise the error paths when trying to apply an overlay
with an unresolved symbol and cleaning up the resulting partial state.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - No changes.
---
 drivers/of/unittest-data/Makefile               |  3 ++-
 .../unittest-data/overlay_bad_unresolved.dtso   |  7 +++++++
 drivers/of/unittest.c                           | 17 +++++++++++++++++
 3 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 drivers/of/unittest-data/overlay_bad_unresolved.dtso

diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
index ea5f4da68e23acd0..f79daa1d45713958 100644
--- a/drivers/of/unittest-data/Makefile
+++ b/drivers/of/unittest-data/Makefile
@@ -32,7 +32,8 @@ obj-$(CONFIG_OF_OVERLAY) += overlay.dtbo.o \
 			    overlay_gpio_02b.dtbo.o \
 			    overlay_gpio_03.dtbo.o \
 			    overlay_gpio_04a.dtbo.o \
-			    overlay_gpio_04b.dtbo.o
+			    overlay_gpio_04b.dtbo.o \
+			    overlay_bad_unresolved.dtbo.o
 
 # enable creation of __symbols__ node
 DTC_FLAGS_overlay += -@
diff --git a/drivers/of/unittest-data/overlay_bad_unresolved.dtso b/drivers/of/unittest-data/overlay_bad_unresolved.dtso
new file mode 100644
index 0000000000000000..3b75a53ae8a492bd
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_bad_unresolved.dtso
@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+&this_label_does_not_exist {
+	status = "ok";
+};
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index f0e97cfb31573696..fb668d55308ca48e 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -3300,6 +3300,7 @@ OVERLAY_INFO_EXTERN(overlay_bad_add_dup_node);
 OVERLAY_INFO_EXTERN(overlay_bad_add_dup_prop);
 OVERLAY_INFO_EXTERN(overlay_bad_phandle);
 OVERLAY_INFO_EXTERN(overlay_bad_symbol);
+OVERLAY_INFO_EXTERN(overlay_bad_unresolved);
 
 /* entries found by name */
 static struct overlay_info overlays[] = {
@@ -3335,6 +3336,7 @@ static struct overlay_info overlays[] = {
 	OVERLAY_INFO(overlay_bad_add_dup_prop, -EINVAL, -ENODEV),
 	OVERLAY_INFO(overlay_bad_phandle, -EINVAL, 0),
 	OVERLAY_INFO(overlay_bad_symbol, -EINVAL, -ENODEV),
+	OVERLAY_INFO(overlay_bad_unresolved, -EINVAL, 0),
 	/* end marker */
 	{ }
 };
@@ -3738,6 +3740,21 @@ static __init void of_unittest_overlay_high_level(void)
 	EXPECT_END(KERN_ERR,
 		   "OF: changeset: remove_property failed @/testcase-data-2/substation@100/hvac-medium-2/name");
 
+	/* ---  overlay_bad_unresolved  --- */
+
+	EXPECT_BEGIN(KERN_ERR,
+		     "OF: resolver: node label 'this_label_does_not_exist' not found in live devicetree symbols table");
+	EXPECT_BEGIN(KERN_ERR,
+		     "OF: resolver: overlay phandle fixup failed: -22");
+
+	unittest(overlay_data_apply("overlay_bad_unresolved", NULL),
+		 "Adding overlay 'overlay_bad_unresolved' failed\n");
+
+	EXPECT_END(KERN_ERR,
+		   "OF: resolver: overlay phandle fixup failed: -22");
+	EXPECT_END(KERN_ERR,
+		   "OF: resolver: node label 'this_label_does_not_exist' not found in live devicetree symbols table");
+
 	return;
 
 err_unlock:
-- 
2.34.1

[PATCH v2 10/13] of: unittest-data: Convert remaining overlay DTS files to sugar syntax

[Geert Uytterhoeven]

Overlay syntactic sugar for generating target-path fragments is
supported by the version of dtc supplied with the kernel since commit
50aafd60898a8b3e ("scripts/dtc: Update to upstream version
v1.4.6-21-g84e414b0b5bc").  Hence convert the remaining unittest overlay
devicetree source files to sugar syntax, improving readability.

This completes the work started in commit db2f3762d609318e ("of: convert
unittest overlay devicetree source to sugar syntax").

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - No changes.
---
 drivers/of/unittest-data/overlay_0.dtso  | 11 +++--------
 drivers/of/unittest-data/overlay_1.dtso  | 11 +++--------
 drivers/of/unittest-data/overlay_12.dtso | 11 +++--------
 drivers/of/unittest-data/overlay_13.dtso | 11 +++--------
 4 files changed, 12 insertions(+), 32 deletions(-)

diff --git a/drivers/of/unittest-data/overlay_0.dtso b/drivers/of/unittest-data/overlay_0.dtso
index ac0f9e0fe65f80f3..bb46582e0485318f 100644
--- a/drivers/of/unittest-data/overlay_0.dtso
+++ b/drivers/of/unittest-data/overlay_0.dtso
@@ -2,13 +2,8 @@
 /dts-v1/;
 /plugin/;
 
-/ {
-	/* overlay_0 - enable using absolute target path */
+/* overlay_0 - enable using absolute target path */
 
-	fragment@0 {
-		target-path = "/testcase-data/overlay-node/test-bus/test-unittest0";
-		__overlay__ {
-			status = "okay";
-		};
-	};
+&{/testcase-data/overlay-node/test-bus/test-unittest0} {
+	status = "okay";
 };
diff --git a/drivers/of/unittest-data/overlay_1.dtso b/drivers/of/unittest-data/overlay_1.dtso
index e92a626e29483a32..9c0fc8ffa4a1d3d8 100644
--- a/drivers/of/unittest-data/overlay_1.dtso
+++ b/drivers/of/unittest-data/overlay_1.dtso
@@ -2,13 +2,8 @@
 /dts-v1/;
 /plugin/;
 
-/ {
-	/* overlay_1 - disable using absolute target path */
+/* overlay_1 - disable using absolute target path */
 
-	fragment@0 {
-		target-path = "/testcase-data/overlay-node/test-bus/test-unittest1";
-		__overlay__ {
-			status = "disabled";
-		};
-	};
+&{/testcase-data/overlay-node/test-bus/test-unittest1} {
+	status = "disabled";
 };
diff --git a/drivers/of/unittest-data/overlay_12.dtso b/drivers/of/unittest-data/overlay_12.dtso
index ca3441e2cbec94ce..8d5087793eb42688 100644
--- a/drivers/of/unittest-data/overlay_12.dtso
+++ b/drivers/of/unittest-data/overlay_12.dtso
@@ -2,13 +2,8 @@
 /dts-v1/;
 /plugin/;
 
-/ {
-	/* overlay_12 - enable using absolute target path (i2c) */
+/* overlay_12 - enable using absolute target path (i2c) */
 
-	fragment@0 {
-		target-path = "/testcase-data/overlay-node/test-bus/i2c-test-bus/test-unittest12";
-		__overlay__ {
-			status = "okay";
-		};
-	};
+&{/testcase-data/overlay-node/test-bus/i2c-test-bus/test-unittest12} {
+	status = "okay";
 };
diff --git a/drivers/of/unittest-data/overlay_13.dtso b/drivers/of/unittest-data/overlay_13.dtso
index 3c30dec6389436df..da200ae94f459ade 100644
--- a/drivers/of/unittest-data/overlay_13.dtso
+++ b/drivers/of/unittest-data/overlay_13.dtso
@@ -2,13 +2,8 @@
 /dts-v1/;
 /plugin/;
 
-/ {
-	/* overlay_13 - disable using absolute target path (i2c) */
+/* overlay_13 - disable using absolute target path (i2c) */
 
-	fragment@0 {
-		target-path = "/testcase-data/overlay-node/test-bus/i2c-test-bus/test-unittest13";
-		__overlay__ {
-			status = "disabled";
-		};
-	};
+&{/testcase-data/overlay-node/test-bus/i2c-test-bus/test-unittest13} {
+	status = "disabled";
 };
-- 
2.34.1

[PATCH v2 11/13] of: unittest-data: Fix whitespace - blank lines

[Geert Uytterhoeven]

Blank line between properties and subnodes.
Blank line between subsequent subnodes.
No blank line after subnode opening curly brace.
No blank line after subnode closing curly brace.
No blank line at end of file.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - No changes.
---
 drivers/of/unittest-data/overlay.dtso                  | 2 --
 drivers/of/unittest-data/overlay_11.dtso               | 1 -
 drivers/of/unittest-data/overlay_15.dtso               | 1 +
 drivers/of/unittest-data/overlay_4.dtso                | 1 -
 drivers/of/unittest-data/overlay_bad_add_dup_node.dtso | 1 -
 drivers/of/unittest-data/overlay_bad_add_dup_prop.dtso | 1 -
 drivers/of/unittest-data/overlay_bad_phandle.dtso      | 1 -
 drivers/of/unittest-data/overlay_bad_symbol.dtso       | 1 -
 drivers/of/unittest-data/overlay_common.dtsi           | 2 --
 drivers/of/unittest-data/overlay_gpio_01.dtso          | 1 +
 drivers/of/unittest-data/overlay_gpio_02a.dtso         | 1 +
 drivers/of/unittest-data/overlay_gpio_02b.dtso         | 1 +
 drivers/of/unittest-data/overlay_gpio_03.dtso          | 1 +
 drivers/of/unittest-data/overlay_gpio_04a.dtso         | 1 +
 drivers/of/unittest-data/overlay_gpio_04b.dtso         | 1 +
 drivers/of/unittest-data/testcases_common.dtsi         | 1 +
 drivers/of/unittest-data/tests-interrupts.dtsi         | 1 +
 drivers/of/unittest-data/tests-overlay.dtsi            | 1 -
 drivers/of/unittest-data/tests-phandle.dtsi            | 2 ++
 19 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/of/unittest-data/overlay.dtso b/drivers/of/unittest-data/overlay.dtso
index 3bbc59e922fe71c3..0c078b08ef083561 100644
--- a/drivers/of/unittest-data/overlay.dtso
+++ b/drivers/of/unittest-data/overlay.dtso
@@ -3,7 +3,6 @@
 /plugin/;
 
 &electric_1 {
-
 	status = "okay";
 
 	hvac_2: hvac-large-1 {
@@ -57,7 +56,6 @@ ride_200_right: track@20 {
 };
 
 &lights_2 {
-
 	status = "okay";
 	color = "purple", "white", "red", "green";
 	rate = < 3 256 >;
diff --git a/drivers/of/unittest-data/overlay_11.dtso b/drivers/of/unittest-data/overlay_11.dtso
index 9a79b253a8098833..7d04ff503a18ffba 100644
--- a/drivers/of/unittest-data/overlay_11.dtso
+++ b/drivers/of/unittest-data/overlay_11.dtso
@@ -23,6 +23,5 @@ test-unittest111 {
 			status = "okay";
 			reg = <1>;
 		};
-
 	};
 };
diff --git a/drivers/of/unittest-data/overlay_15.dtso b/drivers/of/unittest-data/overlay_15.dtso
index 5728490474f6bd2d..ba02ae1fed38b64a 100644
--- a/drivers/of/unittest-data/overlay_15.dtso
+++ b/drivers/of/unittest-data/overlay_15.dtso
@@ -7,6 +7,7 @@
 &unittest_i2c_test_bus {
 	#address-cells = <1>;
 	#size-cells = <0>;
+
 	test-unittest15 {
 		reg = <11>;
 		compatible = "unittest-i2c-mux";
diff --git a/drivers/of/unittest-data/overlay_4.dtso b/drivers/of/unittest-data/overlay_4.dtso
index a8a77ddf9abe829c..9b9eadddb4a09af3 100644
--- a/drivers/of/unittest-data/overlay_4.dtso
+++ b/drivers/of/unittest-data/overlay_4.dtso
@@ -5,7 +5,6 @@
 /* overlay_4 - test insertion of a full node */
 
 &unittest_test_bus {
-
 	/* suppress DTC warning */
 	#address-cells = <1>;
 	#size-cells = <0>;
diff --git a/drivers/of/unittest-data/overlay_bad_add_dup_node.dtso b/drivers/of/unittest-data/overlay_bad_add_dup_node.dtso
index 145dfc3b1024191a..a8d8534f725c10ea 100644
--- a/drivers/of/unittest-data/overlay_bad_add_dup_node.dtso
+++ b/drivers/of/unittest-data/overlay_bad_add_dup_node.dtso
@@ -13,7 +13,6 @@
  */
 
 &electric_1 {
-
 	motor-1 {
 		controller {
 			power_bus = < 0x1 0x2 >;
diff --git a/drivers/of/unittest-data/overlay_bad_add_dup_prop.dtso b/drivers/of/unittest-data/overlay_bad_add_dup_prop.dtso
index 6327d1ffb9636589..3f0ee9cbefb815be 100644
--- a/drivers/of/unittest-data/overlay_bad_add_dup_prop.dtso
+++ b/drivers/of/unittest-data/overlay_bad_add_dup_prop.dtso
@@ -24,7 +24,6 @@
  */
 
 &electric_1 {
-
 	motor-1 {
 		electric {
 			rpm_avail = < 100 >;
diff --git a/drivers/of/unittest-data/overlay_bad_phandle.dtso b/drivers/of/unittest-data/overlay_bad_phandle.dtso
index 83b79736031878fc..ab2c7df1019674f1 100644
--- a/drivers/of/unittest-data/overlay_bad_phandle.dtso
+++ b/drivers/of/unittest-data/overlay_bad_phandle.dtso
@@ -3,7 +3,6 @@
 /plugin/;
 
 &electric_1 {
-
 	// This label should cause an error when the overlay
 	// is applied.  There is already a phandle value
 	// in the base tree for motor-1.
diff --git a/drivers/of/unittest-data/overlay_bad_symbol.dtso b/drivers/of/unittest-data/overlay_bad_symbol.dtso
index 98c6d1de144a21ab..5d4e34baf1d8e3ed 100644
--- a/drivers/of/unittest-data/overlay_bad_symbol.dtso
+++ b/drivers/of/unittest-data/overlay_bad_symbol.dtso
@@ -3,7 +3,6 @@
 /plugin/;
 
 &electric_1 {
-
 	// This label should cause an error when the overlay
 	// is applied.  There is already a symbol hvac_1
 	// in the base tree
diff --git a/drivers/of/unittest-data/overlay_common.dtsi b/drivers/of/unittest-data/overlay_common.dtsi
index 08874a72556e050f..491b89c43cf38321 100644
--- a/drivers/of/unittest-data/overlay_common.dtsi
+++ b/drivers/of/unittest-data/overlay_common.dtsi
@@ -85,7 +85,5 @@ retail_1: vending@50000 {
 			compatible = "ot,tickets";
 			status = "disabled";
 		};
-
 	};
 };
-
diff --git a/drivers/of/unittest-data/overlay_gpio_01.dtso b/drivers/of/unittest-data/overlay_gpio_01.dtso
index 699ff104ae1062f9..bb3a31a2137aea17 100644
--- a/drivers/of/unittest-data/overlay_gpio_01.dtso
+++ b/drivers/of/unittest-data/overlay_gpio_01.dtso
@@ -5,6 +5,7 @@
 &unittest_test_bus {
 	#address-cells = <1>;
 	#size-cells = <0>;
+
 	gpio@0 {
 		compatible = "unittest-gpio";
 		reg = <0>;
diff --git a/drivers/of/unittest-data/overlay_gpio_02a.dtso b/drivers/of/unittest-data/overlay_gpio_02a.dtso
index ec59aff6ed47ed50..da955537df74be90 100644
--- a/drivers/of/unittest-data/overlay_gpio_02a.dtso
+++ b/drivers/of/unittest-data/overlay_gpio_02a.dtso
@@ -5,6 +5,7 @@
 &unittest_test_bus {
 	#address-cells = <1>;
 	#size-cells = <0>;
+
 	gpio@2 {
 		compatible = "unittest-gpio";
 		reg = <2>;
diff --git a/drivers/of/unittest-data/overlay_gpio_02b.dtso b/drivers/of/unittest-data/overlay_gpio_02b.dtso
index 43ce111d41ceb8d5..79503965d3d7d090 100644
--- a/drivers/of/unittest-data/overlay_gpio_02b.dtso
+++ b/drivers/of/unittest-data/overlay_gpio_02b.dtso
@@ -5,6 +5,7 @@
 &unittest_test_bus {
 	#address-cells = <1>;
 	#size-cells = <0>;
+
 	gpio@2 {
 		line-a {
 			gpio-hog;
diff --git a/drivers/of/unittest-data/overlay_gpio_03.dtso b/drivers/of/unittest-data/overlay_gpio_03.dtso
index 6e0312340a1ba758..d8c709616029b8ba 100644
--- a/drivers/of/unittest-data/overlay_gpio_03.dtso
+++ b/drivers/of/unittest-data/overlay_gpio_03.dtso
@@ -5,6 +5,7 @@
 &unittest_test_bus {
 	#address-cells = <1>;
 	#size-cells = <0>;
+
 	gpio@3 {
 		compatible = "unittest-gpio";
 		reg = <3>;
diff --git a/drivers/of/unittest-data/overlay_gpio_04a.dtso b/drivers/of/unittest-data/overlay_gpio_04a.dtso
index 7b1e04ebfa7a0410..de86511972c202a0 100644
--- a/drivers/of/unittest-data/overlay_gpio_04a.dtso
+++ b/drivers/of/unittest-data/overlay_gpio_04a.dtso
@@ -5,6 +5,7 @@
 &unittest_test_bus {
 	#address-cells = <1>;
 	#size-cells = <0>;
+
 	gpio@4 {
 		compatible = "unittest-gpio";
 		reg = <4>;
diff --git a/drivers/of/unittest-data/overlay_gpio_04b.dtso b/drivers/of/unittest-data/overlay_gpio_04b.dtso
index a14e95c6699aeb06..dc6eff22f927218b 100644
--- a/drivers/of/unittest-data/overlay_gpio_04b.dtso
+++ b/drivers/of/unittest-data/overlay_gpio_04b.dtso
@@ -5,6 +5,7 @@
 &unittest_test_bus {
 	#address-cells = <1>;
 	#size-cells = <0>;
+
 	gpio@4 {
 		line-c {
 			gpio-hog;
diff --git a/drivers/of/unittest-data/testcases_common.dtsi b/drivers/of/unittest-data/testcases_common.dtsi
index e7887f2301c102e6..1c2cdf353ae36b50 100644
--- a/drivers/of/unittest-data/testcases_common.dtsi
+++ b/drivers/of/unittest-data/testcases_common.dtsi
@@ -5,6 +5,7 @@ testcase-data {
 		changeset {
 			prop-update = "hello";
 			prop-remove = "world";
+
 			node-remove {
 			};
 		};
diff --git a/drivers/of/unittest-data/tests-interrupts.dtsi b/drivers/of/unittest-data/tests-interrupts.dtsi
index ecc74dbcc373f655..7c9f31cc131bae79 100644
--- a/drivers/of/unittest-data/tests-interrupts.dtsi
+++ b/drivers/of/unittest-data/tests-interrupts.dtsi
@@ -5,6 +5,7 @@ testcase-data {
 		interrupts {
 			#address-cells = <1>;
 			#size-cells = <1>;
+
 			test_intc0: intc0 {
 				interrupt-controller;
 				#interrupt-cells = <1>;
diff --git a/drivers/of/unittest-data/tests-overlay.dtsi b/drivers/of/unittest-data/tests-overlay.dtsi
index 4ea024d908ee22d6..eb35e8aa5d5ace95 100644
--- a/drivers/of/unittest-data/tests-overlay.dtsi
+++ b/drivers/of/unittest-data/tests-overlay.dtsi
@@ -3,7 +3,6 @@
 / {
 	testcase-data {
 		overlay-node {
-
 			/* test bus */
 			unittest_test_bus: test-bus {
 				compatible = "simple-bus";
diff --git a/drivers/of/unittest-data/tests-phandle.dtsi b/drivers/of/unittest-data/tests-phandle.dtsi
index 6b33be4c4416ce7b..d01f92f0f0db7f57 100644
--- a/drivers/of/unittest-data/tests-phandle.dtsi
+++ b/drivers/of/unittest-data/tests-phandle.dtsi
@@ -8,7 +8,9 @@ aliases {
 	testcase: testcase-data {
 		security-password = "password";
 		duplicate-name = "duplicate";
+
 		duplicate-name { };
+
 		phandle-tests {
 			provider0: provider0 {
 				#phandle-cells = <0>;
-- 
2.34.1

[PATCH v2 12/13] of: unittest-data: Fix whitespace - indentation

[Geert Uytterhoeven]

Single space for each indentation level.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - No changes.
---
 drivers/of/unittest-data/overlay_bad_add_dup_node.dtso | 6 +++---
 drivers/of/unittest-data/overlay_bad_add_dup_prop.dtso | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/of/unittest-data/overlay_bad_add_dup_node.dtso b/drivers/of/unittest-data/overlay_bad_add_dup_node.dtso
index a8d8534f725c10ea..11aa1401244d9685 100644
--- a/drivers/of/unittest-data/overlay_bad_add_dup_node.dtso
+++ b/drivers/of/unittest-data/overlay_bad_add_dup_node.dtso
@@ -21,7 +21,7 @@ controller {
 };
 
 &spin_ctrl_1 {
-		controller {
-			power_bus_emergency = < 0x101 0x102 >;
-		};
+	controller {
+		power_bus_emergency = < 0x101 0x102 >;
+	};
 };
diff --git a/drivers/of/unittest-data/overlay_bad_add_dup_prop.dtso b/drivers/of/unittest-data/overlay_bad_add_dup_prop.dtso
index 3f0ee9cbefb815be..5af099cc2174e273 100644
--- a/drivers/of/unittest-data/overlay_bad_add_dup_prop.dtso
+++ b/drivers/of/unittest-data/overlay_bad_add_dup_prop.dtso
@@ -32,7 +32,7 @@ electric {
 };
 
 &spin_ctrl_1 {
-		electric {
-			rpm_avail = < 100 200 >;
-		};
+	electric {
+		rpm_avail = < 100 200 >;
+	};
 };
-- 
2.34.1

[PATCH v2 13/13] of: unittest-data: Fix whitespace - angular brackets

[Geert Uytterhoeven]

No space after opening or before closing angular bracket.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - No changes.
---
 drivers/of/unittest-data/overlay.dtso         | 30 ++++++++--------
 .../overlay_bad_add_dup_node.dtso             |  4 +--
 .../overlay_bad_add_dup_prop.dtso             |  4 +--
 .../of/unittest-data/overlay_bad_phandle.dtso |  4 +--
 .../of/unittest-data/overlay_bad_symbol.dtso  |  4 +--
 drivers/of/unittest-data/overlay_common.dtsi  | 34 +++++++++----------
 6 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/drivers/of/unittest-data/overlay.dtso b/drivers/of/unittest-data/overlay.dtso
index 0c078b08ef083561..b3e807b99852363e 100644
--- a/drivers/of/unittest-data/overlay.dtso
+++ b/drivers/of/unittest-data/overlay.dtso
@@ -7,8 +7,8 @@ &electric_1 {
 
 	hvac_2: hvac-large-1 {
 		compatible = "ot,hvac-large";
-		heat-range = < 40 75 >;
-		cool-range = < 65 80 >;
+		heat-range = <40 75>;
+		cool-range = <65 80>;
 	};
 };
 
@@ -23,11 +23,11 @@ ride@100 {
 		#size-cells = <1>;
 
 		track@30 {
-			incline-up = < 48 32 16 >;
+			incline-up = <48 32 16>;
 		};
 
 		track@40 {
-			incline-up = < 47 31 15 >;
+			incline-up = <47 31 15>;
 		};
 	};
 
@@ -35,22 +35,22 @@ ride_200: ride@200 {
 		#address-cells = <1>;
 		#size-cells = <1>;
 		compatible = "ot,ferris-wheel";
-		reg = < 0x00000200 0x100 >;
-		hvac-provider = < &hvac_2 >;
-		hvac-thermostat = < 27 32 > ;
-		hvac-zones = < 12 5 >;
+		reg = <0x00000200 0x100>;
+		hvac-provider = <&hvac_2>;
+		hvac-thermostat = <27 32> ;
+		hvac-zones = <12 5>;
 		hvac-zone-names = "operator", "snack-bar";
-		spin-controller = < &spin_ctrl_1 3 >;
-		spin-rph = < 30 >;
-		gondolas = < 16 >;
-		gondola-capacity = < 6 >;
+		spin-controller = <&spin_ctrl_1 3>;
+		spin-rph = <30>;
+		gondolas = <16>;
+		gondola-capacity = <6>;
 
 		ride_200_left: track@10 {
-			reg = < 0x00000010 0x10 >;
+			reg = <0x00000010 0x10>;
 		};
 
 		ride_200_right: track@20 {
-			reg = < 0x00000020 0x10 >;
+			reg = <0x00000020 0x10>;
 		};
 	};
 };
@@ -58,5 +58,5 @@ ride_200_right: track@20 {
 &lights_2 {
 	status = "okay";
 	color = "purple", "white", "red", "green";
-	rate = < 3 256 >;
+	rate = <3 256>;
 };
diff --git a/drivers/of/unittest-data/overlay_bad_add_dup_node.dtso b/drivers/of/unittest-data/overlay_bad_add_dup_node.dtso
index 11aa1401244d9685..9b53412b20796e29 100644
--- a/drivers/of/unittest-data/overlay_bad_add_dup_node.dtso
+++ b/drivers/of/unittest-data/overlay_bad_add_dup_node.dtso
@@ -15,13 +15,13 @@
 &electric_1 {
 	motor-1 {
 		controller {
-			power_bus = < 0x1 0x2 >;
+			power_bus = <0x1 0x2>;
 		};
 	};
 };
 
 &spin_ctrl_1 {
 	controller {
-		power_bus_emergency = < 0x101 0x102 >;
+		power_bus_emergency = <0x101 0x102>;
 	};
 };
diff --git a/drivers/of/unittest-data/overlay_bad_add_dup_prop.dtso b/drivers/of/unittest-data/overlay_bad_add_dup_prop.dtso
index 5af099cc2174e273..e03f791655b07b3f 100644
--- a/drivers/of/unittest-data/overlay_bad_add_dup_prop.dtso
+++ b/drivers/of/unittest-data/overlay_bad_add_dup_prop.dtso
@@ -26,13 +26,13 @@
 &electric_1 {
 	motor-1 {
 		electric {
-			rpm_avail = < 100 >;
+			rpm_avail = <100>;
 		};
 	};
 };
 
 &spin_ctrl_1 {
 	electric {
-		rpm_avail = < 100 200 >;
+		rpm_avail = <100 200>;
 	};
 };
diff --git a/drivers/of/unittest-data/overlay_bad_phandle.dtso b/drivers/of/unittest-data/overlay_bad_phandle.dtso
index ab2c7df1019674f1..a61ffc0738e3bf01 100644
--- a/drivers/of/unittest-data/overlay_bad_phandle.dtso
+++ b/drivers/of/unittest-data/overlay_bad_phandle.dtso
@@ -7,7 +7,7 @@ &electric_1 {
 	// is applied.  There is already a phandle value
 	// in the base tree for motor-1.
 	spin_ctrl_1_conflict: motor-1 {
-		accelerate = < 3 >;
-		decelerate = < 5 >;
+		accelerate = <3>;
+		decelerate = <5>;
 	};
 };
diff --git a/drivers/of/unittest-data/overlay_bad_symbol.dtso b/drivers/of/unittest-data/overlay_bad_symbol.dtso
index 5d4e34baf1d8e3ed..07f730384cdde1f8 100644
--- a/drivers/of/unittest-data/overlay_bad_symbol.dtso
+++ b/drivers/of/unittest-data/overlay_bad_symbol.dtso
@@ -8,8 +8,8 @@ &electric_1 {
 	// in the base tree
 	hvac_1: hvac-medium-2 {
 		compatible = "ot,hvac-medium";
-		heat-range = < 50 75 >;
-		cool-range = < 60 80 >;
+		heat-range = <50 75>;
+		cool-range = <60 80>;
 	};
 
 };
diff --git a/drivers/of/unittest-data/overlay_common.dtsi b/drivers/of/unittest-data/overlay_common.dtsi
index 491b89c43cf38321..a9d7cdbd5ddc470c 100644
--- a/drivers/of/unittest-data/overlay_common.dtsi
+++ b/drivers/of/unittest-data/overlay_common.dtsi
@@ -16,19 +16,19 @@ testcase-data-2 {
 
 		electric_1: substation@100 {
 			compatible = "ot,big-volts-control";
-			reg = < 0x00000100 0x100 >;
+			reg = <0x00000100 0x100>;
 			status = "disabled";
 
 			hvac_1: hvac-medium-1 {
 				compatible = "ot,hvac-medium";
-				heat-range = < 50 75 >;
-				cool-range = < 60 80 >;
+				heat-range = <50 75>;
+				cool-range = <60 80>;
 			};
 
 			spin_ctrl_1: motor-1 {
 				compatible = "ot,ferris-wheel-motor";
 				spin = "clockwise";
-				rpm_avail = < 50 >;
+				rpm_avail = <50>;
 			};
 
 			spin_ctrl_2: motor-8 {
@@ -41,27 +41,27 @@ rides_1: fairway-1 {
 			#size-cells = <1>;
 			compatible = "ot,rides";
 			status = "disabled";
-			orientation = < 127 >;
+			orientation = <127>;
 
 			ride@100 {
 				#address-cells = <1>;
 				#size-cells = <1>;
 				compatible = "ot,roller-coaster";
-				reg = < 0x00000100 0x100 >;
-				hvac-provider = < &hvac_1 >;
-				hvac-thermostat = < 29 > ;
-				hvac-zones = < 14 >;
+				reg = <0x00000100 0x100>;
+				hvac-provider = <&hvac_1>;
+				hvac-thermostat = <29> ;
+				hvac-zones = <14>;
 				hvac-zone-names = "operator";
-				spin-controller = < &spin_ctrl_2 5 &spin_ctrl_2 7 >;
+				spin-controller = <&spin_ctrl_2 5 &spin_ctrl_2 7>;
 				spin-controller-names = "track_1", "track_2";
-				queues = < 2 >;
+				queues = <2>;
 
 				track@30 {
-					reg = < 0x00000030 0x10 >;
+					reg = <0x00000030 0x10>;
 				};
 
 				track@40 {
-					reg = < 0x00000040 0x10 >;
+					reg = <0x00000040 0x10>;
 				};
 
 			};
@@ -69,19 +69,19 @@ track@40 {
 
 		lights_1: lights@30000 {
 			compatible = "ot,work-lights";
-			reg = < 0x00030000 0x1000 >;
+			reg = <0x00030000 0x1000>;
 			status = "disabled";
 		};
 
 		lights_2: lights@40000 {
 			compatible = "ot,show-lights";
-			reg = < 0x00040000 0x1000 >;
+			reg = <0x00040000 0x1000>;
 			status = "disabled";
-			rate = < 13 138 >;
+			rate = <13 138>;
 		};
 
 		retail_1: vending@50000 {
-			reg = < 0x00050000 0x1000 >;
+			reg = <0x00050000 0x1000>;
 			compatible = "ot,tickets";
 			status = "disabled";
 		};
-- 
2.34.1

Re: [PATCH v2 00/13] of: overlay/unittest: Miscellaneous fixes and improvements

[Rob Herring]

On Fri, Jul 28, 2023 at 10:50:26AM +0200, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> This patch series contains miscellaneous fixes and improvements for
> dynamic DT overlays and the related unit tests.
> 
> The first two patches are fixes for a lock-up and a crash.
> The remaining patches are smaller fixes, enhancements and cleanups for
> the overlay tests, including one new test.
> 
> I ran into the crash when accidentally loading the wrong overlay (using
> the out-of-tree DT overlay configfs[1]), and removing it afterwards.
> As this case was not yet covered by the unittests, I added a test.
> I enhanced the tests to clean up partial state after a failed
> overlay apply attempt, which triggered the lock-up.
> 
> Changes compared to v1[2]:
>   - Correct fixes tag and update description.
>   - Merge differently, as requested by Rob.
> 
> Thanks for your comments!
> 
> [1] https://elinux.org/R-Car/DT-Overlays
> [2] https://lore.kernel.org/r/cover.1689776064.git.geert+renesas@glider.be
> 
> Geert Uytterhoeven (13):
>   of: dynamic: Do not use "%pOF" while holding devtree_lock
>   of: overlay: Call of_changeset_init() early
>   of: unittest: Fix overlay type in apply/revert check
>   of: unittest: Restore indentation in overlay_bad_add_dup_prop test
>   of: unittest: Improve messages and comments in apply/revert checks
>   of: unittest: Merge of_unittest_apply{,_revert}_overlay_check()
>   of: unittest: Cleanup partially-applied overlays
>   of: unittest: Add separators to of_unittest_overlay_high_level()
>   of: overlay: unittest: Add test for unresolved symbol
>   of: unittest-data: Convert remaining overlay DTS files to sugar syntax
>   of: unittest-data: Fix whitespace - blank lines
>   of: unittest-data: Fix whitespace - indentation
>   of: unittest-data: Fix whitespace - angular brackets

I've applied patches 2-13. For patch 1, I sent an alternative[1].

Rob

[1] https://lore.kernel.org/all/20230728231950.1619073-1-robh@kernel.org/

Re: [PATCH v2 00/13] of: overlay/unittest: Miscellaneous fixes and improvements

[Rob Herring]

On Mon, Jul 31, 2023 at 10:14:25AM -0600, Rob Herring wrote:
> On Fri, Jul 28, 2023 at 10:50:26AM +0200, Geert Uytterhoeven wrote:
> > 	Hi all,
> > 
> > This patch series contains miscellaneous fixes and improvements for
> > dynamic DT overlays and the related unit tests.
> > 
> > The first two patches are fixes for a lock-up and a crash.
> > The remaining patches are smaller fixes, enhancements and cleanups for
> > the overlay tests, including one new test.
> > 
> > I ran into the crash when accidentally loading the wrong overlay (using
> > the out-of-tree DT overlay configfs[1]), and removing it afterwards.
> > As this case was not yet covered by the unittests, I added a test.
> > I enhanced the tests to clean up partial state after a failed
> > overlay apply attempt, which triggered the lock-up.
> > 
> > Changes compared to v1[2]:
> >   - Correct fixes tag and update description.
> >   - Merge differently, as requested by Rob.
> > 
> > Thanks for your comments!
> > 
> > [1] https://elinux.org/R-Car/DT-Overlays
> > [2] https://lore.kernel.org/r/cover.1689776064.git.geert+renesas@glider.be
> > 
> > Geert Uytterhoeven (13):
> >   of: dynamic: Do not use "%pOF" while holding devtree_lock
> >   of: overlay: Call of_changeset_init() early
> >   of: unittest: Fix overlay type in apply/revert check
> >   of: unittest: Restore indentation in overlay_bad_add_dup_prop test
> >   of: unittest: Improve messages and comments in apply/revert checks
> >   of: unittest: Merge of_unittest_apply{,_revert}_overlay_check()
> >   of: unittest: Cleanup partially-applied overlays
> >   of: unittest: Add separators to of_unittest_overlay_high_level()
> >   of: overlay: unittest: Add test for unresolved symbol
> >   of: unittest-data: Convert remaining overlay DTS files to sugar syntax
> >   of: unittest-data: Fix whitespace - blank lines
> >   of: unittest-data: Fix whitespace - indentation
> >   of: unittest-data: Fix whitespace - angular brackets
> 
> I've applied patches 2-13. For patch 1, I sent an alternative[1].

I guess there's a dependency on patch 1 because I hang here:

[    1.341292] OF: overlay: ERROR: multiple fragments add and/or delete node /testcase-data-2/substation@100/motor-1/controller
[    1.343222] OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/controller/name

Rob

Re: [PATCH v2 07/13] of: unittest: Cleanup partially-applied overlays

[Rob Herring]

On Fri, Jul 28, 2023 at 2:50 AM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> When of_overlay_fdt_apply() fails, the changeset may be partially
> applied, and the caller is still expected to call of_overlay_remove() to
> clean up this partial state.  However, overlay_17 is the only test that
> takes care of cleaning up after an (expected) failure.
>
> Instead of adding cleanup code to each individual test, extend
> overlay_info with the optional expected return value of
> of_overlay_remove(), and handle cleanup in the overlay_data_apply()
> helper.  While at it, simplify the end marker in the overlay_info table.
>
> Update the expected error output for errors during the newly cleanup.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - No changes.
> ---
>  drivers/of/unittest.c | 130 +++++++++++++++++++++++++++---------------
>  1 file changed, 83 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index b23a44de091bd044..f9c09d5787362601 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -2964,12 +2964,6 @@ static void __init of_unittest_overlay_notify(void)
>
>         unittest(ovcs_id, "ovcs_id not created for overlay_17\n");
>
> -       if (ovcs_id) {
> -               ret = of_overlay_remove(&ovcs_id);
> -               unittest(!ret,
> -                       "overlay_17 of_overlay_remove(), ret = %d\n", ret);
> -       }
> -
>         /* ---  overlay 18  --- */
>
>         unittest(overlay_data_apply("overlay_18", &ovcs_id),
> @@ -3257,17 +3251,19 @@ static void __init of_unittest_lifecycle(void)
>         extern uint8_t __dtbo_##overlay_name##_begin[]; \
>         extern uint8_t __dtbo_##overlay_name##_end[]
>
> -#define OVERLAY_INFO(overlay_name, expected) \
> -{      .dtbo_begin       = __dtbo_##overlay_name##_begin, \
> -       .dtbo_end         = __dtbo_##overlay_name##_end, \
> -       .expected_result = expected, \
> -       .name            = #overlay_name, \
> +#define OVERLAY_INFO(overlay_name, expected, expected_remove) \
> +{      .dtbo_begin             = __dtbo_##overlay_name##_begin, \
> +       .dtbo_end               = __dtbo_##overlay_name##_end, \
> +       .expected_result        = expected, \
> +       .expected_result_remove = expected_remove, \
> +       .name                   = #overlay_name, \
>  }
>
>  struct overlay_info {
>         uint8_t         *dtbo_begin;
>         uint8_t         *dtbo_end;
>         int             expected_result;
> +       int             expected_result_remove; /* if apply failed */
>         int             ovcs_id;
>         char            *name;
>  };
> @@ -3307,40 +3303,40 @@ OVERLAY_INFO_EXTERN(overlay_bad_symbol);
>
>  /* entries found by name */
>  static struct overlay_info overlays[] = {
> -       OVERLAY_INFO(overlay_base, -9999),
> -       OVERLAY_INFO(overlay, 0),
> -       OVERLAY_INFO(overlay_0, 0),
> -       OVERLAY_INFO(overlay_1, 0),
> -       OVERLAY_INFO(overlay_2, 0),
> -       OVERLAY_INFO(overlay_3, 0),
> -       OVERLAY_INFO(overlay_4, 0),
> -       OVERLAY_INFO(overlay_5, 0),
> -       OVERLAY_INFO(overlay_6, 0),
> -       OVERLAY_INFO(overlay_7, 0),
> -       OVERLAY_INFO(overlay_8, 0),
> -       OVERLAY_INFO(overlay_9, 0),
> -       OVERLAY_INFO(overlay_10, 0),
> -       OVERLAY_INFO(overlay_11, 0),
> -       OVERLAY_INFO(overlay_12, 0),
> -       OVERLAY_INFO(overlay_13, 0),
> -       OVERLAY_INFO(overlay_15, 0),
> -       OVERLAY_INFO(overlay_16, -EBUSY),
> -       OVERLAY_INFO(overlay_17, -EEXIST),
> -       OVERLAY_INFO(overlay_18, 0),
> -       OVERLAY_INFO(overlay_19, 0),
> -       OVERLAY_INFO(overlay_20, 0),
> -       OVERLAY_INFO(overlay_gpio_01, 0),
> -       OVERLAY_INFO(overlay_gpio_02a, 0),
> -       OVERLAY_INFO(overlay_gpio_02b, 0),
> -       OVERLAY_INFO(overlay_gpio_03, 0),
> -       OVERLAY_INFO(overlay_gpio_04a, 0),
> -       OVERLAY_INFO(overlay_gpio_04b, 0),
> -       OVERLAY_INFO(overlay_bad_add_dup_node, -EINVAL),
> -       OVERLAY_INFO(overlay_bad_add_dup_prop, -EINVAL),
> -       OVERLAY_INFO(overlay_bad_phandle, -EINVAL),
> -       OVERLAY_INFO(overlay_bad_symbol, -EINVAL),
> +       OVERLAY_INFO(overlay_base, -9999, 0),
> +       OVERLAY_INFO(overlay, 0, 0),
> +       OVERLAY_INFO(overlay_0, 0, 0),
> +       OVERLAY_INFO(overlay_1, 0, 0),
> +       OVERLAY_INFO(overlay_2, 0, 0),
> +       OVERLAY_INFO(overlay_3, 0, 0),
> +       OVERLAY_INFO(overlay_4, 0, 0),
> +       OVERLAY_INFO(overlay_5, 0, 0),
> +       OVERLAY_INFO(overlay_6, 0, 0),
> +       OVERLAY_INFO(overlay_7, 0, 0),
> +       OVERLAY_INFO(overlay_8, 0, 0),
> +       OVERLAY_INFO(overlay_9, 0, 0),
> +       OVERLAY_INFO(overlay_10, 0, 0),
> +       OVERLAY_INFO(overlay_11, 0, 0),
> +       OVERLAY_INFO(overlay_12, 0, 0),
> +       OVERLAY_INFO(overlay_13, 0, 0),
> +       OVERLAY_INFO(overlay_15, 0, 0),
> +       OVERLAY_INFO(overlay_16, -EBUSY, 0),
> +       OVERLAY_INFO(overlay_17, -EEXIST, 0),
> +       OVERLAY_INFO(overlay_18, 0, 0),
> +       OVERLAY_INFO(overlay_19, 0, 0),
> +       OVERLAY_INFO(overlay_20, 0, 0),
> +       OVERLAY_INFO(overlay_gpio_01, 0, 0),
> +       OVERLAY_INFO(overlay_gpio_02a, 0, 0),
> +       OVERLAY_INFO(overlay_gpio_02b, 0, 0),
> +       OVERLAY_INFO(overlay_gpio_03, 0, 0),
> +       OVERLAY_INFO(overlay_gpio_04a, 0, 0),
> +       OVERLAY_INFO(overlay_gpio_04b, 0, 0),
> +       OVERLAY_INFO(overlay_bad_add_dup_node, -EINVAL, -ENODEV),
> +       OVERLAY_INFO(overlay_bad_add_dup_prop, -EINVAL, -ENODEV),
> +       OVERLAY_INFO(overlay_bad_phandle, -EINVAL, 0),
> +       OVERLAY_INFO(overlay_bad_symbol, -EINVAL, -ENODEV),
>         /* end marker */
> -       {.dtbo_begin = NULL, .dtbo_end = NULL, .expected_result = 0, .name = NULL}
> +       { }
>  };
>
>  static struct device_node *overlay_base_root;
> @@ -3435,8 +3431,9 @@ void __init unittest_unflatten_overlay_base(void)
>  static int __init overlay_data_apply(const char *overlay_name, int *ovcs_id)
>  {
>         struct overlay_info *info;
> +       int passed = 1;
>         int found = 0;
> -       int ret;
> +       int ret, ret2;
>         u32 size;
>
>         for (info = overlays; info && info->name; info++) {
> @@ -3463,11 +3460,24 @@ static int __init overlay_data_apply(const char *overlay_name, int *ovcs_id)
>         pr_debug("%s applied\n", overlay_name);
>
>  out:
> -       if (ret != info->expected_result)
> +       if (ret != info->expected_result) {
>                 pr_err("of_overlay_fdt_apply() expected %d, ret=%d, %s\n",
>                        info->expected_result, ret, overlay_name);
> +               passed = 0;
> +       }
> +
> +       if (ret < 0) {
> +               /* changeset may be partially applied */
> +               ret2 = of_overlay_remove(&info->ovcs_id);
> +               if (ret2 != info->expected_result_remove) {
> +                       pr_err("of_overlay_remove() expected %d, ret=%d, %s\n",
> +                              info->expected_result_remove, ret2,
> +                              overlay_name);
> +                       passed = 0;
> +               }
> +       }
>
> -       return (ret == info->expected_result);
> +       return passed;
>  }
>
>  /*
> @@ -3660,10 +3670,18 @@ static __init void of_unittest_overlay_high_level(void)
>                      "OF: overlay: ERROR: multiple fragments add and/or delete node /testcase-data-2/substation@100/motor-1/controller");
>         EXPECT_BEGIN(KERN_ERR,
>                      "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/controller/name");
> +       EXPECT_BEGIN(KERN_ERR,
> +                    "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/controller/name");
> +       EXPECT_BEGIN(KERN_ERR,
> +                    "OF: Error reverting changeset (-19)");
>
>         unittest(overlay_data_apply("overlay_bad_add_dup_node", NULL),
>                  "Adding overlay 'overlay_bad_add_dup_node' failed\n");
>
> +       EXPECT_END(KERN_ERR,
> +                  "OF: Error reverting changeset (-19)");
> +       EXPECT_END(KERN_ERR,
> +                  "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/controller/name");
>         EXPECT_END(KERN_ERR,
>                    "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/controller/name");
>         EXPECT_END(KERN_ERR,
> @@ -3675,10 +3693,18 @@ static __init void of_unittest_overlay_high_level(void)
>                      "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/rpm_avail");
>         EXPECT_BEGIN(KERN_ERR,
>                      "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/name");
> +       EXPECT_BEGIN(KERN_ERR,
> +                    "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/electric/name");
> +       EXPECT_BEGIN(KERN_ERR,
> +                    "OF: Error reverting changeset (-19)");
>
>         unittest(overlay_data_apply("overlay_bad_add_dup_prop", NULL),
>                  "Adding overlay 'overlay_bad_add_dup_prop' failed\n");
>
> +       EXPECT_END(KERN_ERR,
> +                  "OF: Error reverting changeset (-19)");
> +       EXPECT_END(KERN_ERR,
> +                  "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/electric/name");
>         EXPECT_END(KERN_ERR,
>                    "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/name");
>         EXPECT_END(KERN_ERR,
> @@ -3689,9 +3715,19 @@ static __init void of_unittest_overlay_high_level(void)
>         unittest(overlay_data_apply("overlay_bad_phandle", NULL),
>                  "Adding overlay 'overlay_bad_phandle' failed\n");
>
> +       EXPECT_BEGIN(KERN_ERR,
> +                    "OF: changeset: remove_property failed @/testcase-data-2/substation@100/hvac-medium-2/name");

I noticed my printing patch will have an issue on this because it
prints the changeset pointer value. I guess we have to manually check
that if EXPECT can't handle some type of wildcard.

> +       EXPECT_BEGIN(KERN_ERR,
> +                    "OF: resolver: overlay phandle fixup failed: -22");
> +
>         unittest(overlay_data_apply("overlay_bad_symbol", NULL),
>                  "Adding overlay 'overlay_bad_symbol' failed\n");
>
> +       EXPECT_END(KERN_ERR,
> +                  "OF: resolver: overlay phandle fixup failed: -22");

I'm seeing "OF: Error reverting changeset (-19)" here instead.
Cut-n-paste error?

Rob

Re: [PATCH v2 07/13] of: unittest: Cleanup partially-applied overlays

[Geert Uytterhoeven]

Hi Rob,

On Tue, Aug 1, 2023 at 10:50 PM Rob Herring <robh+dt@kernel.org> wrote:
> On Fri, Jul 28, 2023 at 2:50 AM Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> > When of_overlay_fdt_apply() fails, the changeset may be partially
> > applied, and the caller is still expected to call of_overlay_remove() to
> > clean up this partial state.  However, overlay_17 is the only test that
> > takes care of cleaning up after an (expected) failure.
> >
> > Instead of adding cleanup code to each individual test, extend
> > overlay_info with the optional expected return value of
> > of_overlay_remove(), and handle cleanup in the overlay_data_apply()
> > helper.  While at it, simplify the end marker in the overlay_info table.
> >
> > Update the expected error output for errors during the newly cleanup.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > v2:
> >   - No changes.

> > --- a/drivers/of/unittest.c
> > +++ b/drivers/of/unittest.c

> > @@ -3689,9 +3715,19 @@ static __init void of_unittest_overlay_high_level(void)
> >         unittest(overlay_data_apply("overlay_bad_phandle", NULL),
> >                  "Adding overlay 'overlay_bad_phandle' failed\n");
> >
> > +       EXPECT_BEGIN(KERN_ERR,
> > +                    "OF: changeset: remove_property failed @/testcase-data-2/substation@100/hvac-medium-2/name");
> > +       EXPECT_BEGIN(KERN_ERR,
> > +                    "OF: resolver: overlay phandle fixup failed: -22");
> > +
> >         unittest(overlay_data_apply("overlay_bad_symbol", NULL),
> >                  "Adding overlay 'overlay_bad_symbol' failed\n");
> >
> > +       EXPECT_END(KERN_ERR,
> > +                  "OF: resolver: overlay phandle fixup failed: -22");
>
> I'm seeing "OF: Error reverting changeset (-19)" here instead.
> Cut-n-paste error?

Indeed. Thanks for pointing this out!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 00/13] of: overlay/unittest: Miscellaneous fixes and improvements

[Geert Uytterhoeven]

Hi Rob,

On Mon, Jul 31, 2023 at 6:35 PM Rob Herring <robh@kernel.org> wrote:
> On Mon, Jul 31, 2023 at 10:14:25AM -0600, Rob Herring wrote:
> > On Fri, Jul 28, 2023 at 10:50:26AM +0200, Geert Uytterhoeven wrote:
> > > This patch series contains miscellaneous fixes and improvements for
> > > dynamic DT overlays and the related unit tests.
> > >
> > > The first two patches are fixes for a lock-up and a crash.
> > > The remaining patches are smaller fixes, enhancements and cleanups for
> > > the overlay tests, including one new test.
> > >
> > > I ran into the crash when accidentally loading the wrong overlay (using
> > > the out-of-tree DT overlay configfs[1]), and removing it afterwards.
> > > As this case was not yet covered by the unittests, I added a test.
> > > I enhanced the tests to clean up partial state after a failed
> > > overlay apply attempt, which triggered the lock-up.
> > >
> > > Changes compared to v1[2]:
> > >   - Correct fixes tag and update description.
> > >   - Merge differently, as requested by Rob.
> > >
> > > Thanks for your comments!
> > >
> > > [1] https://elinux.org/R-Car/DT-Overlays
> > > [2] https://lore.kernel.org/r/cover.1689776064.git.geert+renesas@glider.be
> > >
> > > Geert Uytterhoeven (13):
> > >   of: dynamic: Do not use "%pOF" while holding devtree_lock
> > >   of: overlay: Call of_changeset_init() early
> > >   of: unittest: Fix overlay type in apply/revert check
> > >   of: unittest: Restore indentation in overlay_bad_add_dup_prop test
> > >   of: unittest: Improve messages and comments in apply/revert checks
> > >   of: unittest: Merge of_unittest_apply{,_revert}_overlay_check()
> > >   of: unittest: Cleanup partially-applied overlays
> > >   of: unittest: Add separators to of_unittest_overlay_high_level()
> > >   of: overlay: unittest: Add test for unresolved symbol
> > >   of: unittest-data: Convert remaining overlay DTS files to sugar syntax
> > >   of: unittest-data: Fix whitespace - blank lines
> > >   of: unittest-data: Fix whitespace - indentation
> > >   of: unittest-data: Fix whitespace - angular brackets
> >
> > I've applied patches 2-13. For patch 1, I sent an alternative[1].
>
> I guess there's a dependency on patch 1 because I hang here:
>
> [    1.341292] OF: overlay: ERROR: multiple fragments add and/or delete node /testcase-data-2/substation@100/motor-1/controller
> [    1.343222] OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/controller/name

Yes there is: during removal of a partially-applied overlay, some
errors are printed using %pOF, which must not be done while holding
devtree_lock.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 07/13] of: unittest: Cleanup partially-applied overlays

[Rob Herring]

On Mon, Aug 14, 2023 at 4:21 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Rob,
>
> On Tue, Aug 1, 2023 at 10:50 PM Rob Herring <robh+dt@kernel.org> wrote:
> > On Fri, Jul 28, 2023 at 2:50 AM Geert Uytterhoeven
> > <geert+renesas@glider.be> wrote:
> > > When of_overlay_fdt_apply() fails, the changeset may be partially
> > > applied, and the caller is still expected to call of_overlay_remove() to
> > > clean up this partial state.  However, overlay_17 is the only test that
> > > takes care of cleaning up after an (expected) failure.
> > >
> > > Instead of adding cleanup code to each individual test, extend
> > > overlay_info with the optional expected return value of
> > > of_overlay_remove(), and handle cleanup in the overlay_data_apply()
> > > helper.  While at it, simplify the end marker in the overlay_info table.
> > >
> > > Update the expected error output for errors during the newly cleanup.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > > v2:
> > >   - No changes.
>
> > > --- a/drivers/of/unittest.c
> > > +++ b/drivers/of/unittest.c
>
> > > @@ -3689,9 +3715,19 @@ static __init void of_unittest_overlay_high_level(void)
> > >         unittest(overlay_data_apply("overlay_bad_phandle", NULL),
> > >                  "Adding overlay 'overlay_bad_phandle' failed\n");
> > >
> > > +       EXPECT_BEGIN(KERN_ERR,
> > > +                    "OF: changeset: remove_property failed @/testcase-data-2/substation@100/hvac-medium-2/name");
> > > +       EXPECT_BEGIN(KERN_ERR,
> > > +                    "OF: resolver: overlay phandle fixup failed: -22");
> > > +
> > >         unittest(overlay_data_apply("overlay_bad_symbol", NULL),
> > >                  "Adding overlay 'overlay_bad_symbol' failed\n");
> > >
> > > +       EXPECT_END(KERN_ERR,
> > > +                  "OF: resolver: overlay phandle fixup failed: -22");
> >
> > I'm seeing "OF: Error reverting changeset (-19)" here instead.
> > Cut-n-paste error?
>
> Indeed. Thanks for pointing this out!

I've fixed this up along with the other EXPECTs that changed due to my
rework and applied the series.

Thanks,
Rob