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

This is a discussion on [PATCH 1/4] Char: merge ip2main and ip2base - Kernel ; 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 --- drivers/char/ip2/Makefile | 2 +- drivers/char/ip2/ip2base.c | ...

+ Reply to Thread
Results 1 to 7 of 7

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

  1. [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
    ---
    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
    -#include
    -#include
    -
    -#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@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

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

    Signed-off-by: Jiri Slaby
    ---
    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

    --
    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/

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

    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
    ---
    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

    --
    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/

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

    On Sat, 5 Jul 2008 15:28:23 +0200 Jiri Slaby 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.
    >




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

    On 14 July, Akinobu Mita 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
    To: linux-kernel@vger.kernel.org
    Cc: "Michael H. Warfield"
    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
    Cc: Michael H. Warfield
    ---
    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/

    --
    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/

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

    > I applied these four, but there's a fly in our ointment.
    >
    > On 14 July, Akinobu Mita 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@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

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

    On Tue, 22 Jul 2008 10:44:41 +0100 Alan Cox wrote:

    > > I applied these four, but there's a fly in our ointment.
    > >
    > > On 14 July, Akinobu Mita 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@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

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

    On Tue, 22 Jul 2008 03:06:55 -0700
    Andrew Morton wrote:

    > On Tue, 22 Jul 2008 10:44:41 +0100 Alan Cox wrote:
    >
    > > > I applied these four, but there's a fly in our ointment.
    > > >
    > > > On 14 July, Akinobu Mita 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@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread