[dpdk-dev] [PATCH v6 3/8] pci: split match and probe function

[Ferruh Yigit]

On 1/16/2017 3:38 PM, Shreyansh Jain wrote:
> Matching of PCI device address and driver ID table is being done at two
> discreet locations duplicating the code. (rte_eal_pci_probe_one_driver
> and rte_eal_pci_detach_dev).
> 
> Splitting the matching function into a public fn rte_pci_match.
> 
> Signed-off-by: Shreyansh Jain <shreyansh.jain at nxp.com>

<...>

>  /*
> - * If vendor/device ID match, call the remove() function of the
> + * If vendor/device ID match, call the probe() function of the
>   * driver.
>   */
>  static int
> -rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
> -		struct rte_pci_device *dev)
> +rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
> +			     struct rte_pci_device *dev)
>  {
> -	const struct rte_pci_id *id_table;
> +	int ret;
> +	struct rte_pci_addr *loc;
>  
>  	if ((dr == NULL) || (dev == NULL))
>  		return -EINVAL;
>  
> -	for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
> +	loc = &dev->addr;
>  
> -		/* check if device's identifiers match the driver's ones */
> -		if (id_table->vendor_id != dev->id.vendor_id &&
> -				id_table->vendor_id != PCI_ANY_ID)
> -			continue;
> -		if (id_table->device_id != dev->id.device_id &&
> -				id_table->device_id != PCI_ANY_ID)
> -			continue;
> -		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
> -				id_table->subsystem_vendor_id != PCI_ANY_ID)
> -			continue;
> -		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
> -				id_table->subsystem_device_id != PCI_ANY_ID)
> -			continue;
> +	RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> +			loc->domain, loc->bus, loc->devid, loc->function,
> +			dev->device.numa_node);

This cause bunch of log printed during app startup, what about printing
this log when probed device found?

>  
> -		struct rte_pci_addr *loc = &dev->addr;
> +	/* The device is not blacklisted; Check if driver supports it */
> +	ret = rte_pci_match(dr, dev);
> +	if (ret) {
> +		/* Match of device and driver failed */
> +		RTE_LOG(DEBUG, EAL, "Driver (%s) doesn't match the device\n",
> +			dr->driver.name);
> +		return 1;
> +	}
> +
> +	/* no initialization when blacklisted, return without error */
> +	if (dev->device.devargs != NULL &&
> +		dev->device.devargs->type ==
> +			RTE_DEVTYPE_BLACKLISTED_PCI) {
> +		RTE_LOG(INFO, EAL, "  Device is blacklisted, not"
> +			" initializing\n");
> +		return 1;
> +	}
>  
> -		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> -				loc->domain, loc->bus, loc->devid,
> -				loc->function, dev->device.numa_node);
> +	RTE_LOG(INFO, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
> +		dev->id.device_id, dr->driver.name);

Same for this one, this line cause printing all registered drivers for
each device during app initialization, only matched one can be logged.

>  
> -		RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n", dev->id.vendor_id,
> -				dev->id.device_id, dr->driver.name);
> +	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
> +		/* map resources for devices that use igb_uio */
> +		ret = rte_eal_pci_map_device(dev);
> +		if (ret != 0)
> +			return ret;
> +	}
>  
> -		if (dr->remove && (dr->remove(dev) < 0))
> -			return -1;	/* negative value is an error */
> +	/* reference driver structure */
> +	dev->driver = dr;
>  
> -		/* clear driver structure */
> +	/* call the driver probe() function */
> +	ret = dr->probe(dr, dev);
> +	if (ret) {
>  		dev->driver = NULL;
> -
>  		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
> -			/* unmap resources for devices that use igb_uio */
>  			rte_eal_pci_unmap_device(dev);
> +	}
>  
> -		return 0;
> +	return ret;
> +}

<...>

> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index b553b13..5ed2589 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -186,5 +186,6 @@ DPDK_17.02 {
>  	rte_bus_dump;
>  	rte_bus_register;
>  	rte_bus_unregister;
> +	rte_pci_match;

I think this is internal API, should library expose this API?

>  
>  } DPDK_16.11;
>