Received: by 2002:a05:6358:d09b:b0:dc:cd0c:909e with SMTP id jc27csp1513164rwb; Thu, 10 Nov 2022 17:57:58 -0800 (PST) X-Google-Smtp-Source: AA0mqf4Exol5IFydIFC/lUnr8VwYwz7P84MtjOM2OOVezNFmn63w9eUC40U8YsZT3cdcKD0FvbJf X-Received: by 2002:a17:907:2b26:b0:7ae:c460:c65f with SMTP id gc38-20020a1709072b2600b007aec460c65fmr245447ejc.226.1668131878282; Thu, 10 Nov 2022 17:57:58 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1668131878; cv=none; d=google.com; s=arc-20160816; b=QzSwgSxe9foWlQ8GXmjoYustQCicsDlavYurgNFB9/x+38etOmn40AfKZLVHkWtI4x aaOFacPlB93q9W/53eQDGqQWEndHpojC3EaiUUTTShUROyrITu/cLd5BoUvicVba8gC8 CCCVdJzhOCTcfkjut+YzIGxch2LeLNoagrV02wzpjbxOALbhgviupti737c8DpeT9gAo SfhXzuZe5BI7j3XuwuoyuUkkYuxFk9eq4IJNyzDFPoGx7xIPJJXI6nIscbMdTfJPOu6o YivI0L3kT1NmFPHz42sxzPwaDU8q4tBE7ww8HFenbKfKwZL6I7k0MkFklQwXPw8YeRa1 J9YA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=jHD5mLLJZONA0/xtdbh0Srevx71YFYJ07K0NkniAf9c=; b=map5BfkcBbQoTbKHhed7EmDQZSyCoa+ydi3DGu68robg/9uVptPRNQcE0Lz5EupF2T cZ9Cx64Fhq8TdjDF0MYZjOTEYNxUSf/vOjqlu2M9btJGPYVxP0c3KqRg+oPrQBqXmkvg WPM8mkMUheCGeUWltnwLfqSQZTRjyX5IX8rdMKM25RQNpsIwbTY7SLlnoBUzc2+xJ+oC x3XjNMem+PerQtrwELVmcEm7xVmBQUSENSSaiN7S63NQaHKgrwFtDfAl4pGbYd2qAHia CBJ49+WJP3L6ajPMfkLfW5yMU452AMh94BW8Hfbe0avcoMw5j9H3mEtlhKUIJF701A1a F8xw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=GwMk6CFn; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id gi18-20020a1709070c9200b007ae1d040bffsi628110ejc.223.2022.11.10.17.57.36; Thu, 10 Nov 2022 17:57:58 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=GwMk6CFn; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232062AbiKKB3i (ORCPT + 93 others); Thu, 10 Nov 2022 20:29:38 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48466 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229536AbiKKB3g (ORCPT ); Thu, 10 Nov 2022 20:29:36 -0500 Received: from mail-vs1-xe2f.google.com (mail-vs1-xe2f.google.com [IPv6:2607:f8b0:4864:20::e2f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D5E3960E8E; Thu, 10 Nov 2022 17:29:35 -0800 (PST) Received: by mail-vs1-xe2f.google.com with SMTP id t5so3972514vsh.8; Thu, 10 Nov 2022 17:29:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=jHD5mLLJZONA0/xtdbh0Srevx71YFYJ07K0NkniAf9c=; b=GwMk6CFn4Fnc6IN9tCgX19EH7VibmJZBqzocmwJvqRP3eNGSAUehrluMmLt2xKa+kE pocfyS6amZ1+0ETBiF4A3y/+LX18Rr+5Og7WgLRaNRgblEvAlaiK8Ead7q44d5UprvOF UIVTFjkuP4v81L89nWMwIZsiN7LjiqxyGOZlwVfpHF2+p3tJ/GTHNf1fweWimSoIsEax tgpyS1sxr4C4FBLDj6awpZGu3vVCqwCLVmwoBU0d4e9BXHebXbAuWZKnQSFVapa059t8 spSjIzuOpzPcXXt0cgQNcdkVY+KL6aEvpUvqEBYA3vIItHxW4+pPek7yC/QdSxTebHpv JC6g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=jHD5mLLJZONA0/xtdbh0Srevx71YFYJ07K0NkniAf9c=; b=AIiDOm0C36Y9A5O4UKqJBGliLjUZuKbKYs/xwNPKPwg4WT/C7UcQkkLh8SznGmZ6d+ zGVRa+YHZKd1NNX/MaB1hNsja5ydgUTN7CkDcwHCPaLz6GV/QKQW9r+CM0jOHVoirHQQ m/vWEggWIkKPRa2Jg0v9yxWuqDX+te1Mq1enH3/L0bJD6Jf8kE7tBv2ogcK6+YH6UErr 47GXbiROoQa36bM5pji8fyhxDPMgg4nFZxqBwBuzVDFEOiijCjAilBOva/tSMEBlb7A0 h8eXe3UKCDhXObG3tjoBSc68TvDR9IfzpQTljjRhnLTPW4uLVkTcufNryHk6TeY002rb KwRg== X-Gm-Message-State: ACrzQf3xlw8V/x7/GvyVFYE2bUJK5Nd46dNwaA6r1f4/PQlx4NWqyEgG VBQjZEdnEVotNUpxvVpGkX9RxOBZsMHI3W4LJJA= X-Received: by 2002:a05:6102:3ca3:b0:3a7:c762:9209 with SMTP id c35-20020a0561023ca300b003a7c7629209mr4104254vsv.1.1668130174920; Thu, 10 Nov 2022 17:29:34 -0800 (PST) MIME-Version: 1.0 References: <1666620275-139704-1-git-send-email-pawell@cadence.com> <20221027072421.GA75844@nchen-desktop> <20221106090221.GA152143@nchen-desktop> In-Reply-To: From: Peter Chen Date: Fri, 11 Nov 2022 09:28:30 +0800 Message-ID: Subject: Re: [PATCH v2] usb: cdnsp: fix issue with ZLP - added TD_SIZE = 1 To: Pawel Laszczak Cc: Peter Chen , "gregkh@linuxfoundation.org" , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "stable@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Nov 10, 2022 at 1:38 PM Pawel Laszczak wrote: > > >On Mon, Nov 7, 2022 at 1:39 PM Pawel Laszczak > >wrote: > >> > >> > > >> > > >> >On 22-10-27 08:46:17, Pawel Laszczak wrote: > >> >> > >> >> > > >> >> >On 22-10-24 10:04:35, Pawel Laszczak wrote: > >> >> >> Patch modifies the TD_SIZE in TRB before ZLP TRB. > >> >> >> The TD_SIZE in TRB before ZLP TRB must be set to 1 to force > >> >> >> processing ZLP TRB by controller. > >> >> >> > >> >> >> cc: > >> >> >> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence > >> >> >> USBSSP DRD Driver") > >> >> >> Signed-off-by: Pawel Laszczak > >> >> >> > >> >> >> --- > >> >> >> Changelog: > >> >> >> v2: > >> >> >> - returned value for last TRB must be 0 > >> >> >> > >> >> >> drivers/usb/cdns3/cdnsp-ring.c | 7 ++++++- > >> >> >> 1 file changed, 6 insertions(+), 1 deletion(-) > >> >> >> > >> >> >> diff --git a/drivers/usb/cdns3/cdnsp-ring.c > >> >> >> b/drivers/usb/cdns3/cdnsp-ring.c index > >> >> >> 04dfcaa08dc4..aa79bce89d8a > >> >> >> 100644 > >> >> >> --- a/drivers/usb/cdns3/cdnsp-ring.c > >> >> >> +++ b/drivers/usb/cdns3/cdnsp-ring.c > >> >> >> @@ -1769,8 +1769,13 @@ static u32 cdnsp_td_remainder(struct > >> >> >> cdnsp_device *pdev, > >> >> >> > >> >> >> /* One TRB with a zero-length data packet. */ > >> >> >> if (!more_trbs_coming || (transferred == 0 && trb_buff_len == 0) || > >> >> >> - trb_buff_len == td_total_len) > >> >> >> + trb_buff_len == td_total_len) { > >> >> >> + /* Before ZLP driver needs set TD_SIZE=1. */ > >> >> >> + if (more_trbs_coming) > >> >> >> + return 1; > >> >> >> + > >> >> >> return 0; > >> >> >> + } > >> >> > > >> >> >Does that fix the issue you want at bulk transfer, which has > >> >> >zero-length packet at the last packet? It seems not align with > >> >> >your previous > >> >fix. > >> >> >Would you mind explaining more? > >> >> > >> >> Value returned by function cdnsp_td_remainder is used as TD_SIZE in > >> >> TRB. > >> >> > >> >> The last TRB in TD should have TD_SIZE=0, so trb for ZLP should > >> >> have set also TD_SIZE=0. If driver set TD_SIZE=0 on before the last > >> >> one TRB then the controller stops the transfer and ignore trb for ZLP > >packet. > >> >> > >> >> To fix this, the driver in such case must set TD_SIZE = 1 before > >> >> the last TRB. > >> > > >> > if (!more_trbs_coming || (transferred == 0 && trb_buff_len == 0) || > >> > - trb_buff_len == td_total_len) > >> > + trb_buff_len == td_total_len) { > >> > + /* Before ZLP driver needs set TD_SIZE=1. */ > >> > + if (more_trbs_coming) > >> > + return 1; > >> > + > >> > return 0; > >> > + } > >> > > >> >How your above fix could return TD_SIZE as 1 for last non-ZLP TRB? > >> >Which conditions are satisfied? > >> > >> For last non-ZLP TRB TD_SIZE should be 0 or 1. > >> > >> We have three casess: > >> 1. > >> TRB1 - length > 1 > >> TRb2 - ZLP > >> > >> In this case TRB1 should have set TD_SIZE = 1. In this case the condition > >> if (more_trbs_coming) > >> return 1; > >> > >> returns TD_SIZE=1. In this case more_trb_comming for TRB1 is 1 and for > >> TRB2 is 0 > >> > > > >This one is my question. How below condition is true for your case 1: > > > > if (!more_trbs_coming || (transferred == 0 && trb_buff_len == 0) || > > trb_buff_len == td_total_len) > > For TRB1: > more_trbs_coming = true > transferred == 0 && trb_buff_len == 0 - false - it does not matter in this case > trb_buff_len == td_total_len - true Why trb_buff_len equals to td_total_length? Considering the bulk transfer with request length larger than 64KB. Peter > so whole condition is true. > > Because more_trb_comming = true so: > if (more_trbs_coming) > return 1; > returns TD_SIZE = 1 > > For TRB2 - ZLP: > more_trbs_coming = false > transferred == 0 && trb_buff_len == 0 - false - it does not matter in this case > trb_buff_len == td_total_len - true > > Because more_trb_comming = false so function returns TD_SIZE = 0 for last ZLP trb. > > Pawel