On Thu, Nov 30, 2023 at 11:46:00AM +0200, Jani Nikula wrote: > 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. Yeah, I agree. Plus, if we need to have an extra Kconfig option, it's pretty equivalent to using W=1 Maxime
Attachment:
signature.asc
Description: PGP signature
