Re: [RFC] Moving from pipes to usb_host_endpoint pointers

On Wednesday 18 July 2007, Alan Stern wrote:
> This has led to a rather strange difficulty.  As part of dequeuing an
> URB, the HCD will have to verify that the URB is linked into the
> endpoint's urb_list.  However the endpoint is accessed via a pointer in
> an array (udev->ep_in[] or udev->ep_out[]) indexed by the endpoint 
> number as stored in urb->pipe -- and when an endpoint is disabled the 
> pointer is set to NULL.  This will make it impossible for 
> usb_hcd_endpoint_disable() to do its job!  Once the endpoint has been 
> disabled, the unlink verification won't work.

I'd expect HCD signatures to change.  E.g. dequeue() can take a
host endpoint just like enqueue().  Eventually I'd hope the HCDs
would stop doing such lookups entirely...


> I've got two proposed changes to help deal with this.  The first is to 
> add an "enabled" flag to struct usb_host_endpoint.  This simplifies the 
> testing a lot and is generally superior to the array entry approach 
> IMO.

A backpointer to the device could do double duty here ... if it's
null then enqueue() is disallowed, and otherwise it would be used
for whatever.


> The second is to change struct urb by adding a pointer to the 
> usb_host_endpoint.  We have discussed this before; eventually urb->ep 
> should replace urb->pipe entirely.  For now, until the drivers can be 
> changed we'll have to live with both fields.  Still, it's a step in the 
> right direction.

When you set it up in submit_urb() you aren't testing if it's enabled.

I wouldn't mark that as "out"; for now it's internal to core+hcd, and
probably shouldn't be meaningful except between submit and complete...


> Of particular interest is the new routine usb_control_urb_is_out().  
> The one piece of information present in a pipe but not an endpoint 
> descriptor is the direction of a control transfer.  This routine 
> figures out the direction by looking at bRequestType and wLength in the 
> setup packet.

ISTR other special casing that kicks in "early", e.g. before the
SET_ADDRESS request completes.


> The patch below shows how all this will work.  It adds some extra
> things as well, utility routines for extracting an endpoint's transfer
> type and number.  The changes to usb_submit_urb() include a local
> variable rename or two, so they aren't quite as bad as they may look.
> 
> Does anybody object to this?

Not in principle, modulo the notes above, but I don't have time for
anything like a careful review.

- Dave


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel