From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 7CB09804A for ; Thu, 11 Dec 2014 10:56:02 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP; 11 Dec 2014 01:55:54 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,556,1413270000"; d="scan'208";a="622093068" Received: from bricha3-mobl3.ger.corp.intel.com ([10.243.20.26]) by orsmga001.jf.intel.com with SMTP; 11 Dec 2014 01:55:51 -0800 Received: by (sSMTP sendmail emulation); Thu, 11 Dec 2014 09:55:50 +0025 Date: Thu, 11 Dec 2014 09:55:50 +0000 From: Bruce Richardson To: "Qiu, Michael" Message-ID: <20141211095550.GA5668@bricha3-MOBL3> References: <1416474399-16851-1-git-send-email-mukawa@igel.co.jp> <1418106629-22227-1-git-send-email-mukawa@igel.co.jp> <1418106629-22227-21-git-send-email-mukawa@igel.co.jp> <533710CFB86FA344BFBF2D6802E60286C9EC20@SHSMSX101.ccr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <533710CFB86FA344BFBF2D6802E60286C9EC20@SHSMSX101.ccr.corp.intel.com> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: "dev@dpdk.org" , "nakajima.yoshihiro@lab.ntt.co.jp" , "masutani.hitoshi@lab.ntt.co.jp" , "menrigh@brocade.com" Subject: Re: [dpdk-dev] [PATCH v3 20/28] eal/pci: Add rte_eal_pci_close_one_driver X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Dec 2014 09:56:03 -0000 On Thu, Dec 11, 2014 at 03:41:06AM +0000, Qiu, Michael wrote: > On 12/9/2014 2:33 PM, Tetsuya Mukawa wrote: > > The function is used for closing the specified driver and device. > > > > Signed-off-by: Tetsuya Mukawa > > --- > > lib/librte_eal/common/eal_private.h | 15 +++++++++ > > lib/librte_eal/linuxapp/eal/eal_pci.c | 61 +++++++++++++++++++++++++++++++++++ > > 2 files changed, 76 insertions(+) > > > > diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h > > index a1127ab..f892ac4 100644 > > --- a/lib/librte_eal/common/eal_private.h > > +++ b/lib/librte_eal/common/eal_private.h > > @@ -176,6 +176,21 @@ int rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, > > struct rte_pci_device *dev); > > > > /** > > + * Munmap memory for single PCI device > > + * > > + * This function is private to EAL. > > + * > > + * @param dr > > + * The pointer to the pci driver structure > > + * @param dev > > + * The pointer to the pci device structure > > + * @return > > + * 0 on success, negative on error > > + */ > > +int rte_eal_pci_close_one_driver(struct rte_pci_driver *dr, > > + struct rte_pci_device *dev); > > + > > +/** > > * Init tail queues for non-EAL library structures. This is to allow > > * the rings, mempools, etc. lists to be shared among multiple processes > > * > > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c > > index 8d683f5..93456f3 100644 > > --- a/lib/librte_eal/linuxapp/eal/eal_pci.c > > +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c > > @@ -616,6 +616,67 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d > > return 1; > > } > > > > +#if defined(RTE_LIBRTE_EAL_HOTPLUG) && defined(RTE_LIBRTE_EAL_LINUXAPP) > > +/* > > + * If vendor/device ID match, call the devshutdown() function of the > > + * driver. > > + */ > > +int > > +rte_eal_pci_close_one_driver(struct rte_pci_driver *dr, > > + struct rte_pci_device *dev) > > +{ > > + struct rte_pci_id *id_table; > > + > > + if ((dr == NULL) || (dev == NULL)) > > + return -EINVAL; > > + > > + for (id_table = dr->id_table ; id_table->vendor_id != 0; id_table++) { > > + > > + /* 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) > > Here and below, it is better to make indentation like this: > > + if (id_table->vendor_id != dev->id.vendor_id && > + id_table->vendor_id != PCI_ANY_ID) > > But I don't know if there are any strict rule of dpdk community :) > I don't think we have any strict rules - I'm sure Thomas can confirm. However, I prefer the original suggested version, as for the second version you propose the second half of the if will appear indended as part of the if body for those people who use a 4-character tab-stop in their editor. By indenting the second line with two tabs, or tab + some space, you can guarantee that the rest of the if statement never lines up with the if body, making the separation between statement and body clearer. Just my 2c. /Bruce