speedtch usbatm.c,1.20,1.21 usbatm.h,1.13,1.14

[Duncan Sands]

Hi Roman,

> > I don't see how it can be overdesign since I'm just pointing out that we
> > can already accommodate this kind of thing and more by doing... nothing
> > at all!  Since there are currently no drivers which need this, doing
> > nothing seems like a good strategy to me.
> 
> But that won't work as soon as you want to use different transfer types:
> you'll have to add code to detect which endpoint type it is, and set
> channel.endpoint to the appropriate pipe (and handle wrong endpoint
> number and type).

I'm kind of confused here - what's wrong with having the usbatm core
detect the endpoint type?  It sounds like a good idea to me.  But
maybe you are thinking: what if the minidriver expects it to be an
iso endpoint, but it was bulk - how to catch this mistake?  The
minidriver will have to set the right altsetting in bind of course.

> Actually, the do-nothing strategy was implemented in my original code:
> it initialized the .endpoint in each channel to a bulk pipe on the
> endpoint number from struct usbatm_driver, and assumed that the
> minidriver can override it with a different pipe in its .bind(); still,
> you could trust the minidriver to do it right, without error checking.
> Thus leaving this thing as it was will make it work unmodified for bulk
> transfers, and, in order to use isoc in transfer, the subdriver would
> only need _one_ extra line of code in its .bind(),
> 
> usbatm->rx_channel.endpoint = usb_rcvisocpipe(usbatm->usb_dev, isoc_ep_num);

What if it gets it wrong (eg: endpoint doesn't exist, or of the wrong
type)?  To be correct, shouldn't minidrivers check this kind of thing?
But maybe the USB core would catch it anyway.  Anyway, I'd forgotten
that the speedtch has an isochronous as well as a bulk receive endpoint.
I'll implement support for it - having a concrete example should
improve my thinking...

> > > If the USB layer can guarantee that urb.urb_list isn't used by the USB
> > > core in the completion routine (and it certainly isn't used after the
> > > completion routine and until submission), we could use it and get rid of
> > > struct usbatm_transceiver, whose only purpose is to wrap struct urb with
> > > struct list_head.  Anybody knows if it can be done?
> > 
> > Maybe it can be done, but I don't like it - what if the USB core starts
> > using it completion handlers from time to time?  It's not like anybody
> > is going to tell us.
> 
> Well, we can humbly ask if urb.urb_list can be declared "public", and,
> once it is, then the core developers will hopefully stick to not using
> it outside the allowed scope.  Frankly, I'm not even sure that that
> assumption is true in the current tree, but I see no problem asking.
> Since multiple urbs queued on an endpoint is the recommended practice
> now, I guess many other drivers would profit from the general-purpose
> list head present in struct urb.

Fair enough - will you ask (I have to leave soon)?

> > Another possibility is to define a "fat urb type":
> > a struct containing an urb followed by a list head.
> 
> But that's exactly what our struct usbatm_transceiver is.

I didn't mean a struct with an urb pointer and a list_head,
I meant a struct with a struct urb and a list_head.  There's
no real advantage to it though.

D.