[3/3] ixgbe: synchronize the link_speed and link_up of a slave interface

From: Zhu Yanjun <yanjun.zhu@windriver.com>

According to the suggestion from Rustad, Mark D, this behavior perhaps
is more related to the copper phy. But to make fiber phy more robust,
to all the interfaces as a slave interface, the link_speed and link_up
is synchronized.

Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

zyjzyj2000@gmail.com wrote:

> From: Zhu Yanjun <yanjun.zhu@windriver.com>
> 
> According to the suggestion from Rustad, Mark D, this behavior perhaps
> is more related to the copper phy. But to make fiber phy more robust,
> to all the interfaces as a slave interface, the link_speed and link_up
> is synchronized.
> 
> Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 1bb6056..ce47639 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -6441,10 +6441,12 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
> 	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
> 	 * independent interface, it is not necessary to synchronize link_up
> 	 * and link_speed.
> -	 * In the end, not continue if (X540 NIC && SLAVE && link_speed UNKNOWN)
> +	 * According to the suggestion from Rustad, Mark D, this behavior
> +	 * perhaps is related to the copper phy. To make fiber phy more robust,
> +	 * To all the interfaces as a slave, the link_speed is checked.
> +	 * In the end, not continue if (SLAVE && link_speed UNKNOWN)

There is no need to make reference to my suggestion in the comment, especially since that is in the commit message. Please simplify your comment above to be something like:
	 * For all slave interfaces, wait for the link_speed to be known.

> 	 */
> -	if ((hw->mac.type == ixgbe_mac_X540) &&
> -	    (netdev->flags & IFF_SLAVE))
> +	if (netdev->flags & IFF_SLAVE)
> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
> 			return;

The above would be better as:
	if ((netdev->flags & IFF_SLAVE) &&
	    link_speed == IXGBE_LINK_SPEED_UNKNOWN)
		return;

Please do not send a series of patches - it just adds needless confusion and is a bisect hazard. Just send a single patch with the desired change as a V5.

--
Mark Rustad, Networking Division, Intel Corporation

Hi, all

Thanks for the suggestions from Rustad, Mark D.
According to his suggestions, the logs and source code are simplified.

Zhu Yanjun
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Thu, 2015-12-31 at 13:04 +0800, zyjzyj2000@gmail.com wrote:
> Thanks for the suggestions from Rustad, Mark D.
> According to his suggestions, the logs and source code are
> simplified.

I find it funny that this email (no patch) is got the correct subject,
yet the updated patch you sent does not.  The subject line for this
email should have been used for the updated patch you sent out.

On 12/31/2015 01:17 PM, Jeff Kirsher wrote:
> On Thu, 2015-12-31 at 13:04 +0800, zyjzyj2000@gmail.com wrote:
>> Thanks for the suggestions from Rustad, Mark D.
>> According to his suggestions, the logs and source code are
>> simplified.
> I find it funny that this email (no patch) is got the correct subject,
> yet the updated patch you sent does not.  The subject line for this
> email should have been used for the updated patch you sent out.
Thanks for your reply.
I will resend the patch soon.

Best Regards!
Zhu Yanjun
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html