On Thu, 30 Nov 2023, Javier Martinez Canillas <javierm@xxxxxxxxxx> wrote: > Maxime Ripard <mripard@xxxxxxxxxx> writes: > >> Hi, >> >> On Thu, Nov 30, 2023 at 10:52:17AM +0200, Jani Nikula wrote: >>> On Wed, 29 Nov 2023, Hamza Mahfooz <hamza.mahfooz@xxxxxxx> wrote: >>> > Cc: Nathan Chancellor <nathan@xxxxxxxxxx> >>> > >>> > On 11/29/23 13:12, Jani Nikula wrote: >>> >> At least the i915 and amd drivers enable a bunch more compiler warnings >>> >> than the kernel defaults. >>> >> >>> >> Extend the W=1 warnings to the entire drm subsystem by default. Use the >>> >> copy-pasted warnings from scripts/Makefile.extrawarn with >>> >> s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and keep >>> >> up with them in the future. >>> >> >>> >> This is similar to the approach currently used in i915. >>> >> >>> >> Some of the -Wextra warnings do need to be disabled, just like in >>> >> Makefile.extrawarn, but take care to not disable them for W=2 or W=3 >>> >> builds, depending on the warning. >>> > >>> > I think this should go in after drm-misc-next has a clean build (for >>> > COMPILE_TEST builds) with this patch applied. Otherwise, it will break a >>> > lot of build configs. >>> >>> Oh, I'm absolutely not suggesting this should be merged before known >>> warnings have been addressed one way or another. Either by fixing them >>> or by disabling said warning in driver local Makefiles, depending on the >>> case. >> >> I'm all for it, but yeah, we need some easy way to opt-in/opt-out. Some >> drivers are pretty much unmaintained now and are likely to never fix >> those warnings. Then I'd go for enabling in drm level and disabling individual warnings in the driver Makefile level if they won't get fixed. > Maybe add a Kconfig symbol for it instead of making unconditional? > > Something like: > > +# Unconditionally enable W=1 warnings locally > +# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn > +subdir-ccflags-$(CONFIG_DRM_EXTRA_CHECKS) += -Wextra -Wunused -Wno-unused-parameter > ... Then we'll have a ping pong of people not using W=1 or CONFIG_DRM_EXTRA_CHECKS introducing warnings, and people using them fixing the warnings... I really do think making it unconditional is the only way. BR, Jani. -- Jani Nikula, Intel
