ChangeSet 1.1288, 2003/12/12 11:49:05-08:00, zaitcev@redhat.com [PATCH] USB: Backport of printer 2.6=>2.4 > Pete, I tested your backport + this change above on 2.4.23 and it works > well on my HP psc 2110. > I did have to apply part of the patch by hand, so I rediffed it against > 2.4.23. Patch is below. OK, either way is good. However, I don't understand why it didn't apply for you. I tried this way and that, it all matches. The patch you posted applies cleanly, too. Hmmm.... I think the attached should be perfect. I do not much like the syntax of "while ( 1==1 )", but if 2.6 has it... drivers/usb/printer.c | 285 +++++++++++++++++++++++++++++++++----------------- 1 files changed, 192 insertions(+), 93 deletions(-) diff -Nru a/drivers/usb/printer.c b/drivers/usb/printer.c --- a/drivers/usb/printer.c Fri Dec 12 15:05:46 2003 +++ b/drivers/usb/printer.c Fri Dec 12 15:05:46 2003 @@ -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@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 ( 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,24 +575,24 @@ 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; default: - retval = -EINVAL; + retval = -ENOTTY; } done: @@ -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(); }