Skip to content

Commit

Permalink
usb-serial: change referencing of port and serial structures
Browse files Browse the repository at this point in the history 
This patch (as1284) changes the referencing of the usb_serial and
usb_serial_port structures in usb-serial.c.  It's not feasible to make
the port structures keep a reference to the serial structure, because
the ports need to remain in existence when serial is released -- quite
a few of the drivers expect this.  Consequently taking a reference
to the port when the device file is open is insufficient; such a
reference would not pin serial.

To fix this, we now take a reference to serial when the device file is
opened.  The final put_device() for the ports occurs in
destroy_serial(), so that the ports will last as long as they are
needed.

The patch initializes all the port devices, including those in the
unused "fake" ports.  This makes the code more uniform because they
can all be released in the same way.  The error handling code in
usb_serial_probe() is much simplified by this approach; instead of
freeing everything by hand we can use a single usb_serial_put() call.

Also simplified is the port-release mechanism.  Instead of being two
separate routines, port_release() and port_free() can be combined into
one.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
  • Loading branch information
Alan Stern authored and Live-CD User committed Sep 19, 2009
1 parent a75b7b6 commit 41bd34d
Showing 1 changed file with 20 additions and 75 deletions.
95 changes: 20 additions & 75 deletions drivers/usb/serial/usb-serial.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@
#define DRIVER_AUTHOR "Greg Kroah-Hartman, greg@kroah.com, http://www.kroah.com/linux/"
#define DRIVER_DESC "USB Serial Driver core"

static void port_free(struct usb_serial_port *port);

/* Driver structure we register with the USB core */
static struct usb_driver usb_serial_driver = {
.name = "usbserial",
Expand Down Expand Up @@ -145,27 +143,16 @@ static void destroy_serial(struct kref *kref)

serial->type->release(serial);

for (i = 0; i < serial->num_ports; ++i) {
/* Now that nothing is using the ports, they can be freed */
for (i = 0; i < serial->num_port_pointers; ++i) {
port = serial->port[i];
if (port)
if (port) {
port->serial = NULL;
put_device(&port->dev);
}

/* If this is a "fake" port, we have to clean it up here, as it will
* not get cleaned up in port_release() as it was never registered with
* the driver core */
if (serial->num_ports < serial->num_port_pointers) {
for (i = serial->num_ports;
i < serial->num_port_pointers; ++i) {
port = serial->port[i];
if (port)
port_free(port);
}
}

usb_put_dev(serial->dev);

/* free up any memory that we allocated */
kfree(serial);
}

Expand Down Expand Up @@ -201,8 +188,6 @@ static int serial_open (struct tty_struct *tty, struct file *filp)
port = serial->port[portNumber];
if (!port || serial->disconnected)
retval = -ENODEV;
else
get_device(&port->dev);
/*
* Note: Our locking order requirement does not allow port->mutex
* to be acquired while serial->disc_mutex is held.
Expand All @@ -213,7 +198,7 @@ static int serial_open (struct tty_struct *tty, struct file *filp)

if (mutex_lock_interruptible(&port->mutex)) {
retval = -ERESTARTSYS;
goto bailout_port_put;
goto bailout_serial_put;
}

++port->port.count;
Expand Down Expand Up @@ -273,8 +258,6 @@ static int serial_open (struct tty_struct *tty, struct file *filp)
tty->driver_data = NULL;
tty_port_tty_set(&port->port, NULL);
mutex_unlock(&port->mutex);
bailout_port_put:
put_device(&port->dev);
bailout_serial_put:
usb_serial_put(serial);
return retval;
Expand Down Expand Up @@ -333,14 +316,13 @@ static void serial_do_free(struct tty_struct *tty)

serial = port->serial;
owner = serial->type->driver.owner;
put_device(&port->dev);
/* Mustn't dereference port any more */

mutex_lock(&serial->disc_mutex);
if (!serial->disconnected)
usb_autopm_put_interface(serial->interface);
mutex_unlock(&serial->disc_mutex);

usb_serial_put(serial);
/* Mustn't dereference serial any more */
module_put(owner);
}

Expand Down Expand Up @@ -581,14 +563,6 @@ static void usb_serial_port_work(struct work_struct *work)
tty_kref_put(tty);
}

static void port_release(struct device *dev)
{
struct usb_serial_port *port = to_usb_serial_port(dev);

dbg ("%s - %s", __func__, dev_name(dev));
port_free(port);
}

static void kill_traffic(struct usb_serial_port *port)
{
usb_kill_urb(port->read_urb);
Expand All @@ -608,8 +582,12 @@ static void kill_traffic(struct usb_serial_port *port)
usb_kill_urb(port->interrupt_out_urb);
}

static void port_free(struct usb_serial_port *port)
static void port_release(struct device *dev)
{
struct usb_serial_port *port = to_usb_serial_port(dev);

dbg ("%s - %s", __func__, dev_name(dev));

/*
* Stop all the traffic before cancelling the work, so that
* nobody will restart it by calling usb_serial_port_softint.
Expand Down Expand Up @@ -955,6 +933,11 @@ int usb_serial_probe(struct usb_interface *interface,
mutex_init(&port->mutex);
INIT_WORK(&port->work, usb_serial_port_work);
serial->port[i] = port;
port->dev.parent = &interface->dev;
port->dev.driver = NULL;
port->dev.bus = &usb_serial_bus_type;
port->dev.release = &port_release;
device_initialize(&port->dev);
}

/* set up the endpoint information */
Expand Down Expand Up @@ -1097,15 +1080,10 @@ int usb_serial_probe(struct usb_interface *interface,
/* register all of the individual ports with the driver core */
for (i = 0; i < num_ports; ++i) {
port = serial->port[i];
port->dev.parent = &interface->dev;
port->dev.driver = NULL;
port->dev.bus = &usb_serial_bus_type;
port->dev.release = &port_release;

dev_set_name(&port->dev, "ttyUSB%d", port->number);
dbg ("%s - registering %s", __func__, dev_name(&port->dev));
port->dev_state = PORT_REGISTERING;
retval = device_register(&port->dev);
retval = device_add(&port->dev);
if (retval) {
dev_err(&port->dev, "Error registering port device, "
"continuing\n");
Expand All @@ -1123,39 +1101,7 @@ int usb_serial_probe(struct usb_interface *interface,
return 0;

probe_error:
for (i = 0; i < num_bulk_in; ++i) {
port = serial->port[i];
if (!port)
continue;
usb_free_urb(port->read_urb);
kfree(port->bulk_in_buffer);
}
for (i = 0; i < num_bulk_out; ++i) {
port = serial->port[i];
if (!port)
continue;
usb_free_urb(port->write_urb);
kfree(port->bulk_out_buffer);
}
for (i = 0; i < num_interrupt_in; ++i) {
port = serial->port[i];
if (!port)
continue;
usb_free_urb(port->interrupt_in_urb);
kfree(port->interrupt_in_buffer);
}
for (i = 0; i < num_interrupt_out; ++i) {
port = serial->port[i];
if (!port)
continue;
usb_free_urb(port->interrupt_out_urb);
kfree(port->interrupt_out_buffer);
}

/* free up any memory that we allocated */
for (i = 0; i < serial->num_port_pointers; ++i)
kfree(serial->port[i]);
kfree(serial);
usb_serial_put(serial);
return -EIO;
}
EXPORT_SYMBOL_GPL(usb_serial_probe);
Expand Down Expand Up @@ -1206,8 +1152,7 @@ void usb_serial_disconnect(struct usb_interface *interface)
}
serial->type->disconnect(serial);

/* let the last holder of this object
* cause it to be cleaned up */
/* let the last holder of this object cause it to be cleaned up */
usb_serial_put(serial);
dev_info(dev, "device disconnected\n");
}
Expand Down

0 comments on commit 41bd34d

Please sign in to comment.