RHL9/FC1 USB serial helper (90442 et. al.)
Pete Zaitcev
zaitcev at redhat.com
Tue Dec 16 21:50:03 UTC 2003
This comes from RHL9, but is applicable to Fedora.
The patch is extensive and solves actual oopses, expirienced
by many users (look at the list of dups for 90442). It did not
come out as I would envision it, but overhauling whole usbserial
with all component drivers is just no feasible at this time.
It pains to ask, but I would like you to take it.
-- Pete
diff -ur linux-2.4.20-20.7.5/drivers/usb/serial/usbserial.c linux-2.4.20-20.7.5-u1/drivers/usb/serial/usbserial.c
--- linux-2.4.20-20.7.5/drivers/usb/serial/usbserial.c 2003-11-17 22:30:34.000000000 -0500
+++ linux-2.4.20-20.7.5-u1/drivers/usb/serial/usbserial.c 2003-11-17 22:33:16.000000000 -0500
@@ -297,6 +297,7 @@
#include <linux/spinlock.h>
#include <linux/list.h>
#include <linux/smp_lock.h>
+#include <linux/spinlock.h>
#include <linux/usb.h>
#ifdef CONFIG_USB_SERIAL_DEBUG
@@ -347,10 +348,24 @@
};
#endif
+/*
+ * The post kludge structures and variables.
+ */
+#define POST_BSIZE 100 /* little below 128 in total */
+struct usb_serial_post_job {
+ struct list_head link;
+ struct usb_serial_port *port;
+ int len;
+ char buff[POST_BSIZE];
+};
+static spinlock_t post_lock = SPIN_LOCK_UNLOCKED; /* Also covers ->ref */
+static struct list_head post_list = LIST_HEAD_INIT(post_list);
+static struct tq_struct post_task;
/* local function prototypes */
static int serial_open (struct tty_struct *tty, struct file * filp);
static void serial_close (struct tty_struct *tty, struct file * filp);
+static int __serial_write (struct usb_serial_port *port, int from_user, const unsigned char *buf, int count);
static int serial_write (struct tty_struct * tty, int from_user, const unsigned char *buf, int count);
static int serial_write_room (struct tty_struct *tty);
static int serial_chars_in_buffer (struct tty_struct *tty);
@@ -448,6 +463,53 @@
return;
}
+/*
+ * The post kludge.
+ *
+ * Our component drivers are hideously buggy and written by people
+ * who have difficulty understanding the concept of spinlocks.
+ * There were so many races and lockups that Greg K-H made a watershed
+ * decision to provide what is essentially a single-threaded sandbox
+ * for component drivers, protected by a semaphore. It helped a lot, but
+ * for one little problem: when tty->low_latency is set, line disciplines
+ * can call ->write from an interrupt, where the semaphore oopses.
+ *
+ * Rather than open the whole can of worms again, we just post writes
+ * into a helper which can sleep.
+ *
+ * Kernel 2.6 has a proper fix, reportedly.
+ *
+ * XXX Nothing prevents this from looping forever.
+ */
+static void post_helper(void *arg)
+{
+ struct usb_serial_post_job *job;
+ struct usb_serial_port *port;
+ struct usb_serial *serial;
+ unsigned int flags;
+
+ spin_lock_irqsave(&post_lock, flags);
+ while (!list_empty(&post_list)) {
+ job = list_entry(post_list.next, struct usb_serial_post_job, link);
+ list_del(&job->link);
+ spin_unlock_irqrestore(&post_lock, flags);
+
+ port = job->port;
+ serial = get_usb_serial (port, __FUNCTION__);
+
+ down(&port->sem);
+ if (port->tty != NULL)
+ __serial_write(port, 0, job->buff, job->len);
+ up(&port->sem);
+
+ kfree(job);
+ spin_lock_irqsave(&post_lock, flags);
+ if (--serial->ref == 0)
+ kfree(serial);
+ }
+ spin_unlock_irqrestore(&post_lock, flags);
+}
+
#ifdef USES_EZUSB_FUNCTIONS
/* EZ-USB Control and Status Register. Bit 0 controls 8051 reset */
#define CPUCS_REG 0x7F92
@@ -576,23 +638,21 @@
/* if disconnect beat us to the punch here, there's nothing to do */
if (tty->driver_data) {
+ /* post_helper(NULL); */ /* Correct, but unimportant for echo.*/
__serial_close(port, filp);
}
up (&port->sem);
}
-static int serial_write (struct tty_struct * tty, int from_user, const unsigned char *buf, int count)
+static int __serial_write (struct usb_serial_port *port, int from_user, const unsigned char *buf, int count)
{
- struct usb_serial_port *port = (struct usb_serial_port *) tty->driver_data;
struct usb_serial *serial = get_usb_serial (port, __FUNCTION__);
int retval = -EINVAL;
if (!serial)
return -ENODEV;
- down (&port->sem);
-
dbg("%s - port %d, %d byte(s)", __FUNCTION__, port->number, count);
if (!port->open_count) {
@@ -607,10 +667,68 @@
retval = generic_write(port, from_user, buf, count);
exit:
- up (&port->sem);
return retval;
}
+static int serial_write (struct tty_struct * tty, int from_user, const unsigned char *buf, int count)
+{
+ struct usb_serial_port *port = (struct usb_serial_port *) tty->driver_data;
+ struct usb_serial *serial = get_usb_serial (port, __FUNCTION__);
+ struct usb_serial_post_job *job;
+ unsigned long flags;
+ int rc;
+
+ if (!in_interrupt()) {
+ /*
+ * Run post_list to reduce a possiblity of reordered writes.
+ * Tasks can make keventd to sleep, sometimes for a long time.
+ */
+ post_helper(NULL);
+
+ down(&port->sem);
+ rc = __serial_write(port, from_user, buf, count);
+ up(&port->sem);
+ return rc;
+ }
+
+ if (from_user) {
+ /*
+ * This is a BUG-able offense because we cannot
+ * pagefault while in_interrupt, but we want to see
+ * something in dmesg rather than just blinking LEDs.
+ */
+ err("user data in interrupt write");
+ return -EINVAL;
+ }
+
+ job = kmalloc(sizeof(struct usb_serial_post_job), GFP_ATOMIC);
+ if (job == NULL)
+ return -ENOMEM;
+
+ job->port = port;
+ if ((job->len = count) >= POST_BSIZE) {
+ static int rate = 0;
+ /*
+ * Data loss due to extreme circumstances.
+ * It's a ususal thing on serial to lose characters, isn't it?
+ * Neener, neener! Actually, it's probably an echo loop anyway.
+ * Only happens when getty starts talking to Visor.
+ */
+ if (++rate % 1000 < 5)
+ err("too much data (%d)", count);
+ job->len = POST_BSIZE;
+ }
+ memcpy(job->buff, buf, job->len);
+
+ spin_lock_irqsave(&post_lock, flags);
+ list_add_tail(&job->link, &post_list);
+ serial->ref++; /* Protect the port->sem from kfree() */
+ schedule_task(&post_task);
+ spin_unlock_irqrestore(&post_lock, flags);
+
+ return count;
+}
+
static int serial_write_room (struct tty_struct *tty)
{
struct usb_serial_port *port = (struct usb_serial_port *) tty->driver_data;
@@ -620,6 +738,9 @@
if (!serial)
return -ENODEV;
+ if (in_interrupt())
+ return POST_BSIZE;
+
down (&port->sem);
dbg("%s - port %d", __FUNCTION__, port->number);
@@ -1128,6 +1249,7 @@
int num_ports;
int max_endpoints;
const struct usb_device_id *id_pattern = NULL;
+ unsigned long flags;
/* loop through our list of known serial converters, and see if this
device matches. */
@@ -1338,11 +1460,15 @@
init_MUTEX (&port->sem);
}
+ spin_lock_irqsave(&post_lock, flags);
+ serial->ref = 1;
+ spin_unlock_irqrestore(&post_lock, flags);
+
/* if this device type has a startup function, call it */
if (type->startup) {
i = type->startup (serial);
if (i < 0)
- goto probe_error;
+ goto startup_error;
if (i > 0)
return serial;
}
@@ -1357,6 +1483,12 @@
return serial; /* success */
+startup_error:
+ spin_lock_irqsave(&post_lock, flags);
+ if (serial->ref != 1) {
+ err("bug in component startup: ref %d\n", serial->ref);
+ }
+ spin_unlock_irqrestore(&post_lock, flags);
probe_error:
for (i = 0; i < num_bulk_in; ++i) {
port = &serial->port[i];
@@ -1392,6 +1524,7 @@
{
struct usb_serial *serial = (struct usb_serial *) ptr;
struct usb_serial_port *port;
+ unsigned long flags;
int i;
dbg ("%s", __FUNCTION__);
@@ -1452,7 +1585,10 @@
return_serial (serial);
/* free up any memory that we allocated */
- kfree (serial);
+ spin_lock_irqsave(&post_lock, flags);
+ if (--serial->ref == 0)
+ kfree(serial);
+ spin_unlock_irqrestore(&post_lock, flags);
} else {
info("device disconnected");
@@ -1504,6 +1640,7 @@
for (i = 0; i < SERIAL_TTY_MINORS; ++i) {
serial_table[i] = NULL;
}
+ post_task.routine = post_helper;
/* register the tty driver */
serial_tty_driver.init_termios = tty_std_termios;
diff -ur linux-2.4.20-20.7.5/drivers/usb/serial/usb-serial.h linux-2.4.20-20.7.5-u1/drivers/usb/serial/usb-serial.h
--- linux-2.4.20-20.7.5/drivers/usb/serial/usb-serial.h 2002-11-28 18:53:15.000000000 -0500
+++ linux-2.4.20-20.7.5-u1/drivers/usb/serial/usb-serial.h 2003-11-17 22:32:55.000000000 -0500
@@ -151,6 +151,9 @@
__u16 product;
struct usb_serial_port port[MAX_NUM_PORTS];
void * private;
+#ifndef __GENKSYMS__
+ int ref;
+#endif
};
More information about the fedora-devel-list
mailing list