From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751551AbdAPKit (ORCPT ); Mon, 16 Jan 2017 05:38:49 -0500 Received: from mga04.intel.com ([192.55.52.120]:57197 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751172AbdAPKiq (ORCPT ); Mon, 16 Jan 2017 05:38:46 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,238,1477983600"; d="asc'?scan'208";a="1083531045" From: Felipe Balbi To: John Youn , Baolin Wang , John Youn Cc: Greg KH , Mark Brown , USB , LKML Subject: Re: [PATCH] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role In-Reply-To: <99cbb96c-be63-0e51-03a1-d10e478f47e7@synopsys.com> References: <87y40luiem.fsf@linux.intel.com> <87o9zcra4f.fsf@linux.intel.com> <99cbb96c-be63-0e51-03a1-d10e478f47e7@synopsys.com> Date: Mon, 16 Jan 2017 12:36:45 +0200 Message-ID: <87pojnmgua.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, John Youn writes: >> Baolin Wang writes: >>>> Baolin Wang writes: >>>>> When dwc3 controller acts as host role with attaching slow speed devi= ce >>>>> (like mouse or keypad). Then if we plugged out the slow speed device, >>>>> it will timeout to run the deconfiguration endpoint command to drop t= he >>>>> endpoint's resources. Some xHCI command timeout log as below when >>>>> disconnecting one slow device: >>>>> >>>>> [ 99.807739] c0 xhci-hcd.0.auto: Port Status Change Event for port 1 >>>>> [ 99.814699] c0 xhci-hcd.0.auto: resume root hub >>>>> [ 99.819992] c0 xhci-hcd.0.auto: handle_port_status: starting port >>>>> polling. >>>>> [ 99.827808] c0 xhci-hcd.0.auto: get port status, actual port 0 sta= tus >>>>> =3D 0x202a0 >>>>> [ 99.835903] c0 xhci-hcd.0.auto: Get port status returned 0x10100 >>>>> [ 99.850052] c0 xhci-hcd.0.auto: clear port connect change, actual >>>>> port 0 status =3D 0x2a0 >>>>> [ 99.859313] c0 xhci-hcd.0.auto: Cancel URB ffffffc01ed6cd00, dev 1, >>>>> ep 0x81, starting at offset 0xc406d2= 10 >>>>> [ 99.869645] c0 xhci-hcd.0.auto: // Ding dong! >>>>> [ 99.874776] c0 xhci-hcd.0.auto: Stopped on Transfer TRB >>>>> [ 99.880713] c0 xhci-hcd.0.auto: Removing canceled TD starting at >>>>> 0xc406d210 (dma). >>>>> [ 99.889012] c0 xhci-hcd.0.auto: Finding endpoint context >>>>> [ 99.895069] c0 xhci-hcd.0.auto: Cycle state =3D 0x1 >>>>> [ 99.900519] c0 xhci-hcd.0.auto: New dequeue segment =3D >>>>> ffffffc1112f0880 (virtual) >>>>> [ 99.908655] c0 xhci-hcd.0.auto: New dequeue pointer =3D 0xc406d220= (DMA) >>>>> [ 99.915927] c0 xhci-hcd.0.auto: Set TR Deq Ptr cmd, new deq seg =3D >>>>> ffffffc1112f0880 (0xc406d000 dma), >>>>> new deq ptr =3D ffffff8002175220 >>>>> (0xc406d220 dma), new cycle =3D 1 >>>>> [ 99.931242] c0 xhci-hcd.0.auto: // Ding dong! >>>>> [ 99.936360] c0 xhci-hcd.0.auto: Successful Set TR Deq Ptr cmd, >>>>> deq =3D @c406d220 >>>>> [ 99.944458] c0 xhci-hcd.0.auto: xhci_hub_status_data: stopping port >>>>> polling. >>>>> [ 100.047619] c0 xhci-hcd.0.auto: xhci_drop_endpoint called for udev >>>>> ffffffc01ae08800 >>>>> [ 100.057002] c0 xhci-hcd.0.auto: drop ep 0x81, slot id 1, new drop >>>>> flags =3D 0x8, new add flags =3D 0x0 >>>>> [ 100.067878] c0 xhci-hcd.0.auto: xhci_check_bandwidth called for ud= ev >>>>> ffffffc01ae08800 >>>>> [ 100.076868] c0 xhci-hcd.0.auto: New Input Control Context: >>>>> >>>>> ...... >>>>> >>>>> [ 100.427252] c0 xhci-hcd.0.auto: // Ding dong! >>>>> [ 105.430728] c0 xhci-hcd.0.auto: Command timeout >>>>> [ 105.436029] c0 xhci-hcd.0.auto: Abort command ring >>>>> [ 113.558223] c0 xhci-hcd.0.auto: Command completion event does not = match >>>>> command >>>>> [ 113.569778] c0 xhci-hcd.0.auto: Timeout while waiting for configure >>>>> endpoint command >>>>> >>>>> The reason is it will suspend USB phy to disable phy clock when >>>>> disconnecting the slow USB decice, that will hang on the xHCI commands >>>>> executing which depends on the phy clock. >>>>> >>>>> Thus we should disable USB2.0 phy suspend feature when dwc3 acts as h= ost >>>>> role. >>>>> >>>>> Signed-off-by: Baolin Wang >>>>> --- >>>>> drivers/usb/dwc3/core.c | 14 ++++++++++++++ >>>>> 1 file changed, 14 insertions(+) >>>>> >>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>>> index 9a4a5e4..0b646cf 100644 >>>>> --- a/drivers/usb/dwc3/core.c >>>>> +++ b/drivers/usb/dwc3/core.c >>>>> @@ -565,6 +565,20 @@ static int dwc3_phy_setup(struct dwc3 *dwc) >>>>> if (dwc->revision > DWC3_REVISION_194A) >>>>> reg |=3D DWC3_GUSB2PHYCFG_SUSPHY; >>>>> >>>>> + /* >>>>> + * When dwc3 controller acts as host role with attaching one sl= ow speed >>>>> + * device (like mouse or keypad). Then if we plugged out the sl= ow speed >>>>> + * device, it will timeout to run the deconfiguration endpoint = command. >>>>> + * The reason is it will suspend USB phy to disable phy clock w= hen >>>>> + * disconnecting slow speed decice, which will affect the xHCI = commands >>>>> + * executing. >>>>> + * >>>>> + * Thus we should disable USB 2.0 phy suspend feature when dwc3= acts as >>>>> + * host role. >>>>> + */ >>>>> + if (dwc->dr_mode =3D=3D USB_DR_MODE_HOST || dwc->dr_mode =3D=3D= USB_DR_MODE_OTG) >>>>> + reg &=3D ~DWC3_GUSB2PHYCFG_SUSPHY; >>>> >>>> which version of the core you're using? Recent version (since 1.94A, >>> >>> My version is 2.80a. >>> >>>> IIRC) can manage core suspend automatically. Also, this patch of yours >>>> will cause a power consumption regression. >>> >>> Yes, it can manage core suspend automatically, that is the problem. >>> When plugging out one mouse or keypad device, the phy will suspend >>> automatically to disable the phy clock. But now the disconnecting >>> process is not finished, and some xHCI commands (like deconfiguration >>> endpoint command to drop endpoint resources) need depend on the phy >>> clock, which will hang on the system to timeout the command or abort >>> command ring to halt the xHCI. >>> >>> I agree with you it will cause a power consumption regression, but it >>> will cause serious problem if not. Do you have some suggestion? >>=20 >> sorry for the long delay. This was lost in my inbox. >>=20 >> I'm not sure this patch is the best solution. There's no mention in >> Databook that we should avoid PHY suspend when acting as host. Adding >> John here to see if John has any idea of how to fix this. >>=20 > > I'm not familiar enough with XHCI side of things to say. > > I'll ask around to see if anyone has an idea. Thank you =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlh8oj0ACgkQzL64meEa mQYRog/+IWclPrUUzAnNeHmcsif1P0zmWIyrIjEdgPJllwwi6DUW3ngRzsLIVwfn FCqKvH5FnFhUpS34UKmvdvFMfhG/uaNv4L6eCBYntBqgNggMWUR2c/rY9eO/xT6X 32yzu/o7Zn6OxWjeK8cOfAmbH40B9eTNos0+2UCGAo5hVlITtd/ylhr1kbrGf/Iw D8KsPAOUGuMJqY7hljasC0331+IEEW2X29vvRA5IMUw8KX6JQ/+6eScjOH9PaKXi 9RxRvVW/D2NmphqpZ6lMZrhLYThwauogwlGKAWv80IjqMjUtbIZq0BbqshQkbtdm 92R91J70VgOCrP89WIyob8jpLpXUqHDfz7RDc/6Fd03kIG7NvLoAK7Mp0wCA96m6 hTK5deNOqxQ9xuA1woDSlGgGaFAIzyhuI6SPc+N89waRGI79680Z89OgMTr+bQ7i tkgdP+lJRatqNe+FLMEA6ikCwrc27ifvV/lrDTgFDvU6sbqBoL5dee9f+ALzOIBz Tgj4XIvTKAl5anZU3NJ4QPEM1XnrMivR30+9pQHEZ6PsLREnn0KUdeRsOTHbIXmt fvHIL7SOQ1zk5TXXqwDkQD8m4PAFtAhEJzWc40wh9i0dk7/ayokfUJTB8VZEgFTp zMDokfK+53C/Tj2Nc08R6OMEzqi35TQz8S371qHchu8A0rF8UFg= =Kbyg -----END PGP SIGNATURE----- --=-=-=--