Hi Maxime, all
On 3/23/23 12:21, Greg Kroah-Hartman wrote:
On Thu, Mar 23, 2023 at 11:12:16AM +0100, Maxime Ripard wrote:
On Wed, Mar 22, 2023 at 07:57:10PM +0100, Greg Kroah-Hartman wrote:
+/**
- 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.
Any device can access devm_ code, there's no need for it to be a platform device at all.
Sure but the resources are only released if the device is part of a bus, so it can't be a root_device (or bare device) either
The resources are not cleaned up when the device is freed no matter if it's on a bus or not? If so, then that's a bug that needs to be fixed, and tested :)
This is strange. I just ran a test on a beaglebone black using Linux 6.3.0-rc2 + the IIO patches we se here (but the IIO test patch modified to use the root_device_register() and root_device_unregister().
I passed the device pointer from root_device_register() to the devm_iio_init_iio_gts()
// snip dev = root_device_register(IIO_GTS_TEST_DEV); KUNIT_EXPECT_NOT_ERR_OR_NULL(test, dev); if (IS_ERR_OR_NULL(dev)) return NULL;
ret = devm_iio_init_iio_gts(dev, TEST_SCALE_1X, 0, g_table, num_g, i_table, num_i, gts);
- and saw the tables for available scales allocated:
if (gts.num_avail_all_scales) pr_info("GTS: table allocation succeeded\n"); else pr_info("GTS: table allocation failed\n");
pr_info("gts: num_avail_all_scales %d\n", gts.num_avail_all_scales);
(this printed: [ 52.132966] # Subtest: iio-gain-time-scale [ 52.132982] 1..7 [ 52.157455] GTS: table allocation succeeded [ 52.164077] gts: num_avail_all_scales 16
Next I unregister the root-device and check if the unwinding code which frees the tables and zeroes the scale count was ran:
root_device_unregister(dev); pr_info("gts: num_avail_all_scales %d\n", gts.num_avail_all_scales);
if (gts.num_avail_all_scales) pr_info("devm unwinding not done\n"); else pr_info("devm unwinding succeeded\n");
Which printed: [ 52.168101] gts: num_avail_all_scales 0 [ 52.171957] devm unwinding succeeded
I can send patch(es) just for testing this on other machines if someone want's to see if the lack of devm unwinding is somehow architecture specific (which sounds very strange to me) - although using this IIO series just for checking the unwinding is a bit of an overkill. I just happened to have these tests at my hands / in my tree for testing.
In any case, devm unwinding using root_device_[un]register() seems to "work on my machine".
Naxime, what was the environment where you observed lack of unwinding? (Huh, I am so afraid of sending this post out - I've experienced too many "Oh, boy - how I didn't notice THAT" moments in the past and maybe I am again overlooking something...)
Yours, -- Matti