This is taken by Greg and is "obviously correct", and it was on
my 2.4 TODO list forever, but the bug requestor is not helping
too much... Your call. Please let me know if you accept or reject
so that I can advance the bug state into MODIFIED or WONTFIX.
-- Pete
--- linux-2.4.23-rc1/drivers/usb/printer.c 2002-11-28 15:53:14.000000000 -0800
+++ linux-2.4.23-rc1-nip/drivers/usb/printer.c 2003-11-19 17:11:37.000000000 -0800
@@ -22,8 +22,11 @@
* v0.7 - fixed bulk-IN read and poll (David Paschal)
* v0.8 - add devfs support
* v0.9 - fix unplug-while-open paths
- * v0.10 - add proto_bias option (Pete Zaitcev)
- * v0.11 - add
hpoj.sourceforge.net ioctls (David Paschal)
+ * v0.10- remove sleep_on, fix error on oom (oliver(a)neukum.org)
+ * v0.11 - add proto_bias option (Pete Zaitcev)
+ * v0.12 - add
hpoj.sourceforge.net ioctls (David Paschal)
+ * v0.13 - alloc space for statusbuf (<status> not on stack);
+ * use usb_buffer_alloc() for read buf & write buf;
*/
/*
@@ -58,12 +61,12 @@
/*
* Version Information
*/
-#define DRIVER_VERSION "v0.11"
+#define DRIVER_VERSION "v0.13"
#define DRIVER_AUTHOR "Michael Gee, Pavel Machek, Vojtech Pavlik, Randy Dunlap, Pete
Zaitcev, David Paschal"
#define DRIVER_DESC "USB Printer Device Class driver"
#define USBLP_BUF_SIZE 8192
-#define DEVICE_ID_SIZE 1024
+#define USBLP_DEVICE_ID_SIZE 1024
/* ioctls: */
#define LPGETSTATUS 0x060b /* same as in drivers/char/lp.c */
@@ -115,12 +118,20 @@
#define USBLP_LAST_PROTOCOL 3
#define USBLP_MAX_PROTOCOLS (USBLP_LAST_PROTOCOL+1)
+/*
+ * some arbitrary status buffer size;
+ * need a status buffer that is allocated via kmalloc(), not on stack
+ */
+#define STATUS_BUF_SIZE 8
+
struct usblp {
struct usb_device *dev; /* USB device */
devfs_handle_t devfs; /* devfs device */
struct semaphore sem; /* locks this struct, especially "dev" */
- char *buf; /* writeurb.transfer_buffer */
- struct urb readurb, writeurb; /* The urbs */
+ char *writebuf; /* write transfer_buffer */
+ char *readbuf; /* read transfer_buffer */
+ char *statusbuf; /* status transfer_buffer */
+ struct urb *readurb, *writeurb; /* The urbs */
wait_queue_head_t wait; /* Zzzzz ... */
int readcount; /* Counter for reads */
int ifnum; /* Interface number */
@@ -133,8 +144,11 @@
} protocol[USBLP_MAX_PROTOCOLS];
int current_protocol;
int minor; /* minor number of device */
+ int wcomplete; /* writing is completed */
+ int rcomplete; /* reading is completed */
unsigned int quirks; /* quirks flags */
unsigned char used; /* True if open */
+ unsigned char present; /* True if not disconnected */
unsigned char bidir; /* interface is bidirectional */
unsigned char *device_id_string; /* IEEE 1284 DEVICE ID string (ptr) */
/* first 2 bytes are (big-endian) length */
@@ -146,8 +160,11 @@
dbg("usblp=0x%p", usblp);
dbg("dev=0x%p", usblp->dev);
- dbg("devfs=0x%p", usblp->devfs);
- dbg("buf=0x%p", usblp->buf);
+ dbg("present=%d", usblp->present);
+ dbg("readbuf=0x%p", usblp->readbuf);
+ dbg("writebuf=0x%p", usblp->writebuf);
+ dbg("readurb=0x%p", usblp->readurb);
+ dbg("writeurb=0x%p", usblp->writeurb);
dbg("readcount=%d", usblp->readcount);
dbg("ifnum=%d", usblp->ifnum);
for (p = USBLP_FIRST_PROTOCOL; p <= USBLP_LAST_PROTOCOL; p++) {
@@ -157,6 +174,8 @@
}
dbg("current_protocol=%d", usblp->current_protocol);
dbg("minor=%d", usblp->minor);
+ dbg("wcomplete=%d", usblp->wcomplete);
+ dbg("rcomplete=%d", usblp->rcomplete);
dbg("quirks=%d", usblp->quirks);
dbg("used=%d", usblp->used);
dbg("bidir=%d", usblp->bidir);
@@ -239,17 +258,31 @@
* URB callback.
*/
-static void usblp_bulk(struct urb *urb)
+static void usblp_bulk_read(struct urb *urb)
{
struct usblp *usblp = urb->context;
- if (!usblp || !usblp->dev || !usblp->used)
+ if (!usblp || !usblp->dev || !usblp->used || !usblp->present)
return;
- if (urb->status)
+ if (unlikely(urb->status))
warn("usblp%d: nonzero read/write bulk status received: %d",
usblp->minor, urb->status);
+ usblp->rcomplete = 1;
+ wake_up_interruptible(&usblp->wait);
+}
+
+static void usblp_bulk_write(struct urb *urb)
+{
+ struct usblp *usblp = urb->context;
+
+ if (!usblp || !usblp->dev || !usblp->used || !usblp->present)
+ return;
+ if (unlikely(urb->status))
+ warn("usblp%d: nonzero read/write bulk status received: %d",
+ usblp->minor, urb->status);
+ usblp->wcomplete = 1;
wake_up_interruptible(&usblp->wait);
}
@@ -257,25 +290,28 @@
* Get and print printer errors.
*/
-static char *usblp_messages[] = { "ok", "out of paper",
"off-line", "unknown error" };
+static char *usblp_messages[] = { "ok", "out of paper",
"off-line", "on fire" };
static int usblp_check_status(struct usblp *usblp, int err)
{
unsigned char status, newerr = 0;
int error;
- error = usblp_read_status (usblp, &status);
+ error = usblp_read_status (usblp, usblp->statusbuf);
if (error < 0) {
err("usblp%d: error %d reading printer status",
usblp->minor, error);
return 0;
}
- if (~status & LP_PERRORP) {
+ status = *usblp->statusbuf;
+
+ if (~status & LP_PERRORP)
newerr = 3;
- if (status & LP_POUTPA) newerr = 1;
- if (~status & LP_PSELECD) newerr = 2;
- }
+ if (status & LP_POUTPA)
+ newerr = 1;
+ if (~status & LP_PSELECD)
+ newerr = 2;
if (newerr != err)
info("usblp%d: %s", usblp->minor, usblp_messages[newerr]);
@@ -300,7 +336,7 @@
usblp = usblp_table[minor];
retval = -ENODEV;
- if (!usblp || !usblp->dev)
+ if (!usblp || !usblp->dev || !usblp->present)
goto out;
retval = -EBUSY;
@@ -324,13 +360,14 @@
usblp->used = 1;
file->private_data = usblp;
- usblp->writeurb.transfer_buffer_length = 0;
- usblp->writeurb.status = 0;
+ usblp->writeurb->transfer_buffer_length = 0;
+ usblp->wcomplete = 1; /* we begin writeable */
+ usblp->rcomplete = 0;
if (usblp->bidir) {
usblp->readcount = 0;
- usblp->readurb.dev = usblp->dev;
- if (usb_submit_urb(&usblp->readurb) < 0) {
+ usblp->readurb->dev = usblp->dev;
+ if (usb_submit_urb(usblp->readurb) < 0) {
retval = -EIO;
usblp->used = 0;
file->private_data = NULL;
@@ -347,16 +384,20 @@
usblp_table [usblp->minor] = NULL;
info("usblp%d: removed", usblp->minor);
- kfree (usblp->writeurb.transfer_buffer);
+ kfree (usblp->writebuf);
+ kfree (usblp->readbuf);
kfree (usblp->device_id_string);
+ kfree (usblp->statusbuf);
+ usb_free_urb(usblp->writeurb);
+ usb_free_urb(usblp->readurb);
kfree (usblp);
}
static void usblp_unlink_urbs(struct usblp *usblp)
{
- usb_unlink_urb(&usblp->writeurb);
+ usb_unlink_urb(usblp->writeurb);
if (usblp->bidir)
- usb_unlink_urb(&usblp->readurb);
+ usb_unlink_urb(usblp->readurb);
}
static int usblp_release(struct inode *inode, struct file *file)
@@ -366,7 +407,7 @@
down (&usblp->sem);
lock_kernel();
usblp->used = 0;
- if (usblp->dev) {
+ if (usblp->present) {
usblp_unlink_urbs(usblp);
up(&usblp->sem);
} else /* finish cleanup from disconnect */
@@ -380,21 +421,21 @@
{
struct usblp *usblp = file->private_data;
poll_wait(file, &usblp->wait, wait);
- return ((!usblp->bidir || usblp->readurb.status == -EINPROGRESS) ? 0 : POLLIN |
POLLRDNORM)
- | (usblp->writeurb.status == -EINPROGRESS ? 0 : POLLOUT | POLLWRNORM);
+ return ((!usblp->bidir || !usblp->rcomplete) ? 0 : POLLIN | POLLRDNORM)
+ | (!usblp->wcomplete ? 0 : POLLOUT | POLLWRNORM);
}
static int usblp_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned
long arg)
{
struct usblp *usblp = file->private_data;
int length, err, i;
- unsigned char lpstatus, newChannel;
+ unsigned char newChannel;
int status;
int twoints[2];
int retval = 0;
down (&usblp->sem);
- if (!usblp->dev) {
+ if (!usblp->present) {
retval = -ENODEV;
goto done;
}
@@ -534,18 +575,18 @@
break;
default:
- retval = -EINVAL;
+ retval = -ENOTTY;
}
else /* old-style ioctl value */
switch (cmd) {
case LPGETSTATUS:
- if (usblp_read_status(usblp, &lpstatus)) {
+ if (usblp_read_status(usblp, usblp->statusbuf)) {
err("usblp%d: failed reading printer status", usblp->minor);
retval = -EIO;
goto done;
}
- status = lpstatus;
+ status = *usblp->statusbuf;
if (copy_to_user ((int *)arg, &status, sizeof(int)))
retval = -EFAULT;
break;
@@ -561,41 +602,48 @@
static ssize_t usblp_write(struct file *file, const char *buffer, size_t count, loff_t
*ppos)
{
+ DECLARE_WAITQUEUE(wait, current);
struct usblp *usblp = file->private_data;
int timeout, err = 0;
size_t writecount = 0;
while (writecount < count) {
-
- // FIXME: only use urb->status inside completion
- // callbacks; this way is racey...
- if (usblp->writeurb.status == -EINPROGRESS) {
-
+ if (!usblp->wcomplete) {
+ barrier();
if (file->f_flags & O_NONBLOCK)
return -EAGAIN;
timeout = USBLP_WRITE_TIMEOUT;
- while (timeout && usblp->writeurb.status == -EINPROGRESS) {
+ add_wait_queue(&usblp->wait, &wait);
+ while ( 1==1 ) {
- if (signal_pending(current))
+ if (signal_pending(current)) {
+ remove_wait_queue(&usblp->wait, &wait);
return writecount ? writecount : -EINTR;
-
- timeout = interruptible_sleep_on_timeout(&usblp->wait, timeout);
+ }
+ set_current_state(TASK_INTERRUPTIBLE);
+ if (timeout && !usblp->wcomplete) {
+ timeout = schedule_timeout(timeout);
+ } else {
+ set_current_state(TASK_RUNNING);
+ break;
+ }
}
+ remove_wait_queue(&usblp->wait, &wait);
}
down (&usblp->sem);
- if (!usblp->dev) {
+ if (!usblp->present) {
up (&usblp->sem);
return -ENODEV;
}
- if (usblp->writeurb.status != 0) {
+ if (usblp->writeurb->status != 0) {
if (usblp->quirks & USBLP_QUIRK_BIDIR) {
- if (usblp->writeurb.status != -EINPROGRESS)
+ if (!usblp->wcomplete)
err("usblp%d: error %d writing to printer",
- usblp->minor, usblp->writeurb.status);
- err = usblp->writeurb.status;
+ usblp->minor, usblp->writeurb->status);
+ err = usblp->writeurb->status;
} else
err = usblp_check_status(usblp, err);
up (&usblp->sem);
@@ -607,25 +655,30 @@
continue;
}
- writecount += usblp->writeurb.transfer_buffer_length;
- usblp->writeurb.transfer_buffer_length = 0;
+ writecount += usblp->writeurb->transfer_buffer_length;
+ usblp->writeurb->transfer_buffer_length = 0;
if (writecount == count) {
up (&usblp->sem);
break;
}
- usblp->writeurb.transfer_buffer_length = (count - writecount) < USBLP_BUF_SIZE ?
- (count - writecount) : USBLP_BUF_SIZE;
+ usblp->writeurb->transfer_buffer_length = (count - writecount) <
USBLP_BUF_SIZE ?
+ (count - writecount) : USBLP_BUF_SIZE;
- if (copy_from_user(usblp->writeurb.transfer_buffer, buffer + writecount,
- usblp->writeurb.transfer_buffer_length)) {
+ if (copy_from_user(usblp->writeurb->transfer_buffer, buffer + writecount,
+ usblp->writeurb->transfer_buffer_length)) {
up(&usblp->sem);
return writecount ? writecount : -EFAULT;
}
- usblp->writeurb.dev = usblp->dev;
- usb_submit_urb(&usblp->writeurb);
+ usblp->writeurb->dev = usblp->dev;
+ usblp->wcomplete = 0;
+ if (usb_submit_urb(usblp->writeurb)) {
+ count = -EIO;
+ up (&usblp->sem);
+ break;
+ }
up (&usblp->sem);
}
@@ -635,63 +688,77 @@
static ssize_t usblp_read(struct file *file, char *buffer, size_t count, loff_t *ppos)
{
struct usblp *usblp = file->private_data;
+ DECLARE_WAITQUEUE(wait, current);
if (!usblp->bidir)
return -EINVAL;
down (&usblp->sem);
- if (!usblp->dev) {
+ if (!usblp->present) {
count = -ENODEV;
goto done;
}
- if (usblp->readurb.status == -EINPROGRESS) {
+ if (!usblp->rcomplete) {
+ barrier();
if (file->f_flags & O_NONBLOCK) {
count = -EAGAIN;
goto done;
}
- // FIXME: only use urb->status inside completion
- // callbacks; this way is racey...
- while (usblp->readurb.status == -EINPROGRESS) {
+ add_wait_queue(&usblp->wait, &wait);
+ while (1==1) {
if (signal_pending(current)) {
count = -EINTR;
+ remove_wait_queue(&usblp->wait, &wait);
goto done;
}
up (&usblp->sem);
- interruptible_sleep_on(&usblp->wait);
+ set_current_state(TASK_INTERRUPTIBLE);
+ if (!usblp->rcomplete) {
+ schedule();
+ } else {
+ set_current_state(TASK_RUNNING);
+ break;
+ }
down (&usblp->sem);
}
+ remove_wait_queue(&usblp->wait, &wait);
}
- if (!usblp->dev) {
+ if (!usblp->present) {
count = -ENODEV;
goto done;
}
- if (usblp->readurb.status) {
+ if (usblp->readurb->status) {
err("usblp%d: error %d reading from printer",
- usblp->minor, usblp->readurb.status);
- usblp->readurb.dev = usblp->dev;
+ usblp->minor, usblp->readurb->status);
+ usblp->readurb->dev = usblp->dev;
usblp->readcount = 0;
- usb_submit_urb(&usblp->readurb);
+ if (usb_submit_urb(usblp->readurb) < 0)
+ dbg("error submitting urb");
count = -EIO;
goto done;
}
- count = count < usblp->readurb.actual_length - usblp->readcount ?
- count : usblp->readurb.actual_length - usblp->readcount;
+ 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)) {
+ if (copy_to_user(buffer, usblp->readurb->transfer_buffer + usblp->readcount,
count)) {
count = -EFAULT;
goto done;
}
- if ((usblp->readcount += count) == usblp->readurb.actual_length) {
+ if ((usblp->readcount += count) == usblp->readurb->actual_length) {
usblp->readcount = 0;
- usblp->readurb.dev = usblp->dev;
- usb_submit_urb(&usblp->readurb);
+ usblp->readurb->dev = usblp->dev;
+ usblp->rcomplete = 0;
+ if (usb_submit_urb(usblp->readurb)) {
+ count = -EIO;
+ goto done;
+ }
}
done:
@@ -766,19 +833,42 @@
}
}
+ usblp->writeurb = usb_alloc_urb(0);
+ if (!usblp->writeurb) {
+ err("out of memory");
+ goto abort;
+ }
+ usblp->readurb = usb_alloc_urb(0);
+ if (!usblp->readurb) {
+ err("out of memory");
+ goto abort;
+ }
+
/* Malloc device ID string buffer to the largest expected length,
* since we can re-query it on an ioctl and a dynamic string
* could change in length. */
- if (!(usblp->device_id_string = kmalloc(DEVICE_ID_SIZE, GFP_KERNEL))) {
+ if (!(usblp->device_id_string = kmalloc(USBLP_DEVICE_ID_SIZE, GFP_KERNEL))) {
err("out of memory for device_id_string");
goto abort;
}
- /* Malloc write/read buffers in one chunk. We somewhat wastefully
+ usblp->writebuf = usblp->readbuf = NULL;
+ /* Malloc write & read buffers. We somewhat wastefully
* malloc both regardless of bidirectionality, because the
* alternate setting can be changed later via an ioctl. */
- if (!(usblp->buf = kmalloc(2 * USBLP_BUF_SIZE, GFP_KERNEL))) {
- err("out of memory for buf");
+ if (!(usblp->writebuf = kmalloc(USBLP_BUF_SIZE, GFP_KERNEL))) {
+ err("out of memory for write buf");
+ goto abort;
+ }
+ if (!(usblp->readbuf = kmalloc(USBLP_BUF_SIZE, GFP_KERNEL))) {
+ err("out of memory for read buf");
+ goto abort;
+ }
+
+ /* Allocate buffer for printer status */
+ usblp->statusbuf = kmalloc(STATUS_BUF_SIZE, GFP_KERNEL);
+ if (!usblp->statusbuf) {
+ err("out of memory for statusbuf");
goto abort;
}
@@ -818,17 +908,26 @@
info("usblp%d: USB %sdirectional printer dev %d "
"if %d alt %d proto %d vid 0x%4.4X pid 0x%4.4X",
- usblp->minor, usblp->bidir ? "Bi" : "Uni", dev->devnum,
ifnum,
+ usblp->minor, usblp->bidir ? "Bi" : "Uni", dev->devnum,
+ usblp->ifnum,
usblp->protocol[usblp->current_protocol].alt_setting,
usblp->current_protocol, usblp->dev->descriptor.idVendor,
usblp->dev->descriptor.idProduct);
+ usblp->present = 1;
+
return usblp;
abort:
if (usblp) {
- if (usblp->buf) kfree(usblp->buf);
- if (usblp->device_id_string) kfree(usblp->device_id_string);
+ if (usblp->writebuf)
+ kfree (usblp->writebuf);
+ if (usblp->readbuf)
+ kfree (usblp->readbuf);
+ kfree(usblp->statusbuf);
+ kfree(usblp->device_id_string);
+ usb_free_urb(usblp->writeurb);
+ usb_free_urb(usblp->readurb);
kfree(usblp);
}
return NULL;
@@ -943,19 +1042,19 @@
return r;
}
- FILL_BULK_URB(&usblp->writeurb, usblp->dev,
+ usb_fill_bulk_urb(usblp->writeurb, usblp->dev,
usb_sndbulkpipe(usblp->dev,
- usblp->protocol[protocol].epwrite->bEndpointAddress),
- usblp->buf, 0,
- usblp_bulk, usblp);
+ usblp->protocol[protocol].epwrite->bEndpointAddress),
+ usblp->writebuf, 0,
+ usblp_bulk_write, usblp);
usblp->bidir = (usblp->protocol[protocol].epread != 0);
if (usblp->bidir)
- FILL_BULK_URB(&usblp->readurb, usblp->dev,
+ usb_fill_bulk_urb(usblp->readurb, usblp->dev,
usb_rcvbulkpipe(usblp->dev,
- usblp->protocol[protocol].epread->bEndpointAddress),
- usblp->buf + USBLP_BUF_SIZE, USBLP_BUF_SIZE,
- usblp_bulk, usblp);
+ usblp->protocol[protocol].epread->bEndpointAddress),
+ usblp->readbuf, USBLP_BUF_SIZE,
+ usblp_bulk_read, usblp);
usblp->current_protocol = protocol;
dbg("usblp%d set protocol %d", usblp->minor, protocol);
@@ -969,7 +1068,7 @@
{
int err, length;
- err = usblp_get_id(usblp, 0, usblp->device_id_string, DEVICE_ID_SIZE - 1);
+ err = usblp_get_id(usblp, 0, usblp->device_id_string, USBLP_DEVICE_ID_SIZE - 1);
if (err < 0) {
dbg("usblp%d: error = %d reading IEEE-1284 Device ID string",
usblp->minor, err);
@@ -983,8 +1082,8 @@
length = (usblp->device_id_string[0] << 8) + usblp->device_id_string[1];
if (length < 2)
length = 2;
- else if (length >= DEVICE_ID_SIZE)
- length = DEVICE_ID_SIZE - 1;
+ else if (length >= USBLP_DEVICE_ID_SIZE)
+ length = USBLP_DEVICE_ID_SIZE - 1;
usblp->device_id_string[length] = '\0';
dbg("usblp%d Device ID string [len=%d]=\"%s\"",
@@ -1004,13 +1103,13 @@
down (&usblp->sem);
lock_kernel();
- usblp->dev = NULL;
+ usblp->present = 0;
usblp_unlink_urbs(usblp);
if (!usblp->used)
usblp_cleanup (usblp);
- else /* cleanup later, on close */
+ else /* cleanup later, on release */
up (&usblp->sem);
unlock_kernel();
}