All of lore.kernel.org
 help / color / mirror / Atom feed
* [Kernel-janitors] [PATCH] remove check_region specialix
@ 2004-07-07 20:24 Kristen Carlson
  2004-07-07 20:40 ` Matthew Wilcox
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Kristen Carlson @ 2004-07-07 20:24 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 9495 bytes --]

Hi, please comment on this patch. It is against 2.6.7

Description: removes check_region call from specialix.c and cleans
up some debug print messages and some dead code.
Signed-off-by: Kristen Carlson Accardi <kristenc@cs.pdx.edu>


--- specialix.c.orig	2004-07-07 12:56:03.000000000 +0000
+++ specialix.c	2004-07-07 13:01:06.000000000 +0000
@@ -118,6 +118,13 @@
 #undef SX_REPORT_OVERRUN
 
 
+#ifdef  SPECIALIX_DEBUG
+static int debug = SPECIALIX_DEBUG;
+#define dprintk(level, fmt, arg...) do { if (level >= debug) printk(KERN_DEBUG "%s: %s: " fmt , MY_NAME , __FUNCTION__ , ## arg); } while (0)
+#else
+static int debug = 0;
+#define dprintk(level, fmt, arg...)
+#endif
 
 #ifdef CONFIG_SPECIALIX_RTSCTS
 #define SX_CRTSCTS(bla) 1
@@ -291,9 +298,9 @@ static inline int sx_check_io_range(stru
 }
 
 
-static inline void sx_request_io_range(struct specialix_board * bp)
+static inline int sx_request_io_range(struct specialix_board * bp)
 {
-	request_region(bp->base, 
+	return request_region(bp->base, 
 	               bp->flags&SX_BOARD_IS_PCI?SX_PCI_IO_SPACE:SX_IO_SPACE,
 	               "specialix IO8+" );
 }
@@ -419,14 +426,11 @@ void missed_irq (unsigned long data)
 static int sx_probe(struct specialix_board *bp)
 {
 	unsigned char val1, val2;
-#if 0
-	int irqs = 0;
-	int retries;
-#endif
 	int rev;
 	int chip;
+	int err;
 
-	if (sx_check_io_range(bp)) 
+	if (!sx_request_io_region(bp))
 		return 1;
 
 	/* Are the I/O ports here ? */
@@ -442,17 +446,16 @@ static int sx_probe(struct specialix_boa
 	if ((val1 != 0x5a) || (val2 != 0xa5)) {
 		printk(KERN_INFO "sx%d: specialix IO8+ Board at 0x%03x not found.\n",
 		       board_No(bp), bp->base);
-		return 1;
+		err = 1;
+		goto probe_err;
 	}
 
 	/* Check the DSR lines that Specialix uses as board 
 	   identification */
 	val1 = read_cross_byte (bp, CD186x_MSVR, MSVR_DSR);
 	val2 = read_cross_byte (bp, CD186x_MSVR, MSVR_RTS);
-#ifdef SPECIALIX_DEBUG
-	printk (KERN_DEBUG "sx%d: DSR lines are: %02x, rts lines are: %02x\n", 
+	dprintk ("sx%d: DSR lines are: %02x, rts lines are: %02x\n", 
 	        board_No(bp),  val1, val2);
-#endif
 	/* They managed to switch the bit order between the docs and
 	   the IO8+ card. The new PCI card now conforms to old docs.
 	   They changed the PCI docs to reflect the situation on the
@@ -461,57 +464,16 @@ static int sx_probe(struct specialix_boa
 	if (val1 != val2) {
 		printk(KERN_INFO "sx%d: specialix IO8+ ID %02x at 0x%03x not found (%02x).\n",
 		       board_No(bp), val2, bp->base, val1);
-		return 1;
+		err = 1;
+		goto probe_err;
 	}
 
-
-#if 0
-	/* It's time to find IRQ for this board */
-	for (retries = 0; retries < 5 && irqs <= 0; retries++) {
-		irqs = probe_irq_on();
-		sx_init_CD186x(bp);	       		/* Reset CD186x chip       */
-		sx_out(bp, CD186x_CAR, 2);               /* Select port 2          */
-		sx_wait_CCR(bp);
-		sx_out(bp, CD186x_CCR, CCR_TXEN);        /* Enable transmitter     */
-		sx_out(bp, CD186x_IER, IER_TXRDY);       /* Enable tx empty intr   */
-		sx_long_delay(HZ/20);	       		
-		irqs = probe_irq_off(irqs);
-
-#if SPECIALIX_DEBUG > 2
-		printk (KERN_DEBUG "SRSR = %02x, ",  sx_in(bp, CD186x_SRSR));
-		printk (           "TRAR = %02x, ",  sx_in(bp, CD186x_TRAR));
-		printk (           "GIVR = %02x, ",  sx_in(bp, CD186x_GIVR));
-		printk (           "GICR = %02x, ",  sx_in(bp, CD186x_GICR));
-		printk (           "\n");
-#endif
-		/* Reset CD186x again      */
-		if (!sx_init_CD186x(bp)) {
-			/* Hmmm. This is dead code anyway. */
-		}
-#if SPECIALIX_DEBUG > 2
-		printk (KERN_DEBUG "val1 = %02x, val2 = %02x, val3 = %02x.\n", 
-		        val1, val2, val3); 
-#endif
-	
-	}
-	
-#if 0
-	if (irqs <= 0) {
-		printk(KERN_ERR "sx%d: Can't find IRQ for specialix IO8+ board at 0x%03x.\n",
-		       board_No(bp), bp->base);
-		return 1;
-	}
-#endif
-	printk (KERN_INFO "Started with irq=%d, but now have irq=%d.\n", bp->irq, irqs);
-	if (irqs > 0)
-		bp->irq = irqs;
-#endif
 	/* Reset CD186x again  */
 	if (!sx_init_CD186x(bp)) {
-		return -EIO;
+		err = -EIO;
+		goto probe_err;
 	}
 
-	sx_request_io_range(bp);
 	bp->flags |= SX_BOARD_PRESENT;
 	
 	/* Chip           revcode   pkgtype
@@ -532,9 +494,7 @@ static int sx_probe(struct specialix_boa
 	default:chip=-1;rev='x';
 	}
 
-#if SPECIALIX_DEBUG > 2
-	printk (KERN_DEBUG " GFCR = 0x%02x\n", sx_in_off(bp, CD186x_GFRCR) );
-#endif
+	dprintk(2, " GFCR = 0x%02x\n", sx_in_off(bp, CD186x_GFRCR) );
 
 #ifdef SPECIALIX_TIMER
 	init_timer (&missed_irq_timer);
@@ -550,6 +510,10 @@ static int sx_probe(struct specialix_boa
 	       chip, rev);
 
 	return 0;
+
+ probe_err:
+	sx_release_io_range(bp);
+	return err;
 }
 
 /* 
@@ -604,10 +568,8 @@ static inline void sx_receive_exc(struct
 	status = sx_in(bp, CD186x_RCSR);
 	if (status & RCSR_OE) {
 		port->overrun++;
-#if SPECIALIX_DEBUG 
-		printk(KERN_DEBUG "sx%d: port %d: Overrun. Total %ld overruns.\n", 
-		       board_No(bp), port_No(port), port->overrun);
-#endif		
+		dprintk(debug, "sx%d: port %d: Overrun. Total %ld overruns.\n", 
+		       board_No(bp), port_No(port), port->overrun);		
 	}
 	status &= port->mark_mask;
 #else	
@@ -623,10 +585,8 @@ static inline void sx_receive_exc(struct
 		return;
 		
 	} else if (status & RCSR_BREAK) {
-#ifdef SPECIALIX_DEBUG
-		printk(KERN_DEBUG "sx%d: port %d: Handling break...\n",
+		dprintk(debug, "sx%d: port %d: Handling break...\n",
 		       board_No(bp), port_No(port));
-#endif
 		*tty->flip.flag_buf_ptr++ = TTY_BREAK;
 		if (port->flags & ASYNC_SAK)
 			do_SAK(tty);
@@ -756,9 +716,8 @@ static inline void sx_check_modem(struct
 	struct tty_struct *tty;
 	unsigned char mcr;
 
-#ifdef SPECIALIX_DEBUG
-	printk (KERN_DEBUG "Modem intr. ");
-#endif
+	dprintk (debug, "Modem intr. ");
+
 	if (!(port = sx_get_port(bp, "Modem")))
 		return;
 	
@@ -768,18 +727,12 @@ static inline void sx_check_modem(struct
 	printk ("mcr = %02x.\n", mcr);
 
 	if ((mcr & MCR_CDCHG)) {
-#ifdef SPECIALIX_DEBUG 
-		printk (KERN_DEBUG "CD just changed... ");
-#endif
+		dprintk (debug, "CD just changed... ");
 		if (sx_in(bp, CD186x_MSVR) & MSVR_CD) {
-#ifdef SPECIALIX_DEBUG
-			printk ( "Waking up guys in open.\n");
-#endif
+			dprintk (debug, "Waking up guys in open.\n");
 			wake_up_interruptible(&port->open_wait);
 		} else {
-#ifdef SPECIALIX_DEBUG
-			printk ( "Sending HUP.\n");
-#endif
+			dprintk (debug, "Sending HUP.\n");
 			schedule_work(&port->tqueue_hangup);
 		}
 	}
@@ -828,9 +781,7 @@ static irqreturn_t sx_interrupt(int irq,
 	bp = dev_id;
 	
 	if (!bp || !(bp->flags & SX_BOARD_ACTIVE)) {
-#ifdef SPECIALIX_DEBUG 
-		printk (KERN_DEBUG "sx: False interrupt. irq %d.\n", irq);
-#endif
+		dprintk (debug, "sx: False interrupt. irq %d.\n", irq);
 		return IRQ_NONE;
 	}
 
@@ -933,9 +884,8 @@ static inline void sx_shutdown_board(str
 	
 	bp->flags &= ~SX_BOARD_ACTIVE;
 	
-#if SPECIALIX_DEBUG > 2
-	printk ("Freeing IRQ%d for board %d.\n", bp->irq, board_No (bp));
-#endif
+	dprintk (3, "Freeing IRQ%d for board %d.\n", bp->irq, board_No (bp));
+
 	free_irq(bp->irq, bp);
 
 	turn_ints_off (bp);
@@ -970,9 +920,7 @@ static void sx_change_speed(struct speci
 		port->MSVR = MSVR_DTR | (sx_in(bp, CD186x_MSVR) & MSVR_RTS);
 	else
 		port->MSVR =  (sx_in(bp, CD186x_MSVR) & MSVR_RTS);
-#ifdef DEBUG_SPECIALIX
-	printk (KERN_DEBUG "sx: got MSVR=%02x.\n", port->MSVR);
-#endif
+	dprintk(debug, "sx: got MSVR=%02x.\n", port->MSVR);
 	baud = C_BAUD(tty);
 	
 	if (baud & CBAUDEX) {
@@ -992,17 +940,13 @@ static void sx_change_speed(struct speci
 	
 	if (!baud_table[baud]) {
 		/* Drop DTR & exit */
-#ifdef SPECIALIX_DEBUG
-		printk (KERN_DEBUG "Dropping DTR...  Hmm....\n");
-#endif
+		dprintk(debug, "Dropping DTR...  Hmm....\n");
 		if (!SX_CRTSCTS (tty)) {
 			port -> MSVR &= ~ MSVR_DTR;
 			sx_out(bp, CD186x_MSVR, port->MSVR );
 		} 
-#ifdef DEBUG_SPECIALIX
 		else
-			printk (KERN_DEBUG "Can't drop DTR: no DTR.\n");
-#endif
+			dprintk (debug, "Can't drop DTR: no DTR.\n");
 		return;
 	} else {
 		/* Set DTR on */
@@ -1147,9 +1091,7 @@ static void sx_change_speed(struct speci
 	sx_wait_CCR(bp);
 	sx_out(bp, CD186x_CCR, CCR_CORCHG1 | CCR_CORCHG2 | CCR_CORCHG3);
 	/* Setting up modem option registers */
-#ifdef DEBUG_SPECIALIX
-	printk ("Mcor1 = %02x, mcor2 = %02x.\n", mcor1, mcor2);
-#endif
+	dprintk (debug, "Mcor1 = %02x, mcor2 = %02x.\n", mcor1, mcor2);
 	sx_out(bp, CD186x_MCOR1, mcor1);
 	sx_out(bp, CD186x_MCOR2, mcor2);
 	/* Enable CD186x transmitter & receiver */
@@ -1372,10 +1314,8 @@ static int sx_open(struct tty_struct * t
 	bp = &sx_board[board];
 	port = sx_port + board * SX_NPORT + SX_PORT(tty->index);
 
-#ifdef DEBUG_SPECIALIX
-	printk (KERN_DEBUG "Board = %d, bp = %p, port = %p, portno = %d.\n", 
+	dprintk (debug, "Board = %d, bp = %p, port = %p, portno = %d.\n", 
 	        board, bp, port, SX_PORT(tty->index));
-#endif
 
 	if (sx_paranoia_check(port, tty->name, "sx_open"))
 		return -ENODEV;
@@ -1669,11 +1609,10 @@ static int sx_tiocmget(struct tty_struct
 	sx_out(bp, CD186x_CAR, port_No(port));
 	status = sx_in(bp, CD186x_MSVR);
 	restore_flags(flags);
-#ifdef DEBUG_SPECIALIX
-	printk (KERN_DEBUG "Got msvr[%d] = %02x, car = %d.\n", 
+	dprintk (debug, "Got msvr[%d] = %02x, car = %d.\n", 
 		port_No(port), status, sx_in (bp, CD186x_CAR));
-	printk (KERN_DEBUG "sx_port = %p, port = %p\n", sx_port, port);
-#endif
+	dprintk (debug, "sx_port = %p, port = %p\n", sx_port, port);
+
 	if (SX_CRTSCTS(port->tty)) {
 		result  = /*   (status & MSVR_RTS) ? */ TIOCM_DTR /* : 0) */ 
 		          |   ((status & MSVR_DTR) ? TIOCM_RTS : 0)

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Kernel-janitors] [PATCH] remove check_region specialix
  2004-07-07 20:24 [Kernel-janitors] [PATCH] remove check_region specialix Kristen Carlson
@ 2004-07-07 20:40 ` Matthew Wilcox
  2004-07-07 21:05 ` Kristen Carlson
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Matthew Wilcox @ 2004-07-07 20:40 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 984 bytes --]

On Wed, Jul 07, 2004 at 01:24:39PM -0700, Kristen Carlson wrote:
> Hi, please comment on this patch. It is against 2.6.7
> 
> Description: removes check_region call from specialix.c and cleans
> up some debug print messages and some dead code.
> Signed-off-by: Kristen Carlson Accardi <kristenc@cs.pdx.edu>

Looks good.  As far as I can tell, you removed the only call to
sx_check_io_range(), but didn't remove the function itself.  Doesn't gcc
warn about functions declared static but never called?  Or is this
different because it's also marked inline?

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Kernel-janitors] [PATCH] remove check_region specialix
  2004-07-07 20:24 [Kernel-janitors] [PATCH] remove check_region specialix Kristen Carlson
  2004-07-07 20:40 ` Matthew Wilcox
@ 2004-07-07 21:05 ` Kristen Carlson
  2004-07-07 21:28 ` Michael Veeck
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Kristen Carlson @ 2004-07-07 21:05 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 9746 bytes --]

Not sure on the gcc question, but it makes no sense to keep dead code
around, so here is another patch that is exactly the same, but removes
the inline function.

Description: remove check_region from specialix.c plus cleanup dead code
and debug prints
Signed-off-by: Kristen Carlson Accardi <kristenc@cs.pdx.edu>



--- specialix.c.orig	2004-07-07 12:56:03.000000000 +0000
+++ specialix.c	2004-07-07 13:52:05.000000000 +0000
@@ -118,6 +118,13 @@
 #undef SX_REPORT_OVERRUN
 
 
+#ifdef  SPECIALIX_DEBUG
+static int debug = SPECIALIX_DEBUG;
+#define dprintk(level, fmt, arg...) do { if (level >= debug) printk(KERN_DEBUG "%s: %s: " fmt , MY_NAME , __FUNCTION__ , ## arg); } while (0)
+#else
+static int debug = 0;
+#define dprintk(level, fmt, arg...)
+#endif
 
 #ifdef CONFIG_SPECIALIX_RTSCTS
 #define SX_CRTSCTS(bla) 1
@@ -284,16 +291,9 @@ static inline void sx_wait_CCR_off(struc
 /*
  *  specialix IO8+ IO range functions.
  */
-
-static inline int sx_check_io_range(struct specialix_board * bp)
+static inline int sx_request_io_range(struct specialix_board * bp)
 {
-	return check_region (bp->base, SX_IO_SPACE);
-}
-
-
-static inline void sx_request_io_range(struct specialix_board * bp)
-{
-	request_region(bp->base, 
+	return request_region(bp->base, 
 	               bp->flags&SX_BOARD_IS_PCI?SX_PCI_IO_SPACE:SX_IO_SPACE,
 	               "specialix IO8+" );
 }
@@ -419,14 +419,11 @@ void missed_irq (unsigned long data)
 static int sx_probe(struct specialix_board *bp)
 {
 	unsigned char val1, val2;
-#if 0
-	int irqs = 0;
-	int retries;
-#endif
 	int rev;
 	int chip;
+	int err;
 
-	if (sx_check_io_range(bp)) 
+	if (!sx_request_io_region(bp))
 		return 1;
 
 	/* Are the I/O ports here ? */
@@ -442,17 +439,16 @@ static int sx_probe(struct specialix_boa
 	if ((val1 != 0x5a) || (val2 != 0xa5)) {
 		printk(KERN_INFO "sx%d: specialix IO8+ Board at 0x%03x not found.\n",
 		       board_No(bp), bp->base);
-		return 1;
+		err = 1;
+		goto probe_err;
 	}
 
 	/* Check the DSR lines that Specialix uses as board 
 	   identification */
 	val1 = read_cross_byte (bp, CD186x_MSVR, MSVR_DSR);
 	val2 = read_cross_byte (bp, CD186x_MSVR, MSVR_RTS);
-#ifdef SPECIALIX_DEBUG
-	printk (KERN_DEBUG "sx%d: DSR lines are: %02x, rts lines are: %02x\n", 
+	dprintk ("sx%d: DSR lines are: %02x, rts lines are: %02x\n", 
 	        board_No(bp),  val1, val2);
-#endif
 	/* They managed to switch the bit order between the docs and
 	   the IO8+ card. The new PCI card now conforms to old docs.
 	   They changed the PCI docs to reflect the situation on the
@@ -461,57 +457,16 @@ static int sx_probe(struct specialix_boa
 	if (val1 != val2) {
 		printk(KERN_INFO "sx%d: specialix IO8+ ID %02x at 0x%03x not found (%02x).\n",
 		       board_No(bp), val2, bp->base, val1);
-		return 1;
+		err = 1;
+		goto probe_err;
 	}
 
-
-#if 0
-	/* It's time to find IRQ for this board */
-	for (retries = 0; retries < 5 && irqs <= 0; retries++) {
-		irqs = probe_irq_on();
-		sx_init_CD186x(bp);	       		/* Reset CD186x chip       */
-		sx_out(bp, CD186x_CAR, 2);               /* Select port 2          */
-		sx_wait_CCR(bp);
-		sx_out(bp, CD186x_CCR, CCR_TXEN);        /* Enable transmitter     */
-		sx_out(bp, CD186x_IER, IER_TXRDY);       /* Enable tx empty intr   */
-		sx_long_delay(HZ/20);	       		
-		irqs = probe_irq_off(irqs);
-
-#if SPECIALIX_DEBUG > 2
-		printk (KERN_DEBUG "SRSR = %02x, ",  sx_in(bp, CD186x_SRSR));
-		printk (           "TRAR = %02x, ",  sx_in(bp, CD186x_TRAR));
-		printk (           "GIVR = %02x, ",  sx_in(bp, CD186x_GIVR));
-		printk (           "GICR = %02x, ",  sx_in(bp, CD186x_GICR));
-		printk (           "\n");
-#endif
-		/* Reset CD186x again      */
-		if (!sx_init_CD186x(bp)) {
-			/* Hmmm. This is dead code anyway. */
-		}
-#if SPECIALIX_DEBUG > 2
-		printk (KERN_DEBUG "val1 = %02x, val2 = %02x, val3 = %02x.\n", 
-		        val1, val2, val3); 
-#endif
-	
-	}
-	
-#if 0
-	if (irqs <= 0) {
-		printk(KERN_ERR "sx%d: Can't find IRQ for specialix IO8+ board at 0x%03x.\n",
-		       board_No(bp), bp->base);
-		return 1;
-	}
-#endif
-	printk (KERN_INFO "Started with irq=%d, but now have irq=%d.\n", bp->irq, irqs);
-	if (irqs > 0)
-		bp->irq = irqs;
-#endif
 	/* Reset CD186x again  */
 	if (!sx_init_CD186x(bp)) {
-		return -EIO;
+		err = -EIO;
+		goto probe_err;
 	}
 
-	sx_request_io_range(bp);
 	bp->flags |= SX_BOARD_PRESENT;
 	
 	/* Chip           revcode   pkgtype
@@ -532,9 +487,7 @@ static int sx_probe(struct specialix_boa
 	default:chip=-1;rev='x';
 	}
 
-#if SPECIALIX_DEBUG > 2
-	printk (KERN_DEBUG " GFCR = 0x%02x\n", sx_in_off(bp, CD186x_GFRCR) );
-#endif
+	dprintk(2, " GFCR = 0x%02x\n", sx_in_off(bp, CD186x_GFRCR) );
 
 #ifdef SPECIALIX_TIMER
 	init_timer (&missed_irq_timer);
@@ -550,6 +503,10 @@ static int sx_probe(struct specialix_boa
 	       chip, rev);
 
 	return 0;
+
+ probe_err:
+	sx_release_io_range(bp);
+	return err;
 }
 
 /* 
@@ -604,10 +561,8 @@ static inline void sx_receive_exc(struct
 	status = sx_in(bp, CD186x_RCSR);
 	if (status & RCSR_OE) {
 		port->overrun++;
-#if SPECIALIX_DEBUG 
-		printk(KERN_DEBUG "sx%d: port %d: Overrun. Total %ld overruns.\n", 
-		       board_No(bp), port_No(port), port->overrun);
-#endif		
+		dprintk(debug, "sx%d: port %d: Overrun. Total %ld overruns.\n", 
+		       board_No(bp), port_No(port), port->overrun);		
 	}
 	status &= port->mark_mask;
 #else	
@@ -623,10 +578,8 @@ static inline void sx_receive_exc(struct
 		return;
 		
 	} else if (status & RCSR_BREAK) {
-#ifdef SPECIALIX_DEBUG
-		printk(KERN_DEBUG "sx%d: port %d: Handling break...\n",
+		dprintk(debug, "sx%d: port %d: Handling break...\n",
 		       board_No(bp), port_No(port));
-#endif
 		*tty->flip.flag_buf_ptr++ = TTY_BREAK;
 		if (port->flags & ASYNC_SAK)
 			do_SAK(tty);
@@ -756,9 +709,8 @@ static inline void sx_check_modem(struct
 	struct tty_struct *tty;
 	unsigned char mcr;
 
-#ifdef SPECIALIX_DEBUG
-	printk (KERN_DEBUG "Modem intr. ");
-#endif
+	dprintk (debug, "Modem intr. ");
+
 	if (!(port = sx_get_port(bp, "Modem")))
 		return;
 	
@@ -768,18 +720,12 @@ static inline void sx_check_modem(struct
 	printk ("mcr = %02x.\n", mcr);
 
 	if ((mcr & MCR_CDCHG)) {
-#ifdef SPECIALIX_DEBUG 
-		printk (KERN_DEBUG "CD just changed... ");
-#endif
+		dprintk (debug, "CD just changed... ");
 		if (sx_in(bp, CD186x_MSVR) & MSVR_CD) {
-#ifdef SPECIALIX_DEBUG
-			printk ( "Waking up guys in open.\n");
-#endif
+			dprintk (debug, "Waking up guys in open.\n");
 			wake_up_interruptible(&port->open_wait);
 		} else {
-#ifdef SPECIALIX_DEBUG
-			printk ( "Sending HUP.\n");
-#endif
+			dprintk (debug, "Sending HUP.\n");
 			schedule_work(&port->tqueue_hangup);
 		}
 	}
@@ -828,9 +774,7 @@ static irqreturn_t sx_interrupt(int irq,
 	bp = dev_id;
 	
 	if (!bp || !(bp->flags & SX_BOARD_ACTIVE)) {
-#ifdef SPECIALIX_DEBUG 
-		printk (KERN_DEBUG "sx: False interrupt. irq %d.\n", irq);
-#endif
+		dprintk (debug, "sx: False interrupt. irq %d.\n", irq);
 		return IRQ_NONE;
 	}
 
@@ -933,9 +877,8 @@ static inline void sx_shutdown_board(str
 	
 	bp->flags &= ~SX_BOARD_ACTIVE;
 	
-#if SPECIALIX_DEBUG > 2
-	printk ("Freeing IRQ%d for board %d.\n", bp->irq, board_No (bp));
-#endif
+	dprintk (3, "Freeing IRQ%d for board %d.\n", bp->irq, board_No (bp));
+
 	free_irq(bp->irq, bp);
 
 	turn_ints_off (bp);
@@ -970,9 +913,7 @@ static void sx_change_speed(struct speci
 		port->MSVR = MSVR_DTR | (sx_in(bp, CD186x_MSVR) & MSVR_RTS);
 	else
 		port->MSVR =  (sx_in(bp, CD186x_MSVR) & MSVR_RTS);
-#ifdef DEBUG_SPECIALIX
-	printk (KERN_DEBUG "sx: got MSVR=%02x.\n", port->MSVR);
-#endif
+	dprintk(debug, "sx: got MSVR=%02x.\n", port->MSVR);
 	baud = C_BAUD(tty);
 	
 	if (baud & CBAUDEX) {
@@ -992,17 +933,13 @@ static void sx_change_speed(struct speci
 	
 	if (!baud_table[baud]) {
 		/* Drop DTR & exit */
-#ifdef SPECIALIX_DEBUG
-		printk (KERN_DEBUG "Dropping DTR...  Hmm....\n");
-#endif
+		dprintk(debug, "Dropping DTR...  Hmm....\n");
 		if (!SX_CRTSCTS (tty)) {
 			port -> MSVR &= ~ MSVR_DTR;
 			sx_out(bp, CD186x_MSVR, port->MSVR );
 		} 
-#ifdef DEBUG_SPECIALIX
 		else
-			printk (KERN_DEBUG "Can't drop DTR: no DTR.\n");
-#endif
+			dprintk (debug, "Can't drop DTR: no DTR.\n");
 		return;
 	} else {
 		/* Set DTR on */
@@ -1147,9 +1084,7 @@ static void sx_change_speed(struct speci
 	sx_wait_CCR(bp);
 	sx_out(bp, CD186x_CCR, CCR_CORCHG1 | CCR_CORCHG2 | CCR_CORCHG3);
 	/* Setting up modem option registers */
-#ifdef DEBUG_SPECIALIX
-	printk ("Mcor1 = %02x, mcor2 = %02x.\n", mcor1, mcor2);
-#endif
+	dprintk (debug, "Mcor1 = %02x, mcor2 = %02x.\n", mcor1, mcor2);
 	sx_out(bp, CD186x_MCOR1, mcor1);
 	sx_out(bp, CD186x_MCOR2, mcor2);
 	/* Enable CD186x transmitter & receiver */
@@ -1372,10 +1307,8 @@ static int sx_open(struct tty_struct * t
 	bp = &sx_board[board];
 	port = sx_port + board * SX_NPORT + SX_PORT(tty->index);
 
-#ifdef DEBUG_SPECIALIX
-	printk (KERN_DEBUG "Board = %d, bp = %p, port = %p, portno = %d.\n", 
+	dprintk (debug, "Board = %d, bp = %p, port = %p, portno = %d.\n", 
 	        board, bp, port, SX_PORT(tty->index));
-#endif
 
 	if (sx_paranoia_check(port, tty->name, "sx_open"))
 		return -ENODEV;
@@ -1669,11 +1602,10 @@ static int sx_tiocmget(struct tty_struct
 	sx_out(bp, CD186x_CAR, port_No(port));
 	status = sx_in(bp, CD186x_MSVR);
 	restore_flags(flags);
-#ifdef DEBUG_SPECIALIX
-	printk (KERN_DEBUG "Got msvr[%d] = %02x, car = %d.\n", 
+	dprintk (debug, "Got msvr[%d] = %02x, car = %d.\n", 
 		port_No(port), status, sx_in (bp, CD186x_CAR));
-	printk (KERN_DEBUG "sx_port = %p, port = %p\n", sx_port, port);
-#endif
+	dprintk (debug, "sx_port = %p, port = %p\n", sx_port, port);
+
 	if (SX_CRTSCTS(port->tty)) {
 		result  = /*   (status & MSVR_RTS) ? */ TIOCM_DTR /* : 0) */ 
 		          |   ((status & MSVR_DTR) ? TIOCM_RTS : 0)

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Kernel-janitors] [PATCH] remove check_region specialix
  2004-07-07 20:24 [Kernel-janitors] [PATCH] remove check_region specialix Kristen Carlson
  2004-07-07 20:40 ` Matthew Wilcox
  2004-07-07 21:05 ` Kristen Carlson
@ 2004-07-07 21:28 ` Michael Veeck
  2004-07-07 21:59 ` Kristen Carlson
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Michael Veeck @ 2004-07-07 21:28 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 10836 bytes --]

One mistake which everbody seems to do at the beginning:
Read Documentation/SubmittingPatches and you'll find that you should 
diff one level up from the linux-source-dir.

Kristen Carlson schrieb:
> Not sure on the gcc question, but it makes no sense to keep dead code
> around, so here is another patch that is exactly the same, but removes
> the inline function.
> 
> Description: remove check_region from specialix.c plus cleanup dead code
> and debug prints
> Signed-off-by: Kristen Carlson Accardi <kristenc@cs.pdx.edu>
> 
> 
> 
> --- specialix.c.orig	2004-07-07 12:56:03.000000000 +0000
> +++ specialix.c	2004-07-07 13:52:05.000000000 +0000
> @@ -118,6 +118,13 @@
>  #undef SX_REPORT_OVERRUN
>  
>  
> +#ifdef  SPECIALIX_DEBUG
> +static int debug = SPECIALIX_DEBUG;
> +#define dprintk(level, fmt, arg...) do { if (level >= debug) printk(KERN_DEBUG "%s: %s: " fmt , MY_NAME , __FUNCTION__ , ## arg); } while (0)
> +#else
> +static int debug = 0;
> +#define dprintk(level, fmt, arg...)
> +#endif
>  
>  #ifdef CONFIG_SPECIALIX_RTSCTS
>  #define SX_CRTSCTS(bla) 1
> @@ -284,16 +291,9 @@ static inline void sx_wait_CCR_off(struc
>  /*
>   *  specialix IO8+ IO range functions.
>   */
> -
> -static inline int sx_check_io_range(struct specialix_board * bp)
> +static inline int sx_request_io_range(struct specialix_board * bp)
>  {
> -	return check_region (bp->base, SX_IO_SPACE);
> -}
> -
> -
> -static inline void sx_request_io_range(struct specialix_board * bp)
> -{
> -	request_region(bp->base, 
> +	return request_region(bp->base, 
>  	               bp->flags&SX_BOARD_IS_PCI?SX_PCI_IO_SPACE:SX_IO_SPACE,
>  	               "specialix IO8+" );
>  }
> @@ -419,14 +419,11 @@ void missed_irq (unsigned long data)
>  static int sx_probe(struct specialix_board *bp)
>  {
>  	unsigned char val1, val2;
> -#if 0
> -	int irqs = 0;
> -	int retries;
> -#endif
>  	int rev;
>  	int chip;
> +	int err;
>  
> -	if (sx_check_io_range(bp)) 
> +	if (!sx_request_io_region(bp))
>  		return 1;
>  
>  	/* Are the I/O ports here ? */
> @@ -442,17 +439,16 @@ static int sx_probe(struct specialix_boa
>  	if ((val1 != 0x5a) || (val2 != 0xa5)) {
>  		printk(KERN_INFO "sx%d: specialix IO8+ Board at 0x%03x not found.\n",
>  		       board_No(bp), bp->base);
> -		return 1;
> +		err = 1;
> +		goto probe_err;
>  	}
>  
>  	/* Check the DSR lines that Specialix uses as board 
>  	   identification */
>  	val1 = read_cross_byte (bp, CD186x_MSVR, MSVR_DSR);
>  	val2 = read_cross_byte (bp, CD186x_MSVR, MSVR_RTS);
> -#ifdef SPECIALIX_DEBUG
> -	printk (KERN_DEBUG "sx%d: DSR lines are: %02x, rts lines are: %02x\n", 
> +	dprintk ("sx%d: DSR lines are: %02x, rts lines are: %02x\n", 
>  	        board_No(bp),  val1, val2);
> -#endif
>  	/* They managed to switch the bit order between the docs and
>  	   the IO8+ card. The new PCI card now conforms to old docs.
>  	   They changed the PCI docs to reflect the situation on the
> @@ -461,57 +457,16 @@ static int sx_probe(struct specialix_boa
>  	if (val1 != val2) {
>  		printk(KERN_INFO "sx%d: specialix IO8+ ID %02x at 0x%03x not found (%02x).\n",
>  		       board_No(bp), val2, bp->base, val1);
> -		return 1;
> +		err = 1;
> +		goto probe_err;
>  	}
>  
> -
> -#if 0
> -	/* It's time to find IRQ for this board */
> -	for (retries = 0; retries < 5 && irqs <= 0; retries++) {
> -		irqs = probe_irq_on();
> -		sx_init_CD186x(bp);	       		/* Reset CD186x chip       */
> -		sx_out(bp, CD186x_CAR, 2);               /* Select port 2          */
> -		sx_wait_CCR(bp);
> -		sx_out(bp, CD186x_CCR, CCR_TXEN);        /* Enable transmitter     */
> -		sx_out(bp, CD186x_IER, IER_TXRDY);       /* Enable tx empty intr   */
> -		sx_long_delay(HZ/20);	       		
> -		irqs = probe_irq_off(irqs);
> -
> -#if SPECIALIX_DEBUG > 2
> -		printk (KERN_DEBUG "SRSR = %02x, ",  sx_in(bp, CD186x_SRSR));
> -		printk (           "TRAR = %02x, ",  sx_in(bp, CD186x_TRAR));
> -		printk (           "GIVR = %02x, ",  sx_in(bp, CD186x_GIVR));
> -		printk (           "GICR = %02x, ",  sx_in(bp, CD186x_GICR));
> -		printk (           "\n");
> -#endif
> -		/* Reset CD186x again      */
> -		if (!sx_init_CD186x(bp)) {
> -			/* Hmmm. This is dead code anyway. */
> -		}
> -#if SPECIALIX_DEBUG > 2
> -		printk (KERN_DEBUG "val1 = %02x, val2 = %02x, val3 = %02x.\n", 
> -		        val1, val2, val3); 
> -#endif
> -	
> -	}
> -	
> -#if 0
> -	if (irqs <= 0) {
> -		printk(KERN_ERR "sx%d: Can't find IRQ for specialix IO8+ board at 0x%03x.\n",
> -		       board_No(bp), bp->base);
> -		return 1;
> -	}
> -#endif
> -	printk (KERN_INFO "Started with irq=%d, but now have irq=%d.\n", bp->irq, irqs);
> -	if (irqs > 0)
> -		bp->irq = irqs;
> -#endif
>  	/* Reset CD186x again  */
>  	if (!sx_init_CD186x(bp)) {
> -		return -EIO;
> +		err = -EIO;
> +		goto probe_err;
>  	}
>  
> -	sx_request_io_range(bp);
>  	bp->flags |= SX_BOARD_PRESENT;
>  	
>  	/* Chip           revcode   pkgtype
> @@ -532,9 +487,7 @@ static int sx_probe(struct specialix_boa
>  	default:chip=-1;rev='x';
>  	}
>  
> -#if SPECIALIX_DEBUG > 2
> -	printk (KERN_DEBUG " GFCR = 0x%02x\n", sx_in_off(bp, CD186x_GFRCR) );
> -#endif
> +	dprintk(2, " GFCR = 0x%02x\n", sx_in_off(bp, CD186x_GFRCR) );
>  
>  #ifdef SPECIALIX_TIMER
>  	init_timer (&missed_irq_timer);
> @@ -550,6 +503,10 @@ static int sx_probe(struct specialix_boa
>  	       chip, rev);
>  
>  	return 0;
> +
> + probe_err:
> +	sx_release_io_range(bp);
> +	return err;
>  }
>  
>  /* 
> @@ -604,10 +561,8 @@ static inline void sx_receive_exc(struct
>  	status = sx_in(bp, CD186x_RCSR);
>  	if (status & RCSR_OE) {
>  		port->overrun++;
> -#if SPECIALIX_DEBUG 
> -		printk(KERN_DEBUG "sx%d: port %d: Overrun. Total %ld overruns.\n", 
> -		       board_No(bp), port_No(port), port->overrun);
> -#endif		
> +		dprintk(debug, "sx%d: port %d: Overrun. Total %ld overruns.\n", 
> +		       board_No(bp), port_No(port), port->overrun);		
>  	}
>  	status &= port->mark_mask;
>  #else	
> @@ -623,10 +578,8 @@ static inline void sx_receive_exc(struct
>  		return;
>  		
>  	} else if (status & RCSR_BREAK) {
> -#ifdef SPECIALIX_DEBUG
> -		printk(KERN_DEBUG "sx%d: port %d: Handling break...\n",
> +		dprintk(debug, "sx%d: port %d: Handling break...\n",
>  		       board_No(bp), port_No(port));
> -#endif
>  		*tty->flip.flag_buf_ptr++ = TTY_BREAK;
>  		if (port->flags & ASYNC_SAK)
>  			do_SAK(tty);
> @@ -756,9 +709,8 @@ static inline void sx_check_modem(struct
>  	struct tty_struct *tty;
>  	unsigned char mcr;
>  
> -#ifdef SPECIALIX_DEBUG
> -	printk (KERN_DEBUG "Modem intr. ");
> -#endif
> +	dprintk (debug, "Modem intr. ");
> +
>  	if (!(port = sx_get_port(bp, "Modem")))
>  		return;
>  	
> @@ -768,18 +720,12 @@ static inline void sx_check_modem(struct
>  	printk ("mcr = %02x.\n", mcr);
>  
>  	if ((mcr & MCR_CDCHG)) {
> -#ifdef SPECIALIX_DEBUG 
> -		printk (KERN_DEBUG "CD just changed... ");
> -#endif
> +		dprintk (debug, "CD just changed... ");
>  		if (sx_in(bp, CD186x_MSVR) & MSVR_CD) {
> -#ifdef SPECIALIX_DEBUG
> -			printk ( "Waking up guys in open.\n");
> -#endif
> +			dprintk (debug, "Waking up guys in open.\n");
>  			wake_up_interruptible(&port->open_wait);
>  		} else {
> -#ifdef SPECIALIX_DEBUG
> -			printk ( "Sending HUP.\n");
> -#endif
> +			dprintk (debug, "Sending HUP.\n");
>  			schedule_work(&port->tqueue_hangup);
>  		}
>  	}
> @@ -828,9 +774,7 @@ static irqreturn_t sx_interrupt(int irq,
>  	bp = dev_id;
>  	
>  	if (!bp || !(bp->flags & SX_BOARD_ACTIVE)) {
> -#ifdef SPECIALIX_DEBUG 
> -		printk (KERN_DEBUG "sx: False interrupt. irq %d.\n", irq);
> -#endif
> +		dprintk (debug, "sx: False interrupt. irq %d.\n", irq);
>  		return IRQ_NONE;
>  	}
>  
> @@ -933,9 +877,8 @@ static inline void sx_shutdown_board(str
>  	
>  	bp->flags &= ~SX_BOARD_ACTIVE;
>  	
> -#if SPECIALIX_DEBUG > 2
> -	printk ("Freeing IRQ%d for board %d.\n", bp->irq, board_No (bp));
> -#endif
> +	dprintk (3, "Freeing IRQ%d for board %d.\n", bp->irq, board_No (bp));
> +
>  	free_irq(bp->irq, bp);
>  
>  	turn_ints_off (bp);
> @@ -970,9 +913,7 @@ static void sx_change_speed(struct speci
>  		port->MSVR = MSVR_DTR | (sx_in(bp, CD186x_MSVR) & MSVR_RTS);
>  	else
>  		port->MSVR =  (sx_in(bp, CD186x_MSVR) & MSVR_RTS);
> -#ifdef DEBUG_SPECIALIX
> -	printk (KERN_DEBUG "sx: got MSVR=%02x.\n", port->MSVR);
> -#endif
> +	dprintk(debug, "sx: got MSVR=%02x.\n", port->MSVR);
>  	baud = C_BAUD(tty);
>  	
>  	if (baud & CBAUDEX) {
> @@ -992,17 +933,13 @@ static void sx_change_speed(struct speci
>  	
>  	if (!baud_table[baud]) {
>  		/* Drop DTR & exit */
> -#ifdef SPECIALIX_DEBUG
> -		printk (KERN_DEBUG "Dropping DTR...  Hmm....\n");
> -#endif
> +		dprintk(debug, "Dropping DTR...  Hmm....\n");
>  		if (!SX_CRTSCTS (tty)) {
>  			port -> MSVR &= ~ MSVR_DTR;
>  			sx_out(bp, CD186x_MSVR, port->MSVR );
>  		} 
> -#ifdef DEBUG_SPECIALIX
>  		else
> -			printk (KERN_DEBUG "Can't drop DTR: no DTR.\n");
> -#endif
> +			dprintk (debug, "Can't drop DTR: no DTR.\n");
>  		return;
>  	} else {
>  		/* Set DTR on */
> @@ -1147,9 +1084,7 @@ static void sx_change_speed(struct speci
>  	sx_wait_CCR(bp);
>  	sx_out(bp, CD186x_CCR, CCR_CORCHG1 | CCR_CORCHG2 | CCR_CORCHG3);
>  	/* Setting up modem option registers */
> -#ifdef DEBUG_SPECIALIX
> -	printk ("Mcor1 = %02x, mcor2 = %02x.\n", mcor1, mcor2);
> -#endif
> +	dprintk (debug, "Mcor1 = %02x, mcor2 = %02x.\n", mcor1, mcor2);
>  	sx_out(bp, CD186x_MCOR1, mcor1);
>  	sx_out(bp, CD186x_MCOR2, mcor2);
>  	/* Enable CD186x transmitter & receiver */
> @@ -1372,10 +1307,8 @@ static int sx_open(struct tty_struct * t
>  	bp = &sx_board[board];
>  	port = sx_port + board * SX_NPORT + SX_PORT(tty->index);
>  
> -#ifdef DEBUG_SPECIALIX
> -	printk (KERN_DEBUG "Board = %d, bp = %p, port = %p, portno = %d.\n", 
> +	dprintk (debug, "Board = %d, bp = %p, port = %p, portno = %d.\n", 
>  	        board, bp, port, SX_PORT(tty->index));
> -#endif
>  
>  	if (sx_paranoia_check(port, tty->name, "sx_open"))
>  		return -ENODEV;
> @@ -1669,11 +1602,10 @@ static int sx_tiocmget(struct tty_struct
>  	sx_out(bp, CD186x_CAR, port_No(port));
>  	status = sx_in(bp, CD186x_MSVR);
>  	restore_flags(flags);
> -#ifdef DEBUG_SPECIALIX
> -	printk (KERN_DEBUG "Got msvr[%d] = %02x, car = %d.\n", 
> +	dprintk (debug, "Got msvr[%d] = %02x, car = %d.\n", 
>  		port_No(port), status, sx_in (bp, CD186x_CAR));
> -	printk (KERN_DEBUG "sx_port = %p, port = %p\n", sx_port, port);
> -#endif
> +	dprintk (debug, "sx_port = %p, port = %p\n", sx_port, port);
> +
>  	if (SX_CRTSCTS(port->tty)) {
>  		result  = /*   (status & MSVR_RTS) ? */ TIOCM_DTR /* : 0) */ 
>  		          |   ((status & MSVR_DTR) ? TIOCM_RTS : 0)
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Kernel-janitors mailing list
> Kernel-janitors@lists.osdl.org
> http://lists.osdl.org/mailman/listinfo/kernel-janitors


[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Kernel-janitors] [PATCH] remove check_region specialix
  2004-07-07 20:24 [Kernel-janitors] [PATCH] remove check_region specialix Kristen Carlson
                   ` (2 preceding siblings ...)
  2004-07-07 21:28 ` Michael Veeck
@ 2004-07-07 21:59 ` Kristen Carlson
  2004-07-11 13:15 ` maximilian attems
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Kristen Carlson @ 2004-07-07 21:59 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 9544 bytes --]

Ok, here's another try:


--- linux-2.6.7-kc/drivers/char/specialix.c.orig	2004-07-07 12:56:03.000000000 +0000
+++ linux-2.6.7-kc/drivers/char/specialix.c	2004-07-07 13:52:05.000000000 +0000
@@ -118,6 +118,13 @@
 #undef SX_REPORT_OVERRUN
 
 
+#ifdef  SPECIALIX_DEBUG
+static int debug = SPECIALIX_DEBUG;
+#define dprintk(level, fmt, arg...) do { if (level >= debug) printk(KERN_DEBUG "%s: %s: " fmt , MY_NAME , __FUNCTION__ , ## arg); } while (0)
+#else
+static int debug = 0;
+#define dprintk(level, fmt, arg...)
+#endif
 
 #ifdef CONFIG_SPECIALIX_RTSCTS
 #define SX_CRTSCTS(bla) 1
@@ -284,16 +291,9 @@ static inline void sx_wait_CCR_off(struc
 /*
  *  specialix IO8+ IO range functions.
  */
-
-static inline int sx_check_io_range(struct specialix_board * bp)
+static inline int sx_request_io_range(struct specialix_board * bp)
 {
-	return check_region (bp->base, SX_IO_SPACE);
-}
-
-
-static inline void sx_request_io_range(struct specialix_board * bp)
-{
-	request_region(bp->base, 
+	return request_region(bp->base, 
 	               bp->flags&SX_BOARD_IS_PCI?SX_PCI_IO_SPACE:SX_IO_SPACE,
 	               "specialix IO8+" );
 }
@@ -419,14 +419,11 @@ void missed_irq (unsigned long data)
 static int sx_probe(struct specialix_board *bp)
 {
 	unsigned char val1, val2;
-#if 0
-	int irqs = 0;
-	int retries;
-#endif
 	int rev;
 	int chip;
+	int err;
 
-	if (sx_check_io_range(bp)) 
+	if (!sx_request_io_region(bp))
 		return 1;
 
 	/* Are the I/O ports here ? */
@@ -442,17 +439,16 @@ static int sx_probe(struct specialix_boa
 	if ((val1 != 0x5a) || (val2 != 0xa5)) {
 		printk(KERN_INFO "sx%d: specialix IO8+ Board at 0x%03x not found.\n",
 		       board_No(bp), bp->base);
-		return 1;
+		err = 1;
+		goto probe_err;
 	}
 
 	/* Check the DSR lines that Specialix uses as board 
 	   identification */
 	val1 = read_cross_byte (bp, CD186x_MSVR, MSVR_DSR);
 	val2 = read_cross_byte (bp, CD186x_MSVR, MSVR_RTS);
-#ifdef SPECIALIX_DEBUG
-	printk (KERN_DEBUG "sx%d: DSR lines are: %02x, rts lines are: %02x\n", 
+	dprintk ("sx%d: DSR lines are: %02x, rts lines are: %02x\n", 
 	        board_No(bp),  val1, val2);
-#endif
 	/* They managed to switch the bit order between the docs and
 	   the IO8+ card. The new PCI card now conforms to old docs.
 	   They changed the PCI docs to reflect the situation on the
@@ -461,57 +457,16 @@ static int sx_probe(struct specialix_boa
 	if (val1 != val2) {
 		printk(KERN_INFO "sx%d: specialix IO8+ ID %02x at 0x%03x not found (%02x).\n",
 		       board_No(bp), val2, bp->base, val1);
-		return 1;
+		err = 1;
+		goto probe_err;
 	}
 
-
-#if 0
-	/* It's time to find IRQ for this board */
-	for (retries = 0; retries < 5 && irqs <= 0; retries++) {
-		irqs = probe_irq_on();
-		sx_init_CD186x(bp);	       		/* Reset CD186x chip       */
-		sx_out(bp, CD186x_CAR, 2);               /* Select port 2          */
-		sx_wait_CCR(bp);
-		sx_out(bp, CD186x_CCR, CCR_TXEN);        /* Enable transmitter     */
-		sx_out(bp, CD186x_IER, IER_TXRDY);       /* Enable tx empty intr   */
-		sx_long_delay(HZ/20);	       		
-		irqs = probe_irq_off(irqs);
-
-#if SPECIALIX_DEBUG > 2
-		printk (KERN_DEBUG "SRSR = %02x, ",  sx_in(bp, CD186x_SRSR));
-		printk (           "TRAR = %02x, ",  sx_in(bp, CD186x_TRAR));
-		printk (           "GIVR = %02x, ",  sx_in(bp, CD186x_GIVR));
-		printk (           "GICR = %02x, ",  sx_in(bp, CD186x_GICR));
-		printk (           "\n");
-#endif
-		/* Reset CD186x again      */
-		if (!sx_init_CD186x(bp)) {
-			/* Hmmm. This is dead code anyway. */
-		}
-#if SPECIALIX_DEBUG > 2
-		printk (KERN_DEBUG "val1 = %02x, val2 = %02x, val3 = %02x.\n", 
-		        val1, val2, val3); 
-#endif
-	
-	}
-	
-#if 0
-	if (irqs <= 0) {
-		printk(KERN_ERR "sx%d: Can't find IRQ for specialix IO8+ board at 0x%03x.\n",
-		       board_No(bp), bp->base);
-		return 1;
-	}
-#endif
-	printk (KERN_INFO "Started with irq=%d, but now have irq=%d.\n", bp->irq, irqs);
-	if (irqs > 0)
-		bp->irq = irqs;
-#endif
 	/* Reset CD186x again  */
 	if (!sx_init_CD186x(bp)) {
-		return -EIO;
+		err = -EIO;
+		goto probe_err;
 	}
 
-	sx_request_io_range(bp);
 	bp->flags |= SX_BOARD_PRESENT;
 	
 	/* Chip           revcode   pkgtype
@@ -532,9 +487,7 @@ static int sx_probe(struct specialix_boa
 	default:chip=-1;rev='x';
 	}
 
-#if SPECIALIX_DEBUG > 2
-	printk (KERN_DEBUG " GFCR = 0x%02x\n", sx_in_off(bp, CD186x_GFRCR) );
-#endif
+	dprintk(2, " GFCR = 0x%02x\n", sx_in_off(bp, CD186x_GFRCR) );
 
 #ifdef SPECIALIX_TIMER
 	init_timer (&missed_irq_timer);
@@ -550,6 +503,10 @@ static int sx_probe(struct specialix_boa
 	       chip, rev);
 
 	return 0;
+
+ probe_err:
+	sx_release_io_range(bp);
+	return err;
 }
 
 /* 
@@ -604,10 +561,8 @@ static inline void sx_receive_exc(struct
 	status = sx_in(bp, CD186x_RCSR);
 	if (status & RCSR_OE) {
 		port->overrun++;
-#if SPECIALIX_DEBUG 
-		printk(KERN_DEBUG "sx%d: port %d: Overrun. Total %ld overruns.\n", 
-		       board_No(bp), port_No(port), port->overrun);
-#endif		
+		dprintk(debug, "sx%d: port %d: Overrun. Total %ld overruns.\n", 
+		       board_No(bp), port_No(port), port->overrun);		
 	}
 	status &= port->mark_mask;
 #else	
@@ -623,10 +578,8 @@ static inline void sx_receive_exc(struct
 		return;
 		
 	} else if (status & RCSR_BREAK) {
-#ifdef SPECIALIX_DEBUG
-		printk(KERN_DEBUG "sx%d: port %d: Handling break...\n",
+		dprintk(debug, "sx%d: port %d: Handling break...\n",
 		       board_No(bp), port_No(port));
-#endif
 		*tty->flip.flag_buf_ptr++ = TTY_BREAK;
 		if (port->flags & ASYNC_SAK)
 			do_SAK(tty);
@@ -756,9 +709,8 @@ static inline void sx_check_modem(struct
 	struct tty_struct *tty;
 	unsigned char mcr;
 
-#ifdef SPECIALIX_DEBUG
-	printk (KERN_DEBUG "Modem intr. ");
-#endif
+	dprintk (debug, "Modem intr. ");
+
 	if (!(port = sx_get_port(bp, "Modem")))
 		return;
 	
@@ -768,18 +720,12 @@ static inline void sx_check_modem(struct
 	printk ("mcr = %02x.\n", mcr);
 
 	if ((mcr & MCR_CDCHG)) {
-#ifdef SPECIALIX_DEBUG 
-		printk (KERN_DEBUG "CD just changed... ");
-#endif
+		dprintk (debug, "CD just changed... ");
 		if (sx_in(bp, CD186x_MSVR) & MSVR_CD) {
-#ifdef SPECIALIX_DEBUG
-			printk ( "Waking up guys in open.\n");
-#endif
+			dprintk (debug, "Waking up guys in open.\n");
 			wake_up_interruptible(&port->open_wait);
 		} else {
-#ifdef SPECIALIX_DEBUG
-			printk ( "Sending HUP.\n");
-#endif
+			dprintk (debug, "Sending HUP.\n");
 			schedule_work(&port->tqueue_hangup);
 		}
 	}
@@ -828,9 +774,7 @@ static irqreturn_t sx_interrupt(int irq,
 	bp = dev_id;
 	
 	if (!bp || !(bp->flags & SX_BOARD_ACTIVE)) {
-#ifdef SPECIALIX_DEBUG 
-		printk (KERN_DEBUG "sx: False interrupt. irq %d.\n", irq);
-#endif
+		dprintk (debug, "sx: False interrupt. irq %d.\n", irq);
 		return IRQ_NONE;
 	}
 
@@ -933,9 +877,8 @@ static inline void sx_shutdown_board(str
 	
 	bp->flags &= ~SX_BOARD_ACTIVE;
 	
-#if SPECIALIX_DEBUG > 2
-	printk ("Freeing IRQ%d for board %d.\n", bp->irq, board_No (bp));
-#endif
+	dprintk (3, "Freeing IRQ%d for board %d.\n", bp->irq, board_No (bp));
+
 	free_irq(bp->irq, bp);
 
 	turn_ints_off (bp);
@@ -970,9 +913,7 @@ static void sx_change_speed(struct speci
 		port->MSVR = MSVR_DTR | (sx_in(bp, CD186x_MSVR) & MSVR_RTS);
 	else
 		port->MSVR =  (sx_in(bp, CD186x_MSVR) & MSVR_RTS);
-#ifdef DEBUG_SPECIALIX
-	printk (KERN_DEBUG "sx: got MSVR=%02x.\n", port->MSVR);
-#endif
+	dprintk(debug, "sx: got MSVR=%02x.\n", port->MSVR);
 	baud = C_BAUD(tty);
 	
 	if (baud & CBAUDEX) {
@@ -992,17 +933,13 @@ static void sx_change_speed(struct speci
 	
 	if (!baud_table[baud]) {
 		/* Drop DTR & exit */
-#ifdef SPECIALIX_DEBUG
-		printk (KERN_DEBUG "Dropping DTR...  Hmm....\n");
-#endif
+		dprintk(debug, "Dropping DTR...  Hmm....\n");
 		if (!SX_CRTSCTS (tty)) {
 			port -> MSVR &= ~ MSVR_DTR;
 			sx_out(bp, CD186x_MSVR, port->MSVR );
 		} 
-#ifdef DEBUG_SPECIALIX
 		else
-			printk (KERN_DEBUG "Can't drop DTR: no DTR.\n");
-#endif
+			dprintk (debug, "Can't drop DTR: no DTR.\n");
 		return;
 	} else {
 		/* Set DTR on */
@@ -1147,9 +1084,7 @@ static void sx_change_speed(struct speci
 	sx_wait_CCR(bp);
 	sx_out(bp, CD186x_CCR, CCR_CORCHG1 | CCR_CORCHG2 | CCR_CORCHG3);
 	/* Setting up modem option registers */
-#ifdef DEBUG_SPECIALIX
-	printk ("Mcor1 = %02x, mcor2 = %02x.\n", mcor1, mcor2);
-#endif
+	dprintk (debug, "Mcor1 = %02x, mcor2 = %02x.\n", mcor1, mcor2);
 	sx_out(bp, CD186x_MCOR1, mcor1);
 	sx_out(bp, CD186x_MCOR2, mcor2);
 	/* Enable CD186x transmitter & receiver */
@@ -1372,10 +1307,8 @@ static int sx_open(struct tty_struct * t
 	bp = &sx_board[board];
 	port = sx_port + board * SX_NPORT + SX_PORT(tty->index);
 
-#ifdef DEBUG_SPECIALIX
-	printk (KERN_DEBUG "Board = %d, bp = %p, port = %p, portno = %d.\n", 
+	dprintk (debug, "Board = %d, bp = %p, port = %p, portno = %d.\n", 
 	        board, bp, port, SX_PORT(tty->index));
-#endif
 
 	if (sx_paranoia_check(port, tty->name, "sx_open"))
 		return -ENODEV;
@@ -1669,11 +1602,10 @@ static int sx_tiocmget(struct tty_struct
 	sx_out(bp, CD186x_CAR, port_No(port));
 	status = sx_in(bp, CD186x_MSVR);
 	restore_flags(flags);
-#ifdef DEBUG_SPECIALIX
-	printk (KERN_DEBUG "Got msvr[%d] = %02x, car = %d.\n", 
+	dprintk (debug, "Got msvr[%d] = %02x, car = %d.\n", 
 		port_No(port), status, sx_in (bp, CD186x_CAR));
-	printk (KERN_DEBUG "sx_port = %p, port = %p\n", sx_port, port);
-#endif
+	dprintk (debug, "sx_port = %p, port = %p\n", sx_port, port);
+
 	if (SX_CRTSCTS(port->tty)) {
 		result  = /*   (status & MSVR_RTS) ? */ TIOCM_DTR /* : 0) */ 
 		          |   ((status & MSVR_DTR) ? TIOCM_RTS : 0)
-- 
WWXD (What Would Xena Do?) 


[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Kernel-janitors] [PATCH] remove check_region specialix
  2004-07-07 20:24 [Kernel-janitors] [PATCH] remove check_region specialix Kristen Carlson
                   ` (3 preceding siblings ...)
  2004-07-07 21:59 ` Kristen Carlson
@ 2004-07-11 13:15 ` maximilian attems
  2004-07-12 16:11 ` Kristen Carlson
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: maximilian attems @ 2004-07-11 13:15 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 10445 bytes --]

On Wed, 07 Jul 2004, Kristen Carlson wrote:

> Ok, here's another try:
> 
> 
> --- linux-2.6.7-kc/drivers/char/specialix.c.orig	2004-07-07 12:56:03.000000000 +0000
> +++ linux-2.6.7-kc/drivers/char/specialix.c	2004-07-07 13:52:05.000000000 +0000
> @@ -118,6 +118,13 @@
>  #undef SX_REPORT_OVERRUN
>  
>  
> +#ifdef  SPECIALIX_DEBUG
> +static int debug = SPECIALIX_DEBUG;
> +#define dprintk(level, fmt, arg...) do { if (level >= debug) printk(KERN_DEBUG "%s: %s: " fmt , MY_NAME , __FUNCTION__ , ## arg); } while (0)
> +#else
> +static int debug = 0;
> +#define dprintk(level, fmt, arg...)
> +#endif
>  
>  #ifdef CONFIG_SPECIALIX_RTSCTS
>  #define SX_CRTSCTS(bla) 1
> @@ -284,16 +291,9 @@ static inline void sx_wait_CCR_off(struc
>  /*
>   *  specialix IO8+ IO range functions.
>   */
> -
> -static inline int sx_check_io_range(struct specialix_board * bp)
> +static inline int sx_request_io_range(struct specialix_board * bp)
>  {
> -	return check_region (bp->base, SX_IO_SPACE);
> -}
> -
> -
> -static inline void sx_request_io_range(struct specialix_board * bp)
> -{
> -	request_region(bp->base, 
> +	return request_region(bp->base, 
trailing space
>  	               bp->flags&SX_BOARD_IS_PCI?SX_PCI_IO_SPACE:SX_IO_SPACE,
>  	               "specialix IO8+" );
>  }
> @@ -419,14 +419,11 @@ void missed_irq (unsigned long data)
>  static int sx_probe(struct specialix_board *bp)
>  {
>  	unsigned char val1, val2;
> -#if 0
> -	int irqs = 0;
> -	int retries;
> -#endif
>  	int rev;
>  	int chip;
> +	int err;
>  
> -	if (sx_check_io_range(bp)) 
> +	if (!sx_request_io_region(bp))
>  		return 1;
>  
>  	/* Are the I/O ports here ? */
> @@ -442,17 +439,16 @@ static int sx_probe(struct specialix_boa
>  	if ((val1 != 0x5a) || (val2 != 0xa5)) {
>  		printk(KERN_INFO "sx%d: specialix IO8+ Board at 0x%03x not found.\n",
>  		       board_No(bp), bp->base);
> -		return 1;
> +		err = 1;
> +		goto probe_err;
>  	}
>  
>  	/* Check the DSR lines that Specialix uses as board 
>  	   identification */
>  	val1 = read_cross_byte (bp, CD186x_MSVR, MSVR_DSR);
>  	val2 = read_cross_byte (bp, CD186x_MSVR, MSVR_RTS);
> -#ifdef SPECIALIX_DEBUG
> -	printk (KERN_DEBUG "sx%d: DSR lines are: %02x, rts lines are: %02x\n", 
> +	dprintk ("sx%d: DSR lines are: %02x, rts lines are: %02x\n", 
ditto
>  	        board_No(bp),  val1, val2);
> -#endif
>  	/* They managed to switch the bit order between the docs and
>  	   the IO8+ card. The new PCI card now conforms to old docs.
>  	   They changed the PCI docs to reflect the situation on the
> @@ -461,57 +457,16 @@ static int sx_probe(struct specialix_boa
>  	if (val1 != val2) {
>  		printk(KERN_INFO "sx%d: specialix IO8+ ID %02x at 0x%03x not found (%02x).\n",
>  		       board_No(bp), val2, bp->base, val1);
> -		return 1;
> +		err = 1;
> +		goto probe_err;
>  	}
>  
> -
> -#if 0
> -	/* It's time to find IRQ for this board */
> -	for (retries = 0; retries < 5 && irqs <= 0; retries++) {
> -		irqs = probe_irq_on();
> -		sx_init_CD186x(bp);	       		/* Reset CD186x chip       */
> -		sx_out(bp, CD186x_CAR, 2);               /* Select port 2          */
> -		sx_wait_CCR(bp);
> -		sx_out(bp, CD186x_CCR, CCR_TXEN);        /* Enable transmitter     */
> -		sx_out(bp, CD186x_IER, IER_TXRDY);       /* Enable tx empty intr   */
> -		sx_long_delay(HZ/20);	       		
> -		irqs = probe_irq_off(irqs);
> -
> -#if SPECIALIX_DEBUG > 2
> -		printk (KERN_DEBUG "SRSR = %02x, ",  sx_in(bp, CD186x_SRSR));
> -		printk (           "TRAR = %02x, ",  sx_in(bp, CD186x_TRAR));
> -		printk (           "GIVR = %02x, ",  sx_in(bp, CD186x_GIVR));
> -		printk (           "GICR = %02x, ",  sx_in(bp, CD186x_GICR));
> -		printk (           "\n");
> -#endif
> -		/* Reset CD186x again      */
> -		if (!sx_init_CD186x(bp)) {
> -			/* Hmmm. This is dead code anyway. */
> -		}
> -#if SPECIALIX_DEBUG > 2
> -		printk (KERN_DEBUG "val1 = %02x, val2 = %02x, val3 = %02x.\n", 
> -		        val1, val2, val3); 
> -#endif
> -	
> -	}
> -	
> -#if 0
> -	if (irqs <= 0) {
> -		printk(KERN_ERR "sx%d: Can't find IRQ for specialix IO8+ board at 0x%03x.\n",
> -		       board_No(bp), bp->base);
> -		return 1;
> -	}
> -#endif
> -	printk (KERN_INFO "Started with irq=%d, but now have irq=%d.\n", bp->irq, irqs);
> -	if (irqs > 0)
> -		bp->irq = irqs;
> -#endif
>  	/* Reset CD186x again  */
>  	if (!sx_init_CD186x(bp)) {
> -		return -EIO;
> +		err = -EIO;
> +		goto probe_err;
>  	}
>  
> -	sx_request_io_range(bp);
>  	bp->flags |= SX_BOARD_PRESENT;
>  	
>  	/* Chip           revcode   pkgtype
> @@ -532,9 +487,7 @@ static int sx_probe(struct specialix_boa
>  	default:chip=-1;rev='x';
>  	}
>  
> -#if SPECIALIX_DEBUG > 2
> -	printk (KERN_DEBUG " GFCR = 0x%02x\n", sx_in_off(bp, CD186x_GFRCR) );
> -#endif
> +	dprintk(2, " GFCR = 0x%02x\n", sx_in_off(bp, CD186x_GFRCR) );
>  
>  #ifdef SPECIALIX_TIMER
>  	init_timer (&missed_irq_timer);
> @@ -550,6 +503,10 @@ static int sx_probe(struct specialix_boa
>  	       chip, rev);
>  
>  	return 0;
> +
> + probe_err:
> +	sx_release_io_range(bp);
> +	return err;
>  }
>  
>  /* 
> @@ -604,10 +561,8 @@ static inline void sx_receive_exc(struct
>  	status = sx_in(bp, CD186x_RCSR);
>  	if (status & RCSR_OE) {
>  		port->overrun++;
> -#if SPECIALIX_DEBUG 
> -		printk(KERN_DEBUG "sx%d: port %d: Overrun. Total %ld overruns.\n", 
> -		       board_No(bp), port_No(port), port->overrun);
> -#endif		
> +		dprintk(debug, "sx%d: port %d: Overrun. Total %ld overruns.\n", 
> +		       board_No(bp), port_No(port), port->overrun);		
aahem
>  	}
>  	status &= port->mark_mask;
>  #else	
> @@ -623,10 +578,8 @@ static inline void sx_receive_exc(struct
>  		return;
>  		
>  	} else if (status & RCSR_BREAK) {
> -#ifdef SPECIALIX_DEBUG
> -		printk(KERN_DEBUG "sx%d: port %d: Handling break...\n",
> +		dprintk(debug, "sx%d: port %d: Handling break...\n",
>  		       board_No(bp), port_No(port));
> -#endif
>  		*tty->flip.flag_buf_ptr++ = TTY_BREAK;
>  		if (port->flags & ASYNC_SAK)
>  			do_SAK(tty);
> @@ -756,9 +709,8 @@ static inline void sx_check_modem(struct
>  	struct tty_struct *tty;
>  	unsigned char mcr;
>  
> -#ifdef SPECIALIX_DEBUG
> -	printk (KERN_DEBUG "Modem intr. ");
> -#endif
> +	dprintk (debug, "Modem intr. ");
> +
>  	if (!(port = sx_get_port(bp, "Modem")))
>  		return;
>  	
> @@ -768,18 +720,12 @@ static inline void sx_check_modem(struct
>  	printk ("mcr = %02x.\n", mcr);
>  
>  	if ((mcr & MCR_CDCHG)) {
> -#ifdef SPECIALIX_DEBUG 
> -		printk (KERN_DEBUG "CD just changed... ");
> -#endif
> +		dprintk (debug, "CD just changed... ");
>  		if (sx_in(bp, CD186x_MSVR) & MSVR_CD) {
> -#ifdef SPECIALIX_DEBUG
> -			printk ( "Waking up guys in open.\n");
> -#endif
> +			dprintk (debug, "Waking up guys in open.\n");
>  			wake_up_interruptible(&port->open_wait);
>  		} else {
> -#ifdef SPECIALIX_DEBUG
> -			printk ( "Sending HUP.\n");
> -#endif
> +			dprintk (debug, "Sending HUP.\n");
>  			schedule_work(&port->tqueue_hangup);
>  		}
>  	}
> @@ -828,9 +774,7 @@ static irqreturn_t sx_interrupt(int irq,
>  	bp = dev_id;
>  	
>  	if (!bp || !(bp->flags & SX_BOARD_ACTIVE)) {
> -#ifdef SPECIALIX_DEBUG 
> -		printk (KERN_DEBUG "sx: False interrupt. irq %d.\n", irq);
> -#endif
> +		dprintk (debug, "sx: False interrupt. irq %d.\n", irq);
>  		return IRQ_NONE;
>  	}
>  
> @@ -933,9 +877,8 @@ static inline void sx_shutdown_board(str
>  	
>  	bp->flags &= ~SX_BOARD_ACTIVE;
>  	
> -#if SPECIALIX_DEBUG > 2
> -	printk ("Freeing IRQ%d for board %d.\n", bp->irq, board_No (bp));
> -#endif
> +	dprintk (3, "Freeing IRQ%d for board %d.\n", bp->irq, board_No (bp));
> +
>  	free_irq(bp->irq, bp);
>  
>  	turn_ints_off (bp);
> @@ -970,9 +913,7 @@ static void sx_change_speed(struct speci
>  		port->MSVR = MSVR_DTR | (sx_in(bp, CD186x_MSVR) & MSVR_RTS);
>  	else
>  		port->MSVR =  (sx_in(bp, CD186x_MSVR) & MSVR_RTS);
> -#ifdef DEBUG_SPECIALIX
> -	printk (KERN_DEBUG "sx: got MSVR=%02x.\n", port->MSVR);
> -#endif
> +	dprintk(debug, "sx: got MSVR=%02x.\n", port->MSVR);
>  	baud = C_BAUD(tty);
>  	
>  	if (baud & CBAUDEX) {
> @@ -992,17 +933,13 @@ static void sx_change_speed(struct speci
>  	
>  	if (!baud_table[baud]) {
>  		/* Drop DTR & exit */
> -#ifdef SPECIALIX_DEBUG
> -		printk (KERN_DEBUG "Dropping DTR...  Hmm....\n");
> -#endif
> +		dprintk(debug, "Dropping DTR...  Hmm....\n");
>  		if (!SX_CRTSCTS (tty)) {
>  			port -> MSVR &= ~ MSVR_DTR;
>  			sx_out(bp, CD186x_MSVR, port->MSVR );
>  		} 
> -#ifdef DEBUG_SPECIALIX
>  		else
> -			printk (KERN_DEBUG "Can't drop DTR: no DTR.\n");
> -#endif
> +			dprintk (debug, "Can't drop DTR: no DTR.\n");
>  		return;
>  	} else {
>  		/* Set DTR on */
> @@ -1147,9 +1084,7 @@ static void sx_change_speed(struct speci
>  	sx_wait_CCR(bp);
>  	sx_out(bp, CD186x_CCR, CCR_CORCHG1 | CCR_CORCHG2 | CCR_CORCHG3);
>  	/* Setting up modem option registers */
> -#ifdef DEBUG_SPECIALIX
> -	printk ("Mcor1 = %02x, mcor2 = %02x.\n", mcor1, mcor2);
> -#endif
> +	dprintk (debug, "Mcor1 = %02x, mcor2 = %02x.\n", mcor1, mcor2);
>  	sx_out(bp, CD186x_MCOR1, mcor1);
>  	sx_out(bp, CD186x_MCOR2, mcor2);
>  	/* Enable CD186x transmitter & receiver */
> @@ -1372,10 +1307,8 @@ static int sx_open(struct tty_struct * t
>  	bp = &sx_board[board];
>  	port = sx_port + board * SX_NPORT + SX_PORT(tty->index);
>  
> -#ifdef DEBUG_SPECIALIX
> -	printk (KERN_DEBUG "Board = %d, bp = %p, port = %p, portno = %d.\n", 
> +	dprintk (debug, "Board = %d, bp = %p, port = %p, portno = %d.\n", 
another
>  	        board, bp, port, SX_PORT(tty->index));
> -#endif
>  
>  	if (sx_paranoia_check(port, tty->name, "sx_open"))
>  		return -ENODEV;
> @@ -1669,11 +1602,10 @@ static int sx_tiocmget(struct tty_struct
>  	sx_out(bp, CD186x_CAR, port_No(port));
>  	status = sx_in(bp, CD186x_MSVR);
>  	restore_flags(flags);
> -#ifdef DEBUG_SPECIALIX
> -	printk (KERN_DEBUG "Got msvr[%d] = %02x, car = %d.\n", 
> +	dprintk (debug, "Got msvr[%d] = %02x, car = %d.\n", 
you'll guess..
>  		port_No(port), status, sx_in (bp, CD186x_CAR));
> -	printk (KERN_DEBUG "sx_port = %p, port = %p\n", sx_port, port);
> -#endif
> +	dprintk (debug, "sx_port = %p, port = %p\n", sx_port, port);
> +
>  	if (SX_CRTSCTS(port->tty)) {
>  		result  = /*   (status & MSVR_RTS) ? */ TIOCM_DTR /* : 0) */ 
>  		          |   ((status & MSVR_DTR) ? TIOCM_RTS : 0)
> -- 
> WWXD (What Would Xena Do?) 

xena prefers patches conataining no trailing spaces! :)
oooh and i just picked ramdom places above, 
there are more over all that patch..
thx corrected for the next kjt

a++ maks


[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Kernel-janitors] [PATCH] remove check_region specialix
  2004-07-07 20:24 [Kernel-janitors] [PATCH] remove check_region specialix Kristen Carlson
                   ` (4 preceding siblings ...)
  2004-07-11 13:15 ` maximilian attems
@ 2004-07-12 16:11 ` Kristen Carlson
  2004-07-12 16:30 ` Roger Luethi
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Kristen Carlson @ 2004-07-12 16:11 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 330 bytes --]

Thanks!  For my benefit, what's the easiest way to check for trailing spaces?
I thought I had gotten all of them by just looking at the patch and noting
which lines shouldn't have needed modifications but I obviously missed some,
especially on the lines that I was modifying anyway.  Do you have a more 
foolproof method?
Kristen

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Kernel-janitors] [PATCH] remove check_region specialix
  2004-07-07 20:24 [Kernel-janitors] [PATCH] remove check_region specialix Kristen Carlson
                   ` (5 preceding siblings ...)
  2004-07-12 16:11 ` Kristen Carlson
@ 2004-07-12 16:30 ` Roger Luethi
  2004-07-12 16:42 ` maximilian attems
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Roger Luethi @ 2004-07-12 16:30 UTC (permalink / raw)
  To: kernel-janitors

On Mon, 12 Jul 2004 09:11:51 -0700, Kristen Carlson wrote:
> Thanks!  For my benefit, what's the easiest way to check for trailing spaces?

One way is to have your editor show them. I have this in my .vimrc
(the characters I picked may or may not work well for your locale):
set list
set listchars=tab:»·,trail:·

Works for me.

Roger
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Kernel-janitors] [PATCH] remove check_region specialix
  2004-07-07 20:24 [Kernel-janitors] [PATCH] remove check_region specialix Kristen Carlson
                   ` (6 preceding siblings ...)
  2004-07-12 16:30 ` Roger Luethi
@ 2004-07-12 16:42 ` maximilian attems
  2004-07-12 21:14 ` [Kernel-janitors] [PATCH] remove check_region from eepro.c Kristen Carlson
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: maximilian attems @ 2004-07-12 16:42 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 878 bytes --]

On Mon, 12 Jul 2004, Kristen Carlson wrote:

> Thanks!  For my benefit, what's the easiest way to check for trailing spaces?
> I thought I had gotten all of them by just looking at the patch and noting
> which lines shouldn't have needed modifications but I obviously missed some,
> especially on the lines that I was modifying anyway.  Do you have a more 
> foolproof method?
> Kristen

Domen posted that one on kj (bonus detects also spaces instead of tabs):
egrep -n "^\+(.* $|        )"

in vim you may want to :set list for viewing those spaces
and afterwards :set nolist.

another possibility is to use the patch-scripts from Andrew Morton,
they warn you when trailing space is introduced.
that's the place, where i see most of them ;)
-> http://developer.osdl.org/rddunlap/patch-scripts/
or latest http://www.zip.com.au/~akpm/linux/patches/patch-scripts-0.18/

a++ maks


[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Kernel-janitors] [PATCH] remove check_region from eepro.c
  2004-07-07 20:24 [Kernel-janitors] [PATCH] remove check_region specialix Kristen Carlson
                   ` (7 preceding siblings ...)
  2004-07-12 16:42 ` maximilian attems
@ 2004-07-12 21:14 ` Kristen Carlson
  2004-07-13  1:55 ` [Kernel-janitors] [PATCH] remove check_region specialix Randy.Dunlap
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Kristen Carlson @ 2004-07-12 21:14 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 925 bytes --]

Description: remove check_region from eepro.c 
(note that PnPWakeup must be defined to have this code compile)

Signed-off-by: Kristen Carlson Accardi <kristenc@cs.pdx.edu>


--- linux-2.6.7-kc/drivers/net/eepro.orig	2004-07-12 10:42:08.000000000 +0000
+++ linux-2.6.7-kc/drivers/net/eepro.c	2004-07-12 10:29:46.000000000 +0000
@@ -545,9 +545,9 @@ static int __init do_eepro_probe(struct 
 
 	{
 		unsigned short int WS[32]=WakeupSeq;
-
-		if (check_region(WakeupPort, 2)==0) {
-
+		if (!request_region(WakeupPort, 2, dev->name))
+			printk(KERN_WARNING "PnPWakeup Port not available");
+		else { 
 			if (net_debug>5)
 				printk(KERN_DEBUG "Waking UP\n");
 
@@ -557,7 +557,8 @@ static int __init do_eepro_probe(struct 
 				outb_p(WS[i],WakeupPort);
 				if (net_debug>5) printk(KERN_DEBUG ": %#x ",WS[i]);
 			}
-		} else printk(KERN_WARNING "Checkregion Failed!\n");
+			release_region(WakeupPort, 2);
+		}
 	}
 #endif
 

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Kernel-janitors] [PATCH] remove check_region specialix
  2004-07-07 20:24 [Kernel-janitors] [PATCH] remove check_region specialix Kristen Carlson
                   ` (8 preceding siblings ...)
  2004-07-12 21:14 ` [Kernel-janitors] [PATCH] remove check_region from eepro.c Kristen Carlson
@ 2004-07-13  1:55 ` Randy.Dunlap
  2004-07-13 10:53 ` Roger Luethi
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Randy.Dunlap @ 2004-07-13  1:55 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 711 bytes --]

On Mon, 12 Jul 2004 09:11:51 -0700 (PDT) Kristen Carlson wrote:

| Thanks!  For my benefit, what's the easiest way to check for trailing spaces?
| I thought I had gotten all of them by just looking at the patch and noting
| which lines shouldn't have needed modifications but I obviously missed some,
| especially on the lines that I was modifying anyway.  Do you have a more 
| foolproof method?


Every time that I apply a patch (similar to what max suggested),
I have a script that does 'patch --dry-run' and also greps for
trailing spaces.

This is my 'dryrun' script:

#! /bin/sh
patch -sp1 --dry-run -i $1 $2 $3 $4 $5 $6 $7 $8 $9
echo "===== trailing spaces? ====="
grep -n "^+.* $" $1 | more


--
~Randy

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Kernel-janitors] [PATCH] remove check_region specialix
  2004-07-07 20:24 [Kernel-janitors] [PATCH] remove check_region specialix Kristen Carlson
                   ` (9 preceding siblings ...)
  2004-07-13  1:55 ` [Kernel-janitors] [PATCH] remove check_region specialix Randy.Dunlap
@ 2004-07-13 10:53 ` Roger Luethi
  2004-08-14 14:15 ` [Kernel-janitors] [PATCH] remove check_region from eepro.c maximilian attems
  2004-08-14 20:06 ` [Kernel-janitors] [PATCH] remove check_region specialix maximilian attems
  12 siblings, 0 replies; 14+ messages in thread
From: Roger Luethi @ 2004-07-13 10:53 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 554 bytes --]

On Mon, 12 Jul 2004 18:55:39 -0700, Randy.Dunlap wrote:
> This is my 'dryrun' script:
> 
> #! /bin/sh
> patch -sp1 --dry-run -i $1 $2 $3 $4 $5 $6 $7 $8 $9

What are all these arguments meant for? Why not $@?

> echo "===== trailing spaces? ====="
> grep -n "^+.* $" $1 | more

How about
grep -n '^+.*[[:space:]]$'
instead? Catches trailing tabs as well. And yes, there are plenty of those
in the kernel.

I notice whitespace problems when I review the code anyway, but script
users should also try and catch these:

  	f();
should really be
	f();

Roger

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Kernel-janitors] [PATCH] remove check_region from eepro.c
  2004-07-07 20:24 [Kernel-janitors] [PATCH] remove check_region specialix Kristen Carlson
                   ` (10 preceding siblings ...)
  2004-07-13 10:53 ` Roger Luethi
@ 2004-08-14 14:15 ` maximilian attems
  2004-08-14 20:06 ` [Kernel-janitors] [PATCH] remove check_region specialix maximilian attems
  12 siblings, 0 replies; 14+ messages in thread
From: maximilian attems @ 2004-08-14 14:15 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 310 bytes --]

On Mon, 12 Jul 2004, Kristen Carlson wrote:

> Description: remove check_region from eepro.c 
> (note that PnPWakeup must be defined to have this code compile)
> 
> Signed-off-by: Kristen Carlson Accardi <kristenc@cs.pdx.edu>

as matthew wilcox suggested rip out that unused "not-really PnP Wakeup"

a++ maks


[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Kernel-janitors] [PATCH] remove check_region specialix
  2004-07-07 20:24 [Kernel-janitors] [PATCH] remove check_region specialix Kristen Carlson
                   ` (11 preceding siblings ...)
  2004-08-14 14:15 ` [Kernel-janitors] [PATCH] remove check_region from eepro.c maximilian attems
@ 2004-08-14 20:06 ` maximilian attems
  12 siblings, 0 replies; 14+ messages in thread
From: maximilian attems @ 2004-08-14 20:06 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 459 bytes --]

On Mon, 12 Jul 2004, Kristen Carlson wrote:

> Thanks!  For my benefit, what's the easiest way to check for trailing spaces?
> I thought I had gotten all of them by just looking at the patch and noting
> which lines shouldn't have needed modifications but I obviously missed some,
> especially on the lines that I was modifying anyway.  Do you have a more 
> foolproof method?
> Kristen

your patch doesn't anymore apply to 2.6.8,
please resend it!
a++ maks


[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2004-08-14 20:06 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-07-07 20:24 [Kernel-janitors] [PATCH] remove check_region specialix Kristen Carlson
2004-07-07 20:40 ` Matthew Wilcox
2004-07-07 21:05 ` Kristen Carlson
2004-07-07 21:28 ` Michael Veeck
2004-07-07 21:59 ` Kristen Carlson
2004-07-11 13:15 ` maximilian attems
2004-07-12 16:11 ` Kristen Carlson
2004-07-12 16:30 ` Roger Luethi
2004-07-12 16:42 ` maximilian attems
2004-07-12 21:14 ` [Kernel-janitors] [PATCH] remove check_region from eepro.c Kristen Carlson
2004-07-13  1:55 ` [Kernel-janitors] [PATCH] remove check_region specialix Randy.Dunlap
2004-07-13 10:53 ` Roger Luethi
2004-08-14 14:15 ` [Kernel-janitors] [PATCH] remove check_region from eepro.c maximilian attems
2004-08-14 20:06 ` [Kernel-janitors] [PATCH] remove check_region specialix maximilian attems

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.