linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paul Elder <paul.elder@ideasonboard.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: laurent.pinchart@ideasonboard.com,
	kieran.bingham@ideasonboard.com, b-liu@ti.com, rogerq@ti.com,
	balbi@kernel.org, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 4/6] usb: gadget: add mechanism to specify an explicit status stage
Date: Sun, 20 Jan 2019 12:59:29 -0500	[thread overview]
Message-ID: <20190120175929.GF7331@localhost.localdomain> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1901181148100.1425-100000@iolanthe.rowland.org>

On Fri, Jan 18, 2019 at 11:52:57AM -0500, Alan Stern wrote:
> On Fri, 18 Jan 2019, Paul Elder wrote:
> 
> > > > I meant the functions (procedures) in the function driver, so the setup
> > > > handler (uvc_function_setup), the completion handler
> > > > (uvc_function_ep0_complete), and the status sender (uvc_send_response),
> > > > although the last one actually sends the data stage for control IN.
> > > > So after the status is sent on the uvc gadget driver's end, its
> > > > completion handler is called again without the setup handler being
> > > > called beforehand and I cant figure out why.
> > > 
> > > Isn't this what you should expect?  Every usb_request, if it is queued
> > > successfully, eventually gets a completion callback.  That promise is
> > > made by every UDC driver; it's part of the gadget API.  So for a
> > > control transfer with a data stage, you expect to have:
> > > 
> > > 	Setup handler called
> > > 	Data-stage request submitted
> > > 	Data-stage request completion callback
> > > 	Status-stage request submitted
> > > 	Status-stage request completion callback
> > > 
> > > Thus, two completion callbacks but only one setup callback.
> > 
> > omg how did I not notice this :/
> > 
> > I guess I have to fix the uvc function driver so it works with that.
> > musb doesn't call the status stage completion callback though; not that
> > it does anything so it seems fine to me, but indeed the function driver
> > has to be ready for it if it is called.
> 
> musb _has_ to call the status-stage completion callback.  As just one
> reason, if the explicit_status flag isn't set then that callback is
> responsible for deallocating the status request.  Without it, the
> status request will leak.

Ah, I see what you mean. I forgot about that because we reuse the
request in uvc gadget. I'll have to add the status stage completion
callback to musb too, then.


Thanks,

Paul

  reply	other threads:[~2019-01-20 18:00 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-09  7:08 [PATCH v5 0/6] usb: gadget: add mechanism to asynchronously validate data stage of ctrl out request Paul Elder
2019-01-09  7:08 ` [PATCH v5 1/6] usb: uvc: include videodev2.h in g_uvc.h Paul Elder
2019-01-09  7:08 ` [PATCH v5 2/6] usb: gadget: uvc: enqueue usb request in setup handler for control OUT Paul Elder
2019-01-09  7:08 ` [PATCH v5 3/6] usb: gadget: uvc: package setup and data for control OUT requests Paul Elder
2019-01-09  7:08 ` [PATCH v5 4/6] usb: gadget: add mechanism to specify an explicit status stage Paul Elder
2019-01-09 19:06   ` Alan Stern
2019-01-11  8:23     ` Paul Elder
2019-01-11 15:50       ` Alan Stern
2019-01-14  5:11         ` Paul Elder
2019-01-14 15:24           ` Alan Stern
2019-01-16  5:00             ` Paul Elder
2019-01-16 15:06               ` Alan Stern
2019-01-18 16:31                 ` Paul Elder
2019-01-18 16:52                   ` Alan Stern
2019-01-20 17:59                     ` Paul Elder [this message]
2019-01-23 21:10           ` Alan Stern
2019-01-24  2:48             ` Paul Elder
2019-01-09  7:08 ` [PATCH v5 5/6] usb: musb: gadget: implement optional " Paul Elder
2019-01-09  7:08 ` [PATCH v5 6/6] usb: gadget: uvc: allow ioctl to send response in " Paul Elder
2019-01-10 20:39 ` [PATCH v5 0/6] usb: gadget: add mechanism to asynchronously validate data stage of ctrl out request Alan Stern
2019-01-11  8:43   ` Paul Elder
2019-01-11 18:32     ` Alan Stern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190120175929.GF7331@localhost.localdomain \
    --to=paul.elder@ideasonboard.com \
    --cc=b-liu@ti.com \
    --cc=balbi@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=kieran.bingham@ideasonboard.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=rogerq@ti.com \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).