Hi Frank, On Sun, Mar 5, 2023 at 4:33 AM Frank Rowand <frowand.list@xxxxxxxxx> wrote: > On 3/2/23 13:47, Geert Uytterhoeven wrote: > > On Thu, Mar 2, 2023 at 8:28 PM Stephen Boyd <sboyd@xxxxxxxxxx> wrote: > >> Quoting Rob Herring (2023-03-02 09:32:09) > >>> On Thu, Mar 2, 2023 at 2:14 AM David Gow <davidgow@xxxxxxxxxx> wrote: > >>>> On Thu, 2 Mar 2023 at 09:38, Stephen Boyd <sboyd@xxxxxxxxxx> wrote: > >>>>> This patch series adds unit tests for the clk fixed rate basic type and > >>>>> the clk registration functions that use struct clk_parent_data. To get > >>>>> there, we add support for loading a DTB into the UML kernel that's > >>>>> running the unit tests along with probing platform drivers to bind to > >>>>> device nodes specified in DT. > >>>>> > >>>>> With this series, we're able to exercise some of the code in the common > >>>>> clk framework that uses devicetree lookups to find parents and the fixed > >>>>> rate clk code that scans devicetree directly and creates clks. Please > >>>>> review. > >>>>> > >>>> > >>>> Thanks Stephen -- this is really neat! > >>>> > >>>> This works well here, and I love all of the tests for the > >>>> KUnit/device-tree integration as well. > >>>> > >>>> I'm still looking through the details of it (alas, I've mostly lived > >>>> in x86-land, so my device-tree knowledge is, uh, spotty to say the > >>>> least), but apart from possibly renaming some things or similarly > >>>> minor tweaks, I've not got any real suggestions thus far. > >>>> > >>>> I do wonder whether we'll want, on the KUnit side, to have some way of > >>>> supporting KUnit device trees on non-UML architecctures (e.g., if we > >>>> need to test something architecture-specific, or on a big-endian > >>>> platform, etc), but I think that's a question for the future, rather > >>>> than something that affects this series. > >>> > >>> I'll say that's a requirement. We should be able to structure the > >>> tests to not interfere with the running system's DT. The DT unittest > >>> does that. > >> > >> That could be another choice in the unit test choice menu. > >> CONFIG_OF_KUNIT_NOT_UML that injects some built-in DTB overlay on an > >> architecture that wants to run tests. > > > > As long as you use compatible values that don't exist elsewhere, > > and don't overwrite anything, you can load your kunit test overlays > > on any running system that has DT support. > > > >>> As a side topic, Is anyone looking at getting UML to work on arm64? > >>> It's surprising how much x86 stuff there is which is I guess one > >>> reason it hasn't happened. > >> > >> I've no idea but it would be nice indeed. > > > > I believe that's non-trivial. At least for arm32 (I didn't have any arm64 > > systems last time I asked the experts). > > > >>>> Similarly, I wonder if there's something we could do with device tree > >>>> overlays, in order to make it possible for tests to swap nodes in and > >>>> out for testing. > >>> > >>> Yes, that's how the DT unittest works. But it is pretty much one big > >>> overlay (ignoring the overlay tests). It could probably be more > >>> modular where it is apply overlay, test, remove overlay, repeat. > >> > >> I didn't want to rely on the overlay code to inject DT nodes. Having > >> tests written for the fake KUnit machine is simple. It closely matches > >> how clk code probes the DTB and how nodes are created and populated on > >> the platform bus as devices. CLK_OF_DECLARE() would need the overlay to > >> be applied early too, which doesn't happen otherwise as far as I know. > > > > Don't all generic clock drivers also create a platform driver? > > At least drivers/clk/clk-fixed-factor.c does. > > > >> But perhaps this design is too much of an end-to-end test and not a unit > >> test? In the spirit of unit testing we shouldn't care about how the node > >> is added to the live devicetree, just that there is a devicetree at all. > >> > >> Supporting overlays to more easily test combinations sounds like a good > >> idea. Probably some kunit_*() prefixed functions could be used to > >> apply a test managed overlay and automatically remove it when the test > >> is over would work. The clk registration tests could use this API to > >> inject an overlay and then manually call the of_platform_populate() > >> function to create the platform device(s). The overlay could be built in > >> drivers/clk/ too and then probably some macroish function can find the > >> blob and apply it. > > > > No need to manually call of_platform_populate() to create the > > platform devices. That is taken care of automatically when applying > > an overlay. > > > >> Is there some way to delete the platform devices that we populate from > >> the overlay? I'd like the tests to be hermetic. > > > Removing the overlay will delete the platform devices. > > I _think_ that is incorrect. Do you have a pointer to the overlay code that > deletes the device? (If I remember correctly, the overlay remove code does not > even check whether the device exists and whether a driver is bound to it -- but > this is on my todo list to look into.) https://elixir.bootlin.com/linux/latest/source/drivers/of/platform.c#L769 > > All of that works if you have your own code to apply a DT overlay. > > The recent fw_devlinks patches did cause some regressions, cfr. > > https://lore.kernel.org/all/CAMuHMdXEnSD4rRJ-o90x4OprUacN_rJgyo8x6=9F9rZ+-KzjOg@xxxxxxxxxxxxxx Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
