[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZCGFgypeuJXqNwQt@kroah.com>
Date: Mon, 27 Mar 2023 14:01:07 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
Brendan Higgins <brendan.higgins@...ux.dev>,
David Gow <davidgow@...gle.com>, linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com,
Stephen Boyd <sboyd@...nel.org>,
Jonathan Cameron <jic23@...nel.org>, linux-iio@...r.kernel.org
Subject: Re: [PATCH v6 3/7] kunit: Add kunit wrappers for (root) device
creation
On Mon, Mar 27, 2023 at 02:34:02PM +0300, Matti Vaittinen wrote:
> A few tests need to have a valid struct device. One such example is
> tests which want to be testing devm-managed interfaces.
>
> Add kunit wrapper for root_device_[un]register(), which create a root
> device and also add a kunit managed clean-up routine for the device
> destruction upon test exit.
I really do not like this as a "root device" is a horrible hack and
should only be used if you have to hang other devices off of it and you
don't have a real device to tie those devices to.
Here you are abusing it and attempting to treat it as a real device,
which it is not at all, because:
> Special note: In some cases the device reference-count does not reach
> zero and devm-unwinding is not done if device is not sitting on a bus.
> The root_device_[un]register() are dealing with such devices and thus
> this interface may not be usable by all in its current form. More
> information can be found from:
> https://lore.kernel.org/dri-devel/20221117165311.vovrc7usy4efiytl@houat/
See, not a real device, doesn't follow normal "struct device" rules and
lifetimes, don't try to use it for a test as it will only cause problems
and you will be forced to work around that in a test.
Do the right thing here, create a fake bus and add devices to it.
Heck, I'll even write that code if you want it, what's the requirement,
something like:
struct device *kunit_device_create(struct kunit *test, const char *name);
void kunit_device_destroy(struct device *dev);
?
Why do you want a "match" function? You don't provide documentation
here for it so I have no idea.
Anything else needed?
> The use of root-devices in the kunit helpers is intended to be an
> intermediate solution to allow tests which do not require device to sit
> on a bus avoid directly abusing the root_device_[un]register() while
> proper kunit device solution is being worked on. Related discussion can be
> found from:
> https://lore.kernel.org/lkml/CABVgOSmx3A4Vwos2_8xO-XQrQAw5gvY0nc5zLpLmcJ7FtA-dTQ@mail.gmail.com/
Again, no, please let's not get this wrong now and say "we will fix this
later" as that's not how kernel development should work...
thanks,
greg k-h
Powered by blists - more mailing lists