On Wed, 7 Jun 2017, Felipe Balbi wrote: > Move the code which was part of pullup() to the newly introduced > method.
Comments inline... > Signed-off-by: Felipe Balbi <[email protected]> > --- > drivers/usb/gadget/udc/dummy_hcd.c | 57 > +++++++++++++++++++++++++++----------- > 1 file changed, 41 insertions(+), 16 deletions(-) > > diff --git a/drivers/usb/gadget/udc/dummy_hcd.c > b/drivers/usb/gadget/udc/dummy_hcd.c > index c79081952ea0..7195a5b21e87 100644 > --- a/drivers/usb/gadget/udc/dummy_hcd.c > +++ b/drivers/usb/gadget/udc/dummy_hcd.c > @@ -888,22 +888,6 @@ static int dummy_pullup(struct usb_gadget *_gadget, int > value) > unsigned long flags; > > dum = gadget_dev_to_dummy(&_gadget->dev); > - > - if (value && dum->driver) { > - if (mod_data.is_super_speed) > - dum->gadget.speed = dum->driver->max_speed; > - else if (mod_data.is_high_speed) > - dum->gadget.speed = min_t(u8, USB_SPEED_HIGH, > - dum->driver->max_speed); > - else > - dum->gadget.speed = USB_SPEED_FULL; > - dummy_udc_update_ep0(dum); > - > - if (dum->gadget.speed < dum->driver->max_speed) > - dev_dbg(udc_dev(dum), "This device can perform faster" > - " if you connect it to a %s port...\n", > - usb_speed_string(dum->driver->max_speed)); > - } > dum_hcd = gadget_to_dummy_hcd(_gadget); > > spin_lock_irqsave(&dum->lock, flags); > @@ -918,6 +902,8 @@ static int dummy_pullup(struct usb_gadget *_gadget, int > value) > static int dummy_udc_start(struct usb_gadget *g, > struct usb_gadget_driver *driver); > static int dummy_udc_stop(struct usb_gadget *g); > +static void dummy_udc_set_speed(struct usb_gadget *g, > + enum usb_device_speed speed); > > static const struct usb_gadget_ops dummy_ops = { > .get_frame = dummy_g_get_frame, > @@ -926,6 +912,7 @@ static const struct usb_gadget_ops dummy_ops = { > .pullup = dummy_pullup, > .udc_start = dummy_udc_start, > .udc_stop = dummy_udc_stop, > + .udc_set_speed = dummy_udc_set_speed, > }; > > /*-------------------------------------------------------------------------*/ > @@ -988,6 +975,44 @@ static int dummy_udc_stop(struct usb_gadget *g) > return 0; > } > > +static void dummy_udc_set_speed(struct usb_gadget *_gadget, > + enum usb_device_speed speed) > +{ I would put this new function immediately after dummy_pullup(), to avoid the need for a forward declaration. Afer all, their jobs are kind of related. > + struct dummy *dum; > + > + dum = gadget_dev_to_dummy(&_gadget->dev); > + > + switch (speed) { > + case USB_SPEED_SUPER: > + if (mod_data.is_super_speed) > + dum->gadget.speed = dum->driver->max_speed; > + break; > + case USB_SPEED_HIGH: > + if (mod_data.is_high_speed) > + dum->gadget.speed = min_t(u8, USB_SPEED_HIGH, > + dum->driver->max_speed); > + break; > + case USB_SPEED_FULL: > + dum->gadget.speed = USB_SPEED_FULL; > + break; > + default: > + if (mod_data.is_super_speed) > + dum->gadget.speed = USB_SPEED_SUPER; > + else if (mod_data.is_high_speed) > + dum->gadget.speed = USB_SPEED_HIGH; > + else > + dum->gadget.speed = USB_SPEED_FULL; > + break; > + } IMO this should be structured differently. Instead of using the value of speed, use the values of mod_data.is_super_speed and mod_data.is_high_speed for the top-level decision making. That is: if (mod_data.is_super_speed) dum->gadget.speed = min_t(u8, USB_SPEED_SUPER, speed); else if (mod_data.is_high_speed) dum->gadget.speed = min_t(u8, USB_SPEED_HIGH, speed); else dum->gadget.speed = USB_SPEED_FULL; In other words, structure this like the original code, but use speed instead of dum->driver->max_speed. (If you're worried about speed being equal to USB_SPEED_WIRELESS or USB_SPEED_LOW, you can add checks for unsupported values.) > + > + dummy_udc_update_ep0(dum); > + > + if (dum->gadget.speed < dum->driver->max_speed) > + dev_dbg(udc_dev(dum), "This device can perform faster" > + " if you connect it to a %s port...\n", > + usb_speed_string(dum->driver->max_speed)); > +} Here also the occurrences of dum->driver->max_speed should be replaced with the speed function parameter. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
