mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Felipe Balbi <balbi@kernel.org>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Paul Elder <paul.elder@ideasonboard.com>, Bin Liu <b-liu@ti.com>,
	kieran.bingham@ideasonboard.com, gregkh@linuxfoundation.org,
	USB list <linux-usb@vger.kernel.org>,
	Kernel development list <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: Wed, 07 Nov 2018 08:53:43 +0200	[thread overview]
Message-ID: <87tvkttm2w.fsf@linux.intel.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1811060951100.1450-100000@iolanthe.rowland.org>

[-- Attachment #1: Type: text/plain, Size: 2431 bytes --]


Hi,

Alan Stern <stern@rowland.harvard.edu> writes:
>> Alan Stern <stern@rowland.harvard.edu> writes:
>> > There's a similar race at the hardware level.  What happens if the
>> > controller receives a new SETUP packet and concurrently the driver is
>> > setting up the controller registers for a response to an earlier
>> > SETUP?  I don't know how real controllers handle this.
>> 
>> That's HW implementation detail. DWC3, for instance, will ignore the
>> TRBs and return me the status "setup packet pending". Then I just start
>> a new SETUP TRB.
>
> You mean the UDC hardware sets a "setup pending" flag in some register,
> and then ignores any attempts to do anything with ep0 until the driver
> clears this flag?

Yes, except that the "flag" is a status on the TRB itself (TRB is dwc3's
DMA transfer descriptor).

>> > You mean, should we allow function drivers to queue the data-stage
>> > request after the setup handler has returned?  I don't see any reason
>> 
>> that's already done:
>> 
>> static void dwc3_ep0_xfer_complete(struct dwc3 *dwc,
>> 			const struct dwc3_event_depevt *event)
>> {
>> 	struct dwc3_ep		*dep = dwc->eps[event->endpoint_number];
>> 
>> 	dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
>> 	dep->resource_index = 0;
>> 	dwc->setup_packet_pending = false;
>> 
>> 	switch (dwc->ep0state) {
>> 	case EP0_SETUP_PHASE:
>> 		dwc3_ep0_inspect_setup(dwc, event);
>> 		break;
>> [...]
>> }
>
> ...
>
> You mean, it's already done in DWC3.  What about other UDC drivers?

if they're not implementing this possibility, then that's a bug on
those UDC drivers :)

>> > why not.  After all, some drivers may require this.  Likewise for the 
>> > data stage of a control-IN.
>> >
>> > Another thing we should do is give function drivers a way to send a
>> > STALL response for the status stage.  Currently there's no way to do
>> > it, if a data stage is present.
>> 
>> Status stage can only be stalled if host tries to move data on the wrong
>> direction.
>
> The USB-2 spec disagrees.  See Table 8-7 in section 8.5.3.1 and the
> following paragraphs.  (Although, I can't see why a function would ever
> fail to complete the command sequence for a control-IN transfer after
> the data had already been sent.)

I can't see how we could ever STALL after returning the data requested
by the host. Seems like that wasn't well thought out.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

  reply	other threads:[~2018-11-07  6:53 UTC|newest]

Thread overview: 33+ 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 13:42   ` 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:49 ` [PATCH 3/6] usb: gadget: uvc: package setup and data for control OUT requests 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-11 16:10   ` Bin Liu
2018-10-17 23:45     ` Laurent Pinchart
2018-10-18 12:46       ` Bin Liu
2018-10-18 14:07       ` Alan Stern
2018-11-01 23:40         ` Paul Elder
2018-11-02 12:44           ` Laurent Pinchart
     [not found]             ` <87h8gzy5y7.fsf@linux.intel.com>
2018-11-02 14:36               ` Laurent Pinchart
2018-11-02 16:18                 ` Alan Stern
2018-11-02 17:10                   ` Laurent Pinchart
2018-11-02 19:46                     ` Alan Stern
2018-11-06 11:24                       ` Felipe Balbi
2018-11-06 15:01                         ` Alan Stern
2018-11-07  6:53                           ` Felipe Balbi [this message]
2018-11-06 11:17                     ` Felipe Balbi
2018-11-06 14:51                       ` Alan Stern
2018-11-07  7:00                         ` Felipe Balbi
2018-11-07 16:23                           ` Alan Stern
2018-12-14  3:47                             ` Paul Elder
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-11 16:07   ` Bin Liu
2018-10-31 23:26     ` 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 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=87tvkttm2w.fsf@linux.intel.com \
    --to=balbi@kernel.org \
    --cc=b-liu@ti.com \
    --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 \
    --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

Powered by JetHome