ark3116: (3rd try) Setup some basic infrastructure for new ark3116 driver.
in [Kernel]
|
Prev: [PATCH 4/4] pci: remove min_size for hotplug bridge
Next: [PATCH 1/6] fsnotify: kill FSNOTIFY_EVENT_FILE
From: Greg KH on 27 Oct 2009 14:00 On Sun, Oct 25, 2009 at 06:50:58PM +0100, bart.hartgers(a)gmail.com wrote: > Signed-off-by: Bart Hartgers <bart.hartgers(a)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(a)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(a)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. Care to respin this series based on the review so far? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 27 Oct 2009 18:40
On Tue, Oct 27, 2009 at 10:35:31PM +0100, Bart Hartgers wrote: > 2009/10/27 Greg KH <greg(a)kroah.com>: > > On Sun, Oct 25, 2009 at 06:50:58PM +0100, bart.hartgers(a)gmail.com wrote: > >> Signed-off-by: Bart Hartgers <bart.hartgers(a)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(a)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(a)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. Yes, just use a spinlock. atomic_t are bad as they can be a mess (you will thrash cachelines multiple times if you hit multiple atomic_t values, but only once for a spinlock for those same multiple values.) It's just not worth it for a driver like this, just use a u32 and a spinlock. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |