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: [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)
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.
WARNING: multiple messages have this Message-ID (diff)
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 --]
next reply other threads:[~2018-11-07 6:53 UTC|newest]
Thread overview: 61+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-07 6:53 Felipe Balbi [this message]
2018-11-07 6:53 ` [PATCH 4/6] usb: gadget: add functions to signal udc driver to delay status stage Felipe Balbi
-- strict thread matches above, loose matches on Subject: below --
2018-12-14 15:35 [4/6] " Alan Stern
2018-12-14 15:35 ` [PATCH 4/6] " Alan Stern
2018-12-14 3:47 [4/6] " Paul Elder
2018-12-14 3:47 ` [PATCH 4/6] " Paul Elder
2018-11-07 16:23 [4/6] " Alan Stern
2018-11-07 16:23 ` [PATCH 4/6] " Alan Stern
2018-11-07 7:00 [4/6] " Felipe Balbi
2018-11-07 7:00 ` [PATCH 4/6] " Felipe Balbi
2018-11-06 15:01 [4/6] " Alan Stern
2018-11-06 15:01 ` [PATCH 4/6] " Alan Stern
2018-11-06 14:51 [4/6] " Alan Stern
2018-11-06 14:51 ` [PATCH 4/6] " Alan Stern
2018-11-06 11:24 [4/6] " Felipe Balbi
2018-11-06 11:24 ` [PATCH 4/6] " Felipe Balbi
2018-11-06 11:17 [4/6] " Felipe Balbi
2018-11-06 11:17 ` [PATCH 4/6] " Felipe Balbi
2018-11-02 19:46 [4/6] " Alan Stern
2018-11-02 19:46 ` [PATCH 4/6] " Alan Stern
2018-11-02 17:10 [4/6] " Laurent Pinchart
2018-11-02 17:10 ` [PATCH 4/6] " Laurent Pinchart
2018-11-02 16:18 [4/6] " Alan Stern
2018-11-02 16:18 ` [PATCH 4/6] " Alan Stern
2018-11-02 14:36 [4/6] " Laurent Pinchart
2018-11-02 14:36 ` [PATCH 4/6] " Laurent Pinchart
2018-11-02 12:44 [4/6] " Laurent Pinchart
2018-11-02 12:44 ` [PATCH 4/6] " Laurent Pinchart
2018-11-01 23:40 [4/6] " Paul Elder
2018-11-01 23:40 ` [PATCH 4/6] " Paul Elder
2018-10-31 23:26 [5/6] usb: musb: gadget: implement send_response Paul Elder
2018-10-31 23:26 ` [PATCH 5/6] " Paul Elder
2018-10-18 14:07 [4/6] usb: gadget: add functions to signal udc driver to delay status stage Alan Stern
2018-10-18 14:07 ` [PATCH 4/6] " Alan Stern
2018-10-18 12:46 [4/6] " Bin Liu
2018-10-18 12:46 ` [PATCH 4/6] " Bin Liu
2018-10-17 23:45 [4/6] " Laurent Pinchart
2018-10-17 23:45 ` [PATCH 4/6] " Laurent Pinchart
2018-10-11 16:10 [4/6] " Bin Liu
2018-10-11 16:10 ` [PATCH 4/6] " Bin Liu
2018-10-11 16:07 [5/6] usb: musb: gadget: implement send_response Bin Liu
2018-10-11 16:07 ` [PATCH 5/6] " Bin Liu
2018-10-10 13:42 [1/6] usb: uvc: include videodev2.h in g_uvc.h Laurent Pinchart
2018-10-10 13:42 ` [PATCH 1/6] " Laurent Pinchart
2018-10-10 2:49 [6/6] usb: gadget: uvc: allow ioctl to send response in status stage Paul Elder
2018-10-10 2:49 ` [PATCH 6/6] " Paul Elder
2018-10-10 2:49 [5/6] usb: musb: gadget: implement send_response Paul Elder
2018-10-10 2:49 ` [PATCH 5/6] " Paul Elder
2018-10-10 2:49 [4/6] usb: gadget: add functions to signal udc driver to delay status stage Paul Elder
2018-10-10 2:49 ` [PATCH 4/6] " Paul Elder
2018-10-10 2:49 [3/6] usb: gadget: uvc: package setup and data for control OUT requests Paul Elder
2018-10-10 2:49 ` [PATCH 3/6] " Paul Elder
2018-10-10 2:48 [2/6] usb: gadget: uvc: enqueue usb request in setup handler for control OUT Paul Elder
2018-10-10 2:48 ` [PATCH 2/6] " Paul Elder
2018-10-10 2:48 [1/6] usb: uvc: include videodev2.h in g_uvc.h Paul Elder
2018-10-10 2:48 ` [PATCH 1/6] " Paul Elder
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 12:57 ` 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 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.