On Fri, Feb 09, 2018 at 09:59:46AM +0800, Peter Chen wrote:
> On Wed, Feb 07, 2018 at 03:43:23PM +0200, Mathias Nyman wrote:
> > On 07.02.2018 11:45, Peter Chen wrote:
> > >Hi Mathias,
> > >
> > >I am implementing USB2 EHSET SINGLE_STEP_SET_FEATURE Test for XHCI port,
> > >(see ehset_single_step_set_feature for EHCI), it needs to set IOC
> > >for setup packet, and software waits 15 seconds before DATA + STATUS stage.
> > >After porting such design for XHCI, it triggers above warning, and
> > >returns -ESHUTDOWN for URB. Any reasons why we don't allow completion
> > >interrupt for SETUP stage? Thanks.
> > >
> >
> > Current xhci driver control transfer implementation doesn't support
> > queuing a control transfer in parts. we set the IOC only for the status
> > stage, and we always queue a status stage (and possibly data stage) automatically
> > when queuing a control transfer URB.
> >
> > If we get a success event for the SETUP stage the driver will finish the TD and
> > return the whole URB.
> >
> > For testing purposes you would need to make sure we don't call finish_td() for
> > a success event at the SETUP stage, something like this (untested):
> >
> > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> > index c5cbc68..f6d005e 100644
> > --- a/drivers/usb/host/xhci-ring.c
> > +++ b/drivers/usb/host/xhci-ring.c
> > @@ -2014,12 +2014,8 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td,
> > switch (trb_comp_code) {
> > case COMP_SUCCESS:
> > - if (trb_type != TRB_STATUS) {
> > - xhci_warn(xhci, "WARN: Success on ctrl %s TRB without IOC set?\n",
> > - (trb_type == TRB_DATA) ? "data" : "setup");
> > - *status = -ESHUTDOWN;
> > - break;
> > - }
> > + if (trb_type == TRB_SETUP)
> > + return 0;
> > *status = 0;
> > break;
> > case COMP_SHORT_PACKET:
> >
>
> Thanks, Mathias. I need to call finish_td since it needs to
> giveback and cleanup URB. And I see below code at process_ctrl_td,
> why we can't call it? In fact, I only see warning message, but
> without any errors for transactions. How about only delete message
> or change debug level as xhci_dbg?
Should be "or change debug message level and *status value as zero."
>
> /* stopped at setup stage, no data transferred */
> if (trb_type == TRB_SETUP)
> goto finish_td;
>
> --
>
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
