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:	Wed, 17 Aug 2011 17:09:37 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Michal Nazarewicz <mnazarewicz@...gle.com>
cc:	Sergei Shtylyov <sshtylyov@...sta.com>,
	Felipe Balbi <balbi@...com>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Yang Rui Rui <ruirui.r.yang@...to.com>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	<linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv2] usb: gadget: get rid of USB_GADGET_DUALSPEED and
 USB_GADGET_SUPERSPEED

On Wed, 17 Aug 2011, Michal Nazarewicz wrote:

> From: Michal Nazarewicz <mina86@...a86.com>
> 
> This commit removes the use of USB_GADGET_DUALSPEED and
> USB_GADGET_SUPERSPEED Kconfig options.  Those were selected
> by UDC drivers which supported respective speeds.
> 
> However, since kernel now allows multiple UDC drivers to be
> compiled, the options in question may no longer reflect the
> state of all gadgets.
> 
> For instance, if one driver that supports dual speed is selected
> and another that does not, the USB_GADGE_DUALSPEED will be set
> "for both".
> 
> This commit replaces all the #ifdefs by a run-time checks made
> by calling gadget_is_dualspeed().


> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -1559,12 +1559,6 @@ composite_resume(struct usb_gadget *gadget)
>  /*-------------------------------------------------------------------------*/
>  
>  static struct usb_gadget_driver composite_driver = {
> -#ifdef CONFIG_USB_GADGET_SUPERSPEED
> -	.speed		= USB_SPEED_SUPER,
> -#else
> -	.speed		= USB_SPEED_HIGH,
> -#endif
> -
>  	.unbind		= composite_unbind,
>  
>  	.setup		= composite_setup,
> @@ -1609,8 +1603,7 @@ int usb_composite_probe(struct usb_composite_driver *driver,
>  		driver->iProduct = driver->name;
>  	composite_driver.function =  (char *) driver->name;
>  	composite_driver.driver.name = driver->name;
> -	composite_driver.speed = min((u8)composite_driver.speed,
> -				     (u8)driver->max_speed);
> +	composite_driver.speed = driver->max_speed;
>  	composite = driver;
>  	composite_gadget_bind = bind;

I see what's going on here.  Your original patch was wrong and then my 
correction was wrong as well.

This line has to remain the way it was (although those (u8) typecasts
don't seem to be necessary).  Above, you have to initialize
composite_driver.speed to an appropriate value, probably
USB_SPEED_SUPER.

What you didn't realize in your original patch is that 
usb_composite_probe() gets called more than once.  Each time it is 
called, it has to adjust composite_driver.speed.

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/