From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shreyansh jain Subject: Re: [PATCH v6 05/17] eal: introduce init macros Date: Thu, 14 Jul 2016 10:57:55 +0530 Message-ID: <578722DB.3030900@nxp.com> References: <1466510566-9240-1-git-send-email-shreyansh.jain@nxp.com> <1468303282-2806-1-git-send-email-shreyansh.jain@nxp.com> <1468303282-2806-6-git-send-email-shreyansh.jain@nxp.com> <20160713112043.2fd847ca@jvn> <20160713193430.65b6df9c@jvn> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Cc: , , To: Jan Viktorin Return-path: Received: from NAM03-CO1-obe.outbound.protection.outlook.com (mail-co1nam03on0076.outbound.protection.outlook.com [104.47.40.76]) by dpdk.org (Postfix) with ESMTP id BA9062BA2 for ; Thu, 14 Jul 2016 07:27:25 +0200 (CEST) In-Reply-To: <20160713193430.65b6df9c@jvn> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Jan, On Wednesday 13 July 2016 11:04 PM, Jan Viktorin wrote: > On Wed, 13 Jul 2016 11:20:43 +0200 > Jan Viktorin wrote: > >> Hello Shreyansh, >> >> On Tue, 12 Jul 2016 11:31:10 +0530 >> Shreyansh Jain wrote: >> >>> Introduce a RTE_INIT macro used to mark an init function as a constructor. >>> Current eal macros have been converted to use this (no functional impact). >>> DRIVER_REGISTER_PCI is added as a helper for pci drivers. >>> >>> Suggested-by: Jan Viktorin >>> Signed-off-by: David Marchand >>> Signed-off-by: Shreyansh Jain >>> --- >> >> [...] >> >>> +#define RTE_INIT(func) \ >>> +static void __attribute__((constructor, used)) func(void) >>> + >>> #ifdef __cplusplus >>> } >>> #endif >>> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h >>> index fa74962..3027adf 100644 >>> --- a/lib/librte_eal/common/include/rte_pci.h >>> +++ b/lib/librte_eal/common/include/rte_pci.h >>> @@ -470,6 +470,14 @@ void rte_eal_pci_dump(FILE *f); >>> */ >>> void rte_eal_pci_register(struct rte_pci_driver *driver); >>> >>> +/** Helper for PCI device registeration from driver (eth, crypto) instance */ >>> +#define DRIVER_REGISTER_PCI(nm, drv) \ >>> +RTE_INIT(pciinitfn_ ##nm); \ >>> +static void pciinitfn_ ##nm(void) \ >>> +{ \ >> >> You are missing setting the name here like PMD_REGISTER_DRIVER does >> now. Or should I include it in my patch set? >> >> (drv).name = RTE_STR(nm); That is a miss from my side. I will publish v7 with this. You want this right away or should I wait a little while (more reviews, or any pending additions as per Thomas's notes) before publishing? > > Moreover, it should accept the rte_pci_driver *, shouldn't it? Here, it > expects a wrapper around it (eth_driver)... I now, my SoC patches were > supposing the some... but I think it is wrong. > > The original David's patch set contains calls like this: > > RTE_EAL_PCI_REGISTER(bnx2xvf, rte_bnx2xvf_pmd.pci_drv); > > So, I think, we should go the original way. I have a slightly different opinion of the above. IMO, aim of the helpers is to hide the PCI details and continue to make driver consider itself as a generic ETH driver. In that case, dereferencing pci_drv would be done by macro. Also, considering that in future pci_drv would also have soc_drv, the helpers can effectively hide the intra-structure naming of these. It would help when more such device types (would there be?) are introduced - in which case, driver framework has a consistent coding convention. But, I am ok switching back to David's way as well - I don't have any strong argument against that. > > Jan > >> >>> + rte_eal_pci_register(&drv.pci_drv); \ >>> +} >>> + >>> /** >>> * Unregister a PCI driver. >>> * [...] - Shreyansh