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
