From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH v1] usb: phy: tegra: Increase PHY clock stabilization timeout Date: Mon, 11 Dec 2017 10:53:22 +0100 Message-ID: <20171211095322.GC10671@ulmo> References: <20171210225535.8532-1-digetx@gmail.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="MnLPg7ZWsaic7Fhd" Return-path: Content-Disposition: inline In-Reply-To: <20171210225535.8532-1-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Sender: linux-tegra-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Dmitry Osipenko Cc: Felipe Balbi , Jonathan Hunter , linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-tegra@vger.kernel.org --MnLPg7ZWsaic7Fhd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Dec 11, 2017 at 01:55:35AM +0300, Dmitry Osipenko wrote: > This fixes "utmi_phy_clk_enable: timeout waiting for phy to stabilize" > error message. >=20 > Signed-off-by: Dmitry Osipenko > --- > drivers/usb/phy/phy-tegra-usb.c | 13 ++++--------- > 1 file changed, 4 insertions(+), 9 deletions(-) >=20 > diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-= usb.c > index f668bfb708d3..7d5db625f800 100644 > --- a/drivers/usb/phy/phy-tegra-usb.c > +++ b/drivers/usb/phy/phy-tegra-usb.c > @@ -16,7 +16,7 @@ > #include > #include > #include > -#include > +#include > #include > #include > #include > @@ -305,14 +305,9 @@ static int utmip_pad_power_off(struct tegra_usb_phy = *phy) > =20 > static int utmi_wait_register(void __iomem *reg, u32 mask, u32 result) > { > - unsigned long timeout =3D 2000; > - do { > - if ((readl(reg) & mask) =3D=3D result) > - return 0; > - udelay(1); > - timeout--; > - } while (timeout); > - return -1; > + u32 tmp; > + > + return readl_poll_timeout(reg, tmp, (tmp & mask) =3D=3D result, 1, 5000= ); Technically I think this should be readl_poll_timeout_atomic() because the above used to use udelay() instead of usleep_range(). But since the function is never used inside atomic context, this looks fine. You may want to bump the sleep time between reads to something like 10 or 20. usleep_range() doesn't always work well with very short values, and given that you already bump the timeout from 2 ms to 5 ms indicates to me that we're actually spending a lot of time in this loop, and iterating somewhere between 2000 and 5000 times isn't any good. We only use this function to wait for the USB_PHY_CLK_VALID bit which happens during clock enable and disable, which isn't going to be very often, so even in the best case (where the clock is immediately valid) there's no need to return within a microsecond. Thierry --MnLPg7ZWsaic7Fhd Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAlouVZAACgkQ3SOs138+ s6EPghAAv6vPAwumYNXxv8GF2DdFzg14baJqXarudQZxRtk5fTyUNuaeAKNjuEFy rrr4ACMYio4k/hk2axFufES+4MdXknNzQg/1gpNkLYZQvU4yYLLqd69NhFZonPDZ 4hyUgb5J+PYZFWEngpBliRbZarBPmTZQ4SsnnB+ZZ6zzrl8QOoUsfJ/wWz2KHA61 sL0dOf1odD1ii+3hdDBF4n0vjas0qyeEHqVwrrbA1cM2N2bu9jwpd7PZJmDAhioQ 2MtaAJBRaUadCOuNRUcPODvRgfiYw2DKplVPnNUgNx5OUWhoUSH3Kny7jCF63kTG 1xwOUvAwFT9smDac1psv9WIehTAtZLXFZ9Jc9P7LuU4vMVvmUMEaVXac+AyGwhil 2VnMdPnPBIaC++gPlOtSOr1oJPQD8lduzKf2A1NukAbPUM8FVmwcVv3zyo6lVw3o WPKR+BgxSnYKdL1q1upEoI0e6xrTJXDVp+FxK8MnA2xXQPrzZEfb8bFuJjKZj9Ei i2DT9VjXon9dahe3wAEsCFIL+JdQyRrMZ79xoIux0TNwKS7Bv1EcMzYL+iI8CgeA iu1+lbTJ5eFmufR+xI+BO6xiCHmxqJb+N/c9mXwl6lYrwlWvyNhNtBEJCEpLE9zX 82daFdIw4S8Q0D4SMJqN5keVJbb6zPRXBgCcgFdhAG9GJQy25Qk= =DwOm -----END PGP SIGNATURE----- --MnLPg7ZWsaic7Fhd-- 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: [v1] usb: phy: tegra: Increase PHY clock stabilization timeout From: Thierry Reding Message-Id: <20171211095322.GC10671@ulmo> Date: Mon, 11 Dec 2017 10:53:22 +0100 To: Dmitry Osipenko Cc: Felipe Balbi , Jonathan Hunter , linux-usb@vger.kernel.org, linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org List-ID: T24gTW9uLCBEZWMgMTEsIDIwMTcgYXQgMDE6NTU6MzVBTSArMDMwMCwgRG1pdHJ5IE9zaXBlbmtv IHdyb3RlOgo+IFRoaXMgZml4ZXMgInV0bWlfcGh5X2Nsa19lbmFibGU6IHRpbWVvdXQgd2FpdGlu ZyBmb3IgcGh5IHRvIHN0YWJpbGl6ZSIKPiBlcnJvciBtZXNzYWdlLgo+IAo+IFNpZ25lZC1vZmYt Ynk6IERtaXRyeSBPc2lwZW5rbyA8ZGlnZXR4QGdtYWlsLmNvbT4KPiAtLS0KPiAgZHJpdmVycy91 c2IvcGh5L3BoeS10ZWdyYS11c2IuYyB8IDEzICsrKystLS0tLS0tLS0KPiAgMSBmaWxlIGNoYW5n ZWQsIDQgaW5zZXJ0aW9ucygrKSwgOSBkZWxldGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0IGEvZHJp dmVycy91c2IvcGh5L3BoeS10ZWdyYS11c2IuYyBiL2RyaXZlcnMvdXNiL3BoeS9waHktdGVncmEt dXNiLmMKPiBpbmRleCBmNjY4YmZiNzA4ZDMuLjdkNWRiNjI1ZjgwMCAxMDA2NDQKPiAtLS0gYS9k cml2ZXJzL3VzYi9waHkvcGh5LXRlZ3JhLXVzYi5jCj4gKysrIGIvZHJpdmVycy91c2IvcGh5L3Bo eS10ZWdyYS11c2IuYwo+IEBAIC0xNiw3ICsxNiw3IEBACj4gICNpbmNsdWRlIDxsaW51eC9leHBv cnQuaD4KPiAgI2luY2x1ZGUgPGxpbnV4L21vZHVsZS5oPgo+ICAjaW5jbHVkZSA8bGludXgvcGxh dGZvcm1fZGV2aWNlLmg+Cj4gLSNpbmNsdWRlIDxsaW51eC9pby5oPgo+ICsjaW5jbHVkZSA8bGlu dXgvaW9wb2xsLmg+Cj4gICNpbmNsdWRlIDxsaW51eC9ncGlvLmg+Cj4gICNpbmNsdWRlIDxsaW51 eC9vZi5oPgo+ICAjaW5jbHVkZSA8bGludXgvb2ZfZGV2aWNlLmg+Cj4gQEAgLTMwNSwxNCArMzA1 LDkgQEAgc3RhdGljIGludCB1dG1pcF9wYWRfcG93ZXJfb2ZmKHN0cnVjdCB0ZWdyYV91c2JfcGh5 ICpwaHkpCj4gIAo+ICBzdGF0aWMgaW50IHV0bWlfd2FpdF9yZWdpc3Rlcih2b2lkIF9faW9tZW0g KnJlZywgdTMyIG1hc2ssIHUzMiByZXN1bHQpCj4gIHsKPiAtCXVuc2lnbmVkIGxvbmcgdGltZW91 dCA9IDIwMDA7Cj4gLQlkbyB7Cj4gLQkJaWYgKChyZWFkbChyZWcpICYgbWFzaykgPT0gcmVzdWx0 KQo+IC0JCQlyZXR1cm4gMDsKPiAtCQl1ZGVsYXkoMSk7Cj4gLQkJdGltZW91dC0tOwo+IC0JfSB3 aGlsZSAodGltZW91dCk7Cj4gLQlyZXR1cm4gLTE7Cj4gKwl1MzIgdG1wOwo+ICsKPiArCXJldHVy biByZWFkbF9wb2xsX3RpbWVvdXQocmVnLCB0bXAsICh0bXAgJiBtYXNrKSA9PSByZXN1bHQsIDEs IDUwMDApOwoKVGVjaG5pY2FsbHkgSSB0aGluayB0aGlzIHNob3VsZCBiZSByZWFkbF9wb2xsX3Rp bWVvdXRfYXRvbWljKCkgYmVjYXVzZQp0aGUgYWJvdmUgdXNlZCB0byB1c2UgdWRlbGF5KCkgaW5z dGVhZCBvZiB1c2xlZXBfcmFuZ2UoKS4gQnV0IHNpbmNlIHRoZQpmdW5jdGlvbiBpcyBuZXZlciB1 c2VkIGluc2lkZSBhdG9taWMgY29udGV4dCwgdGhpcyBsb29rcyBmaW5lLgoKWW91IG1heSB3YW50 IHRvIGJ1bXAgdGhlIHNsZWVwIHRpbWUgYmV0d2VlbiByZWFkcyB0byBzb21ldGhpbmcgbGlrZSAx MApvciAyMC4gdXNsZWVwX3JhbmdlKCkgZG9lc24ndCBhbHdheXMgd29yayB3ZWxsIHdpdGggdmVy eSBzaG9ydCB2YWx1ZXMsCmFuZCBnaXZlbiB0aGF0IHlvdSBhbHJlYWR5IGJ1bXAgdGhlIHRpbWVv dXQgZnJvbSAyIG1zIHRvIDUgbXMgaW5kaWNhdGVzCnRvIG1lIHRoYXQgd2UncmUgYWN0dWFsbHkg c3BlbmRpbmcgYSBsb3Qgb2YgdGltZSBpbiB0aGlzIGxvb3AsIGFuZAppdGVyYXRpbmcgc29tZXdo ZXJlIGJldHdlZW4gMjAwMCBhbmQgNTAwMCB0aW1lcyBpc24ndCBhbnkgZ29vZC4gV2Ugb25seQp1 c2UgdGhpcyBmdW5jdGlvbiB0byB3YWl0IGZvciB0aGUgVVNCX1BIWV9DTEtfVkFMSUQgYml0IHdo aWNoIGhhcHBlbnMKZHVyaW5nIGNsb2NrIGVuYWJsZSBhbmQgZGlzYWJsZSwgd2hpY2ggaXNuJ3Qg Z29pbmcgdG8gYmUgdmVyeSBvZnRlbiwgc28KZXZlbiBpbiB0aGUgYmVzdCBjYXNlICh3aGVyZSB0 aGUgY2xvY2sgaXMgaW1tZWRpYXRlbHkgdmFsaWQpIHRoZXJlJ3Mgbm8KbmVlZCB0byByZXR1cm4g d2l0aGluIGEgbWljcm9zZWNvbmQuCgpUaGllcnJ5Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752487AbdLKJx3 (ORCPT ); Mon, 11 Dec 2017 04:53:29 -0500 Received: from mail-qt0-f196.google.com ([209.85.216.196]:32873 "EHLO mail-qt0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751967AbdLKJx0 (ORCPT ); Mon, 11 Dec 2017 04:53:26 -0500 X-Google-Smtp-Source: AGs4zMY67piq6JjHBcalU5s2bobHzqs4d6qjb9Mt0q9INRnLGcyYeAbjrKjW83oTSU3YrPztmoXRBg== Date: Mon, 11 Dec 2017 10:53:22 +0100 From: Thierry Reding To: Dmitry Osipenko Cc: Felipe Balbi , Jonathan Hunter , linux-usb@vger.kernel.org, linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1] usb: phy: tegra: Increase PHY clock stabilization timeout Message-ID: <20171211095322.GC10671@ulmo> References: <20171210225535.8532-1-digetx@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="MnLPg7ZWsaic7Fhd" Content-Disposition: inline In-Reply-To: <20171210225535.8532-1-digetx@gmail.com> User-Agent: Mutt/1.9.1 (2017-09-22) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --MnLPg7ZWsaic7Fhd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Dec 11, 2017 at 01:55:35AM +0300, Dmitry Osipenko wrote: > This fixes "utmi_phy_clk_enable: timeout waiting for phy to stabilize" > error message. >=20 > Signed-off-by: Dmitry Osipenko > --- > drivers/usb/phy/phy-tegra-usb.c | 13 ++++--------- > 1 file changed, 4 insertions(+), 9 deletions(-) >=20 > diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-= usb.c > index f668bfb708d3..7d5db625f800 100644 > --- a/drivers/usb/phy/phy-tegra-usb.c > +++ b/drivers/usb/phy/phy-tegra-usb.c > @@ -16,7 +16,7 @@ > #include > #include > #include > -#include > +#include > #include > #include > #include > @@ -305,14 +305,9 @@ static int utmip_pad_power_off(struct tegra_usb_phy = *phy) > =20 > static int utmi_wait_register(void __iomem *reg, u32 mask, u32 result) > { > - unsigned long timeout =3D 2000; > - do { > - if ((readl(reg) & mask) =3D=3D result) > - return 0; > - udelay(1); > - timeout--; > - } while (timeout); > - return -1; > + u32 tmp; > + > + return readl_poll_timeout(reg, tmp, (tmp & mask) =3D=3D result, 1, 5000= ); Technically I think this should be readl_poll_timeout_atomic() because the above used to use udelay() instead of usleep_range(). But since the function is never used inside atomic context, this looks fine. You may want to bump the sleep time between reads to something like 10 or 20. usleep_range() doesn't always work well with very short values, and given that you already bump the timeout from 2 ms to 5 ms indicates to me that we're actually spending a lot of time in this loop, and iterating somewhere between 2000 and 5000 times isn't any good. We only use this function to wait for the USB_PHY_CLK_VALID bit which happens during clock enable and disable, which isn't going to be very often, so even in the best case (where the clock is immediately valid) there's no need to return within a microsecond. Thierry --MnLPg7ZWsaic7Fhd Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAlouVZAACgkQ3SOs138+ s6EPghAAv6vPAwumYNXxv8GF2DdFzg14baJqXarudQZxRtk5fTyUNuaeAKNjuEFy rrr4ACMYio4k/hk2axFufES+4MdXknNzQg/1gpNkLYZQvU4yYLLqd69NhFZonPDZ 4hyUgb5J+PYZFWEngpBliRbZarBPmTZQ4SsnnB+ZZ6zzrl8QOoUsfJ/wWz2KHA61 sL0dOf1odD1ii+3hdDBF4n0vjas0qyeEHqVwrrbA1cM2N2bu9jwpd7PZJmDAhioQ 2MtaAJBRaUadCOuNRUcPODvRgfiYw2DKplVPnNUgNx5OUWhoUSH3Kny7jCF63kTG 1xwOUvAwFT9smDac1psv9WIehTAtZLXFZ9Jc9P7LuU4vMVvmUMEaVXac+AyGwhil 2VnMdPnPBIaC++gPlOtSOr1oJPQD8lduzKf2A1NukAbPUM8FVmwcVv3zyo6lVw3o WPKR+BgxSnYKdL1q1upEoI0e6xrTJXDVp+FxK8MnA2xXQPrzZEfb8bFuJjKZj9Ei i2DT9VjXon9dahe3wAEsCFIL+JdQyRrMZ79xoIux0TNwKS7Bv1EcMzYL+iI8CgeA iu1+lbTJ5eFmufR+xI+BO6xiCHmxqJb+N/c9mXwl6lYrwlWvyNhNtBEJCEpLE9zX 82daFdIw4S8Q0D4SMJqN5keVJbb6zPRXBgCcgFdhAG9GJQy25Qk= =DwOm -----END PGP SIGNATURE----- --MnLPg7ZWsaic7Fhd--