Greg KH wrote:


On Fri, Sep 12, 2003 at 01:31:28PM +0900, Shin-ichiro KAWASAKI wrote:

I wish pl2303.c would accept write request while the device is busy.


Patches are always welcome :)

I tried to make my wish come true on 2.4. Here's the patch. Five functions including pl2303_write() and pl2303_write_room() were copied from drivers/usb/serial/visor.c shamelessly :-)

This patch avoids the trouble I complained, and works well with my pl2303 device.
Could you examine if this patch is appropirate or not?


(What I said before was already discussed in later August in this mailing list, but I have not read those mails when I posted my complaint, sorry.)

To do this on 2.6 would be quite easy, 2.4 will be a bit tougher.

I don't understand why it's easier on 2.6. I need some explanation (or some documents to refer).

Regards,
shin

--
Hitachi, Ltd. Systems Development Lab. 405 Unit
Shin-ichiro KAWASAKI
[EMAIL PROTECTED]
Intra. Tel, 869-3455    (since Aug. 1st)
--- a/drivers/usb/serial/pl2303.c       2003-03-14 07:23:04.000000000 +0900
+++ b/drivers/usb/serial/pl2303.c       2003-09-18 18:18:17.000000000 +0900
@@ -110,6 +110,8 @@
 static void pl2303_write_bulk_callback (struct urb *urb);
 static int pl2303_write (struct usb_serial_port *port, int from_user,
                         const unsigned char *buf, int count);
+static int pl2303_write_room           (struct usb_serial_port *port);
+static int pl2303_chars_in_buffer      (struct usb_serial_port *port);
 static void pl2303_break_ctl(struct usb_serial_port *port,int break_state);
 static int pl2303_startup (struct usb_serial *serial);
 static void pl2303_shutdown (struct usb_serial *serial);
@@ -127,6 +129,8 @@
        .open =                 pl2303_open,
        .close =                pl2303_close,
        .write =                pl2303_write,
+       .write_room =           pl2303_write_room,
+       .chars_in_buffer =      pl2303_chars_in_buffer,
        .ioctl =                pl2303_ioctl,
        .break_ctl =            pl2303_break_ctl,
        .set_termios =          pl2303_set_termios,
@@ -137,6 +141,11 @@
        .shutdown =             pl2303_shutdown,
 };
 
+#define NUM_URBS                       24
+#define URB_TRANSFER_BUFFER_SIZE       768
+static struct urb      *write_urb_pool[NUM_URBS];
+static spinlock_t      write_urb_pool_lock;
+
 struct pl2303_private { 
        u8 line_control;
        u8 termios_initialized;
@@ -171,36 +180,120 @@
 
 static int pl2303_write (struct usb_serial_port *port, int from_user,  const unsigned 
char *buf, int count)
 {
-       int result;
+       struct usb_serial *serial = port->serial;
+       struct urb *urb;
+       const unsigned char *current_position = buf;
+       unsigned long flags;
+       int status;
+       int i;
+       int bytes_sent = 0;
+       int transfer_size;
 
-       dbg("%s - port %d, %d bytes", __FUNCTION__, port->number, count);
+       dbg("%s - port %d", __FUNCTION__, port->number);
 
-       if (port->write_urb->status == -EINPROGRESS) {
-               dbg("%s - already writing", __FUNCTION__);
-               return 0;
+       while (count > 0) {
+               /* try to find a free urb in our list of them */
+               urb = NULL;
+               spin_lock_irqsave (&write_urb_pool_lock, flags);
+               for (i = 0; i < NUM_URBS; ++i) {
+                       if (write_urb_pool[i]->status != -EINPROGRESS) {
+                               urb = write_urb_pool[i];
+                               break;
+                       }
+               }
+               spin_unlock_irqrestore (&write_urb_pool_lock, flags);
+               if (urb == NULL) {
+                       dbg("%s - no more free urbs", __FUNCTION__);
+                       goto exit;
+               }
+               if (urb->transfer_buffer == NULL) {
+                       urb->transfer_buffer = kmalloc (URB_TRANSFER_BUFFER_SIZE, 
GFP_KERNEL);
+                       if (urb->transfer_buffer == NULL) {
+                               err("%s no more kernel memory...", __FUNCTION__);
+                               goto exit;
+                       }
        }
 
-       count = (count > port->bulk_out_size) ? port->bulk_out_size : count;
+               transfer_size = min (count, URB_TRANSFER_BUFFER_SIZE);
        if (from_user) {
-               if (copy_from_user (port->write_urb->transfer_buffer, buf, count))
-                       return -EFAULT;
+                       if (copy_from_user (urb->transfer_buffer, current_position, 
transfer_size)) {
+                               bytes_sent = -EFAULT;
+                               break;
+                       }
        } else {
-               memcpy (port->write_urb->transfer_buffer, buf, count);
+                       memcpy (urb->transfer_buffer, current_position, transfer_size);
        }
        
-       usb_serial_debug_data (__FILE__, __FUNCTION__, count, 
port->write_urb->transfer_buffer);
+               usb_serial_debug_data (__FILE__, __FUNCTION__, transfer_size, 
urb->transfer_buffer);
 
-       port->write_urb->transfer_buffer_length = count;
-       port->write_urb->dev = port->serial->dev;
-       result = usb_submit_urb (port->write_urb);
-       if (result)
-               err("%s - failed submitting write urb, error %d", __FUNCTION__, 
result);
-       else
-               result = count;
+               /* build up our urb */
+               FILL_BULK_URB (urb, serial->dev, usb_sndbulkpipe(serial->dev, 
port->bulk_out_endpointAddress), 
+                               urb->transfer_buffer, transfer_size, 
pl2303_write_bulk_callback, port);
+               urb->transfer_flags |= USB_QUEUE_BULK;
+
+               /* send it down the pipe */
+               status = usb_submit_urb(urb);
+               if (status) {
+                       err("%s - usb_submit_urb(write bulk) failed with status = %d", 
__FUNCTION__, status);
+                       bytes_sent = status;
+                       break;
+               }
 
-       return result;
+               current_position += transfer_size;
+               bytes_sent += transfer_size;
+               count -= transfer_size;
+               /*bytes_out += transfer_size;*/
+       }
+
+exit:
+       return bytes_sent;
+#endif
 }
 
+static int pl2303_write_room (struct usb_serial_port *port)
+{
+       unsigned long flags;
+       int i;
+       int room = 0;
+
+       dbg("%s - port %d", __FUNCTION__, port->number);
+       
+       spin_lock_irqsave (&write_urb_pool_lock, flags);
+
+       for (i = 0; i < NUM_URBS; ++i) {
+               if (write_urb_pool[i]->status != -EINPROGRESS) {
+                       room += URB_TRANSFER_BUFFER_SIZE;
+               }
+       }
+       
+       spin_unlock_irqrestore (&write_urb_pool_lock, flags);
+       
+       dbg("%s - returns %d", __FUNCTION__, room);
+       return (room);
+}
+
+
+static int pl2303_chars_in_buffer (struct usb_serial_port *port)
+{
+       unsigned long flags;
+       int i;
+       int chars = 0;
+
+       dbg("%s - port %d", __FUNCTION__, port->number);
+       
+       spin_lock_irqsave (&write_urb_pool_lock, flags);
+
+       for (i = 0; i < NUM_URBS; ++i) {
+               if (write_urb_pool[i]->status == -EINPROGRESS) {
+                       chars += URB_TRANSFER_BUFFER_SIZE;
+               }
+       }
+       
+       spin_unlock_irqrestore (&write_urb_pool_lock, flags);
+
+       dbg("%s - returns %d", __FUNCTION__, chars);
+       return (chars);
+}
 
 
 static void pl2303_set_termios (struct usb_serial_port *port, struct termios 
*old_termios)
@@ -673,7 +766,6 @@
 static void pl2303_write_bulk_callback (struct urb *urb)
 {
        struct usb_serial_port *port = (struct usb_serial_port *) urb->context;
-       int result;
 
        if (port_paranoia_check (port, __FUNCTION__))
                return;
@@ -681,18 +773,7 @@
        dbg("%s - port %d", __FUNCTION__, port->number);
        
        if (urb->status) {
-               /* error in the urb, so we have to resubmit it */
-               if (serial_paranoia_check (port->serial, __FUNCTION__)) {
-                       return;
-               }
-               dbg("%s - Overflow in write", __FUNCTION__);
                dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, 
urb->status);
-               port->write_urb->transfer_buffer_length = 1;
-               port->write_urb->dev = port->serial->dev;
-               result = usb_submit_urb (port->write_urb);
-               if (result)
-                       err("%s - failed resubmitting write urb, error %d", 
__FUNCTION__, result);
-
                return;
        }
 
@@ -705,15 +786,58 @@
 
 static int __init pl2303_init (void)
 {
+       struct urb *urb;
+       int i;
+
        usb_serial_register (&pl2303_device);
+
+       /* create our write urb pool and transfer buffers */ 
+       spin_lock_init (&write_urb_pool_lock);
+       for (i = 0; i < NUM_URBS; ++i) {
+               urb = usb_alloc_urb(0);
+               write_urb_pool[i] = urb;
+               if (urb == NULL) {
+                       err("No more urbs???");
+                       continue;
+               }
+
+               urb->transfer_buffer = NULL;
+               urb->transfer_buffer = kmalloc (URB_TRANSFER_BUFFER_SIZE, GFP_KERNEL);
+               if (!urb->transfer_buffer) {
+                       err("%s - out of memory for urb buffers.", __FUNCTION__);
+                       continue;
+               }
+       }
+
        info(DRIVER_DESC " " DRIVER_VERSION);
+
        return 0;
 }
 
 
 static void __exit pl2303_exit (void)
 {
+       int i;
+       unsigned long flags;
+
        usb_serial_deregister (&pl2303_device);
+
+       spin_lock_irqsave (&write_urb_pool_lock, flags);
+
+       for (i = 0; i < NUM_URBS; ++i) {
+               if (write_urb_pool[i]) {
+                       /* FIXME - uncomment the following usb_unlink_urb call when
+                        * the host controllers get fixed to set urb->dev = NULL after
+                        * the urb is finished.  Otherwise this call oopses. */
+                       /* This comment is taken from visor.c */
+                       /* usb_unlink_urb(write_urb_pool[i]); */
+                       if (write_urb_pool[i]->transfer_buffer)
+                               kfree(write_urb_pool[i]->transfer_buffer);
+                       usb_free_urb (write_urb_pool[i]);
+               }
+       }
+
+       spin_unlock_irqrestore (&write_urb_pool_lock, flags);
 }
 
 

Reply via email to