Re: [RFC PATCH 07/12] usb: xhci: split core allocation and initialization

[Michal Pecio <michal.pecio@gmail.com>]

On Thu,  5 Mar 2026 15:48:19 +0100, Niklas Neronin wrote:
> Separate allocation and initialization in the xHCI core:
> * xhci_mem_init() now only handles memory allocation.
> * xhci_init() now only handles initialization.
> 
> This split allows xhci_init() to be reused when resuming from S4
> suspend-to-disk.
> 
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
> ---
>  drivers/usb/host/xhci.c | 37 +++++++++++++------------------------
>  1 file changed, 13 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 338e93f39937..0c2239b5b805 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -536,23 +536,9 @@ static void xhci_set_dev_notifications(struct xhci_hcd *xhci)
>  	writel(dev_notf, &xhci->op_regs->dev_notification);
>  }
>  
> -/*
> - * Initialize memory for HCD and xHC (one-time init).
> - *
> - * Program the PAGESIZE register, initialize the device context array, create
> - * device contexts (?), set up a command ring segment (or two?), create event
> - * ring (one for now).
> - */

OK, the old comment is no longer relevant, but maybe add a new one?
/* Setup basic xHCI registers after HC reset */ or something like that.

> -static int xhci_init(struct usb_hcd *hcd)
> +static void xhci_init(struct usb_hcd *hcd)
>  {
>  	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> -	int retval;
> -
> -	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Starting %s", __func__);
> -
> -	retval = xhci_mem_init(xhci, GFP_KERNEL);
> -	if (retval)
> -		return retval;
>  
>  	/* Set the Number of Device Slots Enabled to the maximum supported value */
>  	xhci_enable_max_dev_slots(xhci);
> @@ -587,9 +573,6 @@ static int xhci_init(struct usb_hcd *hcd)
>  		xhci->quirks |= XHCI_COMP_MODE_QUIRK;
>  		compliance_mode_recovery_timer_init(xhci);
>  	}
> -
> -	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Finished %s", __func__);
> -	return 0;
>  }
>  
>  /*-------------------------------------------------------------------------*/
> @@ -1187,11 +1170,14 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
>  		 * first with the primary HCD, and then with the secondary HCD.
>  		 * If we don't do the same, the host will never be started.
>  		 */
> -		xhci_dbg(xhci, "Initialize the xhci_hcd\n");
> -		retval = xhci_init(hcd);
> +		xhci_dbg(xhci, "Allocate the xhci_hcd\n");
> +		retval = xhci_mem_init(xhci, GFP_KERNEL);

xhci_hcd is allocated, now we allocate everything else :)

I would move those xhci_dbg() inside xhci_mem_init() and xhci_init()
instead of replicating them at every call site. If they are neeeded.

>  		if (retval)
>  			return retval;
>  
> +		xhci_dbg(xhci, "Initialize the xhci_hcd\n");
> +		xhci_init(hcd);
> +
>  		xhci_dbg(xhci, "Start the primary HCD\n");
>  		retval = xhci_run(hcd);
>  		if (!retval && xhci->shared_hcd) {
> @@ -5523,12 +5509,15 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
>  
>  	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);
> +	xhci_dbg(xhci, "Allocate the xhci_hcd\n");
> +	/* Allocate xHCI data structures */
> +	retval = xhci_mem_init(xhci, GFP_KERNEL);
>  	if (retval)
>  		return retval;
> -	xhci_dbg(xhci, "Called HCD init\n");
> +
> +	xhci_dbg(xhci, "Initialize the xhci_hcd\n");
> +	/* Initialize HCD and host controller data structures */
> +	xhci_init(hcd);
>  
>  	if (xhci_hcd_is_usb3(hcd))
>  		xhci_hcd_init_usb3_data(xhci, hcd);
> -- 
> 2.50.1
>