From: Alan Stern <stern@rowland.harvard.edu>
To: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Cc: Dan Scally <dan.scally@ideasonboard.com>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
"rogerq@kernel.org" <rogerq@kernel.org>
Subject: Re: Explicit status phase for DWC3
Date: Thu, 2 Feb 2023 15:15:58 -0500 [thread overview]
Message-ID: <Y9wZ/rgw4CqZ6RxB@rowland.harvard.edu> (raw)
In-Reply-To: <20230202194823.jqhyevhbjw3x7sen@synopsys.com>
On Thu, Feb 02, 2023 at 07:48:29PM +0000, Thinh Nguyen wrote:
> On Thu, Feb 02, 2023, Alan Stern wrote:
> > > > Keep in mind that there are two separate issues here:
> > > >
> > > > Status/data stage for a control-IN or 0-length control-OUT
> > > > transfer.
> > > >
> > > > Status stage for a non-0-length control-OUT transfer.
> > > >
> > > > The USB_GADGET_DELAYED_STATUS mechanism was meant to help with the
> > > > first, not the second. explicit_status was meant to help with the
> > > > second; it may be able to help with both.
>
> While we may not have a convenient function to do the status completion,
> the gadget driver can always use the same mechanism for delayed status
> and explicitly queue the status stage on the OUT data completion. The
> dwc3 driver should already be able to handle that. However, it's better
> to have a convenient function for that, and probably remove any warning
> we have in the composite layer.
Indeed. The difficulty is that gadget drivers have to work with all UDC
drivers, not just with dwc3, and the others may not support explicit
queuing of status transactions. The explicit_status mechanism was
designed to make this work correctly in all cases (or, at least as
correctly as it works now).
> > > Ack - thanks. That thread I linked was very informative, I wish I'd found it
> > > sooner!
> >
> > There is still a race in the gadget layer's handling of control
> > requests. The host can send a SETUP packet at any time. So when a
> > function driver queues a usb_request for ep0, how does the UDC driver
> > know whether it is in response to the SETUP packet that just now arrived
> > or in response to one that arrived earlier (and is now superseded)?
>
> Different stages of different the control transfers shouldn't
> interleave. If a new SETUP packet is received before completing the
> previous control transfer, the previous control transfer is canceled.
> Control transfer doesn't have something like bulk stream-id to allow for
> interleaving.
>
> The gadget driver should handle the different control transfers
> synchronously.
Unfortunately gadget drivers cannot always do that, because sometimes
the work they need to do in response to a control transfer cannot be
carried out in interrupt context. Since ->setup() is called as part of
an interrupt handler, the gadget driver may not be able to complete the
necessary operations before returning from the callback. Therefore the
status stage of a control transfer may have to be handled asynchronously
-- which obviously opens up possibilities for races.
> > This race exists even at the hardware level, and I'm pretty sure that a
> > lot of UDC controllers don't handle it properly. But there's nothing we
> > can do about that...
>
> I can't speak for other controllers, but this is for dwc3 controllers:
>
> If the host sends a new SETUP packet without finishing with the previous
> control transfer data/status, the data/status TRB would be completed
> with "SETUP_PENDING" status. This indicates that the host canceled the
> previous control transfer and the UDC driver needs to setup a SETUP TRB
> to service the new SETUP packet. The previous control transfer would be
> returned with a canceled status.
Hans Selasky mentioned in a recent email that the only controllers he
trusts to get this right are the ones that use a "transaction log" sort
of approach, like xHCI does. I'm not claiming that controllers using
other approaches will always be wrong, but this does explain why dwc3 is
able to manage control transfers correctly.
Alan Stern
next prev parent reply other threads:[~2023-02-02 20:16 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-24 14:27 Explicit status phase for DWC3 Dan Scally
2023-01-26 0:20 ` Thinh Nguyen
2023-01-26 10:30 ` Dan Scally
2023-01-26 19:31 ` Thinh Nguyen
2023-01-26 20:31 ` Alan Stern
2023-01-26 23:57 ` Thinh Nguyen
2023-02-02 10:12 ` Dan Scally
2023-02-02 14:51 ` Roger Quadros
2023-02-02 14:52 ` Alan Stern
2023-02-02 15:45 ` Dan Scally
2023-02-02 16:37 ` Alan Stern
2023-02-02 19:48 ` Thinh Nguyen
2023-02-02 20:15 ` Alan Stern [this message]
2023-04-05 19:35 ` Hans Petter Selasky
2023-02-02 20:01 ` Thinh Nguyen
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=Y9wZ/rgw4CqZ6RxB@rowland.harvard.edu \
--to=stern@rowland.harvard.edu \
--cc=Thinh.Nguyen@synopsys.com \
--cc=dan.scally@ideasonboard.com \
--cc=linux-usb@vger.kernel.org \
--cc=rogerq@kernel.org \
/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.