From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755264AbdKBIxV (ORCPT ); Thu, 2 Nov 2017 04:53:21 -0400 Received: from fllnx210.ext.ti.com ([198.47.19.17]:41854 "EHLO fllnx210.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751587AbdKBIxQ (ORCPT ); Thu, 2 Nov 2017 04:53:16 -0400 Subject: Re: [PATCH v2][RESEND] usb: gadget: core: Fix ->udc_set_speed() speed handling To: Alan Stern CC: , , , , "# v4 . 13+" References: From: Roger Quadros Message-ID: <46a6dae0-b9f1-9d7b-d6c7-8f3c8f59ca6e@ti.com> Date: Thu, 2 Nov 2017 10:53:08 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-GB Content-Transfer-Encoding: 8bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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