[PATCH 1/4] Char: merge ip2main and ip2base

[PATCH 1/4] Char: merge ip2main and ip2base

[Jiri Slaby]

It's pretty useless to have one setup() function separated along with
module_init() which only calls a function from ip2main anyway. Get rid
of ip2base.

Remove also checks of always-true now.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
---
 drivers/char/ip2/Makefile  |    2 +-
 drivers/char/ip2/ip2base.c |  108 --------------------------------------------
 drivers/char/ip2/ip2main.c |   92 +++++++++++++++++++++++++++++++-------
 3 files changed, 77 insertions(+), 125 deletions(-)
 delete mode 100644 drivers/char/ip2/ip2base.c

diff --git a/drivers/char/ip2/Makefile b/drivers/char/ip2/Makefile
index 939618f..bc397d9 100644
--- a/drivers/char/ip2/Makefile
+++ b/drivers/char/ip2/Makefile
@@ -4,5 +4,5 @@
 
 obj-$(CONFIG_COMPUTONE)         += ip2.o
 
-ip2-objs			:= ip2base.o ip2main.o
+ip2-objs			:= ip2main.o
 
diff --git a/drivers/char/ip2/ip2base.c b/drivers/char/ip2/ip2base.c
deleted file mode 100644
index 8155e24..0000000
--- a/drivers/char/ip2/ip2base.c
+++ /dev/null
@@ -1,108 +0,0 @@
-// ip2.c
-// This is a dummy module to make the firmware available when needed
-// and allows it to be unloaded when not. Rumor is the __initdata 
-// macro doesn't always works on all platforms so we use this kludge.
-// If not compiled as a module it just makes fip_firm avaliable then
-//  __initdata should work as advertized
-//
-
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/wait.h>
-
-#ifndef __init
-#define __init
-#endif
-#ifndef __initfunc
-#define __initfunc(a) a
-#endif
-#ifndef __initdata
-#define __initdata
-#endif
-
-#include "ip2types.h"		
-
-int
-ip2_loadmain(int *, int *); // ref into ip2main.c
-
-/* Note: Add compiled in defaults to these arrays, not to the structure
-	in ip2.h any longer.  That structure WILL get overridden
-	by these values, or command line values, or insmod values!!!  =mhw=
-*/
-static int io[IP2_MAX_BOARDS]= { 0, 0, 0, 0 };
-static int irq[IP2_MAX_BOARDS] = { -1, -1, -1, -1 }; 
-
-static int poll_only = 0;
-
-MODULE_AUTHOR("Doug McNash");
-MODULE_DESCRIPTION("Computone IntelliPort Plus Driver");
-module_param_array(irq, int, NULL, 0);
-MODULE_PARM_DESC(irq,"Interrupts for IntelliPort Cards");
-module_param_array(io, int, NULL, 0);
-MODULE_PARM_DESC(io,"I/O ports for IntelliPort Cards");
-module_param(poll_only, bool, 0);
-MODULE_PARM_DESC(poll_only,"Do not use card interrupts");
-
-
-static int __init ip2_init(void)
-{
-	if( poll_only ) {
-		/* Hard lock the interrupts to zero */
-		irq[0] = irq[1] = irq[2] = irq[3] = 0;
-	}
-
-	return ip2_loadmain(io, irq);
-}
-module_init(ip2_init);
-
-MODULE_LICENSE("GPL");
-
-#ifndef MODULE
-/******************************************************************************
- *	ip2_setup:
- *		str: kernel command line string
- *
- *	Can't autoprobe the boards so user must specify configuration on
- *	kernel command line.  Sane people build it modular but the others
- *	come here.
- *
- *	Alternating pairs of io,irq for up to 4 boards.
- *		ip2=io0,irq0,io1,irq1,io2,irq2,io3,irq3
- *
- *		io=0 => No board
- *		io=1 => PCI
- *		io=2 => EISA
- *		else => ISA I/O address
- *
- *		irq=0 or invalid for ISA will revert to polling mode
- *
- *		Any value = -1, do not overwrite compiled in value.
- *
- ******************************************************************************/
-static int __init ip2_setup(char *str)
-{
-	int	ints[10];	/* 4 boards, 2 parameters + 2 */
-	int	i, j;
-
-	str = get_options (str, ARRAY_SIZE(ints), ints);
-
-	for( i = 0, j = 1; i < 4; i++ ) {
-		if( j > ints[0] ) {
-			break;
-		}
-		if( ints[j] >= 0 ) {
-			io[i] = ints[j];
-		}
-		j++;
-		if( j > ints[0] ) {
-			break;
-		}
-		if( ints[j] >= 0 ) {
-			irq[i] = ints[j];
-		}
-		j++;
-	}
-	return 1;
-}
-__setup("ip2=", ip2_setup);
-#endif /* !MODULE */
diff --git a/drivers/char/ip2/ip2main.c b/drivers/char/ip2/ip2main.c
index d00b132..8ac1dfe 100644
--- a/drivers/char/ip2/ip2main.c
+++ b/drivers/char/ip2/ip2main.c
@@ -157,9 +157,6 @@ static char *pcVersion = "1.2.14";
 static char *pcDriver_name   = "ip2";
 static char *pcIpl    		 = "ip2ipl";
 
-// cheezy kludge or genius - you decide?
-int ip2_loadmain(int *, int *);
-
 /***********************/
 /* Function Prototypes */
 /***********************/
@@ -287,6 +284,7 @@ static int tracewrap;
 
 MODULE_AUTHOR("Doug McNash");
 MODULE_DESCRIPTION("Computone IntelliPort Plus Driver");
+MODULE_LICENSE("GPL");
 
 static int poll_only = 0;
 
@@ -297,6 +295,22 @@ static int iindx;
 static char rirqs[IP2_MAX_BOARDS];
 static int Valid_Irqs[] = { 3, 4, 5, 7, 10, 11, 12, 15, 0};
 
+/* Note: Add compiled in defaults to these arrays, not to the structure
+	in ip2.h any longer.  That structure WILL get overridden
+	by these values, or command line values, or insmod values!!!  =mhw=
+*/
+static int io[IP2_MAX_BOARDS];
+static int irq[IP2_MAX_BOARDS] = { -1, -1, -1, -1 };
+
+MODULE_AUTHOR("Doug McNash");
+MODULE_DESCRIPTION("Computone IntelliPort Plus Driver");
+module_param_array(irq, int, NULL, 0);
+MODULE_PARM_DESC(irq, "Interrupts for IntelliPort Cards");
+module_param_array(io, int, NULL, 0);
+MODULE_PARM_DESC(io, "I/O ports for IntelliPort Cards");
+module_param(poll_only, bool, 0);
+MODULE_PARM_DESC(poll_only, "Do not use card interrupts");
+
 /* for sysfs class support */
 static struct class *ip2_class;
 
@@ -492,8 +506,53 @@ static const struct firmware *ip2_request_firmware(void)
 	return fw;
 }
 
-int
-ip2_loadmain(int *iop, int *irqp)
+#ifndef MODULE
+/******************************************************************************
+ *	ip2_setup:
+ *		str: kernel command line string
+ *
+ *	Can't autoprobe the boards so user must specify configuration on
+ *	kernel command line.  Sane people build it modular but the others
+ *	come here.
+ *
+ *	Alternating pairs of io,irq for up to 4 boards.
+ *		ip2=io0,irq0,io1,irq1,io2,irq2,io3,irq3
+ *
+ *		io=0 => No board
+ *		io=1 => PCI
+ *		io=2 => EISA
+ *		else => ISA I/O address
+ *
+ *		irq=0 or invalid for ISA will revert to polling mode
+ *
+ *		Any value = -1, do not overwrite compiled in value.
+ *
+ ******************************************************************************/
+static int __init ip2_setup(char *str)
+{
+	int j, ints[10];	/* 4 boards, 2 parameters + 2 */
+	unsigned int i;
+
+	str = get_options(str, ARRAY_SIZE(ints), ints);
+
+	for (i = 0, j = 1; i < 4; i++) {
+		if (j > ints[0])
+			break;
+		if (ints[j] >= 0)
+			io[i] = ints[j];
+		j++;
+		if (j > ints[0])
+			break;
+		if (ints[j] >= 0)
+			irq[i] = ints[j];
+		j++;
+	}
+	return 1;
+}
+__setup("ip2=", ip2_setup);
+#endif /* !MODULE */
+
+static int ip2_loadmain(void)
 {
 	int i, j, box;
 	int err = 0;
@@ -503,6 +562,11 @@ ip2_loadmain(int *iop, int *irqp)
 	static struct pci_dev *pci_dev_i = NULL;
 	const struct firmware *fw = NULL;
 
+	if (poll_only) {
+		/* Hard lock the interrupts to zero */
+		irq[0] = irq[1] = irq[2] = irq[3] = poll_only = 0;
+	}
+
 	ip2trace (ITRC_NO_PORT, ITRC_INIT, ITRC_ENTER, 0 );
 
 	/* process command line arguments to modprobe or
@@ -510,14 +574,11 @@ ip2_loadmain(int *iop, int *irqp)
 	/* irqp and iop should ALWAYS be specified now...  But we check
 		them individually just to be sure, anyways... */
 	for ( i = 0; i < IP2_MAX_BOARDS; ++i ) {
-		if (iop) {
-			ip2config.addr[i] = iop[i];
-			if (irqp) {
-				if( irqp[i] >= 0 ) {
-					ip2config.irq[i] = irqp[i];
-				} else {
-					ip2config.irq[i] = 0;
-				}
+		ip2config.addr[i] = io[i];
+		if (irq[i] >= 0)
+			ip2config.irq[i] = irq[i];
+		else
+			ip2config.irq[i] = 0;
 	// This is a little bit of a hack.  If poll_only=1 on command
 	// line back in ip2.c OR all IRQs on all specified boards are
 	// explicitly set to 0, then drop to poll only mode and override
@@ -529,9 +590,7 @@ ip2_loadmain(int *iop, int *irqp)
 	// to -1, is to use 0 as a hard coded, do not probe.
 	//
 	//	/\/\|=mhw=|\/\/
-				poll_only |= irqp[i];
-			}
-		}
+		poll_only |= irq[i];
 	}
 	poll_only = !poll_only;
 
@@ -781,6 +840,7 @@ out_chrdev:
 out:
 	return err;
 }
+module_init(ip2_loadmain);
 
 /******************************************************************************/
 /* Function:   ip2_init_board()                                               */
-- 
1.5.5.1

[PATCH 2/4] Char: ip2, cleanup globals

[Jiri Slaby]

- do not init .bss zeroed data to zero again (by memset or
  explicit assignment)
- use char [] instead of char * for string constants

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
---
 drivers/char/ip2/ip2main.c |   18 +++++++-----------
 1 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/char/ip2/ip2main.c b/drivers/char/ip2/ip2main.c
index 8ac1dfe..4174c65 100644
--- a/drivers/char/ip2/ip2main.c
+++ b/drivers/char/ip2/ip2main.c
@@ -150,12 +150,12 @@ static int ip2_read_proc(char *, char **, off_t, int, int *, void * );
 /*************/
 
 /* String constants to identify ourselves */
-static char *pcName    = "Computone IntelliPort Plus multiport driver";
-static char *pcVersion = "1.2.14";
+static const char pcName[] = "Computone IntelliPort Plus multiport driver";
+static const char pcVersion[] = "1.2.14";
 
 /* String constants for port names */
-static char *pcDriver_name   = "ip2";
-static char *pcIpl    		 = "ip2ipl";
+static const char pcDriver_name[] = "ip2";
+static const char pcIpl[] = "ip2ipl";
 
 /***********************/
 /* Function Prototypes */
@@ -237,8 +237,8 @@ static const struct file_operations ip2_ipl = {
 	.open		= ip2_ipl_open,
 }; 
 
-static unsigned long irq_counter = 0;
-static unsigned long bh_counter = 0;
+static unsigned long irq_counter;
+static unsigned long bh_counter;
 
 // Use immediate queue to service interrupts
 #define USE_IQI
@@ -286,7 +286,7 @@ MODULE_AUTHOR("Doug McNash");
 MODULE_DESCRIPTION("Computone IntelliPort Plus Driver");
 MODULE_LICENSE("GPL");
 
-static int poll_only = 0;
+static int poll_only;
 
 static int Eisa_irq;
 static int Eisa_slot;
@@ -613,10 +613,6 @@ static int ip2_loadmain(void)
 	/* Initialise the iiEllis subsystem. */
 	iiEllisInit();
 
-	/* Initialize arrays. */
-	memset( i2BoardPtrTable, 0, sizeof i2BoardPtrTable );
-	memset( DevTable, 0, sizeof DevTable );
-
 	/* Initialise all the boards we can find (up to the maximum). */
 	for ( i = 0; i < IP2_MAX_BOARDS; ++i ) {
 		switch ( ip2config.addr[i] ) { 
-- 
1.5.5.1

[PATCH 3/4] Char: ip2, fix sparse warnings

[Jiri Slaby]

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
---
 drivers/char/ip2/ip2main.c |   14 +++++---------
 1 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/char/ip2/ip2main.c b/drivers/char/ip2/ip2main.c
index 4174c65..d9b30ab 100644
--- a/drivers/char/ip2/ip2main.c
+++ b/drivers/char/ip2/ip2main.c
@@ -375,9 +375,7 @@ have_requested_irq( char irq )
 /* handle subsequent installations of the driver. All memory allocated by the */
 /* driver should be returned since it may be unloaded from memory.            */
 /******************************************************************************/
-#ifdef MODULE
-void __exit
-ip2_cleanup_module(void)
+static void __exit ip2_cleanup_module(void)
 {
 	int err;
 	int i;
@@ -431,7 +429,8 @@ ip2_cleanup_module(void)
 			ip2config.pci_dev[i] = NULL;
 		}
 #endif
-		if ((pB = i2BoardPtrTable[i]) != 0 ) {
+		pB = i2BoardPtrTable[i];
+		if (pB != NULL) {
 			kfree ( pB );
 			i2BoardPtrTable[i] = NULL;
 		}
@@ -448,7 +447,6 @@ ip2_cleanup_module(void)
 #endif
 }
 module_exit(ip2_cleanup_module);
-#endif /* MODULE */
 
 static const struct tty_operations ip2_ops = {
 	.open            = ip2_open,
@@ -1253,9 +1251,8 @@ ip2_polled_interrupt(void)
 {
 	int i;
 	i2eBordStrPtr  pB;
-	const int irq = 0;
 
-	ip2trace (ITRC_NO_PORT, ITRC_INTR, 99, 1, irq );
+	ip2trace(ITRC_NO_PORT, ITRC_INTR, 99, 1, 0);
 
 	/* Service just the boards on the list using this irq */
 	for( i = 0; i < i2nBoards; ++i ) {
@@ -1264,9 +1261,8 @@ ip2_polled_interrupt(void)
 //		Only process those boards which match our IRQ.
 //			IRQ = 0 for polled boards, we won't poll "IRQ" boards
 
-		if ( pB && (pB->i2eUsingIrq == irq) ) {
+		if (pB && pB->i2eUsingIrq == 0)
 			ip2_irq_work(pB);
-		}
 	}
 
 	++irq_counter;
-- 
1.5.5.1

[PATCH 4/4] Char: ip2, init/deinit cleanup

[Jiri Slaby]

Cleanup of module_init/exit:
- mostly whitespace
- remove empty functions
- replace c++ comments
- remove useless prints (module loaded, unloaded)
- mark the calls as __exit and __init
- use break; and return; to save some indent levels after it
- note resource leakage

It's still mess, but now it's readable.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
---
 drivers/char/ip2/i2ellis.c |   32 ----
 drivers/char/ip2/i2ellis.h |    2 -
 drivers/char/ip2/ip2main.c |  419 +++++++++++++++++++++-----------------------
 3 files changed, 200 insertions(+), 253 deletions(-)

diff --git a/drivers/char/ip2/i2ellis.c b/drivers/char/ip2/i2ellis.c
index 3601017..29db44d 100644
--- a/drivers/char/ip2/i2ellis.c
+++ b/drivers/char/ip2/i2ellis.c
@@ -69,38 +69,6 @@ static DEFINE_RWLOCK(Dl_spinlock);
 //=======================================================
 
 //******************************************************************************
-// Function:   iiEllisInit()
-// Parameters: None
-//
-// Returns:    Nothing
-//
-// Description:
-//
-// This routine performs any required initialization of the iiEllis subsystem.
-//
-//******************************************************************************
-static void
-iiEllisInit(void)
-{
-}
-
-//******************************************************************************
-// Function:   iiEllisCleanup()
-// Parameters: None
-//
-// Returns:    Nothing
-//
-// Description:
-//
-// This routine performs any required cleanup of the iiEllis subsystem.
-//
-//******************************************************************************
-static void
-iiEllisCleanup(void)
-{
-}
-
-//******************************************************************************
 // Function:   iiSetAddress(pB, address, delay)
 // Parameters: pB      - pointer to the board structure
 //             address - the purported I/O address of the board
diff --git a/drivers/char/ip2/i2ellis.h b/drivers/char/ip2/i2ellis.h
index c88a64e..fb6df24 100644
--- a/drivers/char/ip2/i2ellis.h
+++ b/drivers/char/ip2/i2ellis.h
@@ -511,7 +511,6 @@ typedef void (*delayFunc_t)(unsigned int);
 //
 // Initialization of a board & structure is in four (five!) parts:
 //
-// 0) iiEllisInit()  - Initialize iiEllis subsystem.
 // 1) iiSetAddress() - Define the board address & delay function for a board.
 // 2) iiReset()      - Reset the board   (provided it exists)
 //       -- Note you may do this to several boards --
@@ -523,7 +522,6 @@ typedef void (*delayFunc_t)(unsigned int);
 // loadware.  To change loadware, you must begin again with step 2, resetting
 // the board again (step 1 not needed).
 
-static void iiEllisInit(void);
 static int iiSetAddress(i2eBordStrPtr, int, delayFunc_t );
 static int iiReset(i2eBordStrPtr);
 static int iiResetDelay(i2eBordStrPtr);
diff --git a/drivers/char/ip2/ip2main.c b/drivers/char/ip2/ip2main.c
index d9b30ab..1146e88 100644
--- a/drivers/char/ip2/ip2main.c
+++ b/drivers/char/ip2/ip2main.c
@@ -314,31 +314,27 @@ MODULE_PARM_DESC(poll_only, "Do not use card interrupts");
 /* for sysfs class support */
 static struct class *ip2_class;
 
-// Some functions to keep track of what irq's we have
+/* Some functions to keep track of what irqs we have */
 
-static int
-is_valid_irq(int irq)
+static int __init is_valid_irq(int irq)
 {
 	int *i = Valid_Irqs;
 	
-	while ((*i != 0) && (*i != irq)) {
+	while (*i != 0 && *i != irq)
 		i++;
-	}
-	return (*i);
+
+	return *i;
 }
 
-static void
-mark_requested_irq( char irq )
+static void __init mark_requested_irq(char irq)
 {
 	rirqs[iindx++] = irq;
 }
 
-#ifdef MODULE
-static int
-clear_requested_irq( char irq )
+static int __exit clear_requested_irq(char irq)
 {
 	int i;
-	for ( i = 0; i < IP2_MAX_BOARDS; ++i ) {
+	for (i = 0; i < IP2_MAX_BOARDS; ++i) {
 		if (rirqs[i] == irq) {
 			rirqs[i] = 0;
 			return 1;
@@ -346,17 +342,15 @@ clear_requested_irq( char irq )
 	}
 	return 0;
 }
-#endif
 
-static int
-have_requested_irq( char irq )
+static int have_requested_irq(char irq)
 {
-	// array init to zeros so 0 irq will not be requested as a side effect
+	/* array init to zeros so 0 irq will not be requested as a side
+	 * effect */
 	int i;
-	for ( i = 0; i < IP2_MAX_BOARDS; ++i ) {
+	for (i = 0; i < IP2_MAX_BOARDS; ++i)
 		if (rirqs[i] == irq)
 			return 1;
-	}
 	return 0;
 }
 
@@ -380,46 +374,44 @@ static void __exit ip2_cleanup_module(void)
 	int err;
 	int i;
 
-#ifdef IP2DEBUG_INIT
-	printk (KERN_DEBUG "Unloading %s: version %s\n", pcName, pcVersion );
-#endif
 	/* Stop poll timer if we had one. */
-	if ( TimerOn ) {
-		del_timer ( &PollTimer );
+	if (TimerOn) {
+		del_timer(&PollTimer);
 		TimerOn = 0;
 	}
 
 	/* Reset the boards we have. */
-	for( i = 0; i < IP2_MAX_BOARDS; ++i ) {
-		if ( i2BoardPtrTable[i] ) {
-			iiReset( i2BoardPtrTable[i] );
-		}
-	}
+	for (i = 0; i < IP2_MAX_BOARDS; i++)
+		if (i2BoardPtrTable[i])
+			iiReset(i2BoardPtrTable[i]);
 
 	/* The following is done at most once, if any boards were installed. */
-	for ( i = 0; i < IP2_MAX_BOARDS; ++i ) {
-		if ( i2BoardPtrTable[i] ) {
-			iiResetDelay( i2BoardPtrTable[i] );
+	for (i = 0; i < IP2_MAX_BOARDS; i++) {
+		if (i2BoardPtrTable[i]) {
+			iiResetDelay(i2BoardPtrTable[i]);
 			/* free io addresses and Tibet */
-			release_region( ip2config.addr[i], 8 );
+			release_region(ip2config.addr[i], 8);
 			device_destroy(ip2_class, MKDEV(IP2_IPL_MAJOR, 4 * i));
-			device_destroy(ip2_class, MKDEV(IP2_IPL_MAJOR, 4 * i + 1));
+			device_destroy(ip2_class, MKDEV(IP2_IPL_MAJOR,
+						4 * i + 1));
 		}
 		/* Disable and remove interrupt handler. */
-		if ( (ip2config.irq[i] > 0) && have_requested_irq(ip2config.irq[i]) ) {	
-			free_irq ( ip2config.irq[i], (void *)&pcName);
-			clear_requested_irq( ip2config.irq[i]);
+		if (ip2config.irq[i] > 0 &&
+				have_requested_irq(ip2config.irq[i])) {
+			free_irq(ip2config.irq[i], (void *)&pcName);
+			clear_requested_irq(ip2config.irq[i]);
 		}
 	}
 	class_destroy(ip2_class);
-	if ( ( err = tty_unregister_driver ( ip2_tty_driver ) ) ) {
-		printk(KERN_ERR "IP2: failed to unregister tty driver (%d)\n", err);
-	}
+	err = tty_unregister_driver(ip2_tty_driver);
+	if (err)
+		printk(KERN_ERR "IP2: failed to unregister tty driver (%d)\n",
+				err);
 	put_tty_driver(ip2_tty_driver);
 	unregister_chrdev(IP2_IPL_MAJOR, pcIpl);
 	remove_proc_entry("ip2mem", NULL);
 
-	// free memory
+	/* free memory */
 	for (i = 0; i < IP2_MAX_BOARDS; i++) {
 		void *pB;
 #ifdef CONFIG_PCI
@@ -431,20 +423,14 @@ static void __exit ip2_cleanup_module(void)
 #endif
 		pB = i2BoardPtrTable[i];
 		if (pB != NULL) {
-			kfree ( pB );
+			kfree(pB);
 			i2BoardPtrTable[i] = NULL;
 		}
-		if ((DevTableMem[i]) != NULL ) {
-			kfree ( DevTableMem[i]  );
+		if (DevTableMem[i] != NULL) {
+			kfree(DevTableMem[i]);
 			DevTableMem[i] = NULL;
 		}
 	}
-
-	/* Cleanup the iiEllis subsystem. */
-	iiEllisCleanup();
-#ifdef IP2DEBUG_INIT
-	printk (KERN_DEBUG "IP2 Unloaded\n" );
-#endif
 }
 module_exit(ip2_cleanup_module);
 
@@ -550,14 +536,13 @@ static int __init ip2_setup(char *str)
 __setup("ip2=", ip2_setup);
 #endif /* !MODULE */
 
-static int ip2_loadmain(void)
+static int __init ip2_loadmain(void)
 {
 	int i, j, box;
 	int err = 0;
-	static int loaded;
 	i2eBordStrPtr pB = NULL;
 	int rc = -1;
-	static struct pci_dev *pci_dev_i = NULL;
+	struct pci_dev *pdev = NULL;
 	const struct firmware *fw = NULL;
 
 	if (poll_only) {
@@ -565,119 +550,108 @@ static int ip2_loadmain(void)
 		irq[0] = irq[1] = irq[2] = irq[3] = poll_only = 0;
 	}
 
-	ip2trace (ITRC_NO_PORT, ITRC_INIT, ITRC_ENTER, 0 );
+	ip2trace(ITRC_NO_PORT, ITRC_INIT, ITRC_ENTER, 0);
 
 	/* process command line arguments to modprobe or
 		insmod i.e. iop & irqp */
 	/* irqp and iop should ALWAYS be specified now...  But we check
 		them individually just to be sure, anyways... */
-	for ( i = 0; i < IP2_MAX_BOARDS; ++i ) {
+	for (i = 0; i < IP2_MAX_BOARDS; ++i) {
 		ip2config.addr[i] = io[i];
 		if (irq[i] >= 0)
 			ip2config.irq[i] = irq[i];
 		else
 			ip2config.irq[i] = 0;
-	// This is a little bit of a hack.  If poll_only=1 on command
-	// line back in ip2.c OR all IRQs on all specified boards are
-	// explicitly set to 0, then drop to poll only mode and override
-	// PCI or EISA interrupts.  This superceeds the old hack of
-	// triggering if all interrupts were zero (like da default).
-	// Still a hack but less prone to random acts of terrorism.
-	//
-	// What we really should do, now that the IRQ default is set
-	// to -1, is to use 0 as a hard coded, do not probe.
-	//
-	//	/\/\|=mhw=|\/\/
+	/* This is a little bit of a hack.  If poll_only=1 on command
+	   line back in ip2.c OR all IRQs on all specified boards are
+	   explicitly set to 0, then drop to poll only mode and override
+	   PCI or EISA interrupts.  This superceeds the old hack of
+	   triggering if all interrupts were zero (like da default).
+	   Still a hack but less prone to random acts of terrorism.
+
+	   What we really should do, now that the IRQ default is set
+	   to -1, is to use 0 as a hard coded, do not probe.
+
+		/\/\|=mhw=|\/\/
+	*/
 		poll_only |= irq[i];
 	}
 	poll_only = !poll_only;
 
 	/* Announce our presence */
-	printk( KERN_INFO "%s version %s\n", pcName, pcVersion );
-
-	// ip2 can be unloaded and reloaded for no good reason
-	// we can't let that happen here or bad things happen
-	// second load hoses board but not system - fixme later
-	if (loaded) {
-		printk( KERN_INFO "Still loaded\n" );
-		return 0;
-	}
-	loaded++;
+	printk(KERN_INFO "%s version %s\n", pcName, pcVersion);
 
 	ip2_tty_driver = alloc_tty_driver(IP2_MAX_PORTS);
 	if (!ip2_tty_driver)
 		return -ENOMEM;
 
-	/* Initialise the iiEllis subsystem. */
-	iiEllisInit();
-
 	/* Initialise all the boards we can find (up to the maximum). */
-	for ( i = 0; i < IP2_MAX_BOARDS; ++i ) {
-		switch ( ip2config.addr[i] ) { 
+	for (i = 0; i < IP2_MAX_BOARDS; ++i) {
+		switch (ip2config.addr[i]) {
 		case 0:	/* skip this slot even if card is present */
 			break;
 		default: /* ISA */
 		   /* ISA address must be specified */
-			if ( (ip2config.addr[i] < 0x100) || (ip2config.addr[i] > 0x3f8) ) {
-				printk ( KERN_ERR "IP2: Bad ISA board %d address %x\n",
-							 i, ip2config.addr[i] );
+			if (ip2config.addr[i] < 0x100 ||
+					ip2config.addr[i] > 0x3f8) {
+				printk(KERN_ERR "IP2: Bad ISA board %d "
+						"address %x\n", i,
+						ip2config.addr[i]);
 				ip2config.addr[i] = 0;
-			} else {
-				ip2config.type[i] = ISA;
-
-				/* Check for valid irq argument, set for polling if invalid */
-				if (ip2config.irq[i] && !is_valid_irq(ip2config.irq[i])) {
-					printk(KERN_ERR "IP2: Bad IRQ(%d) specified\n",ip2config.irq[i]);
-					ip2config.irq[i] = 0;// 0 is polling and is valid in that sense
-				}
+				break;
+			}
+			ip2config.type[i] = ISA;
+
+			/* Check for valid irq argument, set for polling if
+			 * invalid */
+			if (ip2config.irq[i] &&
+					!is_valid_irq(ip2config.irq[i])) {
+				printk(KERN_ERR "IP2: Bad IRQ(%d) specified\n",
+						ip2config.irq[i]);
+				/* 0 is polling and is valid in that sense */
+				ip2config.irq[i] = 0;
 			}
 			break;
 		case PCI:
 #ifdef CONFIG_PCI
-			{
-				int status;
+		{
+			u32 addr;
+			int status;
 
-				pci_dev_i = pci_get_device(PCI_VENDOR_ID_COMPUTONE,
-							  PCI_DEVICE_ID_COMPUTONE_IP2EX, pci_dev_i);
-				if (pci_dev_i != NULL) {
-					unsigned int addr;
-
-					if (pci_enable_device(pci_dev_i)) {
-						printk( KERN_ERR "IP2: can't enable PCI device at %s\n",
-							pci_name(pci_dev_i));
-						break;
-					}
-					ip2config.type[i] = PCI;
-					ip2config.pci_dev[i] = pci_dev_get(pci_dev_i);
-					status =
-					pci_read_config_dword(pci_dev_i, PCI_BASE_ADDRESS_1, &addr);
-					if ( addr & 1 ) {
-						ip2config.addr[i]=(USHORT)(addr&0xfffe);
-					} else {
-						printk( KERN_ERR "IP2: PCI I/O address error\n");
-					}
+			pdev = pci_get_device(PCI_VENDOR_ID_COMPUTONE,
+					PCI_DEVICE_ID_COMPUTONE_IP2EX, pdev);
+			if (pdev == NULL) {
+				ip2config.addr[i] = 0;
+				printk(KERN_ERR "IP2: PCI board %d not "
+						"found\n", i);
+				break;
+			}
 
-//		If the PCI BIOS assigned it, lets try and use it.  If we
-//		can't acquire it or it screws up, deal with it then.
-
-//					if (!is_valid_irq(pci_irq)) {
-//						printk( KERN_ERR "IP2: Bad PCI BIOS IRQ(%d)\n",pci_irq);
-//						pci_irq = 0;
-//					}
-					ip2config.irq[i] = pci_dev_i->irq;
-				} else {	// ann error
-					ip2config.addr[i] = 0;
-					printk(KERN_ERR "IP2: PCI board %d not found\n", i);
-				} 
+			if (pci_enable_device(pdev)) {
+				dev_err(&pdev->dev, "can't enable device\n");
+				break;
 			}
+			ip2config.type[i] = PCI;
+			ip2config.pci_dev[i] = pci_dev_get(pdev);
+			status = pci_read_config_dword(pdev, PCI_BASE_ADDRESS_1,
+					&addr);
+			if (addr & 1)
+				ip2config.addr[i] = (USHORT)(addr & 0xfffe);
+			else
+				dev_err(&pdev->dev, "I/O address error\n");
+
+			ip2config.irq[i] = pdev->irq;
+		}
 #else
-			printk( KERN_ERR "IP2: PCI card specified but PCI support not\n");
-			printk( KERN_ERR "IP2: configured in this kernel.\n");
-			printk( KERN_ERR "IP2: Recompile kernel with CONFIG_PCI defined!\n");
+			printk(KERN_ERR "IP2: PCI card specified but PCI "
+					"support not enabled.\n");
+			printk(KERN_ERR "IP2: Recompile kernel with CONFIG_PCI "
+					"defined!\n");
 #endif /* CONFIG_PCI */
 			break;
 		case EISA:
-			if ( (ip2config.addr[i] = find_eisa_board( Eisa_slot + 1 )) != 0) {
+			ip2config.addr[i] = find_eisa_board(Eisa_slot + 1);
+			if (ip2config.addr[i] != 0) {
 				/* Eisa_irq set as side effect, boo */
 				ip2config.type[i] = EISA;
 			} 
@@ -685,31 +659,32 @@ static int ip2_loadmain(void)
 			break;
 		}	/* switch */
 	}	/* for */
-	if (pci_dev_i)
-		pci_dev_put(pci_dev_i);
+	pci_dev_put(pdev);
 
-	for ( i = 0; i < IP2_MAX_BOARDS; ++i ) {
-		if ( ip2config.addr[i] ) {
+	for (i = 0; i < IP2_MAX_BOARDS; ++i) {
+		if (ip2config.addr[i]) {
 			pB = kzalloc(sizeof(i2eBordStr), GFP_KERNEL);
 			if (pB) {
 				i2BoardPtrTable[i] = pB;
-				iiSetAddress( pB, ip2config.addr[i], ii2DelayTimer );
-				iiReset( pB );
-			} else {
-				printk(KERN_ERR "IP2: board memory allocation error\n");
-			}
+				iiSetAddress(pB, ip2config.addr[i],
+						ii2DelayTimer);
+				iiReset(pB);
+			} else
+				printk(KERN_ERR "IP2: board memory allocation "
+						"error\n");
 		}
 	}
-	for ( i = 0; i < IP2_MAX_BOARDS; ++i ) {
-		if ( ( pB = i2BoardPtrTable[i] ) != NULL ) {
-			iiResetDelay( pB );
+	for (i = 0; i < IP2_MAX_BOARDS; ++i) {
+		pB = i2BoardPtrTable[i];
+		if (pB != NULL) {
+			iiResetDelay(pB);
 			break;
 		}
 	}
-	for ( i = 0; i < IP2_MAX_BOARDS; ++i ) {
+	for (i = 0; i < IP2_MAX_BOARDS; ++i) {
 		/* We don't want to request the firmware unless we have at
 		   least one board */
-		if ( i2BoardPtrTable[i] != NULL ) {
+		if (i2BoardPtrTable[i] != NULL) {
 			if (!fw)
 				fw = ip2_request_firmware();
 			if (!fw)
@@ -720,7 +695,7 @@ static int ip2_loadmain(void)
 	if (fw)
 		release_firmware(fw);
 
-	ip2trace (ITRC_NO_PORT, ITRC_INIT, 2, 0 );
+	ip2trace(ITRC_NO_PORT, ITRC_INIT, 2, 0);
 
 	ip2_tty_driver->owner		    = THIS_MODULE;
 	ip2_tty_driver->name                 = "ttyF";
@@ -731,20 +706,23 @@ static int ip2_loadmain(void)
 	ip2_tty_driver->subtype              = SERIAL_TYPE_NORMAL;
 	ip2_tty_driver->init_termios         = tty_std_termios;
 	ip2_tty_driver->init_termios.c_cflag = B9600|CS8|CREAD|HUPCL|CLOCAL;
-	ip2_tty_driver->flags                = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
+	ip2_tty_driver->flags                = TTY_DRIVER_REAL_RAW |
+		TTY_DRIVER_DYNAMIC_DEV;
 	tty_set_operations(ip2_tty_driver, &ip2_ops);
 
-	ip2trace (ITRC_NO_PORT, ITRC_INIT, 3, 0 );
+	ip2trace(ITRC_NO_PORT, ITRC_INIT, 3, 0);
 
-	/* Register the tty devices. */
-	if ( ( err = tty_register_driver ( ip2_tty_driver ) ) ) {
-		printk(KERN_ERR "IP2: failed to register tty driver (%d)\n", err);
+	err = tty_register_driver(ip2_tty_driver);
+	if (err) {
+		printk(KERN_ERR "IP2: failed to register tty driver\n");
 		put_tty_driver(ip2_tty_driver);
-		return -EINVAL;
-	} else
-	/* Register the IPL driver. */
-	if ( ( err = register_chrdev ( IP2_IPL_MAJOR, pcIpl, &ip2_ipl ) ) ) {
-		printk(KERN_ERR "IP2: failed to register IPL device (%d)\n", err );
+		return err; /* leaking resources */
+	}
+
+	err = register_chrdev(IP2_IPL_MAJOR, pcIpl, &ip2_ipl);
+	if (err) {
+		printk(KERN_ERR "IP2: failed to register IPL device (%d)\n",
+				err);
 	} else {
 		/* create the sysfs class */
 		ip2_class = class_create(THIS_MODULE, "ip2");
@@ -756,82 +734,85 @@ static int ip2_loadmain(void)
 	/* Register the read_procmem thing */
 	if (!proc_create("ip2mem",0,NULL,&ip2mem_proc_fops)) {
 		printk(KERN_ERR "IP2: failed to register read_procmem\n");
-	} else {
+		return -EIO; /* leaking resources */
+	}
 
-	ip2trace (ITRC_NO_PORT, ITRC_INIT, 4, 0 );
-		/* Register the interrupt handler or poll handler, depending upon the
-		 * specified interrupt.
-		 */
+	ip2trace(ITRC_NO_PORT, ITRC_INIT, 4, 0);
+	/* Register the interrupt handler or poll handler, depending upon the
+	 * specified interrupt.
+	 */
 
-		for( i = 0; i < IP2_MAX_BOARDS; ++i ) {
-			if ( 0 == ip2config.addr[i] ) {
-				continue;
-			}
+	for (i = 0; i < IP2_MAX_BOARDS; ++i) {
+		if (ip2config.addr[i] == 0)
+			continue;
 
-			if ( NULL != ( pB = i2BoardPtrTable[i] ) ) {
-				device_create_drvdata(ip2_class, NULL,
-						      MKDEV(IP2_IPL_MAJOR, 4 * i),
-						      NULL, "ipl%d", i);
-				device_create_drvdata(ip2_class, NULL,
-						      MKDEV(IP2_IPL_MAJOR, 4 * i + 1),
-						      NULL, "stat%d", i);
-
-			    for ( box = 0; box < ABS_MAX_BOXES; ++box )
-			    {
-			        for ( j = 0; j < ABS_BIGGEST_BOX; ++j )
-			        {
-				    if ( pB->i2eChannelMap[box] & (1 << j) )
-				    {
-				        tty_register_device(ip2_tty_driver,
-					    j + ABS_BIGGEST_BOX *
-						    (box+i*ABS_MAX_BOXES), NULL);
-			    	    }
-			        }
-			    }
-			}
+		pB = i2BoardPtrTable[i];
+		if (pB != NULL) {
+			device_create_drvdata(ip2_class, NULL,
+					      MKDEV(IP2_IPL_MAJOR, 4 * i),
+					      NULL, "ipl%d", i);
+			device_create_drvdata(ip2_class, NULL,
+					      MKDEV(IP2_IPL_MAJOR, 4 * i + 1),
+					      NULL, "stat%d", i);
+
+			for (box = 0; box < ABS_MAX_BOXES; box++)
+				for (j = 0; j < ABS_BIGGEST_BOX; j++)
+					if (pB->i2eChannelMap[box] & (1 << j))
+						tty_register_device(
+							ip2_tty_driver,
+							j + ABS_BIGGEST_BOX *
+							(box+i*ABS_MAX_BOXES),
+							NULL);
+		}
 
-			if (poll_only) {
-//		Poll only forces driver to only use polling and
-//		to ignore the probed PCI or EISA interrupts.
-				ip2config.irq[i] = CIR_POLL;
-			}
-			if ( ip2config.irq[i] == CIR_POLL ) {
+		if (poll_only) {
+			/* Poll only forces driver to only use polling and
+			   to ignore the probed PCI or EISA interrupts. */
+			ip2config.irq[i] = CIR_POLL;
+		}
+		if (ip2config.irq[i] == CIR_POLL) {
 retry:
-				if (!TimerOn) {
-					PollTimer.expires = POLL_TIMEOUT;
-					add_timer ( &PollTimer );
-					TimerOn = 1;
-					printk( KERN_INFO "IP2: polling\n");
-				}
-			} else {
-				if (have_requested_irq(ip2config.irq[i]))
-					continue;
-				rc = request_irq( ip2config.irq[i], ip2_interrupt,
-					IP2_SA_FLAGS | (ip2config.type[i] == PCI ? IRQF_SHARED : 0),
-					pcName, i2BoardPtrTable[i]);
-				if (rc) {
-					printk(KERN_ERR "IP2: an request_irq failed: error %d\n",rc);
-					ip2config.irq[i] = CIR_POLL;
-					printk( KERN_INFO "IP2: Polling %ld/sec.\n",
-							(POLL_TIMEOUT - jiffies));
-					goto retry;
-				} 
-				mark_requested_irq(ip2config.irq[i]);
-				/* Initialise the interrupt handler bottom half (aka slih). */
+			if (!TimerOn) {
+				PollTimer.expires = POLL_TIMEOUT;
+				add_timer(&PollTimer);
+				TimerOn = 1;
+				printk(KERN_INFO "IP2: polling\n");
 			}
-		}
-		for( i = 0; i < IP2_MAX_BOARDS; ++i ) {
-			if ( i2BoardPtrTable[i] ) {
-				set_irq( i, ip2config.irq[i] ); /* set and enable board interrupt */
+		} else {
+			if (have_requested_irq(ip2config.irq[i]))
+				continue;
+			rc = request_irq(ip2config.irq[i], ip2_interrupt,
+				IP2_SA_FLAGS |
+				(ip2config.type[i] == PCI ? IRQF_SHARED : 0),
+				pcName, i2BoardPtrTable[i]);
+			if (rc) {
+				printk(KERN_ERR "IP2: request_irq failed: "
+						"error %d\n", rc);
+				ip2config.irq[i] = CIR_POLL;
+				printk(KERN_INFO "IP2: Polling %ld/sec.\n",
+						(POLL_TIMEOUT - jiffies));
+				goto retry;
 			}
+			mark_requested_irq(ip2config.irq[i]);
+			/* Initialise the interrupt handler bottom half
+			 * (aka slih). */
 		}
 	}
-	ip2trace (ITRC_NO_PORT, ITRC_INIT, ITRC_RETURN, 0 );
-	goto out;
+
+	for (i = 0; i < IP2_MAX_BOARDS; ++i) {
+		if (i2BoardPtrTable[i]) {
+			/* set and enable board interrupt */
+			set_irq(i, ip2config.irq[i]);
+		}
+	}
+
+	ip2trace(ITRC_NO_PORT, ITRC_INIT, ITRC_RETURN, 0);
+
+	return 0;
 
 out_chrdev:
 	unregister_chrdev(IP2_IPL_MAJOR, "ip2");
-out:
+	/* unregister and put tty here */
 	return err;
 }
 module_init(ip2_loadmain);
-- 
1.5.5.1

Re: [PATCH 1/4] Char: merge ip2main and ip2base

[Andrew Morton]

On Sat,  5 Jul 2008 15:28:23 +0200 Jiri Slaby <jirislaby@gmail.com> wrote:

> It's pretty useless to have one setup() function separated along with
> module_init() which only calls a function from ip2main anyway. Get rid
> of ip2base.
> 
> Remove also checks of always-true now.
> 

<catching up>

I applied these four, but there's a fly in our ointment.

On 14 July, Akinobu Mita <akinobu.mita@gmail.com> sent a patch titled
"ip2: avoid add_timer() with pending timer" which Alan acked and then
claimed was "Queued for ttydev tree".  But afaict he then lost it.

If it gets unlost, it will conflict with these changes and I might need
to drop these four (I didn't check).

Perhaps you could review this patch and maybe merge this change on top
of these four?

Thanks.


Date: 	Mon, 14 Jul 2008 11:49:55 +0900
From: Akinobu Mita <akinobu.mita@gmail.com>
To: linux-kernel@vger.kernel.org
Cc: "Michael H. Warfield" <mhw@wittsend.com>
Subject: [PATCH] ip2: avoid add_timer() with pending timer


add_timer() is not suppose to be called when the timer is pending.
ip2 driver attempts to avoid that condition by setting and resetting
a flag (TimerOn) in timer function. But there is some gap between
add_timer() and setting TimerOn.

This patch fix this problem by using mod_timer() and remove TimerOn
which has been unnecessary by this change.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Michael H. Warfield <mhw@wittsend.com>
---
 drivers/char/ip2/ip2main.c |   18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

Index: 2.6-mmotm/drivers/char/ip2/ip2main.c
===================================================================
--- 2.6-mmotm.orig/drivers/char/ip2/ip2main.c
+++ 2.6-mmotm/drivers/char/ip2/ip2main.c
@@ -252,7 +252,6 @@ static unsigned long bh_counter = 0;
  */
 #define  POLL_TIMEOUT   (jiffies + 1)
 static DEFINE_TIMER(PollTimer, ip2_poll, 0, 0);
-static char  TimerOn;
 
 #ifdef IP2DEBUG_TRACE
 /* Trace (debug) buffer data */
@@ -372,10 +371,7 @@ ip2_cleanup_module(void)
 	printk (KERN_DEBUG "Unloading %s: version %s\n", pcName, pcVersion );
 #endif
 	/* Stop poll timer if we had one. */
-	if ( TimerOn ) {
-		del_timer ( &PollTimer );
-		TimerOn = 0;
-	}
+	del_timer_sync(&PollTimer);
 
 	/* Reset the boards we have. */
 	for( i = 0; i < IP2_MAX_BOARDS; ++i ) {
@@ -746,10 +742,8 @@ ip2_loadmain(int *iop, int *irqp)
 			}
 			if ( ip2config.irq[i] == CIR_POLL ) {
 retry:
-				if (!TimerOn) {
-					PollTimer.expires = POLL_TIMEOUT;
-					add_timer ( &PollTimer );
-					TimerOn = 1;
+				if (!timer_pending(&PollTimer)) {
+					mod_timer(&PollTimer, POLL_TIMEOUT);
 					printk( KERN_INFO "IP2: polling\n");
 				}
 			} else {
@@ -1250,16 +1244,12 @@ ip2_poll(unsigned long arg)
 {
 	ip2trace (ITRC_NO_PORT, ITRC_INTR, 100, 0 );
 
-	TimerOn = 0; // it's the truth but not checked in service
-
 	// Just polled boards, IRQ = 0 will hit all non-interrupt boards.
 	// It will NOT poll boards handled by hard interrupts.
 	// The issue of queued BH interrupts is handled in ip2_interrupt().
 	ip2_polled_interrupt();
 
-	PollTimer.expires = POLL_TIMEOUT;
-	add_timer( &PollTimer );
-	TimerOn = 1;
+	mod_timer(&PollTimer, POLL_TIMEOUT);
 
 	ip2trace (ITRC_NO_PORT, ITRC_INTR, ITRC_RETURN, 0 );
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] Char: merge ip2main and ip2base

[Alan Cox]

> I applied these four, but there's a fly in our ointment.
> 
> On 14 July, Akinobu Mita <akinobu.mita@gmail.com> sent a patch titled
> "ip2: avoid add_timer() with pending timer" which Alan acked and then
> claimed was "Queued for ttydev tree".  But afaict he then lost it.

Its queued but I can unqueue it as it isn't in the pile pending for Linus
yet.

Re: [PATCH 1/4] Char: merge ip2main and ip2base

[Andrew Morton]

On Tue, 22 Jul 2008 10:44:41 +0100 Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > I applied these four, but there's a fly in our ointment.
> > 
> > On 14 July, Akinobu Mita <akinobu.mita@gmail.com> sent a patch titled
> > "ip2: avoid add_timer() with pending timer" which Alan acked and then
> > claimed was "Queued for ttydev tree".  But afaict he then lost it.
> 
> Its queued but I can unqueue it as it isn't in the pile pending for Linus
> yet.

You've had it queued longer than I've had Jiri's sent-earlier patches
queued :(

I guess if Jiri can redo that patch for us then I can slip all five into
2.6.27-rc1?

Re: [PATCH 1/4] Char: merge ip2main and ip2base

[Alan Cox]

On Tue, 22 Jul 2008 03:06:55 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 22 Jul 2008 10:44:41 +0100 Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> 
> > > I applied these four, but there's a fly in our ointment.
> > > 
> > > On 14 July, Akinobu Mita <akinobu.mita@gmail.com> sent a patch titled
> > > "ip2: avoid add_timer() with pending timer" which Alan acked and then
> > > claimed was "Queued for ttydev tree".  But afaict he then lost it.
> > 
> > Its queued but I can unqueue it as it isn't in the pile pending for Linus
> > yet.
> 
> You've had it queued longer than I've had Jiri's sent-earlier patches
> queued :(
> 
> I guess if Jiri can redo that patch for us then I can slip all five into
> 2.6.27-rc1?

Fine by me

Alan

[PATCH 3/4] Char: ip2, fix sparse warnings

[Jiri Slaby]

Unlock two grabbed locks on some paths.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
---
 drivers/char/ip2/i2lib.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/char/ip2/i2lib.c b/drivers/char/ip2/i2lib.c
index 618f5fe..1d5388c 100644
--- a/drivers/char/ip2/i2lib.c
+++ b/drivers/char/ip2/i2lib.c
@@ -643,12 +643,12 @@ i2QueueCommands(int type, i2ChanStrPtr pCh, int timeout, int nCommands,
 				// Normal Expected path - We still hold LOCK
 				break; /* from for()- Enough room: goto proceed */
 			}
-		}
-
-		ip2trace (CHANN, ITRC_QUEUE, 3, 1, totalsize );
+			ip2trace(CHANN, ITRC_QUEUE, 3, 1, totalsize);
+			write_unlock_irqrestore(lock_var_p, flags);
+		} else
+			ip2trace(CHANN, ITRC_QUEUE, 3, 1, totalsize);
 
 		// Prepare to wait for buffers to empty
-		write_unlock_irqrestore(lock_var_p, flags);
 		serviceOutgoingFifo(pB);	// Dump what we got
 
 		if (timeout == 0) {
@@ -1834,6 +1834,8 @@ i2StripFifo(i2eBordStrPtr pB)
 		default: // Neither packet? should be impossible
 			ip2trace (ITRC_NO_PORT, ITRC_SFIFO, 5, 1,
 				PTYPE_OF(pB->i2eLeadoffWord) );
+			write_unlock_irqrestore(&pB->read_fifo_spinlock,
+					bflags);
 
 			break;
 		}  // End of switch on type of packets
-- 
1.5.4.4

Re: [PATCH 3/4] Char: ip2, fix sparse warnings

[Andrew Morton]

On Fri, 28 Mar 2008 22:18:43 +0100
Jiri Slaby <jirislaby@gmail.com> wrote:

> Unlock two grabbed locks on some paths.
> 
> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
> ---
>  drivers/char/ip2/i2lib.c |   10 ++++++----
>  1 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/char/ip2/i2lib.c b/drivers/char/ip2/i2lib.c
> index 618f5fe..1d5388c 100644
> --- a/drivers/char/ip2/i2lib.c
> +++ b/drivers/char/ip2/i2lib.c
> @@ -643,12 +643,12 @@ i2QueueCommands(int type, i2ChanStrPtr pCh, int timeout, int nCommands,
>  				// Normal Expected path - We still hold LOCK
>  				break; /* from for()- Enough room: goto proceed */
>  			}
> -		}
> -
> -		ip2trace (CHANN, ITRC_QUEUE, 3, 1, totalsize );
> +			ip2trace(CHANN, ITRC_QUEUE, 3, 1, totalsize);
> +			write_unlock_irqrestore(lock_var_p, flags);
> +		} else
> +			ip2trace(CHANN, ITRC_QUEUE, 3, 1, totalsize);
>  
>  		// Prepare to wait for buffers to empty
> -		write_unlock_irqrestore(lock_var_p, flags);
>  		serviceOutgoingFifo(pB);	// Dump what we got
>  
>  		if (timeout == 0) {
> @@ -1834,6 +1834,8 @@ i2StripFifo(i2eBordStrPtr pB)
>  		default: // Neither packet? should be impossible
>  			ip2trace (ITRC_NO_PORT, ITRC_SFIFO, 5, 1,
>  				PTYPE_OF(pB->i2eLeadoffWord) );
> +			write_unlock_irqrestore(&pB->read_fifo_spinlock,
> +					bflags);
>  
>  			break;
>  		}  // End of switch on type of packets

Your tree apparently has a sensible-looking write_unlock_irqrestore(),
whereas mine has a stupid-looking WRITE_UNLOCK_IRQRESTORE().  How did this
happen?

Re: [PATCH 3/4] Char: ip2, fix sparse warnings

[Jiri Slaby]

Andrew Morton napsal(a):
> On Fri, 28 Mar 2008 22:18:43 +0100
> Jiri Slaby <jirislaby@gmail.com> wrote:
> 
>> Unlock two grabbed locks on some paths.
>>
>> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
>> ---
>>  drivers/char/ip2/i2lib.c |   10 ++++++----
>>  1 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/char/ip2/i2lib.c b/drivers/char/ip2/i2lib.c
>> index 618f5fe..1d5388c 100644
>> --- a/drivers/char/ip2/i2lib.c
>> +++ b/drivers/char/ip2/i2lib.c
[...]
>> @@ -1834,6 +1834,8 @@ i2StripFifo(i2eBordStrPtr pB)
>>  		default: // Neither packet? should be impossible
>>  			ip2trace (ITRC_NO_PORT, ITRC_SFIFO, 5, 1,
>>  				PTYPE_OF(pB->i2eLeadoffWord) );
>> +			write_unlock_irqrestore(&pB->read_fifo_spinlock,
>> +					bflags);
>>  
>>  			break;
>>  		}  // End of switch on type of packets
> 
> Your tree apparently has a sensible-looking write_unlock_irqrestore(),
> whereas mine has a stupid-looking WRITE_UNLOCK_IRQRESTORE().  How did this
> happen?

This was changed in the previous wrong patch, I'm preparing a correct one.