On Tue, Jul 7, 2020 at 12:57 AM Gerd Hoffmann <kra...@redhat.com> wrote:
> Hi, > > > > Gerd, do you know the purpose of the 'short_not_ok' parameter to > > usb_packet_setup()? > > Well, some usb host controllers have a flag in the transfer control > blocks to indicate the host controller should stop processing requests > in case of a short transfer. > > The usb core uses the flag to just to that (i.e. halt the endpoint on > short transfers). It is also checked when usb-host pipelines requests > (requests queued after a short-not-ok packet can't be pipelined because > we don't know yet whenever we should continue processing the endpoint or > not). > > xhci has no such flag so the flag is never set. > > > The simple patch below fixes the reported problem, > > but I don't know if it could cause some other problems for XHCI. > > hcd-ehci, hcd-ohci, hcd-uhci all set the parameter conditionally, > > but hcd-xhci never sets it. I don't understand the purpose of the > > parameter myself. > > > > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c > > index b330e36fe6..9fb96fdd66 100644 > > --- a/hw/usb/hcd-xhci.c > > +++ b/hw/usb/hcd-xhci.c > > @@ -1614,7 +1614,7 @@ static int xhci_setup_packet(XHCITransfer *xfer) > > xhci_xfer_create_sgl(xfer, dir == USB_TOKEN_IN); /* Also sets > int_req */ > > usb_packet_setup(&xfer->packet, dir, ep, xfer->streamid, > > - xfer->trbs[0].addr, false, xfer->int_req); > > + xfer->trbs[0].addr, dir == USB_TOKEN_IN, > xfer->int_req); > > I suspect this might lead to queues being halted even though they should > not. > > Why does 7ad3d51ebb8a522ffcad391c4bef281245739dde look at short-not-ok? > Because the patch changes dev-storage to terminate the transfer if a short packet is received, so I figured 'short-not-ok' should affect that behavior. I guess instead I could add another flag that only hcd-dwc2 sets. Does that sound OK to you? Thanks, Paul > take care, > Gerd > >