On 19-10-30 10:44:10, Roger Quadros wrote: > > > On 30/10/2019 08:36, Peter Chen wrote: > > On 19-10-29 17:15:14, Roger Quadros wrote: > > > Take into account gadget driver's speed limit when programming > > > controller speed. > > > > > > Signed-off-by: Roger Quadros <rogerq@xxxxxx> > > > --- > > > Hi Greg, > > > > > > Please apply this for -rc. > > > Without this, g_audio is broken on cdns3 USB controller is > > > connected to a Super-Speed host. > > > > > > cheers, > > > -roger > > > > > > drivers/usb/cdns3/gadget.c | 31 ++++++++++++++++++++++++++----- > > > 1 file changed, 26 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c > > > index 40dad4e8d0dc..1c724c20d468 100644 > > > --- a/drivers/usb/cdns3/gadget.c > > > +++ b/drivers/usb/cdns3/gadget.c > > > @@ -2338,9 +2338,35 @@ static int cdns3_gadget_udc_start(struct usb_gadget *gadget, > > > { > > > struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget); > > > unsigned long flags; > > > + enum usb_device_speed max_speed = driver->max_speed; > > > spin_lock_irqsave(&priv_dev->lock, flags); > > > priv_dev->gadget_driver = driver; > > > + > > > + /* limit speed if necessary */ > > > + max_speed = min(driver->max_speed, gadget->max_speed); > > > + > > > + switch (max_speed) { > > > + case USB_SPEED_FULL: > > > + writel(USB_CONF_SFORCE_FS, &priv_dev->regs->usb_conf); > > > + writel(USB_CONF_USB3DIS, &priv_dev->regs->usb_conf); > > > + break; > > > + case USB_SPEED_HIGH: > > > + writel(USB_CONF_USB3DIS, &priv_dev->regs->usb_conf); > > > + break; > > > + case USB_SPEED_SUPER: > > > + break; > > > + default: > > > + dev_err(priv_dev->dev, > > > + "invalid maximum_speed parameter %d\n", > > > + max_speed); > > > + /* fall through */ > > > + case USB_SPEED_UNKNOWN: > > > + /* default to superspeed */ > > > + max_speed = USB_SPEED_SUPER; > > > + break; > > > + } > > > + > > > cdns3_gadget_config(priv_dev); > > > spin_unlock_irqrestore(&priv_dev->lock, flags); > > > return 0; > > > @@ -2570,12 +2596,7 @@ static int cdns3_gadget_start(struct cdns3 *cdns) > > > /* Check the maximum_speed parameter */ > > > switch (max_speed) { > > > case USB_SPEED_FULL: > > > - writel(USB_CONF_SFORCE_FS, &priv_dev->regs->usb_conf); > > > - writel(USB_CONF_USB3DIS, &priv_dev->regs->usb_conf); > > > - break; > > > case USB_SPEED_HIGH: > > > - writel(USB_CONF_USB3DIS, &priv_dev->regs->usb_conf); > > > - break; > > > case USB_SPEED_SUPER: > > > break; > > > default: > > > > Just a small comment: > > > > You could delete switch-case at cdns3_gadget_start, and just use > > if() statement, eg: > > > > max_speed = usb_get_maximum_speed(cdns->dev); > > if (max_speed == USB_SPEED_UNKNOWN) > > max_speed = USB_SPEED_SUPER; > > But then it will not take care of bailing out for USB_SPEED_WIRELESS, > USB_SPEED_SUPER_PLUS and any future speeds. This IP only supports FS/HS/SS. It doesn't a big issue, if you would like to keep the code like your patch, it is also OK. -- Thanks, Peter Chen