Re: [Qemu-devel] [PATCH 1/2] e1000: no need auto-negotiation if link was down

[Stefan Hajnoczi <stefanha@redhat.com>]

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 <jasowang@redhat.com>
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  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.

Stefan