From mboxrd@z Thu Jan 1 00:00:00 1970 From: Felipe Balbi Subject: Re: [RFC PATCH 3/3] usb: phy: msm: Do not do runtime pm if the phy is not idle Date: Mon, 30 Jun 2014 12:13:05 -0500 Message-ID: <20140630171305.GF31442@saruman.home> References: <1403110801-3790-1-git-send-email-srinivas.kandagatla@linaro.org> <1403110868-3924-1-git-send-email-srinivas.kandagatla@linaro.org> <20140627155440.GK8069@saruman.home> <53B13AAF.7060909@linaro.org> Reply-To: Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="xaMk4Io5JJdpkLEb" Return-path: Content-Disposition: inline In-Reply-To: <53B13AAF.7060909-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Sender: linux-usb-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Srinivas Kandagatla Cc: balbi-l0cyMroinI0@public.gmane.org, linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Greg Kroah-Hartman , linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-arm-msm@vger.kernel.org --xaMk4Io5JJdpkLEb Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jun 30, 2014 at 11:23:43AM +0100, Srinivas Kandagatla wrote: > Hi Felipe, >=20 > On 27/06/14 16:54, Felipe Balbi wrote: > >Hi, > > > >On Wed, Jun 18, 2014 at 06:01:08PM +0100, Srinivas Kandagatla wrote: > >>Use case is when the phy is configured in host mode and a usb device is > >>attached to board before bootup. On bootup, with the existing code and > >>runtime pm enabled, the driver would decrement the pm usage count > >>without checking the current state of the phy. This pm usage count > >>decrement would trigger the runtime pm which than would abort the > >>usb enumeration which was in progress. In my case a usb stick gets > >>detected and then immediatly the driver goes to low power mode which is > >>not correct. > >> > >>log: > >>[ 1.631412] msm_hsusb_host 12520000.usb: EHCI Host Controller > >>[ 1.636556] msm_hsusb_host 12520000.usb: new USB bus registered, ass= igned bus number 1 > >>[ 1.642563] msm_hsusb_host 12520000.usb: irq 220, io mem 0x12520000 > >>[ 1.658197] msm_hsusb_host 12520000.usb: USB 2.0 started, EHCI 1.00 > >>[ 1.659473] hub 1-0:1.0: USB hub found > >>[ 1.663415] hub 1-0:1.0: 1 port detected > >>... > >>[ 1.973352] usb 1-1: new high-speed USB device number 2 using msm_hs= usb_host > >>[ 2.107707] usb-storage 1-1:1.0: USB Mass Storage device detected > >>[ 2.108993] scsi0 : usb-storage 1-1:1.0 > >>[ 2.678341] msm_otg 12520000.phy: USB in low power mode > >>[ 3.168977] usb 1-1: USB disconnect, device number 2 > >> > >>This issue was detected on IFC6410 board. > >> > >>This patch fixes the intial runtime pm trigger by checking the phy > >>state and decrementing the pm use count only when the phy state is IDLE. > >> > >>Signed-off-by: Srinivas Kandagatla > >>--- > >> drivers/usb/phy/phy-msm-usb.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >>diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-us= b.c > >>index 3bb559d..78cc870 100644 > >>--- a/drivers/usb/phy/phy-msm-usb.c > >>+++ b/drivers/usb/phy/phy-msm-usb.c > >>@@ -1229,7 +1229,9 @@ static void msm_otg_sm_work(struct work_struct *w) > >> motg->chg_state =3D USB_CHG_STATE_UNDEFINED; > >> motg->chg_type =3D USB_INVALID_CHARGER; > >> } > >>- pm_runtime_put_sync(otg->phy->dev); > >>+ > >>+ if (otg->phy->state =3D=3D OTG_STATE_B_IDLE) > >>+ pm_runtime_put_sync(otg->phy->dev); > > > >instead, you might as well just use autosuspend. >=20 > autosuspend is a good idea and will provide a delay before rumtime suspen= d, > however the bug which is fixed here still needs to be fixed anyway. >=20 > runtime PM in this driver is not that great, the driver just increments t= he > pm use count if the phy is configured in host or deivce mode. This patch > fixes a bug in its state-machine which decrements pm use-count without > checking the state. >=20 > As this is a PHY driver, am not sure moving to autosuspend means we would > end up just adding some static inactivity delay? Is it really going to add > any value to this PHY driver? yeah, perhaps not... I guess we can apply this as a fix; you're right. --=20 balbi --xaMk4Io5JJdpkLEb Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJTsZqhAAoJEIaOsuA1yqREN8kP/AuwMduYqVn0Ekeda5Zq9Vbf ViVaemzrl6jo2B3//Ss3qFZRzNBNstdQLAz395k2rNgCFZV4O+dd77Rg3oc0wri0 u4gVL597a6wuZg4tbLGomXDY0mF5csL/UjYvCtPFHwORxlvxYY4TIID8R7Ptsgo/ UTTE946uQvMsspNhskWcIJci3F0PQjLv9qCclPnpN+kCCTWt4jENFeG1f4/JtOMi qN4tNQZjeye6oE5SShqwMgk1h4jYhyvqCca03XUcpTGe7XdbZj/6tUyafoQokmFr mNYgMxiUQztCZ5pU9IZQztXR4wOprW9tjYCZpf3c1/d29VkX7MZfW2URXJXPddWy smclhrFBLKkEMNwJbicoW7YuvKVFZ5UhFksK9Ua9vsDmtFPTNpc8RrwLBdmMYNov F9zuMnBcb+cv3AJiS9JJHKUJ8w9PpDZAiYmwcec4EmIPL03ROeyOyRM8K6eUy9L7 L4ji0UYhtCnnMwHsta1xxPhqUNtCd+dgV7W/tk299xFvvcKwJmpvp70AwvvJODPE 0VFQYm1HDE6GvYi8zMghgK2IH7Qn4/6wotbAmB5Ajx+U299/ptXVA0i/5REJ+wOd V8DUwF9T+TyYIG3yIfyuVGALPZGxVgxFAFHDnX46HjJ6RDfuqWTy8LhyfaFwsuDf 9iwbE3Nw71dRzSln/rsv =mxvT -----END PGP SIGNATURE----- --xaMk4Io5JJdpkLEb-- -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html