On 3/6/23 06:53, Rob Herring wrote: > On Sat, Mar 4, 2023 at 9:39 AM Frank Rowand <frowand.list@xxxxxxxxx> wrote: >> >> On 3/2/23 17:57, Stephen Boyd wrote: >>> Quoting Rob Herring (2023-03-02 12:18:34) >>>> On Thu, Mar 2, 2023 at 1:44 PM Stephen Boyd <sboyd@xxxxxxxxxx> wrote: >>>>> >>>>> Quoting Rob Herring (2023-03-02 09:13:59) >>>>>> >>>>>> 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'? >>> >>> Sounds good. >>> >>>> >>>>>> 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. >>>> >>> >>> Ok. I suspect we may want to test error paths though so I don't know >> >> Yes, exactly. >> >>> what to do here. For now I'll just leave the bindings in place and >>> change the prefix to "test". >>> >>>> >>>>>> 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. >>> >>> Got it. >>> >>>> >>>>>> 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. >>> >>> Phew! >>> >>>> >>>> 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. >>>> >>> >>> Ok. I think I have some time to try this overlay approach so let me see >>> what is needed. >> >> Please avoid overlays. See my other replies in this thread for why. > > If overlays work for the constrained environment of unit tests, then > use them. If overlays are not to be used, then remove the support from > the kernel. Putting issues in a todo list is not going to get them > done. Having users will. Overlays are not used to enable OF unittests that are unrelated to overlays (to the best of my memory - I reserve the right to be corrected). Overlay usage in OF unittests is specifically to test overlay features. > > Rob
