[PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable

[PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable

[Jason Yan]

Fix the following gcc warning:

drivers/video/fbdev/matrox/g450_pll.c:336:15: warning: variable
‘pixel_vco’ set but not used [-Wunused-but-set-variable]
  unsigned int pixel_vco;
               ^~~~~~~~~

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Jason Yan <yanaijie@huawei.com>
---
 drivers/video/fbdev/matrox/g450_pll.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/drivers/video/fbdev/matrox/g450_pll.c b/drivers/video/fbdev/matrox/g450_pll.c
index c15f8a57498e..ff8e321a22ce 100644
--- a/drivers/video/fbdev/matrox/g450_pll.c
+++ b/drivers/video/fbdev/matrox/g450_pll.c
@@ -333,11 +333,9 @@ static int __g450_setclk(struct matrox_fb_info *minfo, unsigned int fout,
 			 unsigned int *deltaarray)
 {
 	unsigned int mnpcount;
-	unsigned int pixel_vco;
 	const struct matrox_pll_limits* pi;
 	struct matrox_pll_cache* ci;
 
-	pixel_vco = 0;
 	switch (pll) {
 		case M_PIXEL_PLL_A:
 		case M_PIXEL_PLL_B:
@@ -420,7 +418,6 @@ static int __g450_setclk(struct matrox_fb_info *minfo, unsigned int fout,
 				
 				mnp = matroxfb_DAC_in(minfo, M1064_XPIXPLLCM) << 16;
 				mnp |= matroxfb_DAC_in(minfo, M1064_XPIXPLLCN) << 8;
-				pixel_vco = g450_mnp2vco(minfo, mnp);
 				matroxfb_DAC_unlock_irqrestore(flags);
 			}
 			pi = &minfo->limits.video;
@@ -441,25 +438,6 @@ static int __g450_setclk(struct matrox_fb_info *minfo, unsigned int fout,
 			unsigned int delta;
 
 			vco = g450_mnp2vco(minfo, mnp);
-#if 0			
-			if (pll = M_VIDEO_PLL) {
-				unsigned int big, small;
-
-				if (vco < pixel_vco) {
-					small = vco;
-					big = pixel_vco;
-				} else {
-					small = pixel_vco;
-					big = vco;
-				}
-				while (big > small) {
-					big >>= 1;
-				}
-				if (big = small) {
-					continue;
-				}
-			}
-#endif			
 			delta = pll_freq_delta(fout, g450_vco2f(mnp, vco));
 			for (idx = mnpcount; idx > 0; idx--) {
 				/* = is important; due to nextpll algorithm we get
-- 
2.17.2

Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable

[Sam Ravnborg]

Hi Jason.

On Fri, Apr 03, 2020 at 10:16:09AM +0800, Jason Yan wrote:
> Fix the following gcc warning:
> 
> drivers/video/fbdev/matrox/g450_pll.c:336:15: warning: variable
> ‘pixel_vco’ set but not used [-Wunused-but-set-variable]
>   unsigned int pixel_vco;
>                ^~~~~~~~~
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Jason Yan <yanaijie@huawei.com>

Thanks, committed and pushed to drm-misc-next.
The fix will show up in upstream kernel at the next
merge window.

	Sam

> ---
>  drivers/video/fbdev/matrox/g450_pll.c | 22 ----------------------
>  1 file changed, 22 deletions(-)
> 
> diff --git a/drivers/video/fbdev/matrox/g450_pll.c b/drivers/video/fbdev/matrox/g450_pll.c
> index c15f8a57498e..ff8e321a22ce 100644
> --- a/drivers/video/fbdev/matrox/g450_pll.c
> +++ b/drivers/video/fbdev/matrox/g450_pll.c
> @@ -333,11 +333,9 @@ static int __g450_setclk(struct matrox_fb_info *minfo, unsigned int fout,
>  			 unsigned int *deltaarray)
>  {
>  	unsigned int mnpcount;
> -	unsigned int pixel_vco;
>  	const struct matrox_pll_limits* pi;
>  	struct matrox_pll_cache* ci;
>  
> -	pixel_vco = 0;
>  	switch (pll) {
>  		case M_PIXEL_PLL_A:
>  		case M_PIXEL_PLL_B:
> @@ -420,7 +418,6 @@ static int __g450_setclk(struct matrox_fb_info *minfo, unsigned int fout,
>  				
>  				mnp = matroxfb_DAC_in(minfo, M1064_XPIXPLLCM) << 16;
>  				mnp |= matroxfb_DAC_in(minfo, M1064_XPIXPLLCN) << 8;
> -				pixel_vco = g450_mnp2vco(minfo, mnp);
>  				matroxfb_DAC_unlock_irqrestore(flags);
>  			}
>  			pi = &minfo->limits.video;
> @@ -441,25 +438,6 @@ static int __g450_setclk(struct matrox_fb_info *minfo, unsigned int fout,
>  			unsigned int delta;
>  
>  			vco = g450_mnp2vco(minfo, mnp);
> -#if 0			
> -			if (pll = M_VIDEO_PLL) {
> -				unsigned int big, small;
> -
> -				if (vco < pixel_vco) {
> -					small = vco;
> -					big = pixel_vco;
> -				} else {
> -					small = pixel_vco;
> -					big = vco;
> -				}
> -				while (big > small) {
> -					big >>= 1;
> -				}
> -				if (big = small) {
> -					continue;
> -				}
> -			}
> -#endif			
>  			delta = pll_freq_delta(fout, g450_vco2f(mnp, vco));
>  			for (idx = mnpcount; idx > 0; idx--) {
>  				/* = is important; due to nextpll algorithm we get
> -- 
> 2.17.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable

[Andy Shevchenko]

On Wed, Apr 08, 2020 at 10:18:52AM +0000, Sam Ravnborg wrote:
> On Fri, Apr 03, 2020 at 10:16:09AM +0800, Jason Yan wrote:
> > Fix the following gcc warning:
> > 
> > drivers/video/fbdev/matrox/g450_pll.c:336:15: warning: variable
> > ‘pixel_vco’ set but not used [-Wunused-but-set-variable]
> >   unsigned int pixel_vco;
> >                ^~~~~~~~~
> > 
> > Reported-by: Hulk Robot <hulkci@huawei.com>
> > Signed-off-by: Jason Yan <yanaijie@huawei.com>
> 
> Thanks, committed and pushed to drm-misc-next.
> The fix will show up in upstream kernel at the next
> merge window.

The most of the patches from so called Hulk Robot appeared to be controversial.

First of all, even so called "dead code" may have side effects on the registers
in HW which may lead to other issues. Second, the mentioned dead code elimination
patch doesn't improve anything as now the dead code is 'mnp' variable (that's how
I got into that, I still have a build error).

That said, for the starter I suggest to revert this change. After one need go
carefully through code to understand if it's exactly the case and what to do with
'mnp' which involves some IO.

+Cc: current fbdev maintainers

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable

[Jason Yan]

在 2026/3/18 15:45, Andy Shevchenko 写道:
> On Wed, Apr 08, 2020 at 10:18:52AM +0000, Sam Ravnborg wrote:
>> On Fri, Apr 03, 2020 at 10:16:09AM +0800, Jason Yan wrote:
>>> Fix the following gcc warning:
>>>
>>> drivers/video/fbdev/matrox/g450_pll.c:336:15: warning: variable
>>> ‘pixel_vco’ set but not used [-Wunused-but-set-variable]
>>>    unsigned int pixel_vco;
>>>                 ^~~~~~~~~
>>>
>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>>
>> Thanks, committed and pushed to drm-misc-next.
>> The fix will show up in upstream kernel at the next
>> merge window.
> 
> The most of the patches from so called Hulk Robot appeared to be controversial.
> 
> First of all, even so called "dead code" may have side effects on the registers
> in HW which may lead to other issues. Second, the mentioned dead code elimination
> patch doesn't improve anything as now the dead code is 'mnp' variable (that's how
> I got into that, I still have a build error).

Sorry, I do not understand what side effects this may cause. Would you 
please explain it in detail?

Thanks,
Jason

> 
> That said, for the starter I suggest to revert this change. After one need go
> carefully through code to understand if it's exactly the case and what to do with
> 'mnp' which involves some IO.
> 
> +Cc: current fbdev maintainers
> 

Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable

[Andy Shevchenko]

On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:
> 在 2026/3/18 15:45, Andy Shevchenko 写道:
> > On Wed, Apr 08, 2020 at 10:18:52AM +0000, Sam Ravnborg wrote:
> > > On Fri, Apr 03, 2020 at 10:16:09AM +0800, Jason Yan wrote:
> > > > Fix the following gcc warning:
> > > > 
> > > > drivers/video/fbdev/matrox/g450_pll.c:336:15: warning: variable
> > > > ‘pixel_vco’ set but not used [-Wunused-but-set-variable]
> > > >    unsigned int pixel_vco;
> > > >                 ^~~~~~~~~
> > > > Reported-by: Hulk Robot <hulkci@huawei.com>
> > > > Signed-off-by: Jason Yan <yanaijie@huawei.com>
> > > 
> > > Thanks, committed and pushed to drm-misc-next.
> > > The fix will show up in upstream kernel at the next
> > > merge window.
> > 
> > The most of the patches from so called Hulk Robot appeared to be controversial.
> > 
> > First of all, even so called "dead code" may have side effects on the registers
> > in HW which may lead to other issues. Second, the mentioned dead code elimination
> > patch doesn't improve anything as now the dead code is 'mnp' variable (that's how
> > I got into that, I still have a build error).
> 
> Sorry, I do not understand what side effects this may cause. Would you
> please explain it in detail?

Any IO (and specifically on PCI bus) may have side effects. If the removed code
did some HW accesses (especially reads), it affects the state of HW. You can
read more about PCI bus and what read from it does.

Helge, can we revert this change and start over, please? (I can also send
revert if you think it's a better way)

I do not believe this change was ever tested on real HW, and I see an evidence
that this most likely was blindly made as it led to the similar issue after
the change.

> > That said, for the starter I suggest to revert this change. After one need go
> > carefully through code to understand if it's exactly the case and what to do with
> > 'mnp' which involves some IO.
> > 
> > +Cc: current fbdev maintainers

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable

[Jason Yan]

在 2026/3/19 15:38, Andy Shevchenko 写道:
> On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:
>> 在 2026/3/18 15:45, Andy Shevchenko 写道:
>>> On Wed, Apr 08, 2020 at 10:18:52AM +0000, Sam Ravnborg wrote:
>>>> On Fri, Apr 03, 2020 at 10:16:09AM +0800, Jason Yan wrote:
>>>>> Fix the following gcc warning:
>>>>>
>>>>> drivers/video/fbdev/matrox/g450_pll.c:336:15: warning: variable
>>>>> ‘pixel_vco’ set but not used [-Wunused-but-set-variable]
>>>>>     unsigned int pixel_vco;
>>>>>                  ^~~~~~~~~
>>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>>>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>>>>
>>>> Thanks, committed and pushed to drm-misc-next.
>>>> The fix will show up in upstream kernel at the next
>>>> merge window.
>>>
>>> The most of the patches from so called Hulk Robot appeared to be controversial.
>>>
>>> First of all, even so called "dead code" may have side effects on the registers
>>> in HW which may lead to other issues. Second, the mentioned dead code elimination
>>> patch doesn't improve anything as now the dead code is 'mnp' variable (that's how
>>> I got into that, I still have a build error).
>>
>> Sorry, I do not understand what side effects this may cause. Would you
>> please explain it in detail?
> 
> Any IO (and specifically on PCI bus) may have side effects. If the removed code
> did some HW accesses (especially reads), it affects the state of HW. You can
> read more about PCI bus and what read from it does.

No, the removed code did not do any IO and will not affect the state of HW.

> 
> Helge, can we revert this change and start over, please? (I can also send
> revert if you think it's a better way)

I don't think we need to revert this patch. Please provide proof that 
this modification will lead to real issues.

Thanks,
Jason

> 
> I do not believe this change was ever tested on real HW, and I see an evidence
> that this most likely was blindly made as it led to the similar issue after
> the change.
> 
>>> That said, for the starter I suggest to revert this change. After one need go
>>> carefully through code to understand if it's exactly the case and what to do with
>>> 'mnp' which involves some IO.
>>>
>>> +Cc: current fbdev maintainers
> 

Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable

[Andy Shevchenko]

On Thu, Mar 19, 2026 at 04:06:44PM +0800, Jason Yan wrote:
> 在 2026/3/19 15:38, Andy Shevchenko 写道:
> > On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:
> > > 在 2026/3/18 15:45, Andy Shevchenko 写道:
> > > > On Wed, Apr 08, 2020 at 10:18:52AM +0000, Sam Ravnborg wrote:
> > > > > On Fri, Apr 03, 2020 at 10:16:09AM +0800, Jason Yan wrote:
> > > > > > Fix the following gcc warning:
> > > > > > 
> > > > > > drivers/video/fbdev/matrox/g450_pll.c:336:15: warning: variable
> > > > > > ‘pixel_vco’ set but not used [-Wunused-but-set-variable]
> > > > > >     unsigned int pixel_vco;
> > > > > >                  ^~~~~~~~~
> > > > > > Reported-by: Hulk Robot <hulkci@huawei.com>
> > > > > > Signed-off-by: Jason Yan <yanaijie@huawei.com>
> > > > > 
> > > > > Thanks, committed and pushed to drm-misc-next.
> > > > > The fix will show up in upstream kernel at the next
> > > > > merge window.
> > > > 
> > > > The most of the patches from so called Hulk Robot appeared to be controversial.
> > > > 
> > > > First of all, even so called "dead code" may have side effects on the registers
> > > > in HW which may lead to other issues. Second, the mentioned dead code elimination
> > > > patch doesn't improve anything as now the dead code is 'mnp' variable (that's how
> > > > I got into that, I still have a build error).
> > > 
> > > Sorry, I do not understand what side effects this may cause. Would you
> > > please explain it in detail?
> > 
> > Any IO (and specifically on PCI bus) may have side effects. If the removed code
> > did some HW accesses (especially reads), it affects the state of HW. You can
> > read more about PCI bus and what read from it does.
> 
> No, the removed code did not do any IO and will not affect the state of HW.

Excellent, but it leaves the code that does IO and now still breaks the
compilation.

> > Helge, can we revert this change and start over, please? (I can also send
> > revert if you think it's a better way)
> 
> I don't think we need to revert this patch. Please provide proof that this
> modification will lead to real issues.

It's not me, who should provide an evidence, it had to be the author
who must had provided testing and everything when submitting the change.

> > I do not believe this change was ever tested on real HW, and I see an evidence
> > that this most likely was blindly made as it led to the similar issue after
> > the change.

No answer here? That's what I expected based on my observations on the quality
of the patches from so called Hulk Robot.

> > > > That said, for the starter I suggest to revert this change. After one need go
> > > > carefully through code to understand if it's exactly the case and what to do with
> > > > 'mnp' which involves some IO.
> > > > 
> > > > +Cc: current fbdev maintainers

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable

[Helge Deller]

Hi Andy,

On 3/19/26 09:21, Andy Shevchenko wrote:
> On Thu, Mar 19, 2026 at 04:06:44PM +0800, Jason Yan wrote:
>> 在 2026/3/19 15:38, Andy Shevchenko 写道:
>>> On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:
>>>> 在 2026/3/18 15:45, Andy Shevchenko 写道:
>>>>> On Wed, Apr 08, 2020 at 10:18:52AM +0000, Sam Ravnborg wrote:
>>>>>> On Fri, Apr 03, 2020 at 10:16:09AM +0800, Jason Yan wrote:
>>>>>>> Fix the following gcc warning:
>>>>>>>
>>>>>>> drivers/video/fbdev/matrox/g450_pll.c:336:15: warning: variable
>>>>>>> ‘pixel_vco’ set but not used [-Wunused-but-set-variable]
>>>>>>>      unsigned int pixel_vco;
>>>>>>>                   ^~~~~~~~~
>>>>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>>>>>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>>>>>>
>>>>>> Thanks, committed and pushed to drm-misc-next.
>>>>>> The fix will show up in upstream kernel at the next
>>>>>> merge window.
>>>>>
>>>>> The most of the patches from so called Hulk Robot appeared to be controversial.
>>>>>
>>>>> First of all, even so called "dead code" may have side effects on the registers
>>>>> in HW which may lead to other issues. Second, the mentioned dead code elimination
>>>>> patch doesn't improve anything as now the dead code is 'mnp' variable (that's how
>>>>> I got into that, I still have a build error).
>>>>
>>>> Sorry, I do not understand what side effects this may cause. Would you
>>>> please explain it in detail?
>>>
>>> Any IO (and specifically on PCI bus) may have side effects. If the removed code
>>> did some HW accesses (especially reads), it affects the state of HW. You can
>>> read more about PCI bus and what read from it does.
>>
>> No, the removed code did not do any IO and will not affect the state of HW.
> 
> Excellent, but it leaves the code that does IO and now still breaks the
> compilation.
> 
>>> Helge, can we revert this change and start over, please? (I can also send
>>> revert if you think it's a better way)

Andy, all points you make against removing relevant code is absolutely right.

But for this specific commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code and
set but not used variable") I have to agree with Jason, that the patch is ok.
I don't see any build errors either.

Are we mixing up things here maybe?

Helge

Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable

[Andy Shevchenko]

On Thu, Mar 19, 2026 at 09:35:33AM +0100, Helge Deller wrote:
> On 3/19/26 09:21, Andy Shevchenko wrote:
> > On Thu, Mar 19, 2026 at 04:06:44PM +0800, Jason Yan wrote:
> > > 在 2026/3/19 15:38, Andy Shevchenko 写道:
> > > > On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:

...

> > > > Helge, can we revert this change and start over, please? (I can also send
> > > > revert if you think it's a better way)
> 
> Andy, all points you make against removing relevant code is absolutely right.
> 
> But for this specific commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code and
> set but not used variable") I have to agree with Jason, that the patch is ok.
> I don't see any build errors either.

Just run on today's Linux Next (since the driver has not been changed in that
sense for a few years, the very same issue is present for a long time):

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.

> Are we mixing up things here maybe?

...

FWIW, I dove to the history of the driver.

So, the code, that was guarded by #if 0 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.").

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.

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

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable

[Helge Deller]

On 3/19/26 10:08, Andy Shevchenko wrote:
> On Thu, Mar 19, 2026 at 09:35:33AM +0100, Helge Deller wrote:
>> On 3/19/26 09:21, Andy Shevchenko wrote:
>>> On Thu, Mar 19, 2026 at 04:06:44PM +0800, Jason Yan wrote:
>>>> 在 2026/3/19 15:38, Andy Shevchenko 写道:
>>>>> On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:
> 
> ...
> 
>>>>> Helge, can we revert this change and start over, please? (I can also send
>>>>> revert if you think it's a better way)
>>
>> Andy, all points you make against removing relevant code is absolutely right.
>>
>> But for this specific commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code and
>> set but not used variable") I have to agree with Jason, that the patch is ok.
>> I don't see any build errors either.
> 
> Just run on today's Linux Next (since the driver has not been changed in that
> sense for a few years, the very same issue is present for a long time):
> 
> 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.

Ok.

  
>> Are we mixing up things here maybe?
> 
> ...
> 
> FWIW, I dove to the history of the driver.
> 
> So, the code, that was guarded by #if 0 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.").
> 
> 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.
> 
> So, 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.

Yes, I think that's what I'd prefer.
Do you mind sending a patch?

Helge

Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable

[Andy Shevchenko]

On Thu, Mar 19, 2026 at 01:44:28PM +0100, Helge Deller wrote:
> On 3/19/26 10:08, Andy Shevchenko wrote:
> > On Thu, Mar 19, 2026 at 09:35:33AM +0100, Helge Deller wrote:
> > > On 3/19/26 09:21, Andy Shevchenko wrote:
> > > > On Thu, Mar 19, 2026 at 04:06:44PM +0800, Jason Yan wrote:
> > > > > 在 2026/3/19 15:38, Andy Shevchenko 写道:
> > > > > > On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:

...

> > > > > > Helge, can we revert this change and start over, please? (I can also send
> > > > > > revert if you think it's a better way)
> > > 
> > > Andy, all points you make against removing relevant code is absolutely right.
> > > 
> > > But for this specific commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code and
> > > set but not used variable") I have to agree with Jason, that the patch is ok.
> > > I don't see any build errors either.
> > 
> > Just run on today's Linux Next (since the driver has not been changed in that
> > sense for a few years, the very same issue is present for a long time):
> > 
> > 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.
> 
> Ok.
> 
> > > Are we mixing up things here maybe?
> > 
> > ...
> > 
> > FWIW, I dove to the history of the driver.
> > 
> > So, the code, that was guarded by #if 0 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.").
> > 
> > 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.
> > 
> > So, 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.
> 
> Yes, I think that's what I'd prefer.
> Do you mind sending a patch?

I have just sent 20260320143646.3218199-1-andriy.shevchenko@linux.intel.com.

-- 
With Best Regards,
Andy Shevchenko