On 3/24/23 08:34, David Gow wrote:
On Fri, 24 Mar 2023 at 14:11, Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
On 3/23/23 18:36, Maxime Ripard wrote:
On Thu, Mar 23, 2023 at 03:02:03PM +0200, Matti Vaittinen wrote:
On 3/23/23 14:29, Maxime Ripard wrote:
On Thu, Mar 23, 2023 at 02:16:52PM +0200, Matti Vaittinen wrote:
Ok. Fair enough. Besides, if the root-device was sufficient - then I
would actually not see the need for a helper. People could in that case
directly use the root_device_register(). So, if helpers are provided
they should be backed up by a device with a bus then.
I think there is _some_ value in helpers even without a bus, but it's
much more limited:
- It's less confusing if KUnit test devices are using kunit labelled
structs and functions.
- Helpers could use KUnit's resource management API to ensure any
device created is properly unregistered and removed when the test
exits (particularly if it exits early due to, e.g., an assertion).
Ah. That's true. Being able to abort the test on error w/o being forced
to do a clean-up dance for the dummy device would be convenient.
I've played around implementing those with a proper struct
kunit_device and the automatic cleanup on test failure, and thus far
it -- like root_device_register -- works for all of the tests except
the drm-test-managed one.
So if we really wanted to, we could use KUnit-specific helpers for
just those tests which currently work with root_device_register(), but
if we're going to try to implement a KUnit bus -- which I think is at
least worth investigating -- I'd rather not either hold up otherwise
good tests on helper development, or rush a helper out only to have to
change it a lot when we see exactly what the bus implementation would
look like.
It's easy for me to agree.
As I said, in my very specific IIO related test the test device does not
need a bus. Hence I'll drop the 'generic helpers' from this series.
I think that sounds like a good strategy for now, and we can work on a
set of 'generic helpers' which have an associated bus and struct
kunit_device in the meantime. If we can continue to use
root_device_register until those are ready, that'd be very convenient.
Would it be a tiny bit more acceptable if we did add a very simple:
#define kunit_root_device_register(name) root_device_register(name)
#define kunit_root_device_unregister(dev) root_device_unregister(dev)
to include/kunit/device.h (or somesuch)
This should help us later to at least spot the places where
root_device_[un]register() is abused and (potentially mass-)covert them
to use the proper helpers when they're available.
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
