Re: [PATCH v4 7/9] PCI: Make pci_match_one_device() match on ID instead of device

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, Jul 23, 2026 at 11:00:46PM +0100, Gary Guo wrote:
> PCI dynamic ID needs to match IDs against a new ID to see if it already
> exists. Existing APIs can only match IDs against devices, causing the
> dynamic ID insertion code to create a temporary device only for matching
> purpose. Rename pci_match_one_device() to pci_match_one_id() so it can be
> used for this purpose instead; add a pci_id_from_device() helper to make it
> easy to convert users.

s/causing the ... code to create/so the ... code creates/
s/purpose/purposes/

> Similarly, convert pci_match_id() to do_pci_match_id(). But keep the
> existing API because there are many users.
> 
> Reviewed-by: Danilo Krummrich <dakr@kernel.org>
> Signed-off-by: Gary Guo <gary@garyguo.net>

Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>

What's the merge strategy?  If these all need to be merged together,
feel free to include the PCI patches whereever you merge the series.

> ---
>  drivers/pci/pci-driver.c | 38 ++++++++++++++++++++++++++++----------
>  drivers/pci/pci.h        | 43 +++++++++++++++++++++++++++++++------------
>  drivers/pci/search.c     |  8 +++++---
>  3 files changed, 64 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index f36778e62ac1..c9424edb4548 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -90,6 +90,27 @@ static void pci_free_dynids(struct pci_driver *drv)
>  	spin_unlock(&drv->dynids.lock);
>  }
>  
> +/**
> + * do_pci_match_id - See if a PCI ID matches a given pci_id table
> + * @ids: array of PCI device ID structures to search in
> + * @dev_id: the actual PCI device ID structure to match against.
> + *
> + * Return: the matching pci_device_id structure or %NULL if there is no match.
> + */
> +static const struct pci_device_id *
> +do_pci_match_id(const struct pci_device_id *ids,
> +		const struct pci_device_id *dev_id)
> +{
> +	if (ids) {
> +		while (ids->vendor || ids->subvendor || ids->class_mask) {
> +			if (pci_match_one_id(ids, dev_id))
> +				return ids;
> +			ids++;
> +		}
> +	}
> +	return NULL;
> +}
> +
>  /**
>   * pci_match_id - See if a PCI device matches a given pci_id table
>   * @ids: array of PCI device ID structures to search in
> @@ -105,14 +126,9 @@ static void pci_free_dynids(struct pci_driver *drv)
>  const struct pci_device_id *pci_match_id(const struct pci_device_id *ids,
>  					 struct pci_dev *dev)
>  {
> -	if (ids) {
> -		while (ids->vendor || ids->subvendor || ids->class_mask) {
> -			if (pci_match_one_device(ids, dev))
> -				return ids;
> -			ids++;
> -		}
> -	}
> -	return NULL;
> +	struct pci_device_id dev_id = pci_id_from_device(dev);
> +
> +	return do_pci_match_id(ids, &dev_id);
>  }
>  EXPORT_SYMBOL(pci_match_id);
>  
> @@ -138,6 +154,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
>  {
>  	struct pci_dynid *dynid;
>  	const struct pci_device_id *found_id = NULL, *ids;
> +	struct pci_device_id dev_id;
>  	int ret;
>  
>  	/* When driver_override is set, only bind to the matching driver */
> @@ -145,10 +162,11 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
>  	if (ret == 0)
>  		return NULL;
>  
> +	dev_id = pci_id_from_device(dev);
>  	/* Look at the dynamic ids first, before the static ones */
>  	spin_lock(&drv->dynids.lock);
>  	list_for_each_entry(dynid, &drv->dynids.list, node) {
> -		if (pci_match_one_device(&dynid->id, dev)) {
> +		if (pci_match_one_id(&dynid->id, &dev_id)) {
>  			found_id = &dynid->id;
>  			break;
>  		}
> @@ -158,7 +176,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
>  	if (found_id)
>  		return found_id;
>  
> -	for (ids = drv->id_table; (found_id = pci_match_id(ids, dev));
> +	for (ids = drv->id_table; (found_id = do_pci_match_id(ids, &dev_id));
>  	     ids = found_id + 1) {
>  		/*
>  		 * The match table is split based on driver_override.
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 93731242d314..71cda71f3bc9 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -443,21 +443,40 @@ static inline int pci_setup_cardbus(char *str) { return -ENOENT; }
>  #endif /* CONFIG_CARDBUS */
>  
>  /**
> - * pci_match_one_device - Tell if a PCI device structure has a matching
> - *			  PCI device id structure
> - * @id: single PCI device id structure to match
> - * @dev: the PCI device structure to match against
> + * pci_id_from_device - Obtain a pci_device_id from a PCI device
> + * @dev: the PCI device
>   *
> - * Returns the matching pci_device_id structure or %NULL if there is no match.
> + * Return: a pci_device_id filled.
> + */
> +static inline struct pci_device_id pci_id_from_device(const struct pci_dev *dev)
> +{
> +	return (struct pci_device_id) {
> +		.vendor = dev->vendor,
> +		.device = dev->device,
> +		.subvendor = dev->subsystem_vendor,
> +		.subdevice = dev->subsystem_device,
> +		.class = dev->class,
> +	};
> +}
> +
> +/**
> + * pci_match_one_id - Tell if a PCI device ID matches a needle PCI device ID
> + * @id: single PCI device id structure to match against (needle)
> + * @dev_id: the actual ID from the PCI device
> + *
> + * ID can be retrieved from device using pci_id_from_device().
> + *
> + * Return: the matching pci_device_id structure or %NULL if there is no match.
>   */
>  static inline const struct pci_device_id *
> -pci_match_one_device(const struct pci_device_id *id, const struct pci_dev *dev)
> -{
> -	if ((id->vendor == PCI_ANY_ID || id->vendor == dev->vendor) &&
> -	    (id->device == PCI_ANY_ID || id->device == dev->device) &&
> -	    (id->subvendor == PCI_ANY_ID || id->subvendor == dev->subsystem_vendor) &&
> -	    (id->subdevice == PCI_ANY_ID || id->subdevice == dev->subsystem_device) &&
> -	    !((id->class ^ dev->class) & id->class_mask))
> +pci_match_one_id(const struct pci_device_id *id,
> +		 const struct pci_device_id *dev_id)
> +{
> +	if ((id->vendor == PCI_ANY_ID || id->vendor == dev_id->vendor) &&
> +	    (id->device == PCI_ANY_ID || id->device == dev_id->device) &&
> +	    (id->subvendor == PCI_ANY_ID || id->subvendor == dev_id->subvendor) &&
> +	    (id->subdevice == PCI_ANY_ID || id->subdevice == dev_id->subdevice) &&
> +	    !((id->class ^ dev_id->class) & id->class_mask))
>  		return id;
>  	return NULL;
>  }
> diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> index e3d3177fce54..34f8de551d58 100644
> --- a/drivers/pci/search.c
> +++ b/drivers/pci/search.c
> @@ -245,8 +245,10 @@ static int match_pci_dev_by_id(struct device *dev, const void *data)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  	const struct pci_device_id *id = data;
> +	struct pci_device_id dev_id;
>  
> -	if (pci_match_one_device(id, pdev))
> +	dev_id = pci_id_from_device(pdev);
> +	if (pci_match_one_id(id, &dev_id))
>  		return 1;
>  	return 0;
>  }
> @@ -416,9 +418,9 @@ EXPORT_SYMBOL(pci_get_class);
>   * @class: search for a PCI device with this base class code
>   * @from: Previous PCI device found in search, or %NULL for new search.
>   *
> - * Iterates through the list of known PCI devices. If a PCI device is found
> + * Iterate through the list of known PCI devices. If a PCI device is found
>   * with a matching base class code, the reference count to the device is
> - * incremented. See pci_match_one_device() to figure out how does this works.
> + * incremented. See pci_match_one_id() to figure out how this works.
>   * A new search is initiated by passing %NULL as the @from argument.
>   * Otherwise if @from is not %NULL, searches continue from next device on the
>   * global list. The reference count for @from is always decremented if it is
> 
> -- 
> 2.54.0
>