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=-6.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,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 C378BC5ACCC for ; Thu, 18 Oct 2018 14:07:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7C1192145D for ; Thu, 18 Oct 2018 14:07:39 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7C1192145D 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 S1728241AbeJRWIs (ORCPT ); Thu, 18 Oct 2018 18:08:48 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:45852 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1727062AbeJRWIs (ORCPT ); Thu, 18 Oct 2018 18:08:48 -0400 Received: (qmail 1612 invoked by uid 2102); 18 Oct 2018 10:07:36 -0400 Received: from localhost (sendmail-bs@127.0.0.1) by localhost with SMTP; 18 Oct 2018 10:07:36 -0400 Date: Thu, 18 Oct 2018 10:07:36 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Laurent Pinchart cc: Bin Liu , Paul Elder , , , , , , Subject: Re: [PATCH 4/6] usb: gadget: add functions to signal udc driver to delay status stage In-Reply-To: <3055233.Lvy0NcWSZ5@avalon> 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 Thu, 18 Oct 2018, Laurent Pinchart wrote: > Hi Bin, > > On Thursday, 11 October 2018 19:10:21 EEST Bin Liu wrote: > > On Tue, Oct 09, 2018 at 10:49:01PM -0400, Paul Elder wrote: > > > A usb gadget function driver may or may not want to delay the status > > > stage of a control OUT request. An instance it might want to is to > > > asynchronously validate the data of a class-specific request. > > > > > > Add a function usb_ep_delay_status to allow function drivers to choose > > > to delay the status stage in the request completion handler. The UDC > > > should then check the usb_ep->delayed_status flag and act accordingly to > > > delay the status stage. > > > > > > Also add a function usb_ep_send_response as a wrapper for > > > usb_ep->ops->send_response, whose prototype is added as well. This > > > function should be called by function drivers to tell the UDC what to > > > reply in the status stage that it has requested to be delayed. > > > > > > Signed-off-by: Paul Elder > > > Reviewed-by: Laurent Pinchart > > > --- > > > > > > drivers/usb/gadget/udc/core.c | 35 +++++++++++++++++++++++++++++++++++ > > > include/linux/usb/gadget.h | 11 +++++++++++ > > > 2 files changed, 46 insertions(+) > > > > > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > > > index af88b48c1cea..1ec5ce6b43cd 100644 > > > --- a/drivers/usb/gadget/udc/core.c > > > +++ b/drivers/usb/gadget/udc/core.c > > > @@ -443,6 +443,41 @@ void usb_ep_fifo_flush(struct usb_ep *ep) > > > } > > > EXPORT_SYMBOL_GPL(usb_ep_fifo_flush); > > > > > > +/** > > > + * usb_ep_ep_delay_status - set delay_status flag > > > + * @ep: the endpoint whose delay_status flag is being set > > > + * > > > + * This function instructs the UDC to delay the status stage of a control > > > + * request. It can only be called from the request completion handler of > > > a > > > + * control request. > > > + */ > > > +void usb_ep_delay_status(struct usb_ep *ep) > > > +{ > > > + ep->delayed_status = true; > > > +} > > > +EXPORT_SYMBOL_GPL(usb_ep_delay_status); > > > > Is usb_ep_set_delay_status() better? I thought it implies get/return > > action if a verb is missing in the function name. > > For what it's worth, I understand the function name as "delay the status > stage", with "delay" being a verb. Maybe the short description could be > updated accordingly. Is there a reason for adding a new function for this? This is exactly what the USB_GADGET_DELAYED_STATUS return value from the setup callback is meant for (and it is already used by some gadget drivers). Is it a question of when the gadget driver learns that it will need to delay the status stage? If that's the case, why not always return USB_GADGET_DELAYED_STATUS from the setup callback? Then instead of calling usb_ep_delay_status() when a delay is needed, you could queue the status request when a delay isn't needed. As a more general solution, Felipe has said that a UDC driver should _never_ carry out the status stage transaction until the gadget driver has told it to do so. Then there would be no need for any sort of delay indicator. (But implementing this would require significant changes to a bunch of different drivers...) 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: Thu, 18 Oct 2018 10:07:36 -0400 (EDT) To: Laurent Pinchart Cc: Bin Liu , Paul Elder , kieran.bingham@ideasonboard.com, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, balbi@kernel.org, rogerq@ti.com List-ID: T24gVGh1LCAxOCBPY3QgMjAxOCwgTGF1cmVudCBQaW5jaGFydCB3cm90ZToKCj4gSGkgQmluLAo+ IAo+IE9uIFRodXJzZGF5LCAxMSBPY3RvYmVyIDIwMTggMTk6MTA6MjEgRUVTVCBCaW4gTGl1IHdy b3RlOgo+ID4gT24gVHVlLCBPY3QgMDksIDIwMTggYXQgMTA6NDk6MDFQTSAtMDQwMCwgUGF1bCBF bGRlciB3cm90ZToKPiA+ID4gQSB1c2IgZ2FkZ2V0IGZ1bmN0aW9uIGRyaXZlciBtYXkgb3IgbWF5 IG5vdCB3YW50IHRvIGRlbGF5IHRoZSBzdGF0dXMKPiA+ID4gc3RhZ2Ugb2YgYSBjb250cm9sIE9V VCByZXF1ZXN0LiBBbiBpbnN0YW5jZSBpdCBtaWdodCB3YW50IHRvIGlzIHRvCj4gPiA+IGFzeW5j aHJvbm91c2x5IHZhbGlkYXRlIHRoZSBkYXRhIG9mIGEgY2xhc3Mtc3BlY2lmaWMgcmVxdWVzdC4K PiA+ID4gCj4gPiA+IEFkZCBhIGZ1bmN0aW9uIHVzYl9lcF9kZWxheV9zdGF0dXMgdG8gYWxsb3cg ZnVuY3Rpb24gZHJpdmVycyB0byBjaG9vc2UKPiA+ID4gdG8gZGVsYXkgdGhlIHN0YXR1cyBzdGFn ZSBpbiB0aGUgcmVxdWVzdCBjb21wbGV0aW9uIGhhbmRsZXIuIFRoZSBVREMKPiA+ID4gc2hvdWxk IHRoZW4gY2hlY2sgdGhlIHVzYl9lcC0+ZGVsYXllZF9zdGF0dXMgZmxhZyBhbmQgYWN0IGFjY29y ZGluZ2x5IHRvCj4gPiA+IGRlbGF5IHRoZSBzdGF0dXMgc3RhZ2UuCj4gPiA+IAo+ID4gPiBBbHNv IGFkZCBhIGZ1bmN0aW9uIHVzYl9lcF9zZW5kX3Jlc3BvbnNlIGFzIGEgd3JhcHBlciBmb3IKPiA+ ID4gdXNiX2VwLT5vcHMtPnNlbmRfcmVzcG9uc2UsIHdob3NlIHByb3RvdHlwZSBpcyBhZGRlZCBh cyB3ZWxsLiBUaGlzCj4gPiA+IGZ1bmN0aW9uIHNob3VsZCBiZSBjYWxsZWQgYnkgZnVuY3Rpb24g ZHJpdmVycyB0byB0ZWxsIHRoZSBVREMgd2hhdCB0bwo+ID4gPiByZXBseSBpbiB0aGUgc3RhdHVz IHN0YWdlIHRoYXQgaXQgaGFzIHJlcXVlc3RlZCB0byBiZSBkZWxheWVkLgo+ID4gPiAKPiA+ID4g U2lnbmVkLW9mZi1ieTogUGF1bCBFbGRlciA8cGF1bC5lbGRlckBpZGVhc29uYm9hcmQuY29tPgo+ ID4gPiBSZXZpZXdlZC1ieTogTGF1cmVudCBQaW5jaGFydCA8bGF1cmVudC5waW5jaGFydEBpZGVh c29uYm9hcmQuY29tPgo+ID4gPiAtLS0KPiA+ID4gCj4gPiA+ICBkcml2ZXJzL3VzYi9nYWRnZXQv dWRjL2NvcmUuYyB8IDM1ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrCj4gPiA+ ICBpbmNsdWRlL2xpbnV4L3VzYi9nYWRnZXQuaCAgICB8IDExICsrKysrKysrKysrCj4gPiA+ICAy IGZpbGVzIGNoYW5nZWQsIDQ2IGluc2VydGlvbnMoKykKPiA+ID4gCj4gPiA+IGRpZmYgLS1naXQg YS9kcml2ZXJzL3VzYi9nYWRnZXQvdWRjL2NvcmUuYyBiL2RyaXZlcnMvdXNiL2dhZGdldC91ZGMv Y29yZS5jCj4gPiA+IGluZGV4IGFmODhiNDhjMWNlYS4uMWVjNWNlNmI0M2NkIDEwMDY0NAo+ID4g PiAtLS0gYS9kcml2ZXJzL3VzYi9nYWRnZXQvdWRjL2NvcmUuYwo+ID4gPiArKysgYi9kcml2ZXJz L3VzYi9nYWRnZXQvdWRjL2NvcmUuYwo+ID4gPiBAQCAtNDQzLDYgKzQ0Myw0MSBAQCB2b2lkIHVz Yl9lcF9maWZvX2ZsdXNoKHN0cnVjdCB1c2JfZXAgKmVwKQo+ID4gPiAgfQo+ID4gPiAgRVhQT1JU X1NZTUJPTF9HUEwodXNiX2VwX2ZpZm9fZmx1c2gpOwo+ID4gPiAKPiA+ID4gKy8qKgo+ID4gPiAr ICogdXNiX2VwX2VwX2RlbGF5X3N0YXR1cyAtIHNldCBkZWxheV9zdGF0dXMgZmxhZwo+ID4gPiAr ICogQGVwOiB0aGUgZW5kcG9pbnQgd2hvc2UgZGVsYXlfc3RhdHVzIGZsYWcgaXMgYmVpbmcgc2V0 Cj4gPiA+ICsgKgo+ID4gPiArICogVGhpcyBmdW5jdGlvbiBpbnN0cnVjdHMgdGhlIFVEQyB0byBk ZWxheSB0aGUgc3RhdHVzIHN0YWdlIG9mIGEgY29udHJvbAo+ID4gPiArICogcmVxdWVzdC4gSXQg Y2FuIG9ubHkgYmUgY2FsbGVkIGZyb20gdGhlIHJlcXVlc3QgY29tcGxldGlvbiBoYW5kbGVyIG9m Cj4gPiA+IGEKPiA+ID4gKyAqIGNvbnRyb2wgcmVxdWVzdC4KPiA+ID4gKyAqLwo+ID4gPiArdm9p ZCB1c2JfZXBfZGVsYXlfc3RhdHVzKHN0cnVjdCB1c2JfZXAgKmVwKQo+ID4gPiArewo+ID4gPiAr CWVwLT5kZWxheWVkX3N0YXR1cyA9IHRydWU7Cj4gPiA+ICt9Cj4gPiA+ICtFWFBPUlRfU1lNQk9M X0dQTCh1c2JfZXBfZGVsYXlfc3RhdHVzKTsKPiA+IAo+ID4gSXMgdXNiX2VwX3NldF9kZWxheV9z dGF0dXMoKSBiZXR0ZXI/IEkgdGhvdWdodCBpdCBpbXBsaWVzIGdldC9yZXR1cm4KPiA+IGFjdGlv biBpZiBhIHZlcmIgaXMgbWlzc2luZyBpbiB0aGUgZnVuY3Rpb24gbmFtZS4KPiAKPiBGb3Igd2hh dCBpdCdzIHdvcnRoLCBJIHVuZGVyc3RhbmQgdGhlIGZ1bmN0aW9uIG5hbWUgYXMgImRlbGF5IHRo ZSBzdGF0dXMgCj4gc3RhZ2UiLCB3aXRoICJkZWxheSIgYmVpbmcgYSB2ZXJiLiBNYXliZSB0aGUg c2hvcnQgZGVzY3JpcHRpb24gY291bGQgYmUgCj4gdXBkYXRlZCBhY2NvcmRpbmdseS4KCklzIHRo ZXJlIGEgcmVhc29uIGZvciBhZGRpbmcgYSBuZXcgZnVuY3Rpb24gZm9yIHRoaXM/ICBUaGlzIGlz IGV4YWN0bHkKd2hhdCB0aGUgVVNCX0dBREdFVF9ERUxBWUVEX1NUQVRVUyByZXR1cm4gdmFsdWUg ZnJvbSB0aGUgc2V0dXAgY2FsbGJhY2sKaXMgbWVhbnQgZm9yIChhbmQgaXQgaXMgYWxyZWFkeSB1 c2VkIGJ5IHNvbWUgZ2FkZ2V0IGRyaXZlcnMpLgoKSXMgaXQgYSBxdWVzdGlvbiBvZiB3aGVuIHRo ZSBnYWRnZXQgZHJpdmVyIGxlYXJucyB0aGF0IGl0IHdpbGwgbmVlZCB0bwpkZWxheSB0aGUgc3Rh dHVzIHN0YWdlPyAgSWYgdGhhdCdzIHRoZSBjYXNlLCB3aHkgbm90IGFsd2F5cyByZXR1cm4KVVNC X0dBREdFVF9ERUxBWUVEX1NUQVRVUyBmcm9tIHRoZSBzZXR1cCBjYWxsYmFjaz8gIFRoZW4gaW5z dGVhZCBvZgpjYWxsaW5nIHVzYl9lcF9kZWxheV9zdGF0dXMoKSB3aGVuIGEgZGVsYXkgaXMgbmVl ZGVkLCB5b3UgY291bGQgcXVldWUKdGhlIHN0YXR1cyByZXF1ZXN0IHdoZW4gYSBkZWxheSBpc24n dCBuZWVkZWQuCgpBcyBhIG1vcmUgZ2VuZXJhbCBzb2x1dGlvbiwgRmVsaXBlIGhhcyBzYWlkIHRo YXQgYSBVREMgZHJpdmVyIHNob3VsZCAKX25ldmVyXyBjYXJyeSBvdXQgdGhlIHN0YXR1cyBzdGFn ZSB0cmFuc2FjdGlvbiB1bnRpbCB0aGUgZ2FkZ2V0IGRyaXZlciAKaGFzIHRvbGQgaXQgdG8gZG8g c28uICBUaGVuIHRoZXJlIHdvdWxkIGJlIG5vIG5lZWQgZm9yIGFueSBzb3J0IG9mIApkZWxheSBp bmRpY2F0b3IuICAoQnV0IGltcGxlbWVudGluZyB0aGlzIHdvdWxkIHJlcXVpcmUgc2lnbmlmaWNh bnQgCmNoYW5nZXMgdG8gYSBidW5jaCBvZiBkaWZmZXJlbnQgZHJpdmVycy4uLikKCkFsYW4gU3Rl cm4K