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

[Duncan Sands]

Hi Roman,

> > I have a general solution for this kind of problem: suppose a minidriver
> > wants to modify in bind some of things it specified in its
> > struct usbatm_driver.  Then it should dynamically allocate a
> > struct usbatm_driver in its probe method (maybe copying into it a static
> > struct usbatm_driver that it has prepared in advance), and pass that to
> > usbatm_usb_probe.  In the bind call it modifies that struct via
> > usbatm->driver, eg: changing the rx/tx endpoints.  It frees the struct
> > in unbind.  This will work as long as usbatm reads the driver data after
> > the call to bind, not before (except for the bind method of course).  The
> > reason for allocating the struct rather than modifying the static struct
> > is that that avoids problems if there are several devices for the same
> > minidriver, all trying to modify the same bit of memory...
> 
> This seems a bit of an overdesign to me...  IMHO allowing the minidriver
> to directly modify channel's pipe (as opposed to endpoint) is both
> simpler to use and requires less code.  Violating encapsulation is not a
> deadly sin, we aren't in C++ in the end :)

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.

> BTW speaking of using private fields: from include/linux/usb.h:
> 
> /**
>  * struct urb - USB Request Block
>  * @urb_list: For use by current owner of the URB.
>  ...
>  */
> struct urb
> {
> 	/* private, usb core and host controller only fields in the urb */
> 	...
> 	struct list_head urb_list;	/* list pointer to all active urbs */
> 	...
> };
> 
> 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.  Another possibility is to define a "fat urb type":
a struct containing an urb followed by a list head.

Ciao,

Duncan.