On Thu, Mar 23, 2023 at 03:30:34PM +0800, David Gow wrote:
On Wed, 22 Mar 2023 at 17:06, Matti Vaittinen mazziesaccount@gmail.com wrote:
The creation of a dummy device in order to test managed interfaces is a generally useful test feature. The drm test helpers drm_kunit_helper_alloc_device() and drm_kunit_helper_free_device() are doing exactly this. It makes no sense that each and every component which intends to be testing managed interfaces will create similar helpers so stole these for more generic use.
Signed-off-by: Matti Vaittinen mazziesaccount@gmail.com
Changes: v4 => v5:
- Add accidentally dropped header and email recipients
- do not rename + move helpers from DRM but add temporary dublicates to simplify merging. (This patch does not touch DRM and can be merged separately. DRM patch and IIO test patch still depend on this one).
Please note that there's something similar ongoing in the CCF: https://lore.kernel.org/all/20230302013822.1808711-1-sboyd@kernel.org/
I do like the simplicity of these DRM-originated helpers so I think they're worth. I do equally like the Stephen's idea of having the "dummy platform device" related helpers under drivers/base and the header being in include/kunit/platform_device.h which is similar to real platform device under include/linux/platform_device.h
Thanks for sending this my way.
It's clear we need some way of creating "fake" devices for KUnit tests. Given that there are now (at least) three different drivers looking to use this, we'll ultimately need something which works for everyone.
I think the questions we therefore need to answer are:
- Do we specifically need a platform_device (or, any other specific
device struct), or would any old struct device work? I can see why we would need a platform device for cases where we're testing things like device-tree properties (or, in the future, having e.g. USB-specific helpers which create usb_device). Most tests just use root_device_register() thus far, though.
Any struct device would work, so please do NOT use a platform device.
Use aux devices, or better yet, just a normal virtual device by calling device_create().
- What helpers do we need to make creating, using, and cleaning up
these devices as simple as possible.
Becides what the driver core already provides you? What exactly are you wanting to do here?
I think that in this particular case, we don't actually need a struct platform_device. Replacing these helpers with simple calls to root_device_register() and root_device_unregister() seems to work fine with this patch series for me. (It does break the drm_test_managed_run_action test, though.) So I don't think having these helpers actually help this series at the moment.
Why abuse root_device_*() for something that really is not a root device in the system? Why not use a virtual device? Or better yet, a kunit bus with devices on it that are just for testing? That way you can properly test the bus and driver and device code fully, which is what you are implying is needed here.
That being said, if they used the KUnit resource system to automatically clean up the device when the test is finished (or otherwise exits), that would seem like a real advantage. The clk and drm examples both do this, and I'm hoping to add an API to make it even simpler going forward. (With the work-in-progress API described here[1], the whole thing should boil down to "struct device *dev = root_device_register(name); kunit_defer(root_device_unregister, dev);".)
So, I guess we have three cases we need to look at:
- A test just needs any old struct device. Tests thus far have
hardcoded, or had their own wrappers around root_device_register() for this.
Again, make a kunit bus and devices, that might be "simplest" overall.
- A test needs a device attached to a bus (but doesn't care which
bus). Thus far, people have used struct platform_device for this (see the DRM helpers, which use a platform device for managed resource tests[2]). Maybe the right solution here is something like a struct kunit_device?
Yes.
- A test needs a device attached to a specific bus. We'll probably
need some more detailed faking of that bus. This covers cases like having fake USB devices, devicetree integration, etc.
Have your own bus. No need to mess with USB or any real bus UNLESS you want to actually test those busses, and if so, just create fake USB or clk or whatever devices so that you can test them.
Or just rely on the testing that some busses have for their devices (USB has this today and syzbot knows how to test it), as busses for hardware can be complex and usually require a "real" driver to be written for them to test things.
thanks,
gre gk-h