On Tue, 23 Apr 2024 at 07:24, Stephen Boyd sboyd@kernel.org wrote:
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 all build upon each other. Or the DT part can be merged through the DT tree to reduce the dependencies.
Changes from v3 (https://lore.kernel.org/r/20230327222159.3509818-1-sboyd@kernel.org):
- No longer depend on Frank's series[1] because it was merged upstream[2]
- Use kunit_add_action_or_reset() to shorten code
- Skip tests properly when CONFIG_OF_OVERLAY isn't set
Changes from v2 (https://lore.kernel.org/r/20230315183729.2376178-1-sboyd@kernel.org):
- Overlays don't depend on __symbols__ node
- Depend on Frank's always create root node if CONFIG_OF series[1]
- Added kernel-doc to KUnit API doc
- Fixed some kernel-doc on functions
- More test cases for fixed rate clk
Changes from v1 (https://lore.kernel.org/r/20230302013822.1808711-1-sboyd@kernel.org):
- 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
[1] https://lore.kernel.org/r/20230317053415.2254616-1-frowand.list@gmail.com [2] https://lore.kernel.org/r/20240308195737.GA1174908-robh@kernel.org
Thanks very much. I'm about halfway through reviewing these, and I like them a lot so far.
Most of my thoughts are just naming ideas. I fear some of them may be the reverse of previous suggestions, as we've since landed the KUnit device wrappers in include/kunit/device.h, which we decided would live as part of KUnit, not as part of the device infrastructure. I don't enormously mind if we make the opposite decision for these, though it does seem a bit inconsistent if we do 'devices' differently from 'platform_devices'. Thoughts?
The other thing I've noted so far is that the of_apply_kunit_platform_device and of_overlay_apply_kunit_cleanup tests fail (and BUG() with a NULL pointer) on powerpc:
[15:18:51] # of_overlay_apply_kunit_platform_device: EXPECTATION FAILED at drivers/of/overlay_test.c:47 [15:18:51] Expected pdev is not null, but is [15:18:51] BUG: Kernel NULL pointer dereference at 0x0000004c
<...>
[15:18:51] # of_overlay_apply_kunit_platform_device: try faulted: last line seen lib/kunit/resource.c:99 [15:18:51] # of_overlay_apply_kunit_platform_device: internal error occurred preventing test case from running: -4 [15:18:51] [FAILED] of_overlay_apply_kunit_platform_device
[15:18:51] BUG: Kernel NULL pointer dereference at 0x0000004c [15:18:51] note: kunit_try_catch[698] exited with irqs disabled [15:18:51] # of_overlay_apply_kunit_cleanup: try faulted: last line seen drivers/of/overlay_test.c:77 [15:18:51] # of_overlay_apply_kunit_cleanup: internal error occurred preventing test case from running: -4 [15:18:51] [FAILED] of_overlay_apply_kunit_cleanup
I've not had a chance to dig into it any further, yet, but it appears to work on all of the other architectures I tried.
Otherwise, I think this would be fine to take via either the clk or DT and clk trees: there are no conflicts with the current KUnit changes for 6.10. At worst, we might hit some conflicts in the documentation, but there's nothing scheduled yet.
Cheers, -- David
Stephen Boyd (10): 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
Documentation/dev-tools/kunit/api/clk.rst | 10 + Documentation/dev-tools/kunit/api/index.rst | 21 + Documentation/dev-tools/kunit/api/of.rst | 13 + .../dev-tools/kunit/api/platformdevice.rst | 10 + .../bindings/clock/test,clk-parent-data.yaml | 47 ++ .../bindings/test/test,clk-fixed-rate.yaml | 35 ++ .../devicetree/bindings/test/test,empty.yaml | 30 ++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + drivers/base/test/Makefile | 3 + drivers/base/test/platform_kunit-test.c | 140 ++++++ drivers/base/test/platform_kunit.c | 174 +++++++ drivers/clk/.kunitconfig | 2 + drivers/clk/Kconfig | 9 + drivers/clk/Makefile | 9 +- drivers/clk/clk-fixed-rate_test.c | 377 +++++++++++++++ drivers/clk/clk-fixed-rate_test.h | 8 + drivers/clk/clk_kunit.c | 198 ++++++++ drivers/clk/clk_parent_data_test.h | 10 + drivers/clk/clk_test.c | 451 +++++++++++++++++- drivers/clk/kunit_clk_fixed_rate_test.dtso | 19 + drivers/clk/kunit_clk_parent_data_test.dtso | 28 ++ drivers/of/.kunitconfig | 1 + drivers/of/Kconfig | 10 + drivers/of/Makefile | 2 + drivers/of/kunit_overlay_test.dtso | 9 + drivers/of/of_kunit.c | 99 ++++ drivers/of/overlay_test.c | 115 +++++ include/kunit/clk.h | 28 ++ include/kunit/of.h | 94 ++++ include/kunit/platform_device.h | 15 + 30 files changed, 1967 insertions(+), 2 deletions(-) create mode 100644 Documentation/dev-tools/kunit/api/clk.rst create mode 100644 Documentation/dev-tools/kunit/api/of.rst create mode 100644 Documentation/dev-tools/kunit/api/platformdevice.rst create mode 100644 Documentation/devicetree/bindings/clock/test,clk-parent-data.yaml create mode 100644 Documentation/devicetree/bindings/test/test,clk-fixed-rate.yaml create mode 100644 Documentation/devicetree/bindings/test/test,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/kunit_overlay_test.dtso create mode 100644 drivers/of/of_kunit.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: 4cece764965020c22cff7665b18a012006359095
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/ https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git