From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [4/6] usb: gadget: add functions to signal udc driver to delay status stage From: Felipe Balbi Message-Id: <87a7mmv46v.fsf@linux.intel.com> Date: Tue, 06 Nov 2018 13:24:56 +0200 To: Alan Stern , Laurent Pinchart Cc: Paul Elder , Bin Liu , kieran.bingham@ideasonboard.com, gregkh@linuxfoundation.org, USB list , Kernel development list , rogerq@ti.com List-ID: SGksCgpBbGFuIFN0ZXJuIDxzdGVybkByb3dsYW5kLmhhcnZhcmQuZWR1PiB3cml0ZXM6Cj4gVGhl cmUncyBhIHNpbWlsYXIgcmFjZSBhdCB0aGUgaGFyZHdhcmUgbGV2ZWwuICBXaGF0IGhhcHBlbnMg aWYgdGhlCj4gY29udHJvbGxlciByZWNlaXZlcyBhIG5ldyBTRVRVUCBwYWNrZXQgYW5kIGNvbmN1 cnJlbnRseSB0aGUgZHJpdmVyIGlzCj4gc2V0dGluZyB1cCB0aGUgY29udHJvbGxlciByZWdpc3Rl cnMgZm9yIGEgcmVzcG9uc2UgdG8gYW4gZWFybGllcgo+IFNFVFVQPyAgSSBkb24ndCBrbm93IGhv dyByZWFsIGNvbnRyb2xsZXJzIGhhbmRsZSB0aGlzLgoKVGhhdCdzIEhXIGltcGxlbWVudGF0aW9u IGRldGFpbC4gRFdDMywgZm9yIGluc3RhbmNlLCB3aWxsIGlnbm9yZSB0aGUKVFJCcyBhbmQgcmV0 dXJuIG1lIHRoZSBzdGF0dXMgInNldHVwIHBhY2tldCBwZW5kaW5nIi4gVGhlbiBJIGp1c3Qgc3Rh cnQKYSBuZXcgU0VUVVAgVFJCLgoKPj4gPiA+IEkgd29uZGVyIGlmIHRoZXJlJ3MgcmVhbGx5IGEg dXNlIGNhc2UgZm9yIGRlbGF5aW5nIHRoZSBkYXRhIHN0YWdlIG9mCj4+ID4gPiBjb250cm9sIE9V VCByZXF1ZXN0cywgYXMgaXQgc2VlbXMgdG8gbWUgdGhhdCB3ZSBjYW4gcGVyZm9ybSB0aGUKPj4g PiA+IGFzeW5jaHJvbm91cyB2YWxpZGF0aW9uIG9mIHRoZSBzZXR1cCBhbmQgZGF0YSBzdGFnZXMg dG9nZXRoZXIsIGluIHdoaWNoCj4+ID4gPiBjYXNlIHdlIHdvdWxkIGFsd2F5cyBwcm9jZWVkIHRv IHRoZSBkYXRhIHN0YWdlLCBhbmQgb25seSBwb3RlbnRpYWxseQo+PiA+ID4gZGVsYXkgdGhlIHN0 YXR1cyBzdGFnZS4gSG93ZXZlciwgaWYgd2Ugc3dpdGNoIHRvIGFuIGV4cGxpY2l0IEFQSSB3aGVy ZQo+PiA+ID4gdGhlIHRyYW5zaXRpb24gZnJvbSB0aGUgc2V0dXAgdG8gdGhlIGRhdGEgc3RhZ2Ug aXMgdHJpZ2dlcmVkIGJ5IHF1ZXVlaW5nCj4+ID4gPiBhIHJlcXVlc3QsIGFuZCBnaXZlbiB0aGF0 IHN1Y2ggYSB0cmFuc2l0aW9uIG1heSBuZWVkIHRvIGJlIGRlbGF5ZWQgZm9yCj4+ID4gPiB0aGUg Y29udHJvbCBJTiBjYXNlLCBkZWxheWluZyB0aGUgZGF0YSBzdGFnZSBmb3IgY29udHJvbCBPVVQg d291bGQKPj4gPiA+IGVzc2VudGlhbGx5IGNvbWUgZm9yIGZyZWUuCj4+IAo+PiBXaGF0IGRvIHlv dSB0aGluayBhYm91dCB0aGlzID8gU2hvdWxkIHdlIGFsbG93IGZ1bmN0aW9uIGRyaXZlcnMgdG8g ZGVsYXkgdGhlIAo+PiBkYXRhIHN0YWdlIG9mIGNvbnRyb2wgT1VUIHJlcXVlc3RzID8KPgo+IFlv dSBtZWFuLCBzaG91bGQgd2UgYWxsb3cgZnVuY3Rpb24gZHJpdmVycyB0byBxdWV1ZSB0aGUgZGF0 YS1zdGFnZQo+IHJlcXVlc3QgYWZ0ZXIgdGhlIHNldHVwIGhhbmRsZXIgaGFzIHJldHVybmVkPyAg SSBkb24ndCBzZWUgYW55IHJlYXNvbgoKdGhhdCdzIGFscmVhZHkgZG9uZToKCnN0YXRpYyB2b2lk IGR3YzNfZXAwX3hmZXJfY29tcGxldGUoc3RydWN0IGR3YzMgKmR3YywKCQkJY29uc3Qgc3RydWN0 IGR3YzNfZXZlbnRfZGVwZXZ0ICpldmVudCkKewoJc3RydWN0IGR3YzNfZXAJCSpkZXAgPSBkd2Mt PmVwc1tldmVudC0+ZW5kcG9pbnRfbnVtYmVyXTsKCglkZXAtPmZsYWdzICY9IH5EV0MzX0VQX1RS QU5TRkVSX1NUQVJURUQ7CglkZXAtPnJlc291cmNlX2luZGV4ID0gMDsKCWR3Yy0+c2V0dXBfcGFj a2V0X3BlbmRpbmcgPSBmYWxzZTsKCglzd2l0Y2ggKGR3Yy0+ZXAwc3RhdGUpIHsKCWNhc2UgRVAw X1NFVFVQX1BIQVNFOgoJCWR3YzNfZXAwX2luc3BlY3Rfc2V0dXAoZHdjLCBldmVudCk7CgkJYnJl YWs7ClsuLi5dCn0KCnN0YXRpYyB2b2lkIGR3YzNfZXAwX2luc3BlY3Rfc2V0dXAoc3RydWN0IGR3 YzMgKmR3YywKCQljb25zdCBzdHJ1Y3QgZHdjM19ldmVudF9kZXBldnQgKmV2ZW50KQp7CglzdHJ1 Y3QgdXNiX2N0cmxyZXF1ZXN0ICpjdHJsID0gKHZvaWQgKikgZHdjLT5lcDBfdHJiOwoJaW50IHJl dCA9IC1FSU5WQUw7Cgl1MzIgbGVuOwoKCWlmICghZHdjLT5nYWRnZXRfZHJpdmVyKQoJCWdvdG8g b3V0OwoKCXRyYWNlX2R3YzNfY3RybF9yZXEoY3RybCk7CgoJbGVuID0gbGUxNl90b19jcHUoY3Ry bC0+d0xlbmd0aCk7CglpZiAoIWxlbikgewoJCWR3Yy0+dGhyZWVfc3RhZ2Vfc2V0dXAgPSBmYWxz ZTsKCQlkd2MtPmVwMF9leHBlY3RfaW4gPSBmYWxzZTsKCQlkd2MtPmVwMF9uZXh0X2V2ZW50ID0g RFdDM19FUDBfTlJEWV9TVEFUVVM7Cgl9IGVsc2UgewoJCWR3Yy0+dGhyZWVfc3RhZ2Vfc2V0dXAg PSB0cnVlOwoJCWR3Yy0+ZXAwX2V4cGVjdF9pbiA9ICEhKGN0cmwtPmJSZXF1ZXN0VHlwZSAmIFVT Ql9ESVJfSU4pOwoJCWR3Yy0+ZXAwX25leHRfZXZlbnQgPSBEV0MzX0VQMF9OUkRZX0RBVEE7Cgl9 ClsuLi5dCn0KCnN0YXRpYyBpbnQgX19kd2MzX2dhZGdldF9lcDBfcXVldWUoc3RydWN0IGR3YzNf ZXAgKmRlcCwKCQlzdHJ1Y3QgZHdjM19yZXF1ZXN0ICpyZXEpCnsKCXN0cnVjdCBkd2MzCQkqZHdj ID0gZGVwLT5kd2M7CgoJcmVxLT5yZXF1ZXN0LmFjdHVhbAk9IDA7CglyZXEtPnJlcXVlc3Quc3Rh dHVzCT0gLUVJTlBST0dSRVNTOwoJcmVxLT5lcG51bQkJPSBkZXAtPm51bWJlcjsKCglsaXN0X2Fk ZF90YWlsKCZyZXEtPmxpc3QsICZkZXAtPnBlbmRpbmdfbGlzdCk7CgpbLi4uXQoKCWlmIChkd2Mt PnRocmVlX3N0YWdlX3NldHVwKSB7CgkJdW5zaWduZWQgICAgICAgIGRpcmVjdGlvbjsKCgkJZGly ZWN0aW9uID0gZHdjLT5lcDBfZXhwZWN0X2luOwoJCWR3Yy0+ZXAwc3RhdGUgPSBFUDBfREFUQV9Q SEFTRTsKCgkJX19kd2MzX2VwMF9kb19jb250cm9sX2RhdGEoZHdjLCBkd2MtPmVwc1tkaXJlY3Rp b25dLCByZXEpOwoKCQlkZXAtPmZsYWdzICY9IH5EV0MzX0VQMF9ESVJfSU47Cgl9CgoJcmV0dXJu IDA7Cn0KClJlZ2FyZGxlc3Mgb2YgdGhlIGRpcmVjdGlvbiwgY29udHJvbCBkYXRhIGFsd2F5cyBk ZXBlbmRzIG9uIGEgY2FsbCB0bwp1c2JfZXBfcXVldWUoKQoKPiB3aHkgbm90LiAgQWZ0ZXIgYWxs LCBzb21lIGRyaXZlcnMgbWF5IHJlcXVpcmUgdGhpcy4gIExpa2V3aXNlIGZvciB0aGUgCj4gZGF0 YSBzdGFnZSBvZiBhIGNvbnRyb2wtSU4uCj4KPiBBbm90aGVyIHRoaW5nIHdlIHNob3VsZCBkbyBp cyBnaXZlIGZ1bmN0aW9uIGRyaXZlcnMgYSB3YXkgdG8gc2VuZCBhCj4gU1RBTEwgcmVzcG9uc2Ug Zm9yIHRoZSBzdGF0dXMgc3RhZ2UuICBDdXJyZW50bHkgdGhlcmUncyBubyB3YXkgdG8gZG8KPiBp dCwgaWYgYSBkYXRhIHN0YWdlIGlzIHByZXNlbnQuCgpTdGF0dXMgc3RhZ2UgY2FuIG9ubHkgYmUg c3RhbGxlZCBpZiBob3N0IHRyaWVzIHRvIG1vdmUgZGF0YSBvbiB0aGUgd3JvbmcKZGlyZWN0aW9u LiBDdXJyZW50bHksIHRoYXQncyBoYW5kbGVkIGludGVybmFsbHkgYnkgVURDcyBzaW5jZSB0aGF0 J3MKZWFzeSBlbm91Z2ggdG8gdHJhY2suCgpEYXRhIHN0YWdlIGFscmVhZHkgaGFzIGV4cGxpY2l0 IHN0YWxsIGhhbmRsaW5nLgo= 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=-1.0 required=3.0 tests=MAILING_LIST_MULTI,SPF_PASS 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 619F3C32789 for ; Tue, 6 Nov 2018 11:25:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 25BBA2086A for ; Tue, 6 Nov 2018 11:25:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 25BBA2086A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730407AbeKFUts (ORCPT ); Tue, 6 Nov 2018 15:49:48 -0500 Received: from mga05.intel.com ([192.55.52.43]:35425 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726976AbeKFUts (ORCPT ); Tue, 6 Nov 2018 15:49:48 -0500 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Nov 2018 03:25:04 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,471,1534834800"; d="asc'?scan'208";a="271754305" Received: from pipin.fi.intel.com (HELO localhost) ([10.237.72.128]) by orsmga005.jf.intel.com with ESMTP; 06 Nov 2018 03:25:00 -0800 From: Felipe Balbi To: Alan Stern , Laurent Pinchart Cc: Paul Elder , Bin Liu , kieran.bingham@ideasonboard.com, gregkh@linuxfoundation.org, USB list , Kernel development list , rogerq@ti.com Subject: Re: [PATCH 4/6] usb: gadget: add functions to signal udc driver to delay status stage In-Reply-To: References: Date: Tue, 06 Nov 2018 13:24:56 +0200 Message-ID: <87a7mmv46v.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Alan Stern writes: > There's a similar race at the hardware level. What happens if the > controller receives a new SETUP packet and concurrently the driver is > setting up the controller registers for a response to an earlier > SETUP? I don't know how real controllers handle this. That's HW implementation detail. DWC3, for instance, will ignore the TRBs and return me the status "setup packet pending". Then I just start a new SETUP TRB. >> > > I wonder if there's really a use case for delaying the data stage of >> > > control OUT requests, as it seems to me that we can perform the >> > > asynchronous validation of the setup and data stages together, in wh= ich >> > > case we would always proceed to the data stage, and only potentially >> > > delay the status stage. However, if we switch to an explicit API whe= re >> > > the transition from the setup to the data stage is triggered by queu= eing >> > > a request, and given that such a transition may need to be delayed f= or >> > > the control IN case, delaying the data stage for control OUT would >> > > essentially come for free. >>=20 >> What do you think about this ? Should we allow function drivers to delay= the=20 >> data stage of control OUT requests ? > > You mean, should we allow function drivers to queue the data-stage > request after the setup handler has returned? I don't see any reason that's already done: static void dwc3_ep0_xfer_complete(struct dwc3 *dwc, const struct dwc3_event_depevt *event) { struct dwc3_ep *dep =3D dwc->eps[event->endpoint_number]; dep->flags &=3D ~DWC3_EP_TRANSFER_STARTED; dep->resource_index =3D 0; dwc->setup_packet_pending =3D false; switch (dwc->ep0state) { case EP0_SETUP_PHASE: dwc3_ep0_inspect_setup(dwc, event); break; [...] } static void dwc3_ep0_inspect_setup(struct dwc3 *dwc, const struct dwc3_event_depevt *event) { struct usb_ctrlrequest *ctrl =3D (void *) dwc->ep0_trb; int ret =3D -EINVAL; u32 len; if (!dwc->gadget_driver) goto out; trace_dwc3_ctrl_req(ctrl); len =3D le16_to_cpu(ctrl->wLength); if (!len) { dwc->three_stage_setup =3D false; dwc->ep0_expect_in =3D false; dwc->ep0_next_event =3D DWC3_EP0_NRDY_STATUS; } else { dwc->three_stage_setup =3D true; dwc->ep0_expect_in =3D !!(ctrl->bRequestType & USB_DIR_IN); dwc->ep0_next_event =3D DWC3_EP0_NRDY_DATA; } [...] } static int __dwc3_gadget_ep0_queue(struct dwc3_ep *dep, struct dwc3_request *req) { struct dwc3 *dwc =3D dep->dwc; req->request.actual =3D 0; req->request.status =3D -EINPROGRESS; req->epnum =3D dep->number; list_add_tail(&req->list, &dep->pending_list); [...] if (dwc->three_stage_setup) { unsigned direction; direction =3D dwc->ep0_expect_in; dwc->ep0state =3D EP0_DATA_PHASE; __dwc3_ep0_do_control_data(dwc, dwc->eps[direction], req); dep->flags &=3D ~DWC3_EP0_DIR_IN; } return 0; } Regardless of the direction, control data always depends on a call to usb_ep_queue() > why not. After all, some drivers may require this. Likewise for the=20 > data stage of a control-IN. > > Another thing we should do is give function drivers a way to send a > STALL response for the status stage. Currently there's no way to do > it, if a data stage is present. Status stage can only be stalled if host tries to move data on the wrong direction. Currently, that's handled internally by UDCs since that's easy enough to track. Data stage already has explicit stall handling. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlvheggACgkQzL64meEa mQYUWQ//QKqUTzvDjFhnwAxDZtYUGagYIOnk98FI3Xrn7rHyA3jGMfnqbe55mVO7 FvaNwtYZTI0ADk8o63EkfW2Ey8HNtn7ejyQROHMKM8jrPdf9O+00fryqnhbXkKJk ZdCys12+4MMWCfDq2XR1bvH98Xy949m8XiVNFN3IYkZ4a9GXyQ0B8zQ/gqP7nP36 bO3OyJOoj2ffXLITRyT37j3v6MqsFS80B+ioQKp0I41rCJnXBRsisa2HSsM3iDtN z2GF7keXPfORIjblqvM13Dunxhfze11Tz+BwQfcmghzkur3KIS3zRpsur9tL8fI7 Y8XwzKeMjSsIrqOOT019Cqj3hhNzt797qGbHT5kfNlK8ZTsm8LeLK9WPd6GQ2gzu oyvha7EMFo5RPzcOER++e3WxgXrLOSBL5AvPsuUXUWLdE2Ao79QLqE0r8UfoXtfn tcCWLzxPtzSvxWfF8/6JNiNHiXu76pn6w0ZTDHLPEtkVJew9khmiyWsbSqehteTO hcMtY1ipIlbjwJZm/33NY3iYlSzkFKLOrWLcFcRy/wcpNRm3wquJ5/aXy4JH3EYc 3ustJuOzM3CcH3uvJX3h+WkK8VBR/EcMsJ266e+KW0eS5cJbmPJFJETinJnNfHY4 D5KPU/5XdS9H19nqrw4PY/QjwgtkY1bkrKklZFzkLUbZXcqK5Kc= =sMbS -----END PGP SIGNATURE----- --=-=-=--