Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754414AbcJML0Y (ORCPT ); Thu, 13 Oct 2016 07:26:24 -0400 Received: from mga01.intel.com ([192.55.52.88]:43779 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754164AbcJML0O (ORCPT ); Thu, 13 Oct 2016 07:26:14 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,339,1473145200"; d="asc'?scan'208";a="19626614" From: Felipe Balbi To: Baolin Wang Cc: Greg KH , Mark Brown , USB , LKML Subject: Re: [PATCH v2] usb: dwc3: gadget: Wait for end transfer complete before free irq In-Reply-To: References: <87r37kviic.fsf@linux.intel.com> <87inswvg80.fsf@linux.intel.com> <87a8e8vaif.fsf@linux.intel.com> <877f9cv7no.fsf@linux.intel.com> Date: Thu, 13 Oct 2016 14:23:37 +0300 Message-ID: <87y41struu.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3085 Lines: 78 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Baolin Wang writes: >>> Baolin Wang writes: >>>>>>> I'm thinking this is a bug in configfs interface of Gadget API, not >>>>>>> dwc3. The only reason for this to happen would be if we get to >>>>>>> ->udc_stop() with endpoints still enabled. >>>>>>> >>>>>>> Can you check if that's the case? i.e. can you check if any endpoin= ts >>>>>>> are still enabled when we get here? >>>>>> >>>>>> No, it is not any endpoints are still enabled. Like I said in commit >>>>>> message, we will start end transfer command when disable the endpoin= t, >>>>>> if the end transfer command complete event comes after we have freed >>>>>> the gadget irq, it will trigger the interrupt line all the time to >>>>>> crash the system. >>>>> >>>>> I see what the problem is. Databook tells us we *must* set CMDIOC when >>>>> issuing EndTransfer command and we should always wait for Command >>>>> Complete IRQ. However, we took a shortcut and just delayed for 100us >>>>> after issuing End Transfer. >>>> >>>> Yes, but 100us delay is not enough in some scenarios, like changing >>>> function with configfs frequently, we will met problems. >>> >>> heh, 100us *is* enough. However we still get an IRQ because we requested >>> for it. If you want to fix this, then you need to find a way to get rid >>> of that 100us udelay() and add a proper wait_for_completion() to delay >>> execution until command complete IRQ fires up. >> >> I haven't tested this yet, but it shows the idea (I think we might still >> have a race with ep_queue if we're still waiting for End Transfer, but > > Yes, maybe we need check DWC3_EP_END_TRANSFER_PENDING flag when > queuing one request. Yeah, I'll add that check later. I still need to make sure we would still try to kick any transfers that might have been queued while waiting for End Transfer Command Complete IRQ. >> that's easy to sort out by checking for DWC3_EP_END_TRANSFER_PENDING >> before calling kick_transfer). Could you have a look and, perhaps, run a >> test? > > Sure. I will test it and send out the result tomorrow. Thank you =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBCAAGBQJX/265AAoJEMy+uJnhGpkGtGcQAMtw8q1QHgGS40LnAOVAJqRx aETA1MdIRS4Whp4/vt+/kupVginMYT7z8HNayKRJQV4GHXqgsQ10G95XHfr9EpuF fTdLb2k4ch5/575d0vkF/loV38k7wVDhNDznu7Tk8C+KKrG6mjqluSJI6/Bm0RnL EjkNv1mM5YHRdMjvgPDvME+t44OrnFvM9ioQM0cHt1GB+z70GQZp/737CpoTUDVh /KN9mWLJetL7/9a0mkNEDRXe8HnvUR0R9XhoF5QsKY4CyMLR16XL5SlH/fVfl3qh a4wQU7D4nBJ2oklj7Lnmn09Eg7IfXFD78wj0lTF9yUm//HEUPopwO7/3EcGmyXdb C0KQm5yWRznizcyF+3VCVUiNOqToHzlVFzEJoPryBDkC/sWAPsiDAAV8B6POOwoZ 8FsHoBrboOfwySnDjr4sguj/OLb6Zk4un7vrF/RFI4j3Ewi1GJbHrlUg3P6tG7O8 pXAgVrm3oGXvWifJb26DkJ3E3KNfUq/fWLMrBdEtJzjKll1vuJrsyrGH74LKxefH apdAN1y+9+FMh0pJfCPi8yh/2AA1lVdtpr6TgPY2hmyg0hk/Mihye8xzvAndJkHT xUP+1i21RO3G0qjtAKF0+8ZoOZlfzLpsnv2Jqn1VoMue8KuIwH8IOVr1lfYDAdkJ m0e3Dg9D5C36xDxU+he9 =8kEg -----END PGP SIGNATURE----- --=-=-=--