diff mbox series

video/fbdev/matroxfb: Remove dead code

Message ID 1588370291-7746-1-git-send-email-jrdr.linux@gmail.com (mailing list archive)
State New, archived
Headers show
Series video/fbdev/matroxfb: Remove dead code | expand

Commit Message

Souptick Joarder May 1, 2020, 9:58 p.m. UTC
These are dead code since 3.15. If there is no plan to use it further
it can be removed forever.

Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
---
 drivers/video/fbdev/matrox/matroxfb_DAC1064.c | 11 -----------
 1 file changed, 11 deletions(-)

Comments

Sam Ravnborg May 3, 2020, 5:40 p.m. UTC | #1
Hi Souptick

On Sat, May 02, 2020 at 03:28:11AM +0530, Souptick Joarder wrote:
> These are dead code since 3.15. If there is no plan to use it further
> it can be removed forever.
Could you explain why you conclude this is dead code sine 3.15 -
and maybe point to the commit that made it dead.
I failed to look it up.
And I would assume I did not have to look it up, but that you have
provided enough background to evaluate the patch.

Sometimes dead code are kept becasue it documents something etc.
So it is not always a simple removal.

	Sam

> 
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> ---
>  drivers/video/fbdev/matrox/matroxfb_DAC1064.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/video/fbdev/matrox/matroxfb_DAC1064.c b/drivers/video/fbdev/matrox/matroxfb_DAC1064.c
> index 765e805..9c2a2c0 100644
> --- a/drivers/video/fbdev/matrox/matroxfb_DAC1064.c
> +++ b/drivers/video/fbdev/matrox/matroxfb_DAC1064.c
> @@ -603,9 +603,6 @@ static void MGA1064_ramdac_init(struct matrox_fb_info *minfo)
>  /* BIOS environ */
>  static int x7AF4 = 0x10;	/* flags, maybe 0x10 = SDRAM, 0x00 = SGRAM??? */
>  				/* G100 wants 0x10, G200 SGRAM does not care... */
> -#if 0
> -static int def50 = 0;	/* reg50, & 0x0F, & 0x3000 (only 0x0000, 0x1000, 0x2000 (0x3000 disallowed and treated as 0) */
> -#endif
>  
>  static void MGAG100_progPixClock(const struct matrox_fb_info *minfo, int flags,
>  				 int m, int n, int p)
> @@ -843,9 +840,6 @@ static int MGAG100_preinit(struct matrox_fb_info *minfo)
>  	struct matrox_hw_state *hw = &minfo->hw;
>  
>          u_int32_t reg50;
> -#if 0
> -	u_int32_t q;
> -#endif
>  
>  	DBG(__func__)
>  
> @@ -927,11 +921,6 @@ static int MGAG100_preinit(struct matrox_fb_info *minfo)
>  		mga_writeb(minfo->video.vbase, 0x0000, 0xAA);
>  		mga_writeb(minfo->video.vbase, 0x0800, 0x55);
>  		mga_writeb(minfo->video.vbase, 0x4000, 0x55);
> -#if 0
> -		if (mga_readb(minfo->video.vbase, 0x0000) != 0xAA) {
> -			hw->MXoptionReg &= ~0x1000;
> -		}
> -#endif
>  		hw->MXoptionReg |= 0x00078020;
>  	} else if (minfo->devflags.accelerator == FB_ACCEL_MATROX_MGAG200) {
>  		pci_read_config_dword(minfo->pcidev, PCI_OPTION2_REG, &reg50);
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Souptick Joarder May 3, 2020, 7:59 p.m. UTC | #2
On Sun, May 3, 2020 at 11:10 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Souptick
>
> On Sat, May 02, 2020 at 03:28:11AM +0530, Souptick Joarder wrote:
> > These are dead code since 3.15. If there is no plan to use it further
> > it can be removed forever.
> Could you explain why you conclude this is dead code sine 3.15 -
> and maybe point to the commit that made it dead.
> I failed to look it up.
> And I would assume I did not have to look it up, but that you have
> provided enough background to evaluate the patch.

I trace this back till linux version 2.3.9 & 2.4.0 in
drivers/video/matrox/matroxfb_DAC1064.c
(3.15 was the version when this is moved to
drivers/video/fbdev/matrox/matroxfb_DAC1064.c)
and I think this driver support was added mostly in 2.4.0, but as it
is pre git era, unable to provide
commit id. Code commented with *#if 0* in discussion existed since 2.4.0.

First two *#if 0* can be removed as these are just declaration of
variables, never used.

-#if 0
-static int def50 = 0;  /* reg50, & 0x0F, & 0x3000 (only 0x0000,
0x1000, 0x2000 (0x3000 disallowed and treated as 0) */
-#endif

 static void MGAG100_progPixClock(const struct matrox_fb_info *minfo, int flags,
                                 int m, int n, int p)
@@ -843,9 +840,6 @@ static int MGAG100_preinit(struct matrox_fb_info *minfo)
        struct matrox_hw_state *hw = &minfo->hw;

         u_int32_t reg50;
-#if 0
-       u_int32_t q;
-#endif

In my opinion 3rd *#if 0 is not for documentation purpose and can be
removed as well.

-#if 0
-               if (mga_readb(minfo->video.vbase, 0x0000) != 0xAA) {
-                       hw->MXoptionReg &= ~0x1000;
-               }
-#endif

There is a 4th *#if 0 in this file. But it wasn't removed as there are
some documentation
comment exists for it.

#if 0
/* HELP! If we boot without DFP connected to DVI, we can
  poweroff TMDS. But if we boot with DFP connected,
  TMDS generated clocks are used instead of ALL pixclocks
  available... If someone knows which register
  handles it, please reveal this secret to me... */
hw->DACreg[POS1064_XPWRCTRL] &= ~0x04; /* Poweroff TMDS */
#endif



>
> Sometimes dead code are kept becasue it documents something etc.
> So it is not always a simple removal.
>
Yes, I agree there are some dead code kept tree wide for documentation purpose,
but many of them are truly dead code as well and it differ from case to case.
diff mbox series

Patch

diff --git a/drivers/video/fbdev/matrox/matroxfb_DAC1064.c b/drivers/video/fbdev/matrox/matroxfb_DAC1064.c
index 765e805..9c2a2c0 100644
--- a/drivers/video/fbdev/matrox/matroxfb_DAC1064.c
+++ b/drivers/video/fbdev/matrox/matroxfb_DAC1064.c
@@ -603,9 +603,6 @@  static void MGA1064_ramdac_init(struct matrox_fb_info *minfo)
 /* BIOS environ */
 static int x7AF4 = 0x10;	/* flags, maybe 0x10 = SDRAM, 0x00 = SGRAM??? */
 				/* G100 wants 0x10, G200 SGRAM does not care... */
-#if 0
-static int def50 = 0;	/* reg50, & 0x0F, & 0x3000 (only 0x0000, 0x1000, 0x2000 (0x3000 disallowed and treated as 0) */
-#endif
 
 static void MGAG100_progPixClock(const struct matrox_fb_info *minfo, int flags,
 				 int m, int n, int p)
@@ -843,9 +840,6 @@  static int MGAG100_preinit(struct matrox_fb_info *minfo)
 	struct matrox_hw_state *hw = &minfo->hw;
 
         u_int32_t reg50;
-#if 0
-	u_int32_t q;
-#endif
 
 	DBG(__func__)
 
@@ -927,11 +921,6 @@  static int MGAG100_preinit(struct matrox_fb_info *minfo)
 		mga_writeb(minfo->video.vbase, 0x0000, 0xAA);
 		mga_writeb(minfo->video.vbase, 0x0800, 0x55);
 		mga_writeb(minfo->video.vbase, 0x4000, 0x55);
-#if 0
-		if (mga_readb(minfo->video.vbase, 0x0000) != 0xAA) {
-			hw->MXoptionReg &= ~0x1000;
-		}
-#endif
 		hw->MXoptionReg |= 0x00078020;
 	} else if (minfo->devflags.accelerator == FB_ACCEL_MATROX_MGAG200) {
 		pci_read_config_dword(minfo->pcidev, PCI_OPTION2_REG, &reg50);