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

Felipe Balbi
2011-06-06T15:39:05Z

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