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;
From:   Peter Chen <peter.chen@nxp.com>
To:     Roger Quadros <rogerq@ti.com>
CC:     "felipe.balbi@linux.intel.com" <felipe.balbi@linux.intel.com>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "pawell@cadence.com" <pawell@cadence.com>,
        "nsekhar@ti.com" <nsekhar@ti.com>,
        "kurahul@cadence.com" <kurahul@cadence.com>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] usb: cdns3: gadget: Fix g_audio use case when connected
 to Super-Speed host
Thread-Topic: [PATCH] usb: cdns3: gadget: Fix g_audio use case when connected
 to Super-Speed host
Thread-Index: AQHVjmu0ISNy1ghJZk6ChiNFIxaNSqdyu+kAgAAjpACAAANYAA==
Date:   Wed, 30 Oct 2019 08:56:19 +0000
Message-ID: <20191030085608.GA11664@b29397-desktop>
References: <20191029151514.28495-1-rogerq@ti.com>
 <20191030063636.GE26815@b29397-desktop>
 <b780ffea-dca0-310e-1d66-4ceca380b4ee@ti.com>
In-Reply-To: <b780ffea-dca0-310e-1d66-4ceca380b4ee@ti.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: nxp.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-ID: <49ED2D1C0BBB7B48A45C798F05788BA6@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: b7261180-1ef7-4821-4674-08d75d1707e6
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Oct 2019 08:56:19.3817
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: zN/iuEx2Pyzp7wXye7o0hjcdeeqyjhb1pOGXPqZ8mtIE67Z5iKvRZ+u6k0lmoaEDjExtmVD3vSbHnyz/d+6t4A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB5824
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 19-10-30 10:44:10, Roger Quadros wrote:
>=20
>=20
> 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.
> > >=20
> > > Signed-off-by: Roger Quadros <rogerq@ti.com>
> > > ---
> > > Hi Greg,
> > >=20
> > > Please apply this for -rc.
> > > Without this, g_audio is broken on cdns3 USB controller is
> > > connected to a Super-Speed host.
> > >=20
> > > cheers,
> > > -roger
> > >=20
> > >   drivers/usb/cdns3/gadget.c | 31 ++++++++++++++++++++++++++-----
> > >   1 file changed, 26 insertions(+), 5 deletions(-)
> > >=20
> > > 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_g=
adget *gadget,
> > >   {
> > >   	struct cdns3_device *priv_dev =3D gadget_to_cdns3_device(gadget);
> > >   	unsigned long flags;
> > > +	enum usb_device_speed max_speed =3D driver->max_speed;
> > >   	spin_lock_irqsave(&priv_dev->lock, flags);
> > >   	priv_dev->gadget_driver =3D driver;
> > > +
> > > +	/* limit speed if necessary */
> > > +	max_speed =3D 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 =3D 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 *cd=
ns)
> > >   	/* 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:
> >=20
> > Just a small comment:
> >=20
> > You could delete switch-case at cdns3_gadget_start, and just use
> > if() statement, eg:
> >=20
> > 	max_speed =3D usb_get_maximum_speed(cdns->dev);
> > 	if (max_speed =3D=3D USB_SPEED_UNKNOWN)
> > 		max_speed =3D USB_SPEED_SUPER;
>=20
> 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.

--=20

Thanks,
Peter Chen=