linux-kernel - Re: [PATCHv2] usb: gadget: get rid of USB_GADGET_DUALSPEED and USB_GADGET

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] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 19 Aug 2011 10:29:15 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Michal Nazarewicz <mina86@...a86.com>
cc:	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Sergei Shtylyov <sshtylyov@...sta.com>,
	Felipe Balbi <balbi@...com>,
	Yang Rui Rui <ruirui.r.yang@...to.com>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	USB list <linux-usb@...r.kernel.org>,
	Kernel development list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv2] usb: gadget: get rid of USB_GADGET_DUALSPEED and
 USB_GADGET_SUPERSPEED

On Fri, 19 Aug 2011, Michal Nazarewicz wrote:

> On Fri, 19 Aug 2011 13:13:41 +0200, Sebastian Andrzej Siewior  
> <bigeasy@...utronix.de> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/