From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755254AbcCBM7n (ORCPT ); Wed, 2 Mar 2016 07:59:43 -0500 Received: from mga11.intel.com ([192.55.52.93]:46851 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753334AbcCBM7m (ORCPT ); Wed, 2 Mar 2016 07:59:42 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,528,1449561600"; d="asc'?scan'208";a="58338324" From: Felipe Balbi To: Mathias Nyman , Lu Baolu , Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v7 07/10] usb: dbc: handle endpoint stall In-Reply-To: <56D6E380.3060800@linux.intel.com> References: <1453813096-6991-1-git-send-email-baolu.lu@linux.intel.com> <1453813096-6991-8-git-send-email-baolu.lu@linux.intel.com> <56D6E380.3060800@linux.intel.com> User-Agent: Notmuch/0.21 (http://notmuchmail.org) Emacs/25.0.50.2 (x86_64-pc-linux-gnu) Date: Wed, 02 Mar 2016 14:58:59 +0200 Message-ID: <87h9gp8298.fsf@ti.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; 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, Mathias Nyman writes: > [ text/plain ] > On 26.01.2016 14:58, Lu Baolu wrote: >> In case of endpoint stall, software is able to detect the situation >> by reading DCCTRL.HIT or DCCTRL.HOT bits. DbC follows the normal USB >> framework to handle endpoint stall. When software detects endpoint >> stall situation, it should wait until endpoint is recovered before >> read or write oprations. >> >> Signed-off-by: Lu Baolu >> --- >> drivers/usb/early/xhci-dbc.c | 36 ++++++++++++++++++++++++++++++++++++ >> 1 file changed, 36 insertions(+) >> >> diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c >> index c81df40..344d93e 100644 >> --- a/drivers/usb/early/xhci-dbc.c >> +++ b/drivers/usb/early/xhci-dbc.c >> @@ -1163,6 +1163,37 @@ static int xdbc_wait_until_dbc_configured(void) >> return -ETIMEDOUT; >> } >> >> +static int xdbc_wait_until_epstall_cleared(bool read) >> +{ >> + int timeout =3D 0; >> + >> + if (read) { >> + do { >> + if (!(readl(&xdbcp->xdbc_reg->control) & CTRL_HIT)) { >> + xdbcp->in_ep_state =3D EP_RUNNING; >> + >> + return 0; >> + } >> + >> + xdbcp->in_ep_state =3D EP_HALTED; >> + xdbc_udelay(10); >> + } while (timeout++ < XDBC_LOOPS); >> + } else { >> + do { >> + if (!(readl(&xdbcp->xdbc_reg->control) & CTRL_HOT)) { >> + xdbcp->out_ep_state =3D EP_RUNNING; >> + >> + return 0; >> + } >> + >> + xdbcp->out_ep_state =3D EP_HALTED; >> + xdbc_udelay(10); >> + } while (timeout++ < XDBC_LOOPS); >> + } >> + >> + return -ETIMEDOUT; >> +} >> > > how about something like: > > enum xdbc_ep_state *ep_state; > u32 halt_bit; > > if (read) { > ep_state =3D &xdbcp->in_ep_state > halt_bit =3D CTRL_HIT > } else { > ep_state =3D &xdbcp->out_ep_state > halt_bit =3D CTRL_HOT > } > do { > if (!(readl(..) & halt_bit)) { > *ep_state =3D EP_RUNNING; > return 0; > } > *ep_state =3D EP_HALTED; > delay() > } while (..) I'll agree, this looks better. Might also want to refactor the handshake loop to its own function: static int xdbg_ep_state_handshake(enum xdbc_ep_state *ep_state, u32 halt_b= it) { do { if (!(readl(...) & halt_bit)) { ... } *ep_state =3D EP_HALTED; delay(...); } while (...) >> @@ -1182,6 +1213,11 @@ static int xdbc_bulk_transfer(void *data, int siz= e, int loops, bool read) >> return -EPERM; >> } >> >> + if (xdbc_wait_until_epstall_cleared(read)) { >> + xdbc_trace("%s: endpoint not ready\n", __func__); >> + return -EPERM; > > Is -EPERM appropriate here? > > Not sure about what error codes the device side is using, but usually > HALT is set due to some Data buffer/transmission/TRB error. EIO perhaps ? > In this case the failure is that debug host failed to send a > clearFeature(EP_HALT) request in time. I haven't read the spec, but does it define a maximum time for this to happen ? =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJW1uOUAAoJEIaOsuA1yqREVsgQAIL3FWQNizz44jV+pE1gzzp9 gJk37knpR2mT3JaGE5n0hQOdRFnHVDGhTlTJh3ZTfT1chi4qbaHkkE8WAk2KV8DG axKvpy4K/Z1fGphTdEKbI4c+W+Lw3r3L3wsnu7uqLwBL0uYetY26KJhFPxLNAzkf knXlYZ15j8DWzsR2wizGbGOAcBX99RWw3d7lSO+e5/5dewYUQZo+AfU082fUhxrW vn6hLehQ1ndzhSzvGVXh2Avu6OXvfcc4Sh2f/LyBDqqIMsKREOWaUOZ7FuVw9Hlx cMyNdhoq3pSzQnQv+v38Ia9zy83UjVU3Jl5wpbjCLl9XUpqd/Uxb32AiAIVcFuub J2sYQuXL4sl51rri48br9M9bULoN5gGpLpPyZ1uSleTHlQX9mkDUSo+SahpG0DgH 85KrrW5sDqOpW+njPA9R8KIWEYbJ6zKLSzNCG6VLruw5iZv4sOpP37xTyJCIWWoS zElDYK/XRuhmmk2Of5EV7VyN3JpkdNyX3/oU4oYYSsP4mXdxjWk7MiR6kEvLDvbc 2JVGXfpDJCPkRJAwx3Cb4n96MHWGILHKexXfD5KjbijkLcOZkokxGvzziwA/eHhR LQFyd0rYrrgnwfzLwxjqe1aeiz0Zk182kKBtHswQrfl7ragyLRhbTuiiE5UdvLHE GiGKU7Uj0+N/wPn1ofcX =lKci -----END PGP SIGNATURE----- --=-=-=--