Re: [PATCH v7 07/12] PCI: Refactor matching logic for pci_dev_acs_ops

[Bjorn Helgaas]

On Fri, Jul 10, 2026 at 09:26:10PM +0000, David Matlack wrote:
> Refactor the logic to match devices to pci_dev_acs_ops by factoring out
> the loop and device matching into its own routine. This eliminates some

Can you mention the name of the new routine here?  I guess it's
pci_dev_acs_ops_get()?

> duplicate code between pci_dev_specific_enable_acs() and
> pci_dev_specific_disable_acs_redir(), and will also be used in a
> subsequent commit to check if a device requires device-specific
> enable_acs() during a Live Update.
> 
> No functional change intended.
> 
> Reviewed-by: Pranjal Shrivastava <praan@xxxxxxxxxx>
> Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx>

Reviewed-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>

> ---
>  drivers/pci/quirks.c | 51 ++++++++++++++++++--------------------------
>  1 file changed, 21 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index b09f27f7846f..7ac39ec2843e 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -5374,9 +5374,6 @@ static void pci_quirk_enable_intel_rp_mpc_acs(struct pci_dev *dev)
>   */
>  static int pci_quirk_enable_intel_pch_acs(struct pci_dev *dev)
>  {
> -	if (!pci_quirk_intel_pch_acs_match(dev))
> -		return -ENOTTY;
> -
>  	if (pci_quirk_enable_intel_lpc_acs(dev)) {
>  		pci_warn(dev, "Failed to enable Intel PCH ACS quirk\n");
>  		return 0;
> @@ -5396,9 +5393,6 @@ static int pci_quirk_enable_intel_spt_pch_acs(struct pci_dev *dev)
>  	int pos;
>  	u32 cap, ctrl;
>  
> -	if (!pci_quirk_intel_spt_pch_acs_match(dev))
> -		return -ENOTTY;
> -
>  	pos = dev->acs_cap;
>  	if (!pos)
>  		return -ENOTTY;
> @@ -5426,9 +5420,6 @@ static int pci_quirk_disable_intel_spt_pch_acs_redir(struct pci_dev *dev)
>  	int pos;
>  	u32 cap, ctrl;
>  
> -	if (!pci_quirk_intel_spt_pch_acs_match(dev))
> -		return -ENOTTY;
> -
>  	pos = dev->acs_cap;
>  	if (!pos)
>  		return -ENOTTY;
> @@ -5448,56 +5439,56 @@ static int pci_quirk_disable_intel_spt_pch_acs_redir(struct pci_dev *dev)
>  static const struct pci_dev_acs_ops {
>  	u16 vendor;
>  	u16 device;
> +	bool (*match)(struct pci_dev *dev);
>  	int (*enable_acs)(struct pci_dev *dev);
>  	int (*disable_acs_redir)(struct pci_dev *dev);
>  } pci_dev_acs_ops[] = {
>  	{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
> +	    .match = pci_quirk_intel_pch_acs_match,
>  	    .enable_acs = pci_quirk_enable_intel_pch_acs,
>  	},
>  	{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
> +	    .match = pci_quirk_intel_spt_pch_acs_match,
>  	    .enable_acs = pci_quirk_enable_intel_spt_pch_acs,
>  	    .disable_acs_redir = pci_quirk_disable_intel_spt_pch_acs_redir,
>  	},
>  };
>  
> -int pci_dev_specific_enable_acs(struct pci_dev *dev)
> +static const struct pci_dev_acs_ops *pci_dev_acs_ops_get(struct pci_dev *dev)
>  {
>  	const struct pci_dev_acs_ops *p;
> -	int i, ret;
> +	int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(pci_dev_acs_ops); i++) {
>  		p = &pci_dev_acs_ops[i];
>  		if ((p->vendor == dev->vendor ||
>  		     p->vendor == (u16)PCI_ANY_ID) &&
>  		    (p->device == dev->device ||
> -		     p->device == (u16)PCI_ANY_ID) &&
> -		    p->enable_acs) {
> -			ret = p->enable_acs(dev);
> -			if (ret >= 0)
> -				return ret;
> +		     p->device == (u16)PCI_ANY_ID)) {
> +			if (!p->match || p->match(dev))
> +				return p;
>  		}
>  	}
>  
> +	return NULL;
> +}
> +
> +int pci_dev_specific_enable_acs(struct pci_dev *dev)
> +{
> +	const struct pci_dev_acs_ops *p = pci_dev_acs_ops_get(dev);
> +
> +	if (p && p->enable_acs)
> +		return p->enable_acs(dev);
> +
>  	return -ENOTTY;
>  }
>  
>  int pci_dev_specific_disable_acs_redir(struct pci_dev *dev)
>  {
> -	const struct pci_dev_acs_ops *p;
> -	int i, ret;
> +	const struct pci_dev_acs_ops *p = pci_dev_acs_ops_get(dev);
>  
> -	for (i = 0; i < ARRAY_SIZE(pci_dev_acs_ops); i++) {
> -		p = &pci_dev_acs_ops[i];
> -		if ((p->vendor == dev->vendor ||
> -		     p->vendor == (u16)PCI_ANY_ID) &&
> -		    (p->device == dev->device ||
> -		     p->device == (u16)PCI_ANY_ID) &&
> -		    p->disable_acs_redir) {
> -			ret = p->disable_acs_redir(dev);
> -			if (ret >= 0)
> -				return ret;
> -		}
> -	}
> +	if (p && p->disable_acs_redir)
> +		return p->disable_acs_redir(dev);
>  
>  	return -ENOTTY;
>  }
> -- 
> 2.55.0.795.g602f6c329a-goog
>