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=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,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 21494C31E5D for ; Mon, 17 Jun 2019 17:37:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DE8082084D for ; Mon, 17 Jun 2019 17:37:22 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=synopsys.com header.i=@synopsys.com header.b="g0LDtnl7" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726336AbfFQRhW (ORCPT ); Mon, 17 Jun 2019 13:37:22 -0400 Received: from dc8-smtprelay2.synopsys.com ([198.182.47.102]:55002 "EHLO smtprelay-out1.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725995AbfFQRhW (ORCPT ); Mon, 17 Jun 2019 13:37:22 -0400 Received: from mailhost.synopsys.com (dc2-mailhost2.synopsys.com [10.12.135.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by smtprelay-out1.synopsys.com (Postfix) with ESMTPS id 5C549C003F; Mon, 17 Jun 2019 17:37:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=synopsys.com; s=mail; t=1560793041; bh=6zM6ChjKGFSvisAhJoUFOUDPu7+LldI4L4GT8yx0QcU=; h=From:To:CC:Subject:Date:References:In-Reply-To:From; b=g0LDtnl7isbxnfgBRp4ewceoihT6ojgtbnvrORJVlcR0Uy53B/aXUwyQ3tC0wISs3 dn9Z1IMhhF0b5pIMBSC7wv6pOY7+ucbO11HdLSxKh/B4S2/rRrYKE2dsfAGpwFF0in FScQAeZevh++1tv5hkd8xJ+Z6bRtbFGhZaK49g62ZxhGDOD4GlYNQgDBCy0y4efS7B cTVGy+R+1taAD5TLVyFcc4AUoxL01HjWrPZSV6+C2+g1voWH671rQPm8TSYmEDbYZE EZdYHcI5C2a0z8igk8MmUZN7kFa/+4Jt6FY+E9GnDraDUiDBEd7JU+lwGYoBhT9HNF QV/XzyfwM/PXg== Received: from US01WXQAHTC1.internal.synopsys.com (us01wxqahtc1.internal.synopsys.com [10.12.238.230]) (using TLSv1.2 with cipher AES128-SHA256 (128/128 bits)) (No client certificate requested) by mailhost.synopsys.com (Postfix) with ESMTPS id 124ABA009A; Mon, 17 Jun 2019 17:37:20 +0000 (UTC) Received: from us01wembx1.internal.synopsys.com ([169.254.1.22]) by US01WXQAHTC1.internal.synopsys.com ([::1]) with mapi id 14.03.0415.000; Mon, 17 Jun 2019 10:37:20 -0700 From: John Youn To: Felipe Balbi , John Youn , Thinh Nguyen CC: "linux-usb@vger.kernel.org" Subject: RE: [RFC] Sorting out dwc3 ISOC endpoints once and for all Thread-Topic: [RFC] Sorting out dwc3 ISOC endpoints once and for all Thread-Index: AQHVHRZktHmVHA84kESiBGd9iNhFnKagKHCg Date: Mon, 17 Jun 2019 17:37:20 +0000 Message-ID: <2B3535C5ECE8B5419E3ECBE30077290902E78AF3D7@us01wembx1.internal.synopsys.com> References: <87a7etd8s7.fsf@linux.intel.com> In-Reply-To: <87a7etd8s7.fsf@linux.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-dg-ref: =?us-ascii?Q?PG1ldGE+PGF0IG5tPSJib2R5LnR4dCIgcD0iYzpcdXNlcnNcam9obnlvdW5c?= =?us-ascii?Q?YXBwZGF0YVxyb2FtaW5nXDA5ZDg0OWI2LTMyZDMtNGE0MC04NWVlLTZiODRi?= =?us-ascii?Q?YTI5ZTM1Ylxtc2dzXG1zZy04ZDNlMWZkYi05MTI2LTExZTktODk4Mi1iODA4?= =?us-ascii?Q?Y2Y0NTI0ZTJcYW1lLXRlc3RcOGQzZTFmZGMtOTEyNi0xMWU5LTg5ODItYjgw?= =?us-ascii?Q?OGNmNDUyNGUyYm9keS50eHQiIHN6PSIzNTAwIiB0PSIxMzIwNTI2NjYzODkw?= =?us-ascii?Q?ODAyOTciIGg9InZBYTJJYVc5aU1OdmdxRGJIRDBzc2lEZGZ6az0iIGlkPSIi?= =?us-ascii?Q?IGJsPSIwIiBibz0iMSIgY2k9ImNBQUFBRVJIVTFSU1JVRk5DZ1VBQUJRSkFB?= =?us-ascii?Q?RHBxTU5QTXlYVkFWazI3Wko2Y002K1dUYnRrbnB3enI0T0FBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFIQUFBQUNrQ0FBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFFQUFRQUJBQUFBNUN3ZzVRQUFBQUFBQUFBQUFBQUFBSjRBQUFCbUFHa0Fi?= =?us-ascii?Q?Z0JoQUc0QVl3QmxBRjhBY0FCc0FHRUFiZ0J1QUdrQWJnQm5BRjhBZHdCaEFI?= =?us-ascii?Q?UUFaUUJ5QUcwQVlRQnlBR3NBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUVBQUFBQUFBQUFBZ0FBQUFBQW5nQUFBR1lBYndCMUFHNEFaQUJ5QUhrQVh3?= =?us-ascii?Q?QndBR0VBY2dCMEFHNEFaUUJ5QUhNQVh3Qm5BR1lBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFRQUFBQUFBQUFBQ0FB?= =?us-ascii?Q?QUFBQUNlQUFBQVpnQnZBSFVBYmdCa0FISUFlUUJmQUhBQVlRQnlBSFFBYmdC?= =?us-ascii?Q?bEFISUFjd0JmQUhNQVlRQnRBSE1BZFFCdUFHY0FYd0JqQUc4QWJnQm1BQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFCQUFBQUFBQUFBQUlBQUFBQUFKNEFBQUJtQUc4?= =?us-ascii?Q?QWRRQnVBR1FBY2dCNUFGOEFjQUJoQUhJQWRBQnVBR1VBY2dCekFGOEFjd0Jo?= =?us-ascii?Q?QUcwQWN3QjFBRzRBWndCZkFISUFaUUJ6QUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBRUFBQUFBQUFBQUFnQUFBQUFBbmdBQUFHWUFid0IxQUc0QVpBQnlBSGtB?= =?us-ascii?Q?WHdCd0FHRUFjZ0IwQUc0QVpRQnlBSE1BWHdCekFHMEFhUUJqQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQVFBQUFBQUFBQUFD?= =?us-ascii?Q?QUFBQUFBQ2VBQUFBWmdCdkFIVUFiZ0JrQUhJQWVRQmZBSEFBWVFCeUFIUUFi?= =?us-ascii?Q?Z0JsQUhJQWN3QmZBSE1BZEFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUJBQUFBQUFBQUFBSUFBQUFBQUo0QUFBQm1B?= =?us-ascii?Q?RzhBZFFCdUFHUUFjZ0I1QUY4QWNBQmhBSElBZEFCdUFHVUFjZ0J6QUY4QWRB?= =?us-ascii?Q?QnpBRzBBWXdBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFFQUFBQUFBQUFBQWdBQUFBQUFuZ0FBQUdZQWJ3QjFBRzRBWkFCeUFI?= =?us-ascii?Q?a0FYd0J3QUdFQWNnQjBBRzRBWlFCeUFITUFYd0IxQUcwQVl3QUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBUUFBQUFBQUFB?= =?us-ascii?Q?QUNBQUFBQUFDZUFBQUFad0IwQUhNQVh3QndBSElBYndCa0FIVUFZd0IwQUY4?= =?us-ascii?Q?QWRBQnlBR0VBYVFCdUFHa0FiZ0JuQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQkFBQUFBQUFBQUFJQUFBQUFBSjRBQUFC?= =?us-ascii?Q?ekFHRUFiQUJsQUhNQVh3QmhBR01BWXdCdkFIVUFiZ0IwQUY4QWNBQnNBR0VB?= =?us-ascii?Q?YmdBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUVBQUFBQUFBQUFBZ0FBQUFBQW5nQUFBSE1BWVFCc0FHVUFjd0Jm?= =?us-ascii?Q?QUhFQWRRQnZBSFFBWlFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFRQUFBQUFB?= =?us-ascii?Q?QUFBQ0FBQUFBQUNlQUFBQWN3QnVBSEFBY3dCZkFHd0FhUUJqQUdVQWJnQnpB?= =?us-ascii?Q?R1VBWHdCMEFHVUFjZ0J0QUY4QU1RQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFCQUFBQUFBQUFBQUlBQUFBQUFKNEFB?= =?us-ascii?Q?QUJ6QUc0QWNBQnpBRjhBYkFCcEFHTUFaUUJ1QUhNQVpRQmZBSFFBWlFCeUFH?= =?us-ascii?Q?MEFYd0J6QUhRQWRRQmtBR1VBYmdCMEFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBRUFBQUFBQUFBQUFnQUFBQUFBbmdBQUFIWUFad0JmQUdzQVpR?= =?us-ascii?Q?QjVBSGNBYndCeUFHUUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB?= =?us-ascii?Q?QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQVFBQUFB?= =?us-ascii?Q?QUFBQUFDQUFBQUFBQT0iLz48L21ldGE+?= x-dg-rorf: true x-originating-ip: [10.13.184.19] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org > -----Original Message----- > From: Felipe Balbi > Sent: Friday, June 7, 2019 2:50 AM > To: John Youn > Cc: linux-usb@vger.kernel.org > Subject: [RFC] Sorting out dwc3 ISOC endpoints once and for all >=20 ++ Thinh Hi Felipe, Sorry, missed this e-mail. > Now that we have dwc3_gadget_start_isoc_quirk() which figures out the > correct combination for the top-most 2 bits in the frame number, why > don't we just use that to start isochronous transfers and never, again, > have Bus Expiry problems? We should only see expiry problems on the affected versions with incorrect = top-2 bits right? >=20 > I mean something along the lines of below diff (completely untested): >=20 > modified drivers/usb/dwc3/gadget.c > @@ -1369,9 +1369,8 @@ static int dwc3_gadget_start_isoc_quirk(struct > dwc3_ep *dep) > else if (test0 && test1) > dep->combo_num =3D 0; >=20 > - dep->frame_number &=3D 0x3fff; > dep->frame_number |=3D dep->combo_num << 14; > - dep->frame_number +=3D max_t(u32, 4, dep->interval); > + dep->frame_number =3D DWC3_ALIGN_FRAME(dep, 1); >=20 > /* Reinitialize test variables */ > dep->start_cmd_status =3D 0; > @@ -1383,33 +1382,16 @@ static int dwc3_gadget_start_isoc_quirk(struct > dwc3_ep *dep) > static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep) > { > struct dwc3 *dwc =3D dep->dwc; > - int ret; > - int i; >=20 > if (list_empty(&dep->pending_list)) { > dep->flags |=3D DWC3_EP_PENDING_REQUEST; > return -EAGAIN; > } >=20 > - if (!dwc->dis_start_transfer_quirk && dwc3_is_usb31(dwc) && > - (dwc->revision <=3D DWC3_USB31_REVISION_160A || > - (dwc->revision =3D=3D DWC3_USB31_REVISION_170A && > - dwc->version_type >=3D DWC31_VERSIONTYPE_EA01 && > - dwc->version_type <=3D DWC31_VERSIONTYPE_EA06))) { > - > - if (dwc->gadget.speed <=3D USB_SPEED_HIGH && dep->direction) > - return dwc3_gadget_start_isoc_quirk(dep); > - } > - > - for (i =3D 0; i < DWC3_ISOC_MAX_RETRIES; i++) { > - dep->frame_number =3D DWC3_ALIGN_FRAME(dep, i + 1); > + dep->frame_number =3D __dwc3_gadget_get_frame(dwc); > + dep->frame_number =3D DWC3_ALIGN_FRAME(dep, 1); >=20 > - ret =3D __dwc3_gadget_kick_transfer(dep); > - if (ret !=3D -EAGAIN) > - break; > - } > - > - return ret; > + return dwc3_gadget_start_isoc_quirk(dep); > } >=20 > static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct > dwc3_request *req) >=20 >=20 > Would there be any obvious draw-back to going down this route? The thing > is that, as it is, it seems like we will *always* have some corner case > where we can't guarantee that we can even start a transfer since there's > no upper-bound between XferNotReady and gadget driver finally queueing a > request. Also, I can't simply read DSTS for the frame number because of > top-most 2 bits. >=20 For non-affected version of the IP, the xfernotready -> starttransfer time = will have to be off by more than a couple seconds for the driver to produce= an incorrect 16-bit frame number. If you're seeing errors here, maybe we j= ust need to code review the relevant sections to make sure the 14/16-bit an= d rollover conditions are all handled correctly. But I can't think of any obvious drawbacks of the quirk, other than doing s= ome unnecessary work, which shouldn't produce any bad side-effects. But we = haven't really tested that. John