e1000 serdes link flap

[Jack Vogel]

I will not commit this patch, this is shared code used by all the different
OS's we do
drivers for, and it is unnecessary in any case, because the latest version
of the internal
code has this resolved....  So, I will soon do some updates to the e1000
drivers anyway
and as part of that I will sync the shared code base with the latest, this
should address
the issue... OK?

Cheers,

Jack


On Mon, Jan 28, 2013 at 8:14 PM, Neel Natu <neelnatu at gmail.com> wrote:

> 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
>