Re: [PATCHv2] usb: gadget: get rid of USB_GADGET_DUALSPEED andUSB_GADGET_SUPERSPEED

[Alan Stern]

On Fri, 19 Aug 2011, Michal Nazarewicz wrote:

> On Fri, 19 Aug 2011 13:13:41 +0200, Sebastian Andrzej Siewior  
> <bigeasy@xxxxxxxxxxxxx> wrote:
> > There are three speeds:
> > - the max speed the UDC supports i.e. HS (no field for that?)
> 
> Yep, the field seems to be missing for that one.  We have is_dualspeed and
> comment in gadget_is_superspeed() says about is_superspeed but that does
> not exist.

Evidently we need is_superspeed.

> > - the max speed the gadget / function driver support i.e. SS
> >    (driver->max_speed ?)
> 
> Actually âmax_speedâ is only in usb_composite_driver structure.
> usb_gadget_driver has only âspeedâ and it's description says that it is
> the âhighest speed the driver handles.â  Because of that description I've
> assumed that it is UDC driver's responsibility to figure out maximum speed
> it can use.

Correct.

> > - the speed the UDC is connected at the moment i.e. FS. (gadget->speed)
> 
> OK, I see my mistake.  I misinterpreted gadget->speed as the max speed
> gadget supports.
> 
> So in the end, it would seem to me that we need to do the following is
> usb_gadget_probe_direvr() before passing the usb_gadget_driver to the
> usb_gadegt's bind:
> 
> if (driver->speed == USB_SPEED_SUPER && !gadget_is_superspeed(udc->gadget))
> 	driver->speed = USB_SPEED_HIGH;
> if (driver->speed == USB_SPEED_HIGH && !gadget_is_dualspeed(udc->gadget))
> 	driver->speed = USB_SPEED_FULL;

No, the usb_gadget_driver structure is read-only to the UDC driver
(except for the .driver field).  .speed is the maximum speed supported
by the gadget driver -- it doesn't change at runtime.  If the driver
was written to support speeds up to high speed, then those are the
speeds it supports, regardless of the UDC's capabilities.

Maybe if would help if the .speed field in struct usb_gadget_driver was
renamed to max_speed.

> (Although I'm not sure about USB_SPEED_FULL vs. USB_SPEED_LOW.)
> 
> Either than, or make sure that all UDC drivers handle correctly gadgets
> that declare speed as highest of what the UDC can handle.

I don't quite understand that sentence.  However, it is definitely true 
that all UDC drivers _must_ avoid connecting at speeds that are faster 
than the usb_gadget_driver's .speed field.

> At the moment however, gadget_is_superspeed() is implemented using #ifdefs
> (which I missed before) so we need to add a is_superspeed to usb_gadget
> structure.  Or maybe better yet get rid of is_dualspeed and add max_speed?

Either one would be okay.  max_speed is simpler, although it would 
require some work to change the existing usages of is_dualspeed.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/