[PATCH 0/8] Homogenize application output and fix sparse warnings.

[PATCH 0/8] Homogenize application output and fix sparse warnings.

[Miguel Gómez]

This serie of patches mailnly try to homogenize module's output by removing some
custom macros and printk calls, and using pr_* and dev_* functions instead.
Also, they fix the definition of a fuction only in debug mode and fix some
sparse warnings.

Miguel Gómez (8):
  Staging: xgifb: reformat module parameter descriptions.
  Staging: xgifb: remove DEBUGPRN() macro and its uses.
  Staging: xgifb: remove DPRINTK() macro and replace it with pr_debug.
  Staging: xgifb: define real dumpVGAReg() content only in DEBUG mode.
  Staging: xgifb: Remove XGIFAIL() macro and its calls.
  Staging: xgifb: Remove printk usage.
  Staging: xgifb: Fix sparse warnings.
  Staging: xgifb: Replace pr_* with dev_* when possible.

 drivers/staging/xgifb/XGI_main.h    |    2 -
 drivers/staging/xgifb/XGI_main_26.c |  189 ++++++++++++++++-------------------
 drivers/staging/xgifb/vb_init.c     |   48 +--------
 drivers/staging/xgifb/vb_setmode.c  |    2 +-
 drivers/staging/xgifb/vb_util.c     |    1 +
 5 files changed, 92 insertions(+), 150 deletions(-)

-- 
1.7.9.5

[PATCH 1/8] Staging: xgifb: reformat module parameter descriptions.

[Miguel Gómez]

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/xgifb/XGI_main_26.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/xgifb/XGI_main_26.c b/drivers/staging/xgifb/XGI_main_26.c
index 3382272..8e0786d 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -2297,23 +2297,23 @@ static struct pci_driver xgifb_driver = {
 
 module_param(mode, charp, 0);
 MODULE_PARM_DESC(mode,
-	"\nSelects the desired default display mode in the format XxYxDepth,\n"
-	"eg. 1024x768x16.\n");
+	"Selects the desired default display mode in the format XxYxDepth "
+	"(eg. 1024x768x16).");
 
 module_param(forcecrt2type, charp, 0);
 MODULE_PARM_DESC(forcecrt2type,
-	"\nForce the second display output type. Possible values are NONE,\n"
-	"LCD, TV, VGA, SVIDEO or COMPOSITE.\n");
+	"Force the second display output type. Possible values are NONE, "
+	"LCD, TV, VGA, SVIDEO or COMPOSITE.");
 
 module_param(vesa, int, 0);
 MODULE_PARM_DESC(vesa,
-	"\nSelects the desired default display mode by VESA mode number, eg.\n"
-	"0x117.\n");
+	"Selects the desired default display mode by VESA mode number "
+	"(eg. 0x117).");
 
 module_param(filter, int, 0);
 MODULE_PARM_DESC(filter,
-	"\nSelects TV flicker filter type (only for systems with a SiS301 video bridge).\n"
-	"(Possible values 0-7, default: [no filter])\n");
+	"Selects TV flicker filter type (only for systems with a SiS301 video bridge). "
+	"Possible values 0-7. Default: [no filter]).");
 
 static int __init xgifb_init(void)
 {
-- 
1.7.9.5

[PATCH 2/8] Staging: xgifb: remove DEBUGPRN() macro and its uses.

[Miguel Gómez]

Remove DEBUGPRN() macro which is defined as empty.

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/xgifb/XGI_main_26.c |   13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/staging/xgifb/XGI_main_26.c b/drivers/staging/xgifb/XGI_main_26.c
index 8e0786d..143dd31 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -117,12 +117,6 @@ static inline void dumpVGAReg(void)
 }
 #endif
 
-#if 1
-#define DEBUGPRN(x)
-#else
-#define DEBUGPRN(x) pr_info(x "\n");
-#endif
-
 /* --------------- Hardware Access Routines -------------------------- */
 
 static int XGIfb_mode_rate_to_dclock(struct vb_device_info *XGI_Pr,
@@ -1054,7 +1048,6 @@ static int XGIfb_do_set_var(struct fb_var_screeninfo *var, int isactive,
 	int old_mode;
 	/* unsigned char reg, reg1; */
 
-	DEBUGPRN("Inside do_set_var");
 	/* printk(KERN_DEBUG "XGIfb:var->yres=%d, var->upper_margin=%d, var->lower_margin=%d, var->vsync_len=%d\n", var->yres, var->upper_margin, var->lower_margin, var->vsync_len); */
 
 	info->var.xres_virtual = var->xres_virtual;
@@ -1199,7 +1192,6 @@ static int XGIfb_do_set_var(struct fb_var_screeninfo *var, int isactive,
 		}
 	}
 	XGIfb_bpp_to_var(xgifb_info, var); /*update ARGB info*/
-	DEBUGPRN("End of do_set_var");
 
 	dumpVGAReg();
 	return 0;
@@ -1321,7 +1313,6 @@ static int XGIfb_get_fix(struct fb_fix_screeninfo *fix, int con,
 {
 	struct xgifb_video_info *xgifb_info = info->par;
 
-	DEBUGPRN("inside get_fix");
 	memset(fix, 0, sizeof(struct fb_fix_screeninfo));
 
 	strncpy(fix->id, "XGI", sizeof(fix->id) - 1);
@@ -1352,7 +1343,6 @@ static int XGIfb_get_fix(struct fb_fix_screeninfo *fix, int con,
 	fix->mmio_len = xgifb_info->mmio_size;
 	fix->accel = FB_ACCEL_SIS_XABRE;
 
-	DEBUGPRN("end of get_fix");
 	return 0;
 }
 
@@ -1379,8 +1369,6 @@ static int XGIfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 	int found_mode = 0;
 	int refresh_rate, search_idx;
 
-	DEBUGPRN("Inside check_var");
-
 	if ((var->vmode & FB_VMODE_MASK) == FB_VMODE_NONINTERLACED) {
 		vtotal = var->upper_margin + var->yres + var->lower_margin
 				+ var->vsync_len;
@@ -1511,7 +1499,6 @@ static int XGIfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 	var->blue.msb_right =
 	var->transp.offset = var->transp.length = var->transp.msb_right = 0;
 
-	DEBUGPRN("end of check_var");
 	return 0;
 }
 
-- 
1.7.9.5

[PATCH 3/8] Staging: xgifb: remove DPRINTK() macro and replace it with pr_debug.

[Miguel Gómez]

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/xgifb/XGI_main_26.c |   42 +++++++++++++++--------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/xgifb/XGI_main_26.c b/drivers/staging/xgifb/XGI_main_26.c
index 143dd31..9ffc853 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -33,12 +33,6 @@ static unsigned int refresh_rate;
 #undef XGIFBDEBUG
 
 #ifdef XGIFBDEBUG
-#define DPRINTK(fmt, args...) pr_debug("%s: " fmt, __func__ , ## args)
-#else
-#define DPRINTK(fmt, args...)
-#endif
-
-#ifdef XGIFBDEBUG
 static void dumpVGAReg(void)
 {
 	u8 i, reg;
@@ -662,8 +656,8 @@ static u8 XGIfb_search_refresh_rate(struct xgifb_video_info *xgifb_info,
 				break;
 			} else if (XGIfb_vrate[i].refresh > rate) {
 				if ((XGIfb_vrate[i].refresh - rate) <= 3) {
-					DPRINTK("XGIfb: Adjusting rate from %d up to %d\n",
-						rate, XGIfb_vrate[i].refresh);
+					pr_debug("XGIfb: Adjusting rate from %d up to %d\n",
+						 rate, XGIfb_vrate[i].refresh);
 					xgifb_info->rate_idx =
 						XGIfb_vrate[i].idx;
 					xgifb_info->refresh_rate =
@@ -671,8 +665,8 @@ static u8 XGIfb_search_refresh_rate(struct xgifb_video_info *xgifb_info,
 				} else if (((rate - XGIfb_vrate[i - 1].refresh)
 						<= 2) && (XGIfb_vrate[i].idx
 						!= 1)) {
-					DPRINTK("XGIfb: Adjusting rate from %d down to %d\n",
-						rate, XGIfb_vrate[i-1].refresh);
+					pr_debug("XGIfb: Adjusting rate from %d down to %d\n",
+						 rate, XGIfb_vrate[i-1].refresh);
 					xgifb_info->rate_idx =
 						XGIfb_vrate[i - 1].idx;
 					xgifb_info->refresh_rate =
@@ -680,8 +674,8 @@ static u8 XGIfb_search_refresh_rate(struct xgifb_video_info *xgifb_info,
 				}
 				break;
 			} else if ((rate - XGIfb_vrate[i].refresh) <= 2) {
-				DPRINTK("XGIfb: Adjusting rate from %d down to %d\n",
-					rate, XGIfb_vrate[i].refresh);
+				pr_debug("XGIfb: Adjusting rate from %d down to %d\n",
+					 rate, XGIfb_vrate[i].refresh);
 				xgifb_info->rate_idx = XGIfb_vrate[i].idx;
 				break;
 			}
@@ -995,15 +989,15 @@ static void XGIfb_post_setmode(struct xgifb_video_info *xgifb_info)
 			}
 
 			if ((filter >= 0) && (filter <= 7)) {
-				DPRINTK("FilterTable[%d]-%d: %02x %02x %02x %02x\n",
-					filter_tb, filter,
-					XGI_TV_filter[filter_tb].
+				pr_debug("FilterTable[%d]-%d: %02x %02x %02x %02x\n",
+					 filter_tb, filter,
+					 XGI_TV_filter[filter_tb].
 						filter[filter][0],
-					XGI_TV_filter[filter_tb].
+					 XGI_TV_filter[filter_tb].
 						filter[filter][1],
-					XGI_TV_filter[filter_tb].
+					 XGI_TV_filter[filter_tb].
 						filter[filter][2],
-					XGI_TV_filter[filter_tb].
+					 XGI_TV_filter[filter_tb].
 						filter[filter][3]
 				);
 				xgifb_reg_set(
@@ -1064,7 +1058,7 @@ static int XGIfb_do_set_var(struct fb_var_screeninfo *var, int isactive,
 	}
 
 	if (!htotal || !vtotal) {
-		DPRINTK("XGIfb: Invalid 'var' information\n");
+		pr_debug("XGIfb: Invalid 'var' information\n");
 		return -EINVAL;
 	} pr_debug("var->pixclock=%d, htotal=%d, vtotal=%d\n",
 			var->pixclock, htotal, vtotal);
@@ -1140,11 +1134,11 @@ static int XGIfb_do_set_var(struct fb_var_screeninfo *var, int isactive,
 
 		XGIfb_post_setmode(xgifb_info);
 
-		DPRINTK("XGIfb: Set new mode: %dx%dx%d-%d\n",
-				XGIbios_mode[xgifb_info->mode_idx].xres,
-				XGIbios_mode[xgifb_info->mode_idx].yres,
-				XGIbios_mode[xgifb_info->mode_idx].bpp,
-				xgifb_info->refresh_rate);
+		pr_debug("XGIfb: Set new mode: %dx%dx%d-%d\n",
+			 XGIbios_mode[xgifb_info->mode_idx].xres,
+			 XGIbios_mode[xgifb_info->mode_idx].yres,
+			 XGIbios_mode[xgifb_info->mode_idx].bpp,
+			 xgifb_info->refresh_rate);
 
 		xgifb_info->video_bpp = XGIbios_mode[xgifb_info->mode_idx].bpp;
 		xgifb_info->video_vwidth = info->var.xres_virtual;
-- 
1.7.9.5

[PATCH 4/8] Staging: xgifb: define real dumpVGAReg() content only in DEBUG mode.

[Miguel Gómez]

Define real content of dumpVGAReg() only in debug mode. Remove old condition.

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/xgifb/XGI_main_26.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/xgifb/XGI_main_26.c b/drivers/staging/xgifb/XGI_main_26.c
index 9ffc853..a45a497 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -30,9 +30,7 @@ static unsigned int refresh_rate;
 
 /* -------------------- Macro definitions ---------------------------- */
 
-#undef XGIFBDEBUG
-
-#ifdef XGIFBDEBUG
+#ifdef DEBUG
 static void dumpVGAReg(void)
 {
 	u8 i, reg;
-- 
1.7.9.5

[PATCH 5/8] Staging: xgifb: Remove XGIFAIL() macro and its calls.

[Miguel Gómez]

XGIFAIL() prints a message and returns a value, but it's used only in one
place. Better remove it and replace the call with the macro content.

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/xgifb/XGI_main.h    |    2 --
 drivers/staging/xgifb/XGI_main_26.c |    6 ++++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/xgifb/XGI_main.h b/drivers/staging/xgifb/XGI_main.h
index ce18f8c..be6bb7d 100644
--- a/drivers/staging/xgifb/XGI_main.h
+++ b/drivers/staging/xgifb/XGI_main.h
@@ -4,8 +4,6 @@
 #include "XGIfb.h"
 #include "vb_def.h"
 
-#define XGIFAIL(x) do { printk(x "\n"); return -EINVAL; } while (0)
-
 #ifndef PCI_DEVICE_ID_XGI_42
 #define PCI_DEVICE_ID_XGI_42      0x042
 #endif
diff --git a/drivers/staging/xgifb/XGI_main_26.c b/drivers/staging/xgifb/XGI_main_26.c
index a45a497..58cddb4 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -1376,8 +1376,10 @@ static int XGIfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 		vtotal = var->upper_margin + var->yres + var->lower_margin
 				+ var->vsync_len;
 
-	if (!(htotal) || !(vtotal))
-		XGIFAIL("XGIfb: no valid timing data");
+	if (!(htotal) || !(vtotal)) {
+		pr_debug("XGIfb: no valid timing data\n");
+		return -EINVAL;
+	}
 
 	if (var->pixclock && htotal && vtotal) {
 		drate = 1000000000 / var->pixclock;
-- 
1.7.9.5

[PATCH 6/8] Staging: xgifb: Remove printk usage.

[Miguel Gómez]

Replace meaninful printks with pr_debug and remove useless ones.

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/xgifb/XGI_main_26.c |   36 ++++++++++-----------------
 drivers/staging/xgifb/vb_init.c     |   47 ++---------------------------------
 2 files changed, 15 insertions(+), 68 deletions(-)

diff --git a/drivers/staging/xgifb/XGI_main_26.c b/drivers/staging/xgifb/XGI_main_26.c
index 58cddb4..df6ef46 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -69,37 +69,37 @@ static void dumpVGAReg(void)
 
 	for (i = 0; i < 0x4f; i++) {
 		reg = xgifb_reg_get(XGISR, i);
-		printk("\no 3c4 %x", i);
-		printk("\ni 3c5 => %x", reg);
+		pr_debug("\no 3c4 %x", i);
+		pr_debug("\ni 3c5 => %x", reg);
 	}
 
 	for (i = 0; i < 0xF0; i++) {
 		reg = xgifb_reg_get(XGICR, i);
-		printk("\no 3d4 %x", i);
-		printk("\ni 3d5 => %x", reg);
+		pr_debug("\no 3d4 %x", i);
+		pr_debug("\ni 3d5 => %x", reg);
 	}
 	/*
 	xgifb_reg_set(XGIPART1,0x2F,1);
 	for (i=1; i < 0x50; i++) {
 		reg = xgifb_reg_get(XGIPART1, i);
-		printk("\no d004 %x", i);
-		printk("\ni d005 => %x", reg);
+		pr_debug("\no d004 %x", i);
+		pr_debug("\ni d005 => %x", reg);
 	}
 
 	for (i=0; i < 0x50; i++) {
 		 reg = xgifb_reg_get(XGIPART2, i);
-		 printk("\no d010 %x", i);
-		 printk("\ni d011 => %x", reg);
+		 pr_debug("\no d010 %x", i);
+		 pr_debug("\ni d011 => %x", reg);
 	}
 	for (i=0; i < 0x50; i++) {
 		reg = xgifb_reg_get(XGIPART3, i);
-		printk("\no d012 %x",i);
-		printk("\ni d013 => %x",reg);
+		pr_debug("\no d012 %x",i);
+		pr_debug("\ni d013 => %x",reg);
 	}
 	for (i=0; i < 0x50; i++) {
 		reg = xgifb_reg_get(XGIPART4, i);
-		printk("\no d014 %x",i);
-		printk("\ni d015 => %x",reg);
+		pr_debug("\no d014 %x",i);
+		pr_debug("\ni d015 => %x",reg);
 	}
 	*/
 }
@@ -1040,8 +1040,6 @@ static int XGIfb_do_set_var(struct fb_var_screeninfo *var, int isactive,
 	int old_mode;
 	/* unsigned char reg, reg1; */
 
-	/* printk(KERN_DEBUG "XGIfb:var->yres=%d, var->upper_margin=%d, var->lower_margin=%d, var->vsync_len=%d\n", var->yres, var->upper_margin, var->lower_margin, var->vsync_len); */
-
 	info->var.xres_virtual = var->xres_virtual;
 	info->var.yres_virtual = var->yres_virtual;
 	info->var.bits_per_pixel = var->bits_per_pixel;
@@ -1194,8 +1192,6 @@ static int XGIfb_pan_var(struct fb_var_screeninfo *var, struct fb_info *info)
 	struct xgifb_video_info *xgifb_info = info->par;
 	unsigned int base;
 
-	/* printk("Inside pan_var"); */
-
 	base = var->yoffset * info->var.xres_virtual + var->xoffset;
 
 	/* calculate base bpp dep. */
@@ -1229,7 +1225,6 @@ static int XGIfb_pan_var(struct fb_var_screeninfo *var, struct fb_info *info)
 				 0x7F,
 				 ((base >> 24) & 0x01) << 7);
 	}
-	/* printk("End of pan_var"); */
 	return 0;
 }
 
@@ -1342,12 +1337,10 @@ static int XGIfb_set_par(struct fb_info *info)
 {
 	int err;
 
-	/* printk("XGIfb: inside set_par\n"); */
 	err = XGIfb_do_set_var(&info->var, 1, info);
 	if (err)
 		return err;
 	XGIfb_get_fix(&info->fix, -1, info);
-	/* printk("XGIfb: end of set_par\n"); */
 	return 0;
 }
 
@@ -1501,8 +1494,6 @@ static int XGIfb_pan_display(struct fb_var_screeninfo *var,
 {
 	int err;
 
-	/* printk("\nInside pan_display:\n"); */
-
 	if (var->xoffset > (info->var.xres_virtual - info->var.xres))
 		return -EINVAL;
 	if (var->yoffset > (info->var.yres_virtual - info->var.yres))
@@ -1529,7 +1520,6 @@ static int XGIfb_pan_display(struct fb_var_screeninfo *var,
 	else
 		info->var.vmode &= ~FB_VMODE_YWRAP;
 
-	/* printk("End of pan_display\n"); */
 	return 0;
 }
 
@@ -1986,7 +1976,7 @@ static int __devinit xgifb_probe(struct pci_dev *pdev,
 		/* else if (reg >= 0xB0) {
 			hw_info->ujVBChipID = VB_CHIP_301B;
 			reg1 = xgifb_reg_get(XGIPART4, 0x23);
-			printk("XGIfb: XGI301B bridge detected\n");
+			pr_debug("XGIfb: XGI301B bridge detected\n");
 		} */
 		else {
 			hw_info->ujVBChipID = VB_CHIP_301;
diff --git a/drivers/staging/xgifb/vb_init.c b/drivers/staging/xgifb/vb_init.c
index a3d54b7..64d1311 100644
--- a/drivers/staging/xgifb/vb_init.c
+++ b/drivers/staging/xgifb/vb_init.c
@@ -1318,22 +1318,17 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 	/* Newdebugcode(0x99); */
 
 	if (pVBInfo->FBAddr == NULL) {
-		printk("\n pVBInfo->FBAddr == 0 ");
+		pr_debug("pVBInfo->FBAddr == 0\n");
 		return 0;
 	}
-	printk("1");
 	if (pVBInfo->BaseAddr == 0) {
-		printk("\npVBInfo->BaseAddr == 0 ");
+		pr_debug("pVBInfo->BaseAddr == 0\n");
 		return 0;
 	}
-	printk("2");
 
 	outb(0x67, (pVBInfo->BaseAddr + 0x12)); /* 3c2 <- 67 ,ynlai */
 
 	pVBInfo->ISXPDOS = 0;
-	printk("3");
-
-	printk("4");
 
 	/* VBIOSVersion[4] = 0x0; */
 
@@ -1356,7 +1351,6 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 	pVBInfo->Part3Port = pVBInfo->BaseAddr + SIS_CRT2_PORT_12;
 	pVBInfo->Part4Port = pVBInfo->BaseAddr + SIS_CRT2_PORT_14;
 	pVBInfo->Part5Port = pVBInfo->BaseAddr + SIS_CRT2_PORT_14 + 2;
-	printk("5");
 
 	if (HwDeviceExtension->jChipType < XG20) /* kuku 2004/06/25 */
 		/* Run XGI_GetVBType before InitTo330Pointer */
@@ -1368,7 +1362,6 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 
 	/* 1.Openkey */
 	xgifb_reg_set(pVBInfo->P3c4, 0x05, 0x86);
-	printk("6");
 
 	/* GetXG21Sense (GPIO) */
 	if (HwDeviceExtension->jChipType == XG21)
@@ -1377,8 +1370,6 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 	if (HwDeviceExtension->jChipType == XG27)
 		XGINew_GetXG27Sense(HwDeviceExtension, pVBInfo);
 
-	printk("7");
-
 	/* 2.Reset Extended register */
 
 	for (i = 0x06; i < 0x20; i++)
@@ -1390,11 +1381,8 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 	/* for(i = 0x06; i <= 0x27; i++) */
 	/* xgifb_reg_set(pVBInfo->P3c4, i, 0); */
 
-	printk("8");
-
 	for (i = 0x31; i <= 0x3B; i++)
 		xgifb_reg_set(pVBInfo->P3c4, i, 0);
-	printk("9");
 
 	/* [Hsuan] 2004/08/20 Auto over driver for XG42 */
 	if (HwDeviceExtension->jChipType == XG42)
@@ -1406,8 +1394,6 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 	for (i = 0x79; i <= 0x7C; i++)
 		xgifb_reg_set(pVBInfo->P3d4, i, 0); /* shampoo 0208 */
 
-	printk("10");
-
 	if (HwDeviceExtension->jChipType >= XG20)
 		xgifb_reg_set(pVBInfo->P3d4, 0x97, pVBInfo->XGINew_CR97);
 
@@ -1416,8 +1402,6 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 	pVBInfo->ram_type = XGINew_GetXG20DRAMType(HwDeviceExtension, pVBInfo);
 	*/
 
-	printk("11");
-
 	/* 4.SetDefExt1Regs begin */
 	xgifb_reg_set(pVBInfo->P3c4, 0x07, XGI330_SR07);
 	if (HwDeviceExtension->jChipType == XG27) {
@@ -1437,8 +1421,6 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 	/* SR11 = 0x0F; */
 	/* xgifb_reg_set(pVBInfo->P3c4, 0x11, SR11); */
 
-	printk("12");
-
 	if (HwDeviceExtension->jChipType < XG20) { /* kuku 2004/06/25 */
 		u32 Temp;
 
@@ -1484,8 +1466,6 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 		}
 		*/
 
-		printk("13");
-
 		/* Set AGP customize registers (in SetDefAGPRegs) Start */
 		for (i = 0x47; i <= 0x4C; i++)
 			xgifb_reg_set(pVBInfo->P3d4,
@@ -1514,14 +1494,12 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 
 		if (Temp == 1)
 			xgifb_reg_set(pVBInfo->P3d4, 0x48, 0x20); /* CR48 */
-		printk("14");
 	} /* != XG20 */
 
 	/* Set PCI */
 	xgifb_reg_set(pVBInfo->P3c4, 0x23, XGI330_SR23);
 	xgifb_reg_set(pVBInfo->P3c4, 0x24, XGI330_SR24);
 	xgifb_reg_set(pVBInfo->P3c4, 0x25, XGI330_SR25);
-	printk("15");
 
 	if (HwDeviceExtension->jChipType < XG20) { /* kuku 2004/06/25 */
 		/* Set VB */
@@ -1536,8 +1514,6 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 		xgifb_reg_set(pVBInfo->Part1Port,
 			      0x02, XGI330_CRT2Data_1_2);
 
-		printk("16");
-
 		xgifb_reg_set(pVBInfo->Part1Port, 0x2E, 0x08); /* use VB */
 	} /* != XG20 */
 
@@ -1557,7 +1533,6 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 		xgifb_reg_set(pVBInfo->P3c4, 0x32, XGI330_SR32);
 	}
 	xgifb_reg_set(pVBInfo->P3c4, 0x33, XGI330_SR33);
-	printk("17");
 
 	/*
 	 SetPowerConsume (HwDeviceExtension, pVBInfo->P3c4);	*/
@@ -1578,18 +1553,11 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 			XGI_LockCRT2(HwDeviceExtension, pVBInfo);
 		}
 	} /* != XG20 */
-	printk("18");
-
-	printk("181");
-
-	printk("182");
 
 	XGI_SenseCRT1(pVBInfo);
 
-	printk("183");
 	/* XGINew_DetectMonitor(HwDeviceExtension); */
 	if (HwDeviceExtension->jChipType == XG21) {
-		printk("186");
 
 		xgifb_reg_and_or(pVBInfo->P3d4,
 				 0x32,
@@ -1597,7 +1565,6 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 				 Monitor1Sense); /* Z9 default has CRT */
 		temp = GetXG21FPBits(pVBInfo);
 		xgifb_reg_and_or(pVBInfo->P3d4, 0x37, ~0x01, temp);
-		printk("187");
 
 	}
 	if (HwDeviceExtension->jChipType == XG27) {
@@ -1608,7 +1575,6 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 		temp = GetXG27FPBits(pVBInfo);
 		xgifb_reg_and_or(pVBInfo->P3d4, 0x37, ~0x03, temp);
 	}
-	printk("19");
 
 	pVBInfo->ram_type = XGINew_GetXG20DRAMType(HwDeviceExtension, pVBInfo);
 
@@ -1616,11 +1582,7 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 					 pVBInfo->P3d4,
 					 pVBInfo);
 
-	printk("20");
 	XGINew_SetDRAMSize_340(xgifb_info, HwDeviceExtension, pVBInfo);
-	printk("21");
-
-	printk("22");
 
 	/* SetDefExt2Regs begin */
 	/*
@@ -1651,16 +1613,11 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 
 	xgifb_reg_set(pVBInfo->P3c4, 0x21, pVBInfo->SR21);
 
-	printk("23");
-
 	XGINew_ChkSenseStatus(HwDeviceExtension, pVBInfo);
 	XGINew_SetModeScratch(HwDeviceExtension, pVBInfo);
 
-	printk("24");
-
 	xgifb_reg_set(pVBInfo->P3d4, 0x8c, 0x87);
 	xgifb_reg_set(pVBInfo->P3c4, 0x14, 0x31);
-	printk("25");
 
 	return 1;
 } /* end of init */
-- 
1.7.9.5

[PATCH 7/8] Staging: xgifb: Fix sparse warnings.

[Miguel Gómez]

Add include files with function definitions to avoid sparse warnings.

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/xgifb/vb_init.c    |    1 +
 drivers/staging/xgifb/vb_setmode.c |    2 +-
 drivers/staging/xgifb/vb_util.c    |    1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/xgifb/vb_init.c b/drivers/staging/xgifb/vb_init.c
index 64d1311..143957f 100644
--- a/drivers/staging/xgifb/vb_init.c
+++ b/drivers/staging/xgifb/vb_init.c
@@ -5,6 +5,7 @@
 #include "vb_def.h"
 #include "vb_util.h"
 #include "vb_setmode.h"
+#include "vb_init.h"
 static const unsigned short XGINew_DDRDRAM_TYPE340[4][2] = {
 	{ 16, 0x45},
 	{  8, 0x35},
diff --git a/drivers/staging/xgifb/vb_setmode.c b/drivers/staging/xgifb/vb_setmode.c
index b1713d3..479362e 100644
--- a/drivers/staging/xgifb/vb_setmode.c
+++ b/drivers/staging/xgifb/vb_setmode.c
@@ -4,7 +4,7 @@
 #include "vb_def.h"
 #include "vb_util.h"
 #include "vb_table.h"
-
+#include "vb_setmode.h"
 
 #define  IndexMask 0xff
 #define TVCLKBASE_315_25 (TVCLKBASE_315 + 25)
diff --git a/drivers/staging/xgifb/vb_util.c b/drivers/staging/xgifb/vb_util.c
index 5c93a22..1b452f8 100644
--- a/drivers/staging/xgifb/vb_util.c
+++ b/drivers/staging/xgifb/vb_util.c
@@ -1,4 +1,5 @@
 #include "vgatypes.h"
+#include "vb_util.h"
 
 void xgifb_reg_set(unsigned long port, u8 index, u8 data)
 {
-- 
1.7.9.5

[PATCH 8/8] Staging: xgifb: Replace pr_* with dev_* when possible.

[Miguel Gómez]

Replace pr_* calls with dev_* ones when the device structure is available.

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/xgifb/XGI_main_26.c |   72 +++++++++++++++++++++--------------
 drivers/staging/xgifb/vb_init.c     |    4 +-
 2 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/xgifb/XGI_main_26.c b/drivers/staging/xgifb/XGI_main_26.c
index df6ef46..fe9061d 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -1838,9 +1838,9 @@ static int __devinit xgifb_probe(struct pci_dev *pdev,
 	xgifb_info->mmio_base = pci_resource_start(pdev, 1);
 	xgifb_info->mmio_size = pci_resource_len(pdev, 1);
 	xgifb_info->vga_base = pci_resource_start(pdev, 2) + 0x30;
-	pr_info("Relocate IO address: %Lx [%08lx]\n",
-	       (u64) pci_resource_start(pdev, 2),
-	       xgifb_info->vga_base);
+	dev_info(&pdev->dev, "Relocate IO address: %Lx [%08lx]\n",
+		 (u64) pci_resource_start(pdev, 2),
+		 xgifb_info->vga_base);
 
 	if (pci_enable_device(pdev)) {
 		ret = -EIO;
@@ -1858,7 +1858,7 @@ static int __devinit xgifb_probe(struct pci_dev *pdev,
 	reg1 = xgifb_reg_get(XGISR, IND_SIS_PASSWORD);
 
 	if (reg1 != 0xa1) { /*I/O error */
-		pr_err("I/O error!!!");
+		dev_err(&pdev->dev, "I/O error!!!");
 		ret = -EIO;
 		goto error_disable;
 	}
@@ -1886,11 +1886,12 @@ static int __devinit xgifb_probe(struct pci_dev *pdev,
 		goto error_disable;
 	}
 
-	pr_info("chipid = %x\n", xgifb_info->chip);
+	dev_info(&pdev->dev, "chipid = %x\n", xgifb_info->chip);
 	hw_info->jChipType = xgifb_info->chip;
 
 	if (XGIfb_get_dram_size(xgifb_info)) {
-		pr_err("Fatal error: Unable to determine RAM size.\n");
+		dev_err(&pdev->dev,
+			"Fatal error: Unable to determine RAM size.\n");
 		ret = -ENODEV;
 		goto error_disable;
 	}
@@ -1907,10 +1908,11 @@ static int __devinit xgifb_probe(struct pci_dev *pdev,
 	if (!request_mem_region(xgifb_info->video_base,
 				xgifb_info->video_size,
 				"XGIfb FB")) {
-		pr_err("unable request memory size %x\n",
+		dev_err(&pdev->dev, "unable request memory size %x\n",
 		       xgifb_info->video_size);
-		pr_err("Fatal error: Unable to reserve frame buffer memory\n");
-		pr_err("Is there another framebuffer driver active?\n");
+		dev_err(&pdev->dev,
+			"Fatal error: Unable to reserve frame buffer memory. "
+			"Is there another framebuffer driver active?\n");
 		ret = -ENODEV;
 		goto error_disable;
 	}
@@ -1918,7 +1920,8 @@ static int __devinit xgifb_probe(struct pci_dev *pdev,
 	if (!request_mem_region(xgifb_info->mmio_base,
 				xgifb_info->mmio_size,
 				"XGIfb MMIO")) {
-		pr_err("Fatal error: Unable to reserve MMIO region\n");
+		dev_err(&pdev->dev,
+			"Fatal error: Unable to reserve MMIO region\n");
 		ret = -ENODEV;
 		goto error_0;
 	}
@@ -1928,18 +1931,20 @@ static int __devinit xgifb_probe(struct pci_dev *pdev,
 	xgifb_info->mmio_vbase = ioremap(xgifb_info->mmio_base,
 					    xgifb_info->mmio_size);
 
-	pr_info("Framebuffer at 0x%Lx, mapped to 0x%p, size %dk\n",
-	       (u64) xgifb_info->video_base,
-	       xgifb_info->video_vbase,
-	       xgifb_info->video_size / 1024);
+	dev_info(&pdev->dev,
+		 "Framebuffer at 0x%Lx, mapped to 0x%p, size %dk\n",
+		 (u64) xgifb_info->video_base,
+		 xgifb_info->video_vbase,
+		 xgifb_info->video_size / 1024);
 
-	pr_info("MMIO at 0x%Lx, mapped to 0x%p, size %ldk\n",
-	       (u64) xgifb_info->mmio_base, xgifb_info->mmio_vbase,
-	       xgifb_info->mmio_size / 1024);
+	dev_info(&pdev->dev,
+		 "MMIO at 0x%Lx, mapped to 0x%p, size %ldk\n",
+		 (u64) xgifb_info->mmio_base, xgifb_info->mmio_vbase,
+		 xgifb_info->mmio_size / 1024);
 
 	pci_set_drvdata(pdev, xgifb_info);
 	if (!XGIInitNew(pdev))
-		pr_err("XGIInitNew() failed!\n");
+		dev_err(&pdev->dev, "XGIInitNew() failed!\n");
 
 	xgifb_info->mtrr = (unsigned int) 0;
 
@@ -1968,10 +1973,14 @@ static int __devinit xgifb_probe(struct pci_dev *pdev,
 		reg = xgifb_reg_get(XGIPART4, 0x01);
 		if (reg >= 0xE0) {
 			hw_info->ujVBChipID = VB_CHIP_302LV;
-			pr_info("XGI302LV bridge detected (revision 0x%02x)\n", reg);
+			dev_info(&pdev->dev,
+				 "XGI302LV bridge detected (revision 0x%02x)\n",
+				 reg);
 		} else if (reg >= 0xD0) {
 			hw_info->ujVBChipID = VB_CHIP_301LV;
-			pr_info("XGI301LV bridge detected (revision 0x%02x)\n", reg);
+			dev_info(&pdev->dev,
+				 "XGI301LV bridge detected (revision 0x%02x)\n",
+				 reg);
 		}
 		/* else if (reg >= 0xB0) {
 			hw_info->ujVBChipID = VB_CHIP_301B;
@@ -1980,17 +1989,21 @@ static int __devinit xgifb_probe(struct pci_dev *pdev,
 		} */
 		else {
 			hw_info->ujVBChipID = VB_CHIP_301;
-			pr_info("XGI301 bridge detected\n");
+			dev_info(&pdev->dev, "XGI301 bridge detected\n");
 		}
 		break;
 	case HASVB_302:
 		reg = xgifb_reg_get(XGIPART4, 0x01);
 		if (reg >= 0xE0) {
 			hw_info->ujVBChipID = VB_CHIP_302LV;
-			pr_info("XGI302LV bridge detected (revision 0x%02x)\n", reg);
+			dev_info(&pdev->dev,
+				 "XGI302LV bridge detected (revision 0x%02x)\n",
+				 reg);
 		} else if (reg >= 0xD0) {
 			hw_info->ujVBChipID = VB_CHIP_301LV;
-			pr_info("XGI302LV bridge detected (revision 0x%02x)\n", reg);
+			dev_info(&pdev->dev,
+				 "XGI302LV bridge detected (revision 0x%02x)\n",
+				 reg);
 		} else if (reg >= 0xB0) {
 			reg1 = xgifb_reg_get(XGIPART4, 0x23);
 
@@ -1998,27 +2011,28 @@ static int __devinit xgifb_probe(struct pci_dev *pdev,
 
 		} else {
 			hw_info->ujVBChipID = VB_CHIP_302;
-			pr_info("XGI302 bridge detected\n");
+			dev_info(&pdev->dev, "XGI302 bridge detected\n");
 		}
 		break;
 	case HASVB_LVDS:
 		hw_info->ulExternalChip = 0x1;
-		pr_info("LVDS transmitter detected\n");
+		dev_info(&pdev->dev, "LVDS transmitter detected\n");
 		break;
 	case HASVB_TRUMPION:
 		hw_info->ulExternalChip = 0x2;
-		pr_info("Trumpion Zurac LVDS scaler detected\n");
+		dev_info(&pdev->dev, "Trumpion Zurac LVDS scaler detected\n");
 		break;
 	case HASVB_CHRONTEL:
 		hw_info->ulExternalChip = 0x4;
-		pr_info("Chrontel TV encoder detected\n");
+		dev_info(&pdev->dev, "Chrontel TV encoder detected\n");
 		break;
 	case HASVB_LVDS_CHRONTEL:
 		hw_info->ulExternalChip = 0x5;
-		pr_info("LVDS transmitter and Chrontel TV encoder detected\n");
+		dev_info(&pdev->dev,
+			 "LVDS transmitter and Chrontel TV encoder detected\n");
 		break;
 	default:
-		pr_info("No or unknown bridge type detected\n");
+		dev_info(&pdev->dev, "No or unknown bridge type detected\n");
 		break;
 	}
 
diff --git a/drivers/staging/xgifb/vb_init.c b/drivers/staging/xgifb/vb_init.c
index 143957f..7c555a7 100644
--- a/drivers/staging/xgifb/vb_init.c
+++ b/drivers/staging/xgifb/vb_init.c
@@ -1319,11 +1319,11 @@ unsigned char XGIInitNew(struct pci_dev *pdev)
 	/* Newdebugcode(0x99); */
 
 	if (pVBInfo->FBAddr == NULL) {
-		pr_debug("pVBInfo->FBAddr == 0\n");
+		dev_dbg(&pdev->dev, "pVBInfo->FBAddr == 0\n");
 		return 0;
 	}
 	if (pVBInfo->BaseAddr == 0) {
-		pr_debug("pVBInfo->BaseAddr == 0\n");
+		dev_dbg(&pdev->dev, "pVBInfo->BaseAddr == 0\n");
 		return 0;
 	}
 
-- 
1.7.9.5

Re: [PATCH 3/8] Staging: xgifb: remove DPRINTK() macro and replace it with pr_debug.

[Dan Carpenter]

On Mon, Jun 18, 2012 at 01:12:01PM +0200, Miguel Gómez wrote:
>  			} else if (XGIfb_vrate[i].refresh > rate) {
>  				if ((XGIfb_vrate[i].refresh - rate) <= 3) {
> -					DPRINTK("XGIfb: Adjusting rate from %d up to %d\n",
> -						rate, XGIfb_vrate[i].refresh);
> +					pr_debug("XGIfb: Adjusting rate from %d up to %d\n",
> +						 rate, XGIfb_vrate[i].refresh);

This file already has a pr_fmt set so if you wanted, you could just
remove the "XGIfb: ".  This was already there in the original code
so don't resend.  But if you want to send another patch which
removes them that would be grand.

regards,
dan carpenter

Re: [PATCH 5/8] Staging: xgifb: Remove XGIFAIL() macro and its calls.

[Dan Carpenter]

On Mon, Jun 18, 2012 at 01:12:03PM +0200, Miguel Gómez wrote:
> --- a/drivers/staging/xgifb/XGI_main_26.c
> +++ b/drivers/staging/xgifb/XGI_main_26.c
> @@ -1376,8 +1376,10 @@ static int XGIfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
>  		vtotal = var->upper_margin + var->yres + var->lower_margin
>  				+ var->vsync_len;
>  
> -	if (!(htotal) || !(vtotal))
> -		XGIFAIL("XGIfb: no valid timing data");
> +	if (!(htotal) || !(vtotal)) {

Don't resend this, but next time feel free to write this like:

	if (!htotal || !vtotal) {

We do have a one change per patch rule but tiny formatting things
like that don't count as a change.

regards,
dan carpenter

> +		pr_debug("XGIfb: no valid timing data\n");
> +		return -EINVAL;
> +	}
>  
>  	if (var->pixclock && htotal && vtotal) {
>  		drate = 1000000000 / var->pixclock;

Re: [PATCH 5/8] Staging: xgifb: Remove XGIFAIL() macro and its calls.

[Miguel Gómez]

>> -	if (!(htotal) || !(vtotal))
>> -		XGIFAIL("XGIfb: no valid timing data");
>> +	if (!(htotal) || !(vtotal)) {
>
> Don't resend this, but next time feel free to write this like:
>
> 	if (!htotal || !vtotal) {
>
> We do have a one change per patch rule but tiny formatting things
> like that don't count as a change.

Sure. I'll write it down together with your other comment to my next 
round of patches ;)

Regards!

-- 
Miguel Gómez
Igalia - http://www.igalia.com