Re: [dpdk-dev] [PATCH v2 8/8] ethdev: Decouple interrupt handling from PCI device

[Shreyansh Jain <shreyansh.jain@nxp.com>]

On Monday 21 November 2016 10:25 PM, Jan Blunck wrote:
> The struct rte_intr_handle is an abstraction layer for different types of
> interrupt mechanisms. It is embedded in the low-level device (e.g. PCI).
> On allocation of a struct rte_eth_dev a reference to the intr_handle
> should be stored for devices supporting interrupts.
>
> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> ---
>  lib/librte_ether/rte_ethdev.c | 18 ++++++++++++++++--
>  lib/librte_ether/rte_ethdev.h |  1 +
>  2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 4288577..4ecea50 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -258,6 +258,7 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
>  			rte_panic("Cannot allocate memzone for private port data\n");
>  	}
>  	eth_dev->pci_dev = pci_dev;
> +	eth_dev->intr_handle = &pci_dev->intr_handle;
>  	eth_dev->driver = eth_drv;
>  	eth_dev->data->rx_mbuf_alloc_failed = 0;
>
> @@ -2543,7 +2544,13 @@ rte_eth_dev_rx_intr_ctl(uint8_t port_id, int epfd, int op, void *data)
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>
>  	dev = &rte_eth_devices[port_id];
> -	intr_handle = &dev->pci_dev->intr_handle;
> +
> +	if (!dev->intr_handle) {
> +		RTE_PMD_DEBUG_TRACE("RX Intr handle unset\n");
> +		return -ENOTSUP;
> +	}
> +
> +	intr_handle = dev->intr_handle;
>  	if (!intr_handle->intr_vec) {
>  		RTE_PMD_DEBUG_TRACE("RX Intr vector unset\n");
>  		return -EPERM;
> @@ -2603,7 +2610,12 @@ rte_eth_dev_rx_intr_ctl_q(uint8_t port_id, uint16_t queue_id,
>  		return -EINVAL;
>  	}
>
> -	intr_handle = &dev->pci_dev->intr_handle;
> +	if (!dev->intr_handle) {
> +		RTE_PMD_DEBUG_TRACE("RX Intr handle unset\n");
> +		return -ENOTSUP;
> +	}
> +
> +	intr_handle = dev->intr_handle;
>  	if (!intr_handle->intr_vec) {
>  		RTE_PMD_DEBUG_TRACE("RX Intr vector unset\n");
>  		return -EPERM;
> @@ -3205,6 +3217,8 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev, struct rte_pci_device *pci_de
>  		return;
>  	}
>
> +	eth_dev->intr_handle = &pci_dev->intr_handle;
> +
>  	eth_dev->data->dev_flags = 0;
>  	if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
>  		eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 3adbb2b..f1f656a 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1629,6 +1629,7 @@ struct rte_eth_dev {
>  	const struct eth_driver *driver;/**< Driver for this device */
>  	const struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */
>  	struct rte_pci_device *pci_dev; /**< PCI info. supplied by probing */
> +	struct rte_intr_handle *intr_handle; /**< Device interrupt handle */
>  	/** User application callbacks for NIC interrupts */
>  	struct rte_eth_dev_cb_list link_intr_cbs;
>  	/**
>

Is there another patch which replaces all uses of 
eth_dev->pci_dev->intr_handle with that of eth_dev->intr_handle?

Now that eth_dev has a reference which is initialized as early as probe, 
we should use that rather than pci_dev->intr_handle for PCI PMDs.

OR maybe, ETH_DEV_INTR_HANDLE() like macro which you have introduced in 
i40e_ethdev.h.

-
Shreyansh