Re: [RFC PATCH 06/12] usb: xhci: move initialization for lifetime objects

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

On 06/03/2026 0.14, Michal Pecio wrote:
> On Thu,  5 Mar 2026 15:48:18 +0100, Niklas Neronin wrote:
>> Initialize objects that exist for the lifetime of the driver only once,
>> rather than repeatedly. These objects do not require re-initialization
>> after events such as S4 (suspend-to-disk).
>>
>> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
>> ---
>>  drivers/usb/host/xhci-mem.c |  1 -
>>  drivers/usb/host/xhci.c     | 15 ++++++++-------
>>  2 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>> index 005b7bc1bfda..fae75969e49a 100644
>> --- a/drivers/usb/host/xhci-mem.c
>> +++ b/drivers/usb/host/xhci-mem.c
>> @@ -2001,7 +2001,6 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
>>  	xhci->port_caps = NULL;
>>  	xhci->interrupters = NULL;
>>  
>> -	xhci->page_size = 0;
>>  	xhci->usb2_rhub.bus_state.bus_suspended = 0;
>>  	xhci->usb3_rhub.bus_state.bus_suspended = 0;
>>  }
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index d9519a9e9e17..338e93f39937 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -549,13 +549,6 @@ static int xhci_init(struct usb_hcd *hcd)
>>  	int retval;
>>  
>>  	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Starting %s", __func__);
>> -	spin_lock_init(&xhci->lock);
>> -
>> -	INIT_LIST_HEAD(&xhci->cmd_list);
> 
> Can we prove that this list is empty?
> 
> If there is any leftover garbage there, INIT_LIST_HEAD simply leaks it.
> Without it, the garbage stays on the list.

I assume you are referring to the resume path.
In this case xhci_cleanup_command_queue() is called before xhci_init().

xhci_resume()
├─xhci_mem_cleanup()
│  └─xhci_cleanup_command_queue()
└─xhci_init()

> 
>> -	INIT_DELAYED_WORK(&xhci->cmd_timer, xhci_handle_command_timeout);
>> -	init_completion(&xhci->cmd_ring_stop_completion);
>> -	xhci_hcd_page_size(xhci);
>> -	memset(xhci->devs, 0, MAX_HC_SLOTS * sizeof(*xhci->devs));
>>  
>>  	retval = xhci_mem_init(xhci, GFP_KERNEL);
>>  	if (retval)
>> @@ -5522,6 +5515,14 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
>>  		dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
>>  	}
>>  
>> +	spin_lock_init(&xhci->lock);
>> +	INIT_LIST_HEAD(&xhci->cmd_list);
>> +	INIT_DELAYED_WORK(&xhci->cmd_timer, xhci_handle_command_timeout);
>> +	init_completion(&xhci->cmd_ring_stop_completion);
>> +	xhci_hcd_page_size(xhci);
>> +
>> +	memset(xhci->devs, 0, MAX_HC_SLOTS * sizeof(*xhci->devs));
>> +
>>  	xhci_dbg(xhci, "Calling HCD init\n");
>>  	/* Initialize HCD and host controller data structures. */
>>  	retval = xhci_init(hcd);
>> -- 
>> 2.50.1
>>