Hi,
this driver was long known to be unclean. This should fix all issues.
However, I have no hardware to test, so I'd appreciate any comment.
Regards
Oliver
-----
--- a/drivers/usb/serial/whiteheat.c 2007-03-20 20:34:36.000000000 +0100
+++ b/drivers/usb/serial/whiteheat.c 2007-03-22 13:05:42.000000000 +0100
@@ -74,6 +74,7 @@
#include <linux/tty_flip.h>
#include <linux/module.h>
#include <linux/spinlock.h>
+#include <linux/mutex.h>
#include <asm/uaccess.h>
#include <asm/termbits.h>
#include <linux/usb.h>
@@ -203,7 +204,8 @@
struct whiteheat_command_private {
- spinlock_t lock;
+ struct mutex mutex;
+ spinlock_t buflock;
__u8 port_running;
__u8 command_finished;
wait_queue_head_t wait_command; /* for handling sleeping while
waiting for a command to finish */
@@ -225,6 +227,7 @@
spinlock_t lock;
__u8 flags;
__u8 mcr;
+ char shutting_down;
struct list_head rx_urbs_free;
struct list_head rx_urbs_submitted;
struct list_head rx_urb_q;
@@ -495,7 +498,8 @@
goto no_command_private;
}
- spin_lock_init(&command_info->lock);
+ spin_lock_init(&command_info->buflock);
+ mutex_init(&command_info->mutex);
command_info->port_running = 0;
init_waitqueue_head(&command_info->wait_command);
usb_set_serial_port_data(command_port, command_info);
@@ -552,7 +556,6 @@
return -ENOMEM;
}
-
static void whiteheat_shutdown (struct usb_serial *serial)
{
struct usb_serial_port *command_port;
@@ -573,6 +576,11 @@
for (i = 0; i < serial->num_ports; i++) {
port = serial->port[i];
info = usb_get_serial_port_data(port);
+
+ spin_lock_irq(&info->lock);
+ info->shutting_down = 1;
+ spin_unlock_irq(&info->lock);
+
list_for_each_safe(tmp, tmp2, &info->rx_urbs_free) {
list_del(tmp);
wrap = list_entry(tmp, struct whiteheat_urb_wrap, list);
@@ -589,6 +597,7 @@
kfree(urb->transfer_buffer);
usb_free_urb(urb);
}
+ flush_scheduled_work();
kfree(info);
}
@@ -685,21 +694,35 @@
/* shutdown our bulk reads and writes */
spin_lock_irqsave(&info->lock, flags);
+ info->shutting_down = 1;
+ smp_wmb();
+ /* this prevents new urbs in the queue
+ * thus we cannot miss an urb due to dropping
+ * the lock. We might doubly kill, but that's
+ * harmless */
list_for_each_safe(tmp, tmp2, &info->rx_urbs_submitted) {
wrap = list_entry(tmp, struct whiteheat_urb_wrap, list);
urb = wrap->urb;
+ spin_unlock_irqrestore(&info->lock, flags);
usb_kill_urb(urb);
- list_move(tmp, &info->rx_urbs_free);
+ spin_lock_irqsave(&info->lock, flags);
}
+ /* must walk again - cannot assume they stayed on the list */
+ list_for_each_safe(tmp, tmp2, &info->rx_urbs_submitted)
+ list_move(tmp, &info->rx_urbs_free);
+
list_for_each_safe(tmp, tmp2, &info->rx_urb_q)
list_move(tmp, &info->rx_urbs_free);
list_for_each_safe(tmp, tmp2, &info->tx_urbs_submitted) {
wrap = list_entry(tmp, struct whiteheat_urb_wrap, list);
urb = wrap->urb;
+ spin_unlock_irqrestore(&info->lock, flags);
usb_kill_urb(urb);
- list_move(tmp, &info->tx_urbs_free);
+ spin_lock_irqsave(&info->lock, flags);
}
+ list_for_each_safe(tmp, tmp2, &info->tx_urbs_submitted)
+ list_move(tmp, &info->tx_urbs_free);
spin_unlock_irqrestore(&info->lock, flags);
stop_command_port(port->serial);
@@ -724,7 +747,7 @@
if (count == 0) {
dbg("%s - write request of 0 bytes", __FUNCTION__);
- return (0);
+ return 0;
}
while (count) {
@@ -746,24 +769,28 @@
urb->dev = serial->dev;
urb->transfer_buffer_length = bytes;
- result = usb_submit_urb(urb, GFP_ATOMIC);
- if (result) {
- err("%s - failed submitting write urb, error %d",
__FUNCTION__, result);
- sent = result;
- spin_lock_irqsave(&info->lock, flags);
+ spin_lock_irqsave(&info->lock, flags);
+ if (!info->shutting_down) {
+ result = usb_submit_urb(urb, GFP_ATOMIC);
+ if (result) {
+ err("%s - failed submitting write urb, error
%d", __FUNCTION__, result);
+ list_add(tmp, &info->tx_urbs_free);
+ spin_unlock_irqrestore(&info->lock, flags);
+ break;
+ } else {
+ sent += bytes;
+ count -= bytes;
+ list_add(tmp, &info->tx_urbs_submitted);
+ spin_unlock_irqrestore(&info->lock, flags);
+ }
+ } else {
list_add(tmp, &info->tx_urbs_free);
spin_unlock_irqrestore(&info->lock, flags);
break;
- } else {
- sent += bytes;
- count -= bytes;
- spin_lock_irqsave(&info->lock, flags);
- list_add(tmp, &info->tx_urbs_submitted);
- spin_unlock_irqrestore(&info->lock, flags);
}
}
- return sent;
+ return sent ? sent : result;
}
@@ -865,10 +892,10 @@
break;
default:
- break;
+ return -ENOIOCTLCMD;
}
- return -ENOIOCTLCMD;
+ return 0;
}
@@ -920,7 +947,7 @@
spin_unlock_irqrestore(&info->lock, flags);
dbg ("%s - returns %d", __FUNCTION__, chars);
- return (chars);
+ return chars;
}
@@ -979,7 +1006,6 @@
struct whiteheat_command_private *command_info;
unsigned char *data = urb->transfer_buffer;
int result;
- unsigned long flags;
dbg("%s", __FUNCTION__);
@@ -995,7 +1021,7 @@
dbg ("%s - command_info is NULL, exiting.", __FUNCTION__);
return;
}
- spin_lock_irqsave(&command_info->lock, flags);
+ spin_lock(&command_info->buflock);
if (data[0] == WHITEHEAT_CMD_COMPLETE) {
command_info->command_finished = WHITEHEAT_CMD_COMPLETE;
@@ -1017,7 +1043,7 @@
/* Continue trying to always read */
command_port->read_urb->dev = command_port->serial->dev;
result = usb_submit_urb(command_port->read_urb, GFP_ATOMIC);
- spin_unlock_irqrestore(&command_info->lock, flags);
+ spin_unlock(&command_info->buflock);
if (result)
dbg("%s - failed resubmitting read urb, error %d",
__FUNCTION__, result);
}
@@ -1040,19 +1066,14 @@
return;
}
list_del(&wrap->list);
- spin_unlock(&info->lock);
if (urb->status) {
dbg("%s - nonzero read bulk status received: %d", __FUNCTION__,
urb->status);
- spin_lock(&info->lock);
list_add(&wrap->list, &info->rx_urbs_free);
spin_unlock(&info->lock);
return;
}
- usb_serial_debug_data(debug, &port->dev, __FUNCTION__,
urb->actual_length, data);
-
- spin_lock(&info->lock);
list_add_tail(&wrap->list, &info->rx_urb_q);
if (info->flags & THROTTLED) {
info->flags |= ACTUALLY_THROTTLED;
@@ -1060,6 +1081,7 @@
return;
}
spin_unlock(&info->lock);
+ usb_serial_debug_data(debug, &port->dev, __FUNCTION__,
urb->actual_length, data);
schedule_work(&info->rx_work);
}
@@ -1070,10 +1092,12 @@
struct usb_serial_port *port = (struct usb_serial_port *)urb->context;
struct whiteheat_private *info = usb_get_serial_port_data(port);
struct whiteheat_urb_wrap *wrap;
+ int status;
dbg("%s - port %d", __FUNCTION__, port->number);
spin_lock(&info->lock);
+ status = urb->status;
wrap = urb_to_wrap(urb, &info->tx_urbs_submitted);
if (!wrap) {
spin_unlock(&info->lock);
@@ -1083,8 +1107,8 @@
list_move(&wrap->list, &info->tx_urbs_free);
spin_unlock(&info->lock);
- if (urb->status) {
- dbg("%s - nonzero write bulk status received: %d",
__FUNCTION__, urb->status);
+ if (status) {
+ dbg("%s - nonzero write bulk status received: %d",
__FUNCTION__, status);
return;
}
@@ -1102,13 +1126,12 @@
struct whiteheat_private *info;
__u8 *transfer_buffer;
int retval = 0;
- unsigned long flags;
dbg("%s - command %d", __FUNCTION__, command);
command_port = port->serial->port[COMMAND_PORT];
command_info = usb_get_serial_port_data(command_port);
- spin_lock_irqsave(&command_info->lock, flags);
+ mutex_lock(&command_info->mutex);
command_info->command_finished = FALSE;
transfer_buffer = (__u8 *)command_port->write_urb->transfer_buffer;
@@ -1116,29 +1139,28 @@
memcpy (&transfer_buffer[1], data, datasize);
command_port->write_urb->transfer_buffer_length = datasize + 1;
command_port->write_urb->dev = port->serial->dev;
- retval = usb_submit_urb (command_port->write_urb, GFP_KERNEL);
+ retval = usb_submit_urb(command_port->write_urb, GFP_NOIO);
if (retval) {
dbg("%s - submit urb failed", __FUNCTION__);
goto exit;
}
- spin_unlock_irqrestore(&command_info->lock, flags);
/* wait for the command to complete */
wait_event_interruptible_timeout(command_info->wait_command,
(command_info->command_finished != FALSE), COMMAND_TIMEOUT);
- spin_lock_irqsave(&command_info->lock, flags);
+ spin_lock_irq(&command_info->buflock);
if (command_info->command_finished == FALSE) {
dbg("%s - command timed out.", __FUNCTION__);
retval = -ETIMEDOUT;
- goto exit;
+ goto exit2;
}
if (command_info->command_finished == WHITEHEAT_CMD_FAILURE) {
dbg("%s - command failed.", __FUNCTION__);
retval = -EIO;
- goto exit;
+ goto exit2;
}
if (command_info->command_finished == WHITEHEAT_CMD_COMPLETE) {
@@ -1150,9 +1172,10 @@
break;
}
}
-
+exit2:
+ spin_unlock_irq(&command_info->buflock);
exit:
- spin_unlock_irqrestore(&command_info->lock, flags);
+ mutex_unlock(&command_info->mutex);
return retval;
}
@@ -1305,12 +1328,11 @@
{
struct usb_serial_port *command_port;
struct whiteheat_command_private *command_info;
- unsigned long flags;
int retval = 0;
command_port = serial->port[COMMAND_PORT];
command_info = usb_get_serial_port_data(command_port);
- spin_lock_irqsave(&command_info->lock, flags);
+ mutex_lock(&command_info->mutex);
if (!command_info->port_running) {
/* Work around HCD bugs */
usb_clear_halt(serial->dev, command_port->read_urb->pipe);
@@ -1325,7 +1347,7 @@
command_info->port_running++;
exit:
- spin_unlock_irqrestore(&command_info->lock, flags);
+ mutex_unlock(&command_info->mutex);
return retval;
}
@@ -1334,15 +1356,14 @@
{
struct usb_serial_port *command_port;
struct whiteheat_command_private *command_info;
- unsigned long flags;
command_port = serial->port[COMMAND_PORT];
command_info = usb_get_serial_port_data(command_port);
- spin_lock_irqsave(&command_info->lock, flags);
+ mutex_lock(&command_info->mutex);
command_info->port_running--;
if (!command_info->port_running)
usb_kill_urb(command_port->read_urb);
- spin_unlock_irqrestore(&command_info->lock, flags);
+ mutex_unlock(&command_info->mutex);
}
@@ -1363,15 +1384,22 @@
wrap = list_entry(tmp, struct whiteheat_urb_wrap, list);
urb = wrap->urb;
urb->dev = port->serial->dev;
- retval = usb_submit_urb(urb, GFP_KERNEL);
+ retval = usb_submit_urb(urb, GFP_ATOMIC);
if (retval) {
+ info->shutting_down = 1;
+ smp_wmb();
list_add(tmp, &info->rx_urbs_free);
list_for_each_safe(tmp, tmp2, &info->rx_urbs_submitted)
{
wrap = list_entry(tmp, struct
whiteheat_urb_wrap, list);
urb = wrap->urb;
+ spin_unlock_irqrestore(&info->lock, flags);
usb_kill_urb(urb);
- list_move(tmp, &info->rx_urbs_free);
+ spin_lock_irqsave(&info->lock, flags);
}
+ list_for_each_safe(tmp, tmp2, &info->rx_urbs_submitted)
+ list_move(tmp, &info->rx_urbs_free);
+ smp_wmb();
+ info->shutting_down = 0;
break;
}
list_add(tmp, &info->rx_urbs_submitted);
@@ -1426,7 +1454,6 @@
list_for_each_safe(tmp, tmp2, &info->rx_urb_q) {
list_del(tmp);
- spin_unlock_irqrestore(&info->lock, flags);
wrap = list_entry(tmp, struct whiteheat_urb_wrap, list);
urb = wrap->urb;
@@ -1435,9 +1462,7 @@
int len = tty_buffer_request_room(tty,
urb->actual_length);
/* This stuff can go away now I suspect */
if (unlikely(len < urb->actual_length)) {
- spin_lock_irqsave(&info->lock, flags);
list_add(tmp, &info->rx_urb_q);
- spin_unlock_irqrestore(&info->lock, flags);
tty_flip_buffer_push(tty);
schedule_work(&info->rx_work);
return;
@@ -1447,15 +1472,16 @@
}
urb->dev = port->serial->dev;
- result = usb_submit_urb(urb, GFP_ATOMIC);
+ if (!info->shutting_down)
+ result = usb_submit_urb(urb, GFP_ATOMIC);
+ else
+ result = -EBUSY;
if (result) {
err("%s - failed resubmitting read urb, error %d",
__FUNCTION__, result);
- spin_lock_irqsave(&info->lock, flags);
list_add(tmp, &info->rx_urbs_free);
continue;
}
- spin_lock_irqsave(&info->lock, flags);
list_add(tmp, &info->rx_urbs_submitted);
}
spin_unlock_irqrestore(&info->lock, flags);
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
