Hello Maxime, On 12/1/22 11:27, Maxime Ripard wrote: [...] >> >> I wonder if now that this header was moved outside of the tests directory, >> if we should add stub functions in the header file that are just defined >> but do nothing if CONFIG_DRM_KUNIT_TEST isn't enabled. So that including >> it in drivers will be a no-op. >> >> Or do you plan to conditionally include this header file in drivers? So >> that is only included when CONFIG_DRM_KUNIT_TEST is enabled? > > I'm not entirely sure. I'd expect only the tests to include it, and thus > would depend on DRM_KUNIT_TEST already. But we can always add the stubs > if it's ever included in a different context. > >> Another thing that wondered is if we want a different namespace for this >> header, i.e: <drm/testing/drm_kunit_helpers.h>, to make it clear that is >> not part of the DRM API but just for testing helpers. > > If there's a single header, I don't think we need to create the > directory. This is also something we can consolidate later on if needed. > Agree on both. It's better to land as is and then figure out if needs to be changed once other drivers add more tests. >> But these are open questions really, and they can be done as follow-up: >> >> Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> > > Thanks :) You are welcome! -- Best regards, Javier Martinez Canillas Core Platforms Red Hat