diff mbox

[dpdk-dev,v5,04/12] bus: add bus iterator to find a device

Message ID 0094d130d0bc93de5d676b67aa28acf6dc81d37b.1498436062.git.gaetan.rivet@6wind.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Gaëtan Rivet June 26, 2017, 12:22 a.m. UTC 
  From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_bus.c          | 24 +++++++++++++++++++++++
 lib/librte_eal/common/include/rte_bus.h         | 26 +++++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 52 insertions(+)

Comments

Bruce Richardson June 27, 2017, 10:14 a.m. UTC | #1 
On Mon, Jun 26, 2017 at 02:22:02AM +0200, Gaetan Rivet wrote:
> From: Jan Blunck <jblunck@infradead.org>
> 
> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
>  lib/librte_eal/common/eal_common_bus.c          | 24 +++++++++++++++++++++++
>  lib/librte_eal/common/include/rte_bus.h         | 26 +++++++++++++++++++++++++
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
>  4 files changed, 52 insertions(+)
> 

Same comment for previous patch on naming of "started" variable.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Bruce Richardson June 27, 2017, 1:54 p.m. UTC | #2 
On Mon, Jun 26, 2017 at 02:22:02AM +0200, Gaetan Rivet wrote:
> From: Jan Blunck <jblunck@infradead.org>
> 
> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
>  lib/librte_eal/common/eal_common_bus.c          | 24 +++++++++++++++++++++++
>  lib/librte_eal/common/include/rte_bus.h         | 26 +++++++++++++++++++++++++
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
>  4 files changed, 52 insertions(+)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index f1a0765..21640d6 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -164,6 +164,7 @@ DPDK_17.05 {
>  
>  	rte_bus_find;
>  	rte_bus_find_by_device;
> +	rte_bus_find_device;
>  	rte_cpu_is_supported;
>  	rte_log_dump;
>  	rte_log_register;
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index d208214..63fd9f1 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -190,3 +190,27 @@ rte_bus_find_by_device(const struct rte_device *dev)
>  {
>  	return rte_bus_find(bus_find_device, (const void *)dev, NULL);
>  }
> +
> +struct rte_device *
> +rte_bus_find_device(rte_dev_cmp_t cmp, const void *data,
> +		    const struct rte_device *start)

One additional suggestion: might it be worthwhile also returning the bus
for the device here, in an optional 4th parameter. This is, after all,
a bus API. :-)

/Bruce
Gaëtan Rivet June 27, 2017, 3:05 p.m. UTC | #3 
On Tue, Jun 27, 2017 at 02:54:34PM +0100, Bruce Richardson wrote:
> On Mon, Jun 26, 2017 at 02:22:02AM +0200, Gaetan Rivet wrote:
> > From: Jan Blunck <jblunck@infradead.org>
> > 
> > Signed-off-by: Jan Blunck <jblunck@infradead.org>
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> >  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
> >  lib/librte_eal/common/eal_common_bus.c          | 24 +++++++++++++++++++++++
> >  lib/librte_eal/common/include/rte_bus.h         | 26 +++++++++++++++++++++++++
> >  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
> >  4 files changed, 52 insertions(+)
> > 
> > diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > index f1a0765..21640d6 100644
> > --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > @@ -164,6 +164,7 @@ DPDK_17.05 {
> >  
> >  	rte_bus_find;
> >  	rte_bus_find_by_device;
> > +	rte_bus_find_device;
> >  	rte_cpu_is_supported;
> >  	rte_log_dump;
> >  	rte_log_register;
> > diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> > index d208214..63fd9f1 100644
> > --- a/lib/librte_eal/common/eal_common_bus.c
> > +++ b/lib/librte_eal/common/eal_common_bus.c
> > @@ -190,3 +190,27 @@ rte_bus_find_by_device(const struct rte_device *dev)
> >  {
> >  	return rte_bus_find(bus_find_device, (const void *)dev, NULL);
> >  }
> > +
> > +struct rte_device *
> > +rte_bus_find_device(rte_dev_cmp_t cmp, const void *data,
> > +		    const struct rte_device *start)
> 
> One additional suggestion: might it be worthwhile also returning the bus
> for the device here, in an optional 4th parameter. This is, after all,
> a bus API. :-)
> 
> /Bruce

I think having the bus is a good info.
However,

1. This makes this API slightly less clean I think.
   A subjective matter, but I don't like adding cruft without good
   justification.

2. The bus will be made accessible by my future patches. The series
   afterward are introducing a generic device type representation, which
   is composed of the bus. This type info is integral to the device and
   thus the bus can be read from within the resulting rte_device here.

So, I won't add this parameter right now. If we have issues with the
rte_devargs series, it might be interesting to come back on this and
edit this API.
Bruce Richardson June 27, 2017, 3:08 p.m. UTC | #4 
On Tue, Jun 27, 2017 at 05:05:14PM +0200, Gaëtan Rivet wrote:
> On Tue, Jun 27, 2017 at 02:54:34PM +0100, Bruce Richardson wrote:
> > On Mon, Jun 26, 2017 at 02:22:02AM +0200, Gaetan Rivet wrote:
> > > From: Jan Blunck <jblunck@infradead.org>
> > > 
> > > Signed-off-by: Jan Blunck <jblunck@infradead.org>
> > > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > > ---
> > >  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
> > >  lib/librte_eal/common/eal_common_bus.c          | 24 +++++++++++++++++++++++
> > >  lib/librte_eal/common/include/rte_bus.h         | 26 +++++++++++++++++++++++++
> > >  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
> > >  4 files changed, 52 insertions(+)
> > > 
> > > diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > > index f1a0765..21640d6 100644
> > > --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > > +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > > @@ -164,6 +164,7 @@ DPDK_17.05 {
> > >  
> > >  	rte_bus_find;
> > >  	rte_bus_find_by_device;
> > > +	rte_bus_find_device;
> > >  	rte_cpu_is_supported;
> > >  	rte_log_dump;
> > >  	rte_log_register;
> > > diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> > > index d208214..63fd9f1 100644
> > > --- a/lib/librte_eal/common/eal_common_bus.c
> > > +++ b/lib/librte_eal/common/eal_common_bus.c
> > > @@ -190,3 +190,27 @@ rte_bus_find_by_device(const struct rte_device *dev)
> > >  {
> > >  	return rte_bus_find(bus_find_device, (const void *)dev, NULL);
> > >  }
> > > +
> > > +struct rte_device *
> > > +rte_bus_find_device(rte_dev_cmp_t cmp, const void *data,
> > > +		    const struct rte_device *start)
> > 
> > One additional suggestion: might it be worthwhile also returning the bus
> > for the device here, in an optional 4th parameter. This is, after all,
> > a bus API. :-)
> > 
> > /Bruce
> 
> I think having the bus is a good info.
> However,
> 
> 1. This makes this API slightly less clean I think.
>    A subjective matter, but I don't like adding cruft without good
>    justification.
> 
> 2. The bus will be made accessible by my future patches. The series
>    afterward are introducing a generic device type representation, which
>    is composed of the bus. This type info is integral to the device and
>    thus the bus can be read from within the resulting rte_device here.
> 

So the device will contain a pointer to it's parent bus? If so, then ok,
let's leave out the extra parameter.

/Bruce

> So, I won't add this parameter right now. If we have issues with the
> rte_devargs series, it might be interesting to come back on this and
> edit this API.
> 
> -- 
> Gaëtan Rivet
> 6WIND
diff mbox

Patch

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index f1a0765..21640d6 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -164,6 +164,7 @@  DPDK_17.05 {
 
 	rte_bus_find;
 	rte_bus_find_by_device;
+	rte_bus_find_device;
 	rte_cpu_is_supported;
 	rte_log_dump;
 	rte_log_register;
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index d208214..63fd9f1 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -190,3 +190,27 @@  rte_bus_find_by_device(const struct rte_device *dev)
 {
 	return rte_bus_find(bus_find_device, (const void *)dev, NULL);
 }
+
+struct rte_device *
+rte_bus_find_device(rte_dev_cmp_t cmp, const void *data,
+		    const struct rte_device *start)
+{
+	struct rte_bus *bus;
+	struct rte_device *dev = NULL;
+	int started = start == NULL;
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		if (!bus->find_device)
+			continue;
+		if (!started) {
+			dev = bus->find_device(cmp_rte_device, start);
+			if (dev)
+				started = 1;
+			continue;
+		}
+		dev = bus->find_device(cmp, data);
+		if (dev)
+			break;
+	}
+	return dev;
+}
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 5441af9..3e83227 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -203,6 +203,32 @@  struct rte_bus *rte_bus_find(rte_bus_cmp_t cmp,
 			     const struct rte_bus *start);
 
 /**
+ * Bus iterator to find a particular device.
+ *
+ * This function searches each registered bus to find a device that matches
+ * the data passed as parameter.
+ *
+ * If the comparison function returns zero this function will stop iterating
+ * over any more buses and devices. To continue a search the device of
+ * a previous search can be passed via the start parameter.
+ *
+ * @param cmp
+ *	Comparison function.
+ *
+ * @param data
+ *	Data to pass to comparison function.
+ *
+ * @param start
+ *	Starting point for the iteration.
+ *
+ * @return
+ *	A pointer to an rte_bus structure or NULL in case no device matches.
+ */
+struct rte_device *rte_bus_find_device(rte_dev_cmp_t cmp,
+				       const void *data,
+				       const struct rte_device *start);
+
+/**
  * Find the registered bus for a particular device.
  */
 struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 6f77222..e0a056d 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -168,6 +168,7 @@  DPDK_17.05 {
 
 	rte_bus_find;
 	rte_bus_find_by_device;
+	rte_bus_find_device;
 	rte_cpu_is_supported;
 	rte_intr_free_epoll_fd;
 	rte_log_dump;