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