From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: usb: dwc3: core: Don't try to get PHYs during suspend/resume From: Felipe Balbi Message-Id: <87373dzvmi.fsf@linux.intel.com> Date: Wed, 10 Jan 2018 15:33:25 +0200 To: Roger Quadros Cc: vigneshr@ti.com, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, "linux-stable # = v4 . 13" List-ID: SGksCgpSb2dlciBRdWFkcm9zIDxyb2dlcnFAdGkuY29tPiB3cml0ZXM6Cj4gRmVsaXBlLAo+Cj4g T24gMTAvMDEvMTggMTU6MTEsIFJvZ2VyIFF1YWRyb3Mgd3JvdGU6Cj4+IFRoZSBVU0IgUEhZcyBz aG91bGQgYmUgcmVxdWVzdGVkIG9ubHkgb25jZSBkdXJpbmcgdGhlIGxpZmUgY3ljbGUgb2YKPj4g dGhpcyBkcml2ZXIuCj4+IAo+PiBBcyBkd2MzX2NvcmVfaW5pdCgpIGlzIGNhbGxlZCBkdXJpbmcg c3lzdGVtIHN1c3BlbmQvcmVzdW1lCj4+IGl0IHdpbGwgcmVzdWx0IGluIG11bHRpcGxlIGNhbGxz IHRvIGR3YzNfY29yZV9nZXRfcGh5KCkgd2hpY2ggaXMgd3JvbmcuCj4+IAo+PiBUbyBwcmV2ZW50 IHRoYXQgbGV0J3MgbW92ZSBkd2MzX2NvcmVfZ2V0X3BoeSgpIGNhbGwKPj4gb3V0c2lkZSBkd2Mz X2NvcmVfaW5pdCgpLgo+PiAKPj4gRml4ZXM6IDU0MTc2OGIwOGE0ICgidXNiOiBkd2MzOiBjb3Jl OiBDYWxsIGR3YzNfY29yZV9nZXRfcGh5KCkgYmVmb3JlIGluaXRpYWxpemluZyBwaHlzIikKPj4g Q2M6IGxpbnV4LXN0YWJsZSA8c3RhYmxlQHZnZXIua2VybmVsLm9yZz4gIyA+PSB2NC4xMwo+PiBT aWduZWQtb2ZmLWJ5OiBSb2dlciBRdWFkcm9zIDxyb2dlcnFAdGkuY29tPgo+Cj4gRllJLiB0aGlz IHBhdGNoIGJyaW5ncyB0aGUgY29kZSBiYWNrIHRvCj4gcmV2ZXJ0IDU0MTc2OGIwOGE0MAkoInVz YjogZHdjMzogY29yZTogQ2FsbCBkd2MzX2NvcmVfZ2V0X3BoeSgpIGJlZm9yZSBpbml0aWFsaXpp bmcgcGh5cyIpCj4gcmV2ZXJ0IGY1NGVkYjUzOWMxMQkoInVzYjogZHdjMzogY29yZTogaW5pdGlh bGl6ZSBVTFBJIGJlZm9yZSB0cnlpbmcgdG8gZ2V0IHRoZSBQSFkiKQo+Cj4gU28gbG9va3MgbGlr ZSB0aGlzIHdpbGwgYnJlYWsgVUxQSSBQSFkgY2FzZT8KPgo+IFdoZXJlIGRvIHdlIGluaXRpYWxp emUgVUxQSSBQSFksIGluIGR3YzNfcGh5X3NldHVwKCk/Cj4KPiBpZiBzbyB0aGVuIDU0MTc2OGIw OGE0MCBicmVha3MgdGhlIFVMUEkgUEhZIGNhc2UgYXMgd2VsbCwgcmlnaHQ/CgppbmRlZWQsIHRo YXQgY29tbWl0IHJlZ3Jlc3NlZCBVTFBJIFBIWXMgOi0oCgpTZWVtcyBsaWtlIGl0IHNob3VsZCBi ZSBtb3JlIGxpa2UgYmVsb3c6CgpAQCAtNzU0LDE1ICs3NTQsMTUgQEAgc3RhdGljIGludCBkd2Mz X2NvcmVfaW5pdChzdHJ1Y3QgZHdjMyAqZHdjKQogCQkJZHdjLT5tYXhpbXVtX3NwZWVkID0gVVNC X1NQRUVEX0hJR0g7CiAJfQogCi0JcmV0ID0gZHdjM19jb3JlX2dldF9waHkoZHdjKTsKKwlyZXQg PSBkd2MzX3BoeV9zZXR1cChkd2MpOwogCWlmIChyZXQpCiAJCWdvdG8gZXJyMDsKIAotCXJldCA9 IGR3YzNfY29yZV9zb2Z0X3Jlc2V0KGR3Yyk7CisJcmV0ID0gZHdjM19jb3JlX2dldF9waHkoZHdj KTsKIAlpZiAocmV0KQogCQlnb3RvIGVycjA7CiAKLQlyZXQgPSBkd2MzX3BoeV9zZXR1cChkd2Mp OworCXJldCA9IGR3YzNfY29yZV9zb2Z0X3Jlc2V0KGR3Yyk7CiAJaWYgKHJldCkKIAkJZ290byBl cnIwOwogCkFuZCBtYXliZSB3ZSByZW5hbWUgZHdjM19waHlfc2V0dXAoKSB0byBkd2MzX3BoeV9p bnRmX2NvbmZpZygpIGp1c3QgdG8KbWFrZSB0aGUgbmFtZSBtYXRjaCB3aGF0IHRoZSBmdW5jdGlv biBhY3R1YWxseSBkb2VzLiBDYW4geW91IGNoZWNrIHRoYXQKaXQgd29uJ3QgcmVncmVzcyB0aGUg Y2FzZSByZXBvcnRlZCBieSBDYXJsb3M/IElmIHRoYXQgd29ya3MsIHRoZW4gd2UKd291bGQgaGF2 ZSB0byBtb3ZlIEJPVEggZHdjM19waHlfc2V0dXAoKSAoZHdjM19waHlfaW50Zl9jb25maWcoKSkg YW5kCmR3YzNfY29yZV9nZXRfcGh5KCkgb3V0c2lkZSBvZiBkd2MzX2NvcmVfaW5pdCgpLCB3aGlj aCB3b3VsZCBtZWFuCmR1cGxpY2F0ZWQgY29kZSBpbiBzdXNwZW5kL3Jlc3VtZSBoYW5kbGVycy4K CkknbSBzdXJlIHdlIGNhbiBzb3J0IHRoYXQgb3V0IGluIGFub3RoZXIgd2F5OyBidXQgdGhlIHBy b3BlciBvcmRlciBpczoKCi0+IGluaXRpYWxpemUgVUxQSSAoaWYgbmVjZXNzYXJ5KQotPiBnZXQg cGh5Ci0+IHNvZnQgcmVzZXQK From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756220AbeAJNdi (ORCPT + 1 other); Wed, 10 Jan 2018 08:33:38 -0500 Received: from mga05.intel.com ([192.55.52.43]:10704 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751356AbeAJNdh (ORCPT ); Wed, 10 Jan 2018 08:33:37 -0500 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,340,1511856000"; d="asc'?scan'208";a="18146822" From: Felipe Balbi To: Roger Quadros Cc: vigneshr@ti.com, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, "linux-stable # \= v4 . 13" Subject: Re: [PATCH] usb: dwc3: core: Don't try to get PHYs during suspend/resume In-Reply-To: <0c2c7e45-9324-316a-d44b-dd17a3a2c68b@ti.com> References: <1515589914-23460-1-git-send-email-rogerq@ti.com> <0c2c7e45-9324-316a-d44b-dd17a3a2c68b@ti.com> Date: Wed, 10 Jan 2018 15:33:25 +0200 Message-ID: <87373dzvmi.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Roger Quadros writes: > Felipe, > > On 10/01/18 15:11, Roger Quadros wrote: >> The USB PHYs should be requested only once during the life cycle of >> this driver. >>=20 >> As dwc3_core_init() is called during system suspend/resume >> it will result in multiple calls to dwc3_core_get_phy() which is wrong. >>=20 >> To prevent that let's move dwc3_core_get_phy() call >> outside dwc3_core_init(). >>=20 >> Fixes: 541768b08a4 ("usb: dwc3: core: Call dwc3_core_get_phy() before in= itializing phys") >> Cc: linux-stable # >=3D v4.13 >> Signed-off-by: Roger Quadros > > FYI. this patch brings the code back to > revert 541768b08a40 ("usb: dwc3: core: Call dwc3_core_get_phy() before in= itializing phys") > revert f54edb539c11 ("usb: dwc3: core: initialize ULPI before trying to g= et the PHY") > > So looks like this will break ULPI PHY case? > > Where do we initialize ULPI PHY, in dwc3_phy_setup()? > > if so then 541768b08a40 breaks the ULPI PHY case as well, right? indeed, that commit regressed ULPI PHYs :-( Seems like it should be more like below: @@ -754,15 +754,15 @@ static int dwc3_core_init(struct dwc3 *dwc) dwc->maximum_speed =3D USB_SPEED_HIGH; } =20 =2D ret =3D dwc3_core_get_phy(dwc); + ret =3D dwc3_phy_setup(dwc); if (ret) goto err0; =20 =2D ret =3D dwc3_core_soft_reset(dwc); + ret =3D dwc3_core_get_phy(dwc); if (ret) goto err0; =20 =2D ret =3D dwc3_phy_setup(dwc); + ret =3D dwc3_core_soft_reset(dwc); if (ret) goto err0; =20 And maybe we rename dwc3_phy_setup() to dwc3_phy_intf_config() just to make the name match what the function actually does. Can you check that it won't regress the case reported by Carlos? If that works, then we would have to move BOTH dwc3_phy_setup() (dwc3_phy_intf_config()) and dwc3_core_get_phy() outside of dwc3_core_init(), which would mean duplicated code in suspend/resume handlers. I'm sure we can sort that out in another way; but the proper order is: =2D> initialize ULPI (if necessary) =2D> get phy =2D> soft reset =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlpWFiUACgkQzL64meEa mQbYUg/+OHmd+EWVYbEwRIiYQ2B4ksj1oXcevXoVyzBbSrQvf6smA0aF1Yic0rna O/63uH1GgYYovOHR5yojs4Wtvru7AYDJR3Ypd27RBU7RrL8V9n8bgBPw84/tHkB8 +qta10mWd18ioHlxSwaO/QHQZnpW+ckQGKgeAUK++cmqnw8QqXiAyr0VSz4li+7e gPjERk3J7GSWuNrKahVs6nYVagexQeKhbrzDGBBZW9KeTwWkmpybivRrC5GATaOv 1orSNyAjsl5/0rjGVRmwiISn1s9DDevT6H2XyLTmOVgGkGSeCls9AlLX6RpS4+o5 5aUMoBSCIXtG/oTcDhdvHNP+2yStonGtoK5rJjMVYkfsuCusMyE6etJS9H9prTfQ 7hVD4l3Sb9KCiC2MZujQyXISN124JlCf2vvMwwf8NhLE4xWKSt1T07eg4U0i0y/L +/Kn5awe65AsZzfLXLZC4KCQReZAI9PGeM+6715z0Go3hgSjOcXxwaEfL4h0Ryvv HWi6wL9EN+NhpeTR3AEIZa0TngQDCuPmKIO0roPKzX5RfW1CKHi8pFEgb/tKVpQQ aVSXI3a0xm1KHDK+b1j8N4D0vuZxGbT/JWWBP6mFNCX0Djk6VMQ8sHtXXGgeqV24 Ys3yE2mTR0yVl/cq2sEPnzhpkqwSdfV5Rh0Xp11dmEuYzrWuQE8= =eEna -----END PGP SIGNATURE----- --=-=-=--