Re: [PATCH v5 1/8] drivers: kunit: Generic helpers for test device creation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/23/23 12:27, Greg Kroah-Hartman wrote:
On Thu, Mar 23, 2023 at 12:01:15PM +0200, Matti Vaittinen wrote:
On 3/23/23 10:58, Greg Kroah-Hartman wrote:
On Thu, Mar 23, 2023 at 07:17:40AM +0000, Vaittinen, Matti wrote:
On 3/22/23 20:57, Greg Kroah-Hartman wrote:
On Wed, Mar 22, 2023 at 03:48:00PM +0200, Matti Vaittinen wrote:
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:

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.

I agree, let's use a virtual device and a virtual bus (you can use the
auxbus code for this as that's all there for this type of thing)

Hm. The auxiliary_devices require parent. What would be the best way to
deal with that in KUnit tests?

If you use NULL as the parent, it goes into the root.

As far as I read this is not the case with auxiliary devices. Judging the
docs they were intended to be representing some part of a (parent) device. I
see the auxiliary_device_init() has explicit check for parent being
populated:

int auxiliary_device_init(struct auxiliary_device *auxdev)
{
         struct device *dev = &auxdev->dev;

         if (!dev->parent) {
                 pr_err("auxiliary_device has a NULL dev->parent\n");
                 return -EINVAL;
         }

Yes as it wants to "split" a device up into smaller devices.  So make a
real device that it can hang off of.

Yep. This is what led me to the root_device_register()... :rolleyes: And seein the root-device alone could do what I need - adding auxiliary device on top of it just for the sake of adding one seems a bit of an over-engineering to me :)

As I wrote in another mail, I thought of using a root_device for this IIO
test as was suggested by David. To tell the truth, implementing a kunit bus
device is starting to feel a bit overwhelming... I started just adding a
driver for a light sensor, ended up adding a helper for IIO gain-time-scale
conversions and I am slightly reluctant to going the extra-extra mile of
adding some UT infrastructure in the context of this driver work...

I think it is worth it as the driver core has no tests.  So it obviously
must be correct, right?  :)

Doh. Greg, I hate you :) How could one argue with something like this? I think I will submit the v6 with the root_device_register() due to the aux-device requiring it in any case. I know that will end up to your table still as IIO is going through your hands anyways.

I will however take a look at what Maxime said about devm unwinding not being done w/o a bus because I think I saw the unwinding done in these IIO tests even when using the root_device_register() root_device_unregister(). If the unwinding really is not done, then I will come back to this auxiliary device rehearsal

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux