Hi Bruce,

-----Original Message-----
From: Richardson, Bruce
Sent: Thursday, February 25, 2016 9:30 PM
To: Lu, Wenzhuo
Cc: He, Shaopeng; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] ixgbe: fix link down issue on x550em_x

On Thu, Feb 25, 2016 at 01:32:33AM +0000, Lu, Wenzhuo wrote:

quoted

Hi Bruce,

quoted

-----Original Message-----
From: Richardson, Bruce
Sent: Wednesday, February 24, 2016 10:26 PM
To: He, Shaopeng
Cc: Lu, Wenzhuo; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] ixgbe: fix link down issue on
x550em_x

On Thu, Feb 04, 2016 at 06:21:04AM +0000, He, Shaopeng wrote:

quoted

quoted

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
Sent: Monday, February 01, 2016 4:43 PM
To: dev@dpdk.org
Subject: [dpdk-dev] [PATCH v2] ixgbe: fix link down issue on
x550em_x

Normally the auto-negotiation is supported by FW. But on
X550EM_X_10G_T it's not supported by FW. As the port of
X550EM_X_10G_T is 10G. If we connect the port with a peer which
is 1G. The link is always down.
We have to supprted auto-neg by SW to avoid such link down issue.

Signed-off-by: Wenzhuo Lu <redacted>

Acked-by: Shaopeng He <redacted>

I'm a bit confused regarding the commit message and the code in the patch.
The commit message talks about enabling speed auto-negotiation,
while the code never refers to any such thing. Instead all we have
are settings for manipulating interrupt masks to enable PHY
interrupts. I think some additional information is needed to connect A and B

together here.

quoted

The reason is that the handler of the link speed auto-neg is already in the base

code. It's ixgbe_handle_lasi.

quoted

What we need is a trigger. When the advertised link speed changes, a PHY

interruption will be triggered. So, we should handle this interruption and call
ixgbe_handle_lasi to set the link speed correct.

quoted

Let me add more explanation in the commit log.

Yes, please do. What you have just explained makes much more sense and
should be included in the log.

quoted

quoted

A second, more minor nit is that the commit title never refers to
link auto- negotiation, but refers to this as a bug fix - which is
also correct. If this is primarily a bug fix, please include a fixes
line if possible, but please also refer to auto-neg in the title if possible too.

I didn't add a fixes line because it doesn't fix an issue introduced by SW, or

even FW, HW. As said in commit log, we hit a link down issue, but the root cause
is the link speed auto-neg is not supported on this specific NIC.

quoted

On the other NICs, link speed auto-neg is supported by FW, SW need no do

anything. So we didn't implement the link speed auto-neg. But we have to
implement this feature on this NIC for FW doesn't implement it.

quoted

Should I change the tittle to "support link speed auto-neg on x550em_x"?

Yes, that is probably better.

Thanks for the comments. I'll send a new version.

Thanks,
/Bruce