From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753837AbcCCGBC (ORCPT ); Thu, 3 Mar 2016 01:01:02 -0500 Received: from mga09.intel.com ([134.134.136.24]:41771 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751175AbcCCGBA (ORCPT ); Thu, 3 Mar 2016 01:01:00 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,531,1449561600"; d="scan'208";a="928531140" Subject: Re: [PATCH v7 07/10] usb: dbc: handle endpoint stall To: Mathias Nyman , Greg Kroah-Hartman 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> Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org From: Lu Baolu Message-ID: <56D7D31A.2090301@linux.intel.com> Date: Thu, 3 Mar 2016 14:00:58 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <56D6E380.3060800@linux.intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/02/2016 08:58 PM, Mathias Nyman wrote: > 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 = 0; >> + >> + if (read) { >> + do { >> + if (!(readl(&xdbcp->xdbc_reg->control) & CTRL_HIT)) { >> + xdbcp->in_ep_state = EP_RUNNING; >> + >> + return 0; >> + } >> + >> + xdbcp->in_ep_state = EP_HALTED; >> + xdbc_udelay(10); >> + } while (timeout++ < XDBC_LOOPS); >> + } else { >> + do { >> + if (!(readl(&xdbcp->xdbc_reg->control) & CTRL_HOT)) { >> + xdbcp->out_ep_state = EP_RUNNING; >> + >> + return 0; >> + } >> + >> + xdbcp->out_ep_state = 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 = &xdbcp->in_ep_state > halt_bit = CTRL_HIT > } else { > ep_state = &xdbcp->out_ep_state > halt_bit = CTRL_HOT > } > do { > if (!(readl(..) & halt_bit)) { > *ep_state = EP_RUNNING; > return 0; > } > *ep_state = EP_HALTED; > delay() > } while (..) Sure. I will refine this part of code. > > > + >> static int xdbc_bulk_transfer(void *data, int size, int loops, bool read) >> { >> u64 addr; >> @@ -1182,6 +1213,11 @@ static int xdbc_bulk_transfer(void *data, int size, 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. > In this case the failure is that debug host failed to send a clearFeature(EP_HALT) request in time. Yes. This is done by debug host firmware. OS needs to check and wait. This return value is used to tell the up layer what happened. -EPERM is definitely not appropriate here. I will use EIO as suggested by Felipe. > > -Mathias > Thanks, -Baolu