linux-kernel - Re: [PATCH v14 4/9] usb: Add max_speed to usb_composite

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Mon, 6 Jun 2011 18:36:20 +0300
From:	Felipe Balbi <balbi@...com>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	Felipe Balbi <balbi@...com>,
	Tanya Brokhman <tlinder@...eaurora.org>, greg@...ah.com,
	linux-usb@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	ablay@...eaurora.org, 'open list' <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v14 4/9] usb: Add max_speed to usb_composite_driver
 structure

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

Download attachment "signature.asc" of type "application/pgp-signature" (491 bytes)