-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi Dave,
Your patch actually seems to do the job when -ETIMEDOUT is added to the
case "disconnect() waiting for us to drop semaphore?". At least for me
:-). Now I simply get:
# cat /boot/vmlinuz-2.2.12-20 > /dev/usb/lp0
cat: write error: Connection timed out
Plus a few lines in /var/log/messages:
May 19 13:00:51 anders kernel: printer.c: usblp0: nonzero read/write bulk
status received: -110
May 19 13:00:51 anders kernel: usb.c: USB disconnect on device 3
May 19 13:00:51 anders kernel: printer.c: usblp0: removed
Thanks a bunch for tracking down that bug (thereby sparing me the cost of
a parallel cable ;).
Sincerely, Anders
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.5 (SunOS)
Comment: For info see http://www.gnupg.org
iD8DBQE7BqhHWyfD6jrb5n4RAhliAKDtAVBiUAuxWiFpzuM0WSNyFxwIfACffObP
5LrUnT1trMQuuIKpim6b1nI=
=H3MG
-----END PGP SIGNATURE-----
--- linux/drivers/usb/printer.c-orig Sun May 13 14:09:35 2001
+++ linux/drivers/usb/printer.c Fri May 18 07:45:00 2001
@@ -19,6 +19,7 @@
* v0.6 - never time out
* v0.7 - fixed bulk-IN read and poll (David Paschal, [EMAIL PROTECTED])
* v0.8 - add devfs support
+ * v0.9 - fix disconnect-while-open paths
*/
/*
@@ -81,6 +82,7 @@
struct usblp {
struct usb_device *dev; /* USB device */
devfs_handle_t devfs; /* devfs device */
+ struct semaphore sem; /* locks this struct,
+especially "dev" */
struct urb readurb, writeurb; /* The urbs */
wait_queue_head_t wait; /* Zzzzz ... */
int readcount; /* Counter for reads */
@@ -237,24 +239,30 @@
return retval;
}
+static void usblp_cleanup (struct usblp *usblp)
+{
+ devfs_unregister (usblp->devfs);
+ usblp_table [usblp->minor] = NULL;
+ info ("usblp%d: removed", usblp->minor);
+
+ kfree (usblp->writeurb.transfer_buffer);
+ kfree (usblp->device_id_string);
+ kfree (usblp);
+}
+
static int usblp_release(struct inode *inode, struct file *file)
{
struct usblp *usblp = file->private_data;
+ down (&usblp->sem);
usblp->used = 0;
-
if (usblp->dev) {
if (usblp->bidir)
usb_unlink_urb(&usblp->readurb);
usb_unlink_urb(&usblp->writeurb);
- return 0;
- }
-
- devfs_unregister(usblp->devfs);
- usblp_table[usblp->minor] = NULL;
- kfree(usblp->device_id_string);
- kfree(usblp);
-
+ up(&usblp->sem);
+ } else /* finish cleanup from disconnect */
+ usblp_cleanup (usblp);
return 0;
}
@@ -272,21 +280,31 @@
struct usblp *usblp = file->private_data;
int length, err;
unsigned char status;
+ int retval = 0;
+
+ down (&usblp->sem);
+ if (!usblp->dev) {
+ retval = -ENODEV;
+ goto done;
+ }
if (_IOC_TYPE(cmd) == 'P') /* new-style ioctl number */
switch (_IOC_NR(cmd)) {
case IOCNR_GET_DEVICE_ID: /* get the DEVICE_ID string */
- if (_IOC_DIR(cmd) != _IOC_READ)
- return -EINVAL;
+ if (_IOC_DIR(cmd) != _IOC_READ) {
+ retval = -EINVAL;
+ goto done;
+ }
err = usblp_get_id(usblp, 0, usblp->device_id_string,
DEVICE_ID_SIZE - 1);
if (err < 0) {
dbg ("usblp%d: error = %d reading IEEE-1284
Device ID string",
usblp->minor, err);
usblp->device_id_string[0] =
usblp->device_id_string[1] = '\0';
- return -EIO;
+ retval = -EIO;
+ goto done;
}
length = (usblp->device_id_string[0] << 8) +
usblp->device_id_string[1]; /* big-endian */
@@ -300,13 +318,16 @@
if (length > _IOC_SIZE(cmd)) length = _IOC_SIZE(cmd);
/* truncate */
- if (copy_to_user((unsigned char *) arg,
usblp->device_id_string, (unsigned long) length))
- return -EFAULT;
+ if (copy_to_user((unsigned char *) arg,
+ usblp->device_id_string, (unsigned
+long) length)) {
+ retval = -EFAULT;
+ goto done;
+ }
break;
default:
- return -EINVAL;
+ retval = -EINVAL;
}
else /* old-style ioctl value */
switch (cmd) {
@@ -314,17 +335,20 @@
case LPGETSTATUS:
if (usblp_read_status(usblp, &status)) {
err("usblp%d: failed reading printer status",
usblp->minor);
- return -EIO;
+ retval = -EIO;
+ goto done;
}
if (copy_to_user ((unsigned char *)arg, &status, 1))
- return -EFAULT;
+ retval = -EFAULT;
break;
default:
- return -EINVAL;
+ retval = -EINVAL;
}
- return 0;
+done:
+ up (&usblp->sem);
+ return retval;
}
static ssize_t usblp_write(struct file *file, const char *buffer, size_t count,
loff_t *ppos)
@@ -332,27 +356,54 @@
struct usblp *usblp = file->private_data;
int timeout, err = 0, writecount = 0;
+ down (&usblp->sem);
+ if (!usblp->dev) {
+ count = -ENODEV;
+ goto done;
+ }
+
while (writecount < count) {
+ // FIXME: testing urb->status outside completion
+ // callbacks is racey...
if (usblp->writeurb.status == -EINPROGRESS) {
- if (file->f_flags & O_NONBLOCK)
- return -EAGAIN;
+ if (file->f_flags & O_NONBLOCK) {
+ count = -EAGAIN;
+ goto done;
+ }
timeout = USBLP_WRITE_TIMEOUT;
while (timeout && usblp->writeurb.status == -EINPROGRESS) {
- if (signal_pending(current))
- return writecount ? writecount : -EINTR;
+ if (signal_pending(current)) {
+ count = writecount ? writecount : -EINTR;
+ goto done;
+ }
timeout = interruptible_sleep_on_timeout(&usblp->wait,
timeout);
}
}
- if (!usblp->dev)
- return -ENODEV;
+ if (!usblp->dev) {
+ count = -ENODEV;
+ goto done;
+ }
+
+ switch (usblp->writeurb.status) {
+ /* no error? */
+ case 0:
+ break;
+
+ /* disconnect() waiting for us to drop semaphore? */
+ case -ETIMEDOUT:
+ case -ECONNRESET:
+ case -ECONNABORTED:
+ count = usblp->writeurb.status;
+ goto done;
- if (usblp->writeurb.status) {
+ /* some other error? presumed recoverable ... */
+ default:
if (usblp->quirks & USBLP_QUIRK_BIDIR) {
if (usblp->writeurb.status != -EINPROGRESS)
err("usblp%d: error %d writing to printer",
@@ -382,6 +432,8 @@
usb_submit_urb(&usblp->writeurb);
}
+done:
+ up (&usblp->sem);
return count;
}
@@ -392,20 +444,36 @@
if (!usblp->bidir)
return -EINVAL;
+ down (&usblp->sem);
+ if (!usblp->dev) {
+ count = -ENODEV;
+ goto done;
+ }
+
if (usblp->readurb.status == -EINPROGRESS) {
- if (file->f_flags & O_NONBLOCK)
- return -EAGAIN;
+ if (file->f_flags & O_NONBLOCK) {
+ count = -EAGAIN;
+ goto done;
+ }
+ // FIXME: testing urb->status outside completion
+ // callbacks is racey...
while (usblp->readurb.status == -EINPROGRESS) {
- if (signal_pending(current))
- return -EINTR;
+ if (signal_pending(current)) {
+ count = -EINTR;
+ goto done;
+ }
+ up (&usblp->sem);
interruptible_sleep_on(&usblp->wait);
+ down (&usblp->sem);
}
}
- if (!usblp->dev)
- return -ENODEV;
+ if (!usblp->dev) {
+ count = -ENODEV;
+ goto done;
+ }
if (usblp->readurb.status) {
err("usblp%d: error %d reading from printer",
@@ -413,14 +481,17 @@
usblp->readurb.dev = usblp->dev;
usblp->readcount = 0;
usb_submit_urb(&usblp->readurb);
- return -EIO;
+ count = -EIO;
+ goto done;
}
count = count < usblp->readurb.actual_length - usblp->readcount ?
count : usblp->readurb.actual_length - usblp->readcount;
- if (copy_to_user(buffer, usblp->readurb.transfer_buffer + usblp->readcount,
count))
- return -EFAULT;
+ if (copy_to_user(buffer, usblp->readurb.transfer_buffer + usblp->readcount,
+count)) {
+ count = -EFAULT;
+ goto done;
+ }
if ((usblp->readcount += count) == usblp->readurb.actual_length) {
usblp->readcount = 0;
@@ -428,6 +499,8 @@
usb_submit_urb(&usblp->readurb);
}
+done:
+ up (&usblp->sem);
return count;
}
@@ -530,6 +603,7 @@
return NULL;
}
memset(usblp, 0, sizeof(struct usblp));
+ init_MUTEX (&usblp->sem);
/* lookup quirks for this printer */
quirks = usblp_quirks(dev->descriptor.idVendor, dev->descriptor.idProduct);
@@ -592,14 +666,12 @@
sprintf(name, "lp%d", minor);
- /* Create with perms=664 */
+ /* if we have devfs, create with perms=660 */
usblp->devfs = devfs_register(usb_devfs_handle, name,
DEVFS_FL_DEFAULT, USB_MAJOR,
USBLP_MINOR_BASE + minor,
S_IFCHR | S_IRUSR | S_IWUSR | S_IRGRP |
S_IWGRP, &usblp_fops, NULL);
- if (usblp->devfs == NULL)
- err("usblp%d: device node registration failed", minor);
info("usblp%d: USB %sdirectional printer dev %d if %d alt %d",
minor, bidir ? "Bi" : "Uni", dev->devnum, ifnum, alts);
@@ -612,25 +684,21 @@
struct usblp *usblp = ptr;
if (!usblp || !usblp->dev) {
- err("disconnect on nonexisting interface");
- return;
+ err("bogus disconnect");
+ BUG ();
}
+ down (&usblp->sem);
usblp->dev = NULL;
usb_unlink_urb(&usblp->writeurb);
if (usblp->bidir)
usb_unlink_urb(&usblp->readurb);
- kfree(usblp->writeurb.transfer_buffer);
-
- if (usblp->used) return;
-
- kfree(usblp->device_id_string);
-
- devfs_unregister(usblp->devfs);
- usblp_table[usblp->minor] = NULL;
- kfree(usblp);
+ if (!usblp->used)
+ usblp_cleanup (usblp);
+ else /* cleanup later, on close */
+ up (&usblp->sem);
}
static struct usb_device_id usblp_ids [] = {
