From patchwork Wed Sep 24 19:18:14 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Felipe Balbi X-Patchwork-Id: 37865 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-la0-f69.google.com (mail-la0-f69.google.com [209.85.215.69]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 1C755202FD for ; Wed, 24 Sep 2014 19:18:22 +0000 (UTC) Received: by mail-la0-f69.google.com with SMTP id ty20sf5741199lab.4 for ; Wed, 24 Sep 2014 12:18:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:date:from:to:cc:subject:message-id :reply-to:references:mime-version:content-type:content-disposition :in-reply-to:user-agent:sender:precedence:list-id:x-original-sender :x-original-authentication-results:mailing-list:list-post:list-help :list-archive:list-unsubscribe; bh=JlWN40vW0HK2xLHvV5mMBmcPDaOmq4Tgh53YUOAVheY=; b=QFnWdP9xbCKO/Jt1ArNxIGBF/dDcdaGa7U/R87Tby+iHpI4BHcxVSyioxjjRMjuREB xb0whsZ8D6QuoNJbfjmA3Gx8VSv0MpO4zC2g8T/TO3otTApNMUaD5ffoD2ppRe3ymuak GVZeq71e9LnJK4KOqvj7ahKXrd+HhyzBPT222tS7g9CkjhfDy3ORHf+NjZJ8VLhO42z7 ybpfHn0b0u+4lVyZu0+PSCDWYBlVg7VUQGpYRCOYJFc7ZDNYazaXvxdSLsl68naMGpTY p8nYunhqqKZ2A5cxh0N6IiibDxJ6Wpt0ecH1nrVmVYY0X4pOm3gJiQmuStgIQXQSZ1uw TYLg== X-Gm-Message-State: ALoCoQld0GA5U+KbtSkDTHupz7qkqoJyFU659BiSiDcwqlxsgjAhlhe/qNWDccl/3hTS9DZJzblp X-Received: by 10.112.92.14 with SMTP id ci14mr7560lbb.23.1411586301578; Wed, 24 Sep 2014 12:18:21 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.5.9 with SMTP id o9ls169112lao.97.gmail; Wed, 24 Sep 2014 12:18:21 -0700 (PDT) X-Received: by 10.152.179.36 with SMTP id dd4mr8312960lac.51.1411586301369; Wed, 24 Sep 2014 12:18:21 -0700 (PDT) Received: from mail-lb0-f182.google.com (mail-lb0-f182.google.com [209.85.217.182]) by mx.google.com with ESMTPS id b3si66690laf.117.2014.09.24.12.18.21 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 24 Sep 2014 12:18:21 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.182 as permitted sender) client-ip=209.85.217.182; Received: by mail-lb0-f182.google.com with SMTP id u10so11317606lbd.13 for ; Wed, 24 Sep 2014 12:18:21 -0700 (PDT) X-Received: by 10.112.75.233 with SMTP id f9mr4826303lbw.102.1411586301104; Wed, 24 Sep 2014 12:18:21 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.130.169 with SMTP id of9csp640048lbb; Wed, 24 Sep 2014 12:18:19 -0700 (PDT) X-Received: by 10.66.154.132 with SMTP id vo4mr11734861pab.117.1411586298632; Wed, 24 Sep 2014 12:18:18 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id ox6si27552545pdb.211.2014.09.24.12.18.18 for ; Wed, 24 Sep 2014 12:18:18 -0700 (PDT) Received-SPF: none (google.com: linux-usb-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751543AbaIXTSQ (ORCPT + 3 others); Wed, 24 Sep 2014 15:18:16 -0400 Received: from devils.ext.ti.com ([198.47.26.153]:58477 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751209AbaIXTSP (ORCPT ); Wed, 24 Sep 2014 15:18:15 -0400 Received: from dlelxv90.itg.ti.com ([172.17.2.17]) by devils.ext.ti.com (8.13.7/8.13.7) with ESMTP id s8OJIDvP025562; Wed, 24 Sep 2014 14:18:13 -0500 Received: from DFLE72.ent.ti.com (dfle72.ent.ti.com [128.247.5.109]) by dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id s8OJID3J025354; Wed, 24 Sep 2014 14:18:13 -0500 Received: from dflp32.itg.ti.com (10.64.6.15) by DFLE72.ent.ti.com (128.247.5.109) with Microsoft SMTP Server id 14.3.174.1; Wed, 24 Sep 2014 14:18:13 -0500 Received: from localhost (ileax41-snat.itg.ti.com [10.172.224.153]) by dflp32.itg.ti.com (8.14.3/8.13.8) with ESMTP id s8OJIC8N017204; Wed, 24 Sep 2014 14:18:13 -0500 Date: Wed, 24 Sep 2014 14:18:14 -0500 From: Felipe Balbi To: Felipe Balbi CC: Alan Stern , Linux USB Mailing List Subject: Re: g_mass_storage bug ? Message-ID: <20140924191814.GM17997@saruman> Reply-To: References: <20140924162031.GH17997@saruman> <20140924180815.GL17997@saruman> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140924180815.GL17997@saruman> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-usb-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-usb@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: balbi@ti.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.182 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , Hi, On Wed, Sep 24, 2014 at 01:08:15PM -0500, Felipe Balbi wrote: > On Wed, Sep 24, 2014 at 01:53:31PM -0400, Alan Stern wrote: > > On Wed, 24 Sep 2014, Felipe Balbi wrote: > > > > > > I'll capture usbmon and send here shortly. > > > > > > here it is... Interesting part starts at line 73 (114 on this email) > > > where the data transport received EPIPE (due to Stall). This time > > > however, I was eventually able to talk to the device and managed to > > > issue quite a few writes to it. > > > > Here's where the unexpected stuff begins: > > > > > ed2541c0 1237463240 S Bo:003:01 -115 31 = 55534243 06000000 c0000000 8000061a 003f00c0 00000000 00000000 000000 > > > ed2541c0 1237463431 C Bo:003:01 0 31 > > > > ec1a8540 1237463873 S Bi:003:01 -115 192 < > > > ec1a8540 1237464053 C Bi:003:01 -32 0 > > > ed2541c0 1237464158 S Co:003:00 s 02 01 0000 0081 0000 0 > > > ed2541c0 1237464359 C Co:003:00 0 0 > > > ed2541c0 1237468607 S Bi:003:01 -115 13 < > > > ed2541c0 1237468802 C Bi:003:01 -75 0 > > > > This is the first MODE SENSE command. The gadget should send as much > > data as it can before halting the bulk-IN endpoint. Instead, the > > endpoint was halted first. > > > > Then, after the host cleared the halt, the gadget apparently sent the > > data that _should_ have been sent previously. The host was expecting > > to receive the CSW at this point, so there was an overflow error. > > That's what caused the host to perform a reset. > > > > Evidently this UDC implements the set_halt method incorrectly. > > According to the kerneldoc for usb_ep_set_halt: > > > > * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any > > * transfer requests are still queued, or if the controller hardware > > * (usually a FIFO) still holds bytes that the host hasn't collected. > > damn old bugs :-) I'll fix that up and Cc stable. alright fixed. Below you can see a combined diff which I'll still split into patches so they can be applied properly. diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 66f6256..834f524 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -210,6 +210,14 @@ #define DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(n) (((n) & (0x0f << 13)) >> 13) #define DWC3_MAX_HIBER_SCRATCHBUFS 15 +/* Global FIFO Space Register */ +#define DWC3_GDBGFIFOSPACE_TXFIFO (0 << 5) +#define DWC3_GDBGFIFOSPACE_RXFIFO (1 << 5) +#define DWC3_GDBGFIFOSPACE_TXREQ_Q (2 << 5) +#define DWC3_GDBGFIFOSPACE_RXREQ_Q (3 << 5) + +#define DWC3_GDBGFIFOSPACE_SPACE_AVAIL(num) (((num) & 0xffff0000) >> 16) + /* Device Configuration Register */ #define DWC3_DCFG_DEVADDR(addr) ((addr) << 3) #define DWC3_DCFG_DEVADDR_MASK DWC3_DCFG_DEVADDR(0x7f) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index de53361..5e89913 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -145,6 +145,75 @@ int dwc3_gadget_set_link_state(struct dwc3 *dwc, enum dwc3_link_state state) } /** + * dwc3_gadget_ep_fifo_space - returns currently available space on FIFO + * @dwc: pointer to our context struct + * @dep: the endpoint to fetch FIFO space + * + * This function will return the currently available FIFO space + */ +static int dwc3_gadget_ep_fifo_space(struct dwc3 *dwc, struct dwc3_ep *dep) +{ + u32 current_fifo; + u32 reg; + + if (dep->direction) + reg = DWC3_GDBGFIFOSPACE_TXFIFO; + else + reg = DWC3_GDBGFIFOSPACE_RXFIFO; + + /* remove direction bit */ + reg |= dep->number >> 1; + + dwc3_writel(dwc->regs, DWC3_GDBGFIFOSPACE, reg); + reg = dwc3_readl(dwc->regs, DWC3_GDBGFIFOSPACE); + current_fifo = DWC3_GDBGFIFOSPACE_SPACE_AVAIL(reg); + + return current_fifo; +} + +/** + * dwc3_gadget_ep_fifo_size - returns allocated FIFO size + * @dwc: pointer to our context struct + * @dep: TX endpoint to fetch allocated FIFO size + * + * This function will read the correct TX FIFO register and + * return the FIFO size + */ +static int dwc3_gadget_ep_fifo_size(struct dwc3 *dwc, struct dwc3_ep *dep) +{ + if (WARN_ON(!dep->direction)) + return 0; + + return dwc3_readl(dwc->regs, DWC3_GTXFIFOSIZ(dep->number)) & 0xffff; +} + +/** + * dwc3_gadget_ep_fifo_empty - returns true when FIFO is empty + * @dwc: pointer to our context struct + * @dep: endpoint to request FIFO space + * + * This function will return a TRUE when FIFO is empty and FALSE + * otherwise. + */ +static int dwc3_gadget_ep_fifo_empty(struct dwc3 *dwc, struct dwc3_ep *dep) +{ + u32 allocated_fifo; + u32 current_fifo; + + /* should not be called for RX endpoints */ + if (WARN_ON(!dep->direction)) + return false; + + current_fifo = dwc3_gadget_ep_fifo_space(dwc, dep); + allocated_fifo = dwc3_gadget_ep_fifo_size(dwc, dep); + + dev_vdbg(dwc->dev, "%s: FIFO space %u/%u\n", dep->name, + current_fifo, allocated_fifo); + + return current_fifo == allocated_fifo; +} + +/** * dwc3_gadget_resize_tx_fifos - reallocate fifo spaces for current use-case * @dwc: pointer to our context structure * @@ -1216,6 +1285,20 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value) memset(¶ms, 0x00, sizeof(params)); if (value) { + if (dep->flags & DWC3_EP_BUSY || + (!list_empty(&dep->req_queued) || + !list_empty(&dep->request_list))) { + dev_dbg(dwc->dev, "%s: pending request, cannot halt\n", + dep->name); + return -EAGAIN; + } + + if (dep->direction && !dwc3_gadget_ep_fifo_empty(dwc, dep)) { + dev_dbg(dwc->dev, "%s: FIFO not empty, cannot halt\n", + dep->name); + return -EAGAIN; + } + ret = dwc3_send_gadget_ep_cmd(dwc, dep->number, DWC3_DEPCMD_SETSTALL, ¶ms); if (ret)