Hi Greg,
Thanks for looking at this.
On 3/22/23 14:07, Greg Kroah-Hartman wrote:
On Wed, Mar 22, 2023 at 11:05:55AM +0200, Matti Vaittinen wrote:
--- /dev/null +++ b/drivers/base/test/test_kunit_device.c @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- These helpers have been extracted from drm test code at
- drm_kunit_helpers.c which was authored by
- Maxime Ripard maxime@cerno.tech
- */
+#include <linux/device.h> +#include <linux/platform_device.h>
+#include <kunit/platform_device.h>
+#define KUNIT_DEVICE_NAME "test-kunit-mock-device"
+static int fake_probe(struct platform_device *pdev)
Please do not abuse platform devices and drivers for things that are not actually platform devices and drivers.
+{
- return 0;
+}
+static int fake_remove(struct platform_device *pdev) +{
- return 0;
+}
+static struct platform_driver fake_platform_driver = {
- .probe = fake_probe,
- .remove = fake_remove,
- .driver = {
.name = KUNIT_DEVICE_NAME,- },
+};
Why do you need this fake platform driver at all?
Why not just use a virtual device?
I can only answer on my behalf. In my case the answer to why I used platform_devices is practicality. I wanted to test devm_ APIs using KUnit tests and I was pointed to an existing implementation in DRM (seen in these patches). It didn't seem to make any sense to re-invent the wheel by writing another implementation for the existing in-tree functionality.
Maybe Maxime had a better reason to go with the platform devices.
+/**
- test_kunit_helper_alloc_device - Allocate a mock device for a KUnit test
- @test: The test context object
- This allocates a fake struct &device to create a mock for a KUnit
- test. The device will also be bound to a fake driver. It will thus be
- able to leverage the usual infrastructure and most notably the
- device-managed resources just like a "real" device.
What specific "usual infrastructure" are you wanting to access here?
And again, if you want a fake device, make a virtual one, by just calling device_create().
Or are you wanting to do "more" with that device pointer than device_create() can give you?
Personally, I was (am) only interested in devm_ unwinding. I guess the device_create(), device_add(), device_remove()... (didn't study this sequence in details so sorry if there is errors) could've been sufficient for me. I haven't looked how much of the code that there is for 'platform devices' should be duplicated to support that sequence for testability purposes.
The biggest thing for me is that I don't like the idea of creating own 'test device' in <add subsystem here> while we already have some in DRM (or others). Thus, I do see value in adding generic helpers for supporting running KUnit tests on devm_* APIs. Hence it'd be good to have _some_ support for it. And having them in drivers/base/test seemed like a correct place to me. What I really don't know is if there are legitimate use-cases for using platform_devices in DRM tests. Perhaps Maxime can shed light on that.
Yours, -- Matti