On Tue, Jun 06, 2017 at 08:35:48AM +0530, Jerin Jacob wrote:
> -----Original Message-----
> > Date: Mon, 5 Jun 2017 14:55:55 +0200
> > From: Gaëtan Rivet <gaetan.ri...@6wind.com>
> > To: Jerin Jacob <jerin.ja...@caviumnetworks.com>
> > Cc: dev@dpdk.org, bruce.richard...@intel.com, harry.van.haa...@intel.com,
> > hemant.agra...@nxp.com, gage.e...@intel.com, nipun.gu...@nxp.com
> > Subject: Re: [dpdk-dev] [PATCH] eventdev: remove PCI dependency
> > User-Agent: Mutt/1.5.23 (2014-03-12)
> >
> > Hi Jerin,
>
> Hi Gaëtan,
>
> >
> > 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?
>
> That is for PCI PMD specific probe(rte_event_pmd_pci_probe() and
> rte_event_pmd_pci_remove()),
> More like, lib/librte_ether/rte_ethdev_pci.h functions in ethdev.
> So, I think, It is OK to keep rte_pci.h for PMD specific functions.
>
>
Ok, sure. However rte_eventdev.c includes both rte_pci.h and
rte_eventdev_pmd.h. Can it be made independent from the PMD specific
include?
> >
> > > CC: Gaetan Rivet <gaetan.ri...@6wind.com>
> > > Signed-off-by: Jerin Jacob <jerin.ja...@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
--
Gaëtan Rivet
6WIND
