Re: [PATCH] eventdev: remove PCI dependency

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

Hi Jerin,

On Thu, Jun 01, 2017 at 10:11:46PM +0530, Jerin Jacob wrote:
> Remove the PCI dependency from generic data structures
> and moved the PCI specific code to rte_event_pmd_pci*
> 

Thanks for working on this.

Do you plan on removing rte_pci.h in rte_eventdev_pmd.h? Do you think it
would be feasible?

> CC: Gaetan Rivet <gaetan.rivet@6wind.com>
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---
>  drivers/event/skeleton/skeleton_eventdev.c | 30 +++++++++-----
>  lib/librte_eventdev/rte_eventdev.c         | 37 +++++++-----------
>  lib/librte_eventdev/rte_eventdev.h         |  2 -
>  lib/librte_eventdev/rte_eventdev_pmd.h     | 63 ++++--------------------------
>  4 files changed, 41 insertions(+), 91 deletions(-)
> 
> diff --git a/drivers/event/skeleton/skeleton_eventdev.c b/drivers/event/skeleton/skeleton_eventdev.c
> index 800bd76e0..34684aba0 100644
> --- a/drivers/event/skeleton/skeleton_eventdev.c
> +++ b/drivers/event/skeleton/skeleton_eventdev.c
> @@ -427,18 +427,28 @@ static const struct rte_pci_id pci_id_skeleton_map[] = {
>  	},
>  };
>  
> -static struct rte_eventdev_driver pci_eventdev_skeleton_pmd = {
> -	.pci_drv = {
> -		.id_table = pci_id_skeleton_map,
> -		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
> -		.probe = rte_event_pmd_pci_probe,
> -		.remove = rte_event_pmd_pci_remove,
> -	},
> -	.eventdev_init = skeleton_eventdev_init,
> -	.dev_private_size = sizeof(struct skeleton_eventdev),
> +static int
> +event_skeleton_pci_probe(struct rte_pci_driver *pci_drv,
> +			 struct rte_pci_device *pci_dev)
> +{
> +	return rte_event_pmd_pci_probe(pci_drv, pci_dev,
> +		sizeof(struct skeleton_eventdev), skeleton_eventdev_init);
> +}
> +
> +static int
> +event_skeleton_pci_remove(struct rte_pci_device *pci_dev)
> +{
> +	return rte_event_pmd_pci_remove(pci_dev, NULL);
> +}
> +
> +static struct rte_pci_driver pci_eventdev_skeleton_pmd = {
> +	.id_table = pci_id_skeleton_map,
> +	.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
> +	.probe = event_skeleton_pci_probe,
> +	.remove = event_skeleton_pci_remove,
>  };
>  
> -RTE_PMD_REGISTER_PCI(event_skeleton_pci, pci_eventdev_skeleton_pmd.pci_drv);
> +RTE_PMD_REGISTER_PCI(event_skeleton_pci, pci_eventdev_skeleton_pmd);
>  RTE_PMD_REGISTER_PCI_TABLE(event_skeleton_pci, pci_id_skeleton_map);
>  
>  /* VDEV based event device */
> diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
> index 20afc3f0e..91f950666 100644
> --- a/lib/librte_eventdev/rte_eventdev.c
> +++ b/lib/librte_eventdev/rte_eventdev.c
> @@ -126,8 +126,6 @@ rte_event_dev_info_get(uint8_t dev_id, struct rte_event_dev_info *dev_info)
>  	dev_info->dequeue_timeout_ns = dev->data->dev_conf.dequeue_timeout_ns;
>  
>  	dev_info->dev = dev->dev;
> -	if (dev->driver)
> -		dev_info->driver_name = dev->driver->pci_drv.driver.name;
>  	return 0;
>  }
>  
> @@ -1250,18 +1248,18 @@ rte_event_pmd_vdev_uninit(const char *name)
>  
>  int
>  rte_event_pmd_pci_probe(struct rte_pci_driver *pci_drv,
> -			struct rte_pci_device *pci_dev)
> +			struct rte_pci_device *pci_dev,
> +			size_t private_data_size,
> +			eventdev_pmd_pci_callback_t devinit)
>  {
> -	struct rte_eventdev_driver *eventdrv;
>  	struct rte_eventdev *eventdev;
>  
>  	char eventdev_name[RTE_EVENTDEV_NAME_MAX_LEN];
>  
>  	int retval;
>  
> -	eventdrv = (struct rte_eventdev_driver *)pci_drv;
> -	if (eventdrv == NULL)
> -		return -ENODEV;
> +	if (devinit == NULL)
> +		return -EINVAL;
>  
>  	rte_pci_device_name(&pci_dev->addr, eventdev_name,
>  			sizeof(eventdev_name));
> @@ -1275,7 +1273,7 @@ rte_event_pmd_pci_probe(struct rte_pci_driver *pci_drv,
>  		eventdev->data->dev_private =
>  				rte_zmalloc_socket(
>  						"eventdev private structure",
> -						eventdrv->dev_private_size,
> +						private_data_size,
>  						RTE_CACHE_LINE_SIZE,
>  						rte_socket_id());
>  
> @@ -1285,10 +1283,9 @@ rte_event_pmd_pci_probe(struct rte_pci_driver *pci_drv,
>  	}
>  
>  	eventdev->dev = &pci_dev->device;
> -	eventdev->driver = eventdrv;
>  
>  	/* Invoke PMD device initialization function */
> -	retval = (*eventdrv->eventdev_init)(eventdev);
> +	retval = devinit(eventdev);
>  	if (retval == 0)
>  		return 0;
>  
> @@ -1307,12 +1304,12 @@ rte_event_pmd_pci_probe(struct rte_pci_driver *pci_drv,
>  }
>  
>  int
> -rte_event_pmd_pci_remove(struct rte_pci_device *pci_dev)
> +rte_event_pmd_pci_remove(struct rte_pci_device *pci_dev,
> +			 eventdev_pmd_pci_callback_t devuninit)
>  {
> -	const struct rte_eventdev_driver *eventdrv;
>  	struct rte_eventdev *eventdev;
>  	char eventdev_name[RTE_EVENTDEV_NAME_MAX_LEN];
> -	int ret;
> +	int ret = 0;
>  
>  	if (pci_dev == NULL)
>  		return -EINVAL;
> @@ -1324,22 +1321,16 @@ rte_event_pmd_pci_remove(struct rte_pci_device *pci_dev)
>  	if (eventdev == NULL)
>  		return -ENODEV;
>  
> -	eventdrv = (const struct rte_eventdev_driver *)pci_dev->driver;
> -	if (eventdrv == NULL)
> -		return -ENODEV;
> -
>  	/* Invoke PMD device un-init function */
> -	if (*eventdrv->eventdev_uninit) {
> -		ret = (*eventdrv->eventdev_uninit)(eventdev);
> -		if (ret)
> -			return ret;
> -	}
> +	if (devuninit)
> +		ret = devuninit(eventdev);
> +	if (ret)
> +		return ret;
>  
>  	/* Free event device */
>  	rte_event_pmd_release(eventdev);
>  
>  	eventdev->dev = NULL;
> -	eventdev->driver = NULL;
>  
>  	return 0;
>  }
> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> index 20e7293e0..c5b2b7453 100644
> --- a/lib/librte_eventdev/rte_eventdev.h
> +++ b/lib/librte_eventdev/rte_eventdev.h
> @@ -1063,8 +1063,6 @@ struct rte_eventdev {
>  	/**< Functions exported by PMD */
>  	struct rte_device *dev;
>  	/**< Device info. supplied by probing */
> -	const struct rte_eventdev_driver *driver;
> -	/**< Driver for this device */
>  
>  	RTE_STD_C11
>  	uint8_t attached : 1;
> diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h b/lib/librte_eventdev/rte_eventdev_pmd.h
> index 4005b3c98..faa6989b4 100644
> --- a/lib/librte_eventdev/rte_eventdev_pmd.h
> +++ b/lib/librte_eventdev/rte_eventdev_pmd.h
> @@ -87,60 +87,6 @@ extern "C" {
>  #define RTE_EVENTDEV_DETACHED  (0)
>  #define RTE_EVENTDEV_ATTACHED  (1)
>  
> -/**
> - * Initialisation function of a event driver invoked for each matching
> - * event PCI device detected during the PCI probing phase.
> - *
> - * @param dev
> - *   The dev pointer is the address of the *rte_eventdev* structure associated
> - *   with the matching device and which has been [automatically] allocated in
> - *   the *rte_event_devices* array.
> - *
> - * @return
> - *   - 0: Success, the device is properly initialised by the driver.
> - *        In particular, the driver MUST have set up the *dev_ops* pointer
> - *        of the *dev* structure.
> - *   - <0: Error code of the device initialisation failure.
> - */
> -typedef int (*eventdev_init_t)(struct rte_eventdev *dev);
> -
> -/**
> - * Finalisation function of a driver invoked for each matching
> - * PCI device detected during the PCI closing phase.
> - *
> - * @param dev
> - *   The dev pointer is the address of the *rte_eventdev* structure associated
> - *   with the matching device and which	has been [automatically] allocated in
> - *   the *rte_event_devices* array.
> - *
> - * @return
> - *   - 0: Success, the device is properly finalised by the driver.
> - *        In particular, the driver MUST free the *dev_ops* pointer
> - *        of the *dev* structure.
> - *   - <0: Error code of the device initialisation failure.
> - */
> -typedef int (*eventdev_uninit_t)(struct rte_eventdev *dev);
> -
> -/**
> - * The structure associated with a PMD driver.
> - *
> - * Each driver acts as a PCI driver and is represented by a generic
> - * *event_driver* structure that holds:
> - *
> - * - An *rte_pci_driver* structure (which must be the first field).
> - *
> - * - The *eventdev_init* function invoked for each matching PCI device.
> - *
> - * - The size of the private data to allocate for each matching device.
> - */
> -struct rte_eventdev_driver {
> -	struct rte_pci_driver pci_drv;	/**< The PMD is also a PCI driver. */
> -	unsigned int dev_private_size;	/**< Size of device private data. */
> -
> -	eventdev_init_t eventdev_init;	/**< Device init function. */
> -	eventdev_uninit_t eventdev_uninit; /**< Device uninit function. */
> -};
> -
>  /** Global structure used for maintaining state of allocated event devices */
>  struct rte_eventdev_global {
>  	uint8_t nb_devs;	/**< Number of devices found */
> @@ -579,18 +525,23 @@ rte_event_pmd_vdev_init(const char *name, size_t dev_private_size,
>  int
>  rte_event_pmd_vdev_uninit(const char *name);
>  
> +typedef int (*eventdev_pmd_pci_callback_t)(struct rte_eventdev *dev);
> +
>  /**
>   * Wrapper for use by pci drivers as a .probe function to attach to a event
>   * interface.
>   */
>  int rte_event_pmd_pci_probe(struct rte_pci_driver *pci_drv,
> -			    struct rte_pci_device *pci_dev);
> +			    struct rte_pci_device *pci_dev,
> +			    size_t private_data_size,
> +			    eventdev_pmd_pci_callback_t devinit);
>  
>  /**
>   * Wrapper for use by pci drivers as a .remove function to detach a event
>   * interface.
>   */
> -int rte_event_pmd_pci_remove(struct rte_pci_device *pci_dev);
> +int rte_event_pmd_pci_remove(struct rte_pci_device *pci_dev,
> +			     eventdev_pmd_pci_callback_t devuninit);
>  
>  #ifdef __cplusplus
>  }
> -- 
> 2.13.0
> 

-- 
Gaëtan Rivet
6WIND