[dpdk-dev,v4,05/15] net/dpaa2: use rte_eth_linkstatus_set
Checks
Commit Message
From: Stephen Hemminger <stephen@networkplumber.org>
Use new helper function to update the link status.
As a good side effect this fixes a but because this driver was not
returning correct status (should be -1 in link_status changed).
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
drivers/net/dpaa2/dpaa2_ethdev.c | 65 +++++-----------------------------------
1 file changed, 7 insertions(+), 58 deletions(-)
Comments
On Thursday 11 January 2018 10:36 PM, Ferruh Yigit wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
>
> Use new helper function to update the link status.
> As a good side effect this fixes a but because this driver was not
^^^^^^
needs rephrasing
> returning correct status (should be -1 in link_status changed).
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> drivers/net/dpaa2/dpaa2_ethdev.c | 65 +++++-----------------------------------
> 1 file changed, 7 insertions(+), 58 deletions(-)
Other than the change in commit message highlighted above:
Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
Thanks.
On Tuesday 16 January 2018 03:14 PM, Shreyansh Jain wrote:
> On Thursday 11 January 2018 10:36 PM, Ferruh Yigit wrote:
>> From: Stephen Hemminger <stephen@networkplumber.org>
>>
>> Use new helper function to update the link status.
>> As a good side effect this fixes a but because this driver was not
> ^^^^^^
> needs rephrasing
>
>> returning correct status (should be -1 in link_status changed).
>>
>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>> ---
>> drivers/net/dpaa2/dpaa2_ethdev.c | 65
>> +++++-----------------------------------
>> 1 file changed, 7 insertions(+), 58 deletions(-)
>
> Other than the change in commit message highlighted above:
>
> Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>
> Thanks.
>
Also, I forgot to add in previous email, I am assuming
_rte_eth_linkstatus_get would be rte_eth_linkstatus_get eventually.
I concur with your comments in [1] - it certainly would be better to
have either @internal or _rte - and not both.
http://dpdk.org/ml/archives/dev/2018-January/086742.html
@@ -57,58 +57,6 @@ static int dpaa2_dev_set_link_up(struct rte_eth_dev *dev);
static int dpaa2_dev_set_link_down(struct rte_eth_dev *dev);
static int dpaa2_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
-/**
- * Atomically reads the link status information from global
- * structure rte_eth_dev.
- *
- * @param dev
- * - Pointer to the structure rte_eth_dev to read from.
- * - Pointer to the buffer to be saved with the link status.
- *
- * @return
- * - On success, zero.
- * - On failure, negative value.
- */
-static inline int
-dpaa2_dev_atomic_read_link_status(struct rte_eth_dev *dev,
- struct rte_eth_link *link)
-{
- struct rte_eth_link *dst = link;
- struct rte_eth_link *src = &dev->data->dev_link;
-
- if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
- *(uint64_t *)src) == 0)
- return -1;
-
- return 0;
-}
-
-/**
- * Atomically writes the link status information into global
- * structure rte_eth_dev.
- *
- * @param dev
- * - Pointer to the structure rte_eth_dev to read from.
- * - Pointer to the buffer to be saved with the link status.
- *
- * @return
- * - On success, zero.
- * - On failure, negative value.
- */
-static inline int
-dpaa2_dev_atomic_write_link_status(struct rte_eth_dev *dev,
- struct rte_eth_link *link)
-{
- struct rte_eth_link *dst = &dev->data->dev_link;
- struct rte_eth_link *src = link;
-
- if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
- *(uint64_t *)src) == 0)
- return -1;
-
- return 0;
-}
-
static int
dpaa2_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
{
@@ -837,7 +785,7 @@ dpaa2_dev_stop(struct rte_eth_dev *dev)
/* clear the recorded link status */
memset(&link, 0, sizeof(link));
- dpaa2_dev_atomic_write_link_status(dev, &link);
+ _rte_eth_linkstatus_set(dev, &link);
}
static void
@@ -869,7 +817,7 @@ dpaa2_dev_close(struct rte_eth_dev *dev)
}
memset(&link, 0, sizeof(link));
- dpaa2_dev_atomic_write_link_status(dev, &link);
+ _rte_eth_linkstatus_set(dev, &link);
}
static void
@@ -1327,8 +1275,8 @@ dpaa2_dev_link_update(struct rte_eth_dev *dev,
RTE_LOG(ERR, PMD, "dpni is NULL\n");
return 0;
}
- memset(&old, 0, sizeof(old));
- dpaa2_dev_atomic_read_link_status(dev, &old);
+
+ _rte_eth_linkstatus_get(dev, &old);
ret = dpni_get_link_state(dpni, CMD_PRI_LOW, priv->token, &state);
if (ret < 0) {
@@ -1350,13 +1298,14 @@ dpaa2_dev_link_update(struct rte_eth_dev *dev,
else
link.link_duplex = ETH_LINK_FULL_DUPLEX;
- dpaa2_dev_atomic_write_link_status(dev, &link);
+ ret = _rte_eth_linkstatus_set(dev, &link);
if (link.link_status)
PMD_DRV_LOG(INFO, "Port %d Link is Up\n", dev->data->port_id);
else
PMD_DRV_LOG(INFO, "Port %d Link is Down", dev->data->port_id);
- return 0;
+
+ return ret;
}
/**