Re: [PATCH v4 2/7] PCI/MSI: Split __pci_enable_msix_range() for reuse

[Philipp Stanner <phasta@mailbox.org>]

On Fri, 2026-04-24 at 15:57 +0800, Shawn Lin wrote:
> Splits the __pci_enable_msix_range() function into two helper
> functions without changing the original behavior. The purpose is to allow
> future functions (particularly managed devres variants) to reuse these
> components.
> 
> The split is as follows:
> 
> 1. pci_msix_range_alloc(): Handles the allocation logic, including
>    parameter validation, hardware vector count verification, and entry
>    validation. This function calculates the actual number of vectors
>    to allocate and returns the hardware-supported vector count.
> 
> 2. pci_msix_range_init(): Handles the initialization of MSI-X context
>    and the actual MSI-X capability setup. This function takes the
>    pre-determined number of vectors and hardware vector count as inputs.
> 
> 3. The original __pci_enable_msix_range() is now a wrapper that calls
>    these two helper functions in sequence, maintaining exact same
>    behavior.
> 
> This is a preparatory step for introducing devres-managed MSI-X allocation
> API that will share the allocation logic while providing automatic cleanup
> via devres.
> 
> No functional changes intended.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> 
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/pci/msi/msi.c | 38 ++++++++++++++++++++++++++++++--------
>  1 file changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c
> index 748dba6..5c196c2 100644
> --- a/drivers/pci/msi/msi.c
> +++ b/drivers/pci/msi/msi.c
> @@ -820,10 +820,10 @@ static bool pci_msix_validate_entries(struct pci_dev *dev, struct msix_entry *en
>  	return true;
>  }
>  
> -int __pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries, int minvec,
> -			    int maxvec, struct irq_affinity *affd, int flags)
> +static int pci_msix_range_alloc(struct pci_dev *dev, struct msix_entry *entries,
> +				int minvec, int maxvec, int flags, int *nvec_ret)

I think it's relatively common to have the 'flags' field as the last
function parameter. Also nvec seems to be suitable more next to the
other vec size parameters.

>  {
> -	int hwsize, rc, nvec = maxvec;
> +	int hwsize, nvec = maxvec;
>  
>  	if (maxvec < minvec)
>  		return -ERANGE;
> @@ -858,12 +858,16 @@ int __pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries, int
>  			nvec = hwsize;
>  	}
>  
> -	if (nvec < minvec)
> -		return -ENOSPC;
> +	*nvec_ret = nvec;
>  
> -	rc = pci_setup_msi_context(dev);
> -	if (rc)
> -		return rc;
> +	return hwsize;
> +}
> +
> +static int pci_msix_range_init(struct pci_dev *dev, struct msix_entry *entries,
> +			       int minvec, int nvec, int hwsize,
> +			       struct irq_affinity *affd)
> +{
> +	int rc;
>  
>  	if (!pci_setup_msix_device_domain(dev, hwsize))
>  		return -ENODEV;
> @@ -888,6 +892,24 @@ int __pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries, int
>  	}
>  }
>  
> +int __pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
> +			    int minvec, int maxvec, struct irq_affinity *affd,
> +			    int flags)
> +{
> +	int hwsize, nvec, rc;
> +
> +	hwsize = pci_msix_range_alloc(dev, entries, minvec,
> +				      maxvec, flags, &nvec);
> +	if (hwsize < 0)
> +		return hwsize;
> +
> +	rc = pci_setup_msi_context(dev);

OK, so IIUC the plan is then to later just remove this line once all
users are ported, correct?

If so, I'd put a TODO comment here: "Remove this after…"

> +	if (rc)
> +		return rc;
> +
> +	return pci_msix_range_init(dev, entries, minvec, nvec, hwsize, affd);
> +}
> +



>  void __pci_restore_msix_state(struct pci_dev *dev)
>  {
>  	struct msi_desc *entry;