Re: [PATCH] zr36067: Debugging cleanups (updated again)

On Wed, 23 May 2007 09:12:38 -0400, Ronald S. Bultje wrote:
> On May 23, 2007, at 9:06 AM, Jean Delvare wrote:
> > On Wed, 23 May 2007 08:17:46 -0400, Ronald S. Bultje wrote:
> >> On May 23, 2007, at 7:35 AM, Jean Delvare wrote:
> >>> * The debug level is between 0 and 5, not 0 and 4.
> >>
> >> This part should not be applied though, since the only use of 5 is an
> >> easter egg, and I think the range use is only descriptive. Can you
> >> attach your next patch so I can pick it and move it to LKML? The rest
> >> of the patch is fine, so let's send it in.
> >
> > I count 5 occurrences of dprintk(5, ...) in the driver, one is indeed
> > an Easter Egg, but the others aren't.
> 
> Ugh, I suck. Let's make that egg a 6 then. :-).

OK, I've done that. New patch below.

> Otherwise I like the patch (and the size decrease along with it :-) ).

And I have a couple more to come, but I'll let Trent commit his changes
first so that my cleanups don't get in the way of his more important
fixes.

Thanks.

* * * * *

Debugging cleanups to the zr36067 driver:

* Use module_param_named() to declare the debug parameter, so we can
  use a single global variable to handle the debug level. This makes
  the driver a bit smaller (by 648 bytes on x86_64), thanks to one
  less level of indirection on every use.

* Change the debug parameter sysfs permissions, so that the debug
  level can be adjusted at runtime, as is done in many other
  media/video drivers.

* The debug level is between 0 and 5, not 0 and 4.

* Move the zr_debug export and dprintk macro definition to a header
  file so that we don't have to define them in each source file.

* Simplify a duplicate test on zr_debug.

Note that zr_debug was subsequently renamed to debug_zr36067 to avoid
possible conflicts with other Zoran device drivers, on a suggestion
by Trent Piepho.

Signed-off-by: Jean Delvare <[email protected]>
Cc: Ronald S. Bultje <[email protected]>
Cc: Trent Piepho <[email protected]>
---
 drivers/media/video/zoran_card.c   |   19 ++++++-------------
 drivers/media/video/zoran_card.h   |    8 ++++++++
 drivers/media/video/zoran_device.c |   17 +++++------------
 drivers/media/video/zoran_driver.c |   20 +++++---------------
 drivers/media/video/zoran_procfs.c |    9 +--------
 5 files changed, 25 insertions(+), 48 deletions(-)

--- linux-2.6.22-rc2.orig/drivers/media/video/zoran_card.c	2007-05-23 10:21:06.000000000 +0200
+++ linux-2.6.22-rc2/drivers/media/video/zoran_card.c	2007-05-23 15:25:31.000000000 +0200
@@ -145,10 +145,9 @@ module_param(pass_through, int, 0);
 MODULE_PARM_DESC(pass_through,
 		 "Pass TV signal through to TV-out when idling");
 
-static int debug = 1;
-int *zr_debug = &debug;
-module_param(debug, int, 0);
-MODULE_PARM_DESC(debug, "Debug level (0-4)");
+int zr36067_debug = 1;
+module_param_named(debug, zr36067_debug, int, 0644);
+MODULE_PARM_DESC(debug, "Debug level (0-5)");
 
 MODULE_DESCRIPTION("Zoran-36057/36067 JPEG codec driver");
 MODULE_AUTHOR("Serguei Miridonov");
@@ -161,12 +160,6 @@ static struct pci_device_id zr36067_pci_
 };
 MODULE_DEVICE_TABLE(pci, zr36067_pci_tbl);
 
-#define dprintk(num, format, args...) \
-	do { \
-		if (*zr_debug >= num) \
-			printk(format, ##args); \
-	} while (0)
-
 int zoran_num;			/* number of Buzs in use */
 struct zoran zoran[BUZ_MAX];
 
@@ -1075,7 +1068,7 @@ test_interrupts (struct zoran *zr)
 	if (timeout) {
 		dprintk(1, ": time spent: %d\n", 1 * HZ - timeout);
 	}
-	if (*zr_debug > 1)
+	if (zr36067_debug > 1)
 		print_interrupts(zr);
 	btwrite(icr, ZR36057_ICR);
 }
@@ -1158,7 +1151,7 @@ zr36057_init (struct zoran *zr)
 		goto exit_unregister;
 
 	zoran_init_hardware(zr);
-	if (*zr_debug > 2)
+	if (zr36067_debug > 2)
 		detect_guest_activity(zr);
 	test_interrupts(zr);
 	if (!pass_through) {
@@ -1620,7 +1613,7 @@ init_dc10_cards (void)
 	}
 
 	/* random nonsense */
-	dprintk(5, KERN_DEBUG "Jotti is een held!\n");
+	dprintk(6, KERN_DEBUG "Jotti is een held!\n");
 
 	/* some mainboards might not do PCI-PCI data transfer well */
 	if (pci_pci_problems & (PCIPCI_FAIL|PCIAGP_FAIL|PCIPCI_ALIMAGIK)) {
--- linux-2.6.22-rc2.orig/drivers/media/video/zoran_card.h	2007-05-23 10:21:06.000000000 +0200
+++ linux-2.6.22-rc2/drivers/media/video/zoran_card.h	2007-05-23 13:28:27.000000000 +0200
@@ -30,6 +30,14 @@
 #ifndef __ZORAN_CARD_H__
 #define __ZORAN_CARD_H__
 
+extern int zr36067_debug;
+
+#define dprintk(num, format, args...) \
+	do { \
+		if (zr36067_debug >= num) \
+			printk(format, ##args); \
+	} while (0)
+
 /* Anybody who uses more than four? */
 #define BUZ_MAX 4
 extern int zoran_num;
--- linux-2.6.22-rc2.orig/drivers/media/video/zoran_device.c	2007-05-23 10:21:06.000000000 +0200
+++ linux-2.6.22-rc2/drivers/media/video/zoran_device.c	2007-05-23 13:28:27.000000000 +0200
@@ -52,6 +52,7 @@
 #include "videocodec.h"
 #include "zoran.h"
 #include "zoran_device.h"
+#include "zoran_card.h"
 
 #define IRQ_MASK ( ZR36057_ISR_GIRQ0 | \
 		   ZR36057_ISR_GIRQ1 | \
@@ -59,14 +60,6 @@
 
 extern const struct zoran_format zoran_formats[];
 
-extern int *zr_debug;
-
-#define dprintk(num, format, args...) \
-	do { \
-		if (*zr_debug >= num) \
-			printk(format, ##args); \
-	} while (0)
-
 static int lml33dpath = 0;	/* 1 will use digital path in capture
 				 * mode instead of analog. It can be
 				 * used for picture adjustments using
@@ -174,7 +167,7 @@ post_office_read (struct zoran *zr,
 static void
 dump_guests (struct zoran *zr)
 {
-	if (*zr_debug > 2) {
+	if (zr36067_debug > 2) {
 		int i, guest[8];
 
 		for (i = 1; i < 8; i++) {	// Don't read jpeg codec here
@@ -1295,7 +1288,7 @@ error_handler (struct zoran *zr,
 		zr->num_errors++;
 
 		/* Report error */
-		if (*zr_debug > 1 && zr->num_errors <= 8) {
+		if (zr36067_debug > 1 && zr->num_errors <= 8) {
 			long frame;
 			frame =
 			    zr->jpg_pend[zr->jpg_dma_tail & BUZ_MASK_FRAME];
@@ -1555,7 +1548,7 @@ zoran_irq (int             irq,
 
 			if (zr->codec_mode == BUZ_MODE_MOTION_DECOMPRESS ||
 			    zr->codec_mode == BUZ_MODE_MOTION_COMPRESS) {
-				if (*zr_debug > 1 &&
+				if (zr36067_debug > 1 &&
 				    (!zr->frame_num || zr->JPEG_error)) {
 					printk(KERN_INFO
 					       "%s: first frame ready: state=0x%08x odd_even=%d field_per_buff=%d delay=%d\n",
@@ -1592,7 +1585,7 @@ zoran_irq (int             irq,
 						    zr->JPEG_missed;
 				}
 
-				if (*zr_debug > 2 && zr->frame_num < 6) {
+				if (zr36067_debug > 2 && zr->frame_num < 6) {
 					int i;
 					printk("%s: seq=%ld stat_com:",
 					       ZR_DEVNAME(zr), zr->jpg_seq_num);
--- linux-2.6.22-rc2.orig/drivers/media/video/zoran_procfs.c	2007-05-23 10:21:06.000000000 +0200
+++ linux-2.6.22-rc2/drivers/media/video/zoran_procfs.c	2007-05-23 13:26:23.000000000 +0200
@@ -48,14 +48,7 @@
 #include "videocodec.h"
 #include "zoran.h"
 #include "zoran_procfs.h"
-
-extern int *zr_debug;
-
-#define dprintk(num, format, args...) \
-	do { \
-		if (*zr_debug >= num) \
-			printk(format, ##args); \
-	} while (0)
+#include "zoran_card.h"
 
 #ifdef CONFIG_PROC_FS
 struct procfs_params_zr36067 {
--- linux-2.6.22-rc2.orig/drivers/media/video/zoran_driver.c	2007-05-23 10:21:06.000000000 +0200
+++ linux-2.6.22-rc2/drivers/media/video/zoran_driver.c	2007-05-23 13:28:27.000000000 +0200
@@ -192,14 +192,6 @@ static const int zoran_num_formats =
 #   include <linux/bigphysarea.h>
 #endif
 
-extern int *zr_debug;
-
-#define dprintk(num, format, args...) \
-	do { \
-		if (*zr_debug >= num) \
-			printk(format, ##args);	\
-	} while (0)
-
 extern int v4l_nbufs;
 extern int v4l_bufsize;
 extern int jpg_nbufs;
@@ -1154,12 +1146,10 @@ jpg_sync (struct file       *file,
 		frame = zr->jpg_pend[zr->jpg_que_tail & BUZ_MASK_FRAME];
 
 	/* buffer should now be in BUZ_STATE_DONE */
-	if (*zr_debug > 0)
-		if (zr->jpg_buffers.buffer[frame].state != BUZ_STATE_DONE)
-			dprintk(2,
-				KERN_ERR
-				"%s: jpg_sync() - internal state error\n",
-				ZR_DEVNAME(zr));
+	if (zr->jpg_buffers.buffer[frame].state != BUZ_STATE_DONE)
+		dprintk(2,
+			KERN_ERR "%s: jpg_sync() - internal state error\n",
+			ZR_DEVNAME(zr));
 
 	*bs = zr->jpg_buffers.buffer[frame].bs;
 	bs->frame = frame;
@@ -1433,7 +1423,7 @@ zoran_close (struct inode *inode,
 		/* disable interrupts */
 		btand(~ZR36057_ICR_IntPinEn, ZR36057_ICR);
 
-		if (*zr_debug > 1)
+		if (zr36067_debug > 1)
 			print_interrupts(zr);
 
 		/* Overlay off */


-- 
Jean Delvare

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/