From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E095CC76186 for ; Tue, 23 Jul 2019 18:52:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A9AED20821 for ; Tue, 23 Jul 2019 18:52:02 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="zYaV2dx0" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388804AbfGWSwC (ORCPT ); Tue, 23 Jul 2019 14:52:02 -0400 Received: from mail-wr1-f67.google.com ([209.85.221.67]:41391 "EHLO mail-wr1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726352AbfGWSwB (ORCPT ); Tue, 23 Jul 2019 14:52:01 -0400 Received: by mail-wr1-f67.google.com with SMTP id c2so41072980wrm.8 for ; Tue, 23 Jul 2019 11:52:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fyhJlYS/nmZVWKWvi27Hp7abC+DbsGqwswaGohdPOpA=; b=zYaV2dx0GCGELJRDEgnEx66kKYH6fLEWGmcTfHxHp8NiO832NO1LV81qL8RIhK9Tsj hAmNwACbpLQr/SARJZqkrV5Brs/mkIBkJ66SR8RQrdD3P8Rb74m8ALaG3ZvugP6HISMV lVG8R8vkB1aqiPQy4ff57S+GbKMrdWCYRLA27pDhg2fyAUHppqyPmvyBX3EoRwzX4vea nDktXIkOmtngTtXxPpPG4fWhlq3hNuDtaCSJjzwI/wh92oa9M7XzcbntymOu0moJLN7+ 2UZTvjHbBH9fBJ04/M14wCNpciOtmbW2JiwZp1oPyB78/W1tEXGQG6PB73YGjqJBft9T TwNQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=fyhJlYS/nmZVWKWvi27Hp7abC+DbsGqwswaGohdPOpA=; b=V3CEu6lLsAf1Oew2WJO/4xrjIKSBn9Fq8BNGgxXTgvK6tFKr6bWTb2SYWz1fy24hEA OkTwTBYO4fch06StuJD/epWBZtLX5Xa4EIPtBchs8CfbnWYYZTPhcY6zUU/4T+7H4tpJ W6UvZTCtaBWiD6cJY8ZtmZuO7enw+2uNKKxNZtjyrbkTiub0Yp3US6qz+u2j9vZARBTa FlQyCO2ccZZA9SK6Un4qgAn+/nmTQijM5uhmAzTV+lg8qY1oUCGdcwEU40iG8I/FbBqR 9VsgvfKWluEdwmoXuBCiW7nplsc7e0TGWZyCfWPJBR6g/fD2IlR7uKr2wWEl1Q/8unfM lcOg== X-Gm-Message-State: APjAAAWA784l6TyMsbnUIy4EcvClVva3viFvYsazDgGBD2hqqosc7jMp NOezT51/ntfrh+o/ggXWYtrOx+yaoKE4yBgKr+wJ+Q== X-Google-Smtp-Source: APXvYqzlIepC9KXFaJ7DC2ZH9C3Jxw5UZ5QWzvCKIs/BFU+5pWBvVgbB6otUiss1YIbgTou6fEMr3+zplpwjb1jwh8w= X-Received: by 2002:a05:6000:145:: with SMTP id r5mr3390652wrx.208.1563907919536; Tue, 23 Jul 2019 11:51:59 -0700 (PDT) MIME-Version: 1.0 References: <1563497183-7114-1-git-send-email-fei.yang@intel.com> In-Reply-To: From: John Stultz Date: Tue, 23 Jul 2019 11:51:47 -0700 Message-ID: Subject: Re: [PATCH v3] usb: dwc3: gadget: trb_dequeue is not updated properly To: Thinh Nguyen Cc: "fei.yang@intel.com" , "felipe.balbi@linux.intel.com" , "andrzej.p@collabora.com" , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "gregkh@linuxfoundation.org" , "stable@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org On Thu, Jul 18, 2019 at 6:12 PM Thinh Nguyen wrote: > fei.yang@intel.com wrote: > > From: Fei Yang > > > > If scatter-gather operation is allowed, a large USB request is split into > > multiple TRBs. These TRBs are chained up by setting DWC3_TRB_CTRL_CHN bit > > except the last one which has DWC3_TRB_CTRL_IOC bit set instead. > > Since only the last TRB has IOC set for the whole USB request, the > > dwc3_gadget_ep_reclaim_trb_sg() gets called only once after the last TRB > > completes and all the TRBs allocated for this request are supposed to be > > reclaimed. However that is not what the current code does. > > > > dwc3_gadget_ep_reclaim_trb_sg() is trying to reclaim all the TRBs in the > > following for-loop, > > for_each_sg(sg, s, pending, i) { > > trb = &dep->trb_pool[dep->trb_dequeue]; > > > > if (trb->ctrl & DWC3_TRB_CTRL_HWO) > > break; > > > > req->sg = sg_next(s); > > req->num_pending_sgs--; > > > > ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req, > > trb, event, status, chain); > > if (ret) > > break; > > } > > but since the interrupt comes only after the last TRB completes, the > > event->status has DEPEVT_STATUS_IOC bit set, so that the for-loop ends for > > the first TRB due to dwc3_gadget_ep_reclaim_completed_trb() returns 1. > > if (event->status & DEPEVT_STATUS_IOC) > > return 1; > > > > This patch addresses the issue by checking each TRB in function > > dwc3_gadget_ep_reclaim_trb_sg() and maing sure the chained ones are properly > > reclaimed. dwc3_gadget_ep_reclaim_completed_trb() will return 1 Only for the > > last TRB. > > > > Signed-off-by: Fei Yang > > Cc: stable > > --- > > v2: Better solution is to reclaim chained TRBs in dwc3_gadget_ep_reclaim_trb_sg() > > and leave the last TRB to the dwc3_gadget_ep_reclaim_completed_trb(). > > v3: Checking DWC3_TRB_CTRL_CHN bit for each TRB instead, and making sure that > > dwc3_gadget_ep_reclaim_completed_trb() returns 1 only for the last TRB. > > --- > > drivers/usb/dwc3/gadget.c | 11 ++++++++--- > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > > index 173f532..88eed49 100644 > > --- a/drivers/usb/dwc3/gadget.c > > +++ b/drivers/usb/dwc3/gadget.c > > @@ -2394,7 +2394,7 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, > > if (event->status & DEPEVT_STATUS_SHORT && !chain) > > return 1; > > > > - if (event->status & DEPEVT_STATUS_IOC) > > + if (event->status & DEPEVT_STATUS_IOC && !chain) > > return 1; > > > > return 0; > > @@ -2404,11 +2404,12 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep, > > struct dwc3_request *req, const struct dwc3_event_depevt *event, > > int status) > > { > > - struct dwc3_trb *trb = &dep->trb_pool[dep->trb_dequeue]; > > + struct dwc3_trb *trb; > > struct scatterlist *sg = req->sg; > > struct scatterlist *s; > > unsigned int pending = req->num_pending_sgs; > > unsigned int i; > > + int chain = false; > > int ret = 0; > > > > for_each_sg(sg, s, pending, i) { > > @@ -2419,9 +2420,13 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep, > > > > req->sg = sg_next(s); > > req->num_pending_sgs--; > > + if (trb->ctrl & DWC3_TRB_CTRL_CHN) > > + chain = true; > > + else > > + chain = false; > > > > ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req, > > - trb, event, status, true); > > + trb, event, status, chain); > > if (ret) > > break; > > } > > There was already a fix a long time ago by Anurag. But it never made it > to the kernel mainline. You can check this out: > https://patchwork.kernel.org/patch/10640137/ So, back from a vacation last week, and just validated that both Fei's patch and a forward ported version of this patch Thinh pointed out both seem to resolve the usb stalls I've been seeing sinice 4.20 w/ dwc3 hardware on both hikey960 and dragonboard 845c. Felipe: Does Anurag's patch above make more sense as a proper fix? thanks -john