lkml.org 
[lkml]   [2006]   [May]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: usb-serial ipaq kernel problem
On Tue, 30 May 2006 10:21:41 +0200
Frank Gevaerts <frank.gevaerts@fks.be> wrote:

| On Mon, May 29, 2006 at 07:33:30PM -0300, Luiz Fernando N. Capitulino wrote:
| > On Mon, 29 May 2006 22:47:24 +0200
| > Frank Gevaerts <frank.gevaerts@fks.be> wrote:
| > |
| > | The panic was caused by the read urb being submitten in ipaq_open,
| > | regardless of success, and never killed in case of failure. What my
| > | patch basically does is to submit the urb only after succesfully sending
| > | the control message, and adding a sleep between tries. As long as this
| > | patch is not applied, we hardly get any other error because the kernel
| > | panics as soon as an ipaq reboots.
| >
| > I see.
| >
| > Did you try to just kill the read urb in the ipaq_open's error path?
|
| Yes, that's what I did at first. It works, but with the long waits (we see
| waits up to 80-90 seconds right now) I was afraid that the urb might timeout
| before the control message succeeds.

Hmmm, I see.

My only (obvious) question is that if it's really safe to send the read
urb after the control message..

Greg, are you following this thread? WDT?

Anyways, if it's safe to do it, I do prefer the following (not tested)
version. Which is cleaner, IMO.

------------
diff --git a/drivers/usb/serial/ipaq.c b/drivers/usb/serial/ipaq.c
index 9a5c979..9333169 100644
--- a/drivers/usb/serial/ipaq.c
+++ b/drivers/usb/serial/ipaq.c
@@ -646,17 +646,6 @@ static int ipaq_open(struct usb_serial_p
port->write_urb->transfer_buffer = port->bulk_out_buffer;
port->read_urb->transfer_buffer_length = URBDATA_SIZE;
port->bulk_out_size = port->write_urb->transfer_buffer_length = URBDATA_SIZE;
-
- /* Start reading from the device */
- usb_fill_bulk_urb(port->read_urb, serial->dev,
- usb_rcvbulkpipe(serial->dev, port->bulk_in_endpointAddress),
- port->read_urb->transfer_buffer, port->read_urb->transfer_buffer_length,
- ipaq_read_bulk_callback, port);
- result = usb_submit_urb(port->read_urb, GFP_KERNEL);
- if (result) {
- err("%s - failed submitting read urb, error %d", __FUNCTION__, result);
- goto error;
- }

/*
* Send out control message observed in win98 sniffs. Not sure what
@@ -665,17 +654,31 @@ static int ipaq_open(struct usb_serial_p
* through. Since this has a reasonably high failure rate, we retry
* several times.
*/
-
while (retries--) {
result = usb_control_msg(serial->dev,
usb_sndctrlpipe(serial->dev, 0), 0x22, 0x21,
0x1, 0, NULL, 0, 100);
- if (result == 0) {
- return 0;
- }
+ if (!result)
+ break;
+ }
+ if (result) {
+ err("%s - failed doing control urb, error %d", __FUNCTION__,
+ result);
+ goto error;
}
- err("%s - failed doing control urb, error %d", __FUNCTION__, result);
- goto error;
+
+ /* Start reading from the device */
+ usb_fill_bulk_urb(port->read_urb, serial->dev,
+ usb_rcvbulkpipe(serial->dev, port->bulk_in_endpointAddress),
+ port->read_urb->transfer_buffer, port->read_urb->transfer_buffer_length,
+ ipaq_read_bulk_callback, port);
+ result = usb_submit_urb(port->read_urb, GFP_KERNEL);
+ if (result) {
+ err("%s - failed submitting read urb, error %d", __FUNCTION__, result);
+ goto error;
+ }
+
+ return 0;

enomem:
result = -ENOMEM;

--
Luiz Fernando N. Capitulino
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-05-30 16:41    [from the cache]
©2003-2011 Jasper Spaans