On Mon, Mar 11, 2024 at 9:33 PM Jani Nikula <jani.nikula@xxxxxxxxx> wrote: > > On Fri, 08 Mar 2024, Jani Nikula <jani.nikula@xxxxxxxxx> wrote: > > Ensure drm headers build, are self-contained, have header guards, and > > have no kernel-doc warnings, when CONFIG_DRM_HEADER_TEST=y. > > > > The mechanism follows similar patters used in i915, xe, and usr/include. > > > > To cover include/drm, we need to recurse there using the top level > > Kbuild and the new include/Kbuild files. > > > > Suggested-by: Daniel Vetter <daniel@xxxxxxxx> > > Cc: David Airlie <airlied@xxxxxxxxx> > > Cc: Daniel Vetter <daniel@xxxxxxxx> > > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > > Cc: Maxime Ripard <mripard@xxxxxxxxxx> > > Cc: Thomas Zimmermann <tzimmermann@xxxxxxx> > > Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx> > > Acked-by: Thomas Zimmermann <tzimmermann@xxxxxxx> > > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> > > Masahiro, ack for merging this? Sorry, I did not notice that I was pinged so long ago. I just realized this because I saw it in linux-next. No, NACK. This was already reverted by fcbb8461fd2376ba3782b5b8bd440c929b8e4980 Please do not re-add it. The reason is explained in the discussion linked in that commit description. I do not believe all headers in include/drm/ must be self-contained. If we know that <drm/drm-foo.h> is included only when CONFIG_DRM_FOO is enabled, it does not need to be self-contained when CONFIG_DRM_FOO=n. So, I am skeptical with this approach, where the 'find' command detects all headers and they must be self-contained regardless. The only case I think makes sense is UAPI headers (implemented in usr/include/Makefile) Masahiro > BR, > Jani. > > > --- > > Kbuild | 1 + > > drivers/gpu/drm/Kconfig | 11 +++++++++++ > > drivers/gpu/drm/Makefile | 18 ++++++++++++++++++ > > include/Kbuild | 1 + > > include/drm/Makefile | 18 ++++++++++++++++++ > > 5 files changed, 49 insertions(+) > > create mode 100644 include/Kbuild > > create mode 100644 include/drm/Makefile > > > > diff --git a/Kbuild b/Kbuild > > index 464b34a08f51..f327ca86990c 100644 > > --- a/Kbuild > > +++ b/Kbuild > > @@ -97,3 +97,4 @@ obj-$(CONFIG_SAMPLES) += samples/ > > obj-$(CONFIG_NET) += net/ > > obj-y += virt/ > > obj-y += $(ARCH_DRIVERS) > > +obj-$(CONFIG_DRM_HEADER_TEST) += include/ > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > > index c08e18108c2a..dd17685ef6e7 100644 > > --- a/drivers/gpu/drm/Kconfig > > +++ b/drivers/gpu/drm/Kconfig > > @@ -429,3 +429,14 @@ config DRM_WERROR > > this config option is disabled by default. > > > > If in doubt, say N. > > + > > +config DRM_HEADER_TEST > > + bool "Ensure DRM headers are self-contained and pass kernel-doc" > > + depends on EXPERT > > + default n > > + help > > + Ensure the DRM subsystem headers both under drivers/gpu/drm and > > + include/drm compile, are self-contained, have header guards, and have > > + no kernel-doc warnings. > > + > > + If in doubt, say N. > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > > index a73c04d2d7a3..6605d5686d01 100644 > > --- a/drivers/gpu/drm/Makefile > > +++ b/drivers/gpu/drm/Makefile > > @@ -218,3 +218,21 @@ obj-y += solomon/ > > obj-$(CONFIG_DRM_SPRD) += sprd/ > > obj-$(CONFIG_DRM_LOONGSON) += loongson/ > > obj-$(CONFIG_DRM_POWERVR) += imagination/ > > + > > +# Ensure drm headers are self-contained and pass kernel-doc > > +hdrtest-files := \ > > + $(shell cd $(srctree)/$(src) && find . -maxdepth 1 -name 'drm_*.h') \ > > + $(shell cd $(srctree)/$(src) && find display lib -name '*.h') > > + > > +always-$(CONFIG_DRM_HEADER_TEST) += \ > > + $(patsubst %.h,%.hdrtest, $(hdrtest-files)) > > + > > +# Include the header twice to detect missing include guard. > > +quiet_cmd_hdrtest = HDRTEST $(patsubst %.hdrtest,%.h,$@) > > + cmd_hdrtest = \ > > + $(CC) $(c_flags) -fsyntax-only -x c /dev/null -include $< -include $<; \ > > + $(srctree)/scripts/kernel-doc -none $(if $(CONFIG_DRM_WERROR),-Werror) $<; \ > > + touch $@ > > + > > +$(obj)/%.hdrtest: $(src)/%.h FORCE > > + $(call if_changed_dep,hdrtest) > > diff --git a/include/Kbuild b/include/Kbuild > > new file mode 100644 > > index 000000000000..5e76a599e2dd > > --- /dev/null > > +++ b/include/Kbuild > > @@ -0,0 +1 @@ > > +obj-$(CONFIG_DRM_HEADER_TEST) += drm/ > > diff --git a/include/drm/Makefile b/include/drm/Makefile > > new file mode 100644 > > index 000000000000..b9f391d7aadd > > --- /dev/null > > +++ b/include/drm/Makefile > > @@ -0,0 +1,18 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > + > > +# Ensure drm headers are self-contained and pass kernel-doc > > +hdrtest-files := \ > > + $(shell cd $(srctree)/$(src) && find * -name '*.h' 2>/dev/null) > > + > > +always-$(CONFIG_DRM_HEADER_TEST) += \ > > + $(patsubst %.h,%.hdrtest, $(hdrtest-files)) > > + > > +# Include the header twice to detect missing include guard. > > +quiet_cmd_hdrtest = HDRTEST $(patsubst %.hdrtest,%.h,$@) > > + cmd_hdrtest = \ > > + $(CC) $(c_flags) -fsyntax-only -x c /dev/null -include $< -include $<; \ > > + $(srctree)/scripts/kernel-doc -none $(if $(CONFIG_DRM_WERROR),-Werror) $<; \ > > + touch $@ > > + > > +$(obj)/%.hdrtest: $(src)/%.h FORCE > > + $(call if_changed_dep,hdrtest) > > -- > Jani Nikula, Intel -- Best Regards Masahiro Yamada
