All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
To: Shreyansh Jain <shreyansh.jain@nxp.com>
Cc: <dev@dpdk.org>, <declan.doherty@intel.com>,
	<david.marchand@6wind.com>, <thomas.monjalon@6wind.com>
Subject: Re: [PATCH v2 2/2] eal: rename dev init API for consistency
Date: Mon, 5 Dec 2016 15:54:04 +0530	[thread overview]
Message-ID: <20161205102404.GA29487@localhost.localdomain> (raw)
In-Reply-To: <d6387545-5cd4-ce73-c1d2-a52c413e5820@nxp.com>

On Mon, Dec 05, 2016 at 03:42:18PM +0530, Shreyansh Jain wrote:
> Hello Jerin,

Hello Shreyansh,

> 
> On Sunday 04 December 2016 02:25 AM, Jerin Jacob wrote:
> > rte_eal_dev_init() is a misleading name.
> > It actually performs the driver->probe for vdev,
> > which is parallel to rte_eal_pci_probe.
> > 
> > Changed to rte_eal_vdev_probe for consistency and
> > moved the vdev specific probe to eal_common_vdev.c
> > 
> > Suggested-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > ---
> > +int
> > +rte_eal_vdev_probe(void)
> > +{
> > +	struct rte_devargs *devargs;
> > +
> > +	/*
> > +	 * Note that the dev_driver_list is populated here
> > +	 * from calls made to rte_eal_driver_register from constructor functions
> > +	 * embedded into PMD modules via the RTE_PMD_REGISTER_VDEV macro
> > +	 */
> > +
> > +	/* call the init function for each virtual device */
> > +	TAILQ_FOREACH(devargs, &devargs_list, next) {
> > +
> > +		if (devargs->type != RTE_DEVTYPE_VIRTUAL)
> > +			continue;
> > +
> > +		if (rte_eal_vdev_init(devargs->virt.drv_name,
> 
> The situation now is:
> rte_eal_init=>rte_eal_vdev_probe()=>rte_eal_vdev_init()=> driver->probe()
> 
> Even though I had suggested this, my intention was to completely do away
> with rte_*_[v]dev_init as it is misleading.
> 
> rte_eal_init=>rte_eal_vdev_probe=>driver->probe()

IMO, We don't need to remove rte_eal_vdev_init() as it is an
application API that uses to create vdev driver instance.Moreover,
change and removing that name will result in ABI breakage.

grep -ri "rte_eal_vdev_init" app/
app/test/test_cryptodev.c:				ret = rte_eal_vdev_init(
app/test/test_cryptodev.c: TEST_ASSERT_SUCCESS(rte_eal_vdev_init(
app/test/test_cryptodev.c: TEST_ASSERT_SUCCESS(rte_eal_vdev_init(
app/test/test_cryptodev.c: TEST_ASSERT_SUCCESS(rte_eal_vdev_init(
app/test/test_cryptodev.c: TEST_ASSERT_SUCCESS(rte_eal_vdev_init(
app/test/test_cryptodev.c: int dev_id = rte_eal_vdev_init(
app/test/test_cryptodev.c: ret = rte_eal_vdev_init(
app/test/test_cryptodev_perf.c: ret = rte_eal_vdev_init(
app/test/test_cryptodev_perf.c:	ret = rte_eal_vdev_init(
app/test/test_cryptodev_perf.c:	ret = rte_eal_vdev_init(
app/test/test_cryptodev_perf.c:	ret = rte_eal_vdev_init(


> 
> should be the ideal order, IMO.
> Apologies, I was not completely clear then.
> 
> > +					devargs->args)) {
> > +			RTE_LOG(ERR, EAL, "failed to initialize %s device\n",
> > +					devargs->virt.drv_name);
> > +			return -1;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
> > index 8840380..146f505 100644
> > --- a/lib/librte_eal/common/include/rte_dev.h
> > +++ b/lib/librte_eal/common/include/rte_dev.h
> > @@ -171,9 +171,9 @@ void rte_eal_driver_register(struct rte_driver *driver);
> >  void rte_eal_driver_unregister(struct rte_driver *driver);
> > 
> >  /**
> > - * Initalize all the registered drivers in this process
> > + * Probe all the registered vdev drivers in this process
> >   */
> > -int rte_eal_dev_init(void);
> > +int rte_eal_vdev_probe(void);
> > 
> >  /**
> >   * Initialize a driver specified by name.
> > diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> > index 16dd5b9..faf75cf 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal.c
> > @@ -884,8 +884,8 @@ rte_eal_init(int argc, char **argv)
> >  	if (rte_eal_pci_probe())
> >  		rte_panic("Cannot probe PCI\n");
> > 
> > -	if (rte_eal_dev_init() < 0)
> > -		rte_panic("Cannot init pmd devices\n");
> > +	if (rte_eal_vdev_probe() < 0)
> > +		rte_panic("Cannot probe vdev drivers\n");
> > 
> >  	rte_eal_mcfg_complete();
> > 
> > diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > index 83721ba..67fc95b 100644
> > --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > @@ -22,7 +22,7 @@ DPDK_2.0 {
> >  	rte_dump_tailq;
> >  	rte_eal_alarm_cancel;
> >  	rte_eal_alarm_set;
> > -	rte_eal_dev_init;
> > +	rte_eal_vdev_probe;
> >  	rte_eal_devargs_add;
> >  	rte_eal_devargs_dump;
> >  	rte_eal_devargs_type_count;
> > 
> 

  reply	other threads:[~2016-12-05 10:24 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-20  8:00 [PATCH] eal: postpone vdev initialization Jerin Jacob
2016-11-20 16:05 ` David Marchand
2016-11-21  5:09 ` Shreyansh Jain
2016-11-21 16:56   ` Jerin Jacob
2016-11-21  9:54 ` Ferruh Yigit
2016-11-21 17:02   ` Jerin Jacob
2016-11-21 17:35     ` Ferruh Yigit
2016-11-23  0:07       ` Jerin Jacob
2016-11-23 13:29         ` Thomas Monjalon
2016-12-03 20:55 ` [PATCH v2 0/2] " Jerin Jacob
2016-12-03 20:55   ` [PATCH v2 1/2] eal: " Jerin Jacob
2016-12-03 20:55   ` [PATCH v2 2/2] eal: rename dev init API for consistency Jerin Jacob
2016-12-05 10:12     ` Shreyansh Jain
2016-12-05 10:24       ` Jerin Jacob [this message]
2016-12-05 14:03         ` Shreyansh Jain
2016-12-18 14:21   ` [PATCH v3 0/6] libeventdev API and northbound implementation Jerin Jacob
2016-12-18 14:21     ` [PATCH v3 1/6] eventdev: introduce event driven programming model Jerin Jacob
2016-12-18 14:21     ` [PATCH v3 2/6] eventdev: define southbound driver interface Jerin Jacob
2016-12-19 15:50       ` Bruce Richardson
2016-12-18 14:21     ` [PATCH v3 3/6] eventdev: implement the northbound APIs Jerin Jacob
2016-12-18 14:21     ` [PATCH v3 4/6] eventdev: implement PMD registration functions Jerin Jacob
2016-12-18 14:21     ` [PATCH v3 5/6] event/skeleton: add skeleton eventdev driver Jerin Jacob
2016-12-19 11:58       ` Bruce Richardson
2016-12-18 14:21     ` [PATCH v3 6/6] app/test: unit test case for eventdev APIs Jerin Jacob
2016-12-19  5:16     ` [PATCH v3 0/6] libeventdev API and northbound implementation Shreyansh Jain
2016-12-20 11:13     ` Bruce Richardson
2016-12-20 13:09       ` Jerin Jacob
2016-12-20 13:22         ` Bruce Richardson
2017-01-11 15:52           ` Jerin Jacob
2016-12-21 14:39   ` [PATCH v2 0/2] postpone vdev initialization Thomas Monjalon
2016-12-21 14:42 ` [PATCH] eal: " Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161205102404.GA29487@localhost.localdomain \
    --to=jerin.jacob@caviumnetworks.com \
    --cc=david.marchand@6wind.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=shreyansh.jain@nxp.com \
    --cc=thomas.monjalon@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.