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: >>>> +/** >>>> + * 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. > >> 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? > then you can test the devm_* code, _AND_ you can test the driver core at > the same time. > >> And having them in drivers/base/test seemed like a correct >> place to me. What I really don't know is if there are legitimate use-cases >> for using platform_devices in DRM tests. Perhaps Maxime can shed light on >> that. > > I agree that this could be in drivers/base/test/ but then let's test the > driver core, not just provide a dummy platform device. > > If you want to test the platform driver/device api, that would be great > too, that can be plaform device/driver specific, but don't use one for > some other random driver core functionality please. I am very conservative what comes to adding unit tests due to the huge inertia they add to any further development. I usually only add tests to APIs which I know won't require changing (I don't know such in-kernel APIs) - or to functions which I think are error-prone. So, I am probably one of the last persons adding UTs to code I don't know :) Yours, -- Matti -- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
