Skip to contentSkip to conversation

clk: Add kunit tests for fixed rate and parent data

PATCH Superseded Stephen Boyd 4 participants 23 messages Mar 15, 2023 – Mar 21, 2023

Subsystems (6)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Stephen Boyd

author maintainer
#0 lore raw

v2 cover clk: Add kunit tests for fixed rate and parent data

This patch series adds unit tests for the clk fixed rate basic type and the clk registration functions that use struct clk_parent_data. To get there, we add support for loading device tree overlays onto the live DTB along with probing platform drivers to bind to device nodes in the overlays. With this series, we're able to exercise some of the code in the common clk framework that uses devicetree lookups to find parents and the fixed rate clk code that scans device tree directly and creates clks. Please review.

I Cced everyone to all the patches so they get the full context. I'm hoping I can take the whole pile through the clk tree as they almost all depend on each other.

Changes from v1 (https://lore.kernel.org/r/[email protected]):
* Don't depend on UML, use unittest data approach to attach nodes
* Introduce overlay loading API for KUnit
* Move platform_device KUnit code to drivers/base/test
* Use #define macros for constants shared between unit tests and overlays
* Settle on "test" as a vendor prefix
* Make KUnit wrappers have "_kunit" postfix

Stephen Boyd (11):
of: Load KUnit DTB from of_core_init()
of: Add test managed wrappers for of_overlay_apply()/of_node_put()
dt-bindings: vendor-prefixes: Add "test" vendor for KUnit and friends
dt-bindings: test: Add KUnit empty node binding
of: Add a KUnit test for overlays and test managed APIs
platform: Add test managed platform_device/driver APIs
dt-bindings: kunit: Add fixed rate clk consumer test
clk: Add test managed clk provider/consumer APIs
clk: Add KUnit tests for clk fixed rate basic type
dt-bindings: clk: Add KUnit clk_parent_data test
clk: Add KUnit tests for clks registered with struct clk_parent_data

 .../clock/test,clk-kunit-parent-data.yaml     |  47 ++
 .../kunit/test,clk-kunit-fixed-rate.yaml      |  35 ++
 .../bindings/test/test,kunit-empty.yaml       |  30 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 drivers/base/test/Makefile                    |   3 +
 drivers/base/test/platform_kunit-test.c       | 108 +++++
 drivers/base/test/platform_kunit.c            | 186 +++++++
 drivers/clk/.kunitconfig                      |   3 +
 drivers/clk/Kconfig                           |   7 +
 drivers/clk/Makefile                          |   9 +-
 drivers/clk/clk-fixed-rate_test.c             | 299 ++++++++++++
 drivers/clk/clk-fixed-rate_test.h             |   8 +
 drivers/clk/clk_kunit.c                       | 219 +++++++++
 drivers/clk/clk_parent_data_test.h            |  10 +
 drivers/clk/clk_test.c                        | 459 +++++++++++++++++-
 drivers/clk/kunit_clk_fixed_rate_test.dtso    |  19 +
 drivers/clk/kunit_clk_parent_data_test.dtso   |  28 ++
 drivers/of/.kunitconfig                       |   5 +
 drivers/of/Kconfig                            |  23 +
 drivers/of/Makefile                           |   7 +
 drivers/of/base.c                             | 182 +++++++
 drivers/of/kunit.dtso                         |  10 +
 drivers/of/kunit_overlay_test.dtso            |   9 +
 drivers/of/of_kunit.c                         | 123 +++++
 drivers/of/of_private.h                       |   6 +
 drivers/of/of_test.c                          |  43 ++
 drivers/of/overlay_test.c                     | 107 ++++
 drivers/of/unittest.c                         | 101 +---
 include/kunit/clk.h                           |  28 ++
 include/kunit/of.h                            |  90 ++++
 include/kunit/platform_device.h               |  15 +
 31 files changed, 2119 insertions(+), 102 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/test,clk-kunit-parent-data.yaml
 create mode 100644 Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml
 create mode 100644 Documentation/devicetree/bindings/test/test,kunit-empty.yaml
 create mode 100644 drivers/base/test/platform_kunit-test.c
 create mode 100644 drivers/base/test/platform_kunit.c
 create mode 100644 drivers/clk/clk-fixed-rate_test.c
 create mode 100644 drivers/clk/clk-fixed-rate_test.h
 create mode 100644 drivers/clk/clk_kunit.c
 create mode 100644 drivers/clk/clk_parent_data_test.h
 create mode 100644 drivers/clk/kunit_clk_fixed_rate_test.dtso
 create mode 100644 drivers/clk/kunit_clk_parent_data_test.dtso
 create mode 100644 drivers/of/.kunitconfig
 create mode 100644 drivers/of/kunit.dtso
 create mode 100644 drivers/of/kunit_overlay_test.dtso
 create mode 100644 drivers/of/of_kunit.c
 create mode 100644 drivers/of/of_test.c
 create mode 100644 drivers/of/overlay_test.c
 create mode 100644 include/kunit/clk.h
 create mode 100644 include/kunit/of.h
 create mode 100644 include/kunit/platform_device.h

base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6

Signature · 2 lines
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
author maintainer
#1 lore raw

PATCH v2 1/11 of: Load KUnit DTB from of_core_init()

Load a small DTB for KUnit from of_core_init() as long as CONFIG_OF=y and CONFIG_KUNIT=y/m. This allows KUnit tests to load overlays into the running system. It also allows KUnit tests to run on any architecture that supports it so that devicetree can be used while unit testing architecture specific code.

Overlays need a target node to apply their overlays to, so make a fake bus called 'kunit-bus' in the root node to allow this. Make the node a simple-bus so that platform devices are automatically created for nodes added as children of this node. Unit test overlays can target this node via the label 'kunit_bus'.

Cc Rob Herring <[email protected]>
Cc Frank Rowand <[email protected]>
---
 drivers/of/.kunitconfig |   3 +
 drivers/of/Kconfig      |  13 +++
 drivers/of/Makefile     |   4 +
 drivers/of/base.c       | 182 ++++++++++++++++++++++++++++++++++++++++
 drivers/of/kunit.dtso   |  10 +++
 drivers/of/of_private.h |   6 ++
 drivers/of/of_test.c    |  43 ++++++++++
 drivers/of/unittest.c   | 101 +---------------------
 8 files changed, 262 insertions(+), 100 deletions(-)
 create mode 100644 drivers/of/.kunitconfig
 create mode 100644 drivers/of/kunit.dtso
 create mode 100644 drivers/of/of_test.c
drivers/of/.kunitconfig new file +3 -0
new file mode 100644
index 000000000000..5a8fee11978c
@@ -0,0 +1,3 @@
+CONFIG_KUNIT=y
+CONFIG_OF=y
+CONFIG_OF_KUNIT_TEST=y
drivers/of/Kconfig +13 -0
index 644386833a7b..f6739b9560c5 100644
@@ -37,6 +37,19 @@ config OF_UNITTEST
 
 	  If unsure, say N here. This option is not safe to enable.
 
+config OF_KUNIT
+	def_bool KUNIT
+	select OF_RESOLVE
+
+config OF_KUNIT_TEST
+	tristate "Devicetree KUnit DTB Test" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  This option builds KUnit unit tests for device tree infrastructure.
+
+	  If unsure, say N here, but this option is safe to enable.
+
 config OF_ALL_DTBS
 	bool "Build all Device Tree Blobs"
 	depends on COMPILE_TEST
drivers/of/Makefile +4 -0
index e0360a44306e..cf6ee7ba6350 100644
@@ -19,4 +19,8 @@ obj-y	+= kexec.o
 endif
 endif
 
+DTC_FLAGS_kunit += -@
+obj-$(CONFIG_OF_KUNIT) += kunit.dtbo.o
+obj-$(CONFIG_OF_KUNIT_TEST) += of_test.o
+
 obj-$(CONFIG_OF_UNITTEST) += unittest-data/
drivers/of/base.c +182 -0

211 lines not on this page · raw message

drivers/of/kunit.dtso new file +10 -0
new file mode 100644
index 000000000000..d512057df98d
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+/ {
+	/* Container node where KUnit tests can load overlays */
+	kunit_bus: kunit-bus {
+		compatible = "simple-bus";
+	};
+};
drivers/of/of_private.h +6 -0
index fb6792d381a6..2151a28ca234 100644
@@ -96,6 +96,12 @@ static inline void of_overlay_mutex_lock(void) {};
 static inline void of_overlay_mutex_unlock(void) {};
 #endif
 
+#if defined(CONFIG_OF_UNITTEST) || defined (CONFIG_KUNIT)
+void __init of_test_attach_node_and_children(struct device_node *np);
+#else
+static inline void __init of_test_attach_node_and_children(struct device_node *np) {}
+#endif
+
 #if defined(CONFIG_OF_UNITTEST) && defined(CONFIG_OF_OVERLAY)
 extern void __init unittest_unflatten_overlay_base(void);
 #else
drivers/of/of_test.c new file +43 -0

43 lines not on this page · raw message

drivers/of/unittest.c +1 -100

113 lines not on this page · raw message

Signature · 2 lines
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
maintainer
#2 lore raw

Re: [PATCH v2 01/11] of: Load KUnit DTB from of_core_init()

Quoted 81 lines · Stephen Boyd

Load a small DTB for KUnit from of_core_init() as long as CONFIG_OF=y and CONFIG_KUNIT=y/m. This allows KUnit tests to load overlays into the running system. It also allows KUnit tests to run on any architecture that supports it so that devicetree can be used while unit testing architecture specific code.

Overlays need a target node to apply their overlays to, so make a fake bus called 'kunit-bus' in the root node to allow this. Make the node a simple-bus so that platform devices are automatically created for nodes added as children of this node. Unit test overlays can target this node via the label 'kunit_bus'.

Cc Rob Herring <[email protected]>
Cc Frank Rowand <[email protected]>
---
 drivers/of/.kunitconfig |   3 +
 drivers/of/Kconfig      |  13 +++
 drivers/of/Makefile     |   4 +
 drivers/of/base.c       | 182 ++++++++++++++++++++++++++++++++++++++++
 drivers/of/kunit.dtso   |  10 +++
 drivers/of/of_private.h |   6 ++
 drivers/of/of_test.c    |  43 ++++++++++
 drivers/of/unittest.c   | 101 +---------------------
 8 files changed, 262 insertions(+), 100 deletions(-)
 create mode 100644 drivers/of/.kunitconfig
 create mode 100644 drivers/of/kunit.dtso
 create mode 100644 drivers/of/of_test.c
drivers/of/.kunitconfig new file +3 -0
new file mode 100644
index 000000000000..5a8fee11978c
@@ -0,0 +1,3 @@
+CONFIG_KUNIT=y
+CONFIG_OF=y
+CONFIG_OF_KUNIT_TEST=y
drivers/of/Kconfig +13 -0
index 644386833a7b..f6739b9560c5 100644
@@ -37,6 +37,19 @@ config OF_UNITTEST
 
 	  If unsure, say N here. This option is not safe to enable.
 
+config OF_KUNIT
+	def_bool KUNIT
+	select OF_RESOLVE
+
+config OF_KUNIT_TEST
+	tristate "Devicetree KUnit DTB Test" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  This option builds KUnit unit tests for device tree infrastructure.
+
+	  If unsure, say N here, but this option is safe to enable.
+
 config OF_ALL_DTBS
 	bool "Build all Device Tree Blobs"
 	depends on COMPILE_TEST
drivers/of/Makefile +4 -0
index e0360a44306e..cf6ee7ba6350 100644
@@ -19,4 +19,8 @@ obj-y	+= kexec.o
 endif
 endif
 
+DTC_FLAGS_kunit += -@
+obj-$(CONFIG_OF_KUNIT) += kunit.dtbo.o
+obj-$(CONFIG_OF_KUNIT_TEST) += of_test.o
+
 obj-$(CONFIG_OF_UNITTEST) += unittest-data/
drivers/of/base.c +0 -0
index ac6fde53342f..090c5d7925e4 100644
@@ -16,13 +16,16 @@
 

drivers/of/base.c line 26

Quoted 10 lines
drivers/of/base.c +2 -0
 #define pr_fmt(fmt)	"OF: " fmt
 
+#include <linux/align.h>
 #include <linux/console.h>
 #include <linux/ctype.h>
 #include <linux/cpu.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_fdt.h>

base.c deals with unflattened trees. There shouldn't be anything FDT related in it.

drivers/of/base.c void __of_phandle_cache_inv_entry(phandle handle) · line 169

Quoted 10 lines
drivers/of/base.c +2 -0
 #include <linux/of_graph.h>
+#include <linux/printk.h>
 #include <linux/spinlock.h>
 #include <linux/slab.h>
 #include <linux/string.h>
@@ -163,10 +166,90 @@ void __of_phandle_cache_inv_entry(phandle handle)
 		phandle_cache[handle_hash] = NULL;
 }
 
+#ifdef CONFIG_OF_KUNIT

base.c is already quite big. This should probably be its own file. Perhaps in kunit code because that's what we do for everything else (e.g. DT clock code goes in drivers/clk/). (My goal is to eliminate drivers/of/. That's easier than finding maintainers. ;) )

Quoted 18 lines
drivers/of/base.c +18 -0
+static int __init of_kunit_add_data(void)
+{
+	void *kunit_fdt;
+	void *kunit_fdt_align;
+	struct device_node *kunit_node = NULL, *np;
+	/*
+	 * __dtbo_kunit_begin[] and __dtbo_kunit_end[] are magically
+	 * created by cmd_dt_S_dtbo in scripts/Makefile.lib
+	 */
+	extern uint8_t __dtbo_kunit_begin[];
+	extern uint8_t __dtbo_kunit_end[];
+	const int size = __dtbo_kunit_end - __dtbo_kunit_begin;
+	int rc;
+	void *ret;
+
+	if (!size) {
+		pr_warn("kunit.dtbo is empty\n");
+		return -ENODATA;

drivers/of/base.c void __of_phandle_cache_inv_entry(phandle handle) · line 197

Quoted 10 lines
drivers/of/base.c +10 -0
+	}
+
+	kunit_fdt = kmalloc(size + FDT_ALIGN_SIZE, GFP_KERNEL);
+	if (!kunit_fdt)
+		return -ENOMEM;
+
+	kunit_fdt_align = PTR_ALIGN(kunit_fdt, FDT_ALIGN_SIZE);
+	memcpy(kunit_fdt_align, __dtbo_kunit_begin, size);
+
+	ret = of_fdt_unflatten_tree(kunit_fdt_align, NULL, &kunit_node);

I don't understand why this doesn't use of_overlay_fdt_apply(). Your test(s) shouldn't be any different than any other overlay user (granted, there aren't many). You apply the overlay, run your test, then remove the overlay.

Quoted 10 lines
drivers/of/base.c +10 -0
+	if (!ret) {
+		pr_warn("unflatten KUnit tree failed\n");
+		kfree(kunit_fdt);
+		return -ENODATA;
+	}
+	if (!kunit_node) {
+		pr_warn("KUnit tree is empty\n");
+		kfree(kunit_fdt);
+		return -ENODATA;
+	}

drivers/of/base.c void __of_phandle_cache_inv_entry(phandle handle) · line 217

Quoted 10 lines
drivers/of/base.c +10 -0
+
+	of_overlay_mutex_lock();
+	rc = of_resolve_phandles(kunit_node);
+	if (rc) {
+		pr_err("Failed to resolve KUnit phandles (rc=%i)\n", rc);
+		of_overlay_mutex_unlock();
+		return -EINVAL;
+	}
+
+	if (!of_root) {

There's patches from Frank and others under review which will always create and empty DT if the bootloader/arch didn't provide one. This series should rely on that. (Or just assume that when that happens, your tests will run in more environments)

Quoted 42 lines
drivers/of/base.c +31 -0
+		of_root = kunit_node;
+		of_chosen = of_find_node_by_path("/chosen");
+	} else {
+		/* attach the sub-tree to live tree */
+		np = kunit_node->child;
+		while (np) {
+			struct device_node *next = np->sibling;
+
+			np->parent = of_root;
+			of_test_attach_node_and_children(np);
+			np = next;
+		}
+	}
+
+	if (!of_aliases)
+		of_aliases = of_find_node_by_path("/aliases");
+
+	of_overlay_mutex_unlock();
+
+	return 0;
+}
+#else
+static inline int __init of_kunit_add_data(void) { return 0; }
+#endif
+
 void __init of_core_init(void)
 {
 	struct device_node *np;
+	int ret;
 
+	ret = of_kunit_add_data();
+	if (ret) {
+		pr_err("failed to add kunit test data\n");
+		return;
+	}
 
 	/* Create the kset, and register existing nodes */
 	mutex_lock(&of_mutex);
@@ -1879,6 +1962,105 @@ int of_update_property(struct device_node *np, struct property *newprop)
 	return rc;
 }
 

drivers/of/base.c int of_update_property(struct device_node *np, struct property *newprop) · line 1974

Quoted 10 lines
drivers/of/base.c +10 -0
+#if defined(CONFIG_OF_UNITTEST) || defined (CONFIG_KUNIT)
+/**
+ * update_node_properties - adds the properties of np into dup node (present in
+ * live tree) and updates parent of children of np to dup.
+ *
+ * @np: node whose properties are being added to the live tree
+ * @dup: node present in live tree to be updated
+ */
+static void __init update_node_properties(struct device_node *np,
+					struct device_node *dup)

Please split any moving of code to separate patches.

I'm not remembering why we need these test functions vs. just applying overlays. Frank? Perhaps because it's trying to test the overlay code itself. But you should just be a user of the overlay API and not need to do anything special.

Quoted 98 lines
drivers/of/base.c +89 -0
+{
+	struct property *prop;
+	struct property *save_next;
+	struct device_node *child;
+	int ret;
+
+	for_each_child_of_node(np, child)
+		child->parent = dup;
+
+	/*
+	 * "unittest internal error: unable to add testdata property"
+	 *
+	 *    If this message reports a property in node '/__symbols__' then
+	 *    the respective unittest overlay contains a label that has the
+	 *    same name as a label in the live devicetree.  The label will
+	 *    be in the live devicetree only if the devicetree source was
+	 *    compiled with the '-@' option.  If you encounter this error,
+	 *    please consider renaming __all__ of the labels in the unittest
+	 *    overlay dts files with an odd prefix that is unlikely to be
+	 *    used in a real devicetree.
+	 */
+
+	/*
+	 * open code for_each_property_of_node() because of_add_property()
+	 * sets prop->next to NULL
+	 */
+	for (prop = np->properties; prop != NULL; prop = save_next) {
+		save_next = prop->next;
+		ret = of_add_property(dup, prop);
+		if (ret) {
+			if (ret == -EEXIST && !strcmp(prop->name, "name"))
+				continue;
+			pr_err("unittest internal error: unable to add testdata property %pOF/%s",
+			       np, prop->name);
+		}
+	}
+}
+
+/**
+ * of_test_attach_node_and_children - attaches nodes and its children to live tree.
+ * @np:	Node to attach to live tree
+ *
+ * CAUTION: misleading function name - if node @np already exists in
+ * the live tree then children of @np are *not* attached to the live
+ * tree.  This works for the current test devicetree nodes because such
+ * nodes do not have child nodes.
+ */
+void __init of_test_attach_node_and_children(struct device_node *np)
+{
+	struct device_node *next, *dup, *child;
+	unsigned long flags;
+	const char *full_name;
+
+	full_name = kasprintf(GFP_KERNEL, "%pOF", np);
+
+	if (!strcmp(full_name, "/__local_fixups__") ||
+	    !strcmp(full_name, "/__fixups__")) {
+		kfree(full_name);
+		return;
+	}
+
+	dup = of_find_node_by_path(full_name);
+	kfree(full_name);
+	if (dup) {
+		update_node_properties(np, dup);
+		return;
+	}
+
+	child = np->child;
+	np->child = NULL;
+
+	mutex_lock(&of_mutex);
+	raw_spin_lock_irqsave(&devtree_lock, flags);
+	np->sibling = np->parent->child;
+	np->parent->child = np;
+	of_node_clear_flag(np, OF_DETACHED);
+	raw_spin_unlock_irqrestore(&devtree_lock, flags);
+
+	__of_attach_node_sysfs(np);
+	mutex_unlock(&of_mutex);
+
+	while (child) {
+		next = child->sibling;
+		of_test_attach_node_and_children(child);
+		child = next;
+	}
+}
+#endif
+
 static void of_alias_add(struct alias_prop *ap, struct device_node *np,
 			 int id, const char *stem, int stem_len)
 {
drivers/of/kunit.dtso new file +0 -0
new file mode 100644
index 000000000000..d512057df98d

drivers/of/kunit.dtso line 10

Quoted 10 lines
drivers/of/kunit.dtso new file +10 -0
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+/ {
+	/* Container node where KUnit tests can load overlays */
+	kunit_bus: kunit-bus {
+		compatible = "simple-bus";
+	};
+};

Why do we need an overlay to apply overlays to?

Quoted 37 lines
drivers/of/of_private.h +6 -0
index fb6792d381a6..2151a28ca234 100644
@@ -96,6 +96,12 @@ static inline void of_overlay_mutex_lock(void) {};
 static inline void of_overlay_mutex_unlock(void) {};
 #endif
 
+#if defined(CONFIG_OF_UNITTEST) || defined (CONFIG_KUNIT)
+void __init of_test_attach_node_and_children(struct device_node *np);
+#else
+static inline void __init of_test_attach_node_and_children(struct device_node *np) {}
+#endif
+
 #if defined(CONFIG_OF_UNITTEST) && defined(CONFIG_OF_OVERLAY)
 extern void __init unittest_unflatten_overlay_base(void);
 #else
drivers/of/of_test.c new file +14 -0
new file mode 100644
index 000000000000..a4d70ac344ad
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for OF APIs
+ */
+#include <linux/kconfig.h>
+#include <linux/of.h>
+
+#include <kunit/test.h>
+
+/*
+ * Test that the root node / exists.
+ */
+static void dtb_root_node_exists(struct kunit *test)
+{

drivers/of/of_test.c line 24

Quoted 10 lines
drivers/of/of_test.c new file +10 -0
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, of_find_node_by_path("/"));
+}
+
+/*
+ * Test that the /__symbols__ node exists.
+ */
+static void dtb_symbols_node_exists(struct kunit *test)
+{
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, of_find_node_by_path("/__symbols__"));
+}

Many base DTs will not have this. And the kunit tests themselves shouldn't need it because they should be independent of the base tree.

Rob

author maintainer
#3 lore raw

Re: [PATCH v2 01/11] of: Load KUnit DTB from of_core_init()

Quoting Rob Herring (2023-03-21 10:33:03)

Quoted 9 lines · Stephen Boyd
Quoted 8 lines
drivers/of/base.c +0 -0
index ac6fde53342f..090c5d7925e4 100644
@@ -16,13 +16,16 @@
 
 #define pr_fmt(fmt)  "OF: " fmt
 
Quoted 11 lines

drivers/of/base.c line 26

Quoted 8 lines
drivers/of/base.c +2 -0
+#include <linux/align.h>
 #include <linux/console.h>
 #include <linux/ctype.h>
 #include <linux/cpu.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_fdt.h>

base.c deals with unflattened trees. There shouldn't be anything FDT related in it.

Ok.

Quoted 10 lines
Quoted 10 lines
drivers/of/base.c +1 -0
 #include <linux/of_graph.h>
+#include <linux/printk.h>
 #include <linux/spinlock.h>
 #include <linux/slab.h>
 #include <linux/string.h>
@@ -163,10 +166,90 @@ void __of_phandle_cache_inv_entry(phandle handle)
              phandle_cache[handle_hash] = NULL;
 }
 
Quoted 6 lines

drivers/of/base.c void __of_phandle_cache_inv_entry(phandle handle) · line 169

Quoted 1 line
drivers/of/base.c +1 -0
+#ifdef CONFIG_OF_KUNIT

base.c is already quite big. This should probably be its own file. Perhaps in kunit code because that's what we do for everything else (e.g. DT clock code goes in drivers/clk/). (My goal is to eliminate drivers/of/. That's easier than finding maintainers. ;) )

Heh, sure.

Quoted 26 lines
Quoted 26 lines
drivers/of/base.c +25 -0
+static int __init of_kunit_add_data(void)
+{
+     void *kunit_fdt;
+     void *kunit_fdt_align;
+     struct device_node *kunit_node = NULL, *np;
+     /*
+      * __dtbo_kunit_begin[] and __dtbo_kunit_end[] are magically
+      * created by cmd_dt_S_dtbo in scripts/Makefile.lib
+      */
+     extern uint8_t __dtbo_kunit_begin[];
+     extern uint8_t __dtbo_kunit_end[];
+     const int size = __dtbo_kunit_end - __dtbo_kunit_begin;
+     int rc;
+     void *ret;
+
+     if (!size) {
+             pr_warn("kunit.dtbo is empty\n");
+             return -ENODATA;
+     }
+
+     kunit_fdt = kmalloc(size + FDT_ALIGN_SIZE, GFP_KERNEL);
+     if (!kunit_fdt)
+             return -ENOMEM;
+
+     kunit_fdt_align = PTR_ALIGN(kunit_fdt, FDT_ALIGN_SIZE);
Quoted 8 lines

drivers/of/base.c void __of_phandle_cache_inv_entry(phandle handle) · line 197

Quoted 3 lines
drivers/of/base.c +3 -0
+     memcpy(kunit_fdt_align, __dtbo_kunit_begin, size);
+
+     ret = of_fdt_unflatten_tree(kunit_fdt_align, NULL, &kunit_node);

I don't understand why this doesn't use of_overlay_fdt_apply(). Your test(s) shouldn't be any different than any other overlay user (granted, there aren't many). You apply the overlay, run your test, then remove the overlay.

This isn't a test. This is loading the kunit.dtso blob, and potentially the root DTB. Maybe of_overlay_fdt_apply() will work if there is a root node already?

Quoted 18 lines
Quoted 18 lines
drivers/of/base.c +17 -0
+     if (!ret) {
+             pr_warn("unflatten KUnit tree failed\n");
+             kfree(kunit_fdt);
+             return -ENODATA;
+     }
+     if (!kunit_node) {
+             pr_warn("KUnit tree is empty\n");
+             kfree(kunit_fdt);
+             return -ENODATA;
+     }
+
+     of_overlay_mutex_lock();
+     rc = of_resolve_phandles(kunit_node);
+     if (rc) {
+             pr_err("Failed to resolve KUnit phandles (rc=%i)\n", rc);
+             of_overlay_mutex_unlock();
+             return -EINVAL;
Quoted 8 lines

drivers/of/base.c void __of_phandle_cache_inv_entry(phandle handle) · line 217

Quoted 3 lines
drivers/of/base.c +3 -0
+     }
+
+     if (!of_root) {

There's patches from Frank and others under review which will always create and empty DT if the bootloader/arch didn't provide one. This series should rely on that. (Or just assume that when that happens, your tests will run in more environments)

Ok. I'll base v3 on the series here[1] unless told otherwise.

Quoted 4 lines
Quoted 4 lines
drivers/of/base.c +0 -0
@@ -1879,6 +1962,105 @@ int of_update_property(struct device_node *np, struct property *newprop)
      return rc;
 }
 
Quoted 12 lines

drivers/of/base.c int of_update_property(struct device_node *np, struct property *newprop) · line 1974

Quoted 10 lines
drivers/of/base.c +10 -0
+#if defined(CONFIG_OF_UNITTEST) || defined (CONFIG_KUNIT)
+/**
+ * update_node_properties - adds the properties of np into dup node (present in
+ * live tree) and updates parent of children of np to dup.
+ *
+ * @np: node whose properties are being added to the live tree
+ * @dup: node present in live tree to be updated
+ */
+static void __init update_node_properties(struct device_node *np,
+                                     struct device_node *dup)

Please split any moving of code to separate patches.

Sure.

Quoted 5 lines

I'm not remembering why we need these test functions vs. just applying overlays. Frank? Perhaps because it's trying to test the overlay code itself. But you should just be a user of the overlay API and not need to do anything special.

Got it.

Quoted 10 lines
Quoted 10 lines
drivers/of/kunit.dtso new file +4 -0
new file mode 100644
index 000000000000..d512057df98d
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
Quoted 8 lines

drivers/of/kunit.dtso line 10

Quoted 6 lines
drivers/of/kunit.dtso new file +6 -0
+/ {
+     /* Container node where KUnit tests can load overlays */
+     kunit_bus: kunit-bus {
+             compatible = "simple-bus";
+     };
+};

Why do we need an overlay to apply overlays to?

I'm applying overlays based on the phandle kunit_bus. Should we allow overlays to modify the root node? I haven't tried that, but I can give it a shot and see if it works.

Quoted 27 lines
Quoted 27 lines
drivers/of/of_test.c new file +19 -0
new file mode 100644
index 000000000000..a4d70ac344ad
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for OF APIs
+ */
+#include <linux/kconfig.h>
+#include <linux/of.h>
+
+#include <kunit/test.h>
+
+/*
+ * Test that the root node / exists.
+ */
+static void dtb_root_node_exists(struct kunit *test)
+{
+     KUNIT_EXPECT_NOT_ERR_OR_NULL(test, of_find_node_by_path("/"));
+}
+
+/*
+ * Test that the /__symbols__ node exists.
Quoted 9 lines

drivers/of/of_test.c line 24

Quoted 5 lines
drivers/of/of_test.c new file +5 -0
+ */
+static void dtb_symbols_node_exists(struct kunit *test)
+{
+     KUNIT_EXPECT_NOT_ERR_OR_NULL(test, of_find_node_by_path("/__symbols__"));
+}

Many base DTs will not have this. And the kunit tests themselves shouldn't need it because they should be independent of the base tree.

Alright. Let me see how modifying the root node works, in which case maybe none of this patch is necessary besides confirming there is a root node.

[1] https://lore.kernel.org/all/[email protected]/

author maintainer
#4 lore raw

Re: [PATCH v2 01/11] of: Load KUnit DTB from of_core_init()

In reply to Rob Herring

Quoting Rob Herring (2023-03-21 10:33:03)

Quoted 26 lines · Stephen Boyd
Quoted 25 lines
drivers/of/of_test.c new file +19 -0
new file mode 100644
index 000000000000..a4d70ac344ad
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for OF APIs
+ */
+#include <linux/kconfig.h>
+#include <linux/of.h>
+
+#include <kunit/test.h>
+
+/*
+ * Test that the root node / exists.
+ */
+static void dtb_root_node_exists(struct kunit *test)
+{
+     KUNIT_EXPECT_NOT_ERR_OR_NULL(test, of_find_node_by_path("/"));
+}
+
+/*
+ * Test that the /__symbols__ node exists.
Quoted 9 lines

drivers/of/of_test.c line 24

Quoted 5 lines
drivers/of/of_test.c new file +5 -0
+ */
+static void dtb_symbols_node_exists(struct kunit *test)
+{
+     KUNIT_EXPECT_NOT_ERR_OR_NULL(test, of_find_node_by_path("/__symbols__"));
+}

Many base DTs will not have this. And the kunit tests themselves shouldn't need it because they should be independent of the base tree.

When I try to apply an overlay it fails

OF: overlay: no fragments or symbols in overlay
OF: overlay: init_overlay_changeset() failed, ret = -22

     # of_overlay_apply_kunit_apply: ASSERTION FAILED at drivers/of/overlay_test.c:18
     Expected 0 == ({ extern uint8_t __dtbo_kunit_overlay_test_begin[]; extern uint8_t __dtbo_kunit_overlay_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_overlay_test_begin, __dtbo_kunit_overlay_test_end); }), but
         ({ extern uint8_t __dtbo_kunit_overlay_test_begin[]; extern uint8_t __dtbo_kunit_overlay_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_overlay_test_begin, __dtbo_kunit_overlay_test_end); }) == -12 (0xfffffffffffffff4)

[FAILED] of_overlay_apply_kunit_apply

Now I'm trying to hack on the fake root node to see if I can make it work.

---8<---

drivers/of/base.c +1 -1
index 090c5d7925e4..12c44c86b8ae 100644
@@ -166,7 +166,7 @@ void __of_phandle_cache_inv_entry(phandle handle)
 		phandle_cache[handle_hash] = NULL;
 }
 
-#ifdef CONFIG_OF_KUNIT
+#if 0
 static int __init of_kunit_add_data(void)
 {
 	void *kunit_fdt;
drivers/of/kunit_overlay_test.dtso +1 -1
index 6e70e2f8cd90..e3ced1467dd9 100644
@@ -2,7 +2,7 @@
 /dts-v1/;
 /plugin/;
 
-&kunit_bus {
+/ {
 	test-kunit {
 		compatible = "test,kunit-empty";
 	};
drivers/of/of_test.c +0 -9
index 543fdf0936f6..08b670aee083 100644
@@ -15,17 +15,8 @@ static void dtb_root_node_exists(struct kunit *test)
 	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, of_find_node_by_path("/"));
 }
 
-/*
- * Test that the /__symbols__ node exists.
- */
-static void dtb_symbols_node_exists(struct kunit *test)
-{
-	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, of_find_node_by_path("/__symbols__"));
-}
-
 static struct kunit_case dtb_test_cases[] = {
 	KUNIT_CASE(dtb_root_node_exists),
-	KUNIT_CASE(dtb_symbols_node_exists),
 	{}
 };
author maintainer
#5 lore raw

Re: [PATCH v2 01/11] of: Load KUnit DTB from of_core_init()

Quoting Stephen Boyd (2023-03-21 11:15:19)

Quoted 39 lines

Quoting Rob Herring (2023-03-21 10:33:03)

Quoted 26 lines · Stephen Boyd
Quoted 25 lines
drivers/of/of_test.c new file +19 -0
new file mode 100644
index 000000000000..a4d70ac344ad
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for OF APIs
+ */
+#include <linux/kconfig.h>
+#include <linux/of.h>
+
+#include <kunit/test.h>
+
+/*
+ * Test that the root node / exists.
+ */
+static void dtb_root_node_exists(struct kunit *test)
+{
+     KUNIT_EXPECT_NOT_ERR_OR_NULL(test, of_find_node_by_path("/"));
+}
+
+/*
+ * Test that the /__symbols__ node exists.
Quoted 9 lines

drivers/of/of_test.c line 24

Quoted 5 lines
drivers/of/of_test.c new file +5 -0
+ */
+static void dtb_symbols_node_exists(struct kunit *test)
+{
+     KUNIT_EXPECT_NOT_ERR_OR_NULL(test, of_find_node_by_path("/__symbols__"));
+}

Many base DTs will not have this. And the kunit tests themselves shouldn't need it because they should be independent of the base tree.

When I try to apply an overlay it fails

Quoted 9 lines

OF: overlay: no fragments or symbols in overlay
OF: overlay: init_overlay_changeset() failed, ret = -22

     # of_overlay_apply_kunit_apply: ASSERTION FAILED at drivers/of/overlay_test.c:18
     Expected 0 == ({ extern uint8_t __dtbo_kunit_overlay_test_begin[]; extern uint8_t __dtbo_kunit_overlay_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_overlay_test_begin, __dtbo_kunit_overlay_test_end); }), but
         ({ extern uint8_t __dtbo_kunit_overlay_test_begin[]; extern uint8_t __dtbo_kunit_overlay_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_overlay_test_begin, __dtbo_kunit_overlay_test_end); }) == -12 (0xfffffffffffffff4)

[FAILED] of_overlay_apply_kunit_apply

Now I'm trying to hack on the fake root node to see if I can make it work.

Aha I figured it out. Needed to read more documentation.

---8<---

drivers/of/kunit_overlay_test.dtso +1 -1
index e3ced1467dd9..7688f9ef1b6b 100644
@@ -2,7 +2,7 @@
 /dts-v1/;
 /plugin/;
 
-/ {
+&{/} {
 	test-kunit {
 		compatible = "test,kunit-empty";
 	};
author maintainer
#6 lore raw

PATCH v2 2/11 of: Add test managed wrappers for of_overlay_apply()/of_node_put()

In reply to Stephen Boyd

Add test managed wrappers for of_overlay_apply() that automatically removes the overlay when the test is finished. This API is intended for use by KUnit tests that test code which relies on 'struct device_node's and of_*() APIs.

KUnit tests will call of_overlay_apply_kunit() to load an overlay that's been built into the kernel image. When the test is complete, the overlay will be removed.

This has a few benefits:

1) It keeps the tests hermetic because the overlay is removed when the test is complete. Tests won't even be aware that an overlay was loaded in another test.

2) The overlay code can live right next to the unit test that loads it. The overlay and the unit test can be compiled into one kernel module if desired.

3) We can test different device tree configurations by loading different overlays. The overlays can be written for a specific test, and there can be many of them loaded per-test without needing to jam all possible combinations into one DTB.

4) It also allows KUnit to test device tree dependent code on any architecture, not just UML. This allows KUnit tests to test architecture specific device tree code.

There are some potential pitfalls though. Test authors need to be careful to not overwrite properties in the live tree. The easiest way to do this is to add and remove nodes to the kunit_bus node.

---
 drivers/of/Makefile   |   2 +
 drivers/of/of_kunit.c | 123 ++++++++++++++++++++++++++++++++++++++++++
 include/kunit/of.h    |  90 +++++++++++++++++++++++++++++++
 3 files changed, 215 insertions(+)
 create mode 100644 drivers/of/of_kunit.c
 create mode 100644 include/kunit/of.h
drivers/of/Makefile +2 -0
index cf6ee7ba6350..d1110358fd45 100644
@@ -19,6 +19,8 @@ obj-y	+= kexec.o
 endif
 endif
 
+obj-$(CONFIG_KUNIT) += of_kunit.o
+
 DTC_FLAGS_kunit += -@
 obj-$(CONFIG_OF_KUNIT) += kunit.dtbo.o
 obj-$(CONFIG_OF_KUNIT_TEST) += of_test.o
drivers/of/of_kunit.c new file +123 -0

123 lines not on this page · raw message

include/kunit/of.h new file +90 -0

90 lines not on this page · raw message

Signature · 2 lines
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
author maintainer
#7 lore raw

PATCH v2 3/11 dt-bindings: vendor-prefixes: Add "test" vendor for KUnit and friends

In reply to Stephen Boyd

Add the vendor prefix "test" to reserve a vendor prefix for bindings that are purely for testing device tree code. This allows test code to write bindings that can be checked by the schema validator.

Cc Rob Herring <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0
index ed64e06ecca4..e50a78ec7344 100644
@@ -1331,6 +1331,8 @@ patternProperties:
     description: Terasic Inc.
   "^tesla,.*":
     description: Tesla, Inc.
+  "^test,.*":
+    description: Reserved for use by tests. For example, KUnit.
   "^tfc,.*":
     description: Three Five Corp
   "^thead,.*":
Signature · 2 lines
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
author maintainer
#8 lore raw

PATCH v2 4/11 dt-bindings: test: Add KUnit empty node binding

In reply to Stephen Boyd

Describe a binding for an empty device node used by KUnit tests to confirm overlays load properly.

Cc Rob Herring <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Brendan Higgins <[email protected]>
Cc David Gow <[email protected]>
---
 .../bindings/test/test,kunit-empty.yaml       | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/test/test,kunit-empty.yaml
Documentation/devicetree/bindings/test/test,kunit-empty.yaml new file +30 -0
new file mode 100644
index 000000000000..592fd06031b8
@@ -0,0 +1,30 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/test,kunit-empty.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: KUnit empty node
+
+maintainers:
+  - David Gow <[email protected]>
+  - Brendan Higgins <[email protected]>
+
+description: |
+  An empty node to confirm KUnit can load device tree overlays.
+
+properties:
+  compatible:
+    const: test,kunit-empty
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    kunit-node {
+      compatible = "test,kunit-empty";
+    };
+...
Signature · 2 lines
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
maintainer
#9 lore raw

Re: [PATCH v2 04/11] dt-bindings: test: Add KUnit empty node binding

Quoted 19 lines · Stephen Boyd

Describe a binding for an empty device node used by KUnit tests to confirm overlays load properly.

Cc Rob Herring <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Brendan Higgins <[email protected]>
Cc David Gow <[email protected]>
---
 .../bindings/test/test,kunit-empty.yaml       | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/test/test,kunit-empty.yaml
Documentation/devicetree/bindings/test/test,kunit-empty.yaml new file +0 -0
new file mode 100644
index 000000000000..592fd06031b8

Documentation/devicetree/bindings/test/test,kunit-empty.yaml line 4

Quoted 4 lines
Documentation/devicetree/bindings/test/test,kunit-empty.yaml new file +4 -0
@@ -0,0 +1,30 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/test,kunit-empty.yaml#

s/clock/test/

'make dt_binding_check' will tell you this.

Documentation/devicetree/bindings/test/test,kunit-empty.yaml line 13

Quoted 9 lines
Documentation/devicetree/bindings/test/test,kunit-empty.yaml new file +9 -0
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: KUnit empty node
+
+maintainers:
+  - David Gow <[email protected]>
+  - Brendan Higgins <[email protected]>
+
+description: |

Don't need '|'

Quoted 21 lines
Documentation/devicetree/bindings/test/test,kunit-empty.yaml new file +17 -0
+  An empty node to confirm KUnit can load device tree overlays.
+
+properties:
+  compatible:
+    const: test,kunit-empty
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    kunit-node {
+      compatible = "test,kunit-empty";
+    };
+...
Signature · 2 lines
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
author maintainer
#10 lore raw

PATCH v2 5/11 of: Add a KUnit test for overlays and test managed APIs

In reply to Stephen Boyd

Test the KUnit test managed overlay APIs. Confirm that platform devices are created and destroyed properly. This provides us confidence that the test managed work correctly and can be relied upon to provide tests with fake platform devices and device nodes via overlays compiled into the kernel image.

Cc Rob Herring <[email protected]>
Cc Frank Rowand <[email protected]>
---
 drivers/of/.kunitconfig            |   2 +
 drivers/of/Kconfig                 |  10 +++
 drivers/of/Makefile                |   1 +
 drivers/of/kunit_overlay_test.dtso |   9 +++
 drivers/of/overlay_test.c          | 107 +++++++++++++++++++++++++++++
 5 files changed, 129 insertions(+)
 create mode 100644 drivers/of/kunit_overlay_test.dtso
 create mode 100644 drivers/of/overlay_test.c
drivers/of/.kunitconfig +2 -0
index 5a8fee11978c..7d570cb922a1 100644
@@ -1,3 +1,5 @@
 CONFIG_KUNIT=y
 CONFIG_OF=y
 CONFIG_OF_KUNIT_TEST=y
+CONFIG_OF_OVERLAY=y
+CONFIG_OF_OVERLAY_KUNIT_TEST=y
drivers/of/Kconfig +10 -0
index f6739b9560c5..736029c120ae 100644
@@ -112,6 +112,16 @@ config OF_OVERLAY
 	  While this option is selected automatically when needed, you can
 	  enable it manually to improve device tree unit test coverage.
 
+config OF_OVERLAY_KUNIT_TEST
+	tristate "Device Tree overlay KUnit tests" if !KUNIT_ALL_TESTS
+	depends on OF_OVERLAY
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  This option builds KUnit unit tests for the device tree overlay code.
+
+	  If unsure, say N here, but this option is safe to enable.
+
 config OF_NUMA
 	bool
 
drivers/of/Makefile +1 -0
index d1110358fd45..b49351c4d0d2 100644
@@ -24,5 +24,6 @@ obj-$(CONFIG_KUNIT) += of_kunit.o
 DTC_FLAGS_kunit += -@
 obj-$(CONFIG_OF_KUNIT) += kunit.dtbo.o
 obj-$(CONFIG_OF_KUNIT_TEST) += of_test.o
+obj-$(CONFIG_OF_OVERLAY_KUNIT_TEST) += overlay_test.o kunit_overlay_test.dtbo.o
 
 obj-$(CONFIG_OF_UNITTEST) += unittest-data/
drivers/of/kunit_overlay_test.dtso new file +9 -0
new file mode 100644
index 000000000000..6e70e2f8cd90
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+&kunit_bus {
+	test-kunit {
+		compatible = "test,kunit-empty";
+	};
+};
drivers/of/overlay_test.c new file +107 -0

107 lines not on this page · raw message

Signature · 2 lines
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
author maintainer
#11 lore raw

PATCH v2 6/11 platform: Add test managed platform_device/driver APIs

In reply to Stephen Boyd

Introduce KUnit resource wrappers around platform_driver_register(), platform_device_alloc(), and platform_device_add() so that test authors can register platform drivers/devices from their tests and have the drivers/devices automatically be unregistered when the test is done.

This makes test setup code simpler when a platform driver or platform device is needed. Add a few test cases at the same time to make sure the APIs work as intended.

Cc Brendan Higgins <[email protected]>
Cc David Gow <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
---
 drivers/base/test/Makefile              |   3 +
 drivers/base/test/platform_kunit-test.c | 108 ++++++++++++++
 drivers/base/test/platform_kunit.c      | 186 ++++++++++++++++++++++++
 include/kunit/platform_device.h         |  15 ++
 4 files changed, 312 insertions(+)
 create mode 100644 drivers/base/test/platform_kunit-test.c
 create mode 100644 drivers/base/test/platform_kunit.c
 create mode 100644 include/kunit/platform_device.h
drivers/base/test/Makefile +3 -0
index 7f76fee6f989..9edebaac4e75 100644
@@ -3,3 +3,6 @@ obj-$(CONFIG_TEST_ASYNC_DRIVER_PROBE)	+= test_async_driver_probe.o
 
 obj-$(CONFIG_DRIVER_PE_KUNIT_TEST) += property-entry-test.o
 CFLAGS_property-entry-test.o += $(DISABLE_STRUCTLEAK_PLUGIN)
+
+obj-$(CONFIG_KUNIT) += platform_kunit.o
+obj-$(CONFIG_KUNIT_TEST) += platform_kunit-test.o
drivers/base/test/platform_kunit-test.c new file +108 -0

108 lines not on this page · raw message

drivers/base/test/platform_kunit.c new file +186 -0

186 lines not on this page · raw message

include/kunit/platform_device.h new file +15 -0
new file mode 100644
index 000000000000..28d28abf15a4
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _KUNIT_PLATFORM_DRIVER_H
+#define _KUNIT_PLATFORM_DRIVER_H
+
+struct kunit;
+struct platform_device;
+struct platform_driver;
+
+struct platform_device *
+platform_device_alloc_kunit(struct kunit *test, const char *name, int id);
+int platform_device_add_kunit(struct kunit *test, struct platform_device *pdev);
+
+int platform_driver_register_kunit(struct kunit *test, struct platform_driver *drv);
+
+#endif
Signature · 2 lines
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
author maintainer
#12 lore raw

PATCH v2 7/11 dt-bindings: kunit: Add fixed rate clk consumer test

In reply to Stephen Boyd

Describe a binding for a device that consumes a fixed rate clk in DT so that a KUnit test can get the clk registered by of_fixed_clk_setup() and test that it is setup properly.

Cc Rob Herring <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Brendan Higgins <[email protected]>
Cc David Gow <[email protected]>
---
 .../kunit/test,clk-kunit-fixed-rate.yaml      | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml
Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml new file +35 -0
new file mode 100644
index 000000000000..58d7826d9c14
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/kunit/test,clk-kunit-fixed-rate.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: KUnit clk fixed rate test clk consumer
+
+maintainers:
+  - Stephen Boyd <[email protected]>
+
+description: |
+  A clk consumer of a fixed rate clk used to test the fixed rate clk
+  implementation in the Linux kernel.
+
+properties:
+  compatible:
+    const: test,clk-kunit-fixed-rate
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    clock-consumer {
+      compatible = "test,clk-kunit-fixed-rate";
+      clocks = <&fixed_clk>;
+    };
+...
Signature · 2 lines
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
maintainer
#13 lore raw

Re: [PATCH v2 07/11] dt-bindings: kunit: Add fixed rate clk consumer test

Quoted 11 lines · Stephen Boyd

Describe a binding for a device that consumes a fixed rate clk in DT so that a KUnit test can get the clk registered by of_fixed_clk_setup() and test that it is setup properly.

Cc Rob Herring <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Brendan Higgins <[email protected]>
Cc David Gow <[email protected]>
---
 .../kunit/test,clk-kunit-fixed-rate.yaml      | 35 +++++++++++++++++++

Some stuff in test and some in kunit? I prefer 'test'.

Quoted 11 lines
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml
Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml new file +2 -0
new file mode 100644
index 000000000000..58d7826d9c14
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2

Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml line 12

Quoted 10 lines
Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml new file +10 -0
+---
+$id: http://devicetree.org/schemas/kunit/test,clk-kunit-fixed-rate.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: KUnit clk fixed rate test clk consumer
+
+maintainers:
+  - Stephen Boyd <[email protected]>
+
+description: |

Drop '|'

Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml line 18

Quoted 6 lines
Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml new file +6 -0
+  A clk consumer of a fixed rate clk used to test the fixed rate clk
+  implementation in the Linux kernel.
+
+properties:
+  compatible:
+    const: test,clk-kunit-fixed-rate

I tend to think we should drop 'kunit' from these bindings. You could use this for different test frameworks.

Quoted 21 lines
Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml new file +17 -0
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    clock-consumer {
+      compatible = "test,clk-kunit-fixed-rate";
+      clocks = <&fixed_clk>;
+    };
+...
Signature · 2 lines
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
author maintainer
#14 lore raw

Re: [PATCH v2 07/11] dt-bindings: kunit: Add fixed rate clk consumer test

Quoting Rob Herring (2023-03-21 13:14:46)

Quoted 13 lines · Stephen Boyd
Quoted 11 lines

Describe a binding for a device that consumes a fixed rate clk in DT so that a KUnit test can get the clk registered by of_fixed_clk_setup() and test that it is setup properly.

Cc Rob Herring <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Brendan Higgins <[email protected]>
Cc David Gow <[email protected]>
---
 .../kunit/test,clk-kunit-fixed-rate.yaml      | 35 +++++++++++++++++++
Quoted 1 line

Some stuff in test and some in kunit? I prefer 'test'.

So only a test directory? Ok.

Quoted 25 lines
Quoted 12 lines
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml
Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml new file +2 -0
new file mode 100644
index 000000000000..58d7826d9c14
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2

Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml line 12

Quoted 10 lines
Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml new file +10 -0
+---
+$id: http://devicetree.org/schemas/kunit/test,clk-kunit-fixed-rate.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: KUnit clk fixed rate test clk consumer
+
+maintainers:
+  - Stephen Boyd <[email protected]>
+
+description: |

Drop '|'

Quoted 9 lines

Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml line 18

Quoted 6 lines
Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml new file +6 -0
+  A clk consumer of a fixed rate clk used to test the fixed rate clk
+  implementation in the Linux kernel.
+
+properties:
+  compatible:
+    const: test,clk-kunit-fixed-rate

I tend to think we should drop 'kunit' from these bindings. You could use this for different test frameworks.

Sure.

author maintainer
#15 lore raw

PATCH v2 8/11 clk: Add test managed clk provider/consumer APIs

In reply to Stephen Boyd

Unit tests are more ergonomic and simpler to understand if they don't have to hoist a bunch of code into the test harness init and exit functions. Add some test managed wrappers for the clk APIs so that clk unit tests can write more code in the actual test and less code in the harness.

Only add APIs that are used for now. More wrappers can be added in the future as necessary.

Cc Brendan Higgins <[email protected]>
Cc David Gow <[email protected]>
---
 drivers/clk/Makefile    |   5 +
 drivers/clk/clk_kunit.c | 219 ++++++++++++++++++++++++++++++++++++++++
 include/kunit/clk.h     |  28 +++++
 3 files changed, 252 insertions(+)
 create mode 100644 drivers/clk/clk_kunit.c
 create mode 100644 include/kunit/clk.h
drivers/clk/Makefile +5 -0
index e3ca0d058a25..226b4b729703 100644
@@ -17,6 +17,11 @@ ifeq ($(CONFIG_OF), y)
 obj-$(CONFIG_COMMON_CLK)	+= clk-conf.o
 endif
 
+# KUnit specific helpers
+ifeq ($(CONFIG_COMMON_CLK), y)
+obj-$(CONFIG_KUNIT)		+= clk_kunit.o
+endif
+
 # hardware specific clock types
 # please keep this section sorted lexicographically by file path name
 obj-$(CONFIG_COMMON_CLK_APPLE_NCO)  	+= clk-apple-nco.o
drivers/clk/clk_kunit.c new file +219 -0

219 lines not on this page · raw message

include/kunit/clk.h new file +28 -0
new file mode 100644
index 000000000000..73bc99cefe7b
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _CLK_KUNIT_H
+#define _CLK_KUNIT_H
+
+struct clk;
+struct clk_hw;
+struct device;
+struct device_node;
+struct kunit;
+
+struct clk *
+clk_get_kunit(struct kunit *test, struct device *dev, const char *con_id);
+struct clk *
+of_clk_get_kunit(struct kunit *test, struct device_node *np, int index);
+
+struct clk *
+clk_hw_get_clk_kunit(struct kunit *test, struct clk_hw *hw, const char *con_id);
+struct clk *
+clk_hw_get_clk_prepared_enabled_kunit(struct kunit *test, struct clk_hw *hw,
+				      const char *con_id);
+
+int clk_prepare_enable_kunit(struct kunit *test, struct clk *clk);
+
+int clk_hw_register_kunit(struct kunit *test, struct device *dev, struct clk_hw *hw);
+int of_clk_hw_register_kunit(struct kunit *test, struct device_node *node,
+			     struct clk_hw *hw);
+
+#endif
Signature · 2 lines
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
author maintainer
#16 lore raw

PATCH v2 9/11 clk: Add KUnit tests for clk fixed rate basic type

In reply to Stephen Boyd

Test that the fixed rate basic type clk works as intended.

Cc Brendan Higgins <[email protected]>
Cc David Gow <[email protected]>
---
 drivers/clk/.kunitconfig                   |   3 +
 drivers/clk/Kconfig                        |   7 +
 drivers/clk/Makefile                       |   1 +
 drivers/clk/clk-fixed-rate_test.c          | 299 +++++++++++++++++++++
 drivers/clk/clk-fixed-rate_test.h          |   8 +
 drivers/clk/kunit_clk_fixed_rate_test.dtso |  19 ++
 6 files changed, 337 insertions(+)
 create mode 100644 drivers/clk/clk-fixed-rate_test.c
 create mode 100644 drivers/clk/clk-fixed-rate_test.h
 create mode 100644 drivers/clk/kunit_clk_fixed_rate_test.dtso
drivers/clk/.kunitconfig +3 -0
index 2fbeb71316f8..5faf67773b74 100644
@@ -1,5 +1,8 @@
 CONFIG_KUNIT=y
+CONFIG_OF=y
+CONFIG_OF_OVERLAY=y
 CONFIG_COMMON_CLK=y
 CONFIG_CLK_KUNIT_TEST=y
+CONFIG_CLK_FIXED_RATE_KUNIT_TEST=y
 CONFIG_CLK_GATE_KUNIT_TEST=y
 CONFIG_UML_PCI_OVER_VIRTIO=n
drivers/clk/Kconfig +7 -0
index b6c5bf69a2b2..a992ca5e1efe 100644
@@ -478,6 +478,13 @@ config CLK_KUNIT_TEST
 	help
 	  Kunit tests for the common clock framework.
 
+config CLK_FIXED_RATE_KUNIT_TEST
+	tristate "Basic fixed rate clk type KUnit test" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  KUnit tests for the basic fixed rate clk type.
+
 config CLK_GATE_KUNIT_TEST
 	tristate "Basic gate type Kunit test" if !KUNIT_ALL_TESTS
 	depends on KUNIT
drivers/clk/Makefile +1 -0
index 226b4b729703..4fb809f4bd95 100644
@@ -6,6 +6,7 @@ obj-$(CONFIG_CLK_KUNIT_TEST)	+= clk_test.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-divider.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-fixed-factor.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-fixed-rate.o
+obj-$(CONFIG_CLK_FIXED_RATE_KUNIT_TEST)	+= clk-fixed-rate_test.o kunit_clk_fixed_rate_test.dtbo.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-gate.o
 obj-$(CONFIG_CLK_GATE_KUNIT_TEST) += clk-gate_test.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-multiplier.o
drivers/clk/clk-fixed-rate_test.c new file +299 -0

299 lines not on this page · raw message

drivers/clk/clk-fixed-rate_test.h new file +8 -0
new file mode 100644
index 000000000000..e0d28e5b6081
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _CLK_FIXED_RATE_TEST_H
+#define _CLK_FIXED_RATE_TEST_H
+
+#define TEST_FIXED_FREQUENCY	50000000
+#define TEST_FIXED_ACCURACY	300
+
+#endif
drivers/clk/kunit_clk_fixed_rate_test.dtso new file +19 -0
new file mode 100644
index 000000000000..758161ceb374
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "clk-fixed-rate_test.h"
+
+&kunit_bus {
+	fixed_50MHz: clock-50MHz {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <TEST_FIXED_FREQUENCY>;
+		clock-accuracy = <TEST_FIXED_ACCURACY>;
+	};
+
+	clock-consumer-fixed-50 {
+		compatible = "test,clk-kunit-fixed-rate";
+		clocks = <&fixed_50MHz>;
+	};
+};
Signature · 2 lines
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
author maintainer
#17 lore raw

PATCH v2 10/11 dt-bindings: clk: Add KUnit clk_parent_data test

In reply to Stephen Boyd

Describe a binding for a device that provides and consumes clks in DT so that a KUnit test can register clks based on the device node and test clk_hw_register() with clk_parent_data.

Cc Rob Herring <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Brendan Higgins <[email protected]>
Cc David Gow <[email protected]>
---
 .../clock/test,clk-kunit-parent-data.yaml     | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/test,clk-kunit-parent-data.yaml
Documentation/devicetree/bindings/clock/test,clk-kunit-parent-data.yaml new file +47 -0
new file mode 100644
index 000000000000..ece47f05c498
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/test,clk-kunit-parent-data.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: KUnit clk provider for clk_parent_data unit tests
+
+maintainers:
+  - Stephen Boyd <[email protected]>
+
+description: |
+  A clk provider to test the struct clk_parent_data implementation in the Linux
+  kernel.
+
+properties:
+  compatible:
+    const: test,clk-kunit-parent-data
+
+  clocks:
+    items:
+      - description: Fixed parent
+      - description: 50 MHz fixed parent
+
+  clock-names:
+    items:
+      - const: parent_fwname
+      - const: "50"
+
+  "#clock-cells":
+    const: 1
+
+required:
+  - compatible
+  - "#clock-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    clock-controller {
+      compatible = "test,clk-kunit-parent-data";
+      #clock-cells = <1>;
+      clocks = <&fixed_parent>, <&fixed_50MHz>;
+      clock-names = "parent_fwname", "50";
+    };
+...
Signature · 2 lines
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
author maintainer
#18 lore raw

PATCH v2 11/11 clk: Add KUnit tests for clks registered with struct clk_parent_data

In reply to Stephen Boyd

Test that clks registered with 'struct clk_parent_data' work as intended and can find their parents.

Cc Christian Marangi <[email protected]>
Cc Brendan Higgins <[email protected]>
Cc David Gow <[email protected]>
---
 drivers/clk/Makefile                        |   3 +-
 drivers/clk/clk_parent_data_test.h          |  10 +
 drivers/clk/clk_test.c                      | 459 +++++++++++++++++++-
 drivers/clk/kunit_clk_parent_data_test.dtso |  28 ++
 4 files changed, 498 insertions(+), 2 deletions(-)
 create mode 100644 drivers/clk/clk_parent_data_test.h
 create mode 100644 drivers/clk/kunit_clk_parent_data_test.dtso
drivers/clk/Makefile +2 -1
index 4fb809f4bd95..2697bdec1c46 100644
@@ -2,7 +2,8 @@
 # common clock types
 obj-$(CONFIG_HAVE_CLK)		+= clk-devres.o clk-bulk.o clkdev.o
 obj-$(CONFIG_COMMON_CLK)	+= clk.o
-obj-$(CONFIG_CLK_KUNIT_TEST)	+= clk_test.o
+obj-$(CONFIG_CLK_KUNIT_TEST)	+= clk_test.o \
+				   kunit_clk_parent_data_test.dtbo.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-divider.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-fixed-factor.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-fixed-rate.o
drivers/clk/clk_parent_data_test.h new file +10 -0
new file mode 100644
index 000000000000..eedd53ae910d
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _CLK_PARENT_DATA_TEST_H
+#define _CLK_PARENT_DATA_TEST_H
+
+#define CLK_PARENT_DATA_1MHZ_NAME	"1mhz_fixed_legacy"
+#define CLK_PARENT_DATA_PARENT1		"parent_fwname"
+#define CLK_PARENT_DATA_PARENT2		"50"
+#define CLK_PARENT_DATA_50MHZ_NAME	"50_clk"
+
+#endif
drivers/clk/clk_test.c +458 -1

483 lines not on this page · raw message

drivers/clk/kunit_clk_parent_data_test.dtso new file +28 -0
new file mode 100644
index 000000000000..68eb5957d05f
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "clk_parent_data_test.h"
+
+&kunit_bus {
+	fixed_50: clock-50MHz {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <50000000>;
+		clock-output-names = CLK_PARENT_DATA_50MHZ_NAME;
+	};
+
+	fixed_parent: clock-1MHz {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <1000000>;
+		clock-output-names = CLK_PARENT_DATA_1MHZ_NAME;
+	};
+
+	clock-controller {
+		compatible = "test,clk-kunit-parent-data";
+		clocks = <&fixed_parent>, <&fixed_50>;
+		clock-names = CLK_PARENT_DATA_PARENT1, CLK_PARENT_DATA_PARENT2;
+		#clock-cells = <1>;
+	};
+};
Signature · 2 lines
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
#19 lore raw
In reply to Stephen Boyd
Quoted 14 lines · Stephen Boyd

This patch series adds unit tests for the clk fixed rate basic type and the clk registration functions that use struct clk_parent_data. To get there, we add support for loading device tree overlays onto the live DTB along with probing platform drivers to bind to device nodes in the overlays. With this series, we're able to exercise some of the code in the common clk framework that uses devicetree lookups to find parents and the fixed rate clk code that scans device tree directly and creates clks. Please review.

I Cced everyone to all the patches so they get the full context. I'm hoping I can take the whole pile through the clk tree as they almost all depend on each other.

Quoted 9 lines

Changes from v1 (https://lore.kernel.org/r/[email protected]):
* Don't depend on UML, use unittest data approach to attach nodes
* Introduce overlay loading API for KUnit
* Move platform_device KUnit code to drivers/base/test
* Use #define macros for constants shared between unit tests and overlays
* Settle on "test" as a vendor prefix
* Make KUnit wrappers have "_kunit" postfix

Thanks! I like this much better, and it is working fine here under UML and x86_64.

There are some issues with other architectures (mostly endianness problems, but I got a refcount underflow in kunit_remove_resource after platform_device_alloc_kunit_test on sparc64 as well), but otherwise it's working on most of the things I've tried.

I'll play around with it some more next week and dig into the code in more detail, but I've not seen anything I dislike about the overall design so far.

Thanks!
-- David

Quoted 72 lines

Stephen Boyd (11):
of: Load KUnit DTB from of_core_init()
of: Add test managed wrappers for of_overlay_apply()/of_node_put()
dt-bindings: vendor-prefixes: Add "test" vendor for KUnit and friends
dt-bindings: test: Add KUnit empty node binding
of: Add a KUnit test for overlays and test managed APIs
platform: Add test managed platform_device/driver APIs
dt-bindings: kunit: Add fixed rate clk consumer test
clk: Add test managed clk provider/consumer APIs
clk: Add KUnit tests for clk fixed rate basic type
dt-bindings: clk: Add KUnit clk_parent_data test
clk: Add KUnit tests for clks registered with struct clk_parent_data

 .../clock/test,clk-kunit-parent-data.yaml     |  47 ++
 .../kunit/test,clk-kunit-fixed-rate.yaml      |  35 ++
 .../bindings/test/test,kunit-empty.yaml       |  30 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 drivers/base/test/Makefile                    |   3 +
 drivers/base/test/platform_kunit-test.c       | 108 +++++
 drivers/base/test/platform_kunit.c            | 186 +++++++
 drivers/clk/.kunitconfig                      |   3 +
 drivers/clk/Kconfig                           |   7 +
 drivers/clk/Makefile                          |   9 +-
 drivers/clk/clk-fixed-rate_test.c             | 299 ++++++++++++
 drivers/clk/clk-fixed-rate_test.h             |   8 +
 drivers/clk/clk_kunit.c                       | 219 +++++++++
 drivers/clk/clk_parent_data_test.h            |  10 +
 drivers/clk/clk_test.c                        | 459 +++++++++++++++++-
 drivers/clk/kunit_clk_fixed_rate_test.dtso    |  19 +
 drivers/clk/kunit_clk_parent_data_test.dtso   |  28 ++
 drivers/of/.kunitconfig                       |   5 +
 drivers/of/Kconfig                            |  23 +
 drivers/of/Makefile                           |   7 +
 drivers/of/base.c                             | 182 +++++++
 drivers/of/kunit.dtso                         |  10 +
 drivers/of/kunit_overlay_test.dtso            |   9 +
 drivers/of/of_kunit.c                         | 123 +++++
 drivers/of/of_private.h                       |   6 +
 drivers/of/of_test.c                          |  43 ++
 drivers/of/overlay_test.c                     | 107 ++++
 drivers/of/unittest.c                         | 101 +---
 include/kunit/clk.h                           |  28 ++
 include/kunit/of.h                            |  90 ++++
 include/kunit/platform_device.h               |  15 +
 31 files changed, 2119 insertions(+), 102 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/test,clk-kunit-parent-data.yaml
 create mode 100644 Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml
 create mode 100644 Documentation/devicetree/bindings/test/test,kunit-empty.yaml
 create mode 100644 drivers/base/test/platform_kunit-test.c
 create mode 100644 drivers/base/test/platform_kunit.c
 create mode 100644 drivers/clk/clk-fixed-rate_test.c
 create mode 100644 drivers/clk/clk-fixed-rate_test.h
 create mode 100644 drivers/clk/clk_kunit.c
 create mode 100644 drivers/clk/clk_parent_data_test.h
 create mode 100644 drivers/clk/kunit_clk_fixed_rate_test.dtso
 create mode 100644 drivers/clk/kunit_clk_parent_data_test.dtso
 create mode 100644 drivers/of/.kunitconfig
 create mode 100644 drivers/of/kunit.dtso
 create mode 100644 drivers/of/kunit_overlay_test.dtso
 create mode 100644 drivers/of/of_kunit.c
 create mode 100644 drivers/of/of_test.c
 create mode 100644 drivers/of/overlay_test.c
 create mode 100644 include/kunit/clk.h
 create mode 100644 include/kunit/of.h
 create mode 100644 include/kunit/platform_device.h

base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6 --
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git

author maintainer
#20 lore raw

Quoting David Gow (2023-03-17 01:22:30)

Quoted 26 lines · Stephen Boyd
Quoted 14 lines

This patch series adds unit tests for the clk fixed rate basic type and the clk registration functions that use struct clk_parent_data. To get there, we add support for loading device tree overlays onto the live DTB along with probing platform drivers to bind to device nodes in the overlays. With this series, we're able to exercise some of the code in the common clk framework that uses devicetree lookups to find parents and the fixed rate clk code that scans device tree directly and creates clks. Please review.

I Cced everyone to all the patches so they get the full context. I'm hoping I can take the whole pile through the clk tree as they almost all depend on each other.

Quoted 9 lines

Changes from v1 (https://lore.kernel.org/r/[email protected]):
* Don't depend on UML, use unittest data approach to attach nodes
* Introduce overlay loading API for KUnit
* Move platform_device KUnit code to drivers/base/test
* Use #define macros for constants shared between unit tests and overlays
* Settle on "test" as a vendor prefix
* Make KUnit wrappers have "_kunit" postfix

Quoted 7 lines

Thanks! I like this much better, and it is working fine here under UML and x86_64.

There are some issues with other architectures (mostly endianness problems, but I got a refcount underflow in kunit_remove_resource after platform_device_alloc_kunit_test on sparc64 as well), but otherwise it's working on most of the things I've tried.

I see the undercount as well. I believe the problem is that we need to not call platform_device_put() after the platform device is added to the bus with platform_device_add(). I think I need to make the platform_device_add_kunit() function find the resource allocated in platform_device_alloc_kunit() and remove it. The kunit_destroy_resource() API still calls the free function though? So I need a kunit_nuke_resource() API that unlinks and doesn't call the free function. Or I can modify the resource free function to call the platform_device_unregister() function once it is added. Or I can call get_device() one more time to balance in the platform_device_add_kunit() wrapper.

Either way, calling platform_device_put() is really only supposed to be done in error cases, so calling it from the kunit resource wrapper is sorta wrong.

#21 lore raw
In reply to Stephen Boyd

Hi,

Quoted 13 lines · Stephen Boyd

This patch series adds unit tests for the clk fixed rate basic type and the clk registration functions that use struct clk_parent_data. To get there, we add support for loading device tree overlays onto the live DTB along with probing platform drivers to bind to device nodes in the overlays. With this series, we're able to exercise some of the code in the common clk framework that uses devicetree lookups to find parents and the fixed rate clk code that scans device tree directly and creates clks. Please review.

I Cced everyone to all the patches so they get the full context. I'm hoping I can take the whole pile through the clk tree as they almost all depend on each other.

Quoted 8 lines

Changes from v1 (https://lore.kernel.org/r/[email protected]):
* Don't depend on UML, use unittest data approach to attach nodes
* Introduce overlay loading API for KUnit
* Move platform_device KUnit code to drivers/base/test
* Use #define macros for constants shared between unit tests and overlays
* Settle on "test" as a vendor prefix
* Make KUnit wrappers have "_kunit" postfix

Maybe I'm overthinking this, but wouldn't it make more sense to have a kunit *prefix* to those functions? Any other function in the kernel taking a kunit test pointer as a parameter starts with kunit (like kunit_kzalloc), so it would make more sense to me that kunit-related clk functions follow the same pattern.

Maxime

author maintainer
#22 lore raw

Quoting Maxime Ripard (2023-03-17 01:39:51)

Quoted 25 lines

Hi,

Quoted 13 lines · Stephen Boyd

This patch series adds unit tests for the clk fixed rate basic type and the clk registration functions that use struct clk_parent_data. To get there, we add support for loading device tree overlays onto the live DTB along with probing platform drivers to bind to device nodes in the overlays. With this series, we're able to exercise some of the code in the common clk framework that uses devicetree lookups to find parents and the fixed rate clk code that scans device tree directly and creates clks. Please review.

I Cced everyone to all the patches so they get the full context. I'm hoping I can take the whole pile through the clk tree as they almost all depend on each other.

Quoted 8 lines

Changes from v1 (https://lore.kernel.org/r/[email protected]):
* Don't depend on UML, use unittest data approach to attach nodes
* Introduce overlay loading API for KUnit
* Move platform_device KUnit code to drivers/base/test
* Use #define macros for constants shared between unit tests and overlays
* Settle on "test" as a vendor prefix
* Make KUnit wrappers have "_kunit" postfix

Quoted 6 lines

Maybe I'm overthinking this, but wouldn't it make more sense to have a kunit *prefix* to those functions? Any other function in the kernel taking a kunit test pointer as a parameter starts with kunit (like kunit_kzalloc), so it would make more sense to me that kunit-related clk functions follow the same pattern.

See this thread on v1

https://lore.kernel.org/all/CABVgOSk4gEob3rokKF_p2Bcd_Sj3ikUN4R-HPHyTR0Eoo==85g@mail.gmail.com/