All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: Paul Elder <paul.elder@ideasonboard.com>
Cc: Felipe Balbi <balbi@kernel.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Bin Liu <b-liu@ti.com>, <kieran.bingham@ideasonboard.com>,
	<gregkh@linuxfoundation.org>, <linux-usb@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <rogerq@ti.com>
Subject: Re: [PATCH 4/6] usb: gadget: add functions to signal udc driver to delay status stage
Date: Fri, 14 Dec 2018 10:35:58 -0500 (EST)	[thread overview]
Message-ID: <Pine.LNX.4.44L0.1812141034280.1570-100000@iolanthe.rowland.org> (raw)
In-Reply-To: <20181214034754.GB7477@garnet.amanokami.net>

On Thu, 13 Dec 2018, Paul Elder wrote:

> > Suppose we have a core library routine like this:
> > 
> > void usb_gadget_control_complete(struct usb_gadget *gadget,
> > 		unsigned int no_implicit_status, int status)
> > {
> > 	struct usb_request *req;
> > 
> > 	if (no_implicit_status || status != 0)
> > 		return;
> > 
> > 	/* Send an implicit status-stage request for ep0 */
> > 	req = usb_ep_alloc_request(gadget->ep0, GFP_ATOMIC);
> > 	if (req) {
> > 		req->length = 0;
> > 		req->no_implicit_status = 1;
> > 		req->complete = /* req's deallocation routine */
> > 		usb_ep_queue(gadget->ep0, req, GFP_ATOMIC);
> > 	}
> > }
> > 
> > Then all a UDC driver would need to do is call 
> > usb_gadget_control_complete() after invoking a control request's 
> > completion handler.  The no_implicit_status and status arguments would 
> > be taken from the request that was just completed.
> > 
> > With this one call added to each UDC, all the existing function drivers
> > would work correctly.  Even though they don't explicitly queue
> > status-stage requests, the new routine will do so for them,
> > transparently.  Function drivers that want to handle their own
> > status-stage requests explicitly will merely have to set the
> > req->no_implicit_status bit.
> 
> I think this is a good idea. We still get the benefits of explicit
> status stage without being overly intrusive in the conversion, and we
> maintain the queue-based API.
> 
> Would it be fine for me to proceed in this direction for a v2?

It is as far as I'm concerned (Felipe might not agree).  Knock yourself
out.  :-)

Alan Stern

> > (We might or might not need to watch out for 0-length control-OUT 
> > transfers.  Function drivers _do_ queue status-stage requests for 
> > those.)
> 
> Thanks,
> 
> Paul Elder


WARNING: multiple messages have this Message-ID (diff)
From: Alan Stern <stern@rowland.harvard.edu>
To: Paul Elder <paul.elder@ideasonboard.com>
Cc: Felipe Balbi <balbi@kernel.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Bin Liu <b-liu@ti.com>,
	kieran.bingham@ideasonboard.com, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	rogerq@ti.com
Subject: [4/6] usb: gadget: add functions to signal udc driver to delay status stage
Date: Fri, 14 Dec 2018 10:35:58 -0500 (EST)	[thread overview]
Message-ID: <Pine.LNX.4.44L0.1812141034280.1570-100000@iolanthe.rowland.org> (raw)

On Thu, 13 Dec 2018, Paul Elder wrote:

> > Suppose we have a core library routine like this:
> > 
> > void usb_gadget_control_complete(struct usb_gadget *gadget,
> > 		unsigned int no_implicit_status, int status)
> > {
> > 	struct usb_request *req;
> > 
> > 	if (no_implicit_status || status != 0)
> > 		return;
> > 
> > 	/* Send an implicit status-stage request for ep0 */
> > 	req = usb_ep_alloc_request(gadget->ep0, GFP_ATOMIC);
> > 	if (req) {
> > 		req->length = 0;
> > 		req->no_implicit_status = 1;
> > 		req->complete = /* req's deallocation routine */
> > 		usb_ep_queue(gadget->ep0, req, GFP_ATOMIC);
> > 	}
> > }
> > 
> > Then all a UDC driver would need to do is call 
> > usb_gadget_control_complete() after invoking a control request's 
> > completion handler.  The no_implicit_status and status arguments would 
> > be taken from the request that was just completed.
> > 
> > With this one call added to each UDC, all the existing function drivers
> > would work correctly.  Even though they don't explicitly queue
> > status-stage requests, the new routine will do so for them,
> > transparently.  Function drivers that want to handle their own
> > status-stage requests explicitly will merely have to set the
> > req->no_implicit_status bit.
> 
> I think this is a good idea. We still get the benefits of explicit
> status stage without being overly intrusive in the conversion, and we
> maintain the queue-based API.
> 
> Would it be fine for me to proceed in this direction for a v2?

It is as far as I'm concerned (Felipe might not agree).  Knock yourself
out.  :-)

Alan Stern

> > (We might or might not need to watch out for 0-length control-OUT 
> > transfers.  Function drivers _do_ queue status-stage requests for 
> > those.)
> 
> Thanks,
> 
> Paul Elder

  reply	other threads:[~2018-12-14 15:36 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-10  2:48 [PATCH 0/6] usb: gadget: add mechanism to asynchronously validate data stage of ctrl out request Paul Elder
2018-10-10  2:48 ` [PATCH 1/6] usb: uvc: include videodev2.h in g_uvc.h Paul Elder
2018-10-10  2:48   ` [1/6] " Paul Elder
2018-10-10 13:42   ` [PATCH 1/6] " Laurent Pinchart
2018-10-10 13:42     ` [1/6] " Laurent Pinchart
2018-10-10  2:48 ` [PATCH 2/6] usb: gadget: uvc: enqueue usb request in setup handler for control OUT Paul Elder
2018-10-10  2:48   ` [2/6] " Paul Elder
2018-10-10  2:49 ` [PATCH 3/6] usb: gadget: uvc: package setup and data for control OUT requests Paul Elder
2018-10-10  2:49   ` [3/6] " Paul Elder
2018-10-10  2:49 ` [PATCH 4/6] usb: gadget: add functions to signal udc driver to delay status stage Paul Elder
2018-10-10  2:49   ` [4/6] " Paul Elder
2018-10-11 16:10   ` [PATCH 4/6] " Bin Liu
2018-10-11 16:10     ` [4/6] " Bin Liu
2018-10-17 23:45     ` [PATCH 4/6] " Laurent Pinchart
2018-10-17 23:45       ` [4/6] " Laurent Pinchart
2018-10-18 12:46       ` [PATCH 4/6] " Bin Liu
2018-10-18 12:46         ` [4/6] " Bin Liu
2018-10-18 14:07       ` [PATCH 4/6] " Alan Stern
2018-10-18 14:07         ` [4/6] " Alan Stern
2018-11-01 23:40         ` [PATCH 4/6] " Paul Elder
2018-11-01 23:40           ` [4/6] " Paul Elder
2018-11-02 12:44           ` [PATCH 4/6] " Laurent Pinchart
2018-11-02 12:44             ` [4/6] " Laurent Pinchart
     [not found]             ` <87h8gzy5y7.fsf@linux.intel.com>
2018-11-02 14:36               ` [PATCH 4/6] " Laurent Pinchart
2018-11-02 14:36                 ` [4/6] " Laurent Pinchart
2018-11-02 16:18                 ` [PATCH 4/6] " Alan Stern
2018-11-02 16:18                   ` [4/6] " Alan Stern
2018-11-02 17:10                   ` [PATCH 4/6] " Laurent Pinchart
2018-11-02 17:10                     ` [4/6] " Laurent Pinchart
2018-11-02 19:46                     ` [PATCH 4/6] " Alan Stern
2018-11-02 19:46                       ` [4/6] " Alan Stern
2018-11-06 11:24                       ` [PATCH 4/6] " Felipe Balbi
2018-11-06 11:24                         ` [4/6] " Felipe Balbi
2018-11-06 15:01                         ` [PATCH 4/6] " Alan Stern
2018-11-06 15:01                           ` [4/6] " Alan Stern
2018-11-07  6:53                           ` [PATCH 4/6] " Felipe Balbi
2018-11-07  6:53                             ` [4/6] " Felipe Balbi
2018-11-06 11:17                     ` [PATCH 4/6] " Felipe Balbi
2018-11-06 11:17                       ` [4/6] " Felipe Balbi
2018-11-06 14:51                       ` [PATCH 4/6] " Alan Stern
2018-11-06 14:51                         ` [4/6] " Alan Stern
2018-11-07  7:00                         ` [PATCH 4/6] " Felipe Balbi
2018-11-07  7:00                           ` [4/6] " Felipe Balbi
2018-11-07 16:23                           ` [PATCH 4/6] " Alan Stern
2018-11-07 16:23                             ` [4/6] " Alan Stern
2018-12-14  3:47                             ` [PATCH 4/6] " Paul Elder
2018-12-14  3:47                               ` [4/6] " Paul Elder
2018-12-14 15:35                               ` Alan Stern [this message]
2018-12-14 15:35                                 ` Alan Stern
2018-10-10  2:49 ` [PATCH 5/6] usb: musb: gadget: implement send_response Paul Elder
2018-10-10  2:49   ` [5/6] " Paul Elder
2018-10-11 16:07   ` [PATCH 5/6] " Bin Liu
2018-10-11 16:07     ` [5/6] " Bin Liu
2018-10-31 23:26     ` [PATCH 5/6] " Paul Elder
2018-10-31 23:26       ` [5/6] " Paul Elder
2018-10-10  2:49 ` [PATCH 6/6] usb: gadget: uvc: allow ioctl to send response in status stage Paul Elder
2018-10-10  2:49   ` [6/6] " Paul Elder
2018-10-10 12:57 ` [PATCH 0/6] usb: gadget: add mechanism to asynchronously validate data stage of ctrl out request Laurent Pinchart
2018-10-11 19:31 ` Bin Liu
2018-10-17 23:42   ` Laurent Pinchart
2018-10-18 12:40     ` Bin Liu

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.1812141034280.1570-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.