From owner-freebsd-net@FreeBSD.ORG Thu Jan 31 22:29:24 2013 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id D7612662; Thu, 31 Jan 2013 22:29:24 +0000 (UTC) (envelope-from neelnatu@gmail.com) Received: from mail-ve0-f171.google.com (mail-ve0-f171.google.com [209.85.128.171]) by mx1.freebsd.org (Postfix) with ESMTP id 87ACA84C; Thu, 31 Jan 2013 22:29:24 +0000 (UTC) Received: by mail-ve0-f171.google.com with SMTP id b10so2494589vea.30 for ; Thu, 31 Jan 2013 14:29:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type; bh=765TEOAGEJQFtLx9tjI9ta3oZNDCPupJZo/zu7Mszqs=; b=smDAufsciRO1rWgaegn+qk34rB/j7zQ0L8VAZfDx7lAJkyWKH83yQ/IUGcr1xWJoCO rGiSWDvV0HNlCR66N5ZQh8Un54mKpDpGKWqIFn7qsXcFMAHQ+SQmq2hZUc9l07TwwlXg n2Kg90bmrd6JKI6X/ET9/iK120Y8j2p5PiGqvggvORFQYw3qcSjAPwnPgQPmAa2/njxu hFTbodMUNQGQ0Fbfa3HovnNMKLOW93MQwCYxHU8ZMjt0vFa5l8cJ5TWozlvvskw5hwf7 4ecXcO7Zkd2LtuvozyXSc+1PhLRlWslMFyJQgUP/PZn03kVa8rS3tviUFdEi+jjJnJIE agGg== MIME-Version: 1.0 X-Received: by 10.52.27.50 with SMTP id q18mr8543177vdg.20.1359671358461; Thu, 31 Jan 2013 14:29:18 -0800 (PST) Received: by 10.220.21.141 with HTTP; Thu, 31 Jan 2013 14:29:18 -0800 (PST) In-Reply-To: References: Date: Thu, 31 Jan 2013 14:29:18 -0800 Message-ID: Subject: Re: e1000 serdes link flap From: Neel Natu To: Jack Vogel Content-Type: text/plain; charset=ISO-8859-1 Cc: Jack F Vogel , freebsd-net , Ryan Stone X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 31 Jan 2013 22:29:24 -0000 Hi Jack, On Thu, Jan 31, 2013 at 12:03 PM, Jack Vogel 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 wrote: >> >> Hi Jack, >> >> On Wed, Jan 23, 2013 at 2:58 PM, Ryan Stone wrote: >> > On Wed, Jan 23, 2013 at 2:13 AM, Neel Natu 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: >> >> >> >> 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 >> >> >> >> >> >> 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 > >