From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com ([134.134.136.100]:23911 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750752AbeAKJb7 (ORCPT ); Thu, 11 Jan 2018 04:31:59 -0500 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" , Heikki Krogerus Subject: Re: [PATCH] usb: dwc3: core: Don't try to get PHYs during suspend/resume In-Reply-To: References: <1515589914-23460-1-git-send-email-rogerq@ti.com> <0c2c7e45-9324-316a-d44b-dd17a3a2c68b@ti.com> <87373dzvmi.fsf@linux.intel.com> <327eac7d-80e2-bbd6-4fb4-98d947335698@ti.com> <87wp0pyfmo.fsf@linux.intel.com> <610c35d0-31b9-0c51-81e4-9f0d1daf5c51@ti.com> <87inc8yf7k.fsf@linux.intel.com> Date: Thu, 11 Jan 2018 11:31:45 +0200 Message-ID: <877esoyc5a.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Sender: stable-owner@vger.kernel.org List-ID: --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Roger Quadros writes: >> Roger Quadros writes: >>>>>> - ret =3D dwc3_core_soft_reset(dwc); >>>>>> + ret =3D dwc3_core_get_phy(dwc); >>>>> >>>>> we can get_phy in dwc3_core_init() as it will get called on resume(). >>>>> This was the $subject of this patch. >>>> >>>> indeed. thanks :-) >>>> >>> >>> oops sorry. I meant we can't call dwc3_core_get_phy() in dwc3_core_init= (). :P >>=20 >> bit of a chicken-and-egg problem. We need to setup the PHY interface >> before getting the PHYs, but can't get PHY during resume. Maybe the best >> way here would be to check for the pointers being valid. Something like: >>=20 >> if (!phy) >> get_phy(); >>=20 > > OK that should take care of not calling get_phy() on suspend. > However there is one more issue with the approach > >> @@ -754,15 +754,15 @@ static int dwc3_core_init(struct dwc3 *dwc) >> dwc->maximum_speed =3D USB_SPEED_HIGH; >> } >>=20=20 >> - ret =3D dwc3_core_get_phy(dwc); >> + ret =3D dwc3_phy_setup(dwc); >> if (ret) >> goto err0; > > here we configure PHY related bits and register the ulpi interface. > >>=20=20 >> - ret =3D dwc3_core_soft_reset(dwc); >> + ret =3D dwc3_core_get_phy(dwc); >> if (ret) >> goto err0; >>=20=20 > > we got the PHYs. all OK here. > >> - ret =3D dwc3_phy_setup(dwc); >> + ret =3D dwc3_core_soft_reset(dwc); >> if (ret) >> goto err0; > > Now we do a soft reset. This means we loose the PHY configuration bits th= at we did > in dwc3_phy_setup. So we need to call dwc3_phy_setup again but not re-reg= ister the ulpi interface. > I can use a flag there so that dwc3_ulpi_init() is done only once. sounds like it's better to extract out a smaller function that just checks if we need ULPI bus and registers it, something akin to: @@ -482,6 +482,21 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc) parms->hwparams8 =3D dwc3_readl(dwc->regs, DWC3_GHWPARAMS8); } =20 +static int dwc3_ulpi_init(struct dwc3 *dwc) +{ + int intf; + + intf =3D DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3); + + if (intf =3D=3D DWC3_GHWPARAMS3_HSPHY_IFC_ULPI || + (intf =3D=3D DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI && + dwc->hsphy_interface && + !strncmp(dwc->hsphy_interface, "ulpi", 4))) + return dwc3_ulpi_init(dwc); + + return 0; +} + /** * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core * @dwc: Pointer to our controller context structure @@ -563,11 +578,6 @@ static int dwc3_phy_setup(struct dwc3 *dwc) break; } /* FALLTHROUGH */ =2D case DWC3_GHWPARAMS3_HSPHY_IFC_ULPI: =2D ret =3D dwc3_ulpi_init(dwc); =2D if (ret) =2D return ret; =2D /* FALLTHROUGH */ default: break; } Then we just call that outside of any functions that get called during PM. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlpXLwEACgkQzL64meEa mQbP5g/+M4Qf5+IGkPyeXxTVmmVGt8HavxVL/MwHUHAUdluHgflU6zoqwN4c8Dfy Ahh7s3XWb2beA1+t7B1teYu9yiifVXtWi9hdQpH5QCExEzhe0ToIVY00cU4z34m/ Qh5FYAe3XxI5uFo8OPIXxT2GiOX4Fy4u4w4YyGXausk0McuN9hn07t72slcSvlpr mHBt5mOgIwGUSB3anX9/oVA/T9QjdntFMhbgF2TgHgZBcoIjW1GXIoCLOPceghOi itUwcTcOB014je+DXpK46U+bhZRLtJCtAqdseO5zy09l3YczF0vv2wHA9314P3I4 BC+irLWFy4vuIr4NK0tOygVyHYQuK/iA+jhXP+O+SISubzJEGM4mLm2vA+vll/8M pbMQKo43z0DJI+sjfJc4h3XQewij0IKU0yY8D9WHxs+dj87vKK96Hd5fUhVtak9T CBttVGMlLOf9uoCL/VVCA4MwQcQQc5x3r8ek27lg5cj2rytQw7HAObbbvG9Lgqn0 MIMMLBqG5oZZrThCYoYB0zVA1MgpURGSU/aVoaDUyCJlZmoXyCH+T1TWpWnlAnO1 WRRhQ3F/hHpdUHlnSuN9h9HpF3A08nzpuG3H/Md9Lies82K+ijf1+MCTCdd/si48 xL6raK+6By2JiEzG7DD0rnsL0O8R1DjH/TrVx//HpIwN94oYv/8= =QDsy -----END PGP SIGNATURE----- --=-=-=--