lkml.org 
[lkml]   [2009]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 1/7] ark3116: (3rd try) Setup some basic infrastructure for new ark3116 driver.
    From
    2009/10/27 Greg KH <greg@kroah.com>:
    > On Sun, Oct 25, 2009 at 06:50:58PM +0100, bart.hartgers@gmail.com wrote:
    >> Signed-off-by: Bart Hartgers <bart.hartgers@gmail.com>
    >> ---
    >> Index: linux-2.6.32-rc4/drivers/usb/serial/ark3116.c
    >> ===================================================================
    >> --- linux-2.6.32-rc4.orig/drivers/usb/serial/ark3116.c        2009-10-18 14:25:02.000000000 +0200
    >> +++ linux-2.6.32-rc4/drivers/usb/serial/ark3116.c     2009-10-18 14:25:14.000000000 +0200
    >> @@ -1,4 +1,6 @@
    >>  /*
    >> + * Copyright (C) 2009 by Bart Hartgers (bart.hartgers+ark3116@gmail.com)
    >> + * Original version:
    >>   * Copyright (C) 2006
    >>   *   Simon Schulz (ark3116_driver <at> auctionant.de)
    >>   *
    >> @@ -6,10 +8,13 @@
    >>   * - implements a driver for the arkmicro ark3116 chipset (vendor=0x6547,
    >>   *   productid=0x0232) (used in a datacable called KQ-U8A)
    >>   *
    >> - * - based on code by krisfx -> thanks !!
    >> - *   (see http://www.linuxquestions.org/questions/showthread.php?p=2184457#post2184457)
    >> + * Supports full modem status lines, break, hardware flow control. Does not
    >> + * support software flow control, since I do not know how to enable it in hw.
    >>   *
    >> - *  - based on logs created by usbsnoopy
    >> + * This driver is a essentially new implementation. I initially dug
    >> + * into the old ark3116.c driver and suddenly realized the ark3116 is
    >> + * a 16450 with a USB interface glued to it. See comments at the
    >> + * bottom of this file.
    >>   *
    >>   * This program is free software; you can redistribute it and/or modify it
    >>   * under the terms of the GNU General Public License as published by the
    >> @@ -19,15 +24,31 @@
    >>
    >>  #include <linux/kernel.h>
    >>  #include <linux/init.h>
    >> +#include <asm/atomic.h>
    >> +#include <linux/ioctl.h>
    >>  #include <linux/tty.h>
    >> +#include <linux/tty_flip.h>
    >>  #include <linux/module.h>
    >>  #include <linux/usb.h>
    >>  #include <linux/usb/serial.h>
    >>  #include <linux/serial.h>
    >> +#include <linux/serial_reg.h>
    >>  #include <linux/uaccess.h>
    >> -
    >> +#include <linux/mutex.h>
    >>
    >>  static int debug;
    >> +/*
    >> + * Version information
    >> + */
    >> +
    >> +#define DRIVER_VERSION "v0.3"
    >> +#define DRIVER_AUTHOR "Bart Hartgers <bart.hartgers+ark3116@gmail.com>"
    >> +#define DRIVER_DESC "USB ARK3116 serial/IrDA driver"
    >> +#define DRIVER_DEV_DESC "ARK3116 RS232/IrDA"
    >> +#define DRIVER_NAME "ark3116"
    >> +
    >> +/* usb timeout of 1 second */
    >> +#define ARK_TIMEOUT (1*HZ)
    >>
    >>  static struct usb_device_id id_table [] = {
    >>       { USB_DEVICE(0x6547, 0x0232) },
    >> @@ -45,6 +66,52 @@ static int is_irda(struct usb_serial *se
    >>       return 0;
    >>  }
    >>
    >> +struct ark3116_private {
    >> +     wait_queue_head_t       delta_msr_wait;
    >> +     struct async_icount     icount;
    >> +     int                     irda;   /* 1 for irda device */
    >> +
    >> +     /* protects hw register updates */
    >> +     struct mutex            lock;
    >> +
    >> +     int                     quot;   /* baudrate divisor */
    >> +     __u8                    lcr;    /* line control register value */
    >> +     __u8                    hcr;    /* handshake control register (0x8)
    >> +                                      * value
    >> +                                      */
    >> +     /* register values - updated asynchronously */
    >> +     atomic_t                mcr;
    >> +     atomic_t                msr;
    >> +     atomic_t                lsr;
    >
    > These don't need to be atomic, please don't use them if they are not
    > needed.  Just use the lock to protect updating them if needed.
    >
    At least lsr and msr are (or will be in patch 6) updated by the
    interrupt_callback. I am happy to add another lock (for
    interrupt-context it would need a spinlock rather than a mutex,
    right?). I am just surprised that is considered cleaner than using
    atomic_t.

    For mcr I am not sure whether a race between tiocmget and tiocmset is
    possible. I didn't see any obvious locking in usb-serial.c or
    tty_io.c, that's why I used an atomic_t. Am I missing something?

    > Care to respin this series based on the review so far?
    Will do somewhere tomorrow. Mike and I are working out how to combine
    our patches.

    Groeten,
    Bart

    >
    > thanks,
    >
    > greg k-h
    >



    --
    Bart Hartgers - New e-mail: bart.hartgers@gmail.com
    --
    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/

    \
     
     \ /
      Last update: 2009-10-27 22:39    [W:0.032 / U:1.336 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site