e1000 serdes link flap

[Neel Natu]

Hi Jack,

On Wed, Jan 23, 2013 at 2:58 PM, Ryan Stone <rysto32 at gmail.com> wrote:
> On Wed, Jan 23, 2013 at 2:13 AM, Neel Natu <neelnatu at gmail.com> wrote:
>>
>> Hi,
>>
>> I am running into a problem in head with the e1000 link state
>> detection logic attached to a 82571EB serdes controller.
>>
>> The symptom is that the link state keeps flapping between "up" and "down".
>>
>> After I enabled the debug output in
>> 'e1000_check_for_serdes_link_82571()' this is what I see:
>> <snip>
>> e1000_check_for_serdes_link_82571
>> ctrl = 0x4c0241, status = 0x803a7, rxcw = 0x44000000
>> FORCED_UP -> AN_PROG
>> em6: link state changed to DOWN
>> e1000_check_for_serdes_link_82571
>> ctrl = 0x4c0201, status = 0x803a4, rxcw = 0x44000000
>> AN_PROG   -> FORCED_UP
>> em6: link state changed to UP
>> e1000_check_for_serdes_link_82571
>> ctrl = 0x4c0241, status = 0x803a7, rxcw = 0x44000000
>> FORCED_UP -> AN_PROG
>> em6: link state changed to DOWN
>> </snip>
>>
>> The problem goes away if I apply the following patch to bring the link
>> state detection logic in line with the e1000e driver in Linux:
>>
>> Index: e1000_82571.c
>> ===================================================================
>> --- e1000_82571.c       (revision 245766)
>> +++ e1000_82571.c       (working copy)
>> @@ -1712,10 +1712,8 @@
>>                          * auto-negotiation in the TXCW register and
>> disable
>>                          * forced link in the Device Control register in
>> an
>>                          * attempt to auto-negotiate with our link
>> partner.
>> -                        * If the partner code word is null, stop forcing
>> -                        * and restart auto negotiation.
>>                          */
>> -                       if ((rxcw & E1000_RXCW_C) || !(rxcw &
>> E1000_RXCW_CW))  {
>> +                       if ((rxcw & E1000_RXCW_C) != 0) {
>>                                 /* Enable autoneg, and unforce link up */
>>                                 E1000_WRITE_REG(hw, E1000_TXCW,
>> mac->txcw);
>>                                 E1000_WRITE_REG(hw, E1000_CTRL,
>>
>> I am not sure why the !(rxcw & E1000_RXCW_CW) check was added and the
>> e1000 SDM does not have any more information.
>>
>> Jack, can you take a look at the patch and commit if it looks alright?
>
>
> I have this change applied internally.  I thought that it was something
> funny in my environment, so I never tried to push it upstream.  Sorry for
> costing you time in having to debug this. :(

Are you planning to commit this patch?

I am happy to get you more information from my system if it helps you
get to the bottom of this quicker.

best
Neel