Re: [PATCH] USB: introduce urb->core_status field

On Sun, Jun 10, 2007 at 10:44:28AM -0400, Alan Stern wrote:
> On Sat, 9 Jun 2007, Greg KH wrote:
> 
> > > > Hm, is there any way to keep this from being in the urb entirely?
> > > > Shouldn't this be an internal-to-the hcd type thing?
> > > 
> > > Yes, it could be done that way, I think.  It'll take more work than a 
> > > simple search-and-replace.  The trickiest part is how to unlink an URB 
> > > after it has been submitted but before it has been handed to the HCD...
> > 
> > We allow that today?  How, multiple threads doing the submit and then
> > unlink call?  That's looney.  We should not worry about that until after
> > the submit call has returned to the caller.
> 
> Sure it's looney.  We still have to handle it correctly, though.  Don't 
> worry, it will be easy -- I figured out the right approach last night.

Ok, thanks to Oliver I now see why we need to handle that.

> Another idea suggested itself too: We could make usbcore allocate 
> urb->hcpriv during submission instead of having each HCD do its own 
> private allocation.  It's not a big deal, just a matter of replacing 
> several different calls to kmalloc/kfree by a single call.  Any reason 
> not to do it?

No objection from me.

> Ideally, of course, each HCD would allocate its own URBs with the 
> private area included.  But we can't do that just now, since some 
> drivers maintain pools of URBs that they use for multiple devices, 
> potentially on different buses.

I don't object to fixing those drivers to do a per-device pool instead
of a per-driver pool to make this easier.

So then you could just do a:
	usb_alloc_urb(struct usb_interface *intf, ...)
which would allow the bus that is assigned to the device to do the
proper allocation?  I think that would be good to do.

> > > Maybe.  It _is_ documented as being private to usbcore -- but that 
> > > might not stop people!
> > 
> > Exactly, we need to make it impossible :)
> > 
> > If I get the chance this week I'll look at your patch and see if we can
> > tweak it somehow.
> 
> No, don't bother; I'll redo it completely.  In the revised version
> there won't be a core_status field and neither usbcore nor the HCDs
> will use urb->status (except for the part about setting it to
> -EINPROGRESS when the URB is submitted and storing the status code
> immediately prior to calling the completion routine, for backward
> compatibility).

Ok, sounds good to me.  Thanks for doing this.

greg k-h

-------------------------------------------------------------------------
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