All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 1/2] of: kobj: export of_node_ktype for use by modules
@ 2026-07-20 10:47 Xu Yang
  2026-07-20 10:47 ` [PATCH v5 2/2] device property: add test cases for fwnode_for_each_child_node() Xu Yang
  2026-07-20 11:13 ` [PATCH v5 1/2] of: kobj: export of_node_ktype for use by modules Andy Shevchenko
  0 siblings, 2 replies; 5+ messages in thread
From: Xu Yang @ 2026-07-20 10:47 UTC (permalink / raw)
  To: gregkh, rafael, dakr, robh, saravanak, andriy.shevchenko
  Cc: bartosz.golaszewski, driver-core, linux-kernel, devicetree, imx

From: Xu Yang <xu.yang_2@nxp.com>

of_node_init() is a static inline that references of_node_ktype when
CONFIG_OF_KOBJ=y. Any module that calls of_node_init() will therefore
have an unresolved reference to of_node_ktype at load time, because
the symbol is defined in drivers/of/kobj.c but was never exported.

This causes a modpost build error when CONFIG_OF_KOBJ=y and
CONFIG_DRIVER_PE_KUNIT_TEST=m:

  ERROR: modpost: "of_node_ktype"
  [drivers/base/test/property-entry-test.ko] undefined!

Add EXPORT_SYMBOL_GPL(of_node_ktype) so that GPL-licensed modules
such as the KUnit property-entry test can call of_node_init() without
hitting this linker error.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202607181651.RnUuV8n6-lkp@intel.com/
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v5:
 - new one
---
 drivers/of/kobj.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/of/kobj.c b/drivers/of/kobj.c
index 1bb61a2c3399..eacc53324d62 100644
--- a/drivers/of/kobj.c
+++ b/drivers/of/kobj.c
@@ -27,6 +27,7 @@ static void of_node_release(struct kobject *kobj)
 const struct kobj_type of_node_ktype = {
 	.release = of_node_release,
 };
+EXPORT_SYMBOL_GPL(of_node_ktype);
 
 static ssize_t of_node_property_read(struct file *filp, struct kobject *kobj,
 				const struct bin_attribute *bin_attr, char *buf,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v5 2/2] device property: add test cases for fwnode_for_each_child_node()
  2026-07-20 10:47 [PATCH v5 1/2] of: kobj: export of_node_ktype for use by modules Xu Yang
@ 2026-07-20 10:47 ` Xu Yang
  2026-07-20 11:02   ` sashiko-bot
  2026-07-20 11:21   ` Andy Shevchenko
  2026-07-20 11:13 ` [PATCH v5 1/2] of: kobj: export of_node_ktype for use by modules Andy Shevchenko
  1 sibling, 2 replies; 5+ messages in thread
From: Xu Yang @ 2026-07-20 10:47 UTC (permalink / raw)
  To: gregkh, rafael, dakr, robh, saravanak, andriy.shevchenko
  Cc: bartosz.golaszewski, driver-core, linux-kernel, devicetree, imx

From: Xu Yang <xu.yang_2@nxp.com>

Add test cases for fwnode_for_each_child_node() API.

Test command:
$ ./tools/testing/kunit/kunit.py run property-entry

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v5:
 - add patch#1 to fix "of_node_ktype undefined error"
---
 drivers/base/test/Kconfig               |   1 +
 drivers/base/test/property-entry-test.c | 136 ++++++++++++++++++++++++
 2 files changed, 137 insertions(+)

diff --git a/drivers/base/test/Kconfig b/drivers/base/test/Kconfig
index 1ecf0791241a..542ce07530a1 100644
--- a/drivers/base/test/Kconfig
+++ b/drivers/base/test/Kconfig
@@ -17,6 +17,7 @@ config DM_KUNIT_TEST
 config DRIVER_PE_KUNIT_TEST
 	tristate "KUnit Tests for property entry API" if !KUNIT_ALL_TESTS
 	depends on KUNIT
+	select OF
 	default KUNIT_ALL_TESTS
 
 config DRIVER_SWNODE_KUNIT_TEST
diff --git a/drivers/base/test/property-entry-test.c b/drivers/base/test/property-entry-test.c
index a8657eb06f94..a3d1caf28cc7 100644
--- a/drivers/base/test/property-entry-test.c
+++ b/drivers/base/test/property-entry-test.c
@@ -4,6 +4,8 @@
 // Copyright 2019 Google LLC.
 
 #include <kunit/test.h>
+
+#include <linux/of.h>
 #include <linux/property.h>
 #include <linux/types.h>
 
@@ -489,6 +491,139 @@ static void pe_test_reference(struct kunit *test)
 	software_node_unregister_node_group(group);
 }
 
+static struct fwnode_handle *create_device_node(struct kunit *test,
+						const char *name,
+						const char *full_name,
+						struct device_node *parent)
+{
+	struct device_node *node;
+
+	node = kunit_kzalloc(test, sizeof(*node), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, node);
+
+	node->name = kunit_kstrdup(test, name, GFP_KERNEL);
+	node->full_name = kunit_kstrdup(test, full_name, GFP_KERNEL);
+
+	if (parent) {
+		node->sibling = parent->child;
+		/* set the node as the first child of the parent */
+		parent->child = node;
+		node->parent = parent;
+	}
+
+	of_node_init(node);
+	return of_fwnode_handle(node);
+}
+
+/* Verifies that fwnode_for_each_child_node() can output correct children */
+static void pe_test_child_iteration(struct kunit *test)
+{
+	struct fwnode_handle *of_node, *of_node1;
+	struct fwnode_handle *sw_node, *sw_node1;
+	struct fwnode_handle *child;
+	int error, i, num;
+
+	static const struct software_node node = { .name = "sw" };
+	static const struct software_node node1 = { .name = "sw-1", .parent = &node};
+	static const struct software_node node2 = { .name = "sw-2", .parent = &node};
+	static const struct software_node node3 = { .name = "sw-3", .parent = &node};
+	static const struct software_node *group[] = { &node, &node1, &node2, &node3, NULL };
+
+	static const char * const of_child_array[] = { "of-1", "of-2", "of-3" };
+	static const char * const sw_child_array[] = { "sw-1", "sw-2", "sw-3" };
+	static const char * const of_sw_child_array[] = { "of-1", "of-2", "of-3",
+							  "sw-1", "sw-2", "sw-3" };
+	static const char * const sw_of_child_array[] = { "sw-1", "sw-2", "sw-3",
+							  "of-1", "of-2", "of-3" };
+
+	/* 1. Test OF node child iteration */
+
+	of_node = create_device_node(test, "of", "of", NULL);
+	create_device_node(test, "of", "of-3", to_of_node(of_node));
+	create_device_node(test, "of", "of-2", to_of_node(of_node));
+	of_node1 = create_device_node(test, "of", "of-1", to_of_node(of_node));
+
+	i = 0;
+	num = ARRAY_SIZE(of_child_array);
+	fwnode_for_each_child_node(of_node, child) {
+		KUNIT_ASSERT_LT(test, i, num);
+		KUNIT_EXPECT_STREQ(test, of_child_array[i++], fwnode_get_name(child));
+	}
+	KUNIT_EXPECT_PTR_EQ(test, child, NULL);
+
+	/* 2. Test SW node child iteration */
+
+	error = software_node_register_node_group(group);
+	KUNIT_ASSERT_EQ(test, error, 0);
+
+	sw_node = software_node_fwnode(&node);
+
+	i = 0;
+	num = ARRAY_SIZE(sw_child_array);
+	fwnode_for_each_child_node(sw_node, child) {
+		KUNIT_ASSERT_LT(test, i, num);
+		KUNIT_EXPECT_STREQ(test, sw_child_array[i++], fwnode_get_name(child));
+	}
+	KUNIT_EXPECT_PTR_EQ(test, child, NULL);
+
+	/* 3. Test OF (primary) + SW (secondary) node child iteration */
+
+	of_node->secondary = sw_node;
+	sw_node->secondary = ERR_PTR(-ENODEV);
+
+	i = 0;
+	num = ARRAY_SIZE(of_sw_child_array);
+	fwnode_for_each_child_node(of_node, child) {
+		KUNIT_ASSERT_LT(test, i, num);
+		KUNIT_EXPECT_STREQ(test, of_sw_child_array[i++], fwnode_get_name(child));
+	}
+	KUNIT_EXPECT_PTR_EQ(test, child, NULL);
+
+	/* 4. Test SW (primary) + OF (secondary) node child iteration */
+
+	sw_node->secondary = of_node;
+	of_node->secondary = ERR_PTR(-ENODEV);
+
+	i = 0;
+	num = ARRAY_SIZE(sw_of_child_array);
+	fwnode_for_each_child_node(sw_node, child) {
+		KUNIT_ASSERT_LT(test, i, num);
+		KUNIT_EXPECT_STREQ(test, sw_of_child_array[i++], fwnode_get_name(child));
+	}
+	KUNIT_EXPECT_PTR_EQ(test, child, NULL);
+
+	/* 5. Test OF (primary) + SW (secondary, but no children) node child iteration */
+
+	sw_node1 = software_node_fwnode(&node1);
+	of_node->secondary = sw_node1;
+	sw_node->secondary = ERR_PTR(-ENODEV);
+
+	i = 0;
+	num = ARRAY_SIZE(of_child_array);
+	fwnode_for_each_child_node(of_node, child) {
+		KUNIT_ASSERT_LT(test, i, num);
+		KUNIT_EXPECT_STREQ(test, of_child_array[i++], fwnode_get_name(child));
+	}
+	KUNIT_EXPECT_PTR_EQ(test, child, NULL);
+
+	/* 6. Test SW (primary) + OF (secondary, but no children) node child iteration */
+
+	sw_node->secondary = of_node1;
+	of_node->secondary = ERR_PTR(-ENODEV);
+
+	i = 0;
+	num = ARRAY_SIZE(sw_child_array);
+	fwnode_for_each_child_node(sw_node, child) {
+		KUNIT_ASSERT_LT(test, i, num);
+		KUNIT_EXPECT_STREQ(test, sw_child_array[i++], fwnode_get_name(child));
+	}
+	KUNIT_EXPECT_PTR_EQ(test, child, NULL);
+
+	of_node->secondary = NULL;
+	sw_node->secondary = NULL;
+	software_node_unregister_node_group(group);
+}
+
 static struct kunit_case property_entry_test_cases[] = {
 	KUNIT_CASE(pe_test_uints),
 	KUNIT_CASE(pe_test_uint_arrays),
@@ -497,6 +632,7 @@ static struct kunit_case property_entry_test_cases[] = {
 	KUNIT_CASE(pe_test_move_inline_u8),
 	KUNIT_CASE(pe_test_move_inline_str),
 	KUNIT_CASE(pe_test_reference),
+	KUNIT_CASE(pe_test_child_iteration),
 	{ }
 };
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v5 2/2] device property: add test cases for fwnode_for_each_child_node()
  2026-07-20 10:47 ` [PATCH v5 2/2] device property: add test cases for fwnode_for_each_child_node() Xu Yang
@ 2026-07-20 11:02   ` sashiko-bot
  2026-07-20 11:21   ` Andy Shevchenko
  1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-07-20 11:02 UTC (permalink / raw)
  To: Xu Yang; +Cc: Frank.Li, robh, imx, devicetree, conor+dt

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The KUnit test fails to use `kunit_add_action()` for global state cleanup, permanently leaking software nodes on test aborts.

Pre-existing issues:
- [High] The newly added tests expose pre-existing infinite loop and double-put bugs in `fwnode_get_next_child_node()` when transitioning between primary OF nodes and secondary SW nodes.
--

--- Patch [2]: [PATCH v5 2/2] device property: add test cases for fwnode_for_each_child_node() ---
commit 6f128f7ffab2a2787ad6820b967d94a17fcbcbc2
Author: Xu Yang <xu.yang_2@nxp.com>
device property: add test cases for fwnode_for_each_child_node()

Add test cases for fwnode_for_each_child_node() API.

> diff --git a/drivers/base/test/property-entry-test.c b/drivers/base/test/property-entry-test.c
> index a8657eb06f94e..a3d1caf28cc7d 100644
> --- a/drivers/base/test/property-entry-test.c
> +++ b/drivers/base/test/property-entry-test.c
[ ... ]
> @@ -489,6 +491,139 @@ static void pe_test_reference(struct kunit *test)
[ ... ]
> +	/* 2. Test SW node child iteration */
> +
> +	error = software_node_register_node_group(group);
> +	KUNIT_ASSERT_EQ(test, error, 0);
> +
> +	sw_node = software_node_fwnode(&node);
> +
> +	i = 0;
> +	num = ARRAY_SIZE(sw_child_array);
> +	fwnode_for_each_child_node(sw_node, child) {
> +		KUNIT_ASSERT_LT(test, i, num);

[Severity: Medium]
If KUNIT_ASSERT_LT() fails, it will immediately abort the test execution.
Does this bypass the manual software_node_unregister_node_group() call at
the end of the function?

To prevent leaking the software nodes and causing sysfs conflicts on
subsequent test runs, should kunit_add_action() be used to register the
cleanup immediately after registration?

> +		KUNIT_EXPECT_STREQ(test, sw_child_array[i++], fwnode_get_name(child));
> +	}
> +	KUNIT_EXPECT_PTR_EQ(test, child, NULL);
> +
> +	/* 3. Test OF (primary) + SW (secondary) node child iteration */
> +
> +	of_node->secondary = sw_node;
> +	sw_node->secondary = ERR_PTR(-ENODEV);
> +
> +	i = 0;
> +	num = ARRAY_SIZE(of_sw_child_array);
> +	fwnode_for_each_child_node(of_node, child) {
> +		KUNIT_ASSERT_LT(test, i, num);

[Severity: High]
This isn't a bug introduced by this patch, but these new tests appear to
expose a pre-existing infinite loop and double-put bug in the core fwnode
iterator.

When fwnode_for_each_child_node() transitions from primary OF children to
secondary SW children, of_fwnode_get_next_child_node() drops the reference
on the last OF child and returns NULL. The core iterator then passes this
same child pointer to the secondary node's iterator:

drivers/base/property.c:fwnode_get_next_child_node() {
    ...
    next = fwnode_call_ptr_op(fwnode, get_next_child_node, child);
    if (next)
        return next;
    return fwnode_call_ptr_op(fwnode->secondary, get_next_child_node, child);
}

Does this cause the secondary iterator (software_node_get_next_child) to
unconditionally drop the reference a second time?

Furthermore, when iterating over the secondary children, the returned SW node
is eventually passed back to the primary iterator. Since to_of_node()
evaluates to NULL for SW nodes, of_get_next_available_child() restarts from
the first OF child:

drivers/of/property.c:of_fwnode_get_next_child_node() {
    ...
    return of_fwnode_handle(of_get_next_available_child(to_of_node(fwnode),
                                                        to_of_node(child)));
}

Will this restart cause an infinite loop in the test runner when testing
mixed primary and secondary firmware nodes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720104730.1285552-1-xu.yang_2@oss.nxp.com?part=2

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5 1/2] of: kobj: export of_node_ktype for use by modules
  2026-07-20 10:47 [PATCH v5 1/2] of: kobj: export of_node_ktype for use by modules Xu Yang
  2026-07-20 10:47 ` [PATCH v5 2/2] device property: add test cases for fwnode_for_each_child_node() Xu Yang
@ 2026-07-20 11:13 ` Andy Shevchenko
  1 sibling, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2026-07-20 11:13 UTC (permalink / raw)
  To: Xu Yang
  Cc: gregkh, rafael, dakr, robh, saravanak, bartosz.golaszewski,
	driver-core, linux-kernel, devicetree, imx

On Mon, Jul 20, 2026 at 06:47:29PM +0800, Xu Yang wrote:

> of_node_init() is a static inline that references of_node_ktype when
> CONFIG_OF_KOBJ=y. Any module that calls of_node_init() will therefore
> have an unresolved reference to of_node_ktype at load time, because
> the symbol is defined in drivers/of/kobj.c but was never exported.
> 
> This causes a modpost build error when CONFIG_OF_KOBJ=y and
> CONFIG_DRIVER_PE_KUNIT_TEST=m:
> 
>   ERROR: modpost: "of_node_ktype"
>   [drivers/base/test/property-entry-test.ko] undefined!
> 
> Add EXPORT_SYMBOL_GPL(of_node_ktype) so that GPL-licensed modules
> such as the KUnit property-entry test can call of_node_init() without
> hitting this linker error.

Use special one for KUnits: EXPORT_SYMBOL_IF_KUNIT().

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5 2/2] device property: add test cases for fwnode_for_each_child_node()
  2026-07-20 10:47 ` [PATCH v5 2/2] device property: add test cases for fwnode_for_each_child_node() Xu Yang
  2026-07-20 11:02   ` sashiko-bot
@ 2026-07-20 11:21   ` Andy Shevchenko
  1 sibling, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2026-07-20 11:21 UTC (permalink / raw)
  To: Xu Yang
  Cc: gregkh, rafael, dakr, robh, saravanak, bartosz.golaszewski,
	driver-core, linux-kernel, devicetree, imx

On Mon, Jul 20, 2026 at 06:47:30PM +0800, Xu Yang wrote:

> Add test cases for fwnode_for_each_child_node() API.

> Test command:
> $ ./tools/testing/kunit/kunit.py run property-entry


> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Nope, I haven't Signed-off it here, it's your new submission where you are
the author. I only made a few cosmetic changes, if you want to give a credit
you can mention that in the comment block (SoB+Co-developed-by here seems
like an overkill).

...

You have a series of two patches but I haven't seen the cover letter.
I use this script [1] that helps me not to forget this.

[1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-20 11:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-20 10:47 [PATCH v5 1/2] of: kobj: export of_node_ktype for use by modules Xu Yang
2026-07-20 10:47 ` [PATCH v5 2/2] device property: add test cases for fwnode_for_each_child_node() Xu Yang
2026-07-20 11:02   ` sashiko-bot
2026-07-20 11:21   ` Andy Shevchenko
2026-07-20 11:13 ` [PATCH v5 1/2] of: kobj: export of_node_ktype for use by modules Andy Shevchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.