Hello Ferruh,

On Tuesday 17 January 2017 01:23 AM, Ferruh Yigit wrote:
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.j...@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?

Only thing I did was move around this log message without adding anything new. Maybe earlier it was in some conditional (match) and now it isn't. I will check again and move to match-only case.



-               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.

Same. Will post v7 shortly with only match case printing.
What about DEBUG for all cases?



-               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?

Idea is that pci_match be useable outside of PCI for any other PCI-like bus (BDF compliant). For example, one of NXP's devices are very close to PCI (but not exactly PCI) and they too rely on BDF for addressing/naming.



 } DPDK_16.11;




-
Shreyansh

Reply via email to