Am Dienstag, 27. März 2007 19:43 schrieb Pete Zaitcev:
> On Tue, 27 Mar 2007 14:30:52 +0200, Oliver Neukum <[EMAIL PROTECTED]> wrote:
> 
> > --- a/drivers/usb/serial/option.c     2007-03-27 13:31:04.000000000 +0200
> > +++ b/drivers/usb/serial/option.c     2007-03-27 14:21:21.000000000 +0200
> > @@ -627,14 +629,23 @@
> > +     *buf = usb_buffer_alloc(serial->dev, len, GFP_KERNEL, handle);
> >               /* Fill URB using supplied data. */
> >       usb_fill_bulk_urb(urb, serial->dev,
> >                     usb_sndbulkpipe(serial->dev, endpoint) | dir,
> > -                   buf, len, callback, ctx);
> > +                   *buf, len, callback, ctx);
> > +     urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> > +     urb->transfer_dma = *handle;
> 
> Why don't you just use kmalloc here? The needless use of usb_buffer_alloc
> always irritates me. It's as if the iommu space is free and endless.

Very well, here's a version with read buffers allocated only while the device
is in use. In addition it fixes

- a small race with setting low latency
- failure to report ENOMEM if no read URB can be allocated

        Regards
                Oliver

Signed-off-by: Oliver Neukum <[EMAIL PROTECTED]>
-----

--- a/drivers/usb/serial/option.c       2007-03-28 13:45:46.000000000 +0200
+++ b/drivers/usb/serial/option.c       2007-03-28 14:21:12.000000000 +0200
@@ -226,10 +226,11 @@
 struct option_port_private {
        /* Input endpoints and buffer for this port */
        struct urb *in_urbs[N_IN_URB];
-       char in_buffer[N_IN_URB][IN_BUFLEN];
+       u8 *in_buffer[N_IN_URB];
+       dma_addr_t in_dma[N_IN_URB];
        /* Output endpoints and buffer for this port */
        struct urb *out_urbs[N_OUT_URB];
-       char out_buffer[N_OUT_URB][OUT_BUFLEN];
+       u8 *out_buffer[N_OUT_URB];
 
        /* Settings for the port */
        int rts_state;  /* Handshaking pins (outputs) */
@@ -538,8 +539,10 @@
 {
        struct option_port_private *portdata;
        struct usb_serial *serial = port->serial;
-       int i, err;
+       dma_addr_t handle;
+       int i, err, count = 0;
        struct urb *urb;
+       u8 *buf;
 
        portdata = usb_get_serial_port_data(port);
 
@@ -548,17 +551,26 @@
        /* Set some sane defaults */
        portdata->rts_state = 1;
        portdata->dtr_state = 1;
+       port->tty->low_latency = 1;
 
        /* Reset low level data toggle and start reading from endpoints */
        for (i = 0; i < N_IN_URB; i++) {
-               urb = portdata->in_urbs[i];
+               urb = portdata->in_urbs[i] = usb_alloc_urb(0, GFP_KERNEL);;
                if (! urb)
                        continue;
-               if (urb->dev != serial->dev) {
-                       dbg("%s: dev %p != %p", __FUNCTION__,
-                               urb->dev, serial->dev);
+               buf = usb_buffer_alloc(serial->dev, IN_BUFLEN, GFP_KERNEL, 
&handle);
+               if (buf == NULL) {
+                       dbg("%s: allocation of a buffer for endpoint %d 
failed.", __FUNCTION__, port->bulk_in_endpointAddress);
+                       usb_free_urb(urb);
+                       portdata->in_urbs[i] = NULL;
                        continue;
                }
+               usb_fill_bulk_urb(urb, serial->dev,
+                       usb_rcvbulkpipe(serial->dev, 
port->bulk_in_endpointAddress),
+                       buf, IN_BUFLEN, option_indat_callback , port);
+               urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+               urb->transfer_dma = portdata->in_dma[i] = handle;
+               count++;
 
                /*
                 * make sure endpoint data toggle is synchronized with the
@@ -573,6 +585,8 @@
                                urb->transfer_buffer_length);
                }
        }
+       if (!count)
+               return -ENOMEM;
 
        /* Reset low level data toggle on out endpoints */
        for (i = 0; i < N_OUT_URB; i++) {
@@ -584,7 +598,7 @@
                                usb_pipeout(urb->pipe), 0); */
        }
 
-       port->tty->low_latency = 1;
+
 
        option_send_setup(port);
 
@@ -593,7 +607,7 @@
 
 static inline void stop_urb(struct urb *urb)
 {
-       if (urb && urb->status == -EINPROGRESS)
+       if (urb)
                usb_kill_urb(urb);
 }
 
@@ -613,8 +627,12 @@
                option_send_setup(port);
 
                /* Stop reading/writing urbs */
-               for (i = 0; i < N_IN_URB; i++)
+               for (i = 0; i < N_IN_URB; i++) {
                        stop_urb(portdata->in_urbs[i]);
+                       usb_buffer_free(serial->dev, IN_BUFLEN,
+                               portdata->in_buffer[i], portdata->in_dma[i]);
+                       usb_free_urb(portdata->in_urbs[i]);
+               }
                for (i = 0; i < N_OUT_URB; i++)
                        stop_urb(portdata->out_urbs[i]);
        }
@@ -623,24 +641,31 @@
 
 /* Helper functions used by option_setup_urbs */
 static struct urb *option_setup_urb(struct usb_serial *serial, int endpoint,
-               int dir, void *ctx, char *buf, int len,
+               int dir, void *ctx, u8 **buf, int len,
                void (*callback)(struct urb *))
 {
        struct urb *urb;
+       u8 *temp_buf;
 
        if (endpoint == -1)
                return NULL;            /* endpoint not needed */
 
+       temp_buf = kmalloc(len, GFP_KERNEL);
+       if (!temp_buf)
+               return NULL;
+
        urb = usb_alloc_urb(0, GFP_KERNEL);             /* No ISO */
        if (urb == NULL) {
                dbg("%s: alloc for endpoint %d failed.", __FUNCTION__, 
endpoint);
+               kfree(temp_buf);
                return NULL;
        }
+       *buf = temp_buf;
 
                /* Fill URB using supplied data. */
        usb_fill_bulk_urb(urb, serial->dev,
                      usb_sndbulkpipe(serial->dev, endpoint) | dir,
-                     buf, len, callback, ctx);
+                     *buf, len, callback, ctx);
 
        return urb;
 }
@@ -658,18 +683,11 @@
                port = serial->port[i];
                portdata = usb_get_serial_port_data(port);
 
-       /* Do indat endpoints first */
-               for (j = 0; j < N_IN_URB; ++j) {
-                       portdata->in_urbs[j] = option_setup_urb (serial,
-                       port->bulk_in_endpointAddress, USB_DIR_IN, port,
-                       portdata->in_buffer[j], IN_BUFLEN, 
option_indat_callback);
-               }
-
                /* outdat endpoints */
                for (j = 0; j < N_OUT_URB; ++j) {
                        portdata->out_urbs[j] = option_setup_urb (serial,
                        port->bulk_out_endpointAddress, USB_DIR_OUT, port,
-                       portdata->out_buffer[j], OUT_BUFLEN, 
option_outdat_callback);
+                       &portdata->out_buffer[j], OUT_BUFLEN, 
option_outdat_callback);
                }
        }
 }

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to