Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 2 Nov 2017 10:14:19 -0400 (EDT)
From:   Alan Stern <stern@rowland.harvard.edu>
To:     Roger Quadros <rogerq@ti.com>
cc:     balbi@kernel.org, <Dylan.Howey@tennantco.com>,
        <linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        "# v4 . 13+" <stable@vger.kernel.org>
Subject: Re: [PATCH v2][RESEND] usb: gadget: core: Fix ->udc_set_speed()
 speed handling
In-Reply-To: <46a6dae0-b9f1-9d7b-d6c7-8f3c8f59ca6e@ti.com>
Message-ID: <Pine.LNX.4.44L0.1711021012070.1373-100000@iolanthe.rowland.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, 2 Nov 2017, Roger Quadros wrote:

> 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.

Ah, I see.  Yes, you can change this too if you want.

Alan Stern


From 1582943814193328827@xxx Thu Nov 02 08:54:19 +0000 2017
X-GM-THRID: 1582781792863818730
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread