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
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
