Re: [RFC] drm: enable W=1 warnings by default across the subsystem

From: Javier Martinez Canillas <javierm@redhat.com>
To: Maxime Ripard <mripard@kernel.org>, Jani Nikula <jani.nikula@intel.com>
Cc: Pan@freedesktop.org, "Karol Herbst" <kherbst@redhat.com>,
	intel-gfx@lists.freedesktop.org, Xinhui <Xinhui.Pan@amd.com>,
	"Abhinav Kumar" <quic_abhinavk@quicinc.com>,
	dri-devel@lists.freedesktop.org,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Dmitry Baryshkov" <dmitry.baryshkov@linaro.org>,
	"Danilo Krummrich" <dakr@redhat.com>,
	"Hamza Mahfooz" <hamza.mahfooz@amd.com>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Marijn Suijten" <marijn.suijten@somainline.org>,
	"Sean Paul" <sean@poorly.run>,
	"Christian König" <christian.koenig@amd.com>
Subject: Re: [RFC] drm: enable W=1 warnings by default across the subsystem
Date: Thu, 30 Nov 2023 10:30:47 +0100	[thread overview]
Message-ID: <87zfyva8qg.fsf@minerva.mail-host-address-is-not-set> (raw)
In-Reply-To: <eba23oyf4i3j6sdyqljjtgaz27brz4dxfrbtoddestjxwp5lr5@qxfj2b6szigc>

Maxime Ripard <mripard@kernel.org> writes:

> Hi,
>
> On Thu, Nov 30, 2023 at 10:52:17AM +0200, Jani Nikula wrote:
>> On Wed, 29 Nov 2023, Hamza Mahfooz <hamza.mahfooz@amd.com> wrote:
>> > Cc: Nathan Chancellor <nathan@kernel.org>
>> >
>> > 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.
>

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
...

> Maxime

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat