diff mbox

[dpdk-dev,v3,12/15] ether: extract function eth_dev_get_intr_handle

Message ID 1473410639-10367-13-git-send-email-shreyansh.jain@nxp.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Shreyansh Jain Sept. 9, 2016, 8:43 a.m. UTC 
  We abstract access to the intr_handle here as we want to get
it either from the pci_dev or soc_dev.

Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 lib/librte_ether/rte_ethdev.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

Hunt, David Sept. 15, 2016, 1:02 p.m. UTC | #1 
On 9/9/2016 9:43 AM, Shreyansh Jain wrote:
> We abstract access to the intr_handle here as we want to get
> it either from the pci_dev or soc_dev.
>
> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
>   lib/librte_ether/rte_ethdev.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index e9f5467..104ea4a 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -2526,6 +2526,17 @@ _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
>   	rte_spinlock_unlock(&rte_eth_dev_cb_lock);
>   }
>   
> +static inline
> +struct rte_intr_handle *eth_dev_get_intr_handle(struct rte_eth_dev *dev)
> +{
> +	if (dev->pci_dev) {
> +		return &dev->pci_dev->intr_handle;
> +	}
> +
> +	RTE_VERIFY(0);

Rather than RTE_VERIFY(0), might I suggest using rte_panic with a more 
relevant error message?


> +	return NULL;
> +}
> +
>   int
>   rte_eth_dev_rx_intr_ctl(uint8_t port_id, int epfd, int op, void *data)
>   {
> @@ -2538,7 +2549,7 @@ 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;
> +	intr_handle = eth_dev_get_intr_handle(dev);
>   	if (!intr_handle->intr_vec) {
>   		RTE_PMD_DEBUG_TRACE("RX Intr vector unset\n");
>   		return -EPERM;
> @@ -2598,7 +2609,7 @@ rte_eth_dev_rx_intr_ctl_q(uint8_t port_id, uint16_t queue_id,
>   		return -EINVAL;
>   	}
>   
> -	intr_handle = &dev->pci_dev->intr_handle;
> +	intr_handle = eth_dev_get_intr_handle(dev);
>   	if (!intr_handle->intr_vec) {
>   		RTE_PMD_DEBUG_TRACE("RX Intr vector unset\n");
>   		return -EPERM;
Thomas Monjalon Sept. 15, 2016, 2:05 p.m. UTC | #2 
2016-09-15 14:02, Hunt, David:
> On 9/9/2016 9:43 AM, Shreyansh Jain wrote:
> > +static inline
> > +struct rte_intr_handle *eth_dev_get_intr_handle(struct rte_eth_dev *dev)
> > +{
> > +	if (dev->pci_dev) {
> > +		return &dev->pci_dev->intr_handle;
> > +	}
> > +
> > +	RTE_VERIFY(0);
> 
> Rather than RTE_VERIFY(0), might I suggest using rte_panic with a more 
> relevant error message?

RTE_ASSERT is preferred.
We must stop adding some rte_panic calls except for debug.
Panu Matilainen Sept. 16, 2016, 7:17 a.m. UTC | #3 
On 09/15/2016 05:05 PM, Thomas Monjalon wrote:
> 2016-09-15 14:02, Hunt, David:
>> On 9/9/2016 9:43 AM, Shreyansh Jain wrote:
>>> +static inline
>>> +struct rte_intr_handle *eth_dev_get_intr_handle(struct rte_eth_dev *dev)
>>> +{
>>> +	if (dev->pci_dev) {
>>> +		return &dev->pci_dev->intr_handle;
>>> +	}
>>> +
>>> +	RTE_VERIFY(0);
>>
>> Rather than RTE_VERIFY(0), might I suggest using rte_panic with a more
>> relevant error message?
>
> RTE_ASSERT is preferred.
> We must stop adding some rte_panic calls except for debug.

+1

It wouldn't hurt to make that a hard rule.

	- Panu -
diff mbox

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index e9f5467..104ea4a 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -2526,6 +2526,17 @@  _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
 	rte_spinlock_unlock(&rte_eth_dev_cb_lock);
 }
 
+static inline
+struct rte_intr_handle *eth_dev_get_intr_handle(struct rte_eth_dev *dev)
+{
+	if (dev->pci_dev) {
+		return &dev->pci_dev->intr_handle;
+	}
+
+	RTE_VERIFY(0);
+	return NULL;
+}
+
 int
 rte_eth_dev_rx_intr_ctl(uint8_t port_id, int epfd, int op, void *data)
 {
@@ -2538,7 +2549,7 @@  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;
+	intr_handle = eth_dev_get_intr_handle(dev);
 	if (!intr_handle->intr_vec) {
 		RTE_PMD_DEBUG_TRACE("RX Intr vector unset\n");
 		return -EPERM;
@@ -2598,7 +2609,7 @@  rte_eth_dev_rx_intr_ctl_q(uint8_t port_id, uint16_t queue_id,
 		return -EINVAL;
 	}
 
-	intr_handle = &dev->pci_dev->intr_handle;
+	intr_handle = eth_dev_get_intr_handle(dev);
 	if (!intr_handle->intr_vec) {
 		RTE_PMD_DEBUG_TRACE("RX Intr vector unset\n");
 		return -EPERM;