From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shreyansh Jain Subject: Re: [PATCH 01/22] eal: introduce one device scan Date: Fri, 8 Jun 2018 16:42:24 +0530 Message-ID: <856a4948-756a-16e7-95c8-c61ecf52916e@nxp.com> References: <20180607123849.14439-1-qi.z.zhang@intel.com> <20180607123849.14439-2-qi.z.zhang@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: thomas@monjalon.net, anatoly.burakov@intel.com, konstantin.ananyev@intel.com, dev@dpdk.org, bruce.richardson@intel.com, ferruh.yigit@intel.com, benjamin.h.shelton@intel.com, narender.vangati@intel.com To: Qi Zhang Return-path: Received: from EUR01-HE1-obe.outbound.protection.outlook.com (mail-he1eur01on0088.outbound.protection.outlook.com [104.47.0.88]) by dpdk.org (Postfix) with ESMTP id 718875B2C for ; Fri, 8 Jun 2018 13:12:48 +0200 (CEST) In-Reply-To: <20180607123849.14439-2-qi.z.zhang@intel.com> Content-Language: en-US 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 6/7/2018 6:08 PM, Qi Zhang wrote: > When hot plug a new device, it is not necessary to scan everything > on the bus since the devname and devargs are already there. So new > rte_bus ops "scan_one" is introduced, bus driver can implement this > function to simply the hotplug process. ^^^^^^^^^ simplify > > Signed-off-by: Qi Zhang > --- > lib/librte_eal/common/eal_common_dev.c | 17 +++++++++++++---- > lib/librte_eal/common/include/rte_bus.h | 4 ++++ > 2 files changed, 17 insertions(+), 4 deletions(-) > > diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c > index 61cb3b162..1ad033536 100644 > --- a/lib/librte_eal/common/eal_common_dev.c > +++ b/lib/librte_eal/common/eal_common_dev.c > @@ -147,11 +147,20 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn > if (ret) > goto err_devarg; > > - ret = bus->scan(); > - if (ret) > - goto err_devarg; > + /** > + * if bus support to scan specific device by devargs, > + * we don't need to scan all devices on the bus. > + */ > + if (bus->scan_one) { > + dev = bus->scan_one(da); > + } else { > + ret = bus->scan(); > + if (ret) > + goto err_devarg; > + > + dev = bus->find_device(NULL, cmp_detached_dev_name, devname); > + } > > - dev = bus->find_device(NULL, cmp_detached_dev_name, devname); > if (dev == NULL) { > RTE_LOG(ERR, EAL, "Cannot find unplugged device (%s)\n", > devname); > diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h > index eb9eded4e..b15cff892 100644 > --- a/lib/librte_eal/common/include/rte_bus.h > +++ b/lib/librte_eal/common/include/rte_bus.h > @@ -83,6 +83,7 @@ enum rte_iova_mode { > */ > typedef int (*rte_bus_scan_t)(void); > > +typedef struct rte_device *(*rte_bus_scan_one_t)(struct rte_devargs *); You should add comments over the declaration, just like the other similar declarations. And, a new line should be here. > /** > * Implementation specific probe function which is responsible for linking > * devices on that bus with applicable drivers. > @@ -95,6 +96,8 @@ typedef int (*rte_bus_scan_t)(void); > */ > typedef int (*rte_bus_probe_t)(void); > > + > + And please remove the extra lines added above in next version of patch. > /** > * Device iterator to find a device on a bus. > * > @@ -204,6 +207,7 @@ struct rte_bus { > TAILQ_ENTRY(rte_bus) next; /**< Next bus object in linked list */ > const char *name; /**< Name of the bus */ > rte_bus_scan_t scan; /**< Scan for devices attached to bus */ > + rte_bus_scan_one_t scan_one; /**< Scan one device by devargs */ I think you mean "Scan one device using devargs" rather than "Scan one device by devargs". > rte_bus_probe_t probe; /**< Probe devices on bus */ > rte_bus_find_device_t find_device; /**< Find a device on the bus */ > rte_bus_plug_t plug; /**< Probe single device for drivers */ >