[dpdk-dev,v4,2/9] bus: add device iterator

Message ID 24f6e244a99903f543925067aa52baafe10cfc84.1497999601.git.gaetan.rivet@6wind.com (mailing list archive)
State Superseded, archived
Headers

Checks

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

Commit Message

Gaëtan Rivet June 20, 2017, 11:29 p.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/common/include/rte_bus.h |  7 +++++++
 lib/librte_eal/common/include/rte_dev.h | 17 +++++++++++++++++
 2 files changed, 24 insertions(+)
  

Comments

Thomas Monjalon June 21, 2017, 11:55 a.m. UTC | #1
21/06/2017 01:29, Gaetan Rivet:
> +/**
> + * Device comparison function.
> + *
> + * @param dev
> + *   Device handle.
> + *
> + * @param data
> + *   Data to compare against.
> + *
> + * @return
> + *   0 if the device matches the data.
> + *   !0 if the device does not match.
> + *   <0 if ordering is possible and the device is lower than the data.
> + *   >0 if ordering is possible and the device is greater than the data.
> + */
> +typedef int (*rte_dev_cmp_t)(const struct rte_device *dev, const void *data);

data is really abstract.
Maybe a comment is missing to explain that data is better specified
in bus implementations?

Why not implement it for PCI?
  
Gaëtan Rivet June 21, 2017, 12:15 p.m. UTC | #2
On Wed, Jun 21, 2017 at 01:55:39PM +0200, Thomas Monjalon wrote:
> 21/06/2017 01:29, Gaetan Rivet:
> > +/**
> > + * Device comparison function.
> > + *
> > + * @param dev
> > + *   Device handle.
> > + *
> > + * @param data
> > + *   Data to compare against.
> > + *
> > + * @return
> > + *   0 if the device matches the data.
> > + *   !0 if the device does not match.
> > + *   <0 if ordering is possible and the device is lower than the data.
> > + *   >0 if ordering is possible and the device is greater than the data.
> > + */
> > +typedef int (*rte_dev_cmp_t)(const struct rte_device *dev, const void *data);
> 
> data is really abstract.
> Maybe a comment is missing to explain that data is better specified
> in bus implementations?
> 

I'm not sure it is better specified in rte_bus though :).
However, the usage can be understood there, why it exists in the first
place.

I think bus iterators could benefit some more explanation about the why.

> Why not implement it for PCI?
> 

I sent this series with only the patches from Jan, initially in the
version he solely developed. Only afterward did I fix a few bugs,
reworked a few APIs.

As such, two other series complete this patchset:

[dpdk-dev] [PATCH v3] pci: implement find_device bus operation
http://dpdk.org/ml/archives/dev/2017-June/067485.html

And

[dpdk-dev] [PATCH v3 0/3] eal: complete attach / detach support
http://dpdk.org/ml/archives/dev/2017-June/067516.html

It might make sense to merge all three series together.
They are conceptually linked very closely. The only reason I did not do
so at first was because I was unsure about who would take responsibility
for the attach / detach patchset, and if it had not be me I did not want to
put undue responsibility of my patches on whomever would.

But that point is moot now.
  

Patch

diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 3e26d4b..fe9573f 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -82,6 +82,12 @@  typedef int (*rte_bus_scan_t)(void);
 typedef int (*rte_bus_probe_t)(void);
 
 /**
+ * Device iterator to find a particular device on a bus.
+ */
+typedef struct rte_device * (*rte_bus_find_device_t)(rte_dev_cmp_t match,
+						     const void *data);
+
+/**
  * A structure describing a generic bus.
  */
 struct rte_bus {
@@ -89,6 +95,7 @@  struct rte_bus {
 	const char *name;            /**< Name of the bus */
 	rte_bus_scan_t scan;         /**< Scan for devices attached to bus */
 	rte_bus_probe_t probe;       /**< Probe devices on bus */
+	rte_bus_find_device_t find_device; /**< Find device on bus */
 };
 
 /**
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index de20c06..724855e 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -191,6 +191,23 @@  int rte_eal_dev_attach(const char *name, const char *devargs);
  */
 int rte_eal_dev_detach(const char *name);
 
+/**
+ * Device comparison function.
+ *
+ * @param dev
+ *   Device handle.
+ *
+ * @param data
+ *   Data to compare against.
+ *
+ * @return
+ *   0 if the device matches the data.
+ *   !0 if the device does not match.
+ *   <0 if ordering is possible and the device is lower than the data.
+ *   >0 if ordering is possible and the device is greater than the data.
+ */
+typedef int (*rte_dev_cmp_t)(const struct rte_device *dev, const void *data);
+
 #define RTE_PMD_EXPORT_NAME_ARRAY(n, idx) n##idx[]
 
 #define RTE_PMD_EXPORT_NAME(name, idx) \