public inbox for linux-usb@vger.kernel.org
 help / color / mirror / Atom feed
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


      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