From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from wes1-so2.wedos.net (wes1-so2.wedos.net [46.28.106.16]) by dpdk.org (Postfix) with ESMTP id DB633C4AE for ; Wed, 15 Jun 2016 11:55:09 +0200 (CEST) Received: from pcviktorin.fit.vutbr.cz (pcviktorin.fit.vutbr.cz [147.229.13.147]) by wes1-so2.wedos.net (Postfix) with ESMTPSA id 3rV21n3bDWzqR; Wed, 15 Jun 2016 11:55:09 +0200 (CEST) Date: Wed, 15 Jun 2016 11:50:03 +0200 From: Jan Viktorin To: Shreyansh Jain Cc: David Marchand , Thomas Monjalon , Bruce Richardson , Declan Doherty , "jianbo.liu@linaro.org" , "jerin.jacob@caviumnetworks.com" , Keith Wiles , Stephen Hemminger , "dev@dpdk.org" Message-ID: <20160615115003.32e6a1c6@pcviktorin.fit.vutbr.cz> In-Reply-To: References: <1451682326-5834-1-git-send-email-viktorin@rehivetech.com> <1462542490-15556-8-git-send-email-viktorin@rehivetech.com> Organization: RehiveTech MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v1 07/28] eal/soc: add rte_eal_soc_register/unregister logic 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, 15 Jun 2016 09:55:10 -0000 On Wed, 15 Jun 2016 05:57:33 +0000 Shreyansh Jain wrote: > Hi Jan, > > One more comment which I missed in previous reply: > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shreyansh Jain > > Sent: Monday, June 13, 2016 7:50 PM > > To: Jan Viktorin > > Cc: David Marchand ; Thomas Monjalon > > ; Bruce Richardson ; > > Declan Doherty ; jianbo.liu@linaro.org; > > jerin.jacob@caviumnetworks.com; Keith Wiles ; Stephen > > Hemminger ; dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v1 07/28] eal/soc: add > > rte_eal_soc_register/unregister logic > > [...] > > > + > > > +/** > > > + * Empty PMD driver based on the SoC infra. > > > + * > > > + * The rte_soc_device is usually wrapped in some higher-level struct > > > + * (eth_driver). We simulate such a wrapper with an anonymous struct here. > > > + */ > > > +struct test_wrapper { > > > + struct rte_soc_driver soc_drv; > > > +}; > > > + > > > +struct test_wrapper empty_pmd0 = { > > > + .soc_drv = { > > > + .name = "empty_pmd0", > > > + }, > > > +}; > > > + > > > +struct test_wrapper empty_pmd1 = { > > > + .soc_drv = { > > > + .name = "empty_pmd1", > > > + }, > > > +}; > > > + > > > +static int > > > +count_registered_socdrvs(void) > > > +{ > > > + int i; > > > + struct rte_soc_driver *drv; > > > + > > > + i = 0; > > > + TAILQ_FOREACH(drv, &soc_driver_list, next) > > > + i += 1; > > > + > > > + return i; > > > +} > > > + > > > +static int > > > +test_register_unregister(void) > > > +{ > > > + struct rte_soc_driver *drv; > > > + int count; > > > + > > > + rte_eal_soc_register(&empty_pmd0.soc_drv); [...] > > > + > > > +extern struct soc_driver_list soc_driver_list; /**< Global list of SoC > > > drivers. */ > > > > > > struct rte_soc_id { > > > const char *compatible; /**< OF compatible specification */ > > > @@ -131,4 +137,21 @@ rte_eal_compare_soc_addr(const struct rte_soc_addr > > *a0, > > > return strcmp(a0->name, a1->name); > > > } > > > > > > +/** > > > + * Register a SoC driver. > > > + */ > > > +void rte_eal_soc_register(struct rte_soc_driver *driver); > > > + > > > +#define RTE_EAL_SOC_REGISTER(name) \ > > > +RTE_INIT(socinitfn_ ##name); \ > > > +static void socinitfn_ ##name(void) \ > > > +{ \ > > > + rte_eal_soc_register(&name.soc_drv); \ > > It should be 'rte_eal_soc_register(&name)'. > As a user of 'RTE_EAL_SOC_REGISTER', I would pass reference to 'rte_soc_driver' object. It doesn't have any 'soc_drv' member. But eth_driver would have it. And other upper-level structs would have it as well. > > I am guessing that because you have created a wrapper structure 'test_wrapper' in test_soc.c which contains a 'soc_drv', the macro reflects that usage. I don't assume to use rte_soc_driver directly (similarly to rte_pci_driver). However, I agree that it is strange, we should have RTE_ETHDRV_SOC_REGISTER for such purpose (if needed). I wanted to avoid redundant arguments to the RTE_EAL_SOC_REGISTER because the name is used to create the constructor function. But, it seems that other parts of DPDK does not care of this so I will probably give up and make it: RTE_EAL_SOC_REGISTER(my_cool_drv, &my_cool_drv.soc_drv); Thanks for your opinion. I'll fix it in v2. Jan > [...] > > - > Shreyansh > -- Jan Viktorin E-mail: Viktorin@RehiveTech.com System Architect Web: www.RehiveTech.com RehiveTech Brno, Czech Republic