On 3/2/23 14:18, Rob Herring wrote: > On Thu, Mar 2, 2023 at 1:44 PM Stephen Boyd <sboyd@xxxxxxxxxx> wrote: >> >> Quoting Rob Herring (2023-03-02 09:13:59) >>> On Wed, Mar 1, 2023 at 7:38 PM 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. >>>> >>>> I Cced everyone to all the patches so they get the full context. I'm >>>> hoping I can take the whole pile through the clk tree as they almost all >>>> depend on each other. In the future I imagine it will be easy to add >>>> more test nodes to the clk.dtsi file and not need to go across various >>>> maintainer trees like this series does. >>>> >>>> Stephen Boyd (8): >>>> dt-bindings: Add linux,kunit binding >>>> of: Enable DTB loading on UML for KUnit tests >>>> kunit: Add test managed platform_device/driver APIs >>>> clk: Add test managed clk provider/consumer APIs >>>> dt-bindings: kunit: Add fixed rate clk consumer test >>>> clk: Add KUnit tests for clk fixed rate basic type >>>> dt-bindings: clk: Add KUnit clk_parent_data test >>>> clk: Add KUnit tests for clks registered with struct clk_parent_data >>> >>> Good to see bindings for this. I've been meaning to do something about >>> the DT unittest ones being undocumented, but I hadn't really decided >>> whether it was worth writing schemas for them. The compatibles at >>> least show up with 'make dt_compatible_check'. Perhaps we want to just >>> define some vendor (not 'linux') that's an exception rather than >>> requiring schemas (actually, that already works for 'foo'). >> >> Sure. Maybe "kunit" should be the vendor prefix? Or "dtbunit"? > > We'd want to use the same thing on the DT unittests or anything else > potentially. How about just 'test'? > >>> It's >>> likely that we want test DTs that fail normal checks and schemas get >>> in the way of that as we don't have a way to turn off checks. >> >> Having the schemas is nice to make sure tests that are expecting some >> binding are actually getting that. But supporting broken bindings is >> also important to test any error paths in functions that parse >> properties. Maybe we keep the schema and have it enforce that incorrect >> properties are being set? > > I wasn't suggesting throwing them out. More why I hadn't written any I guess. > >> Do we really need to test incorrect bindings? Doesn't the >> dt_bindings_check catch these problems so we don't have to write DTB >> verifiers in the kernel? > > Fair enough. Using my frequently stated position against me. :) > > I do have a secret plan to implement (debug) type checks into the > of_property_* APIs by extracting the type information from schemas > into C. > > >>> We already have GPIO tests in the DT unittests, so why is clocks >>> different? Or should the GPIO tests be moved out (yes, please!)? >> >> Ah I didn't notice the GPIO tests in there. There are i2c tests too, >> right? All I can say is clks are using kunit, that's the difference ;-) > > Yeah, they should perhaps all move to the subsystems. > >>> What happens when/if the DT unittest is converted to kunit? I think >>> that would look confusing from the naming. My initial thought is >>> 'kunit' should be dropped from the naming of a lot of this. Note that >>> the original kunit submission converted the DT unittests. I would >>> still like to see that happen. Frank disagreed over what's a unit test >>> or not, then agreed, then didn't... I don't really care. If there's a >>> framework to use, then we should use it IMO. >> >> Honestly I don't want to get involved in migrating the existing DT >> unittest code to kunit. I'm aware that it was attempted years ago when >> kunit was introduced. Maybe if the overlay route works well enough I can >> completely sidestep introducing any code in drivers/of/ besides some >> kunit wrappers for this. I'll cross my fingers! > > Yeah, I wasn't expecting you to. I just want to make sure this meshes > with any future conversion to kunit. > > There's also some plans to always populate the DT root node if not > present. That may help here. Or not. There's been a few versions > posted with Frank's in the last week or 2. As noted in that thread, by code inspection (not actual testing) I think that the patch series breaks DT unittest for UML. It should be a trivial change in the next patch version to fix. > > Rob
