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