From mboxrd@z Thu Jan 1 00:00:00 1970 From: Felipe Balbi Subject: Re: [PATCH] usb: dwc3: gadget: Fix broken gadget on system suspend/resume Date: Wed, 12 Nov 2014 09:54:41 -0600 Message-ID: <20141112155441.GI641@saruman> References: <1415804296-28980-1-git-send-email-rogerq@ti.com> <20141112150809.GE641@saruman> <54637D70.7060208@ti.com> <20141112153640.GH641@saruman> <54637F8B.8040508@ti.com> Reply-To: Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="EVh9lyqKgK19OcEf" Return-path: Content-Disposition: inline In-Reply-To: <54637F8B.8040508@ti.com> Sender: linux-kernel-owner@vger.kernel.org To: Roger Quadros Cc: balbi@ti.com, linux-usb@vger.kernel.org, linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org List-Id: linux-omap@vger.kernel.org --EVh9lyqKgK19OcEf Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 12, 2014 at 05:40:59PM +0200, Roger Quadros wrote: > On 11/12/2014 05:36 PM, Felipe Balbi wrote: > > Hi, > >=20 > > On Wed, Nov 12, 2014 at 05:32:00PM +0200, Roger Quadros wrote: > >> On 11/12/2014 05:08 PM, Felipe Balbi wrote: > >>> On Wed, Nov 12, 2014 at 04:58:16PM +0200, Roger Quadros wrote: > >>>> On TI SoCs (e.g. DRA7) we don't support the DWC3 hibernation feature. > >>>> We need to stop the gadget controller while system suspend > >>>> else it results in L3 Bus errors on resume with broken > >>>> USB gadget on J6-evm. > >>>> > >>>> [ 55.718226] WARNING: CPU: 0 PID: 0 at drivers/bus/omap_l3_noc.c:1= 47 l3_interrupt_handler+0x224/0x31c() > >>>> [ 55.718232] 44000000.ocp:L3 Custom Error: MASTER USB3 TARGET GPMC= (Idle): Data Access in User mode during Functional access > >>>> [ 55.718263] Modules linked in: usb_f_ss_lb g_zero libcomposite co= nfigfs xhci_hcd btwilink bluetooth dwc3 6lowpan_iphc m25p80 dwc3_omap omap_= remoteproc > >>>> [ 55.718271] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.14.19-020= 11-g3ece3ca-dirty #658 > >>>> [ 55.718290] [] (unwind_backtrace) from [] (sh= ow_stack+0x10/0x14) > >>>> [ 55.718302] [] (show_stack) from [] (dump_sta= ck+0x78/0x94) > >>>> [ 55.718315] [] (dump_stack) from [] (warn_slo= wpath_common+0x6c/0x90) > >>>> [ 55.718325] [] (warn_slowpath_common) from []= (warn_slowpath_fmt+0x30/0x40) > >>>> [ 55.718336] [] (warn_slowpath_fmt) from [] (l= 3_interrupt_handler+0x224/0x31c) > >>>> [ 55.718348] [] (l3_interrupt_handler) from []= (handle_irq_event_percpu+0x5c/0x23c) > >>>> [ 55.718358] [] (handle_irq_event_percpu) from [] (handle_irq_event+0x3c/0x5c) > >>>> [ 55.718367] [] (handle_irq_event) from [] (ha= ndle_fasteoi_irq+0x98/0x158) > >>>> [ 55.718377] [] (handle_fasteoi_irq) from [] (= generic_handle_irq+0x20/0x30) > >>>> [ 55.718385] [] (generic_handle_irq) from [] (= handle_IRQ+0x4c/0xb0) > >>>> [ 55.718393] [] (handle_IRQ) from [] (gic_hand= le_irq+0x28/0x5c) > >>>> [ 55.718402] [] (gic_handle_irq) from [] (__ir= q_svc+0x44/0x5c) > >>>> [ 55.718406] Exception stack(0xc09c1f68 to 0xc09c1fb0) > >>>> [ 55.718412] 1f60: 00000001 00000001 00000000 00= 000000 c09c0000 00000000 > >>>> [ 55.718418] 1f80: c09c0000 c09c0000 c0a7a1e4 c09c8938 c09c89b4 00= 000000 60000093 c09c1fb0 > >>>> [ 55.718423] 1fa0: c008a2e4 c00926cc 60000013 ffffffff > >>>> [ 55.718433] [] (__irq_svc) from [] (cpu_start= up_entry+0x100/0x1f4) > >>>> [ 55.718444] [] (cpu_startup_entry) from [] (s= tart_kernel+0x324/0x388) > >>>> > >>>> Signed-off-by: Roger Quadros > >>>> --- > >>>> drivers/usb/dwc3/gadget.c | 8 +++++++- > >>>> 1 file changed, 7 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > >>>> index 3818b26..bc15b54 100644 > >>>> --- a/drivers/usb/dwc3/gadget.c > >>>> +++ b/drivers/usb/dwc3/gadget.c > >>>> @@ -2741,7 +2741,13 @@ int dwc3_gadget_prepare(struct dwc3 *dwc) > >>> > >>> there is no more dwc3_gadget_prepare(), please rebase on 'next'. > >> > >> right. > >> > >>> > >>>> { > >>>> if (dwc->pullups_connected) { > >>>> dwc3_gadget_disable_irq(dwc); > >>>> - dwc3_gadget_run_stop(dwc, true, true); > >>>> + if (dwc->has_hibernation) { > >>>> + dwc3_gadget_run_stop(dwc, true, true); > >>>> + } else { > >>>> + dwc3_gadget_run_stop(dwc, false, true); > >>>> + /* remember to connect back on resume */ > >>>> + dwc->pullups_connected =3D true; > >>>> + } > >>> > >>> Another thing you probably want to do is make sure there is nothing > >>> pending on our request list because if there is, then we need to wait > >>> for any in-flight transfers to complete before disconnecting. > >> > >> shouldn't that check be done in dwc3_gadget_run_stop()? > >=20 > > no, gadget drivers only disconnect pullups when they know there's > > nothing pending. And sometimes, we actually want to break such transfers > > to see how the thing would behave. Only suspend/resume needs to make > > transfers complete before hand. > >=20 > >>> I also wonder if current code isn't really fragile... I mean, when you > >>> clear run_stop bit, you should notify the gadget by means of > >>> ->disconnect() and so on... But since we can't test this with mainlin= e, > >>> then it's difficult to tell. > >>> > >> that is true. > >=20 > > Not sure what to do with this patch :-p > >=20 > :). Maybe we can wait till we have suspend/resume working in mainline. Perhaps... hopefully we won't forget about this :-) --=20 balbi --EVh9lyqKgK19OcEf Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUY4LBAAoJEIaOsuA1yqREiM8QALKFTk/swBHciqE+CzUsXOyr eRjHynVdQD4VlLzqWXI5DK0UyyAzB3QLEOO4iDsytVG+w/pUMPn4Qa+YL0MNJJ2g 1FhIbyrgiY4ja2yvwYGFOkJg6Jd/DQiPwSnYJzVuc/cODv5JA0/T8ZFrJwZVg31S nIczW+wy2G2Kup85isXWb2+z675mCb/yyTYnHivLdbelNcUrtwradrLKLM89+g10 yl9+zOI2apwPvmYPoYyqqujCu279m+nDPLBGKqglQPuzuZKvevPeVIqhT7mBERcP qym3iwhXo5qj/BExhAMmAwdRq6qhWwMDWYezYYo0aPlrzdcIqvT/2SfbJyLrFSQy A+xe38480OoRKVPMYdHXTbKZ2g1NSCaX6nckblxO9siC99MBWZ1fCJcyvb2QZBqf TZCqo/XysZkvhPPEC7ZdPfc/Xg2iIxpVW88D2G44LjT1jmP6+AcZEu10KDBh+7p9 yKi0iWBEwuyztRY+4aEbA+HZsCZVbvBLDWL1xiCneyP5jZsWOvEFRGitNSsCXhWS RpibQjjMcKhiYNwE39N5heAmI+YjT3vqUV4dpZVdn6BlIZeTenoC0VIMynIQr9hu G0a3qdBY5nYqu+53luQl6D5Eb5wGCvQcy6thS+wI4hAksJeIA2Xx02DYKrX2nir4 eXbyx2d6PDcekNcJIbj8 =p/G7 -----END PGP SIGNATURE----- --EVh9lyqKgK19OcEf--