public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Michal Pecio <michal.pecio@gmail.com>
To: Mathias Nyman <mathias.nyman@linux.intel.com>
Cc: Mathias Nyman <mathias.nyman@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Alan Stern <stern@rowland.harvard.edu>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] usb: xhci: Make usb_host_endpoint.hcpriv survive endpoint_disable()
Date: Wed, 1 Apr 2026 16:52:05 +0200	[thread overview]
Message-ID: <20260401165205.56dcfcda.michal.pecio@gmail.com> (raw)
In-Reply-To: <e14fb308-a003-4a76-b908-106b5271eccc@linux.intel.com>

On Wed, 1 Apr 2026 17:34:37 +0300, Mathias Nyman wrote:
> On 3/31/26 02:06, Michal Pecio wrote:
> > xHCI hardware maintains its endpoint state between add_endpoint()
> > and drop_endpoint() calls followed by successful check_bandwidth().
> > So does the driver.
> > 
> > Core may call endpoint_disable() during xHCI endpoint life, so don't
> > clear host_ep->hcpriv then, because this breaks endpoint_reset().
> > 
> > If a driver calls usb_set_interface(), submits URBs which make host
> > sequence state non-zero and calls usb_clear_halt(), the device clears
> > its sequence state but xhci_endpoint_reset() bails out. The next URB
> > malfunctions: USB2 loses one packet, USB3 gets Transaction Error or
> > may not complete at all on some (buggy?) HCs from ASMedia and AMD.
> > This is triggered by uvcvideo on bulk video devices.  
> 
> Were you able to trigger a usb_clear_halt() called with ep->hcpriv == NULL,
> causing a toggle/seq mismatch?
> 
> The ep->hcpriv should be set back correctly in usb_set_interface():
> 
> usb_set_interface()
>    usb_hcd_alloc_bandwidth()
>      hcd->driver->add_endpoint()
>        xhci_add_endpoint()
>          ep->hcpriv = udev;

right, and later:

     usb_disable_interface(dev, iface, true)
       usb_disable_endpoint(dev, ..., true)
         usb_hcd_disable_endpoint(dev, ep)
           hcd->driver->endpoint_disable(hcd, ep)
     usb_enable_interface(dev, iface, true)
       usb_enable_endpoint(dev, ..., true)
         usb_hcd_reset_endpoint(dev, ep)
           hcd->driver->endpoint_reset(hcd, ep)

So it seems set_interface() is broken and doesn't actually reset host
sequence state (while the device is supposed to reset its own).

This alone is rarely a problem because the endpoint is usually "fresh".

But uvcvideo calls usb_clear_halt() *after* stopping a bulk stream,
because that's what Windows does. Then sequence state is random and
gets cleared only on the device, because hcpriv is still NULL.

The next URB gets Transaction Error, the host endpoint is reset and
another URB succeeds (because we restart the endpoint unconditionally).

Regards,
Michal

  reply	other threads:[~2026-04-01 14:52 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-30 23:06 [PATCH] usb: xhci: Make usb_host_endpoint.hcpriv survive endpoint_disable() Michal Pecio
2026-04-01 14:34 ` Mathias Nyman
2026-04-01 14:52   ` Michal Pecio [this message]
2026-04-01 19:27     ` Mathias Nyman

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=20260401165205.56dcfcda.michal.pecio@gmail.com \
    --to=michal.pecio@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@intel.com \
    --cc=mathias.nyman@linux.intel.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