On Wed, 15 Jun 2016 05:57:33 +0000 Shreyansh Jain <shreyansh.jain at nxp.com> wrote:
> Hi Jan, > > One more comment which I missed in previous reply: > > > -----Original Message----- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Shreyansh Jain > > Sent: Monday, June 13, 2016 7:50 PM > > To: Jan Viktorin <viktorin at rehivetech.com> > > Cc: David Marchand <david.marchand at 6wind.com>; Thomas Monjalon > > <thomas.monjalon at 6wind.com>; Bruce Richardson <bruce.richardson at > > intel.com>; > > Declan Doherty <declan.doherty at intel.com>; jianbo.liu at linaro.org; > > jerin.jacob at caviumnetworks.com; Keith Wiles <keith.wiles at intel.com>; > > Stephen > > Hemminger <stephen at networkplumber.org>; dev at 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 at RehiveTech.com System Architect Web: www.RehiveTech.com RehiveTech Brno, Czech Republic
