From: Stephen Boyd <sboyd@kernel.org>
To: David Gow <davidgow@google.com>
Cc: Rob Herring <robh+dt@kernel.org>,
linux-kernel@vger.kernel.org, patches@lists.linux.dev,
linux-um@lists.infradead.org,
linux-arm-kernel@lists.infradead.org, kunit-dev@googlegroups.com,
linux-kselftest@vger.kernel.org, devicetree@vger.kernel.org,
Frank Rowand <frowand.list@gmail.com>,
Brendan Higgins <brendan.higgins@linux.dev>
Subject: Re: [PATCH v3 7/7] of: Add KUnit test to confirm DTB is loaded
Date: Mon, 05 Feb 2024 11:19:01 -0800 [thread overview]
Message-ID: <89892ecd6b1b043db58258705c32b02b.sboyd@kernel.org> (raw)
In-Reply-To: <CABVgOS=A8BQ6HHpBKFqg-N10ckk2XYavaS-MPXvZ0wenrVm=1g@mail.gmail.com>
Quoting David Gow (2024-02-02 20:10:17)
> On Sat, 3 Feb 2024 at 03:59, Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > 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 <robh+dt@kernel.org>
> > Cc: Frank Rowand <frowand.list@gmail.com>
> > Cc: David Gow <davidgow@google.com>
> > Cc: Brendan Higgins <brendan.higgins@linux.dev>
> > Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> > ---
>
> 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: PASSED
> x86_64: PASSED
> x86_64 KASAN: PASSED
> powerpc64: PASSED
> UML: PASSED
> UML LLVM: PASSED
> m68k: FAILED
> > $ 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?
>
>
> 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'?
Sure, I can add of_ prefix to the tests.
>
> In general, though, this test looks good to me. Particularly if m68k
> can be fixed.
>
> Reviewed-by: David Gow <davidgow@google.com>
>
Thanks!
WARNING: multiple messages have this Message-ID (diff)
From: Stephen Boyd <sboyd@kernel.org>
To: David Gow <davidgow@google.com>
Cc: Rob Herring <robh+dt@kernel.org>,
linux-kernel@vger.kernel.org, patches@lists.linux.dev,
linux-um@lists.infradead.org,
linux-arm-kernel@lists.infradead.org, kunit-dev@googlegroups.com,
linux-kselftest@vger.kernel.org, devicetree@vger.kernel.org,
Frank Rowand <frowand.list@gmail.com>,
Brendan Higgins <brendan.higgins@linux.dev>
Subject: Re: [PATCH v3 7/7] of: Add KUnit test to confirm DTB is loaded
Date: Mon, 05 Feb 2024 11:19:01 -0800 [thread overview]
Message-ID: <89892ecd6b1b043db58258705c32b02b.sboyd@kernel.org> (raw)
In-Reply-To: <CABVgOS=A8BQ6HHpBKFqg-N10ckk2XYavaS-MPXvZ0wenrVm=1g@mail.gmail.com>
Quoting David Gow (2024-02-02 20:10:17)
> On Sat, 3 Feb 2024 at 03:59, Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > 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 <robh+dt@kernel.org>
> > Cc: Frank Rowand <frowand.list@gmail.com>
> > Cc: David Gow <davidgow@google.com>
> > Cc: Brendan Higgins <brendan.higgins@linux.dev>
> > Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> > ---
>
> 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: PASSED
> x86_64: PASSED
> x86_64 KASAN: PASSED
> powerpc64: PASSED
> UML: PASSED
> UML LLVM: PASSED
> m68k: FAILED
> > $ 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?
>
>
> 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'?
Sure, I can add of_ prefix to the tests.
>
> In general, though, this test looks good to me. Particularly if m68k
> can be fixed.
>
> Reviewed-by: David Gow <davidgow@google.com>
>
Thanks!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2024-02-05 19:19 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-02 19:59 [PATCH v3 0/7] of: populate of_root node if bootloader doesn't Stephen Boyd
2024-02-02 19:59 ` Stephen Boyd
2024-02-02 19:59 ` [PATCH v3 1/7] of: Always unflatten in unflatten_and_copy_device_tree() Stephen Boyd
2024-02-02 19:59 ` Stephen Boyd
2024-02-02 19:59 ` [PATCH v3 2/7] of: Create of_root if no dtb provided by firmware Stephen Boyd
2024-02-02 19:59 ` Stephen Boyd
2024-02-02 19:59 ` [PATCH v3 3/7] um: Unconditionally call unflatten_device_tree() Stephen Boyd
2024-02-02 19:59 ` Stephen Boyd
2024-02-02 19:59 ` [PATCH v3 4/7] x86/of: Unconditionally call unflatten_and_copy_device_tree() Stephen Boyd
2024-02-02 19:59 ` Stephen Boyd
2024-02-02 19:59 ` [PATCH v3 5/7] arm64: Unconditionally call unflatten_device_tree() Stephen Boyd
2024-02-02 19:59 ` Stephen Boyd
2024-02-02 19:59 ` [PATCH v3 6/7] of: unittest: treat missing of_root as error instead of fixing up Stephen Boyd
2024-02-02 19:59 ` Stephen Boyd
2024-02-02 19:59 ` [PATCH v3 7/7] of: Add KUnit test to confirm DTB is loaded Stephen Boyd
2024-02-02 19:59 ` Stephen Boyd
2024-02-03 4:10 ` David Gow
2024-02-03 4:10 ` David Gow
2024-02-05 19:19 ` Stephen Boyd [this message]
2024-02-05 19:19 ` Stephen Boyd
2024-02-05 19:55 ` Geert Uytterhoeven
2024-02-05 19:55 ` Geert Uytterhoeven
2024-02-10 2:59 ` Stephen Boyd
2024-02-10 2:59 ` Stephen Boyd
2024-02-13 17:52 ` Rob Herring
2024-02-13 17:52 ` Rob Herring
2024-02-15 21:57 ` Stephen Boyd
2024-02-15 21:57 ` Stephen Boyd
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=89892ecd6b1b043db58258705c32b02b.sboyd@kernel.org \
--to=sboyd@kernel.org \
--cc=brendan.higgins@linux.dev \
--cc=davidgow@google.com \
--cc=devicetree@vger.kernel.org \
--cc=frowand.list@gmail.com \
--cc=kunit-dev@googlegroups.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-um@lists.infradead.org \
--cc=patches@lists.linux.dev \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.