[PATCH v1 1/1] video: fbdev: matroxfb: Mark variable with __maybe_unused to avoid W=1 build break

[PATCH v1 1/1] video: fbdev: matroxfb: Mark variable with __maybe_unused to avoid W=1 build break

[Andy Shevchenko]

Clang is not happy about set but unused variable:

drivers/video/fbdev/matrox/g450_pll.c:412:18: error: variable 'mnp' set but not used [-Werror,-Wunused-but-set-variable]
   412 |                                 unsigned int mnp;
       |                                              ^
1 error generated.

Since the commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code
and set but not used variable") the 'mnp' became unused, but eliminating
that code might have side-effects. The question here is what should we do
with 'mnp'? The easiest way out is just mark it with __maybe_unused which
will shut the compiler up and won't change any possible IO flow. So does
this change.

Fixes: 7b987887f97b ("video: fbdev: matroxfb: remove dead code and set but not used variable")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

---
Cc: Jason Yan <yanaijie@huawei.com>
---

Below is a dive into the history of the driver.

The problem was revealed when the #if 0 guarded code along with unused
pixel_vco variable was removed. That code was introduced in the original
commit 213d22146d1f ("[PATCH] (1/3) matroxfb for 2.5.3"). And then guarded
in the commit 705e41f82988 ("matroxfb DVI updates: Handle DVI output on
G450/G550. Powerdown unused portions of G450/G550 DAC. Split G450/G550 DAC
from older DAC1064 handling. Modify PLL setting when both CRTCs use same
pixel clocks.").

NOTE: The two commits mentioned above pre-date Git era and available in
history.git repository for archaeological purposes.

Even without that guard the modern compilers may see that the pixel_vco
wasn't ever used and seems a leftover after some debug or review made
25 years ago.

The g450_mnp2vco() doesn't have any IO and as Jason said doesn't seem
to have any side effects either than some unneeded CPU processing during
runtime. I agree that's unlikely that timeout (or heating up the CPU) has
any effect on the HW (GPU/display) functionality.
---
 drivers/video/fbdev/matrox/g450_pll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/matrox/g450_pll.c b/drivers/video/fbdev/matrox/g450_pll.c
index e2c1478aa47f..6a08f78cd1ac 100644
--- a/drivers/video/fbdev/matrox/g450_pll.c
+++ b/drivers/video/fbdev/matrox/g450_pll.c
@@ -409,7 +409,7 @@ static int __g450_setclk(struct matrox_fb_info *minfo, unsigned int fout,
 		case M_VIDEO_PLL:
 			{
 				u_int8_t tmp;
-				unsigned int mnp;
+				unsigned int mnp __maybe_unused;
 				unsigned long flags;
 
 				matroxfb_DAC_lock_irqsave(flags);
-- 
2.50.1

Re: [PATCH v1 1/1] video: fbdev: matroxfb: Mark variable with __maybe_unused to avoid W=1 build break

[Helge Deller]

On 3/20/26 15:36, Andy Shevchenko wrote:
> Clang is not happy about set but unused variable:
> 
> drivers/video/fbdev/matrox/g450_pll.c:412:18: error: variable 'mnp' set but not used [-Werror,-Wunused-but-set-variable]
>     412 |                                 unsigned int mnp;
>         |                                              ^
> 1 error generated.
> 
> Since the commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code
> and set but not used variable") the 'mnp' became unused, but eliminating
> that code might have side-effects. The question here is what should we do
> with 'mnp'? The easiest way out is just mark it with __maybe_unused which
> will shut the compiler up and won't change any possible IO flow. So does
> this change.
> 
> Fixes: 7b987887f97b ("video: fbdev: matroxfb: remove dead code and set but not used variable")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Helge Deller <deller@gmx.de>

Thanks!
Helge

> 
> ---
> Cc: Jason Yan <yanaijie@huawei.com>
> ---
> 
> Below is a dive into the history of the driver.
> 
> The problem was revealed when the #if 0 guarded code along with unused
> pixel_vco variable was removed. That code was introduced in the original
> commit 213d22146d1f ("[PATCH] (1/3) matroxfb for 2.5.3"). And then guarded
> in the commit 705e41f82988 ("matroxfb DVI updates: Handle DVI output on
> G450/G550. Powerdown unused portions of G450/G550 DAC. Split G450/G550 DAC
> from older DAC1064 handling. Modify PLL setting when both CRTCs use same
> pixel clocks.").
> 
> NOTE: The two commits mentioned above pre-date Git era and available in
> history.git repository for archaeological purposes.
> 
> Even without that guard the modern compilers may see that the pixel_vco
> wasn't ever used and seems a leftover after some debug or review made
> 25 years ago.
> 
> The g450_mnp2vco() doesn't have any IO and as Jason said doesn't seem
> to have any side effects either than some unneeded CPU processing during
> runtime. I agree that's unlikely that timeout (or heating up the CPU) has
> any effect on the HW (GPU/display) functionality.
> ---
>   drivers/video/fbdev/matrox/g450_pll.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/matrox/g450_pll.c b/drivers/video/fbdev/matrox/g450_pll.c
> index e2c1478aa47f..6a08f78cd1ac 100644
> --- a/drivers/video/fbdev/matrox/g450_pll.c
> +++ b/drivers/video/fbdev/matrox/g450_pll.c
> @@ -409,7 +409,7 @@ static int __g450_setclk(struct matrox_fb_info *minfo, unsigned int fout,
>   		case M_VIDEO_PLL:
>   			{
>   				u_int8_t tmp;
> -				unsigned int mnp;
> +				unsigned int mnp __maybe_unused;
>   				unsigned long flags;
>   
>   				matroxfb_DAC_lock_irqsave(flags);

Re: [PATCH v1 1/1] video: fbdev: matroxfb: Mark variable with __maybe_unused to avoid W=1 build break

[Jason Yan]

在 2026/3/20 22:36, Andy Shevchenko 写道:
> Clang is not happy about set but unused variable:
> 
> drivers/video/fbdev/matrox/g450_pll.c:412:18: error: variable 'mnp' set but not used [-Werror,-Wunused-but-set-variable]
>     412 |                                 unsigned int mnp;
>         |                                              ^
> 1 error generated.
> 
> Since the commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code
> and set but not used variable") the 'mnp' became unused, but eliminating
> that code might have side-effects. The question here is what should we do
> with 'mnp'? The easiest way out is just mark it with __maybe_unused which
> will shut the compiler up and won't change any possible IO flow. So does
> this change.
> 
> Fixes: 7b987887f97b ("video: fbdev: matroxfb: remove dead code and set but not used variable")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> ---
> Cc: Jason Yan <yanaijie@huawei.com>
> ---
> 
> Below is a dive into the history of the driver.
> 
> The problem was revealed when the #if 0 guarded code along with unused
> pixel_vco variable was removed. That code was introduced in the original
> commit 213d22146d1f ("[PATCH] (1/3) matroxfb for 2.5.3"). And then guarded
> in the commit 705e41f82988 ("matroxfb DVI updates: Handle DVI output on
> G450/G550. Powerdown unused portions of G450/G550 DAC. Split G450/G550 DAC
> from older DAC1064 handling. Modify PLL setting when both CRTCs use same
> pixel clocks.").
> 
> NOTE: The two commits mentioned above pre-date Git era and available in
> history.git repository for archaeological purposes.
> 
> Even without that guard the modern compilers may see that the pixel_vco
> wasn't ever used and seems a leftover after some debug or review made
> 25 years ago.
> 
> The g450_mnp2vco() doesn't have any IO and as Jason said doesn't seem
> to have any side effects either than some unneeded CPU processing during
> runtime. I agree that's unlikely that timeout (or heating up the CPU) has
> any effect on the HW (GPU/display) functionality.
> ---
>   drivers/video/fbdev/matrox/g450_pll.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/matrox/g450_pll.c b/drivers/video/fbdev/matrox/g450_pll.c
> index e2c1478aa47f..6a08f78cd1ac 100644
> --- a/drivers/video/fbdev/matrox/g450_pll.c
> +++ b/drivers/video/fbdev/matrox/g450_pll.c
> @@ -409,7 +409,7 @@ static int __g450_setclk(struct matrox_fb_info *minfo, unsigned int fout,
>   		case M_VIDEO_PLL:
>   			{
>   				u_int8_t tmp;
> -				unsigned int mnp;
> +				unsigned int mnp __maybe_unused;
>   				unsigned long flags;
>   
>   				matroxfb_DAC_lock_irqsave(flags);

Reviewed-by: Jason Yan <yanaijie@huawei.com>