Re: [PATCH v14 4/9] usb: Add max_speed to usb_composite_driver structure

[Felipe Balbi <balbi@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2263 bytes --]

Hi,

On Mon, Jun 06, 2011 at 11:25:14AM -0400, Alan Stern wrote:
> On Mon, 6 Jun 2011, Felipe Balbi wrote:
> 
> > But we might still want to force HighSpeed even though the HW has
> > SuperSpeed support.
> 
> That would be a useful module parameter for a SuperSpeed UDC driver.  
> Like the parameter Tanya added to dummy-hcd.

what I meant was that if you combine a SS-capable function with a
FS-capable function, you would have to run at FS.

> >  The real solution, for the long run would be to
> > always start with pullups disabled (iow, don't connect to host
> > immediately) and only connect after the gadget driver is all
> > initialized, then we will know the gadget speed before connecting, and
> > we can have the gadget driver "request" for a particular speed from the
> > controller.
> 
> Unfortunately this would mean changing a bunch of UDC drivers.  But I 
> agree, it makes sense for the gadget driver to specifically ask for the 
> pullups to be enabled when it is ready.
> 
> Don't some of the existing UDC drivers fail to implement the pullup 
> method at all?

I would have to check to answer that, but I guess it could be one goal
to 'require' that method to be valid. So we could start by:

diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index dd1571d..6c2586f 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -671,8 +671,8 @@ static inline int usb_gadget_vbus_disconnect(struct usb_gadget *gadge
  */
 static inline int usb_gadget_connect(struct usb_gadget *gadget)
 {
-       if (!gadget->ops->pullup)
-               return -EOPNOTSUPP;
+       BUG_ON(!gadget->ops->pullup);
+
        return gadget->ops->pullup(gadget, 1);
 }
 
@@ -693,8 +693,8 @@ static inline int usb_gadget_connect(struct usb_gadget *gadget)
  */
 static inline int usb_gadget_disconnect(struct usb_gadget *gadget)
 {
-       if (!gadget->ops->pullup)
-               return -EOPNOTSUPP;
+       BUG_ON(!gadget->ops->pullup);
+
        return gadget->ops->pullup(gadget, 0);
 }
 

and checking if anyone complains. Another approach is to grep all udc
driver for the missing pullup field and start bugging their maintainers
to implement that.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]