e1000 serdes link flap

[Neel Natu]

Hi Jack,

On Thu, Jan 31, 2013 at 12:03 PM, Jack Vogel <jfvogel at gmail.com> wrote:
> 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?

Sounds good.

best
Neel

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