From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH v6 3/8] pci: split match and probe function Date: Mon, 16 Jan 2017 19:53:59 +0000 Message-ID: References: <1484581107-2025-1-git-send-email-shreyansh.jain@nxp.com> <1484581107-2025-4-git-send-email-shreyansh.jain@nxp.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Cc: dev@dpdk.org, thomas.monjalon@6wind.com To: Shreyansh Jain , david.marchand@6wind.com Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id AEF9B98 for ; Mon, 16 Jan 2017 20:54:02 +0100 (CET) In-Reply-To: <1484581107-2025-4-git-send-email-shreyansh.jain@nxp.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 <...> > /* > - * 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; >