Re: [RFC PATCH 09/12] usb: xhci: optimize resuming from S4 (suspend-to-RAM)

["Neronin, Niklas" <niklas.neronin@linux.intel.com>]

On 06/03/2026 8.52, raoxu wrote:
> On 2026-03-05 14:48 Niklas Neronin wrote:
> 
>> -	if (power_lost) {
>> +	if (reset_registers) {
>>  		if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) &&
>>  				!(xhci_all_ports_seen_u0(xhci))) {
>>  			timer_delete_sync(&xhci->comp_mode_recovery_timer);
>> @@ -1165,28 +1168,27 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
>>  		if (retval)
>>  			return retval;
>>
>> -		xhci_dbg(xhci, "// Disabling event ring interrupts\n");
>> -		writel(STS_EINT, &xhci->op_regs->status);
>> -		xhci_disable_interrupter(xhci, xhci->interrupters[0]);
>> +		/* Clear data which is re-initilized during runtime */
>> +		xhci_for_each_ring_seg(xhci->interrupters[0]->event_ring->first_seg, seg)
>> +			memset(seg->trbs, 0, sizeof(union xhci_trb) * TRBS_PER_SEGMENT);
> 
> If the driver is moving toward multiple operational
> interrupters, the current reset logic seems to only
> reset interrupter 0 explicitly. Should the secondary
> operational interrupters be reset here as well? If so,
> that would likely require additional
> xhci_for_each_ring_seg() handling and may make the
> resume/reset path more complex.

Yes, I tried to keep my patches as strictly preparatory work for secondary
interrupters. Since this patch set does not actually add support for
secondary interrupters, I also did not introduce any interrupter‑loop logic.

That said, once secondary interrupters are added, something along these lines
will be required:

	for (int i = 0; i < xhci->max_interrupters; i++) {
		if (!xhci->interrupters[i])
			continue;
		
		xhci_for_each_ring_seg(xhci->interrupters[i]->event_ring->first_seg, seg)
			memset(seg->trbs, 0, sizeof(union xhci_trb) * TRBS_PER_SEGMENT);
	}

> 
>> +
>> +		for (int i = xhci->max_ports; i > 0; i--)
>> +			xhci_free_virt_devices_depth_first(xhci, i);
>> +
>> +		xhci->cmd_ring_reserved_trbs = 0;
>> +		xhci_for_each_ring_seg(xhci->cmd_ring->first_seg, seg)
>> +			memset(seg->trbs, 0, sizeof(union xhci_trb) * TRBS_PER_SEGMENT);
>>
>> -		xhci_dbg(xhci, "cleaning up memory\n");
>> -		xhci_mem_cleanup(xhci);
>>  		xhci_debugfs_exit(xhci);
>> -		xhci_dbg(xhci, "xhci_stop completed - status = %x\n",
>> -			    readl(&xhci->op_regs->status));
>>
> 
> xhci_mem_cleanup() also clears software-side runtime
> state, not just rings. Without equivalent cleanup,
> state such as the command queue, cmd_timer, cmd_list,
> and TT-related list state may remain across
> reset-resume, which could lead to unexpected issues on
> some hardware.

Good point, I'll clear them all in v2.

Best Regards,
Niklas

> 
> Thanks,
> 
> Xu Rao