All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: "Alan Stern" <stern@rowland.harvard.edu>,
	"Michał Pecio" <michal.pecio@gmail.com>
Cc: Paul Menzel <pmenzel@molgen.mpg.de>,
	Mathias Nyman <mathias.nyman@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>
Subject: Re: xhci: WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state.
Date: Mon, 7 Apr 2025 10:15:29 +0300	[thread overview]
Message-ID: <dd277ca2-6225-43f6-b833-fe41c2d7f686@linux.intel.com> (raw)
In-Reply-To: <ade0d77a-651a-4b03-bf21-00369fdc22f8@rowland.harvard.edu>

On 6.4.2025 5.40, Alan Stern wrote:
> On Sun, Apr 06, 2025 at 12:23:11AM +0200, Michał Pecio wrote:
>> Looks like some URB stalled and usb_storage reset the device without
>> usb_clear_halt(). Then the core didn't usb_hcd_reset_endpoint() either.
>> And apparently EP_STALLED is still set in xhci_hcd after all that time.
>>
>> Then usb_storage submits one URB which never executes because the EP
>> is in Running-Idle state and the doorbell is inhibited by EP_STALLED.
>> 30s later it times out, unlinks the URB and resets again. Set TR Deq
>> fails because the endpoint is Running.
> 
>> Not sure if it's a USB core bug or something that xHCI should take
>> care of on its own. For now, reverting those two "stall" patches ought
>> to clean up the noise.
> 
> The core believes that resetting a device should erase the endpoint
> information in the HCD.  There is a callback in hub_port_reset() to that
> effect:
> 
> 		if (hcd->driver->reset_device)
> 			hcd->driver->reset_device(hcd, udev);
> 
> So after this the EP should not be in the Running-Idle state; in fact it
> should not exist at all (unless it is ep0, but in this case I think it
> isn't).
> 

> Is the implementation of the reset_device callback in xhci-hcd missing
> something?
> 
> Alan Stern

Thanks, I believe this is at least part of the issue here, thanks for the tip.

We don't clear the virt_dev->eps[ep_index].ep_state flags after device reset.

And the two new patches Michal pointed out rely even more of ep_state flags than
before, causing a regression.

0c74d232578b xhci: Avoid queuing redundant Stop Endpoint command for stalled endpoint
860f5d0d3594 xhci: Prevent early endpoint restart when handling STALL errors.

Does this oneliner help?

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 0452b8d65832..044c70c17746 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3930,6 +3930,7 @@ static int xhci_discover_or_reset_device(struct usb_hcd *hcd,
					&virt_dev->eps[i],
					virt_dev->tt_info);
		xhci_clear_endpoint_bw_info(&virt_dev->eps[i].bw_info);
+		ep->ep_state = 0;
	}
	/* If necessary, update the number of active TTs on this root port */
	xhci_update_tt_active_eps(xhci, virt_dev, old_active_eps);

Thanks
Mathias


  parent reply	other threads:[~2025-04-07  7:14 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-03 18:02 xhci: WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state Paul Menzel
2025-04-04 14:26 ` Mathias Nyman
2025-04-04 14:29   ` Paul Menzel
2025-04-05  5:23   ` Paul Menzel
2025-04-05 22:23     ` Michał Pecio
2025-04-06  2:40       ` Alan Stern
2025-04-06  7:50         ` Michał Pecio
2025-04-06 15:50           ` Michał Pecio
2025-04-06 19:26             ` Alan Stern
2025-04-07  5:49               ` Michał Pecio
2025-04-07 16:11                 ` Alan Stern
2025-04-08 10:18                   ` [PATCH RFC RFT] usb: hcd: Add a usb_device argument to hc_driver.endpoint_reset() Michał Pecio
2025-04-08 13:55                     ` Mathias Nyman
2025-04-09 10:18                       ` Michał Pecio
2025-04-09 14:13                         ` Alan Stern
2025-04-15  8:38                           ` Michał Pecio
2025-04-09 13:03                     ` kernel test robot
2025-04-09 13:14                     ` kernel test robot
2025-04-07  7:15         ` Mathias Nyman [this message]
2025-04-05  6:43 ` xhci: WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state Michał Pecio
2025-04-05  7:36   ` Paul Menzel
2025-04-05  9:49     ` Michał Pecio
2025-04-05 14:08       ` Paul Menzel
2025-04-05 18:13         ` Paul Menzel

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=dd277ca2-6225-43f6-b833-fe41c2d7f686@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=pmenzel@molgen.mpg.de \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.