diff mbox

[dpdk-dev,v4,22/23] ethdev: Decouple interrupt handling from PCI device

Message ID 1482332986-7599-23-git-send-email-jblunck@infradead.org (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

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

Commit Message

Jan Blunck Dec. 21, 2016, 3:09 p.m. UTC 
  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>
Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 lib/librte_ether/rte_ethdev.c | 18 ++++++++++++++++--
 lib/librte_ether/rte_ethdev.h |  1 +
 2 files changed, 17 insertions(+), 2 deletions(-)

Comments

Thomas Monjalon Dec. 22, 2016, 3:13 p.m. UTC | #1 
2016-12-21 16:09, Jan Blunck:
> 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.
[...]
> --- 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;
>  	/**

Why adding interrupt in ethdev?
Shouldn't it be a property of rte_device?
Jan Blunck Dec. 22, 2016, 6:26 p.m. UTC | #2 
On Thu, Dec 22, 2016 at 4:13 PM, Thomas Monjalon
<thomas.monjalon@6wind.com> wrote:
> 2016-12-21 16:09, Jan Blunck:
>> 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.
> [...]
>> --- 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;
>>       /**
>
> Why adding interrupt in ethdev?
> Shouldn't it be a property of rte_device?

Good question. I don't think that rte_device should become an
abstraction layer for all kind of real/virtual devices functionality.
There are devices that do not support interrupts and this
functionality is around enabling irqs per queue. Since this is a
concept of the ethdev I believe the right place to keep a reference to
the IRQ implementation is the rte_eth_dev.
diff mbox

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 200e068..764b6cd 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;
 
@@ -2555,7 +2556,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;
@@ -2615,7 +2622,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;
@@ -3217,6 +3229,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 d6e367c..cc93ca9 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;
 	/**