From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751003AbdALHwH (ORCPT ); Thu, 12 Jan 2017 02:52:07 -0500 Received: from mga11.intel.com ([192.55.52.93]:1289 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750802AbdALHwE (ORCPT ); Thu, 12 Jan 2017 02:52:04 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,349,1477983600"; d="asc'?scan'208";a="921635427" From: Felipe Balbi To: 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: References: <87y40luiem.fsf@linux.intel.com> Date: Thu, 12 Jan 2017 09:49:20 +0200 Message-ID: <87o9zcra4f.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, Baolin Wang writes: >> Baolin Wang writes: >>> When dwc3 controller acts as host role with attaching slow speed device >>> (like mouse or keypad). Then if we plugged out the slow speed device, >>> it will timeout to run the deconfiguration endpoint command to drop the >>> 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 status >>> =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 0xc406d210 >>> [ 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 udev >>> 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 ma= tch >>> 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 host >>> 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 slow= speed >>> + * device (like mouse or keypad). Then if we plugged out the slow= speed >>> + * device, it will timeout to run the deconfiguration endpoint co= mmand. >>> + * The reason is it will suspend USB phy to disable phy clock when >>> + * disconnecting slow speed decice, which will affect the xHCI co= mmands >>> + * executing. >>> + * >>> + * Thus we should disable USB 2.0 phy suspend feature when dwc3 a= cts as >>> + * host role. >>> + */ >>> + if (dwc->dr_mode =3D=3D USB_DR_MODE_HOST || dwc->dr_mode =3D=3D U= SB_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? sorry for the long delay. This was lost in my inbox. 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. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlh3NQAACgkQzL64meEa mQZ7fxAA0qWirwaq4xP2iZrKh4pcWt2VRnI4QF+ZNC4pRB35/tKpIawUfJbnyb4O JSej59IIMuCTY0ITIUCpuFp8VDfC8149rUq+PS0ilQ6JcElDAdXOOF26CaADNIXx DavAjnkTIWF1vlv9O2eCuk6BwonEsVY4b0GVvS4SIWhSlXMLKyEDjAzR+GSMli/1 OdkGVKYC/ldRdNTVePDWBxjXAzHHWsgzx2HaVf/xchqyu0Jt6PDTKbOCwb95beZE X8k+poqb5PclkJGaZLB6AahExGAFg/yd5ygN0ZzPDSymgN7657u1a79gaQiylic7 kCp5gn0kkHq4vbIRks+0JtjpbSrNpzW5TF00MEApRekcfzbzkMiyGSBqnkpx+JYa HSMZOkuH9AlK2Tr25dHgwKDfdkSBIMmjSyuA/SyF3iLb6I2iCegMhRRuSREtEb60 OLZ+v1+8Bc3I6SfqB4eh0r0ls7aFYbODMurKZCunafnrt1YDJYKPAYcVL4xSe3md poHpYk7UXgWY4yGaA8lUXVVJfPetH4VLUI7/QP/6IC6GnN15rQyzZ+a8dxtBeZrl VDnp4kBiq+MiN8GPDDUYmHmh8mLtPIFv8rk/avDEECHJV7UIuldmbHVTOPfq5wKD AGiOZJN9FWgpReQAzppk+pjCxe84+oeCi0QSo4auX0rusC/P3oc= =syQ+ -----END PGP SIGNATURE----- --=-=-=--