linux-kernel - Re: [PATCH v2][RESEND] usb: gadget: core: Fix ->udc_set

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]

Message-ID: <46a6dae0-b9f1-9d7b-d6c7-8f3c8f59ca6e@ti.com>
Date:   Thu, 2 Nov 2017 10:53:08 +0200
From:   Roger Quadros <rogerq@...com>
To:     Alan Stern <stern@...land.harvard.edu>
CC:     <balbi@...nel.org>, <Dylan.Howey@...nantco.com>,
        <linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        "# v4 . 13+" <stable@...r.kernel.org>
Subject: Re: [PATCH v2][RESEND] usb: gadget: core: Fix ->udc_set_speed() speed
 handling

On 01/11/17 17:45, Alan Stern wrote:
> On Wed, 1 Nov 2017, Roger Quadros wrote:
> 
>>> Roger, would you like to send in a follow-up patch that removes the now 
>>> unnecessary code from dummy_udc_set_speed() in dummy_hcd.c?
>>>
>>
>> Sure. I have a question though.
>>
>> In dummy_udc_set_speed() we should just set gadget.speed to whatever is passed in speed.
>> This will be the maximum supported speed by the gadget function driver.
> 
> Following your patch, it will be the maximum speed supported by the 
> gadget function driver or the maximum speed supported by the UDC 
> driver, whichever is lower.  That's the whole point of your patch, 
> right?

Yes.

> 
>> However, is it possible that the gadget will be connected at a lower speed than that?
> 
> Yes, it is possible to connect at a speed lower than the function 
> driver's maximum speed.  The connection speed for dummy-hcd is 
> controlled by a pair of module parameters: is_super_speed and 
> is_high_speed.
> 
>> I see gadget.speed being updated in dummy_hub_control() GetPortStatus: case, but not
>> in case of USB_SPEED_HIGH and USB_SPEED_LOW.
> 
> Here's what the code does, in outline form:
> 
> 	switch (dum_hcd->dum->gadget.speed) {
> 	case USB_SPEED_HIGH:
> 		...
> 		break;
> 	case USB_SPEED_LOW:
> 		...
> 		break;
> 	default:
> 		dum_hcd->dum->gadget.speed = USB_SPEED_FULL;
> 		break;
> 	}
> 
> In other words, if gadget.speed isn't USB_SPEED_HIGH or USB_SPEED_LOW, 
> set it to USB_SPEED_FULL.  And this is all under the case where the 
> connection is USB-2, not USB-3 or higher.
> 
> How is this related to the requested change to dummy_udc_set_speed()?

With or without the requested change to dummy_udc_set_speed() there will
never be a case when gadget.speed isn't already USB_SPEED_FULL
if connection is USB-2 and gadget.speed is not USB_SPEED_HIGH or USB_SPEED_LOW.

So in the above code snippet, default: portion is redundant.

But that can be fixed in a separate patch.

-- 
cheers,
-roger

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki