e1000 serdes link flap

[Ryan Stone]

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. :(