From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:36132) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TrPNe-0006VS-CW for qemu-devel@nongnu.org; Sat, 05 Jan 2013 03:45:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TrPNd-0006ap-46 for qemu-devel@nongnu.org; Sat, 05 Jan 2013 03:45:26 -0500 Received: from mx1.redhat.com ([209.132.183.28]:17725) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TrPNc-0006aj-TH for qemu-devel@nongnu.org; Sat, 05 Jan 2013 03:45:25 -0500 Message-ID: <50E7E81A.8050203@redhat.com> Date: Sat, 05 Jan 2013 16:45:14 +0800 From: Jason Wang MIME-Version: 1.0 References: <1356686951-20305-1-git-send-email-akong@redhat.com> <1356686951-20305-2-git-send-email-akong@redhat.com> <20130103122033.GC6976@stefanha-thinkpad.muc.redhat.com> In-Reply-To: <20130103122033.GC6976@stefanha-thinkpad.muc.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] e1000: no need auto-negotiation if link was down List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: jan.kiszka@siemens.com, Amos Kong , qemu-devel@nongnu.org On 01/03/2013 08:20 PM, Stefan Hajnoczi wrote: > On Fri, Dec 28, 2012 at 05:29:10PM +0800, Amos Kong wrote: >> Commit b9d03e352cb6b31a66545763f6a1e20c9abf0c2c added link >> auto-negotiation emulation, it would always set link up by >> callback function. Problem exists if original link status >> was down, link status should not be changed in auto-negotiation. >> >> Signed-off-by: Jason Wang >> Signed-off-by: Amos Kong >> --- >> hw/e1000.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/hw/e1000.c b/hw/e1000.c >> index 92fb00a..eebcd1d 100644 >> --- a/hw/e1000.c >> +++ b/hw/e1000.c >> @@ -164,6 +164,11 @@ static void >> set_phy_ctrl(E1000State *s, int index, uint16_t val) >> { >> if ((val & MII_CR_AUTO_NEG_EN) && (val & MII_CR_RESTART_AUTO_NEG)) { >> + /* no need auto-negotiation if link was down */ >> + if (s->nic->nc.link_down) { >> + s->phy_reg[PHY_STATUS] |= MII_SR_AUTONEG_COMPLETE; >> + return; >> + } >> s->nic->nc.link_down = true; >> e1000_link_down(s); >> s->phy_reg[PHY_STATUS] &= ~MII_SR_AUTONEG_COMPLETE; > Do we need set_ics(s, 0, E1000_ICR_LSC) when autonegotiation completes? > The code doesn't but I wonder if we should. Not in this case I think. The hack of the auto-negotiation was used to prevent the irq to be injected before the handler is registered in windows guest. So an irq would be raised here if we do this which breaks the hack. > > Stefan