From patchwork Thu Oct 23 14:56:41 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Felipe Balbi X-Patchwork-Id: 39369 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wi0-f198.google.com (mail-wi0-f198.google.com [209.85.212.198]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id DF71820341 for ; Thu, 23 Oct 2014 14:57:15 +0000 (UTC) Received: by mail-wi0-f198.google.com with SMTP id n3sf959538wiv.1 for ; Thu, 23 Oct 2014 07:57:15 -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=Anu0fJkyRMgvICPqvWNbqQ54WcAzzEXzGvd3fJLaHns=; b=Q0vHth/a9bJ4z/mR7JaDYxl9QsjvEKZNtkuqF7L2KghsIDv69D48GpPhWiBChbuIqe C+dl7+5jFu6Fc91ne2ZULH+GZXwhH/7KEqjwqIFjyCaXGnVbJa3ZMqiZ6T2/A0n7GQiH DsamScGI164l/T3Q2etw0GYYxuVS81hNf6xA+xqt6aXgF0JOOfePnY6ySaJecSusmvra sBFGBiV+isMOSVCcmHHcEddxfX6RQfZUM5RPj4WfCX3IQVKUfuUxLT1zQd4r4+gu+Mik RGf3H2928H8eIaWf+Eb13QXEoM3NazdQ7/JazHdXK0HicNCmI6/ZDotTpII6hHybnKUZ 37Lg== X-Gm-Message-State: ALoCoQlrYwDednnCaGm/Eu+7J+b/0DU4XzTFNRqNgyOoFgpM8+pp6LNjw1lrxheUJUc9DefOSgfO X-Received: by 10.112.225.135 with SMTP id rk7mr1167629lbc.6.1414076235032; Thu, 23 Oct 2014 07:57:15 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.5.134 with SMTP id s6ls35486las.27.gmail; Thu, 23 Oct 2014 07:57:14 -0700 (PDT) X-Received: by 10.112.45.228 with SMTP id q4mr5704391lbm.35.1414076234765; Thu, 23 Oct 2014 07:57:14 -0700 (PDT) Received: from mail-lb0-f171.google.com (mail-lb0-f171.google.com. [209.85.217.171]) by mx.google.com with ESMTPS id c6si3008678laa.50.2014.10.23.07.57.14 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 23 Oct 2014 07:57:14 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.171 as permitted sender) client-ip=209.85.217.171; Received: by mail-lb0-f171.google.com with SMTP id z12so998330lbi.16 for ; Thu, 23 Oct 2014 07:57:14 -0700 (PDT) X-Received: by 10.152.116.102 with SMTP id jv6mr5786638lab.40.1414076234637; Thu, 23 Oct 2014 07:57:14 -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.84.229 with SMTP id c5csp269538lbz; Thu, 23 Oct 2014 07:57:12 -0700 (PDT) X-Received: by 10.68.171.100 with SMTP id at4mr5370083pbc.127.1414076231272; Thu, 23 Oct 2014 07:57:11 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id dd1si1800356pbc.122.2014.10.23.07.57.10 for ; Thu, 23 Oct 2014 07:57:11 -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 S1754424AbaJWO5I (ORCPT + 3 others); Thu, 23 Oct 2014 10:57:08 -0400 Received: from comal.ext.ti.com ([198.47.26.152]:59922 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752690AbaJWO5H (ORCPT ); Thu, 23 Oct 2014 10:57:07 -0400 Received: from dlelxv90.itg.ti.com ([172.17.2.17]) by comal.ext.ti.com (8.13.7/8.13.7) with ESMTP id s9NEuvsB003873; Thu, 23 Oct 2014 09:56:57 -0500 Received: from DLEE70.ent.ti.com (dlee70.ent.ti.com [157.170.170.113]) by dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id s9NEuvqu003838; Thu, 23 Oct 2014 09:56:57 -0500 Received: from dlep32.itg.ti.com (157.170.170.100) by DLEE70.ent.ti.com (157.170.170.113) with Microsoft SMTP Server id 14.3.174.1; Thu, 23 Oct 2014 09:56:57 -0500 Received: from localhost (ileax41-snat.itg.ti.com [10.172.224.153]) by dlep32.itg.ti.com (8.14.3/8.13.8) with ESMTP id s9NEuudd015539; Thu, 23 Oct 2014 09:56:56 -0500 Date: Thu, 23 Oct 2014 09:56:41 -0500 From: Felipe Balbi To: Alan Stern CC: Felipe Balbi , Anton Tikhomirov , "'Paul Zimmerman'" , "'David Laight'" , "'Linux USB Mailing List'" Subject: Re: [PATCH 18/28] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to wMaxPacketSize Message-ID: <20141023145641.GG13573@saruman> Reply-To: References: <20141023134903.GB13573@saruman> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: 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.171 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 Thu, Oct 23, 2014 at 10:18:27AM -0400, Alan Stern wrote: > On Thu, 23 Oct 2014, Felipe Balbi wrote: > > > here's v2: > > > > 8<-------------------------------------------------------------- > > > > From 1080b54d66e3e77410b41732e76746ed8e2c01c7 Mon Sep 17 00:00:00 2001 > > From: Felipe Balbi > > Date: Tue, 30 Sep 2014 10:39:14 -0500 > > Subject: [PATCH] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to > > wMaxPacketSize > > > > According to Section 8.5.3.2 of the USB 2.0 specification, > > a USB device must terminate a Data Phase with either a > > short packet or a ZLP (if the previous transfer was > > a multiple of wMaxPacketSize). > > > > For reference, here's what the USB 2.0 specification, section > > 8.5.3.2 says: > > > > " > > 8.5.3.2 Variable-length Data Stage > > > > A control pipe may have a variable-length data phase > > in which the host requests more data than is contained > > in the specified data structure. When all of the data > > structure is returned to the host, the function should > > indicate that the Data stage is ended by returning a > > packet that is shorter than the MaxPacketSize for the > > pipe. If the data structure is an exact multiple of > > wMaxPacketSize for the pipe, the function will return > > a zero-length packet to indicate the end of the Data > > stage. > > " > > > > Signed-off-by: Felipe Balbi > > --- > > drivers/usb/dwc3/ep0.c | 19 +++++++++++++------ > > 1 file changed, 13 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c > > index a47cc1e..ce6b0c7 100644 > > --- a/drivers/usb/dwc3/ep0.c > > +++ b/drivers/usb/dwc3/ep0.c > > @@ -828,12 +828,19 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc, > > > > dwc3_ep0_stall_and_restart(dwc); > > } else { > > - /* > > - * handle the case where we have to send a zero packet. This > > - * seems to be case when req.length > maxpacket. Could it be? > > - */ > > - if (r) > > - dwc3_gadget_giveback(ep0, r, 0); > > + dwc3_gadget_giveback(ep0, r, 0); > > Don't you want to wait until the ZLP has completed before doing the > giveback? > > In fact, shouldn't all this ZLP code run when the transfer is > submitted, rather than when it completes? The idea is that you get a > request, you queue up all the necessary TRBs or whatever, and if an > extra ZLP is needed then you queue up an extra TRB. yeah, this is desirable but it would require a bigger rework of how we handle TRBs in dwc3. Currently, for ep0, we have a single TRB and adding another wouldn't fit into an -rc release. I'll see what I can do to make this better but as of now we need this bug fix. > > + > > + if (IS_ALIGNED(ur->length, ep0->endpoint.maxpacket) && > > + ur->zero) { > > Is this correct in the case where ur->length is 0? When that happens, > there should be only one ZLP, not two. and here I was assuming IS_ALIGNED() would handle that case. Here's v3. 8<------------------------------------------------------------------ >From 36f84ffb45c75ff10442a9f8f2fd91dcf6c6f5dd Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Tue, 30 Sep 2014 10:39:14 -0500 Subject: [PATCH] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to wMaxPacketSize According to Section 8.5.3.2 of the USB 2.0 specification, a USB device must terminate a Data Phase with either a short packet or a ZLP (if the previous transfer was a multiple of wMaxPacketSize). For reference, here's what the USB 2.0 specification, section 8.5.3.2 says: " 8.5.3.2 Variable-length Data Stage A control pipe may have a variable-length data phase in which the host requests more data than is contained in the specified data structure. When all of the data structure is returned to the host, the function should indicate that the Data stage is ended by returning a packet that is shorter than the MaxPacketSize for the pipe. If the data structure is an exact multiple of wMaxPacketSize for the pipe, the function will return a zero-length packet to indicate the end of the Data stage. " Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/ep0.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c index a47cc1e..711b230 100644 --- a/drivers/usb/dwc3/ep0.c +++ b/drivers/usb/dwc3/ep0.c @@ -828,12 +828,19 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc, dwc3_ep0_stall_and_restart(dwc); } else { - /* - * handle the case where we have to send a zero packet. This - * seems to be case when req.length > maxpacket. Could it be? - */ - if (r) - dwc3_gadget_giveback(ep0, r, 0); + dwc3_gadget_giveback(ep0, r, 0); + + if (IS_ALIGNED(ur->length, ep0->endpoint.maxpacket) && + ur->length && ur->zero) { + int ret; + + dwc->ep0_next_event = DWC3_EP0_COMPLETE; + + ret = dwc3_ep0_start_trans(dwc, epnum, + dwc->ctrl_req_addr, 0, + DWC3_TRBCTL_CONTROL_DATA); + WARN_ON(ret < 0); + } } }