Re: [dpdk-dev] [RFC 1/4] drivers/bus/ifpga:Intel FPGA Bus Lib Code

["Gaëtan Rivet" <gaetan.rivet@6wind.com>]

Hi Rosen,

A few comments inline.
(I will skip elements already pointed out by Shreyansh.)

On Tue, Mar 06, 2018 at 09:43:55AM +0800, Rosen Xu wrote:
> Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> ---
>  drivers/bus/ifpga/Makefile                  |  64 ++++
>  drivers/bus/ifpga/ifpga_bus.c               | 527 ++++++++++++++++++++++++++++
>  drivers/bus/ifpga/ifpga_common.c            | 168 +++++++++
>  drivers/bus/ifpga/ifpga_common.h            |  46 +++
>  drivers/bus/ifpga/ifpga_logs.h              |  59 ++++
>  drivers/bus/ifpga/rte_bus_ifpga.h           | 153 ++++++++
>  drivers/bus/ifpga/rte_bus_ifpga_version.map |   8 +
>  7 files changed, 1025 insertions(+)
>  create mode 100644 drivers/bus/ifpga/Makefile
>  create mode 100644 drivers/bus/ifpga/ifpga_bus.c
>  create mode 100644 drivers/bus/ifpga/ifpga_common.c
>  create mode 100644 drivers/bus/ifpga/ifpga_common.h
>  create mode 100644 drivers/bus/ifpga/ifpga_logs.h
>  create mode 100644 drivers/bus/ifpga/rte_bus_ifpga.h
>  create mode 100644 drivers/bus/ifpga/rte_bus_ifpga_version.map
> 
> diff --git a/drivers/bus/ifpga/Makefile b/drivers/bus/ifpga/Makefile
> new file mode 100644
> index 0000000..c71f186
> --- /dev/null
> +++ b/drivers/bus/ifpga/Makefile
> @@ -0,0 +1,64 @@
> +#   BSD LICENSE
> +#
> +#   Copyright(c) 2010-2017 Intel Corporation. All rights reserved.
> +#   All rights reserved.
> +#
> +#   Redistribution and use in source and binary forms, with or without
> +#   modification, are permitted provided that the following conditions
> +#   are met:
> +#
> +#     * Redistributions of source code must retain the above copyright
> +#       notice, this list of conditions and the following disclaimer.
> +#     * Redistributions in binary form must reproduce the above copyright
> +#       notice, this list of conditions and the following disclaimer in
> +#       the documentation and/or other materials provided with the
> +#       distribution.
> +#     * Neither the name of Intel Corporation nor the names of its
> +#       contributors may be used to endorse or promote products derived
> +#       from this software without specific prior written permission.
> +#
> +#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +#
> +# library name
> +#
> +LIB = librte_bus_ifpga.a
> +LIBABIVER := 1
> +EXPORT_MAP := rte_bus_ifpga_version.map
> +
> +ifeq ($(CONFIG_RTE_LIBRTE_DPAA2_DEBUG_INIT),y)
> +CFLAGS += -O0 -g
> +CFLAGS += "-Wno-error"
> +else
> +CFLAGS += -O3
> +CFLAGS += $(WERROR_FLAGS)
> +endif
> +
> +CFLAGS += -I$(RTE_SDK)/drivers/bus/ifpga
> +CFLAGS += -I$(RTE_SDK)/drivers/bus/pci
> +CFLAGS += -I$(RTE_SDK)/lib/librte_eal/linuxapp/eal
> +CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common
> +#CFLAGS += -I$(RTE_SDK)/lib/librte_rawdev
> +#LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring -lrte_rawdev
> +LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
> +#LDLIBS += -lrte_ethdev
> +
> +VPATH += $(SRCDIR)/base
> +
> +SRCS-y += \
> +        ifpga_bus.c \
> +        ifpga_common.c
> +
> +include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
> new file mode 100644
> index 0000000..382d550
> --- /dev/null
> +++ b/drivers/bus/ifpga/ifpga_bus.c
> @@ -0,0 +1,527 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + *   Copyright 2013-2014 6WIND S.A.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +
> +#include <string.h>
> +#include <inttypes.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <sys/queue.h>
> +#include <sys/mman.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +
> +#include <rte_errno.h>
> +#include <rte_bus.h>
> +#include <rte_per_lcore.h>
> +#include <rte_memory.h>
> +#include <rte_memzone.h>
> +#include <rte_eal.h>
> +#include <rte_common.h>
> +
> +#include <rte_devargs.h>
> +#include <rte_pci.h>
> +#include <rte_bus_pci.h>
> +#include <rte_kvargs.h>
> +#include <rte_alarm.h>
> +
> +#include "rte_rawdev.h"
> +#include "rte_rawdev_pmd.h"
> +#include "rte_bus_ifpga.h"
> +#include "ifpga_logs.h"
> +#include "ifpga_common.h"
> +
> +int ifpga_bus_logtype;
> +
> +/*register a ifpga bus based driver */
> +void rte_ifpga_driver_register(struct rte_afu_driver *driver)
> +{
> +	RTE_VERIFY(driver);
> +
> +	TAILQ_INSERT_TAIL(&rte_ifpga_bus.driver_list, driver, next);
> +}
> +
> +/*un-register a fpga bus based driver */
> +void rte_ifpga_driver_unregister(struct rte_afu_driver *driver)
> +{
> +	TAILQ_REMOVE(&rte_ifpga_bus.driver_list, driver, next);
> +}
> +
> +static struct rte_afu_device *
> +ifpga_find_afu_dev(const struct rte_afu_id *afu_id)
> +{
> +	struct rte_afu_device *afu_dev = NULL;
> +	

You seem to have trailing spaces (here tabs), and some inconsistencies
in your file (sometimes spaces are used, sometime tabs).

You need to run checkpatch on your series.

See those errors:
http://dpdk.org/ml/archives/test-report/2018-March/043185.html

A script is available in devtools to run this before submitting.

> +	TAILQ_FOREACH (afu_dev, &rte_ifpga_bus.afu_list, next) {
> +
> +        if(!ifpga_afu_id_cmp(&afu_dev->id, afu_id)) {
> +            return afu_dev;
> +        }
> +	}
> +	return NULL;
> +}
> +
> +static const char *valid_args[] = {
> +#define IFPGA_ARG_BDF          "bdf"
> +	IFPGA_ARG_BDF,
> +#define IFPGA_ARG_PORT         "port"
> +	IFPGA_ARG_PORT,
> +#define IFPGA_ARG_PATH         "path"
> +	IFPGA_ARG_PATH,
> +#define IFPGA_ARG_UUID_HIGH    "uuid_high"
> +	IFPGA_ARG_UUID_HIGH,
> +#define IFPGA_ARG_UUID_LOW     "uuid_low"
> +	IFPGA_ARG_UUID_LOW,
> +#define IFPGA_ARG_PR_ENABLE     "pr_enable"
> +	IFPGA_ARG_PR_ENABLE,
> +#define IFPGA_ARG_DEBUG         "debug"
> +	IFPGA_ARG_DEBUG,
> +	NULL
> +};
> +
> +/*
> + * Scan the content of the FPGA bus, and the devices in the devices
> + * list
> + */
> +static struct rte_afu_device *
> +rte_ifpga_scan_one(struct rte_devargs *devargs)
> +{
> +	struct rte_kvargs *kvlist = NULL;
> +	struct rte_bus *pci_bus = NULL;
> +	struct rte_device *dev = NULL;
> +	struct rte_rawdev *rawdev;
> +	struct rte_afu_device *afu_dev = NULL;
> +	struct rte_afu_pr_conf afu_pr_conf;
> +	char rawdev_name[RTE_RAWDEV_NAME_MAX_LEN];
> +	int ret;
> +	char *path = NULL;
> +	int pr_enable = 1;
> +	int debug = 0;
> +
> +    memset((char *)(&afu_pr_conf), 0, sizeof(struct rte_afu_pr_conf));
> +	
> +    kvlist = rte_kvargs_parse(devargs->args, valid_args);
> +    if (!kvlist) {
> +    	IFPGA_BUS_ERR("error when parsing param");
> +    	goto end;
> +    }
> +    
> +    if (rte_kvargs_count(kvlist, IFPGA_ARG_BDF) == 1) {
> +		if (rte_kvargs_process(kvlist, IFPGA_ARG_BDF,
> +				       &ifpga_get_bdf_arg, &afu_pr_conf.afu_id.pci_addr) < 0) {
> +			IFPGA_BUS_ERR("error to parse %s",
> +				     IFPGA_ARG_BDF);
> +			goto end;
> +		}
> +	} else {
> +		IFPGA_BUS_ERR("arg %s is mandatory for ifpga bus",
> +			  IFPGA_ARG_PATH);
> +		goto end;
> +	}
> +    
> +    if (rte_kvargs_count(kvlist, IFPGA_ARG_PORT) == 1) {
> +		if (rte_kvargs_process(kvlist, IFPGA_ARG_PORT,
> +				       &ifpga_get_integer32_arg, &afu_pr_conf.afu_id.port) < 0) {
> +			IFPGA_BUS_ERR("error to parse %s",
> +				     IFPGA_ARG_PORT);
> +			goto end;
> +		}
> +	}
> +	else {
> +		IFPGA_BUS_ERR("arg %s is mandatory for ifpga bus",
> +			  IFPGA_ARG_PATH);
> +		goto end;
> +	}
> +	
> +    if (rte_kvargs_count(kvlist, IFPGA_ARG_PATH) == 1) {
> +		if (rte_kvargs_process(kvlist, IFPGA_ARG_PATH,
> +				       &ifpga_get_string_arg, &path) < 0) {
> +			IFPGA_BUS_ERR("error to parse %s",
> +				     IFPGA_ARG_PATH);
> +			goto end;
> +		}
> +	} else {
> +		IFPGA_BUS_ERR("arg %s is mandatory for ifpga bus",
> +			  IFPGA_ARG_PATH);
> +		goto end;
> +	}
> +	
> +	if (rte_kvargs_count(kvlist, IFPGA_ARG_UUID_HIGH) == 1) {
> +		if (rte_kvargs_process(kvlist, IFPGA_ARG_UUID_HIGH,
> +				       &ifpga_get_integer64_arg, &afu_pr_conf.afu_id.uuid_high) < 0) {
> +			IFPGA_BUS_ERR("error to parse %s",
> +				     IFPGA_ARG_UUID_HIGH);
> +			goto end;
> +		}
> +	} else {
> +		IFPGA_BUS_ERR("arg %s is mandatory for ifpga bus",
> +			  IFPGA_ARG_PATH);
> +		goto end;
> +	}
> +	
> +	if (rte_kvargs_count(kvlist, IFPGA_ARG_UUID_LOW) == 1) {
> +		if (rte_kvargs_process(kvlist, IFPGA_ARG_UUID_LOW,
> +				       &ifpga_get_integer64_arg, &afu_pr_conf.afu_id.uuid_low) < 0) {
> +			IFPGA_BUS_ERR("error to parse %s",
> +				     IFPGA_ARG_UUID_LOW);
> +			goto end;
> +		}
> +	} else {
> +		IFPGA_BUS_ERR("arg %s is mandatory for ifpga bus",
> +			  IFPGA_ARG_PATH);
> +		goto end;
> +	}
> +	
> +	if (rte_kvargs_count(kvlist, IFPGA_ARG_PR_ENABLE) == 1) {
> +		if (rte_kvargs_process(kvlist, IFPGA_ARG_PR_ENABLE,
> +				       &ifpga_get_integer32_arg, &pr_enable) < 0) {
> +			IFPGA_BUS_ERR("error to parse %s",
> +				     IFPGA_ARG_UUID_HIGH);
> +			goto end;
> +		}
> +	}
> +	
> +	if (rte_kvargs_count(kvlist, IFPGA_ARG_DEBUG) == 1) {
> +		if (rte_kvargs_process(kvlist, IFPGA_ARG_DEBUG,
> +				       &ifpga_get_integer32_arg, &debug) < 0) {
> +			IFPGA_BUS_ERR("error to parse %s",
> +				     IFPGA_ARG_UUID_HIGH);
> +			goto end;
> +		}
> +	}
> +	
> +	if(!debug)
> +	{
> +		pci_bus = rte_bus_find_by_name("pci");
> +	    if (pci_bus == NULL) {
> +	    	IFPGA_BUS_ERR("unable to find PCI bus\n");
> +	    	goto end;
> +	    }
> +	    
> +	    dev = pci_bus->find_device(NULL, ifpga_pci_addr_cmp, &afu_pr_conf.afu_id.pci_addr);
> +	    if (dev == NULL) {
> +	    	IFPGA_BUS_ERR("unable to find PCI device\n");
> +	    	goto end;
> +	    }
> +	}
> +	else
> +	{
> +		IFPGA_BUS_DEBUG("pci_addr domain   : %x\n",   afu_pr_conf.afu_id.pci_addr.domain);
> +		IFPGA_BUS_DEBUG("pci_addr bus      : %x\n",   afu_pr_conf.afu_id.pci_addr.bus);
> +		IFPGA_BUS_DEBUG("pci_addr devid    : %x\n",   afu_pr_conf.afu_id.pci_addr.devid);
> +		IFPGA_BUS_DEBUG("pci_addr function : %x\n",   afu_pr_conf.afu_id.pci_addr.function);

Alternatively, it could be interesting to have a PCI_ADDR_FMT and
PCI_ADDR_ARGS in librte_pci, to help printing PCI addresses in a
standard way. This could be leveraged in librte_bus_pci as well.

> +		
> +		IFPGA_BUS_DEBUG("uuid_low          : %lx\n", afu_pr_conf.afu_id.uuid_low);
> +		IFPGA_BUS_DEBUG("uuid_high         : %lx\n", afu_pr_conf.afu_id.uuid_high);
> +		
> +		IFPGA_BUS_DEBUG("afu port          : %x\n",   afu_pr_conf.afu_id.port);
> +	}
> +	
> +	if (ifpga_find_afu_dev(&afu_pr_conf.afu_id))
> +		goto end;
> +
> +	memset(rawdev_name, sizeof(rawdev_name), 0);
> +	snprintf(rawdev_name, RTE_RAWDEV_NAME_MAX_LEN, "IFPGA:%x:%x:%x", afu_pr_conf.afu_id.pci_addr.bus, afu_pr_conf.afu_id.pci_addr.devid, afu_pr_conf.afu_id.pci_addr.function);
> +	rawdev = rte_rawdev_pmd_get_named_dev(rawdev_name);
> +	if(!rawdev)
> +		goto end;
> +	
> +	rawdev->dev_ops->dev_start(rawdev);
> +		
> +	if (pr_enable)
> +	{
> +		ret=rawdev->dev_ops->firmware_load(rawdev, &afu_pr_conf);
> +		if (ret) {
> +    		printf("do pr error\n");
> +    		return NULL;
> +	    }
> +	}
> +		
> +	afu_dev = calloc(1, sizeof(*afu_dev));
> +	if (!afu_dev)
> +		goto end;
> +
> +	afu_dev->device.devargs = devargs;
> +	afu_dev->device.numa_node = SOCKET_ID_ANY;
> +	afu_dev->device.name = devargs->name;
> +	afu_dev->rawdev = rawdev;
> +	afu_dev->id.pci_addr.domain = afu_pr_conf.afu_id.pci_addr.domain;
> +	afu_dev->id.pci_addr.bus = afu_pr_conf.afu_id.pci_addr.bus;
> +	afu_dev->id.pci_addr.devid = afu_pr_conf.afu_id.pci_addr.devid;
> +	afu_dev->id.pci_addr.function = afu_pr_conf.afu_id.pci_addr.function;

A pci_addr_cpy function could be written.

> +	afu_dev->id.uuid_low  = afu_pr_conf.afu_id.uuid_low;
> +	afu_dev->id.uuid_high = afu_pr_conf.afu_id.uuid_high;
> +	afu_dev->id.port      = afu_pr_conf.afu_id.port;
> +	
> +	rawdev->dev_ops->dev_info_get(rawdev, afu_dev);
> +
> +    return afu_dev;
> +    
> +end:
> +	if (kvlist)
> +		rte_kvargs_free(kvlist);
> +	if (path)
> +		free(path);
> +	
> +	return NULL;
> +}
> +
> +/*
> + * Scan the content of the FPGA bus, and the devices in the devices
> + * list
> + */
> +static int
> +rte_ifpga_scan(void)
> +{
> +	struct rte_bus *pci_bus;
> +	struct rte_afu_device *afu_dev;
> +	struct rte_devargs *devargs;
> +
> +    pci_bus = rte_bus_find_by_name("pci");
> +	if (pci_bus == NULL) {
> +		IFPGA_BUS_ERR("unable to find PCI bus\n");
> +		return -1;
> +	}
> +	
> +	/* for virtual devices we scan the devargs_list populated via cmdline */
> +	TAILQ_FOREACH(devargs, &devargs_list, next) {
> +    
> +		if (devargs->bus != &rte_ifpga_bus.bus)
> +			continue;
> +		
> +		afu_dev = rte_ifpga_scan_one(devargs);
> +	    
> +	    if(afu_dev)
> +		    TAILQ_INSERT_TAIL(&rte_ifpga_bus.afu_list, afu_dev, next);
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +ifpga_probe_one_driver(struct rte_afu_driver *drv,
> +			struct rte_afu_device *afu_dev)
> +{
> +	int ret;
> +
> +    if((drv->id.pci_addr.bus == afu_dev->id.pci_addr.bus) &&
> +       (drv->id.pci_addr.devid == afu_dev->id.pci_addr.devid) &&
> +       (drv->id.pci_addr.function == afu_dev->id.pci_addr.function) &&

Is it expected to ignore the domain here?
You should use rte_pci_addr_cmp instead.

> +       (drv->id.uuid_low == afu_dev->id.uuid_low) &&
> +       (drv->id.uuid_high == afu_dev->id.uuid_high) &&
> +       (drv->id.port == afu_dev->id.port)) {
> +       	
> +       	afu_dev->driver = drv;
> +       	
> +       	/* call the driver probe() function */
> +		ret = drv->probe(afu_dev);
> +		if (ret)
> +			afu_dev->driver = NULL;
> +		return ret;
> +    }
> +	
> +	/* return positive value if driver doesn't support this device */
> +	return 1;
> +}
> +
> +static int
> +ifpga_probe_all_drivers(struct rte_afu_device *afu_dev)
> +{
> +	const char *name;
> +	struct rte_afu_driver *drv = NULL;
> +	int rc;
> +
> +	if (afu_dev == NULL)
> +		return -1;
> +
> +	/* Check if a driver is already loaded */
> +	if (afu_dev->driver != NULL)
> +		return 0;
> +		
> +    name = rte_ifpga_device_name(afu_dev);
> +    IFPGA_BUS_DEBUG("Search driver %s to probe device %s\n", name,
> +		rte_ifpga_device_name(afu_dev));
> +    
> +	TAILQ_FOREACH(drv, &rte_ifpga_bus.driver_list, next) {
> +		rc = ifpga_probe_one_driver(drv, afu_dev);
> +		if (rc < 0)
> +			/* negative value is an error */
> +			return -1;
> +		if (rc > 0)
> +			/* positive value means driver doesn't support it */
> +			continue;
> +		return 0;
> +	}
> +	return 1;
> + }
> +
> +/*
> + * Scan the content of the PCI bus, and call the probe() function for
> + * all registered drivers that have a matching entry in its id_table
> + * for discovered devices.
> + */
> +static int
> +rte_ifpga_probe(void)
> +{
> +	struct rte_afu_device *afu_dev = NULL;
> +	int ret = 0;
> +
> +	TAILQ_FOREACH (afu_dev, &rte_ifpga_bus.afu_list, next) {
> +
> +        if (afu_dev->device.driver)
> +			continue;
> +			
> +		ret = ifpga_probe_all_drivers(afu_dev);
> +		if (ret < 0)
> +		    IFPGA_BUS_ERR("failed to initialize %s device\n",
> +				rte_ifpga_device_name(afu_dev));
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +rte_ifpga_plug(struct rte_device *dev)
> +{
> +	return ifpga_probe_all_drivers(RTE_DEV_TO_AFU(dev));
> +}
> +
> +static int ifpga_remove_driver(struct rte_afu_device *afu_dev)
> +{
> +	const char *name;
> +	const struct rte_afu_driver *driver;
> +
> +    name = rte_ifpga_device_name(afu_dev);
> +	if (!afu_dev->device.driver) {
> +		IFPGA_BUS_DEBUG("no driver attach to device %s\n", name);
> +		return 1;
> +	}
> +
> +	driver = container_of(afu_dev->device.driver, const struct rte_afu_driver,
> +		driver);
> +	return driver->remove(afu_dev);
> +}
> +
> +static int
> +rte_ifpga_unplug(struct rte_device *dev)
> +{
> +	struct rte_afu_device *afu_dev;
> +	struct rte_devargs *devargs;
> +	int ret;
> +
> +	if (dev == NULL)
> +		return -EINVAL;
> +
> +	afu_dev = RTE_DEV_TO_AFU(dev);
> +	if (!dev)
> +		return -ENOENT;
> +
> +	devargs = dev->devargs;
> +
> +	ret = ifpga_remove_driver(afu_dev);
> +	if (ret)
> +		return ret;
> +
> +	TAILQ_REMOVE(&rte_ifpga_bus.afu_list, afu_dev, next);
> +
> +	TAILQ_REMOVE(&devargs_list, devargs, next);
> +
> +	free(devargs->args);
> +	free(devargs);
> +	free(afu_dev);
> +	return 0;
> +
> +}
> +
> +static struct rte_device *
> +rte_ifpga_find_device(const struct rte_device *start, rte_dev_cmp_t cmp,
> +		                                        const void *data)
> +{
> +	struct rte_afu_device *afu_dev;
> +
> +	TAILQ_FOREACH(afu_dev, &rte_ifpga_bus.afu_list, next) {
> +		if (start && &afu_dev->device == start) {
> +			start = NULL;
> +			continue;
> +		}
> +		if (cmp(&afu_dev->device, data) == 0)
> +			return &afu_dev->device;
> +	}
> +	return NULL;
> +}
> +static int
> +rte_ifpga_parse(const char *name, void *addr)
> +{
> +	struct rte_afu_driver **out = addr;
> +	struct rte_afu_driver *driver = NULL;
> +
> +	TAILQ_FOREACH(driver, &rte_ifpga_bus.driver_list, next) {
> +		if (strncmp(driver->driver.name, name,
> +			    strlen(driver->driver.name)) == 0)
> +			break;
> +		if (driver->driver.alias &&
> +		    strncmp(driver->driver.alias, name,
> +			    strlen(driver->driver.alias)) == 0)
> +			break;
> +	}
> +	if (driver != NULL &&
> +	    addr != NULL)
> +		*out = driver;
> +	return driver == NULL;
> +}
> +
> +struct rte_ifpga_bus rte_ifpga_bus = {
> +	 .bus = {
> +		 .scan        = rte_ifpga_scan,
> +		 .probe       = rte_ifpga_probe,
> +		 .find_device = rte_ifpga_find_device,

You should use only one tab each lines.

> +	     .plug        = rte_ifpga_plug,
> +	     .unplug      = rte_ifpga_unplug,
> +	     .parse       = rte_ifpga_parse,
> +	 },
> +	.afu_list    = TAILQ_HEAD_INITIALIZER(rte_ifpga_bus.afu_list),
> +	.driver_list = TAILQ_HEAD_INITIALIZER(rte_ifpga_bus.driver_list),
> + };
> +
> +RTE_REGISTER_BUS(IFPGA_BUS_NAME, rte_ifpga_bus.bus);
> +
> +RTE_INIT(ifpga_init_log)
> +{
> +	ifpga_bus_logtype = rte_log_register("bus.ifpga");
> +	if (ifpga_bus_logtype >= 0)
> +		rte_log_set_level(ifpga_bus_logtype, RTE_LOG_NOTICE);
> +}
> +
> diff --git a/drivers/bus/ifpga/ifpga_common.c b/drivers/bus/ifpga/ifpga_common.c
> new file mode 100644
> index 0000000..2aa6bb0
> --- /dev/null
> +++ b/drivers/bus/ifpga/ifpga_common.c
> @@ -0,0 +1,168 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + *   Copyright 2013-2014 6WIND S.A.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +
> +#include <string.h>
> +#include <inttypes.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <sys/queue.h>
> +#include <sys/mman.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +
> +#include <rte_errno.h>
> +#include <rte_bus.h>
> +#include <rte_per_lcore.h>
> +#include <rte_memory.h>
> +#include <rte_memzone.h>
> +#include <rte_eal.h>
> +#include <rte_common.h>
> +
> +#include <rte_devargs.h>
> +#include <rte_pci.h>
> +#include <rte_bus_pci.h>
> +#include <rte_kvargs.h>
> +#include <rte_alarm.h>
> +
> +#include "rte_bus_ifpga.h"
> +#include "ifpga_logs.h"
> +#include "ifpga_common.h"
> +
> +int ifpga_get_string_arg(const char *key __rte_unused,
> +	                     const char *value, void *extra_args)
> +{
> +	if (!value || !extra_args)
> +		return -EINVAL;
> +
> +	*(char **)extra_args = strdup(value);
> +
> +	if (!*(char **)extra_args)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +int ifpga_get_integer32_arg(const char *key __rte_unused, const char *value, void *extra_args)
> +{
> +	if (!value || !extra_args)
> +		return -EINVAL;
> +
> +	*(int *)extra_args = strtoull(value, NULL, 0);
> +
> +	return 0;
> +}
> +int ifpga_get_integer64_arg(const char *key __rte_unused, const char *value, void *extra_args)
> +{
> +	if (!value || !extra_args)
> +		return -EINVAL;
> +
> +	*(uint64_t *)extra_args = strtoull(value, NULL, 0);
> +
> +	return 0;
> +}
> +int ifpga_get_unsigned_long(const char *str, int base)
> +{
> +	unsigned long num;
> +	char *end = NULL;
> +
> +	errno = 0;
> +
> +	num = strtoul(str, &end, base);
> +	if ((str[0] == '\0') || (end == NULL) || (*end != '\0') || (errno != 0))
> +		return -1;
> +
> +	return num;
> +
> +}
> +int ifpga_get_bdf_arg(const char *key __rte_unused, const char *value, void *extra_args)
> +{
> +#define MAX_PATH_LEN 1024
> +	struct rte_pci_addr *addr;
> +	int num[4];
> +	char str[MAX_PATH_LEN];
> +	int i, j;
> +	
> +	if (!value || !extra_args)
> +		return -EINVAL;
> +
> +    addr = (struct rte_pci_addr *)extra_args;

Okay, you seem to be reading a PCI address here.
Why not use directly rte_pci_addr_parse?

> +	strcpy(str, value);
> +	memset(num, 0, 4 * sizeof(num[0]));
> +	i = strlen(str) - 1;
> +	j = 3;
> +	while (i > 0 && j >= 0) {
> +		while ((str[i - 1] != ':' && str[i - 1] != '.') && i > 0)
> +			i--;
> +		num[j--] = ifpga_get_unsigned_long(&str[i], 16);
> +		i--;
> +		if (i >= 0)
> +			str[i] = '\0';
> +	}
> +	addr->domain = num[0];
> +	addr->bus = num[1];
> +	addr->devid = num[2];
> +	addr->function = num[3];
> +	printf("[%s]: bdf %04d:%02d:%02d.%02d\n",
> +			__func__,
> +			addr->domain,
> +			addr->bus,
> +			addr->devid,
> +			addr->function);
> +
> +	return 0;
> +}
> +int ifpga_afu_id_cmp(const struct rte_afu_id *afu_id0, const struct rte_afu_id *afu_id1)
> +{
> +	if((afu_id0->pci_addr.bus       == afu_id1->pci_addr.bus) &&
> +       (afu_id0->pci_addr.devid     == afu_id1->pci_addr.devid) &&
> +       (afu_id0->pci_addr.function  == afu_id1->pci_addr.function) &&

Same as before: you should use rte_pci_addr_cmp here?

> +       (afu_id0->uuid_low           == afu_id1->uuid_low) &&
> +       (afu_id0->uuid_high          == afu_id1->uuid_high) &&
> +       (afu_id0->port               == afu_id1->port)) {
> +       	
> +        return 0;
> +    }
> +    else
> +    	return 1;
> +}
> +int ifpga_pci_addr_cmp(const struct rte_device *dev, const void *_pci_addr)
> +{
> +	struct rte_pci_device *pdev;
> +	const struct rte_pci_addr *paddr = _pci_addr;
> +
> +	pdev = RTE_DEV_TO_PCI(*(struct rte_device **)(void *)&dev);
> +	return rte_eal_compare_pci_addr(&pdev->addr, paddr);

This function is deprecated, rte_pci_addr_cmp must be used instead.

<snip>

Regards,
-- 
Gaëtan Rivet
6WIND