Am Donnerstag, 20. Dezember 2007 16:51:59 schrieb Alan Stern:
> On Thu, 20 Dec 2007, Oliver Neukum wrote:
>
> > @@ -1080,20 +1081,22 @@ void usb_serial_disconnect(struct usb_in
> > usb_serial_console_disconnect(serial);
> > dbg ("%s", __FUNCTION__);
> >
> > + mutex_lock(&serial->disc_mutex);
> > usb_set_intfdata (interface, NULL);
> > - if (serial) {
> > - for (i = 0; i < serial->num_ports; ++i) {
> > - port = serial->port[i];
> > - if (port) {
> > - if (port->tty)
> > - tty_hangup(port->tty);
> > - kill_traffic(port);
> > - }
> > + /* must use extra flag, as intfdata can be reset */
> > + serial->disconnected = 1;
>
> This comment is misleading. It implies there is a possibility you
> actually could check for disconnects by looking at the value of
> usb_get_intfdata(), but that's completely wrong. There are about 13
> places in various serial drivers where this mistake is made; they all
> need to be fixed.
>
> Don't you also have to check the disconnect flag in serial_close()
> before calling usb_autopm_put_interface()?
Where is that?
This fixes a problem where the mos7720 driver will make io to a device from
which it has been logically disconnected. It does so by introducing a flag by
which the generic usb serial code can signal the subdrivers their
disconnection and appropriate locking.
Signed-off-by: Oliver Neukum <[EMAIL PROTECTED]>
I'd call this 2.6.25 material.
Regards
Oliver
----
--- linux-2.6.24-rc7/include/linux/usb/serial.h 2008-01-16 15:04:38.000000000
+0100
+++ linux-2.6.24-serial_intfdata/include/linux/usb/serial.h 2008-01-16
14:55:12.000000000 +0100
@@ -128,6 +128,7 @@ struct usb_serial {
struct usb_device * dev;
struct usb_serial_driver * type;
struct usb_interface * interface;
+ unsigned char disconnected;
unsigned char minor;
unsigned char num_ports;
unsigned char num_port_pointers;
@@ -137,6 +138,7 @@ struct usb_serial {
char num_bulk_out;
struct usb_serial_port * port[MAX_NUM_PORTS];
struct kref kref;
+ struct mutex disc_mutex;
void * private;
};
#define to_usb_serial(d) container_of(d, struct usb_serial, kref)
--- linux-2.6.24-rc7/drivers/usb/serial/usb-serial.c 2008-01-16
15:04:26.000000000 +0100
+++ linux-2.6.24-serial_intfdata/drivers/usb/serial/usb-serial.c
2008-01-16 14:57:55.000000000 +0100
@@ -625,6 +625,7 @@ static struct usb_serial * create_serial
serial->type = driver;
serial->interface = interface;
kref_init(&serial->kref);
+ mutex_init(&serial->disc_mutex);
return serial;
}
@@ -1080,20 +1081,22 @@ void usb_serial_disconnect(struct usb_in
usb_serial_console_disconnect(serial);
dbg ("%s", __FUNCTION__);
+ mutex_lock(&serial->disc_mutex);
usb_set_intfdata (interface, NULL);
- if (serial) {
- for (i = 0; i < serial->num_ports; ++i) {
- port = serial->port[i];
- if (port) {
- if (port->tty)
- tty_hangup(port->tty);
- kill_traffic(port);
- }
+ /* must set a flag, to signal subdrivers */
+ serial->disconnected = 1;
+ for (i = 0; i < serial->num_ports; ++i) {
+ port = serial->port[i];
+ if (port) {
+ if (port->tty)
+ tty_hangup(port->tty);
+ kill_traffic(port);
}
- /* let the last holder of this object
- * cause it to be cleaned up */
- usb_serial_put(serial);
}
+ /* let the last holder of this object
+ * cause it to be cleaned up */
+ mutex_unlock(&serial->disc_mutex);
+ usb_serial_put(serial);
dev_info(dev, "device disconnected\n");
}
@@ -1103,9 +1106,6 @@ int usb_serial_suspend(struct usb_interf
struct usb_serial_port *port;
int i, r = 0;
- if (!serial) /* device has been disconnected */
- return 0;
-
for (i = 0; i < serial->num_ports; ++i) {
port = serial->port[i];
if (port)
--- linux-2.6.24-rc7/drivers/usb/serial/mos7720.c 2008-01-16
15:04:26.000000000 +0100
+++ linux-2.6.24-serial_intfdata/drivers/usb/serial/mos7720.c 2008-01-16
14:55:12.000000000 +0100
@@ -564,22 +564,25 @@ static void mos7720_close(struct usb_ser
}
/* While closing port, shutdown all bulk read, write *
- * and interrupt read if they exists */
- if (serial->dev) {
- dbg("Shutdown bulk write");
- usb_kill_urb(port->write_urb);
- dbg("Shutdown bulk read");
- usb_kill_urb(port->read_urb);
+ * and interrupt read if they exists, otherwise nop */
+ dbg("Shutdown bulk write");
+ usb_kill_urb(port->write_urb);
+ dbg("Shutdown bulk read");
+ usb_kill_urb(port->read_urb);
+
+ mutex_lock(&serial->disc_mutex);
+ /* these commands must not be issued if the device has
+ * been disconnected */
+ if (!serial->disconnected) {
+ data = 0x00;
+ send_mos_cmd(serial, MOS_WRITE, port->number -
port->serial->minor,
+ 0x04, &data);
+
+ data = 0x00;
+ send_mos_cmd(serial, MOS_WRITE, port->number -
port->serial->minor,
+ 0x01, &data);
}
-
- data = 0x00;
- send_mos_cmd(serial, MOS_WRITE, port->number - port->serial->minor,
- 0x04, &data);
-
- data = 0x00;
- send_mos_cmd(serial, MOS_WRITE, port->number - port->serial->minor,
- 0x01, &data);
-
+ mutex_unlock(&serial->disc_mutex);
mos7720_port->open = 0;
dbg("Leaving %s", __FUNCTION__);
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
