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;
From:   Pawel Laszczak <pawell@cadence.com>
To:     Peter Chen <peter.chen@kernel.org>
CC:     "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: RE: [PATCH v2] usb: cdnsp: fix issue with ZLP - added TD_SIZE = 1
Thread-Topic: [PATCH v2] usb: cdnsp: fix issue with ZLP - added TD_SIZE = 1
Thread-Index: AQHY57Ge68RMVQuhaUeO0fOrXMci/K4h2uCAgAAVsiA=
Date:   Thu, 27 Oct 2022 08:46:17 +0000
Message-ID: <BYAPR07MB5381482129407B849BA9A616DD339@BYAPR07MB5381.namprd07.prod.outlook.com>
References: <1666620275-139704-1-git-send-email-pawell@cadence.com>
 <20221027072421.GA75844@nchen-desktop>
In-Reply-To: <20221027072421.GA75844@nchen-desktop>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?xS27eaPrxk6P2F+uEljzAKITh0ikVpNX6LxipDW1z1BM7+0DuWuQHo8QCIma?=
 =?us-ascii?Q?sfPVnMXiEeIm+ZdGmiIBtlo1jSNj4wX7m2qdeXqRlat7C1PoMmZ+zJe2ULR8?=
 =?us-ascii?Q?TwggGSRp9rsfGSr2oKa+o/SobKeuuPxZquqwEaZRKbPOtJzuJomODLLDysr1?=
 =?us-ascii?Q?88Dera9yADZaiABK2qlu9nsRSxu+qNUlqL0rOS5TNSx6fffLTmvFujfsCe6R?=
 =?us-ascii?Q?QeuKclbaShEFr1HpEJCi9xRdofgiFmPURWDgPP6JKto9dOtywi8iAHhVvICp?=
 =?us-ascii?Q?59sGDsLslXrsDFCGZcfGHDYKxNge+fBLAaKTN3ZVUuieUC8Xj0xL+E4biRJj?=
 =?us-ascii?Q?t9LAFWggfOOEoBJvcf3WXaLzZTd112wUGsGkA/nt41mOyGiyb6GNsuR7k8is?=
 =?us-ascii?Q?5mJTOByPQ1FXmdqV+JrlYgGyEAfpGI6jEswtwHZwxC+Mst/qHLpryhVtUUNh?=
 =?us-ascii?Q?5n2GEvdxmJ9KTLXpjFwVJ6HxIf+c9rsFVpuLKO5CU2c016MXM8B1Mj2f20yI?=
 =?us-ascii?Q?me9LWS1daSKfYn+zA/twermF1QVNX581CbDKBB2EAqIiASCni59ukvDrFzsw?=
 =?us-ascii?Q?lrMHOJqaRJdyTMu4RBPJDfnEJGyQ0IAQe7CU1ig1tue6jRSnxTRIpq8a8d/J?=
 =?us-ascii?Q?wZHuf85G4hS9ql/vt5IfNTzMvPwN15qSM8uqsOKmOLdhiEuHduAIJ6MLi1X2?=
 =?us-ascii?Q?To+zSxCFuGwYfCdP0chSqv4BFW1sutWchZSabphRaw7b+ayVlkzVHcMxPDxC?=
 =?us-ascii?Q?XvGqC7ZWV8xaHUlFi8sH8BNwgjtBVR6Oc32a0d9NhNNqmABbySz0T+6X3mKf?=
 =?us-ascii?Q?jZUZD/VxzCU1nZfcgY/UTrlGJQO3F5TNnYHh1oPUQPhq2nqwBPat+MQf/hwX?=
 =?us-ascii?Q?dTADPKlJP9S6NQF2lXdj77gyeQlIVuAtBPSzFvBXodvsm5WrToVmy2IyhZrJ?=
 =?us-ascii?Q?LXRTUII6Fh5Tzdi/ldcEbSTm8UnMi81RXnZDweaBojp7P8Y6KNy0m7q13tUE?=
 =?us-ascii?Q?PyN7TRYFmj9rsH5B/EW5U6qBCI9mRHnkA6SDufsMMndccL8zubtlFoqllak5?=
 =?us-ascii?Q?rLXPum1mnElfB5dUazHXWqq/F1DVNKqtf4bldadn21uxWOcfLfOB05SyOnAB?=
 =?us-ascii?Q?B+YGgNG8hiWwnOxtmuhSxxeGJg+kR9yBtaJMXvbSx/yUIbu8QTQsqBlpa27N?=
 =?us-ascii?Q?tQ0gdxqdFntw9gjKZx8JCLaJIHdN+lkKv5QuSdC9k+E0GOaF3by7iXKKnR3c?=
 =?us-ascii?Q?8xFrT+4TwhuqpwuzNcoipGUuZuCMwUB09I9p6FwAo3CBA4eqCwJoItsI0nPU?=
 =?us-ascii?Q?k2sUcn2CostiG0sRYJhcq7t6heMSXFDkMcqnEuFOS6/JdzbuH5LY3KWfYPZu?=
 =?us-ascii?Q?dnUO8pYsc+6AeTKLE44ggoezJ5GwkYxONRx1zVEuK52o3VPm5b/xPqNWoZTY?=
 =?us-ascii?Q?Il4EDpB+48JEiRhU3gzrNMpfGAhYhPsrroljeJeqPWwqFw/I6jgdeqkq3cZw?=
 =?us-ascii?Q?PttSaH/v+3bg9lC1JzEnVoauE+rcspCm4F6zG6fY14mjGHzun4FGASxiABE2?=
 =?us-ascii?Q?Pc8Amt+aL6dIz1DUck1rz/Q8lMpB1JSleFXg/0jN?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: cadence.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BYAPR07MB5381.namprd07.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 098415cc-d8f0-4a8d-05e5-08dab7f7b6db
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Oct 2022 08:46:17.8802
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: d36035c5-6ce6-4662-a3dc-e762e61ae4c9
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: /vJwbMpwrnC3j1umLUSndE4XzoWwBWHKMpUKkdIHQoQzQSrzi5fcmL2D1dJlXw+yxi0gkcYyrFTxBQuCEzedAJWo7+wFjejeo//5UVkNXMk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR07MB5865
X-Proofpoint-ORIG-GUID: Ide3UuMkenUoNFJswevV6JV2Il5byl4Q
X-Proofpoint-GUID: Ide3UuMkenUoNFJswevV6JV2Il5byl4Q
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-10-27_03,2022-10-26_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=outbound_check_notspam policy=outbound_check score=0 clxscore=1015
 adultscore=0 spamscore=0 bulkscore=0 priorityscore=1501 mlxscore=0
 phishscore=0 lowpriorityscore=0 mlxlogscore=503 impostorscore=0
 malwarescore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx
 scancount=1 engine=8.12.0-2210170000 definitions=main-2210270049
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,
        SPF_NONE 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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


>
>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: <stable@vger.kernel.org>
>> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence
>> USBSSP DRD Driver")
>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>>
>> ---
>> 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 =3D=3D 0 && trb_buff_len =3D=3D =
0) ||
>> -	    trb_buff_len =3D=3D td_total_len)
>> +	    trb_buff_len =3D=3D td_total_len) {
>> +		/* Before ZLP driver needs set TD_SIZE=3D1. */
>> +		if (more_trbs_coming)
>> +			return 1;
>> +
>>  		return 0;
>> +	}
>
>Does that fix the issue you want at bulk transfer, which has zero-length p=
acket
>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=20
as TD_SIZE in TRB.

The last TRB in TD should have TD_SIZE=3D0, so trb for ZLP should have
set also TD_SIZE=3D0. If driver set TD_SIZE=3D0 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 =3D 1
before the last TRB.=20

e.g.

TD -> TRB1  transfer_length =3D 64KB, TD_SIZE =3D0
          TRB2 transfer_length =3D0, TD_SIZE =3D 0  - controller will
		    ignore this transfer and stop transfer on previous one

TD -> TRB1  transfer_length =3D 64KB, TD_SIZE =3D1
          TRB2 transfer_length =3D0, TD_SIZE =3D 0  - controller will
		    execute this trb and send ZLP

As you noticed previously, previous fix for last TRB returned
TD_SIZE =3D 1 in some cases.
Previous fix was working correct but was not compliance with
controller specification.

>
>>
>>  	maxp =3D usb_endpoint_maxp(preq->pep->endpoint.desc);
>>  	total_packet_count =3D DIV_ROUND_UP(td_total_len, maxp);
>> --
>> 2.25.1
>>
>
>--
>

Thanks,
Pawel Laszczak