From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [RFC 02/14] virtio: use eth_link_read/write (and bug fix) Date: Mon, 17 Jul 2017 09:01:22 -0700 Message-ID: <20170717090122.18655a27@xeon-e3> References: <20170714183027.16021-1-stephen@networkplumber.org> <20170714183027.16021-3-stephen@networkplumber.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: , Stephen Hemminger , Thomas Monjalon To: Andrew Rybchenko Return-path: Received: from mail-pf0-f175.google.com (mail-pf0-f175.google.com [209.85.192.175]) by dpdk.org (Postfix) with ESMTP id 3B9AE47CD for ; Mon, 17 Jul 2017 18:01:25 +0200 (CEST) Received: by mail-pf0-f175.google.com with SMTP id q85so78510154pfq.1 for ; Mon, 17 Jul 2017 09:01:25 -0700 (PDT) In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Sun, 16 Jul 2017 15:33:26 +0300 Andrew Rybchenko wrote: > > + link.link_autoneg = ETH_LINK_SPEED_FIXED; > > As I understand link_autoneg is 1 bit field with boolean semantics. I.e. 0/false - no autoneg, 1/true - autoneg. > It looks like it has wrong comment: > uint16_t link_autoneg : 1; /**< ETH_LINK_SPEED_[AUTONEG/FIXED] */ > > since > #define ETH_LINK_SPEED_AUTONEG (0 << 0) /**< Autonegotiate (all speeds) */ > #define ETH_LINK_SPEED_FIXED (1 << 0) /**< Disable autoneg (fixed speed) */ > > whereas > #define ETH_LINK_FIXED 0 /**< No autonegotiation. */ > #define ETH_LINK_AUTONEG 1 /**< Autonegotiated. */ > > In general this attempt to introduce bug is the result of wrong comment which is caused by very similar > defines with opposite values. Orignal observation was because some drivers (vmxnet3) were setting autoneg = fixed and others were not. Turns out it makes no difference since FIXED == 0, the old code and new code have same effect.