Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752343AbcJMHyj (ORCPT ); Thu, 13 Oct 2016 03:54:39 -0400 Received: from mga05.intel.com ([192.55.52.43]:3384 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751015AbcJMHyc (ORCPT ); Thu, 13 Oct 2016 03:54:32 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,339,1473145200"; d="asc'?scan'208";a="1069820054" 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> Date: Thu, 13 Oct 2016 10:51:59 +0300 Message-ID: <87inswvg80.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: 3111 Lines: 87 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Baolin Wang writes: > Hi, > > On 13 October 2016 at 15:02, Felipe Balbi wrote: >> >> Hi, >> >> Baolin Wang writes: >>> @@ -1742,6 +1791,17 @@ static int dwc3_gadget_stop(struct usb_gadget *g) >>> dwc->gadget_driver =3D NULL; >>> spin_unlock_irqrestore(&dwc->lock, flags); >>> >>> + /* >>> + * Since the xHCI will share the same interrupt with gadget, thus= when >>> + * free the gadget irq, it will not shutdown this gadget irq line= . Then >>> + * the gadget driver can not handle the end transfer command comp= lete >>> + * event after free the gadget irq, which will hang the system to= crash. >>> + * >>> + * So we should wait for the end transfer command complete event = before >>> + * free it to avoid this situation. >>> + */ >>> + dwc3_wait_command_complete(dwc); >> >> this doesn't make sense. We have already masked all interrupts before >> getting here. We have also, already, disabled all endpoints. > > No, you just mask the interrupts described in DEVTEN register, and you > did not disable the endpoint command complete event. True that >> 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 endpoints >> 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 endpoint, > 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. It seems as if *that* should be fixed. We should start actually waiting for Command Complete IRQ. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBCAAGBQJX/z0fAAoJEMy+uJnhGpkGgFkQAMrgTdfUonKs2OjawtZ2Mmbw mLdwf1wMm73xfs2cDL4ulO+jo/WKOaE9QSQc/uZr6GehlGrMx70/Y5HgTzGExrSK NmVd9x3p7xLkW2+AKWkTwQcLkcYqYqv0yDbk1VtKt82VMsaiMzK/vEH+uOIBdWDp wkvinNHNwPNOoD4ipocuwlECTr4QuQ1RNGWOYMO+LSq03ZADqJzxLOUPUjw4X3S3 wdfj9LTWeAVCkued+pa2/h6G2wUQLN0jGLsRgrcx4H70NI6O3CyGvluaXTyByHkY TLdYaqoN9fAHVrzvS0g9tSivqC6BEOB41Xe76FOoHFkKR30cGS7qDmcdG+noowMV fQhSDgAThUjwp+gGM9dyigN1DN3OvKcNFyiw0dBxeJMjWc/ltJjwJCRB9EvRCMN8 P367HvRwMSY6DPAfs6WZ02WZfPve9q8RC4SZbDcw27y4KLOmXzR/kKYBrbdJa86L vseMLaWk6xqGy9c+wO3ucGOYcpzdAoUuzYVfcbOkzcQlbq8tVdIyZz53arj8xruj kAdB6xVoeUgCKlKkV0nSxN70vH0fl/4SlEgW3NQDeOgsLpWCfp4/xGMqdjyg/Ewz gpd9ZWQ1mmBMHGMlSiH7LnOnsm8PGWzJ5EFkWF6+fyXa5wKCjnsIGGfSP/pnxtf6 J9ymUStVx/khXzLVYef9 =mfUF -----END PGP SIGNATURE----- --=-=-=--