Re: [PATCH 1/3] usb: udc: Fix gadget driver's speed check in various UDC drivers

["Heiko Stübner" <heiko@sntech.de>]

Hi Michal,

Am Samstag 19 November 2011, 18:26:25 schrieb Michal Nazarewicz:
> From: Michal Nazarewicz <mina86@mina86.com>
> 
> Several UDC drivers had a gadget driver's speed sanity check of the
> form of:
> 
> 	driver->speed != USB_SPEED_HIGH
> 
> or:
> 
> 	driver->speed != USB_SPEED_HIGH && driver->speed != USB_SPEED_FULL
> 
> As more and more gadget drivers support USB SuperSpeed, driver->speed
> may be set to USB_SPEED_SUPER and UDC driver should handle such gadget
> correctly.  The above checks however fail to recognise USB_SPEED_SUPER
> as a valid speed.
> 
> This commit changes the two checks to:
> 
> 	driver->speed < USB_SPEED_HIGH
> 
> or:
> 
> 	driver->speed < USB_SPEED_FULL
> 
> respectively.
> 
> Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
> Reported-by: Alan Stern <stern@rowland.harvard.edu>
> ---
> 
> I had no way of testing those changes but it appears that those UDC
> drivers check gadget driver's speed only on the entry point and then
> nothing is really dependent on it.
> 
> The only thing that makes me wonder a little is that
> USB_SPEED_WIRELESS will be allowed by the above checks and I'm not
> entirely sure if that's desired (even though that's probably not an
> issue right now).
> 
> Nonetheless, this patch needs to get merged to 3.2 as without it
> file_storage won't work on UDC drivers this patch updates.
> 
> The next two patches that follow this one are replacements for
> [291a72f8: usb: gadget: replace usb_gadget::is_dualspeed with
> max_speed] and [d93bb846: usb: gadget: rename usb_gadget_driver::speed
> to max_speed] from Felipe's gadget tree, which did not apply correctly
> on top of this patch.
> 
> The whole series is based on Greg's for-linus tree.
> 
>  drivers/usb/gadget/amd5536udc.c   |    2 +-
>  drivers/usb/gadget/fsl_qe_udc.c   |    3 +--
>  drivers/usb/gadget/fsl_udc_core.c |    3 +--
>  drivers/usb/gadget/m66592-udc.c   |    2 +-
>  drivers/usb/gadget/net2280.c      |    2 +-
>  drivers/usb/gadget/r8a66597-udc.c |    2 +-
>  drivers/usb/gadget/s3c-hsotg.c    |    4 +---
>  drivers/usb/gadget/s3c-hsudc.c    |    3 +--
>  drivers/usb/musb/musb_gadget.c    |    2 +-
>  9 files changed, 9 insertions(+), 14 deletions(-)
 
[...]

> diff --git a/drivers/usb/gadget/s3c-hsudc.c
> b/drivers/usb/gadget/s3c-hsudc.c index 8d54f89..20a553b 100644
> --- a/drivers/usb/gadget/s3c-hsudc.c
> +++ b/drivers/usb/gadget/s3c-hsudc.c
> @@ -1142,8 +1142,7 @@ static int s3c_hsudc_start(struct usb_gadget_driver
> *driver, int ret;
> 
>  	if (!driver
> -		|| (driver->speed != USB_SPEED_FULL &&
> -			driver->speed != USB_SPEED_HIGH)
> +		|| driver->speed < USB_SPEED_FULL
> 
>  		|| !bind
>  		|| !driver->unbind || !driver->disconnect || !driver->setup)
> 
>  		return -EINVAL;

For s3c-hsudc on a S3C2416 based device

Tested-by: Heiko Stuebner <heiko@sntech.de>


[...]

Heiko