On 2024-08-27 13:49, Louis Chauvet wrote: > Le 19/08/24 - 16:56, Harry Wentland a écrit : > > [...] > >> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c >> index 3d6785d081f2..3ecda70c2b55 100644 >> --- a/drivers/gpu/drm/vkms/vkms_composer.c >> +++ b/drivers/gpu/drm/vkms/vkms_composer.c >> @@ -435,3 +435,7 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name) > diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c > index 3d6785d081f2..3ecda70c2b55 100644 > --- a/drivers/gpu/drm/vkms/vkms_composer.c > +++ b/drivers/gpu/drm/vkms/vkms_composer.c > @@ -435,3 +435,7 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name) > > return ret; > } > + > +#ifdef CONFIG_DRM_VKMS_KUNIT_TESTS > +#include "tests/vkms_color_tests.c" > +#endif> >> return ret; >> } >> + >> +#ifdef CONFIG_DRM_VKMS_KUNIT_TESTS >> +#include "tests/vkms_color_tests.c" >> +#endif > > This is very strange to include a .c in this file, is it something done a > lot in the kernel? I can find only one occurence of this pattern in the > kernel [1], the other tests are in their own modules. > > In addition it crate many warning during compilations: > warning: symbol 'test_*' was not declared. Should it be static? > > As other tests will be introduced (yuv [2], config [3]), it is maybe > interesting to introduce a new module as [2] is doing? The VISIBLE_IF_KUNIT et al. is much nicer than including a .c file. Thanks for pointing me to them. Will change this. Harry > > [1]: https://elixir.bootlin.com/linux/v6.11-rc5/source/fs/ext4/mballoc.c#L7047 > [2]: https://lore.kernel.org/all/20240809-yuv-v10-14-1a7c764166f7@xxxxxxxxxxx/ > [3]: https://lore.kernel.org/all/20240814-google-remove-crtc-index-from-parameter-v1-15-6e179abf9fd4@xxxxxxxxxxx/ >
