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=HEADER_FROM_DIFFERENT_DOMAINS, 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 093FDC32789 for ; Tue, 6 Nov 2018 14:51:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id BF7E520869 for ; Tue, 6 Nov 2018 14:51:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BF7E520869 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=rowland.harvard.edu 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 S2388922AbeKGAQl (ORCPT ); Tue, 6 Nov 2018 19:16:41 -0500 Received: from iolanthe.rowland.org ([192.131.102.54]:36610 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S2388428AbeKGAQl (ORCPT ); Tue, 6 Nov 2018 19:16:41 -0500 Received: (qmail 2264 invoked by uid 2102); 6 Nov 2018 09:51:06 -0500 Received: from localhost (sendmail-bs@127.0.0.1) by localhost with SMTP; 6 Nov 2018 09:51:06 -0500 Date: Tue, 6 Nov 2018 09:51:06 -0500 (EST) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Felipe Balbi cc: Laurent Pinchart , Paul Elder , Bin Liu , , , , , Subject: Re: [PATCH 4/6] usb: gadget: add functions to signal udc driver to delay status stage In-Reply-To: <87d0riv4jw.fsf@linux.intel.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 6 Nov 2018, Felipe Balbi wrote: > DATA stage always depends on a usb_ep_queue() from gadget driver. So > it's always "delayed" in that sense. However, it's conceivable that some UDC drivers might behave differently depending on whether the usb_ep_queue call occurs within the setup callback or after that callback returns. They _shouldn't_, but they might. > it avoids all the special cases. UDC drivers can implement a single > handling for struct usb_request. We could do away with special return > values and so on... It's not quite so simple, because the UDC driver will need to keep track of whether a request queued on ep0 should be in the IN or the OUT direction. (Maybe they have to do this already, I don't know.) > > request and the UDC would then need to check whether that request corresponds > > to a status stage and process it accordingly. A new operation specific to this > > no, it wouldn't. UDC would have to check the size of request, that's > all: > > if (r->length == 0) > special_zlp_handling(); > else > regular_non_zlp_handling(); Checking the length isn't enough. A data stage can have 0 length. > But we don't need to care about special return values and the like. We > don't even need to care (from UDC perspective) if we're dealing with > 2-stage or 3-stage control transfers (well, dwc3 needs to care because > of different TRB types that needs to be used, but that's another story) No, we do need to care because of the direction issue. > > There's also the fact that requests can specify a completion handler, but only > > the data stage request would see its completion handler called (unless we > > require UDCs to call completion requests at the completion of the status > > stage, but I'm not sure that all UDCs can report the event to the driver, and > > that would likely be useless as nobody needs that feature). > > you still wanna know if the host actually processed your status > stage. udc-core can (and should) provide a generic status stage > completion function which, at a minimum, aids with some tracepoints. Helping with tracepoints is fine. However, I don't think function drivers really need to know whether the status stage was processed by the host. Can you point out any examples where such information would be useful? > One way to satisfy what you want, with what I want is to have UDC core > implement something like below: > > int usb_ep_start_status_stage(struct usb_gadget *g) > { > return usb_ep_queue(g->ep0, &g->ep0_status_request); > } > > special function for you, usb_ep_queue() for me :-p Sure, this is one of the options Laurent and I have discussed. > >> (But it does involve a > >> race in cases where the host gets tired of waiting and issues another > >> SETUP packet before the processing of the first transfer is finished.) > > Host would stall first in that case. I don't follow. Suppose the host sends a SETUP packet for an IN transfer, but the gadget takes so long to send the IN data back that the host times out. So then the host sends a SETUP packet for a new transfer. No stalls. (Besides, hosts never send STALL packets anyway. Only peripherals do.) > Driver is already required to > handle stalls for several other conditions. If thehre are bugs in that > area, I'd prefer catching them. > > To simplify function drivers, do you think the above proposal of adding a flag > > to the (data stage) request to request an automatic transition to the status > > stage is a good idea ? We could even possibly invert the logic and transition > > no, I don't think so. Making the status phase always explicit is far > better. UDCs won't have to check flags, or act on magic return > values. It just won't do anything until a request is queued. I don't agree. This would be a simple test in a localized area (the completion callback for control requests). It could even be implemented by a library routine; the UDC driver would simply have to call this routine immediately after invoking the callback. Alan Stern 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: Alan Stern Message-Id: Date: Tue, 6 Nov 2018 09:51:06 -0500 (EST) To: Felipe Balbi Cc: Laurent Pinchart , Paul Elder , Bin Liu , kieran.bingham@ideasonboard.com, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, rogerq@ti.com List-ID: T24gVHVlLCA2IE5vdiAyMDE4LCBGZWxpcGUgQmFsYmkgd3JvdGU6Cgo+IERBVEEgc3RhZ2UgYWx3 YXlzIGRlcGVuZHMgb24gYSB1c2JfZXBfcXVldWUoKSBmcm9tIGdhZGdldCBkcml2ZXIuIFNvCj4g aXQncyBhbHdheXMgImRlbGF5ZWQiIGluIHRoYXQgc2Vuc2UuCgpIb3dldmVyLCBpdCdzIGNvbmNl aXZhYmxlIHRoYXQgc29tZSBVREMgZHJpdmVycyBtaWdodCBiZWhhdmUgCmRpZmZlcmVudGx5IGRl cGVuZGluZyBvbiB3aGV0aGVyIHRoZSB1c2JfZXBfcXVldWUgY2FsbCBvY2N1cnMgd2l0aGluIAp0 aGUgc2V0dXAgY2FsbGJhY2sgb3IgYWZ0ZXIgdGhhdCBjYWxsYmFjayByZXR1cm5zLiAgVGhleSBf c2hvdWxkbid0XywgCmJ1dCB0aGV5IG1pZ2h0LgoKPiBpdCBhdm9pZHMgYWxsIHRoZSBzcGVjaWFs IGNhc2VzLiBVREMgZHJpdmVycyBjYW4gaW1wbGVtZW50IGEgc2luZ2xlCj4gaGFuZGxpbmcgZm9y IHN0cnVjdCB1c2JfcmVxdWVzdC4gV2UgY291bGQgZG8gYXdheSB3aXRoIHNwZWNpYWwgcmV0dXJu Cj4gdmFsdWVzIGFuZCBzbyBvbi4uLgoKSXQncyBub3QgcXVpdGUgc28gc2ltcGxlLCBiZWNhdXNl IHRoZSBVREMgZHJpdmVyIHdpbGwgbmVlZCB0byBrZWVwIAp0cmFjayBvZiB3aGV0aGVyIGEgcmVx dWVzdCBxdWV1ZWQgb24gZXAwIHNob3VsZCBiZSBpbiB0aGUgSU4gb3IgdGhlIE9VVCAKZGlyZWN0 aW9uLiAgKE1heWJlIHRoZXkgaGF2ZSB0byBkbyB0aGlzIGFscmVhZHksIEkgZG9uJ3Qga25vdy4p Cgo+ID4gcmVxdWVzdCBhbmQgdGhlIFVEQyB3b3VsZCB0aGVuIG5lZWQgdG8gY2hlY2sgd2hldGhl ciB0aGF0IHJlcXVlc3QgY29ycmVzcG9uZHMgCj4gPiB0byBhIHN0YXR1cyBzdGFnZSBhbmQgcHJv Y2VzcyBpdCBhY2NvcmRpbmdseS4gQSBuZXcgb3BlcmF0aW9uIHNwZWNpZmljIHRvIHRoaXMgCj4g Cj4gbm8sIGl0IHdvdWxkbid0LiBVREMgd291bGQgaGF2ZSB0byBjaGVjayB0aGUgc2l6ZSBvZiBy ZXF1ZXN0LCB0aGF0J3MKPiBhbGw6Cj4gCj4gCWlmIChyLT5sZW5ndGggPT0gMCkKPiAgICAgICAg IAlzcGVjaWFsX3pscF9oYW5kbGluZygpOwo+IAllbHNlCj4gICAgICAgICAJcmVndWxhcl9ub25f emxwX2hhbmRsaW5nKCk7CgpDaGVja2luZyB0aGUgbGVuZ3RoIGlzbid0IGVub3VnaC4gIEEgZGF0 YSBzdGFnZSBjYW4gaGF2ZSAwIGxlbmd0aC4KCj4gQnV0IHdlIGRvbid0IG5lZWQgdG8gY2FyZSBh Ym91dCBzcGVjaWFsIHJldHVybiB2YWx1ZXMgYW5kIHRoZSBsaWtlLiBXZQo+IGRvbid0IGV2ZW4g bmVlZCB0byBjYXJlIChmcm9tIFVEQyBwZXJzcGVjdGl2ZSkgaWYgd2UncmUgZGVhbGluZyB3aXRo Cj4gMi1zdGFnZSBvciAzLXN0YWdlIGNvbnRyb2wgdHJhbnNmZXJzICh3ZWxsLCBkd2MzIG5lZWRz IHRvIGNhcmUgYmVjYXVzZQo+IG9mIGRpZmZlcmVudCBUUkIgdHlwZXMgdGhhdCBuZWVkcyB0byBi ZSB1c2VkLCBidXQgdGhhdCdzIGFub3RoZXIgc3RvcnkpCgpObywgd2UgZG8gbmVlZCB0byBjYXJl IGJlY2F1c2Ugb2YgdGhlIGRpcmVjdGlvbiBpc3N1ZS4KCj4gPiBUaGVyZSdzIGFsc28gdGhlIGZh Y3QgdGhhdCByZXF1ZXN0cyBjYW4gc3BlY2lmeSBhIGNvbXBsZXRpb24gaGFuZGxlciwgYnV0IG9u bHkgCj4gPiB0aGUgZGF0YSBzdGFnZSByZXF1ZXN0IHdvdWxkIHNlZSBpdHMgY29tcGxldGlvbiBo YW5kbGVyIGNhbGxlZCAodW5sZXNzIHdlIAo+ID4gcmVxdWlyZSBVRENzIHRvIGNhbGwgY29tcGxl dGlvbiByZXF1ZXN0cyBhdCB0aGUgY29tcGxldGlvbiBvZiB0aGUgc3RhdHVzIAo+ID4gc3RhZ2Us IGJ1dCBJJ20gbm90IHN1cmUgdGhhdCBhbGwgVURDcyBjYW4gcmVwb3J0IHRoZSBldmVudCB0byB0 aGUgZHJpdmVyLCBhbmQgCj4gPiB0aGF0IHdvdWxkIGxpa2VseSBiZSB1c2VsZXNzIGFzIG5vYm9k eSBuZWVkcyB0aGF0IGZlYXR1cmUpLgo+IAo+IHlvdSBzdGlsbCB3YW5uYSBrbm93IGlmIHRoZSBo b3N0IGFjdHVhbGx5IHByb2Nlc3NlZCB5b3VyIHN0YXR1cwo+IHN0YWdlLiB1ZGMtY29yZSBjYW4g KGFuZCBzaG91bGQpIHByb3ZpZGUgYSBnZW5lcmljIHN0YXR1cyBzdGFnZQo+IGNvbXBsZXRpb24g ZnVuY3Rpb24gd2hpY2gsIGF0IGEgbWluaW11bSwgYWlkcyB3aXRoIHNvbWUgdHJhY2Vwb2ludHMu CgpIZWxwaW5nIHdpdGggdHJhY2Vwb2ludHMgaXMgZmluZS4gIEhvd2V2ZXIsIEkgZG9uJ3QgdGhp bmsgZnVuY3Rpb24gCmRyaXZlcnMgcmVhbGx5IG5lZWQgdG8ga25vdyB3aGV0aGVyIHRoZSBzdGF0 dXMgc3RhZ2Ugd2FzIHByb2Nlc3NlZCBieSAKdGhlIGhvc3QuICBDYW4geW91IHBvaW50IG91dCBh bnkgZXhhbXBsZXMgd2hlcmUgc3VjaCBpbmZvcm1hdGlvbiB3b3VsZCAKYmUgdXNlZnVsPwoKPiBP bmUgd2F5IHRvIHNhdGlzZnkgd2hhdCB5b3Ugd2FudCwgd2l0aCB3aGF0IEkgd2FudCBpcyB0byBo YXZlIFVEQyBjb3JlCj4gaW1wbGVtZW50IHNvbWV0aGluZyBsaWtlIGJlbG93Ogo+IAo+IGludCB1 c2JfZXBfc3RhcnRfc3RhdHVzX3N0YWdlKHN0cnVjdCB1c2JfZ2FkZ2V0ICpnKQo+IHsKPiAJcmV0 dXJuIHVzYl9lcF9xdWV1ZShnLT5lcDAsICZnLT5lcDBfc3RhdHVzX3JlcXVlc3QpOwo+IH0KPiAK PiBzcGVjaWFsIGZ1bmN0aW9uIGZvciB5b3UsIHVzYl9lcF9xdWV1ZSgpIGZvciBtZSA6LXAKClN1 cmUsIHRoaXMgaXMgb25lIG9mIHRoZSBvcHRpb25zIExhdXJlbnQgYW5kIEkgaGF2ZSBkaXNjdXNz ZWQuCgo+ID4+IChCdXQgaXQgZG9lcyBpbnZvbHZlIGEKPiA+PiByYWNlIGluIGNhc2VzIHdoZXJl IHRoZSBob3N0IGdldHMgdGlyZWQgb2Ygd2FpdGluZyBhbmQgaXNzdWVzIGFub3RoZXIKPiA+PiBT RVRVUCBwYWNrZXQgYmVmb3JlIHRoZSBwcm9jZXNzaW5nIG9mIHRoZSBmaXJzdCB0cmFuc2ZlciBp cyBmaW5pc2hlZC4pCj4gCj4gSG9zdCB3b3VsZCBzdGFsbCBmaXJzdCBpbiB0aGF0IGNhc2UuCgpJ IGRvbid0IGZvbGxvdy4gIFN1cHBvc2UgdGhlIGhvc3Qgc2VuZHMgYSBTRVRVUCBwYWNrZXQgZm9y IGFuIElOIAp0cmFuc2ZlciwgYnV0IHRoZSBnYWRnZXQgdGFrZXMgc28gbG9uZyB0byBzZW5kIHRo ZSBJTiBkYXRhIGJhY2sgdGhhdCAKdGhlIGhvc3QgdGltZXMgb3V0LiAgU28gdGhlbiB0aGUgaG9z dCBzZW5kcyBhIFNFVFVQIHBhY2tldCBmb3IgYSBuZXcgCnRyYW5zZmVyLiAgTm8gc3RhbGxzLgoK KEJlc2lkZXMsIGhvc3RzIG5ldmVyIHNlbmQgU1RBTEwgcGFja2V0cyBhbnl3YXkuICBPbmx5IHBl cmlwaGVyYWxzIGRvLikKCj4gRHJpdmVyIGlzIGFscmVhZHkgcmVxdWlyZWQgdG8KPiBoYW5kbGUg c3RhbGxzIGZvciBzZXZlcmFsIG90aGVyIGNvbmRpdGlvbnMuIElmIHRoZWhyZSBhcmUgYnVncyBp biB0aGF0Cj4gYXJlYSwgSSdkIHByZWZlciBjYXRjaGluZyB0aGVtLgoKPiA+IFRvIHNpbXBsaWZ5 IGZ1bmN0aW9uIGRyaXZlcnMsIGRvIHlvdSB0aGluayB0aGUgYWJvdmUgcHJvcG9zYWwgb2YgYWRk aW5nIGEgZmxhZyAKPiA+IHRvIHRoZSAoZGF0YSBzdGFnZSkgcmVxdWVzdCB0byByZXF1ZXN0IGFu IGF1dG9tYXRpYyB0cmFuc2l0aW9uIHRvIHRoZSBzdGF0dXMgCj4gPiBzdGFnZSBpcyBhIGdvb2Qg aWRlYSA/IFdlIGNvdWxkIGV2ZW4gcG9zc2libHkgaW52ZXJ0IHRoZSBsb2dpYyBhbmQgdHJhbnNp dGlvbiAKPiAKPiBubywgSSBkb24ndCB0aGluayBzby4gTWFraW5nIHRoZSBzdGF0dXMgcGhhc2Ug YWx3YXlzIGV4cGxpY2l0IGlzIGZhcgo+IGJldHRlci4gVURDcyB3b24ndCBoYXZlIHRvIGNoZWNr IGZsYWdzLCBvciBhY3Qgb24gbWFnaWMgcmV0dXJuCj4gdmFsdWVzLiBJdCBqdXN0IHdvbid0IGRv IGFueXRoaW5nIHVudGlsIGEgcmVxdWVzdCBpcyBxdWV1ZWQuCgpJIGRvbid0IGFncmVlLiAgVGhp cyB3b3VsZCBiZSBhIHNpbXBsZSB0ZXN0IGluIGEgbG9jYWxpemVkIGFyZWEgKHRoZSAKY29tcGxl dGlvbiBjYWxsYmFjayBmb3IgY29udHJvbCByZXF1ZXN0cykuICBJdCBjb3VsZCBldmVuIGJlIApp bXBsZW1lbnRlZCBieSBhIGxpYnJhcnkgcm91dGluZTsgdGhlIFVEQyBkcml2ZXIgd291bGQgc2lt cGx5IGhhdmUgdG8gCmNhbGwgdGhpcyByb3V0aW5lIGltbWVkaWF0ZWx5IGFmdGVyIGludm9raW5n IHRoZSBjYWxsYmFjay4KCkFsYW4gU3Rlcm4K