Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

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

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


jirislaby at gmail

Jul 5, 2008, 6:28 AM

Post #1 of 5 (146 views)
Permalink
[PATCH 1/4] Char: merge ip2main and ip2base

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[at]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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


alan at lxorguk

Jul 22, 2008, 2:44 AM

Post #2 of 5 (109 views)
Permalink
Re: [PATCH 1/4] Char: merge ip2main and ip2base [In reply to]

> I applied these four, but there's a fly in our ointment.
>
> On 14 July, Akinobu Mita <akinobu.mita[at]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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


akpm at linux-foundation

Jul 22, 2008, 2:44 AM

Post #3 of 5 (109 views)
Permalink
Re: [PATCH 1/4] Char: merge ip2main and ip2base [In reply to]

On Sat, 5 Jul 2008 15:28:23 +0200 Jiri Slaby <jirislaby[at]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[at]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[at]gmail.com>
To: linux-kernel[at]vger.kernel.org
Cc: "Michael H. Warfield" <mhw[at]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[at]gmail.com>
Cc: Michael H. Warfield <mhw[at]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[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


akpm at linux-foundation

Jul 22, 2008, 3:06 AM

Post #4 of 5 (108 views)
Permalink
Re: [PATCH 1/4] Char: merge ip2main and ip2base [In reply to]

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

> > I applied these four, but there's a fly in our ointment.
> >
> > On 14 July, Akinobu Mita <akinobu.mita[at]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?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


alan at lxorguk

Jul 22, 2008, 3:25 AM

Post #5 of 5 (109 views)
Permalink
Re: [PATCH 1/4] Char: merge ip2main and ip2base [In reply to]

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

> On Tue, 22 Jul 2008 10:44:41 +0100 Alan Cox <alan[at]lxorguk.ukuu.org.uk> wrote:
>
> > > I applied these four, but there's a fly in our ointment.
> > >
> > > On 14 July, Akinobu Mita <akinobu.mita[at]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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact lists@gossamer-threads.com
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.