From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 65A5F2E41 for ; Wed, 25 May 2016 19:36:04 +0200 (CEST) Received: from hmsreliant.think-freely.org ([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1b5cit-0008G0-WF; Wed, 25 May 2016 13:36:02 -0400 Date: Wed, 25 May 2016 13:35:54 -0400 From: Neil Horman To: Thomas Monjalon Cc: dev@dpdk.org, Bruce Richardson , Stephen Hemminger , Panu Matilainen Message-ID: <20160525173554.GC14322@hmsreliant.think-freely.org> References: <1463431287-4551-1-git-send-email-nhorman@tuxdriver.com> <1464118912-19658-1-git-send-email-nhorman@tuxdriver.com> <1464118912-19658-3-git-send-email-nhorman@tuxdriver.com> <19273886.LC4JPNcjGO@xps13> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <19273886.LC4JPNcjGO@xps13> User-Agent: Mutt/1.6.1 (2016-04-27) X-Spam-Score: -2.9 (--) X-Spam-Status: No Subject: Re: [dpdk-dev] [PATCHv4 2/5] drivers: Update driver registration macro usage 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: Wed, 25 May 2016 17:36:04 -0000 On Wed, May 25, 2016 at 06:20:06PM +0200, Thomas Monjalon wrote: > 2016-05-24 15:41, Neil Horman: > > Modify the PMD_REGISTER_DRIVER macro, adding a name argument to it. The > > addition of a name argument creates a token that can be used for subsequent > > macros in the creation of unique symbol names to export additional bits of > > information for use by the pmdinfogen tool. For example: > > > > PMD_REGISTER_DRIVER(ena_driver, ena); > > > > registers the ena_driver struct as it always did, and creates a symbol > > const char this_pmd_name0[] __attribute__((used)) = "ena"; > > > > which pmdinfogen can search for and extract. > > The EAL rework (http://dpdk.org/ml/archives/dev/2016-April/037691.html) > integrates already a name: > > +#define RTE_EAL_PCI_REGISTER(name, d) \ > +RTE_INIT(pciinitfn_ ##name); \ > +static void pciinitfn_ ##name(void) \ > +{ \ > + rte_eal_pci_register(&d); \ > +} > > I think it would be better to rebase on top of it. > Those patches are over a month old and still in the new state according to patchwork. I'm not very comfortable rebasing (and implicitly blocking) acceptance of this patch on that one. Its really a just two lines of conflict. I would suggest that, whichever patch gets integrated first, the other series can rebase on the new head. It should be a pretty easy fix either way. > > The subsequent macro > > > > DRIVER_REGISTER_PCI_TABLE(ena, ena_pci_id_map); > > > > creates a symbol > > const char ena_pci_tbl_export[] __attribute__((used)) = "ena_pci_id_map"; > > > > Which allows pmdinfogen to find the pci table of this driver > > > > Using this pattern, we can export arbitrary bits of information. > > > > pmdinfo uses this information to extract hardware support from an object file > > and create a json string to make hardware support info discoverable later. > > > --- a/drivers/Makefile > > +++ b/drivers/Makefile > > @@ -34,4 +34,6 @@ include $(RTE_SDK)/mk/rte.vars.mk > > DIRS-y += net > > DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto > > > > +DEPDIRS-y += buildtools/pmdinfo > > Why pmdinfo is a build dependency? > beause pmdinfogen has to be built and available for use prior to compiling the rest of the dpdk. I suppose we could build it after, and then go back through and check all the objects for driver info, but I'd rather build it first, and search the objects as they are built. > > --- a/lib/librte_eal/common/include/rte_dev.h > > +++ b/lib/librte_eal/common/include/rte_dev.h > > @@ -48,7 +48,7 @@ extern "C" { > > > > #include > > #include > > - > > +#include > > Why not keep PCI stuff in rte_pci.h? > I am. > > +#define DRV_EXP_TAG(n, t) __##n##_##t > > + > > +#define DRIVER_REGISTER_PCI_TABLE(n, t) \ > > +static const char DRV_EXP_TAG(n, pci_tbl_export)[] __attribute__((used)) = RTE_STR(t) > > I really dislike one-char variables, especially when there is no comment. > Please choose comments or explicit variables. > You mean you want the macro variables to be longer/more descriptive? I suppose, but in fairness, we have lots of macros that use single letter variables, I'm not sure why your concerned about these specifically. I'll change it though. Neil