[v3] net/e1000: fix link status update

  Unassigned variable should not be used as judgment, and there
is no need to update link status according to old link status.
This patch fix the issue.

Change the variable from link_check to link_up.

Fixes: 80ba61115e77 ("net/e1000: use link status helper functions")
Cc: stable@dpdk.org

Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
---
v3:
* Change the variable from link_check to link_up.

v2
* Delete incorrect judgment
---
 drivers/net/e1000/em_ethdev.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
  

Hi, wenzhuo

Could you help ack this patch if you are ok with this change?

Thanks,
Xiaolong

On 11/18, Lunyuan Cui wrote:
>Unassigned variable should not be used as judgment, and there
>is no need to update link status according to old link status.
>This patch fix the issue.
>
>Change the variable from link_check to link_up.
>
>Fixes: 80ba61115e77 ("net/e1000: use link status helper functions")
>Cc: stable@dpdk.org
>
>Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
>---
>v3:
>* Change the variable from link_check to link_up.
>
>v2
>* Delete incorrect judgment
>---
> drivers/net/e1000/em_ethdev.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
>index 9a88b50b2..080cbe2df 100644
>--- a/drivers/net/e1000/em_ethdev.c
>+++ b/drivers/net/e1000/em_ethdev.c
>@@ -1121,9 +1121,9 @@ eth_em_link_update(struct rte_eth_dev *dev, int wait_to_complete)
> 	struct e1000_hw *hw =
> 		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> 	struct rte_eth_link link;
>-	int link_check, count;
>+	int link_up, count;
> 
>-	link_check = 0;
>+	link_up = 0;
> 	hw->mac.get_link_status = 1;
> 
> 	/* possible wait-to-complete in up to 9 seconds */
>@@ -1133,31 +1133,31 @@ eth_em_link_update(struct rte_eth_dev *dev, int wait_to_complete)
> 		case e1000_media_type_copper:
> 			/* Do the work to read phy */
> 			e1000_check_for_link(hw);
>-			link_check = !hw->mac.get_link_status;
>+			link_up = !hw->mac.get_link_status;
> 			break;
> 
> 		case e1000_media_type_fiber:
> 			e1000_check_for_link(hw);
>-			link_check = (E1000_READ_REG(hw, E1000_STATUS) &
>+			link_up = (E1000_READ_REG(hw, E1000_STATUS) &
> 					E1000_STATUS_LU);
> 			break;
> 
> 		case e1000_media_type_internal_serdes:
> 			e1000_check_for_link(hw);
>-			link_check = hw->mac.serdes_has_link;
>+			link_up = hw->mac.serdes_has_link;
> 			break;
> 
> 		default:
> 			break;
> 		}
>-		if (link_check || wait_to_complete == 0)
>+		if (link_up || wait_to_complete == 0)
> 			break;
> 		rte_delay_ms(EM_LINK_UPDATE_CHECK_INTERVAL);
> 	}
> 	memset(&link, 0, sizeof(link));
> 
> 	/* Now we check if a transition has happened */
>-	if (link_check && (link.link_status == ETH_LINK_DOWN)) {
>+	if (link_up) {
> 		uint16_t duplex, speed;
> 		hw->mac.ops.get_link_up_info(hw, &speed, &duplex);
> 		link.link_duplex = (duplex == FULL_DUPLEX) ?
>@@ -1167,7 +1167,7 @@ eth_em_link_update(struct rte_eth_dev *dev, int wait_to_complete)
> 		link.link_status = ETH_LINK_UP;
> 		link.link_autoneg = !(dev->data->dev_conf.link_speeds &
> 				ETH_LINK_SPEED_FIXED);
>-	} else if (!link_check && (link.link_status == ETH_LINK_UP)) {
>+	} else {
> 		link.link_speed = ETH_SPEED_NUM_NONE;
> 		link.link_duplex = ETH_LINK_HALF_DUPLEX;
> 		link.link_status = ETH_LINK_DOWN;
>-- 
>2.17.1
>
  

Hi Lunyuan,


> -----Original Message-----
> From: Cui, LunyuanX
> Sent: Monday, November 18, 2019 10:58 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Cui, LunyuanX
> <lunyuanx.cui@intel.com>; stable@dpdk.org
> Subject: [PATCH v3] net/e1000: fix link status update
> 
> Unassigned variable should not be used as judgment, and there is no need
Don't understand "Unassigned variable should not be used as judgment ". Which one is this " Unassigned variable"?
> to update link status according to old link status.
Don't understand why " no need to update link status according to old link status ".
Please add more info about what problem this patch wants to fix. Thanks.

> This patch fix the issue.
> 
> Change the variable from link_check to link_up.
> 
> Fixes: 80ba61115e77 ("net/e1000: use link status helper functions")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
> ---
> v3:
> * Change the variable from link_check to link_up.
> 
> v2
> * Delete incorrect judgment
> ---
>  drivers/net/e1000/em_ethdev.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/e1000/em_ethdev.c
> b/drivers/net/e1000/em_ethdev.c index 9a88b50b2..080cbe2df 100644
> --- a/drivers/net/e1000/em_ethdev.c
> +++ b/drivers/net/e1000/em_ethdev.c


> 
>  	/* Now we check if a transition has happened */
Looks like this comment need to be removed if the below check changes. But still, don't know why we should change the check.
> -	if (link_check && (link.link_status == ETH_LINK_DOWN)) {
> +	if (link_up) {