From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: Michal Pecio <michal.pecio@gmail.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 22:27:29 +0300 [thread overview]
Message-ID: <88f6d39e-5738-4a55-af75-854dfc0df094@linux.intel.com> (raw)
In-Reply-To: <20260401165205.56dcfcda.michal.pecio@gmail.com>
On 4/1/26 17:52, Michal Pecio wrote:
> 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)
>
True, thanks, usb_set_interface() calls usb_disable_interface() twice.
First time just to flush pending URBs, second time with reset_hardware
flag set.
Adding patch to queue
-Mathias
prev parent reply other threads:[~2026-04-01 19:27 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
2026-04-01 19:27 ` Mathias Nyman [this message]
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=88f6d39e-5738-4a55-af75-854dfc0df094@linux.intel.com \
--to=mathias.nyman@linux.intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@intel.com \
--cc=michal.pecio@gmail.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