Re: 答复: 答复: Consultation on the issue of digital headphones freezing

[Mathias Nyman]

On 1/14/26 12:27, 胡连勤 wrote:
> Hi  Mathias:
>
> > > Ok, looks like we don't call xhci_sideband_notify_ep_ring_free() in all places we should.
> > >
> > > That being said, to solve this crash we need to make xhci_sideband_remove_endpoint() work
> > > in cases where device is being reset or disconnected, and endpoint is being dropped.
> > >
> > > Looks like xhci_initialize_ring_info(ep->ring) was added to xhci_sideband_remove_endpoint()
> > > in v8 of the original patch series due to a comment about leaving the endpoint in a messy state.
> > >
> > > https://lore.kernel.org/linux-usb/20231011002146.1821-1-quic_wcheng@xxxxxxxxxxx/> >
> > > Calling  xhci_initialize_ring_info() does not really clean up anything, it just sets the
> > > software dequeue and enqueue pointers to the beginning of the ring. The xHC hardware
> > > dequeue pointer is still untouched, and the whole ring buffer is still uncleared.
> > >
> > > I think we can drop xhci_initialize_ring_info() call completely here.
> > > The class driver still needs to set up and reinit the endpoint properly if it wants to continue
> > > using it after it is removed from sideband usage.
> > >
> > > We do want to make sure endpoint is stopped when removing it, but take into account that endpoint
> > > might be stopped, disabled or already dropped by then.
> > >
> > > Does the code below solve your crash?
> > >
> > > diff --git a/drivers/usb/host/xhci-sideband.c b/drivers/usb/host/xhci-sideband.c
> > > index a85f62a73313..2bd77255032b 100644
> > > --- a/drivers/usb/host/xhci-sideband.c
> > > +++ b/drivers/usb/host/xhci-sideband.c
> > > @@ -210,7 +210,6 @@ xhci_sideband_remove_endpoint(struct xhci_sideband *sb,
> > >    		return -ENODEV;
> > >
> > >    	__xhci_sideband_remove_endpoint(sb, ep);
> > > -	xhci_initialize_ring_info(ep->ring);
> > >
> > >    	return 0;
> > >    }
> > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > > index a148a1280126..4161c8c7721d 100644
> > > --- a/drivers/usb/host/xhci.c
> > > +++ b/drivers/usb/host/xhci.c
> > > @@ -2891,16 +2891,25 @@ int xhci_stop_endpoint_sync(struct xhci_hcd *xhci, struct xhci_virt_ep *ep, int
> > >    			    gfp_t gfp_flags)
> > >    {
> > >    	struct xhci_command *command;
> > > +	struct xhci_ep_ctx *ep_ctx;
> > >    	unsigned long flags;
> > > -	int ret;
> > > +	int ret = -ENODEV;
> > >
> > >    	command = xhci_alloc_command(xhci, true, gfp_flags);
> > >    	if (!command)
> > >    		return -ENOMEM;
> > >
> > >    	spin_lock_irqsave(&xhci->lock, flags);
> > > -	ret = xhci_queue_stop_endpoint(xhci, command, ep->vdev->slot_id,
> > > -				       ep->ep_index, suspend);
> > > +
> > > +	/* make sure endpoint exists and is running before stopping it */
> > > +	if (ep->ring) {
> > > +		ep_ctx = xhci_get_ep_ctx(xhci, ep->vdev->out_ctx, ep->ep_index);
> > > +		if (GET_EP_CTX_STATE(ep_ctx) == EP_STATE_RUNNING)
> > > +			ret = xhci_queue_stop_endpoint(xhci, command,
> > > +						       ep->vdev->slot_id,
> > > +						       ep->ep_index, suspend);
> > > +	}
> > > +
> > >    	if (ret < 0) {
> > >    		spin_unlock_irqrestore(&xhci->lock, flags);
> > >    		goto out
> >
> > This patch looks like good.
> > I believe this can solve the crash problem I'm currently experiencing.
> > I'll do a stress test and see.
>
> After merging the above patch, the load test results are as follows:
> 1. The digital headset was connected to a  phone , and a total of three rounds of sleep and wake-up stress tests were conducted,
>       with 3,000 sleep and wake-up cycles in each round.
> 2. No system crashes occurred.

Thanks for testing this,
I'll turn it into a proper patch, add Reported-by and Tested-by tags, and send it forward

Thanks
Mathias