Re: [linux-usb-devel] [PATCH 3/7] USB: add direction bit to urb-transfer_flags

2007-07-31 Thread Alan Stern
On Mon, 30 Jul 2007, Pete Zaitcev wrote:

 On Mon, 30 Jul 2007 17:06:16 -0400 (EDT), Alan Stern [EMAIL PROTECTED] 
 wrote:
 
  --- usb-2.6.orig/drivers/usb/core/urb.c
  +++ usb-2.6/drivers/usb/core/urb.c
  @@ -309,7 +309,21 @@ int usb_submit_urb(struct urb *urb, gfp_
  xfertype = usb_endpoint_type(ep-desc);
  -   is_out = usb_pipeout(urb-pipe);
  +   if (xfertype == USB_ENDPOINT_XFER_CONTROL) {
  +   struct usb_ctrlrequest *setup =
  +   (struct usb_ctrlrequest *) urb-setup_packet;
  +
  +   if (!setup)
  +   return -ENOEXEC;
 
 I welcome this. I should be able to rip some code out of usbmon now.
 
 The comment in usb.h says:
  *  (Note
  * that transfer_buffer and setup_packet must still be set because not all
  * host controllers use DMA, nor do virtual root hubs).
 
 But in case of data, this happens when a driver attempts to set up DMA
 with a highmem page without doing a kmap first. I think we never enforced
 the above comment.

It is enforced only for host controllers that use PIO, and only in
the most rudimentary way -- the driver crashes if the field isn't set!

 And I think that I managed to oops usbmon by dereferencing
 garbage in transfer_buffer before but I may not remember right.
 But the setup packet should be mapped to be used, right? There's no
 good reason for setup_packet not to be mapped, right?

Not that I can think of.  Nobody ever sends setup packet contents 
through scatter-gather buffers.

Alan Stern

P.S.: I appreciate the critical review!


-
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now   http://get.splunk.com/
___
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel


Re: [linux-usb-devel] [PATCH 3/7] USB: add direction bit to urb-transfer_flags

2007-07-31 Thread Alan Stern
On Mon, 30 Jul 2007, David Brownell wrote:

 On Monday 30 July 2007, Alan Stern wrote:
  +static inline int usb_urb_dir_in(struct urb *urb)
  +{
  +   return (urb-transfer_flags  URB_DIR_MASK) != URB_DIR_OUT;
  +}
 
 Clearer would be:  == URB_DIR_IN ... or does that generate bad code?

I didn't actually check the generated code before submitting the patch.  
A quick test with gcc 4.1.2 on x86 shows that the two sources give rise
to identical objects.  I guess the same will probably be true on other
architectures too.

Okay, I'll change it.

Alan Stern


-
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now   http://get.splunk.com/
___
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel


Re: [linux-usb-devel] [PATCH 3/7] USB: add direction bit to urb-transfer_flags

2007-07-30 Thread Pete Zaitcev
On Mon, 30 Jul 2007 17:06:16 -0400 (EDT), Alan Stern [EMAIL PROTECTED] wrote:

 --- usb-2.6.orig/drivers/usb/core/urb.c
 +++ usb-2.6/drivers/usb/core/urb.c
 @@ -309,7 +309,21 @@ int usb_submit_urb(struct urb *urb, gfp_
   xfertype = usb_endpoint_type(ep-desc);
 - is_out = usb_pipeout(urb-pipe);
 + if (xfertype == USB_ENDPOINT_XFER_CONTROL) {
 + struct usb_ctrlrequest *setup =
 + (struct usb_ctrlrequest *) urb-setup_packet;
 +
 + if (!setup)
 + return -ENOEXEC;

I welcome this. I should be able to rip some code out of usbmon now.

The comment in usb.h says:
 *  (Note
 * that transfer_buffer and setup_packet must still be set because not all
 * host controllers use DMA, nor do virtual root hubs).

But in case of data, this happens when a driver attempts to set up DMA
with a highmem page without doing a kmap first. I think we never enforced
the above comment. And I think that I managed to oops usbmon by dereferencing
garbage in transfer_buffer before but I may not remember right.
But the setup packet should be mapped to be used, right? There's no
good reason for setup_packet not to be mapped, right?

-- Pete

-
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now   http://get.splunk.com/
___
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel


Re: [linux-usb-devel] [PATCH 3/7] USB: add direction bit to urb-transfer_flags

2007-07-30 Thread David Brownell
On Monday 30 July 2007, Alan Stern wrote:
 +static inline int usb_urb_dir_in(struct urb *urb)
 +{
 +   return (urb-transfer_flags  URB_DIR_MASK) != URB_DIR_OUT;
 +}

Clearer would be:  == URB_DIR_IN ... or does that generate bad code?


-
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now   http://get.splunk.com/
___
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel