linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: Paul Elder <paul.elder@ideasonboard.com>
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: Fri, 11 Jan 2019 10:50:11 -0500 (EST)	[thread overview]
Message-ID: <Pine.LNX.4.44L0.1901111044290.1501-100000@iolanthe.rowland.org> (raw)
In-Reply-To: <20190111082340.GB32268@localhost.localdomain>

On Fri, 11 Jan 2019, Paul Elder wrote:

> On Wed, Jan 09, 2019 at 02:06:31PM -0500, Alan Stern wrote:
> > On Wed, 9 Jan 2019, 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 where it might want to is to
> > > asynchronously validate the data of a class-specific request.
> > > 
> > > A function driver that wants an explicit status stage should set the
> > > newly added explicit_status flag of the usb_request corresponding to the
> > > data stage. Later on, the function driver can explicitly complete the
> > > status stage by enqueueing a usb_request for ACK, or calling
> > > usb_ep_set_halt() for STALL.
> > > 
> > > To support both explicit and implicit status stages, a UDC driver must
> > > call the newly added usb_gadget_control_complete function right before
> > > calling usb_gadget_giveback_request. To support the explicit status
> > > stage, it might then check what stage the usb_request was queued in, and
> > > for control IN ACK the host's zero-length data packet, or for control
> > > OUT send a zero-length DATA1 ACK packet.
> > > 
> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > > v4 Acked-by: Alan Stern <stern@rowland.harvard.edu>
> > > v1 Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > This looks good and has passed my tests so far.
> 
> Good! Thank you :)
> 
> > Can you check your uvc
> > changes using dummy_hcd with the patch below?
> 
> I'm not sure what to make of the test results. I get the same results
> with or without the patch. Which I guess makes sense... in dummy_queue,
> this is getting hit when the uvc function driver tries to complete the
> delayed status:
> 
> 	req = usb_request_to_dummy_request(_req);
> 	if (!_req || !list_empty(&req->queue) || !_req->complete)
> 		return -EINVAL;
> 
> So the delayed/explicit status stage is never completed, afaict.

I presume you are hitting the !list_empty(&req->queue) test, yes?  The 
other two tests are trivial.

Triggering the !list_empty() test means the request has already been
submitted and not yet completed.  This probably indicates there is a
bug in the uvc function driver code.

Alan Stern


  reply	other threads:[~2019-01-11 15:50 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 [this message]
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
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=Pine.LNX.4.44L0.1901111044290.1501-100000@iolanthe.rowland.org \
    --to=stern@rowland.harvard.edu \
    --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=paul.elder@ideasonboard.com \
    --cc=rogerq@ti.com \
    /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).