of: populate of_root node if bootloader doesn't
Subsystems (5)
v3 cover of: populate of_root node if bootloader doesn't
Arch maintainers, please ack/review patches.
This is a resend of a series from Frank last year[1]. I worked in Rob's review comments to unconditionally call unflatten_device_tree() and fixup/audit calls to of_have_populated_dt() so that behavior doesn't change.
I need this series so I can add DT based tests in the clk framework. Either I can merge it through the clk tree once everyone is happy, or Rob can merge it through the DT tree and provide some branch so I can base clk patches on it.
Changes from Frank's series[1]:
* Add a DTB loaded kunit test
* Make of_have_populated_dt() return false if the DTB isn't from the bootloader
* Architecture calls made unconditional so that a root node is always made
Changes from v2 (https://lore.kernel.org/r/[email protected]):
* Reorder patches to have OF changes largely first
* No longer modify initial_boot_params if ACPI=y
* Put arm64 patch back to v1
Changes from v1 (https://lore.kernel.org/r/[email protected]):
* x86 patch included
* arm64 knocks out initial dtb if acpi is in use
* keep Kconfig hidden but def_bool enabled otherwise
Frank Rowand (2):
of: Create of_root if no dtb provided by firmware
of: unittest: treat missing of_root as error instead of fixing up
Stephen Boyd (5):
of: Always unflatten in unflatten_and_copy_device_tree()
um: Unconditionally call unflatten_device_tree() x86/of: Unconditionally call unflatten_and_copy_device_tree()
arm64: Unconditionally call unflatten_device_tree()
of: Add KUnit test to confirm DTB is loaded
arch/arm64/kernel/setup.c | 3 +- arch/um/kernel/dtb.c | 14 ++++---- arch/x86/kernel/devicetree.c | 24 +++++++------- drivers/of/.kunitconfig | 3 ++ drivers/of/Kconfig | 11 ++++++- drivers/of/Makefile | 4 ++- drivers/of/empty_root.dts | 6 ++++ drivers/of/fdt.c | 64 +++++++++++++++++++++++++++--------- drivers/of/of_test.c | 48 +++++++++++++++++++++++++++ drivers/of/platform.c | 3 -- drivers/of/unittest.c | 16 +++------ include/linux/of.h | 25 ++++++++------ 12 files changed, 158 insertions(+), 63 deletions(-) create mode 100644 drivers/of/.kunitconfig create mode 100644 drivers/of/empty_root.dts create mode 100644 drivers/of/of_test.c
[1] https://lore.kernel.org/r/[email protected]
base-commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a
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
PATCH v3 1/7 of: Always unflatten in unflatten_and_copy_device_tree()
We want to populate an empty DT whenever CONFIG_OF is enabled so that overlays can be applied and the DT unit tests can be run. Make unflatten_and_copy_device_tree() stop printing a warning if the 'initial_boot_params' pointer is NULL. Instead, simply copy the dtb if there is one and then unflatten it. If there isn't a DT to copy, then the call to unflatten_device_tree() is largely a no-op, so nothing really changes here.
--- drivers/of/fdt.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-)
drivers/of/fdt.c +17 -15
@@ -1318,6 +1318,21 @@ bool __init early_init_dt_scan(void *params)return true; } +static void *__init copy_device_tree (void *fdt) +{ +int size; +void *dt; + + size =fdt_totalsize (fdt); + dt =early_init_dt_alloc_memory_arch (size, +roundup_pow_of_two (FDT_V17_SIZE )); + +if (dt) +memcpy (dt, fdt, size); + +return dt; +} +/** * unflatten_device_tree - create tree of device_nodes from flat blob *
@@ -1350,22 +1365,9 @@ void __init unflatten_device_tree(void)*/ void __init unflatten_and_copy_device_tree (void ) { -int size; -void *dt; +if (initial_boot_params) + initial_boot_params =copy_device_tree (initial_boot_params); -if (!initial_boot_params) { -pr_warn ("No valid device tree found, continuing without\n" ); -return ; - } - - size =fdt_totalsize (initial_boot_params); - dt =early_init_dt_alloc_memory_arch (size, -roundup_pow_of_two (FDT_V17_SIZE )); - -if (dt) { -memcpy (dt, initial_boot_params, size); - initial_boot_params = dt; - }unflatten_device_tree (); }
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
PATCH v3 2/7 of: Create of_root if no dtb provided by firmware
In reply to Stephen BoydFrom: Frank Rowand <[email protected]>
When enabling CONFIG_OF on a platform where 'of_root' is not populated by firmware, we end up without a root node. In order to apply overlays and create subnodes of the root node, we need one. Create this root node by unflattening an empty builtin dtb.
If firmware provides a flattened device tree (FDT) then the FDT is unflattened via setup_arch(). Otherwise, the call to unflatten(_and_copy)?_device_tree() will create an empty root node.
We make of_have_populated_dt() return true only if the DTB was loaded by firmware so that existing callers don't change behavior after this patch. The call in the of platform code is removed because it prevents overlays from creating platform devices when the platform bus isn't fully initialized.
[[email protected]: Update of_have_populated_dt() to treat this empty dtb as not populated. Drop setup_of() initcall]
--- drivers/of/Kconfig | 2 +- drivers/of/Makefile | 2 +- drivers/of/empty_root.dts | 6 ++++++ drivers/of/fdt.c | 32 +++++++++++++++++++++++++++++++- drivers/of/platform.c | 3 --- include/linux/of.h | 25 +++++++++++++++---------- 6 files changed, 54 insertions(+), 16 deletions(-) create mode 100644 drivers/of/empty_root.dts
drivers/of/Kconfig +1 -1
@@ -54,7 +54,7 @@ config OF_FLATTREE select CRC32 config OF_EARLY_FLATTREE - bool + def_bool OF && !SPARC select DMA_DECLARE_COHERENT if HAS_DMA && HAS_IOMEM select OF_FLATTREE
drivers/of/Makefile +1 -1
@@ -2,7 +2,7 @@ obj-y = base.o cpu.o device.o module.o platform.o property.o obj-$(CONFIG_OF_KOBJ) += kobj.o obj-$(CONFIG_OF_DYNAMIC) += dynamic.o -obj-$(CONFIG_OF_FLATTREE) += fdt.o +obj-$(CONFIG_OF_FLATTREE) += fdt.o empty_root.dtb.o obj-$(CONFIG_OF_EARLY_FLATTREE) += fdt_address.o obj-$(CONFIG_OF_PROMTREE) += pdt.o obj-$(CONFIG_OF_ADDRESS) += address.o
drivers/of/empty_root.dts new file +6 -0
@@ -0,0 +1,6 @@ +// SPDX-License-Identifier: GPL-2.0-only +/dts-v1/; + +/ { + +};
drivers/of/fdt.c +31 -1
@@ -8,6 +8,7 @@#define pr_fmt (fmt) "OF: fdt: " fmt +#include <linux/acpi.h> #include <linux/crash_dump.h> #include <linux/crc32.h> #include <linux/kernel.h>
@@ -32,6 +33,13 @@#include "of_private.h" +/* +* __dtb_empty_root_begin[] and __dtb_empty_root_end[] magically created by +* cmd_dt_S_dtb in scripts/Makefile.lib +*/ +extern uint8_t __dtb_empty_root_begin[]; +extern uint8_t __dtb_empty_root_end[]; +/* * of_fdt_limit_memory - limit the number of regions in the /memory node * @limit: maximum entries
@@ -1343,7 +1351,29 @@ static void *__init copy_device_tree(void *fdt)*/ void __init unflatten_device_tree (void ) { -__unflatten_device_tree (initial_boot_params,NULL , &of_root, +void *fdt = initial_boot_params; + +/* Don't use the bootloader provided DTB if ACPI is enabled */ +if (!acpi_disabled) + fdt =NULL ; + +/* +* Populate an empty root node when ACPI is enabled or bootloader +* doesn't provide one. +*/ +if (!fdt) { + fdt = (void *) __dtb_empty_root_begin; +/* fdt_totalsize() will be used for copy size */ +if (fdt_totalsize (fdt) > + __dtb_empty_root_end - __dtb_empty_root_begin) { +pr_err ("invalid size in dtb_empty_root\n" ); +return ; + } + of_fdt_crc32 =crc32_be (~0 , fdt,fdt_totalsize (fdt)); + fdt =copy_device_tree (fdt); + } + +__unflatten_device_tree (fdt,NULL , &of_root, early_init_dt_alloc_memory_arch, false);/* Get pointer to "/chosen" and "/aliases" nodes for use everywhere */
drivers/of/platform.c +0 -3
@@ -549,9 +549,6 @@ static int __init of_platform_default_populate_init(void)device_links_supplier_sync_state_pause (); -if (!of_have_populated_dt ()) -return -ENODEV ; -if (IS_ENABLED (CONFIG_PPC )) {struct device_node *boot_display =NULL ;struct platform_device *dev;
include/linux/of.h +15 -10
@@ -180,11 +180,6 @@ static inline bool is_of_node(const struct fwnode_handle *fwnode) &__of_fwnode_handle_node->fwnode : NULL; \ }) -static inline bool of_have_populated_dt (void ) -{ -return of_root !=NULL ; -} -static inline bool of_node_is_root (const struct device_node *node) {return node && (node->parent ==NULL );
@@ -549,11 +544,6 @@ static inline struct device_node *of_find_node_with_property(#define of_fwnode_handle (node) NULL -static inline bool of_have_populated_dt (void ) -{ -return false; -} -static inline struct device_node *of_get_compatible_child (const struct device_node *parent,const char *compatible) {
@@ -1634,6 +1624,21 @@ static inline bool of_device_is_system_power_controller(const struct device_nodereturn of_property_read_bool (np,"system-power-controller" ); } +/** +* of_have_populated_dt() - Has DT been populated by bootloader +* +* Return: True if a DTB has been populated by the bootloader and it isn't the +* empty builtin one. False otherwise. +*/ +static inline bool of_have_populated_dt (void ) +{ +#ifdef CONFIG_OF +return of_property_present (of_root,"compatible" ); +#else +return false; +#endif +} +/* * Overlay support */
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
PATCH v3 3/7 um: Unconditionally call unflatten_device_tree()
In reply to Stephen BoydCall this function unconditionally so that we can populate an empty DTB on platforms that don't boot with a command line provided DTB. There's no harm in calling unflatten_device_tree() unconditionally. If there isn't a valid initial_boot_params dtb then unflatten_device_tree() returns early.
--- arch/um/kernel/dtb.c | 14 +++++++------- drivers/of/unittest.c | 4 ---- 2 files changed, 7 insertions(+), 11 deletions(-)
arch/um/kernel/dtb.c +7 -7
@@ -16,16 +16,16 @@ void uml_dtb_init(void)void *area; area =uml_load_file (dtb, &size); -if (!area) -return ; +if (area) { +if (!early_init_dt_scan (area)) { +pr_err ("invalid DTB %s\n" , dtb); +memblock_free (area, size); +return ; + } -if (!early_init_dt_scan (area)) { -pr_err ("invalid DTB %s\n" , dtb); -memblock_free (area, size); -return ; +early_init_fdt_scan_reserved_mem (); } -early_init_fdt_scan_reserved_mem ();unflatten_device_tree (); }
drivers/of/unittest.c +0 -4
@@ -4075,10 +4075,6 @@ static int __init of_unittest(void)add_taint (TAINT_TEST ,LOCKDEP_STILL_OK );/* adding data for unittest */ - -if (IS_ENABLED (CONFIG_UML )) -unittest_unflatten_overlay_base (); - res =unittest_data_add ();if (res)return res;
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
PATCH v3 4/7 x86/of: Unconditionally call unflatten_and_copy_device_tree()
In reply to Stephen BoydCall this function unconditionally so that we can populate an empty DTB on platforms that don't boot with a firmware provided or builtin DTB. There's no harm in calling unflatten_device_tree() unconditionally here. If there isn't a non-NULL 'initial_boot_params' pointer then unflatten_device_tree() returns early.
--- arch/x86/kernel/devicetree.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-)
arch/x86/kernel/devicetree.c +13 -11
@@ -283,22 +283,24 @@ void __init x86_flattree_get_config(void)u32 size, map_len;void *dt; -if (!initial_dtb) -return ; +if (initial_dtb) { + map_len =max (PAGE_SIZE - (initial_dtb & ~PAGE_MASK ), (u64 )128 ); - map_len =max (PAGE_SIZE - (initial_dtb & ~PAGE_MASK ), (u64 )128 ); + dt =early_memremap (initial_dtb, map_len); + size =fdt_totalsize (dt); +if (map_len < size) { +early_memunmap (dt, map_len); + dt =early_memremap (initial_dtb, size); + map_len = size; + } - dt =early_memremap (initial_dtb, map_len); - size =fdt_totalsize (dt); -if (map_len < size) { -early_memunmap (dt, map_len); - dt =early_memremap (initial_dtb, size); - map_len = size; +early_init_dt_verify (dt); } -early_init_dt_verify (dt);unflatten_and_copy_device_tree (); - early_memunmap(dt, map_len); + +if (initial_dtb) + early_memunmap(dt, map_len); }#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
PATCH v3 5/7 arm64: Unconditionally call unflatten_device_tree()
In reply to Stephen BoydCall this function unconditionally so that we can populate an empty DTB on platforms that don't boot with a firmware provided or builtin DTB. When ACPI is in use, unflatten_device_tree() ignores the 'initial_boot_params' pointer so the live DT on those systems won't be whatever that's pointing to. Similarly, when kexec copies the DT data the previous kernel to the new one on ACPI systems, of_kexec_alloc_and_setup_fdt() will ignore the live DT (the empty root one) and copy the 'initial_boot_params' data.
--- arch/arm64/kernel/setup.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
arch/arm64/kernel/setup.c +1 -2
@@ -351,8 +351,7 @@ void __init __no_sanitize_address setup_arch(char **cmdline_p)/* Parse the ACPI tables for possible boot-time configuration */ acpi_boot_table_init (); -if (acpi_disabled) -unflatten_device_tree (); +unflatten_device_tree ();bootmem_init ();
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
PATCH v3 6/7 of: unittest: treat missing of_root as error instead of fixing up
In reply to Stephen BoydFrom: Frank Rowand <[email protected]>
unflatten_device_tree() now ensures that the 'of_root' node is populated with the root of a default empty devicetree. Remove the unittest code that created 'of_root' if it was missing. Verify that 'of_root' is valid before attempting to attach the testcase-data subtree. Remove the unittest code that unflattens the unittest overlay base if architecture is UML because that is always done now.
--- drivers/of/unittest.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
drivers/of/unittest.c +4 -8
@@ -1732,20 +1732,16 @@ static int __init unittest_data_add(void)return -EINVAL ; } +/* attach the sub-tree to live tree */ if (!of_root) { - of_root = unittest_data_node; -for_each_of_allnodes (np) -__of_attach_node_sysfs (np); - of_aliases =of_find_node_by_path ("/aliases" ); - of_chosen =of_find_node_by_path ("/chosen" ); -of_overlay_mutex_unlock (); -return 0 ; +pr_warn ("%s: no live tree to attach sub-tree\n" , __func__); +kfree (unittest_data); +return -ENODEV ; }EXPECT_BEGIN (KERN_INFO ,"Duplicate name in testcase-data, renamed to \"duplicate-name#1\"" ); -/* attach the sub-tree to live tree */ np = unittest_data_node->child;while (np) {struct device_node *next = np->sibling;
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
PATCH v3 7/7 of: Add KUnit test to confirm DTB is loaded
In reply to Stephen BoydAdd a KUnit test that confirms a DTB has been loaded, i.e. there is a root node, and that the of_have_populated_dt() API works properly.
--- drivers/of/.kunitconfig | 3 +++ drivers/of/Kconfig | 9 ++++++++ drivers/of/Makefile | 2 ++ drivers/of/of_test.c | 48 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+) create mode 100644 drivers/of/.kunitconfig create mode 100644 drivers/of/of_test.c
drivers/of/.kunitconfig new file +3 -0
@@ -0,0 +1,3 @@ +CONFIG_KUNIT=y +CONFIG_OF=y +CONFIG_OF_KUNIT_TEST=y
drivers/of/Kconfig +9 -0
@@ -37,6 +37,15 @@ config OF_UNITTEST If unsure, say N here. This option is not safe to enable. +config OF_KUNIT_TEST + tristate "Devicetree KUnit 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 +2 -0
@@ -19,4 +19,6 @@ obj-y += kexec.o endif endif +obj-$(CONFIG_OF_KUNIT_TEST) += of_test.o + obj-$(CONFIG_OF_UNITTEST) += unittest-data/
drivers/of/of_test.c new file +48 -0
48 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
Re: [PATCH v3 7/7] of: Add KUnit test to confirm DTB is loaded
Quoted 10 lines · Stephen Boyd
Add a KUnit test that confirms a DTB has been loaded, i.e. there is a root node, and that the
of_have_populated_dt()API works properly.Cc Rob Herring <[email protected]>Cc Frank Rowand <[email protected]>Cc David Gow <[email protected]>Cc Brendan Higgins <[email protected]>Signed-off-by Stephen Boyd <[email protected]>---
This looks pretty good to me test-wise, though it still fails on m68k. (Everything else I tried it on works, though I've definitely not tried _every_ architecture.)
aarch64: PASSED
i386: PASSEDx86_64: PASSEDx86_64 KASAN: PASSED
powerpc64: PASSED
UML: PASSED
UML LLVM: PASSED
m68k: FAILED
Quoted 4 lines
$ qemu-system-m68k -nodefaults -m 1024 -kernel .kunit-all-m68k/vmlinux -append 'kunit.enable=1 console=hvc0 kunit_shutdown=reboot' -no-reboot -nographic -serial stdio -machine virt [11:55:05] ===================== dtb (2 subtests) ===================== [11:55:05] # dtb_root_node_found_by_path: EXPECTATION FAILED at drivers/of/of_test.c:18 [11:55:05] Expected np is not null, but is
Quoted 8 lines
[11:55:05] [FAILED] dtb_root_node_found_by_path [11:55:05] # dtb_root_node_populates_of_root: EXPECTATION FAILED at drivers/of/of_test.c:28 [11:55:05] Expected of_root is not null, but is [11:55:05] [FAILED] dtb_root_node_populates_of_root [11:55:05] # module: of_test [11:55:05] # dtb: pass:0 fail:2 skip:0 total:2 [11:55:05] # Totals: pass:0 fail:2 skip:0 total:2 [11:55:05] ======================= [FAILED] dtb =======================
My only other question is about the test names: the mix of 'of' and 'dtb' can be a bit confusing. As is, we have:
kconfig name: OF_KUNIT_TEST
module name: of_test
suite name: dtb
test names: all start with dtb_
Given KUnit only really deals with the suite/test names directly, it's not trivial to see that 'dtb.dtb_*' is controlled by OF_KUNIT_TEST and in of_test if built as a module. (This is getting a bit easier now that we have the 'module' attribute in the output, but still.)
Would 'of_dtb' work as a suite name if it's important to keep both 'of' and 'dtb'?
In general, though, this test looks good to me. Particularly if m68k can be fixed.
Cheers,
-- David
Quoted 110 lines
drivers/of/.kunitconfig | 3 +++ drivers/of/Kconfig | 9 ++++++++ drivers/of/Makefile | 2 ++ drivers/of/of_test.c | 48 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+) create mode 100644 drivers/of/.kunitconfig create mode 100644 drivers/of/of_test.cdrivers/of/.kunitconfig new file +3
-0@@ -0,0 +1,3 @@ +CONFIG_KUNIT=y +CONFIG_OF=y +CONFIG_OF_KUNIT_TEST=ydrivers/of/Kconfig +9
-0@@ -37,6 +37,15 @@ config OF_UNITTEST If unsure, say N here. This option is not safe to enable. +config OF_KUNIT_TEST + tristate "Devicetree KUnit 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_TESTdrivers/of/Makefile +2
-0@@ -19,4 +19,6 @@ obj-y += kexec.o endif endif +obj-$(CONFIG_OF_KUNIT_TEST) += of_test.o + obj-$(CONFIG_OF_UNITTEST) += unittest-data/drivers/of/of_test.c new file +48
-1@@ -0,0 +1,48 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* KUnit tests for OF APIs +*/ +#include <linux/module.h> +#include <linux/of.h> + +#include <kunit/test.h> + +/* +* Test that the root node "/" can be found by path. +*/ +static void dtb_root_node_found_by_path (struct kunit *test) +{ +struct device_node *np; + + np =of_find_node_by_path ("/" ); +KUNIT_EXPECT_NOT_ERR_OR_NULL (test, np); +of_node_put (np); +} + +/* +* Test that the 'of_root' global variable is always populated when DT code is +* enabled. +*/ +static void dtb_root_node_populates_of_root (struct kunit *test) +{ +KUNIT_EXPECT_NOT_ERR_OR_NULL (test, of_root); +} + +static struct kunit_case dtb_test_cases[] = { +KUNIT_CASE (dtb_root_node_found_by_path), +KUNIT_CASE (dtb_root_node_populates_of_root), + {} +}; + +/* +* Test suite to confirm a DTB is loaded. +*/ +static struct kunit_suite dtb_suite = { + .name ="dtb" , + .test_cases = dtb_test_cases, +}; + +kunit_test_suites ( + &dtb_suite, +); +MODULE_LICENSE ("GPL" ); --https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git--
You received this message because you are subscribed to the Google Groups "KUnit Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20240202195909.3458162-8-sboyd%40kernel.org.
Re: [PATCH v3 7/7] of: Add KUnit test to confirm DTB is loaded
Quoting David Gow (2024-02-02 20:10:17)
Quoted 16 lines · Stephen Boyd
Quoted 10 lines
Add a KUnit test that confirms a DTB has been loaded, i.e. there is a root node, and that the
of_have_populated_dt()API works properly.Cc Rob Herring <[email protected]>Cc Frank Rowand <[email protected]>Cc David Gow <[email protected]>Cc Brendan Higgins <[email protected]>Signed-off-by Stephen Boyd <[email protected]>---
This looks pretty good to me test-wise, though it still fails on m68k. (Everything else I tried it on works, though I've definitely not tried _every_ architecture.)
Quoted 20 lines
aarch64: PASSED
i386: PASSEDx86_64: PASSEDx86_64KASAN: PASSED
powerpc64: PASSED
UML: PASSED
UML LLVM: PASSED
m68k: FAILEDQuoted 12 lines
$ qemu-system-m68k -nodefaults -m 1024 -kernel .kunit-all-m68k/vmlinux -append 'kunit.enable=1 console=hvc0 kunit_shutdown=reboot' -no-reboot -nographic -serial stdio -machine virt [11:55:05] ===================== dtb (2 subtests) ===================== [11:55:05] # dtb_root_node_found_by_path: EXPECTATION FAILED at drivers/of/of_test.c:18 [11:55:05] Expected np is not null, but is [11:55:05] [FAILED] dtb_root_node_found_by_path [11:55:05] # dtb_root_node_populates_of_root: EXPECTATION FAILED at drivers/of/of_test.c:28 [11:55:05] Expected of_root is not null, but is [11:55:05] [FAILED] dtb_root_node_populates_of_root [11:55:05] # module: of_test [11:55:05] # dtb: pass:0 fail:2 skip:0 total:2 [11:55:05] # Totals: pass:0 fail:2 skip:0 total:2 [11:55:05] ======================= [FAILED] dtb =======================
Ah yeah I forgot to mention that. m68k fails because it doesn't call the unflatten_(and_copy)?_device_tree() function, so we don't populate a root node on that architecture. One solution would be to make CONFIG_OF unavailable on m68k. Or we have to make sure DT works on any architecture. Rob, what do you prefer here?
Quoted 9 lines
My only other question is about the test names: the mix of 'of' and 'dtb' can be a bit confusing. As is, we have:
kconfig name:OF_KUNIT_TEST
module name:of_test
suite name: dtb
test names: all start withdtb_
Quoted 7 lines
Given KUnit only really deals with the suite/test names directly, it's not trivial to see that 'dtb.dtb_*' is controlled by
OF_KUNIT_TESTand inof_testif built as a module. (This is getting a bit easier now that we have the 'module' attribute in the output, but still.)Would '
of_dtb' work as a suite name if it's important to keep both 'of' and 'dtb'?
Sure, I can add of_ prefix to the tests.
Quoted 6 lines
In general, though, this test looks good to me. Particularly if m68k can be fixed.
Reviewed-by David Gow <[email protected]>
Thanks!
Re: [PATCH v3 7/7] of: Add KUnit test to confirm DTB is loaded
Hi Stephen,
Quoted 37 lines · Stephen Boyd
Quoting David Gow (2024-02-02 20:10:17)
Quoted 15 lines · Stephen Boyd
Quoted 9 lines
Add a KUnit test that confirms a DTB has been loaded, i.e. there is a root node, and that the
of_have_populated_dt()API works properly.Cc Rob Herring <[email protected]>Cc Frank Rowand <[email protected]>Cc David Gow <[email protected]>Cc Brendan Higgins <[email protected]>Signed-off-by Stephen Boyd <[email protected]>---
This looks pretty good to me test-wise, though it still fails on m68k. (Everything else I tried it on works, though I've definitely not tried _every_ architecture.)
Quoted 20 lines
aarch64: PASSED
i386: PASSEDx86_64: PASSEDx86_64KASAN: PASSED
powerpc64: PASSED
UML: PASSED
UML LLVM: PASSED
m68k: FAILEDQuoted 12 lines
$ qemu-system-m68k -nodefaults -m 1024 -kernel .kunit-all-m68k/vmlinux -append 'kunit.enable=1 console=hvc0 kunit_shutdown=reboot' -no-reboot -nographic -serial stdio -machine virt [11:55:05] ===================== dtb (2 subtests) ===================== [11:55:05] # dtb_root_node_found_by_path: EXPECTATION FAILED at drivers/of/of_test.c:18 [11:55:05] Expected np is not null, but is [11:55:05] [FAILED] dtb_root_node_found_by_path [11:55:05] # dtb_root_node_populates_of_root: EXPECTATION FAILED at drivers/of/of_test.c:28 [11:55:05] Expected of_root is not null, but is [11:55:05] [FAILED] dtb_root_node_populates_of_root [11:55:05] # module: of_test [11:55:05] # dtb: pass:0 fail:2 skip:0 total:2 [11:55:05] # Totals: pass:0 fail:2 skip:0 total:2 [11:55:05] ======================= [FAILED] dtb =======================
Quoted 5 lines
Ah yeah I forgot to mention that. m68k fails because it doesn't call the unflatten_(and_copy)?_device_tree() function, so we don't populate a root node on that architecture. One solution would be to make
CONFIG_OFunavailable on m68k. Or we have to make sure DT works on any architecture. Rob, what do you prefer here?
I guess the latter?
Alpha, hexagon, parisc, s390, and sparc are also lacking calls to unflatten.*device_tree().
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] 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 v3 7/7] of: Add KUnit test to confirm DTB is loaded
Quoting Geert Uytterhoeven (2024-02-05 11:55:29)
Quoted 44 lines · Stephen Boyd
Quoted 37 lines
Quoting David Gow (2024-02-02 20:10:17)
Quoted 15 lines · Stephen Boyd
Quoted 9 lines
Add a KUnit test that confirms a DTB has been loaded, i.e. there is a root node, and that the
of_have_populated_dt()API works properly.Cc Rob Herring <[email protected]>Cc Frank Rowand <[email protected]>Cc David Gow <[email protected]>Cc Brendan Higgins <[email protected]>Signed-off-by Stephen Boyd <[email protected]>---
This looks pretty good to me test-wise, though it still fails on m68k. (Everything else I tried it on works, though I've definitely not tried _every_ architecture.)
Quoted 20 lines
aarch64: PASSED
i386: PASSEDx86_64: PASSEDx86_64KASAN: PASSED
powerpc64: PASSED
UML: PASSED
UML LLVM: PASSED
m68k: FAILEDQuoted 12 lines
$ qemu-system-m68k -nodefaults -m 1024 -kernel .kunit-all-m68k/vmlinux -append 'kunit.enable=1 console=hvc0 kunit_shutdown=reboot' -no-reboot -nographic -serial stdio -machine virt [11:55:05] ===================== dtb (2 subtests) ===================== [11:55:05] # dtb_root_node_found_by_path: EXPECTATION FAILED at drivers/of/of_test.c:18 [11:55:05] Expected np is not null, but is [11:55:05] [FAILED] dtb_root_node_found_by_path [11:55:05] # dtb_root_node_populates_of_root: EXPECTATION FAILED at drivers/of/of_test.c:28 [11:55:05] Expected of_root is not null, but is [11:55:05] [FAILED] dtb_root_node_populates_of_root [11:55:05] # module: of_test [11:55:05] # dtb: pass:0 fail:2 skip:0 total:2 [11:55:05] # Totals: pass:0 fail:2 skip:0 total:2 [11:55:05] ======================= [FAILED] dtb =======================Quoted 5 lines
Ah yeah I forgot to mention that. m68k fails because it doesn't call the unflatten_(and_copy)?_device_tree() function, so we don't populate a root node on that architecture. One solution would be to make
CONFIG_OFunavailable on m68k. Or we have to make sure DT works on any architecture. Rob, what do you prefer here?
Quoted 4 lines
I guess the latter?
Alpha, hexagon, parisc, s390, and sparc are also lacking calls to unflatten.*device_tree().
sparc does that on purpose. Perhaps it's simplest to call unflatten_device_tree() if of_root is still NULL after setup_arch() returns.
---8<---
init/main.c +4 -0
@@ -97,6 +97,8 @@#include <linux/jump_label.h> #include <linux/kcsan.h> #include <linux/init_syscalls.h> +#include <linux/of.h> +#include <linux/of_fdt.h> #include <linux/stackdepot.h> #include <linux/randomize_kstack.h> #include <net/net_namespace.h>
@@ -895,6 +897,8 @@ void start_kernel(void)pr_notice ("%s" , linux_banner);early_security_init ();setup_arch (&command_line); +if (!of_root) +unflatten_device_tree ();setup_boot_config ();setup_command_line (command_line);setup_nr_cpu_ids ();
Re: [PATCH v3 7/7] of: Add KUnit test to confirm DTB is loaded
Quoted 70 lines · Stephen Boyd
Quoting Geert Uytterhoeven (2024-02-05 11:55:29)
Quoted 44 lines · Stephen Boyd
Quoted 37 lines
Quoting David Gow (2024-02-02 20:10:17)
Quoted 15 lines · Stephen Boyd
Quoted 9 lines
Add a KUnit test that confirms a DTB has been loaded, i.e. there is a root node, and that the
of_have_populated_dt()API works properly.Cc Rob Herring <[email protected]>Cc Frank Rowand <[email protected]>Cc David Gow <[email protected]>Cc Brendan Higgins <[email protected]>Signed-off-by Stephen Boyd <[email protected]>---
This looks pretty good to me test-wise, though it still fails on m68k. (Everything else I tried it on works, though I've definitely not tried _every_ architecture.)
Quoted 20 lines
aarch64: PASSED
i386: PASSEDx86_64: PASSEDx86_64KASAN: PASSED
powerpc64: PASSED
UML: PASSED
UML LLVM: PASSED
m68k: FAILEDQuoted 12 lines
$ qemu-system-m68k -nodefaults -m 1024 -kernel .kunit-all-m68k/vmlinux -append 'kunit.enable=1 console=hvc0 kunit_shutdown=reboot' -no-reboot -nographic -serial stdio -machine virt [11:55:05] ===================== dtb (2 subtests) ===================== [11:55:05] # dtb_root_node_found_by_path: EXPECTATION FAILED at drivers/of/of_test.c:18 [11:55:05] Expected np is not null, but is [11:55:05] [FAILED] dtb_root_node_found_by_path [11:55:05] # dtb_root_node_populates_of_root: EXPECTATION FAILED at drivers/of/of_test.c:28 [11:55:05] Expected of_root is not null, but is [11:55:05] [FAILED] dtb_root_node_populates_of_root [11:55:05] # module: of_test [11:55:05] # dtb: pass:0 fail:2 skip:0 total:2 [11:55:05] # Totals: pass:0 fail:2 skip:0 total:2 [11:55:05] ======================= [FAILED] dtb =======================Quoted 5 lines
Ah yeah I forgot to mention that. m68k fails because it doesn't call the unflatten_(and_copy)?_device_tree() function, so we don't populate a root node on that architecture. One solution would be to make
CONFIG_OFunavailable on m68k. Or we have to make sure DT works on any architecture. Rob, what do you prefer here?Quoted 4 lines
I guess the latter?
Alpha, hexagon, parisc, s390, and sparc are also lacking calls to unflatten.*device_tree().sparc does that on purpose. Perhaps it's simplest to call
unflatten_device_tree()ifof_rootis still NULL aftersetup_arch()returns.---8<---
init/main.c +2
-0@@ -97,6 +97,8 @@#include <linux/jump_label.h> #include <linux/kcsan.h> #include <linux/init_syscalls.h> +#include <linux/of.h> +#include <linux/of_fdt.h> #include <linux/stackdepot.h> #include <linux/randomize_kstack.h> #include <net/net_namespace.h>
init/main.c void start_kernel(void) · line 900
Quoted 4 lines
init/main.c +1
-0@@ -895,6 +897,8 @@ void start_kernel(void)pr_notice ("%s" , linux_banner);early_security_init ();setup_arch (&command_line); +if (!of_root)
of_root is another thing I'd like to remove direct access to. That check could be inside unflatten_device_tree().
init/main.c void start_kernel(void) · line 901
Quoted 1 line
init/main.c +1
-0+unflatten_device_tree ();
That's back to what Frank had essentially and I wanted to avoid.
I think I'd just disable the tests on the above arches and let them opt-in. I could be convinced otherwise though.
Rob
Re: [PATCH v3 7/7] of: Add KUnit test to confirm DTB is loaded
Quoting Rob Herring (2024-02-13 09:52:00)
Quoted 16 lines · Stephen Boyd
Quoted 15 lines
---8<---
init/main.c +2
-0@@ -97,6 +97,8 @@#include <linux/jump_label.h> #include <linux/kcsan.h> #include <linux/init_syscalls.h> +#include <linux/of.h> +#include <linux/of_fdt.h> #include <linux/stackdepot.h> #include <linux/randomize_kstack.h> #include <net/net_namespace.h>
Quoted 8 lines
init/main.c void start_kernel(void) · line 900
Quoted 5 lines
init/main.c +1
-0@@ -895,6 +897,8 @@ void start_kernel(void)pr_notice ("%s" , linux_banner);early_security_init ();setup_arch (&command_line); +if (!of_root)
of_rootis another thing I'd like to remove direct access to. That check could be insideunflatten_device_tree().
Ok.
Quoted 4 lines
init/main.c void start_kernel(void) · line 901
Quoted 2 lines
init/main.c +1
-0+unflatten_device_tree ();That's back to what Frank had essentially and I wanted to avoid.
Alright, fair enough.
Quoted 4 lines
I think I'd just disable the tests on the above arches and let them opt-in. I could be convinced otherwise though.
Kunit folks would prefer to skip tests when dependencies aren't satisfied. The OF_UNITTEST config already depends on !SPARC so perhaps it's simplest to have tests skip if OF_EARLY_FLATREE=n. Then OF_EARLY_FLATREE can be def_bool OF && !(SPARC || M68K || other arches). The OF_UNITTEST config can depend on OF_EARLY_FLATREE instead of select it then. This way new supporting architectures can remove themselves from the def_bool line when they start calling unflatten_device_tree().