From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH v8 3/9] pci: split match and probe function Date: Wed, 18 Jan 2017 08:31:01 +0100 Message-ID: <1982590.t5Omiyb9eb@xps13> References: <1484647774-28984-1-git-send-email-shreyansh.jain@nxp.com> <6385545.1JoApMrEe4@xps13> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org To: Shreyansh Jain Return-path: Received: from mail-wm0-f43.google.com (mail-wm0-f43.google.com [74.125.82.43]) by dpdk.org (Postfix) with ESMTP id 5A098D586 for ; Wed, 18 Jan 2017 08:31:04 +0100 (CET) Received: by mail-wm0-f43.google.com with SMTP id r126so233497487wmr.0 for ; Tue, 17 Jan 2017 23:31:04 -0800 (PST) In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 2017-01-18 06:17, Shreyansh Jain: > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > 2017-01-17 19:07, Shreyansh Jain: > > > +static int > > > +rte_eal_pci_detach_dev(struct rte_pci_driver *dr, > > > + struct rte_pci_device *dev) > > > +{ > > > + int ret; > > > + struct rte_pci_addr *loc; > > > + > > > + if ((dr == NULL) || (dev == NULL)) > > > + return -EINVAL; > > > + > > > + ret = rte_pci_match(dr, dev); > > > > I do not understand this function. > > The driver should be known by the device at this stage. > > Why specifying a driver as parameter? > > > > I know it is not new in this series, but > > pci_detach_all_drivers(struct rte_pci_device *dev) makes no sense to me. > > From what I understand, pci_detach_all_drivers is simply a wrapper over the PCI driver list, calling rte_eal_pci_detach_dev for each driver. Considering that all it does is iterate, it is not functionally required. > > But, let me clarify one more thing: There is a possibility that rte_pci_device->pci_driver be directly used in case of detach rather than searching for a driver. This is assuming that before 'detach', a pci_device would link to a valid pci_driver. Is there some caveat in this assumption? I would say it it always true. But seeing this code existing makes me hesitates. I suggest to go with this assumption for RC1 and remove this loop.