[v1,02/11] drivers/bus/ifpga: add function for AFU search by name

Message ID 1551338000-120348-3-git-send-email-rosen.xu@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series Add patch set for IPN3KE |

Checks

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

Commit Message

Xu, Rosen Feb. 28, 2019, 7:13 a.m. UTC
  In many scenarios, AFU is needed searched by name, this
function add the feature.

Signed-off-by: Rosen Xu <rosen.xu@intel.com>
Signed-off-by: Andy Pei <andy.pei@intel.com>
---
 drivers/bus/ifpga/ifpga_bus.c     | 13 +++++++++++++
 drivers/bus/ifpga/rte_bus_ifpga.h |  3 +++
 2 files changed, 16 insertions(+)
  

Comments

Ferruh Yigit March 6, 2019, 12:44 p.m. UTC | #1
On 2/28/2019 7:13 AM, Rosen Xu wrote:
> In many scenarios, AFU is needed searched by name, this
> function add the feature.
> 
> Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> Signed-off-by: Andy Pei <andy.pei@intel.com>
> ---
>  drivers/bus/ifpga/ifpga_bus.c     | 13 +++++++++++++
>  drivers/bus/ifpga/rte_bus_ifpga.h |  3 +++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
> index 55d3abf..dfd6b1f 100644
> --- a/drivers/bus/ifpga/ifpga_bus.c
> +++ b/drivers/bus/ifpga/ifpga_bus.c
> @@ -73,6 +73,19 @@ void rte_ifpga_driver_unregister(struct rte_afu_driver *driver)
>  	return NULL;
>  }
>  
> +struct rte_afu_device *
> +rte_ifpga_find_afu_by_name(const char *name)
> +{
> +	struct rte_afu_device *afu_dev = NULL;
> +
> +	TAILQ_FOREACH(afu_dev, &ifpga_afu_dev_list, next) {
> +		if (afu_dev &&
> +			!strcmp(afu_dev->device.name, name))
> +			return afu_dev;
> +	}
> +	return NULL;
> +}

I can see this function called from drivers in following patches, did you test
this via shared library build? To be able to called from different library,
should this API needs to be added into .map file?

> +
>  static const char * const valid_args[] = {
>  #define IFPGA_ARG_NAME         "ifpga"
>  	IFPGA_ARG_NAME,
> diff --git a/drivers/bus/ifpga/rte_bus_ifpga.h b/drivers/bus/ifpga/rte_bus_ifpga.h
> index 820eeaa..5762a33 100644
> --- a/drivers/bus/ifpga/rte_bus_ifpga.h
> +++ b/drivers/bus/ifpga/rte_bus_ifpga.h
> @@ -119,6 +119,9 @@ struct rte_afu_driver {
>  	return NULL;
>  }
>  
> +struct rte_afu_device *
> +rte_ifpga_find_afu_by_name(const char *name);

This is public header, can you please add function comments in doxygen format?
  
Xu, Rosen March 11, 2019, 1:10 p.m. UTC | #2
Hi,

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, March 06, 2019 20:45
> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> Cc: Zhang, Tianfei <tianfei.zhang@intel.com>; Wei, Dan
> <dan.wei@intel.com>; Pei, Andy <andy.pei@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Wang, Haiyue <haiyue.wang@intel.com>; Chen,
> Santos <santos.chen@intel.com>; Zhang, Zhang <zhang.zhang@intel.com>
> Subject: Re: [PATCH v1 02/11] drivers/bus/ifpga: add function for AFU search
> by name
> 
> On 2/28/2019 7:13 AM, Rosen Xu wrote:
> > In many scenarios, AFU is needed searched by name, this function add
> > the feature.
> >
> > Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> > Signed-off-by: Andy Pei <andy.pei@intel.com>
> > ---
> >  drivers/bus/ifpga/ifpga_bus.c     | 13 +++++++++++++
> >  drivers/bus/ifpga/rte_bus_ifpga.h |  3 +++
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/drivers/bus/ifpga/ifpga_bus.c
> > b/drivers/bus/ifpga/ifpga_bus.c index 55d3abf..dfd6b1f 100644
> > --- a/drivers/bus/ifpga/ifpga_bus.c
> > +++ b/drivers/bus/ifpga/ifpga_bus.c
> > @@ -73,6 +73,19 @@ void rte_ifpga_driver_unregister(struct
> rte_afu_driver *driver)
> >  	return NULL;
> >  }
> >
> > +struct rte_afu_device *
> > +rte_ifpga_find_afu_by_name(const char *name) {
> > +	struct rte_afu_device *afu_dev = NULL;
> > +
> > +	TAILQ_FOREACH(afu_dev, &ifpga_afu_dev_list, next) {
> > +		if (afu_dev &&
> > +			!strcmp(afu_dev->device.name, name))
> > +			return afu_dev;
> > +	}
> > +	return NULL;
> > +}
> 
> I can see this function called from drivers in following patches, did you test
> this via shared library build? To be able to called from different library,
> should this API needs to be added into .map file?

Fix in patch v2.

> > +
> >  static const char * const valid_args[] = {
> >  #define IFPGA_ARG_NAME         "ifpga"
> >  	IFPGA_ARG_NAME,
> > diff --git a/drivers/bus/ifpga/rte_bus_ifpga.h
> > b/drivers/bus/ifpga/rte_bus_ifpga.h
> > index 820eeaa..5762a33 100644
> > --- a/drivers/bus/ifpga/rte_bus_ifpga.h
> > +++ b/drivers/bus/ifpga/rte_bus_ifpga.h
> > @@ -119,6 +119,9 @@ struct rte_afu_driver {
> >  	return NULL;
> >  }
> >
> > +struct rte_afu_device *
> > +rte_ifpga_find_afu_by_name(const char *name);
> 
> This is public header, can you please add function comments in doxygen
> format?

Okay.
  

Patch

diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
index 55d3abf..dfd6b1f 100644
--- a/drivers/bus/ifpga/ifpga_bus.c
+++ b/drivers/bus/ifpga/ifpga_bus.c
@@ -73,6 +73,19 @@  void rte_ifpga_driver_unregister(struct rte_afu_driver *driver)
 	return NULL;
 }
 
+struct rte_afu_device *
+rte_ifpga_find_afu_by_name(const char *name)
+{
+	struct rte_afu_device *afu_dev = NULL;
+
+	TAILQ_FOREACH(afu_dev, &ifpga_afu_dev_list, next) {
+		if (afu_dev &&
+			!strcmp(afu_dev->device.name, name))
+			return afu_dev;
+	}
+	return NULL;
+}
+
 static const char * const valid_args[] = {
 #define IFPGA_ARG_NAME         "ifpga"
 	IFPGA_ARG_NAME,
diff --git a/drivers/bus/ifpga/rte_bus_ifpga.h b/drivers/bus/ifpga/rte_bus_ifpga.h
index 820eeaa..5762a33 100644
--- a/drivers/bus/ifpga/rte_bus_ifpga.h
+++ b/drivers/bus/ifpga/rte_bus_ifpga.h
@@ -119,6 +119,9 @@  struct rte_afu_driver {
 	return NULL;
 }
 
+struct rte_afu_device *
+rte_ifpga_find_afu_by_name(const char *name);
+
 /**
  * Register a ifpga afu device driver.
  *