Hi David & Greg and thanks for working with this!
On 3/28/23 15:45, David Gow wrote:
Thanks, Gred and Matti.
On Mon, 27 Mar 2023 at 20:38, Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Mon, Mar 27, 2023 at 03:20:06PM +0300, Matti Vaittinen wrote:
On 3/27/23 15:01, Greg Kroah-Hartman wrote:
On Mon, Mar 27, 2023 at 02:34:02PM +0300, Matti Vaittinen wrote:
I'm happy to keep working on this, but would definitely appreciate your feedback.
I've put my work-in-progress code here: https://kunit.googlesource.com/linux/+/refs/heads/kunit/device-helpers%5E%21...
It creates a "kunit" bus, and adds a few helpers to create both devices and drivers on that bus, and clean them up when the test exits. It seems to work on all of the tests which used root_device_register so far (except those -- only test_iio_find_closest_gain_low so far -- which create multiple devices with the same name, as the driver name won't be unique),
I wouldn't worry about it for as long as it's just because an iio-gts test does something silly. Those tests are currently only in my personal playground and changing those tests should be pretty trivial.
And right after saying that - the test_iio_find_closest_gain_low test does
a) register a 'test' device b) perform test on devm_ API c) unregister the 'test' device
d) register a 'test' device (same name as at step a) e) perform test on devm_ API f) unregister the 'test' device
My assumption is that the test device would be gone after step c) because there should be no references to it anywhere. Hence, I wonder why registering at step d) fails? (Or did I misunderstand something?)
and the drm tests work fine when ported to it as well.
There's still a lot of cleanup to do and questions which need answering, including:
- Working out how best to provide an owning module (it's currently
just kunit, but probably should be the module which contains the actual tests)
Maybe there is something I am not seeing but how about wrapping the kunit_device_register() in a macro and getting the THIS_MODULE in caller's context?
In any case, does this seem like the right way forward?
I am by no means an expert on this but this does look good to me. I would keep this as clean, lean and simple as possible in order to keep understanding / debugging the problems exposed by the tests as simple as possible. At some point someone is wondering why a test fails, and ends up looking through these helpers to ensure problem is no lurking there... Hence, I'd kept the code there in minimum - meaning, I might not add kunit class or even a driver until tests require that. (Even if it would not look as good in the sysfs - as far as I understand the kunit sysfs entries are a 'test feature' which should not be present in 'production systems'. This is not an excuse to make things bad - but (in my opinion) this is a good reason to prioritize simplicity.
Anyways, thanks for the work!
Yours, -- Matti