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=-8.1 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable 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 13F9DC3404D for ; Wed, 19 Feb 2020 00:07:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id CCE3A22B48 for ; Wed, 19 Feb 2020 00:07:46 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="key not found in DNS" (0-bit key) header.d=mg.codeaurora.org header.i=@mg.codeaurora.org header.b="wpMLy0VF" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727206AbgBSAHp (ORCPT ); Tue, 18 Feb 2020 19:07:45 -0500 Received: from mail26.static.mailgun.info ([104.130.122.26]:21279 "EHLO mail26.static.mailgun.info" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726482AbgBSAHo (ORCPT ); Tue, 18 Feb 2020 19:07:44 -0500 DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=mg.codeaurora.org; q=dns/txt; s=smtp; t=1582070863; h=In-Reply-To: Content-Type: MIME-Version: References: Message-ID: Subject: Cc: To: From: Date: Sender; bh=ix4I8HGZpni8gwUgc++/jUAdQuDMXvzl5RoCu8Kns8U=; b=wpMLy0VF4kXRrP5CKBxL3D3JUxbKquR+sipEvnOpu7/Dnh2s4ozSd2kaqGciVKtlaS3morQg tkHuGOIOYTWUGV+FCewIlDllx/PoaVHtEAqt98WWcWm06gG5j2Ffl9hdOfYPeXOtpH5LIsAW bNpgAdMU18lb03GxKTXAoK6wBaw= X-Mailgun-Sending-Ip: 104.130.122.26 X-Mailgun-Sid: WyI0MWYwYSIsICJsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnIiwgImJlOWU0YSJd Received: from smtp.codeaurora.org (ec2-35-166-182-171.us-west-2.compute.amazonaws.com [35.166.182.171]) by mxa.mailgun.org with ESMTP id 5e4c7c4e.7f6364d45688-smtp-out-n03; Wed, 19 Feb 2020 00:07:42 -0000 (UTC) Received: by smtp.codeaurora.org (Postfix, from userid 1001) id F0DBCC447A4; Wed, 19 Feb 2020 00:07:41 +0000 (UTC) Received: from jackp-linux.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) (Authenticated sender: jackp) by smtp.codeaurora.org (Postfix) with ESMTPSA id 616A8C43383; Wed, 19 Feb 2020 00:07:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 616A8C43383 Authentication-Results: aws-us-west-2-caf-mail-1.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: aws-us-west-2-caf-mail-1.web.codeaurora.org; spf=none smtp.mailfrom=jackp@codeaurora.org Date: Tue, 18 Feb 2020 16:07:36 -0800 From: Jack Pham To: John Stultz Cc: lkml , Pratham Pratap , Felipe Balbi , Yang Fei , Thinh Nguyen , Tejas Joglekar , Andrzej Pietrasiewicz , Todd Kjos , Greg KH , Linux USB List , stable Subject: Re: [PATCH] usb: dwc3: gadget: Update chain bit correctly when using sg list Message-ID: <20200219000736.GA5511@jackp-linux.qualcomm.com> References: <20200218235104.112323-1-john.stultz@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200218235104.112323-1-john.stultz@linaro.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi John, Thanks for following-up with this! While you're doing minor tweaks anyway, I hope you don't mind me picking some nits below. On Tue, Feb 18, 2020 at 11:51:04PM +0000, John Stultz wrote: > From: Pratham Pratap > > If scatter-gather operation is allowed, a large USB request is split > into multiple TRBs. For preparing TRBs for sg list, driver iterates > over the list and creates TRB for each sg and mark the chain bit to > false for the last sg. The current IOMMU driver is clubbing the list > of sgs which shares a page boundary into one and giving it to USB driver. > With this the number of sgs mapped it not equal to the the number of sgs > passed. Because of this USB driver is not marking the chain bit to false > since it couldn't iterate to the last sg. This patch addresses this issue > by marking the chain bit to false if it is the last mapped sg. > > At a practical level, this patch resolves USB transfer stalls > seen with adb on dwc3 based db845c, pixel3 and other qcom > hardware after functionfs gadget added scatter-gather support > around v4.20. > > Credit also to Anurag Kumar Vulisha > who implemented a very similar fix to this issue. > > Cc: Felipe Balbi > Cc: Yang Fei > Cc: Thinh Nguyen > Cc: Tejas Joglekar > Cc: Andrzej Pietrasiewicz > Cc: Jack Pham > Cc: Todd Kjos > Cc: Greg KH > Cc: Linux USB List > Cc: stable > Signed-off-by: Pratham Pratap > [jstultz: Slight tweak to remove sg_is_last() usage, reworked > commit message, minor comment tweak] > Signed-off-by: John Stultz > --- > drivers/usb/dwc3/gadget.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 1b8014ab0b25..10aa511051e8 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -1071,7 +1071,14 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep, > unsigned int rem = length % maxp; > unsigned chain = true; > > - if (sg_is_last(s)) > + /* > + * IOMMU driver is coalescing the list of sgs which shares a > + * page boundary into one and giving it to USB driver. With > + * this the number of sgs mapped it not equal to the the number ^^ s/it/is/ ^^^ /d Or could we more specifically say "number of sgs mapped could be less than number passed"? > + * of sgs passed. Mark the chain bit to false if it is the last > + * mapped sg. > + */ > + if ((i == remaining - 1)) These outer parens are superfluous. Also wondering if it would be more or less clear to just set the variable once (and awkwardly move the comment to appear above the local var declaration): unsigned chain = (i < remaining - 1); > chain = false; > > if (rem && usb_endpoint_dir_out(dep->endpoint.desc) && !chain) { Jack -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project