Skip to contentSkip to conversation

i810: fix module_param bool abuse.

PATCH Applied Rusty Russell 2 participants 4 messages Dec 15, 2011 – Jan 12, 2012 Framebuffer layer

lore
Share & tools
lore

Rusty Russell

author
#0 lore raw

PATCH 3/15 i810: fix module_param bool abuse.

The driver says "module_param(ddc3, bool, 0);". But it's not a used as a bool, it's used as a count.

Make it a bool.

Cc Antonino Daplas <[email protected]>
Cc Florian Tobias Schandinat <[email protected]>
Cc [email protected]
Signed-off-by Rusty Russell <[email protected]>
---
 drivers/video/i810/i810_main.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/video/i810/i810_main.c +3 -3
@@ -147,7 +147,7 @@ static int vyres      __devinitdata;
 static int sync       __devinitdata;
 static int extvga     __devinitdata;
 static int dcolor     __devinitdata;
-static int ddc3       __devinitdata = 2;
+static bool ddc3      __devinitdata;
 
 /*------------------------------------------------------------*/
 

@@ -1776,7 +1776,7 @@ static void __devinit i810_init_defaults
 	if (sync) 
 		par->dev_flags |= ALWAYS_SYNC;
 
-	par->ddc_num = ddc3;
+	par->ddc_num = (ddc3 ? 3 : 2);
 
 	if (bpp < 8)
 		bpp = 8;

@@ -1999,7 +1999,7 @@ static int __devinit i810fb_setup(char *
 		else if (!strncmp(this_opt, "dcolor", 6))
 			dcolor = 1;
 		else if (!strncmp(this_opt, "ddc3", 4))
-			ddc3 = 3;
+			ddc3 = true;
 		else
 			mode_option = this_opt;
 	}
Applied
#1 lore raw
Quoted 9 lines · Rusty Russell

The driver says "module_param(ddc3, bool, 0);". But it's not a used as a bool, it's used as a count.

Make it a bool.

Cc Antonino Daplas <[email protected]>
Cc Florian Tobias Schandinat <[email protected]>
Cc [email protected]
Signed-off-by Rusty Russell <[email protected]>

Applied.

Thanks,

Florian Tobias Schandinat

Quoted 35 lines
---
 drivers/video/i810/i810_main.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/video/i810/i810_main.c +3 -3
@@ -147,7 +147,7 @@ static int vyres      __devinitdata;
 static int sync       __devinitdata;
 static int extvga     __devinitdata;
 static int dcolor     __devinitdata;
-static int ddc3       __devinitdata = 2;
+static bool ddc3      __devinitdata;
 
 /*------------------------------------------------------------*/
 

@@ -1776,7 +1776,7 @@ static void __devinit i810_init_defaults
 	if (sync) 
 		par->dev_flags |= ALWAYS_SYNC;
 
-	par->ddc_num = ddc3;
+	par->ddc_num = (ddc3 ? 3 : 2);
 
 	if (bpp < 8)
 		bpp = 8;

@@ -1999,7 +1999,7 @@ static int __devinit i810fb_setup(char *
 		else if (!strncmp(this_opt, "dcolor", 6))
 			dcolor = 1;
 		else if (!strncmp(this_opt, "ddc3", 4))
-			ddc3 = 3;
+			ddc3 = true;
 		else
 			mode_option = this_opt;
 	}
author
#2 lore raw
Quoted 4 lines · Florian Tobias Schandinat
Quoted 3 lines · Rusty Russell

The driver says "module_param(ddc3, bool, 0);". But it's not a used as a bool, it's used as a count.

Quoted 8 lines
Quoted 6 lines

Make it a bool.

Cc Antonino Daplas <[email protected]>
Cc Florian Tobias Schandinat <[email protected]>
Cc [email protected]
Signed-off-by Rusty Russell <[email protected]>

Applied.

Hmm, I don't see this in Linus' tree yet, and it's a context-clash pre-requisite for part of the general driver bool conversion. I want to push today since I have linux.conf.au next week, so I've split the i810 conversion into a separate patch, below.

It's been over a week in linux-next, can you push this please?

From: Rusty Russell <[email protected]>
Subject: module_param: make bool parameters really bool (drivers/vidio/i810)

module_param(bool) used to counter-intuitively take an int. In fddd5201 (mid-2009) we allowed bool or int/unsigned int using a messy trick.

It's time to remove the int/unsigned int option. For this version it'll simply give a warning, but it'll break next kernel version.

Signed-off-by Rusty Russell <[email protected]>
drivers/video/i810/i810_main.c +5 -5
@@ -135,8 +135,8 @@ static struct pci_driver i810fb_driver  static char *mode_option __devinitdata = NULL;
 static int vram       __devinitdata = 4;
 static int bpp        __devinitdata = 8;
-static int mtrr       __devinitdata;
-static int accel      __devinitdata;
+static bool mtrr      __devinitdata;
+static bool accel     __devinitdata;
 static int hsync1     __devinitdata;
 static int hsync2     __devinitdata;
 static int vsync1     __devinitdata;

@@ -144,9 +144,9 @@ static int vsync2     __devinitdata;
 static int xres       __devinitdata;
 static int yres;
 static int vyres      __devinitdata;
-static int sync       __devinitdata;
-static int extvga     __devinitdata;
-static int dcolor     __devinitdata;
+static bool sync      __devinitdata;
+static bool extvga    __devinitdata;
+static bool dcolor    __devinitdata;
 static bool ddc3      __devinitdata;
 
 /*------------------------------------------------------------*/
#3 lore raw

Hi Rusty,

Quoted 12 lines · Rusty Russell
Quoted 11 lines · Florian Tobias Schandinat
Quoted 9 lines · Rusty Russell

The driver says "module_param(ddc3, bool, 0);". But it's not a used as a bool, it's used as a count.

Make it a bool.

Cc Antonino Daplas <[email protected]>
Cc Florian Tobias Schandinat <[email protected]>
Cc [email protected]
Signed-off-by Rusty Russell <[email protected]>
Quoted 6 lines
Quoted 1 line

Applied.

Hmm, I don't see this in Linus' tree yet, and it's a context-clash pre-requisite for part of the general driver bool conversion. I want to push today since I have linux.conf.au next week, so I've split the i810 conversion into a separate patch, below.

Sorry for the delay, but with an OMAP pull and a revert this merge window is not as quiet as I wished.

Quoted 1 line

It's been over a week in linux-next, can you push this please?

Sure, I've taken it and also fixed the subject.

Best regards,

Florian Tobias Schandinat

Quoted 44 lines

From: Rusty Russell <[email protected]>
Subject: module_param: make bool parameters really bool (drivers/vidio/i810)

module_param(bool) used to counter-intuitively take an int. In fddd5201 (mid-2009) we allowed bool or int/unsigned int using a messy trick.

It's time to remove the int/unsigned int option. For this version it'll simply give a warning, but it'll break next kernel version.

Signed-off-by Rusty Russell <[email protected]>
drivers/video/i810/i810_main.c +5 -6
@@ -135,8 +135,8 @@ static struct pci_driver i810fb_driver >  static char *mode_option __devinitdata = NULL;
 static int vram       __devinitdata = 4;
 static int bpp        __devinitdata = 8;
-static int mtrr       __devinitdata;
-static int accel      __devinitdata;
+static bool mtrr      __devinitdata;
+static bool accel     __devinitdata;
 static int hsync1     __devinitdata;
 static int hsync2     __devinitdata;
 static int vsync1     __devinitdata;

@@ -144,9 +144,9 @@ static int vsync2     __devinitdata;
 static int xres       __devinitdata;
 static int yres;
 static int vyres      __devinitdata;
-static int sync       __devinitdata;
-static int extvga     __devinitdata;
-static int dcolor     __devinitdata;
+static bool sync      __devinitdata;
+static bool extvga    __devinitdata;
+static bool dcolor    __devinitdata;
 static bool ddc3      __devinitdata;
 
 /*------------------------------------------------------------*/
--

To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html