[PATCH 00/10] -Wunused-const-variable warning fixes

[PATCH 00/10] -Wunused-const-variable warning fixes

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Most patches I sent during the previous kernel cycle have made it in, these 
ten still remain for the moment. Please apply.

Arnd Bergmann (10):
  drm/imagination: avoid unused-const-variable warning
  [v2] clocksource: atmel_tcb: fix kconfig dependency
  [v2] Input: stmpe-ts - use module alias instead of device table
  [RESEND] mux: adg792a: remove incorrect of_match_ptr annotation
  [RESEND] sched: open-code max_rt_runtime definition
  [RESEND] lockdep: change 'static const' variables to enum values
  [RESEND] ARM: fixmap: make __end_of_early_ioremap_region an enum value
  [RESEND 2] comedi: ni_atmio: avoid warning for unused device_ids[] table
  [RESEND 2] apm-emulation: hide an unused variable
  [RESEND 3] dma/contiguous: avoid warning about unused size_bytes

 arch/arm/include/asm/fixmap.h              |  9 ++++-----
 drivers/char/apm-emulation.c               |  5 ++---
 drivers/clocksource/Kconfig                |  4 ++--
 drivers/comedi/drivers/ni_atmio.c          |  2 +-
 drivers/gpu/drm/imagination/pvr_fw_trace.c |  8 ++++----
 drivers/gpu/drm/imagination/pvr_fw_trace.h |  2 --
 drivers/input/touchscreen/stmpe-ts.c       |  7 +------
 drivers/mux/adg792a.c                      |  2 +-
 kernel/dma/contiguous.c                    |  3 +--
 kernel/locking/lockdep_internals.h         | 18 ++++++++++--------
 kernel/sched/rt.c                          |  6 ++----
 11 files changed, 28 insertions(+), 38 deletions(-)

-- 
2.39.5

Cc: Russell King <linux@armlinux.org.uk> (maintainer:ARM PORT)
Cc: Jiri Kosina <jikos@kernel.org> (maintainer:APM DRIVER)
Cc: Arnd Bergmann <arnd@arndb.de> (maintainer:CHAR and MISC DRIVERS,commit_signer:2/2=100%,authored:2/2=100%,added_lines:8/8=100%,removed_lines:10/10=100%,commit_signer:2/3=67%,authored:2/3=67%,added_lines:2/11=18%,removed_lines:2/2=100%,commit_signer:2/4=50%,blamed_fixes:1/1=100%)
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> (maintainer:CHAR and MISC DRIVERS,commit_signer:1/3=33%)
Cc: Daniel Lezcano <daniel.lezcano@linaro.org> (maintainer:CLOCKSOURCE, CLOCKEVENT DRIVERS)
Cc: Thomas Gleixner <tglx@linutronix.de> (maintainer:CLOCKSOURCE, CLOCKEVENT DRIVERS)
Cc: Ian Abbott <abbotti@mev.co.uk> (maintainer:COMEDI DRIVERS,commit_signer:1/3=33%,authored:1/3=33%,added_lines:9/11=82%)
Cc: H Hartley Sweeten <hsweeten@visionengravers.com> (maintainer:COMEDI DRIVERS)
Cc: Frank Binns <frank.binns@imgtec.com> (maintainer:IMGTEC POWERVR DRM DRIVER)
Cc: Matt Coster <matt.coster@imgtec.com> (maintainer:IMGTEC POWERVR DRM DRIVER)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (maintainer:DRM DRIVERS AND MISC GPU PATCHES)
Cc: Maxime Ripard <mripard@kernel.org> (maintainer:DRM DRIVERS AND MISC GPU PATCHES)
Cc: Thomas Zimmermann <tzimmermann@suse.de> (maintainer:DRM DRIVERS AND MISC GPU PATCHES)
Cc: David Airlie <airlied@gmail.com> (maintainer:DRM DRIVERS)
Cc: Simona Vetter <simona@ffwll.ch> (maintainer:DRM DRIVERS)
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> (maintainer:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,commit_signer:2/4=50%)
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> (maintainer:ARM/STM32 ARCHITECTURE)
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com> (maintainer:ARM/STM32 ARCHITECTURE)
Cc: Peter Rosin <peda@axentia.se> (maintainer:MULTIPLEXER SUBSYSTEM)
Cc: Marek Szyprowski <m.szyprowski@samsung.com> (maintainer:DMA MAPPING HELPERS,blamed_fixes:1/1=100%)
Cc: Robin Murphy <robin.murphy@arm.com> (reviewer:DMA MAPPING HELPERS)
Cc: Peter Zijlstra <peterz@infradead.org> (maintainer:LOCKING PRIMITIVES)
Cc: Ingo Molnar <mingo@redhat.com> (maintainer:LOCKING PRIMITIVES)
Cc: Will Deacon <will@kernel.org> (maintainer:LOCKING PRIMITIVES)
Cc: Boqun Feng <boqun.feng@gmail.com> (maintainer:LOCKING PRIMITIVES)
Cc: Waiman Long <longman@redhat.com> (reviewer:LOCKING PRIMITIVES)
Cc: Juri Lelli <juri.lelli@redhat.com> (maintainer:SCHEDULER)
Cc: Vincent Guittot <vincent.guittot@linaro.org> (maintainer:SCHEDULER)
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com> (reviewer:SCHEDULER)
Cc: Steven Rostedt <rostedt@goodmis.org> (reviewer:SCHEDULER)
Cc: Ben Segall <bsegall@google.com> (reviewer:SCHEDULER)
Cc: Mel Gorman <mgorman@suse.de> (reviewer:SCHEDULER)
Cc: Valentin Schneider <vschneid@redhat.com> (reviewer:SCHEDULER)
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> (commit_signer:2/4=50%,authored:2/4=50%,added_lines:2/5=40%,removed_lines:2/5=40%)
Cc: linux-arm-kernel@lists.infradead.org (moderated list:ARM PORT)
Cc: linux-kernel@vger.kernel.org (open list)
Cc: dri-devel@lists.freedesktop.org (open list:DRM DRIVERS)
Cc: linux-input@vger.kernel.org (open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...)
Cc: linux-stm32@st-md-mailman.stormreply.com (moderated list:ARM/STM32 ARCHITECTURE)
Cc: iommu@lists.linux.dev (open list:DMA MAPPING HELPERS)

[PATCH 01/10] drm/imagination: avoid unused-const-variable warning

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

When CONFIG_DEBUG_FS is disabled, the stid_fmts[] array is not referenced
anywhere, causing a W=1 warning with gcc:

In file included from drivers/gpu/drm/imagination/pvr_fw_trace.c:7:
drivers/gpu/drm/imagination/pvr_rogue_fwif_sf.h:75:39: error: 'stid_fmts' defined but not used [-Werror=unused-const-variable=]
   75 | static const struct rogue_km_stid_fmt stid_fmts[] = {
      |                                       ^~~~~~~~~

Rather than adding more #ifdef blocks, address this by changing the
existing #ifdef into equivalent IS_ENABLED() checks so gcc can see
where the symbol is used but still eliminate it from the object file.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpu/drm/imagination/pvr_fw_trace.c | 8 ++++----
 drivers/gpu/drm/imagination/pvr_fw_trace.h | 2 --
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_fw_trace.c b/drivers/gpu/drm/imagination/pvr_fw_trace.c
index 5dbb636d7d4f..93269299d6a4 100644
--- a/drivers/gpu/drm/imagination/pvr_fw_trace.c
+++ b/drivers/gpu/drm/imagination/pvr_fw_trace.c
@@ -122,8 +122,6 @@ void pvr_fw_trace_fini(struct pvr_device *pvr_dev)
 	pvr_fw_object_unmap_and_destroy(fw_trace->tracebuf_ctrl_obj);
 }
 
-#if defined(CONFIG_DEBUG_FS)
-
 /**
  * update_logtype() - Send KCCB command to trigger FW to update logtype
  * @pvr_dev: Target PowerVR device
@@ -447,7 +445,7 @@ static const struct file_operations pvr_fw_trace_fops = {
 void
 pvr_fw_trace_mask_update(struct pvr_device *pvr_dev, u32 old_mask, u32 new_mask)
 {
-	if (old_mask != new_mask)
+	if (IS_ENABLED(CONFIG_DEBUG_FS) && old_mask != new_mask)
 		update_logtype(pvr_dev, new_mask);
 }
 
@@ -457,6 +455,9 @@ pvr_fw_trace_debugfs_init(struct pvr_device *pvr_dev, struct dentry *dir)
 	struct pvr_fw_trace *fw_trace = &pvr_dev->fw_dev.fw_trace;
 	u32 thread_nr;
 
+	if (!IS_ENABLED(CONFIG_DEBUG_FS))
+		return;
+
 	static_assert(ARRAY_SIZE(fw_trace->buffers) <= 10,
 		      "The filename buffer is only large enough for a single-digit thread count");
 
@@ -469,4 +470,3 @@ pvr_fw_trace_debugfs_init(struct pvr_device *pvr_dev, struct dentry *dir)
 				    &pvr_fw_trace_fops);
 	}
 }
-#endif
diff --git a/drivers/gpu/drm/imagination/pvr_fw_trace.h b/drivers/gpu/drm/imagination/pvr_fw_trace.h
index 0074d2b18da0..1d0ef937427a 100644
--- a/drivers/gpu/drm/imagination/pvr_fw_trace.h
+++ b/drivers/gpu/drm/imagination/pvr_fw_trace.h
@@ -65,7 +65,6 @@ struct pvr_fw_trace {
 int pvr_fw_trace_init(struct pvr_device *pvr_dev);
 void pvr_fw_trace_fini(struct pvr_device *pvr_dev);
 
-#if defined(CONFIG_DEBUG_FS)
 /* Forward declaration from <linux/dcache.h>. */
 struct dentry;
 
@@ -73,6 +72,5 @@ void pvr_fw_trace_mask_update(struct pvr_device *pvr_dev, u32 old_mask,
 			      u32 new_mask);
 
 void pvr_fw_trace_debugfs_init(struct pvr_device *pvr_dev, struct dentry *dir);
-#endif /* defined(CONFIG_DEBUG_FS) */
 
 #endif /* PVR_FW_TRACE_H */
-- 
2.39.5

[PATCH 02/10] [v2] clocksource: atmel_tcb: fix kconfig dependency

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Build-testing this driver on arm without CONFIG_OF produces a warning:

drivers/clocksource/timer-atmel-tcb.c:368:34: error: 'atmel_tcb_of_match' defined but not used [-Werror=unused-const-variable=]
  368 | static const struct of_device_id atmel_tcb_of_match[] = {
      |                                  ^~~~~~~~~~~~~~~~~~

Change the dependency to allow build-testing on all architectures but
instead require CONFIG_OF to be present.

Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: fix build regression from dropped CONFIG_ARM dependency
---
 drivers/clocksource/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 487c85259967..a7d5a465100e 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -437,8 +437,8 @@ config ATMEL_ST
 
 config ATMEL_TCB_CLKSRC
 	bool "Atmel TC Block timer driver" if COMPILE_TEST
-	depends on ARM && HAS_IOMEM
-	select TIMER_OF if OF
+	depends on ARM && OF && HAS_IOMEM
+	select TIMER_OF
 	help
 	  Support for Timer Counter Blocks on Atmel SoCs.
 
-- 
2.39.5

Re: [PATCH 02/10] [v2] clocksource: atmel_tcb: fix kconfig dependency

[Alexandre Belloni]

On 09/04/2025 14:22:54+0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Build-testing this driver on arm without CONFIG_OF produces a warning:
> 
> drivers/clocksource/timer-atmel-tcb.c:368:34: error: 'atmel_tcb_of_match' defined but not used [-Werror=unused-const-variable=]
>   368 | static const struct of_device_id atmel_tcb_of_match[] = {
>       |                                  ^~~~~~~~~~~~~~~~~~
> 
> Change the dependency to allow build-testing on all architectures but
> instead require CONFIG_OF to be present.
> 
> Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
> v2: fix build regression from dropped CONFIG_ARM dependency
> ---
>  drivers/clocksource/Kconfig | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 487c85259967..a7d5a465100e 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -437,8 +437,8 @@ config ATMEL_ST
>  
>  config ATMEL_TCB_CLKSRC
>  	bool "Atmel TC Block timer driver" if COMPILE_TEST
> -	depends on ARM && HAS_IOMEM
> -	select TIMER_OF if OF
> +	depends on ARM && OF && HAS_IOMEM
> +	select TIMER_OF
>  	help
>  	  Support for Timer Counter Blocks on Atmel SoCs.
>  
> -- 
> 2.39.5
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 02/10] [v2] clocksource: atmel_tcb: fix kconfig dependency

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 02:22:54PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Build-testing this driver on arm without CONFIG_OF produces a warning:
> 
> drivers/clocksource/timer-atmel-tcb.c:368:34: error: 'atmel_tcb_of_match' defined but not used [-Werror=unused-const-variable=]
>   368 | static const struct of_device_id atmel_tcb_of_match[] = {
>       |                                  ^~~~~~~~~~~~~~~~~~
> 
> Change the dependency to allow build-testing on all architectures but
> instead require CONFIG_OF to be present.

I;m a bit puzzled, maybe due to my misunderstanding of the first line...

>  config ATMEL_TCB_CLKSRC
>  	bool "Atmel TC Block timer driver" if COMPILE_TEST

...^^^ of the dependencies.

> -	depends on ARM && HAS_IOMEM
> -	select TIMER_OF if OF
> +	depends on ARM && OF && HAS_IOMEM

But doesn't this still mean that "all architectures" (from the commit message)
do not apply here (it's all about ARM)?

> +	select TIMER_OF

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 02/10] [v2] clocksource: atmel_tcb: fix kconfig dependency

[Arnd Bergmann]

On Wed, Apr 9, 2025, at 17:30, Andy Shevchenko wrote:
> On Wed, Apr 09, 2025 at 02:22:54PM +0200, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> Build-testing this driver on arm without CONFIG_OF produces a warning:
>> 
>> drivers/clocksource/timer-atmel-tcb.c:368:34: error: 'atmel_tcb_of_match' defined but not used [-Werror=unused-const-variable=]
>>   368 | static const struct of_device_id atmel_tcb_of_match[] = {
>>       |                                  ^~~~~~~~~~~~~~~~~~
>> 
>> Change the dependency to allow build-testing on all architectures but
>> instead require CONFIG_OF to be present.
>
> I;m a bit puzzled, maybe due to my misunderstanding of the first line...
>
>>  config ATMEL_TCB_CLKSRC
>>  	bool "Atmel TC Block timer driver" if COMPILE_TEST
>
> ...^^^ of the dependencies.
>
>> -	depends on ARM && HAS_IOMEM
>> -	select TIMER_OF if OF
>> +	depends on ARM && OF && HAS_IOMEM
>
> But doesn't this still mean that "all architectures" (from the commit message)
> do not apply here (it's all about ARM)?

You are right, my mistake:

v1 of the patch allowed building on all architectures, but that
turned out to run into other bugs, so I changed the contents but
forgot to change the changelog.

I've sent a v3 patch now.

      Arnd

[PATCH 03/10] [v2] Input: stmpe-ts - use module alias instead of device table

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

When compile tested with W=1 on x86_64 with driver as built-in:

  stmpe-ts.c:371:34: error: unused variable 'stmpe_ts_ids' [-Werror,-Wunused-const-variable]

Ideally this would be referenced from the platform_driver, but since
the compatible string is already matched by the mfd driver for its
parent device, that would break probing.

In this case, the of_device_id table just serves as a module alias
for loading the driver, while the device itself is probed using
the platform device name.

Remove the table and instead use a module alias that reflects how
the driver is actually probed.

Link: https://lore.kernel.org/lkml/20240403080702.3509288-8-arnd@kernel.org/
Link: https://lore.kernel.org/lkml/181dbdb8-c050-4966-8cb4-2f39495ff3f9@app.fastmail.com/
Cc: Uwe Kleine-König" <u.kleine-koenig@baylibre.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: use a different approach, hopefully this makes more sense.
---
 drivers/input/touchscreen/stmpe-ts.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/stmpe-ts.c b/drivers/input/touchscreen/stmpe-ts.c
index a94a1997f96b..af0fb38bcfdc 100644
--- a/drivers/input/touchscreen/stmpe-ts.c
+++ b/drivers/input/touchscreen/stmpe-ts.c
@@ -366,12 +366,7 @@ static struct platform_driver stmpe_ts_driver = {
 };
 module_platform_driver(stmpe_ts_driver);
 
-static const struct of_device_id stmpe_ts_ids[] = {
-	{ .compatible = "st,stmpe-ts", },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, stmpe_ts_ids);
-
+MODULE_ALIAS("platform:stmpe-ts");
 MODULE_AUTHOR("Luotao Fu <l.fu@pengutronix.de>");
 MODULE_DESCRIPTION("STMPEXXX touchscreen driver");
 MODULE_LICENSE("GPL");
-- 
2.39.5

Re: [PATCH 03/10] [v2] Input: stmpe-ts - use module alias instead of device table

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 02:22:55PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> When compile tested with W=1 on x86_64 with driver as built-in:
> 
>   stmpe-ts.c:371:34: error: unused variable 'stmpe_ts_ids' [-Werror,-Wunused-const-variable]
> 
> Ideally this would be referenced from the platform_driver, but since
> the compatible string is already matched by the mfd driver for its
> parent device, that would break probing.
> 
> In this case, the of_device_id table just serves as a module alias
> for loading the driver, while the device itself is probed using
> the platform device name.
> 
> Remove the table and instead use a module alias that reflects how
> the driver is actually probed.

...

> +MODULE_ALIAS("platform:stmpe-ts");

Isn't the preferable way to have platform device ID table instead?
Krzysztof?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 03/10] [v2] Input: stmpe-ts - use module alias instead of device table

[Krzysztof Kozlowski]

On 09/04/2025 17:33, Andy Shevchenko wrote:
> On Wed, Apr 09, 2025 at 02:22:55PM +0200, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> When compile tested with W=1 on x86_64 with driver as built-in:
>>
>>   stmpe-ts.c:371:34: error: unused variable 'stmpe_ts_ids' [-Werror,-Wunused-const-variable]
>>
>> Ideally this would be referenced from the platform_driver, but since
>> the compatible string is already matched by the mfd driver for its
>> parent device, that would break probing.
>>
>> In this case, the of_device_id table just serves as a module alias
>> for loading the driver, while the device itself is probed using
>> the platform device name.
>>
>> Remove the table and instead use a module alias that reflects how
>> the driver is actually probed.
> 
> ...
> 
>> +MODULE_ALIAS("platform:stmpe-ts");
> 
> Isn't the preferable way to have platform device ID table instead?
> Krzysztof?

There is no ID table currently and I (usually) object for having both:
ID tables and aliases. In this case it is just simpler to have alias
than entire table and MODULE_DEVICE_TABLE(). Also it is unlikely it will
grow into many aliases. If it tries, then I will object.


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH 04/10] [RESEND] mux: adg792a: remove incorrect of_match_ptr annotation

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Building with W=1 shows a warning about adg792a_of_match being unused
when CONFIG_OF is disabled:

drivers/mux/adg792a.c:134:34: error: unused variable 'adg792a_of_match' [-Werror,-Wunused-const-variable]

Acked-by: Peter Rosin <peda@axentia.se>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/mux/adg792a.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mux/adg792a.c b/drivers/mux/adg792a.c
index 4da5aecb9fc6..a5afe29e3cf1 100644
--- a/drivers/mux/adg792a.c
+++ b/drivers/mux/adg792a.c
@@ -141,7 +141,7 @@ MODULE_DEVICE_TABLE(of, adg792a_of_match);
 static struct i2c_driver adg792a_driver = {
 	.driver		= {
 		.name		= "adg792a",
-		.of_match_table = of_match_ptr(adg792a_of_match),
+		.of_match_table = adg792a_of_match,
 	},
 	.probe		= adg792a_probe,
 	.id_table	= adg792a_id,
-- 
2.39.5

Re: [PATCH 04/10] [RESEND] mux: adg792a: remove incorrect of_match_ptr annotation

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 02:22:56PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Building with W=1 shows a warning about adg792a_of_match being unused
> when CONFIG_OF is disabled:
> 
> drivers/mux/adg792a.c:134:34: error: unused variable 'adg792a_of_match' [-Werror,-Wunused-const-variable]

Yes, please, somebody apply this, I have got 3+ LKP emails about this in the
past.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: (subset) [PATCH 04/10] [RESEND] mux: adg792a: remove incorrect of_match_ptr annotation

[Krzysztof Kozlowski]

On Wed, 09 Apr 2025 14:22:56 +0200, Arnd Bergmann wrote:
> Building with W=1 shows a warning about adg792a_of_match being unused
> when CONFIG_OF is disabled:
> 
> drivers/mux/adg792a.c:134:34: error: unused variable 'adg792a_of_match' [-Werror,-Wunused-const-variable]
> 
> 

Applied, thanks!

[04/10] mux: adg792a: remove incorrect of_match_ptr annotation
        https://git.kernel.org/krzk/linux/c/7ea3876af994e4fd8065af0314bce6619e73667a

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

[PATCH 05/10] [RESEND] sched: open-code max_rt_runtime definition

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Building with clang using "make W=1" normally produces warnings about unused
const variables, but clang does not warn about those in header files.
In case of rt.c it also doesn't warn because this gets included by
from build_policy.c.

Enabling the same warning on gcc also warns about it in included files,
so we end up with a warning when all users of max_rt_runtime are
hidden because both CONFIG_RT_GROUP_SCHED and CONFIG_SYSCTL are
disabled:

kernel/sched/rt.c:9:18: error: 'max_rt_runtime' defined but not used [-Werror=unused-const-variable]

It's not possible to avoid the warning by moving max_rt_runtime
into one of the #ifdef sections, but since it is itself defined
as MAX_BW, simply open-coding that is an easy workaround.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 kernel/sched/rt.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index fa03ec3ed56a..89edb24e63ea 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -5,8 +5,6 @@
  */
 
 int sched_rr_timeslice = RR_TIMESLICE;
-/* More than 4 hours if BW_SHIFT equals 20. */
-static const u64 max_rt_runtime = MAX_BW;
 
 /*
  * period over which we measure -rt task CPU usage in us.
@@ -2778,7 +2776,7 @@ static int tg_set_rt_bandwidth(struct task_group *tg,
 	/*
 	 * Bound quota to defend quota against overflow during bandwidth shift.
 	 */
-	if (rt_runtime != RUNTIME_INF && rt_runtime > max_rt_runtime)
+	if (rt_runtime != RUNTIME_INF && rt_runtime > MAX_BW)
 		return -EINVAL;
 
 	mutex_lock(&rt_constraints_mutex);
@@ -2890,7 +2888,7 @@ static int sched_rt_global_validate(void)
 	if ((sysctl_sched_rt_runtime != RUNTIME_INF) &&
 		((sysctl_sched_rt_runtime > sysctl_sched_rt_period) ||
 		 ((u64)sysctl_sched_rt_runtime *
-			NSEC_PER_USEC > max_rt_runtime)))
+			NSEC_PER_USEC > MAX_BW)))
 		return -EINVAL;
 
 	return 0;
-- 
2.39.5

[PATCH 06/10] [RESEND] lockdep: change 'static const' variables to enum values

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

gcc warns about 'static const' variables even in headers when building
with -Wunused-const-variables enabled:

In file included from kernel/locking/lockdep_proc.c:25:
kernel/locking/lockdep_internals.h:69:28: error: 'LOCKF_USED_IN_IRQ_READ' defined but not used [-Werror=unused-const-variable=]
   69 | static const unsigned long LOCKF_USED_IN_IRQ_READ =
      |                            ^~~~~~~~~~~~~~~~~~~~~~
kernel/locking/lockdep_internals.h:63:28: error: 'LOCKF_ENABLED_IRQ_READ' defined but not used [-Werror=unused-const-variable=]
   63 | static const unsigned long LOCKF_ENABLED_IRQ_READ =
      |                            ^~~~~~~~~~~~~~~~~~~~~~
kernel/locking/lockdep_internals.h:57:28: error: 'LOCKF_USED_IN_IRQ' defined but not used [-Werror=unused-const-variable=]
   57 | static const unsigned long LOCKF_USED_IN_IRQ =
      |                            ^~~~~~~~~~~~~~~~~
kernel/locking/lockdep_internals.h:51:28: error: 'LOCKF_ENABLED_IRQ' defined but not used [-Werror=unused-const-variable=]
   51 | static const unsigned long LOCKF_ENABLED_IRQ =
      |                            ^~~~~~~~~~~~~~~~~

This one is easy to avoid by changing the generated constant definition
into an equivalent enum.

Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 kernel/locking/lockdep_internals.h | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h
index 20f9ef58d3d0..8b4cd6540e49 100644
--- a/kernel/locking/lockdep_internals.h
+++ b/kernel/locking/lockdep_internals.h
@@ -47,29 +47,31 @@ enum {
 	__LOCKF(USED_READ)
 };
 
+enum {
 #define LOCKDEP_STATE(__STATE)	LOCKF_ENABLED_##__STATE |
-static const unsigned long LOCKF_ENABLED_IRQ =
+	LOCKF_ENABLED_IRQ =
 #include "lockdep_states.h"
-	0;
+	0,
 #undef LOCKDEP_STATE
 
 #define LOCKDEP_STATE(__STATE)	LOCKF_USED_IN_##__STATE |
-static const unsigned long LOCKF_USED_IN_IRQ =
+	LOCKF_USED_IN_IRQ =
 #include "lockdep_states.h"
-	0;
+	0,
 #undef LOCKDEP_STATE
 
 #define LOCKDEP_STATE(__STATE)	LOCKF_ENABLED_##__STATE##_READ |
-static const unsigned long LOCKF_ENABLED_IRQ_READ =
+	LOCKF_ENABLED_IRQ_READ =
 #include "lockdep_states.h"
-	0;
+	0,
 #undef LOCKDEP_STATE
 
 #define LOCKDEP_STATE(__STATE)	LOCKF_USED_IN_##__STATE##_READ |
-static const unsigned long LOCKF_USED_IN_IRQ_READ =
+	LOCKF_USED_IN_IRQ_READ =
 #include "lockdep_states.h"
-	0;
+	0,
 #undef LOCKDEP_STATE
+};
 
 #define LOCKF_ENABLED_IRQ_ALL (LOCKF_ENABLED_IRQ | LOCKF_ENABLED_IRQ_READ)
 #define LOCKF_USED_IN_IRQ_ALL (LOCKF_USED_IN_IRQ | LOCKF_USED_IN_IRQ_READ)
-- 
2.39.5

Re: [PATCH 06/10] [RESEND] lockdep: change 'static const' variables to enum values

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 02:22:58PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> gcc warns about 'static const' variables even in headers when building
> with -Wunused-const-variables enabled:
> 
> In file included from kernel/locking/lockdep_proc.c:25:
> kernel/locking/lockdep_internals.h:69:28: error: 'LOCKF_USED_IN_IRQ_READ' defined but not used [-Werror=unused-const-variable=]
>    69 | static const unsigned long LOCKF_USED_IN_IRQ_READ =
>       |                            ^~~~~~~~~~~~~~~~~~~~~~
> kernel/locking/lockdep_internals.h:63:28: error: 'LOCKF_ENABLED_IRQ_READ' defined but not used [-Werror=unused-const-variable=]
>    63 | static const unsigned long LOCKF_ENABLED_IRQ_READ =
>       |                            ^~~~~~~~~~~~~~~~~~~~~~
> kernel/locking/lockdep_internals.h:57:28: error: 'LOCKF_USED_IN_IRQ' defined but not used [-Werror=unused-const-variable=]
>    57 | static const unsigned long LOCKF_USED_IN_IRQ =
>       |                            ^~~~~~~~~~~~~~~~~
> kernel/locking/lockdep_internals.h:51:28: error: 'LOCKF_ENABLED_IRQ' defined but not used [-Werror=unused-const-variable=]
>    51 | static const unsigned long LOCKF_ENABLED_IRQ =
>       |                            ^~~~~~~~~~~~~~~~~
> 
> This one is easy to avoid by changing the generated constant definition
> into an equivalent enum.

Any news here, please? The problem still exists in v6.15-rc4.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 06/10] [RESEND] lockdep: change 'static const' variables to enum values

[Boqun Feng]

On Fri, May 02, 2025 at 05:02:54PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 09, 2025 at 02:22:58PM +0200, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > gcc warns about 'static const' variables even in headers when building
> > with -Wunused-const-variables enabled:
> > 
> > In file included from kernel/locking/lockdep_proc.c:25:
> > kernel/locking/lockdep_internals.h:69:28: error: 'LOCKF_USED_IN_IRQ_READ' defined but not used [-Werror=unused-const-variable=]
> >    69 | static const unsigned long LOCKF_USED_IN_IRQ_READ =
> >       |                            ^~~~~~~~~~~~~~~~~~~~~~
> > kernel/locking/lockdep_internals.h:63:28: error: 'LOCKF_ENABLED_IRQ_READ' defined but not used [-Werror=unused-const-variable=]
> >    63 | static const unsigned long LOCKF_ENABLED_IRQ_READ =
> >       |                            ^~~~~~~~~~~~~~~~~~~~~~
> > kernel/locking/lockdep_internals.h:57:28: error: 'LOCKF_USED_IN_IRQ' defined but not used [-Werror=unused-const-variable=]
> >    57 | static const unsigned long LOCKF_USED_IN_IRQ =
> >       |                            ^~~~~~~~~~~~~~~~~
> > kernel/locking/lockdep_internals.h:51:28: error: 'LOCKF_ENABLED_IRQ' defined but not used [-Werror=unused-const-variable=]
> >    51 | static const unsigned long LOCKF_ENABLED_IRQ =
> >       |                            ^~~~~~~~~~~~~~~~~
> > 
> > This one is easy to avoid by changing the generated constant definition
> > into an equivalent enum.
> 
> Any news here, please? The problem still exists in v6.15-rc4.
> 

Queued for v6.17, thanks!

Regards,
Boqun

> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

[tip: locking/core] locking/lockdep: Change 'static const' variables to enum values

[tip-bot2 for Arnd Bergmann]

The following commit has been merged into the locking/core branch of tip:

Commit-ID:     bd27cfb58c2803923702cd80289b35b7b8108859
Gitweb:        https://git.kernel.org/tip/bd27cfb58c2803923702cd80289b35b7b8108859
Author:        Arnd Bergmann <arnd@arndb.de>
AuthorDate:    Wed, 09 Apr 2025 14:22:58 +02:00
Committer:     Boqun Feng <boqun.feng@gmail.com>
CommitterDate: Mon, 14 Jul 2025 21:57:29 -07:00

locking/lockdep: Change 'static const' variables to enum values

gcc warns about 'static const' variables even in headers when building
with -Wunused-const-variables enabled:

In file included from kernel/locking/lockdep_proc.c:25:
kernel/locking/lockdep_internals.h:69:28: error: 'LOCKF_USED_IN_IRQ_READ' defined but not used [-Werror=unused-const-variable=]
   69 | static const unsigned long LOCKF_USED_IN_IRQ_READ =
      |                            ^~~~~~~~~~~~~~~~~~~~~~
kernel/locking/lockdep_internals.h:63:28: error: 'LOCKF_ENABLED_IRQ_READ' defined but not used [-Werror=unused-const-variable=]
   63 | static const unsigned long LOCKF_ENABLED_IRQ_READ =
      |                            ^~~~~~~~~~~~~~~~~~~~~~
kernel/locking/lockdep_internals.h:57:28: error: 'LOCKF_USED_IN_IRQ' defined but not used [-Werror=unused-const-variable=]
   57 | static const unsigned long LOCKF_USED_IN_IRQ =
      |                            ^~~~~~~~~~~~~~~~~
kernel/locking/lockdep_internals.h:51:28: error: 'LOCKF_ENABLED_IRQ' defined but not used [-Werror=unused-const-variable=]
   51 | static const unsigned long LOCKF_ENABLED_IRQ =
      |                            ^~~~~~~~~~~~~~~~~

This one is easy to avoid by changing the generated constant definition
into an equivalent enum.

Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Link: https://lore.kernel.org/r/20250409122314.2848028-6-arnd@kernel.org
---
 kernel/locking/lockdep_internals.h | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h
index 82156ca..0e5e6ff 100644
--- a/kernel/locking/lockdep_internals.h
+++ b/kernel/locking/lockdep_internals.h
@@ -47,29 +47,31 @@ enum {
 	__LOCKF(USED_READ)
 };
 
+enum {
 #define LOCKDEP_STATE(__STATE)	LOCKF_ENABLED_##__STATE |
-static const unsigned long LOCKF_ENABLED_IRQ =
+	LOCKF_ENABLED_IRQ =
 #include "lockdep_states.h"
-	0;
+	0,
 #undef LOCKDEP_STATE
 
 #define LOCKDEP_STATE(__STATE)	LOCKF_USED_IN_##__STATE |
-static const unsigned long LOCKF_USED_IN_IRQ =
+	LOCKF_USED_IN_IRQ =
 #include "lockdep_states.h"
-	0;
+	0,
 #undef LOCKDEP_STATE
 
 #define LOCKDEP_STATE(__STATE)	LOCKF_ENABLED_##__STATE##_READ |
-static const unsigned long LOCKF_ENABLED_IRQ_READ =
+	LOCKF_ENABLED_IRQ_READ =
 #include "lockdep_states.h"
-	0;
+	0,
 #undef LOCKDEP_STATE
 
 #define LOCKDEP_STATE(__STATE)	LOCKF_USED_IN_##__STATE##_READ |
-static const unsigned long LOCKF_USED_IN_IRQ_READ =
+	LOCKF_USED_IN_IRQ_READ =
 #include "lockdep_states.h"
-	0;
+	0,
 #undef LOCKDEP_STATE
+};
 
 #define LOCKF_ENABLED_IRQ_ALL (LOCKF_ENABLED_IRQ | LOCKF_ENABLED_IRQ_READ)
 #define LOCKF_USED_IN_IRQ_ALL (LOCKF_USED_IN_IRQ | LOCKF_USED_IN_IRQ_READ)

[PATCH 07/10] [RESEND] ARM: fixmap: make __end_of_early_ioremap_region an enum value

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Building with W=1 shows warnings for the __end_of_fixed_addresses
definition:

    In file included from mm/early_ioremap.c:18:
    arch/arm/include/asm/fixmap.h:39:35: error: '__end_of_fixed_addresses' defined but not used [-Werror=unused-const-variable=]

Move the calculation slightly up in the file into the enum that
contains __end_of_fixmap_region and __end_of_early_ioremap_region.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/include/asm/fixmap.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h
index 707068f852c2..90e0bd79b180 100644
--- a/arch/arm/include/asm/fixmap.h
+++ b/arch/arm/include/asm/fixmap.h
@@ -33,12 +33,11 @@ enum fixed_addresses {
 
 	FIX_BTMAP_END = __end_of_permanent_fixed_addresses,
 	FIX_BTMAP_BEGIN = FIX_BTMAP_END + TOTAL_FIX_BTMAPS - 1,
-	__end_of_early_ioremap_region
-};
+	__end_of_early_ioremap_region,
 
-static const enum fixed_addresses __end_of_fixed_addresses =
-	__end_of_fixmap_region > __end_of_early_ioremap_region ?
-	__end_of_fixmap_region : __end_of_early_ioremap_region;
+	__end_of_fixed_addresses = __end_of_fixmap_region > __end_of_early_ioremap_region ?
+				   __end_of_fixmap_region : __end_of_early_ioremap_region,
+};
 
 #define FIXMAP_PAGE_COMMON	(L_PTE_YOUNG | L_PTE_PRESENT | L_PTE_XN | L_PTE_DIRTY)
 
-- 
2.39.5

Re: [PATCH 07/10] [RESEND] ARM: fixmap: make __end_of_early_ioremap_region an enum value

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 02:22:59PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Building with W=1 shows warnings for the __end_of_fixed_addresses
> definition:
> 
>     In file included from mm/early_ioremap.c:18:
>     arch/arm/include/asm/fixmap.h:39:35: error: '__end_of_fixed_addresses' defined but not used [-Werror=unused-const-variable=]
> 
> Move the calculation slightly up in the file into the enum that
> contains __end_of_fixmap_region and __end_of_early_ioremap_region.

...


Maybe format it

> +	__end_of_fixed_addresses = __end_of_fixmap_region > __end_of_early_ioremap_region ?
> +				   __end_of_fixmap_region : __end_of_early_ioremap_region,

as

	__end_of_fixed_addresses =
		__end_of_fixmap_region > __end_of_early_ioremap_region ?
		__end_of_fixmap_region : __end_of_early_ioremap_region,

?

-- 
With Best Regards,
Andy Shevchenko

[PATCH 08/10] [RESEND 2] comedi: ni_atmio: avoid warning for unused device_ids[] table

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

When the driver is built-in, it produces a W=1 warning:

drivers/comedi/drivers/ni_atmio.c:209:35: error: 'device_ids' defined but not used [-Werror=unused-const-variable=]
  209 | static const struct pnp_device_id device_ids[] = {

The actual probe() function has a different way of identifying
the hardware, so just mark this one as __maybe_unused so it
can be dropped when built-in.

Reviewed-by: Ian Abbott <abbotti@mev.co.uk>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/comedi/drivers/ni_atmio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/comedi/drivers/ni_atmio.c b/drivers/comedi/drivers/ni_atmio.c
index 330ae1c58800..b4e759e5703f 100644
--- a/drivers/comedi/drivers/ni_atmio.c
+++ b/drivers/comedi/drivers/ni_atmio.c
@@ -215,7 +215,7 @@ static const int ni_irqpin[] = {
 
 #include "ni_mio_common.c"
 
-static const struct pnp_device_id device_ids[] = {
+static const struct pnp_device_id __maybe_unused device_ids[] = {
 	{.id = "NIC1900", .driver_data = 0},
 	{.id = "NIC2400", .driver_data = 0},
 	{.id = "NIC2500", .driver_data = 0},
-- 
2.39.5

[PATCH 09/10] [RESEND 2] apm-emulation: hide an unused variable

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The driver_version variable is only used inside of an #ifdef block, which
leads to a W=1 warning:

drivers/char/apm-emulation.c:144:19: error: 'driver_version' defined but not used [-Werror=unused-const-variable=]

Move this into the function using it.

Fixes: 7726942fb15e ("[APM] Add shared version of APM emulation")
Acked-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Found while looking through my patch backlog, still applies
---
 drivers/char/apm-emulation.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/char/apm-emulation.c b/drivers/char/apm-emulation.c
index e795390b070f..53ce352f7197 100644
--- a/drivers/char/apm-emulation.c
+++ b/drivers/char/apm-emulation.c
@@ -141,9 +141,6 @@ static struct apm_queue kapmd_queue;
 
 static DEFINE_MUTEX(state_lock);
 
-static const char driver_version[] = "1.13";	/* no spaces */
-
-
 
 /*
  * Compatibility cruft until the IPAQ people move over to the new
@@ -435,6 +432,8 @@ static struct miscdevice apm_device = {
  */
 static int proc_apm_show(struct seq_file *m, void *v)
 {
+	static const char driver_version[] = "1.13";	/* no spaces */
+
 	struct apm_power_info info;
 	char *units;
 
-- 
2.39.5

Re: [PATCH 01/10] drm/imagination: avoid unused-const-variable warning

[Matt Coster]

[-- Attachment #1.1: Type: text/plain, Size: 4756 bytes --]

On 09/04/2025 13:22, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> When CONFIG_DEBUG_FS is disabled, the stid_fmts[] array is not referenced
> anywhere, causing a W=1 warning with gcc:
> 
> In file included from drivers/gpu/drm/imagination/pvr_fw_trace.c:7:
> drivers/gpu/drm/imagination/pvr_rogue_fwif_sf.h:75:39: error: 'stid_fmts' defined but not used [-Werror=unused-const-variable=]
>    75 | static const struct rogue_km_stid_fmt stid_fmts[] = {
>       |                                       ^~~~~~~~~

Hi Arnd,

Thanks for catching this! My dev environment permanently has DEBUG_FS
enabled, we should probably look into a wider variety of testing
configs.

> 
> Rather than adding more #ifdef blocks, address this by changing the
> existing #ifdef into equivalent IS_ENABLED() checks so gcc can see
> where the symbol is used but still eliminate it from the object file.

Possibly a silly question, but wouldn't adding __maybe_unused to
stid_fmts be a simpler change here? Or is there a strong preference to
avoid #ifdef CONFIG_* and let the compiler figure out what to elide?

The contents of the pvr_rogue_fwif*.h headers is essentially normative
outside of company-internal documentation. With types, there's generally
no warnings emitted when they're not used, but I believe this is the
only instance of actual data being stored in these headers.

I suppose technically it should even be moved to an associated *.c file,
but that would (I assume) further confound the compiler's ability to
decide when it's needed in the final binary (or I guess the linker if
it's in a separate object).

> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/gpu/drm/imagination/pvr_fw_trace.c | 8 ++++----
>  drivers/gpu/drm/imagination/pvr_fw_trace.h | 2 --
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_fw_trace.c b/drivers/gpu/drm/imagination/pvr_fw_trace.c
> index 5dbb636d7d4f..93269299d6a4 100644
> --- a/drivers/gpu/drm/imagination/pvr_fw_trace.c
> +++ b/drivers/gpu/drm/imagination/pvr_fw_trace.c
> @@ -122,8 +122,6 @@ void pvr_fw_trace_fini(struct pvr_device *pvr_dev)
>  	pvr_fw_object_unmap_and_destroy(fw_trace->tracebuf_ctrl_obj);
>  }
>  
> -#if defined(CONFIG_DEBUG_FS)
> -
>  /**
>   * update_logtype() - Send KCCB command to trigger FW to update logtype
>   * @pvr_dev: Target PowerVR device
> @@ -447,7 +445,7 @@ static const struct file_operations pvr_fw_trace_fops = {
>  void
>  pvr_fw_trace_mask_update(struct pvr_device *pvr_dev, u32 old_mask, u32 new_mask)
>  {
> -	if (old_mask != new_mask)
> +	if (IS_ENABLED(CONFIG_DEBUG_FS) && old_mask != new_mask)

Logically, there's no reason to add the condition here. This function
was only gated behind CONFIG_DEBUG_FS because that was the only
mechanism provided to invoke update_logtype() but since we're relying on
the compiler to figure out when this function is required, we can skip
the IS_ENABLED() check.

>  		update_logtype(pvr_dev, new_mask);
>  }
>  
> @@ -457,6 +455,9 @@ pvr_fw_trace_debugfs_init(struct pvr_device *pvr_dev, struct dentry *dir)
>  	struct pvr_fw_trace *fw_trace = &pvr_dev->fw_dev.fw_trace;
>  	u32 thread_nr;
>  
> +	if (!IS_ENABLED(CONFIG_DEBUG_FS))
> +		return;
> +
>  	static_assert(ARRAY_SIZE(fw_trace->buffers) <= 10,
>  		      "The filename buffer is only large enough for a single-digit thread count");
>  
> @@ -469,4 +470,3 @@ pvr_fw_trace_debugfs_init(struct pvr_device *pvr_dev, struct dentry *dir)
>  				    &pvr_fw_trace_fops);
>  	}
>  }
> -#endif
> diff --git a/drivers/gpu/drm/imagination/pvr_fw_trace.h b/drivers/gpu/drm/imagination/pvr_fw_trace.h
> index 0074d2b18da0..1d0ef937427a 100644
> --- a/drivers/gpu/drm/imagination/pvr_fw_trace.h
> +++ b/drivers/gpu/drm/imagination/pvr_fw_trace.h
> @@ -65,7 +65,6 @@ struct pvr_fw_trace {
>  int pvr_fw_trace_init(struct pvr_device *pvr_dev);
>  void pvr_fw_trace_fini(struct pvr_device *pvr_dev);
>  
> -#if defined(CONFIG_DEBUG_FS)
>  /* Forward declaration from <linux/dcache.h>. */
>  struct dentry;

With the #ifdef removed, there's no reason to keep this forward
declaration down here. Can you move it up to the top of the file with
the others?

>  
> @@ -73,6 +72,5 @@ void pvr_fw_trace_mask_update(struct pvr_device *pvr_dev, u32 old_mask,
>  			      u32 new_mask);
>  
>  void pvr_fw_trace_debugfs_init(struct pvr_device *pvr_dev, struct dentry *dir);
> -#endif /* defined(CONFIG_DEBUG_FS) */

Having said that, surely it makes sense to keep at least
*_debugfs_init() gated behind CONFIG_DEBUG_FS?

>  
>  #endif /* PVR_FW_TRACE_H */


-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH 01/10] drm/imagination: avoid unused-const-variable warning

[Jani Nikula]

On Thu, 10 Apr 2025, Matt Coster <Matt.Coster@imgtec.com> wrote:
> Having said that, surely it makes sense to keep at least
> *_debugfs_init() gated behind CONFIG_DEBUG_FS?

If they're basically just calls to debugfs_create_{dir,file}, the
compiler pretty much turns them into nops, and you'll get better build
coverage.

BR,
Jani.


-- 
Jani Nikula, Intel

Re: [PATCH 01/10] drm/imagination: avoid unused-const-variable warning

[Arnd Bergmann]

On Thu, Apr 10, 2025, at 13:22, Matt Coster wrote:
> On 09/04/2025 13:22, Arnd Bergmann wrote:
>> 
>> Rather than adding more #ifdef blocks, address this by changing the
>> existing #ifdef into equivalent IS_ENABLED() checks so gcc can see
>> where the symbol is used but still eliminate it from the object file.
>
> Possibly a silly question, but wouldn't adding __maybe_unused to
> stid_fmts be a simpler change here? Or is there a strong preference to
> avoid #ifdef CONFIG_* and let the compiler figure out what to elide?

All three ways work, and it's mainly a matter a preference. A lot
of developers don't like __maybe_unused, and there are enough developers
that really dislike #ifdef blocks in .c files, though a lot of
others don't really care.

I sent the version that I like best because I find that easier to
read and it gives better compile-time coverage in all configurations,
but I mainly care about having no warnings, so pick whatever approach
works for you. 

> The contents of the pvr_rogue_fwif*.h headers is essentially normative
> outside of company-internal documentation. With types, there's generally
> no warnings emitted when they're not used, but I believe this is the
> only instance of actual data being stored in these headers.
>
> I suppose technically it should even be moved to an associated *.c file,
> but that would (I assume) further confound the compiler's ability to
> decide when it's needed in the final binary (or I guess the linker if
> it's in a separate object).

Moving it next to the user of that definition is geneally best
here, if the variable is only ever used in one place. If that
happens to be inside of an existing #ifdef, it just works, and
if someone later replaces the #ifdef with an IS_ENABLED() check
it keeps working.

>> -#if defined(CONFIG_DEBUG_FS)
>>  /* Forward declaration from <linux/dcache.h>. */
>>  struct dentry;
>
> With the #ifdef removed, there's no reason to keep this forward
> declaration down here. Can you move it up to the top of the file with
> the others?

I don't think it changes much, I usually keep these close to
the function prototype that use them as you have here.

>> @@ -73,6 +72,5 @@ void pvr_fw_trace_mask_update(struct pvr_device *pvr_dev, u32 old_mask,
>>  			      u32 new_mask);
>>  
>>  void pvr_fw_trace_debugfs_init(struct pvr_device *pvr_dev, struct dentry *dir);
>> -#endif /* defined(CONFIG_DEBUG_FS) */
>
> Having said that, surely it makes sense to keep at least
> *_debugfs_init() gated behind CONFIG_DEBUG_FS?

With the IS_ENABLED() check, it's an empty function, so the cost
of the function is very small. Leaving the #ifdef in here would
probably again cause warnings about unused functions or variable.

Ideally the entire file would just be left out of the build
here, with the function decarations in pvr_fw_trace.h
replaced with empty stubs for !CONFIG_DEBUG_FS. I don't
know if that's possible, or if you still need to initialize
the fw trace if there is no debugfs interface for it, so
I did not try this myself.

Can you try to come up with a fix for the warning that you
like best and treat my email as "Reported-by: Arnd Bergmann
<arnd@arndb.de>"?

      Arnd

Re: [PATCH 01/10] drm/imagination: avoid unused-const-variable warning

[Andy Shevchenko]

On Thu, Apr 10, 2025 at 11:22:05AM +0000, Matt Coster wrote:
> On 09/04/2025 13:22, Arnd Bergmann wrote:

...

> > Rather than adding more #ifdef blocks, address this by changing the
> > existing #ifdef into equivalent IS_ENABLED() checks so gcc can see
> > where the symbol is used but still eliminate it from the object file.
> 
> Possibly a silly question, but wouldn't adding __maybe_unused to
> stid_fmts be a simpler change here?

I'm not Arnd (and I just have read his answer), but I would like to add that
__maybe_unused should be the last resort as it has more cons than more invasive
solutions. In particular, it makes build time increase with a lot of work to
be made at link time, and also it might hide the real bugs when somebody simply
forgot to use it (depending on the configuration options) or so.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 01/10] drm/imagination: avoid unused-const-variable warning

[Matt Coster]

[-- Attachment #1.1: Type: text/plain, Size: 3730 bytes --]

On 09/04/2025 13:22, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> When CONFIG_DEBUG_FS is disabled, the stid_fmts[] array is not referenced
> anywhere, causing a W=1 warning with gcc:
> 
> In file included from drivers/gpu/drm/imagination/pvr_fw_trace.c:7:
> drivers/gpu/drm/imagination/pvr_rogue_fwif_sf.h:75:39: error: 'stid_fmts' defined but not used [-Werror=unused-const-variable=]
>    75 | static const struct rogue_km_stid_fmt stid_fmts[] = {
>       |                                       ^~~~~~~~~
> 
> Rather than adding more #ifdef blocks, address this by changing the
> existing #ifdef into equivalent IS_ENABLED() checks so gcc can see
> where the symbol is used but still eliminate it from the object file.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Hi Arnd,

After sleeping revisiting this after being on holiday, it seems like
enough of my concerns were due solely to my ignorance that I'd rather
just take this patch as-is than spend time reworking it. (Thanks to Jani
and Andi for filling gaps in my knowledge too!)

To that end,

Reviewed-by: Matt Coster <matt.coster@imgtec.com>

And I'll take this through drm-misc-next tomorrow if there are no
objections.

Cheers,
Matt

> ---
>  drivers/gpu/drm/imagination/pvr_fw_trace.c | 8 ++++----
>  drivers/gpu/drm/imagination/pvr_fw_trace.h | 2 --
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_fw_trace.c b/drivers/gpu/drm/imagination/pvr_fw_trace.c
> index 5dbb636d7d4f..93269299d6a4 100644
> --- a/drivers/gpu/drm/imagination/pvr_fw_trace.c
> +++ b/drivers/gpu/drm/imagination/pvr_fw_trace.c
> @@ -122,8 +122,6 @@ void pvr_fw_trace_fini(struct pvr_device *pvr_dev)
>  	pvr_fw_object_unmap_and_destroy(fw_trace->tracebuf_ctrl_obj);
>  }
>  
> -#if defined(CONFIG_DEBUG_FS)
> -
>  /**
>   * update_logtype() - Send KCCB command to trigger FW to update logtype
>   * @pvr_dev: Target PowerVR device
> @@ -447,7 +445,7 @@ static const struct file_operations pvr_fw_trace_fops = {
>  void
>  pvr_fw_trace_mask_update(struct pvr_device *pvr_dev, u32 old_mask, u32 new_mask)
>  {
> -	if (old_mask != new_mask)
> +	if (IS_ENABLED(CONFIG_DEBUG_FS) && old_mask != new_mask)
>  		update_logtype(pvr_dev, new_mask);
>  }
>  
> @@ -457,6 +455,9 @@ pvr_fw_trace_debugfs_init(struct pvr_device *pvr_dev, struct dentry *dir)
>  	struct pvr_fw_trace *fw_trace = &pvr_dev->fw_dev.fw_trace;
>  	u32 thread_nr;
>  
> +	if (!IS_ENABLED(CONFIG_DEBUG_FS))
> +		return;
> +
>  	static_assert(ARRAY_SIZE(fw_trace->buffers) <= 10,
>  		      "The filename buffer is only large enough for a single-digit thread count");
>  
> @@ -469,4 +470,3 @@ pvr_fw_trace_debugfs_init(struct pvr_device *pvr_dev, struct dentry *dir)
>  				    &pvr_fw_trace_fops);
>  	}
>  }
> -#endif
> diff --git a/drivers/gpu/drm/imagination/pvr_fw_trace.h b/drivers/gpu/drm/imagination/pvr_fw_trace.h
> index 0074d2b18da0..1d0ef937427a 100644
> --- a/drivers/gpu/drm/imagination/pvr_fw_trace.h
> +++ b/drivers/gpu/drm/imagination/pvr_fw_trace.h
> @@ -65,7 +65,6 @@ struct pvr_fw_trace {
>  int pvr_fw_trace_init(struct pvr_device *pvr_dev);
>  void pvr_fw_trace_fini(struct pvr_device *pvr_dev);
>  
> -#if defined(CONFIG_DEBUG_FS)
>  /* Forward declaration from <linux/dcache.h>. */
>  struct dentry;
>  
> @@ -73,6 +72,5 @@ void pvr_fw_trace_mask_update(struct pvr_device *pvr_dev, u32 old_mask,
>  			      u32 new_mask);
>  
>  void pvr_fw_trace_debugfs_init(struct pvr_device *pvr_dev, struct dentry *dir);
> -#endif /* defined(CONFIG_DEBUG_FS) */
>  
>  #endif /* PVR_FW_TRACE_H */


-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: (subset) [PATCH 01/10] drm/imagination: avoid unused-const-variable warning

[Matt Coster]

On Wed, 09 Apr 2025 14:22:53 +0200, Arnd Bergmann wrote:
> When CONFIG_DEBUG_FS is disabled, the stid_fmts[] array is not referenced
> anywhere, causing a W=1 warning with gcc:
> 
> In file included from drivers/gpu/drm/imagination/pvr_fw_trace.c:7:
> drivers/gpu/drm/imagination/pvr_rogue_fwif_sf.h:75:39: error: 'stid_fmts' defined but not used [-Werror=unused-const-variable=]
>    75 | static const struct rogue_km_stid_fmt stid_fmts[] = {
>       |                                       ^~~~~~~~~
> 
> [...]

Applied, thanks!

[01/10] drm/imagination: avoid unused-const-variable warning
        commit: 3206a96675342badb0254558ba4b4c8764aa3ae7

Best regards,
-- 
Matt Coster <matt.coster@imgtec.com>

[PATCH 10/10] [RESEND 3] dma/contiguous: avoid warning about unused size_bytes

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

When building with W=1, this variable is unused for configs with
CONFIG_CMA_SIZE_SEL_PERCENTAGE=y:

kernel/dma/contiguous.c:67:26: error: 'size_bytes' defined but not used [-Werror=unused-const-variable=]

Change this to a macro to avoid the warning.

Fixes: c64be2bb1c6e ("drivers: add Contiguous Memory Allocator")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v3: use a macro instead of constant
v2: fix spelling mistake.
---
 kernel/dma/contiguous.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index 055da410ac71..88888f4c1c73 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -64,8 +64,7 @@ struct cma *dma_contiguous_default_area;
  * Users, who want to set the size of global CMA area for their system
  * should use cma= kernel parameter.
  */
-static const phys_addr_t size_bytes __initconst =
-	(phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
+#define size_bytes (CMA_SIZE_MBYTES * SZ_1M)
 static phys_addr_t  size_cmdline __initdata = -1;
 static phys_addr_t base_cmdline __initdata;
 static phys_addr_t limit_cmdline __initdata;
-- 
2.39.5

Re: [PATCH 10/10] [RESEND 3] dma/contiguous: avoid warning about unused size_bytes

[Marek Szyprowski]

On 09.04.2025 14:24, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> When building with W=1, this variable is unused for configs with
> CONFIG_CMA_SIZE_SEL_PERCENTAGE=y:
>
> kernel/dma/contiguous.c:67:26: error: 'size_bytes' defined but not used [-Werror=unused-const-variable=]
>
> Change this to a macro to avoid the warning.
>
> Fixes: c64be2bb1c6e ("drivers: add Contiguous Memory Allocator")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>


Thanks, applied to dma-mapping-fixes branch.


> ---
> v3: use a macro instead of constant
> v2: fix spelling mistake.
> ---
>   kernel/dma/contiguous.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> index 055da410ac71..88888f4c1c73 100644
> --- a/kernel/dma/contiguous.c
> +++ b/kernel/dma/contiguous.c
> @@ -64,8 +64,7 @@ struct cma *dma_contiguous_default_area;
>    * Users, who want to set the size of global CMA area for their system
>    * should use cma= kernel parameter.
>    */
> -static const phys_addr_t size_bytes __initconst =
> -	(phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
> +#define size_bytes (CMA_SIZE_MBYTES * SZ_1M)
>   static phys_addr_t  size_cmdline __initdata = -1;
>   static phys_addr_t base_cmdline __initdata;
>   static phys_addr_t limit_cmdline __initdata;

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 10/10] [RESEND 3] dma/contiguous: avoid warning about unused size_bytes

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 04:35:57PM +0200, Marek Szyprowski wrote:
> On 09.04.2025 14:24, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > When building with W=1, this variable is unused for configs with
> > CONFIG_CMA_SIZE_SEL_PERCENTAGE=y:
> >
> > kernel/dma/contiguous.c:67:26: error: 'size_bytes' defined but not used [-Werror=unused-const-variable=]
> >
> > Change this to a macro to avoid the warning.

> > -static const phys_addr_t size_bytes __initconst =
> > -	(phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
> > +#define size_bytes (CMA_SIZE_MBYTES * SZ_1M)

This had phys_addr_t type before, do we still have the platforms when it can be
bigger than 32-bit integer?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 10/10] [RESEND 3] dma/contiguous: avoid warning about unused size_bytes

[Arnd Bergmann]

On Wed, Apr 9, 2025, at 16:43, Andy Shevchenko wrote:
> On Wed, Apr 09, 2025 at 04:35:57PM +0200, Marek Szyprowski wrote:
>> On 09.04.2025 14:24, Arnd Bergmann wrote:
>> > From: Arnd Bergmann <arnd@arndb.de>
>> >
>> > When building with W=1, this variable is unused for configs with
>> > CONFIG_CMA_SIZE_SEL_PERCENTAGE=y:
>> >
>> > kernel/dma/contiguous.c:67:26: error: 'size_bytes' defined but not used [-Werror=unused-const-variable=]
>> >
>> > Change this to a macro to avoid the warning.
>
>> > -static const phys_addr_t size_bytes __initconst =
>> > -	(phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
>> > +#define size_bytes (CMA_SIZE_MBYTES * SZ_1M)
>
> This had phys_addr_t type before, do we still have the platforms when it can be
> bigger than 32-bit integer?

It can certainly be larger on 64-bit systems, I think I messed it
up. Marek, can you fix this to put back the cast, or should I
send a patch on top?

      Arnd

Re: [PATCH 10/10] [RESEND 3] dma/contiguous: avoid warning about unused size_bytes

[Marek Szyprowski]

On 09.04.2025 16:51, Arnd Bergmann wrote:
> On Wed, Apr 9, 2025, at 16:43, Andy Shevchenko wrote:
>> On Wed, Apr 09, 2025 at 04:35:57PM +0200, Marek Szyprowski wrote:
>>> On 09.04.2025 14:24, Arnd Bergmann wrote:
>>>> From: Arnd Bergmann <arnd@arndb.de>
>>>>
>>>> When building with W=1, this variable is unused for configs with
>>>> CONFIG_CMA_SIZE_SEL_PERCENTAGE=y:
>>>>
>>>> kernel/dma/contiguous.c:67:26: error: 'size_bytes' defined but not used [-Werror=unused-const-variable=]
>>>>
>>>> Change this to a macro to avoid the warning.
>>>>
>>>> -static const phys_addr_t size_bytes __initconst =
>>>> -	(phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
>>>> +#define size_bytes (CMA_SIZE_MBYTES * SZ_1M)
>> This had phys_addr_t type before, do we still have the platforms when it can be
>> bigger than 32-bit integer?
> It can certainly be larger on 64-bit systems, I think I messed it
> up. Marek, can you fix this to put back the cast, or should I
> send a patch on top?

I will drop this one then as I didn't push my branch yet and apply v2, 
which seems to be fine.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland