Hi Dmitry, On Fri, Aug 31, 2012 at 04:10:47PM -0700, Dmitry Torokhov wrote: > On Fri, Aug 31, 2012 at 06:53:53PM -0400, Forest Bond wrote: > > Hi, > > > > On Fri, Aug 31, 2012 at 04:04:58PM -0400, Alan Stern wrote: > > > On Fri, 31 Aug 2012, Dmitry Torokhov wrote: > > > > > > > > > + /* Send a "check active" packet. The response will be read > > > > > > + * later and ignored. */ > > > > > > + ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), > > > > > > + 0, > > > > > > + USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE, > > > > > > + 0, 0, "\x0A\x01A", 0, > > > > > > > > > > You probably can't send data from the .const section (as well as off the > > > > > stack) -- they can be DMA'ed and there'll be issues with cache consistency on > > > > > non-x86 arches. You should allocate the data with kmalloc(). > > > > > Although, on the second thought, maybe I'm wrong in this case... not really > > > > > sure about sending -- receiving (to the .data section) could certainly be harmful... > > > > > > > > Hmm, do we actually send anything here? The "size" passed to > > > > usb_control_msg() is 0 so I don't think we use that data at all... > > > > > > Good point. Perhaps the 0 is a typo, in which case data does get sent > > > and the buffer must be kmalloc'ed. If the 0 is correct then the buffer > > > should be NULL, not "\x0A\x01A" (and what's the purpose of the leading > > > '0' in the second byte?). > > > > > > In addition, although the bRequestType specifies USB_DIR_OUT, the pipe > > > value is usb_rcvctrlpipe. Is this transfer meant to be IN or OUT? > > > > Thanks again to all for the review. My theory for why the previous patch worked > > in spite of its wrongness is that the device actually switches modes when it > > receives a control message with USB_TYPE_VENDOR even though the documentation > > suggests an actual diagnostic packet must be received. > > > > Does this (untested) patch look more reasonable? > > Yes, but we still need it tested, please. Great, I'll test it later this evening when I am back in front of the hardware and follow up with a properly submitted patch. Thanks again, Forest -- Forest Bond http://www.alittletooquiet.net http://www.rapidrollout.com
Attachment:
signature.asc
Description: Digital signature
