From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755543AbcISJ7g (ORCPT ); Mon, 19 Sep 2016 05:59:36 -0400 Received: from mga02.intel.com ([134.134.136.20]:64843 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754765AbcISJ71 (ORCPT ); Mon, 19 Sep 2016 05:59:27 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,361,1470726000"; d="asc'?scan'208";a="881386972" From: Felipe Balbi To: Baolin Wang Cc: Greg KH , Mark Brown , USB , LKML Subject: Re: [PATCH v2 2/2] usb: dwc3: Wait for control tranfer completed when stopping gadget In-Reply-To: References: <3a4c91231787d31e82874161aa2ef9351f1c4b73.1473405255.git.baolin.wang@linaro.org> <871t0tgwsc.fsf@linux.intel.com> User-Agent: Notmuch/0.22.1+63~g994277e (https://notmuchmail.org) Emacs/25.1.3 (x86_64-pc-linux-gnu) Date: Mon, 19 Sep 2016 12:58:31 +0300 Message-ID: <87eg4g43eg.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-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Baolin Wang writes: >>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>> index 1a33308..c9026ce 100644 >>> --- a/drivers/usb/dwc3/gadget.c >>> +++ b/drivers/usb/dwc3/gadget.c >>> @@ -1441,6 +1441,15 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc= , int is_on, int suspend) >>> if (pm_runtime_suspended(dwc->dev)) >>> return 0; >>> >>> + /* >>> + * Per databook, when we want to stop the gadget, if a control tr= ansfer >>> + * is still in process, complete it and get the core into setup p= hase. >>> + */ >>> + if (!is_on && dwc->ep0state !=3D EP0_SETUP_PHASE) { >>> + reinit_completion(&dwc->ep0_completed); >> >> this seems unnecessary to me. Also, why return here so the caller has to > > We should re-init the completion due to it will complete control > transfer many times before we try to stop gadget. not sure I get this comment, care to furter explain what you mean? >> wait? You could just have called wait_for_completion() here straight >> away: >> >> if (!is_on && dwc->ep0state !=3D EP0_SETUP_PHASE) { >> /* should this be interruptible? */ >> ret =3D wait_for_completion_timeout(&dwc->ep0_in_setup, >> msecs_to_jiffies(500)); >> if (ret =3D=3D 0) { >> dwc3_trace(trace_dwc3_gadget, "RUN/STOP timeout"= ); >> return -ETIMEDOUT; >> } >> } >> >> There's also no need for that "try_again" trickery. We either can halt >> the controller within 500ms or we cannot. > > But this is in atomic context and we can not issue > wait_for_completion_timeout() in atomic context, then we should just > return here. heh, good point. Missed that :-) =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBCAAGBQJX37bHAAoJEMy+uJnhGpkGl0UQAM2hoUIxEBg100RegYakay4V EniEhDKuCTLekwoImx2JhVVdsHvQs6mjbK4F94+k5sDvWo6o0tZZVn9eFxqy6uIw jl14z8VibZ32BWEG7R3UPVIoYkAorIn3a5mcx0N2EAjBYXTXMwahKlLx63J8IpLq Qedax7bNfcdqAIs5HzuCEyYY8pxDL52JKu+FHmAYNXbyfOuoCf/0Uccfi5l76Gri i9xizpA5tC72MoBOUoNB2w0t7YY+OOWCRBMf/Gw4GsW1PEDt34Y4XKP8+uOyt20F dwkeypKJYibc8Ka0Hy1BBm4ijalvSXnHDF2ne4JcXbXQB0hKhB1BCOKlSWhtfA+R 9IVBu+hUpYXrQzfVuNZVhcoak5nMrAWgRTyf6+Cm3rEeyRE0aMz9xoQrnGYPj8vg VYf4432KoptNDAbW500NtRScsZzBxsoxNuGpcNdqt6UP1ktHWw7XyZ54MzNryyfU xcMtT+aGtBhLOSfyzVFVNKjHFlVNyuWoKmghU47izVR7+eqy1aLValErJkjX00ap MK+NowLPNhhClNFn0hPGp/Ai6lAcyFYWtlwoZT2vs5Uzs0HFFBSmuU72lq7Jv3PV vR8QSFx+MNwmABPnXIEgNxWweQKsVTPEMawEv7m7Yb0uxygwzJmTq4qnzmZN/1w1 8YmeoeosubWtz+yb9HeZ =7jsI -----END PGP SIGNATURE----- --=-=-=--