diff mbox series

ethtool: fix SPEED_UNKNOWN definition to avoid signed-unsigned comparison

Message ID 1554138868-7220-1-git-send-email-mzhivich@akamai.com
State Changes Requested
Delegated to: David Miller
Headers show
Series ethtool: fix SPEED_UNKNOWN definition to avoid signed-unsigned comparison | expand

Commit Message

Michael Zhivich April 1, 2019, 5:14 p.m. UTC 
When building C++ userspace code that includes ethtool.h
with "-Werror -Wall", g++ complains about signed-unsigned comparison in
ethtool_validate_speed() due to definition of SPEED_UNKNOWN as -1.

Change definition of SPEED_UNKNOWN to UINT_MAX to match the type of
ethtool_validate_speed() argument (__u32).

Update storage type for link speed in drivers to use u32 instead of int
or u16 to bring drivers up-to-date with ethtool.h which includes SPEED_*
constants larger than range of u16.

Fix usage of SPEED_* constants in
drivers/net/ethernet/cavium/thunder/thunder_bgx.c and
drivers/infiniband/ulp/ipoib/ipoib_ethtool.c.

Signed-off-by: Michael Zhivich <mzhivich@akamai.com>
---
 drivers/infiniband/ulp/ipoib/ipoib_ethtool.c      |  7 ++++---
 drivers/net/dsa/mv88e6xxx/chip.c                  |  5 +++--
 drivers/net/dsa/mv88e6xxx/serdes.c                |  2 +-
 drivers/net/ethernet/alacritech/slic.h            |  2 +-
 drivers/net/ethernet/alacritech/slicoss.c         |  6 +++---
 drivers/net/ethernet/amd/xgbe/xgbe-mdio.c         |  2 +-
 drivers/net/ethernet/amd/xgbe/xgbe-phy-v1.c       |  2 +-
 drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c       |  4 ++--
 drivers/net/ethernet/amd/xgbe/xgbe.h              |  6 +++---
 drivers/net/ethernet/apm/xgene-v2/main.h          |  2 +-
 drivers/net/ethernet/apm/xgene/xgene_enet_main.h  |  2 +-
 drivers/net/ethernet/atheros/alx/hw.h             |  2 +-
 drivers/net/ethernet/atheros/alx/main.c           |  2 +-
 drivers/net/ethernet/broadcom/bgmac.h             |  2 +-
 drivers/net/ethernet/broadcom/tg3.c               |  8 ++++----
 drivers/net/ethernet/broadcom/tg3.h               |  4 ++--
 drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 16 ++++++++--------
 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h       |  2 +-
 drivers/net/ethernet/samsung/sxgbe/sxgbe_common.h |  2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac.h      |  2 +-
 drivers/net/phy/phy-core.c                        |  2 +-
 include/linux/phy.h                               |  4 ++--
 include/linux/phylink.h                           |  2 +-
 include/uapi/linux/ethtool.h                      |  2 +-
 24 files changed, 46 insertions(+), 44 deletions(-)

Comments

David Miller April 2, 2019, 8:26 p.m. UTC | #1 
From: Michael Zhivich <mzhivich@akamai.com>
Date: Mon, 1 Apr 2019 13:14:28 -0400

> When building C++ userspace code that includes ethtool.h
> with "-Werror -Wall", g++ complains about signed-unsigned comparison in
> ethtool_validate_speed() due to definition of SPEED_UNKNOWN as -1.
> 
> Change definition of SPEED_UNKNOWN to UINT_MAX to match the type of
> ethtool_validate_speed() argument (__u32).
> 
> Update storage type for link speed in drivers to use u32 instead of int
> or u16 to bring drivers up-to-date with ethtool.h which includes SPEED_*
> constants larger than range of u16.
> 
> Fix usage of SPEED_* constants in
> drivers/net/ethernet/cavium/thunder/thunder_bgx.c and
> drivers/infiniband/ulp/ipoib/ipoib_ethtool.c.
> 
> Signed-off-by: Michael Zhivich <mzhivich@akamai.com>

Waaaaay too many changes in one patch.  Do one thing at a time.

Also, are you absolutely sure that the things you changed from 'int'
aren't used in other ways where the signedness matters?  For example
for error returns?

And maybe the drivers using u16 are legitimate because they are
operating on speeds only within that bitmap range.  Type matching is
not an end to itself.  So I don't want to see u16 --> u32 conversions
where they aren't even necessary.

Also:

>  /* Return lane speed in unit of 1e6 bit/sec */
> -static inline int ib_speed_enum_to_int(int speed)
> +static inline u32 ib_speed_enum_to_int(int speed)

This is sloppy.

You are changing the return type, but not adjusting the name of the
function appropriately.  The function name says it converts to
an 'int' but now it actually converts to and returns a u32.

You need to split up, audit, and present these changes more carefully
and properly.

Thank you.
Michael Zhivich April 3, 2019, 5:40 p.m. UTC | #2 
On 4/2/19, 4:26 PM, "David Miller" <davem@davemloft.net> wrote:

> From: Michael Zhivich <mzhivich@akamai.com>
> Date: Mon, 1 Apr 2019 13:14:28 -0400
> 
>> When building C++ userspace code that includes ethtool.h
>> with "-Werror -Wall", g++ complains about signed-unsigned comparison in
>> ethtool_validate_speed() due to definition of SPEED_UNKNOWN as -1.
>> 
>> Change definition of SPEED_UNKNOWN to UINT_MAX to match the type of
>> ethtool_validate_speed() argument (__u32).
>> 
>> Update storage type for link speed in drivers to use u32 instead of int
>> or u16 to bring drivers up-to-date with ethtool.h which includes SPEED_*
>> constants larger than range of u16.
>> 
>> Fix usage of SPEED_* constants in
>> drivers/net/ethernet/cavium/thunder/thunder_bgx.c and
>> drivers/infiniband/ulp/ipoib/ipoib_ethtool.c.
>> 
>> Signed-off-by: Michael Zhivich <mzhivich@akamai.com>
>
>Waaaaay too many changes in one patch.  Do one thing at a time.

OK - I will refactor into a patch series.

>
>Also, are you absolutely sure that the things you changed from 'int'
>aren't used in other ways where the signedness matters?  For example
>for error returns?
>

I will double-check, but I haven't observed that during my last review.

>And maybe the drivers using u16 are legitimate because they are
>operating on speeds only within that bitmap range.  Type matching is
>not an end to itself.  So I don't want to see u16 --> u32 conversions
>where they aren't even necessary.
>

In principle, I would agree; however, there's a problem with keeping 
u16 storage with the current definition of ethtool_validate_speed().

Using *current* definitions, if a u16 variable is assigned SPEED_UNKNOWN (-1),
and then passed to ethtool_validate_speed(), the check will succeed,
but for the wrong reasons:  

(__u32)((u16)-1) <= INT_MAX, rather than (__u32)((u16)-1) == -1.

With *proposed* change of SPEED_UNKNOWN to UINT_MAX, using u16 storage 
will result in -Woverflow warnings during compilation, 
hence the changes to drivers.

>Also:
>
>>  /* Return lane speed in unit of 1e6 bit/sec */
>> -static inline int ib_speed_enum_to_int(int speed)
>> +static inline u32 ib_speed_enum_to_int(int speed)
>
>This is sloppy.
>
>You are changing the return type, but not adjusting the name of the
>function appropriately.  The function name says it converts to
>an 'int' but now it actually converts to and returns a u32.
>

Fair enough, will fix.

>You need to split up, audit, and present these changes more carefully
>and properly.
>
>Thank you.

Thanks for the quick response.

~ Michael
diff mbox series

Patch

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
index 8342992..97c301a 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
@@ -157,7 +157,7 @@  static int ipoib_get_sset_count(struct net_device __always_unused *dev,
 }
 
 /* Return lane speed in unit of 1e6 bit/sec */
-static inline int ib_speed_enum_to_int(int speed)
+static inline u32 ib_speed_enum_to_int(int speed)
 {
 	switch (speed) {
 	case IB_SPEED_SDR:
@@ -181,7 +181,8 @@  static int ipoib_get_link_ksettings(struct net_device *netdev,
 {
 	struct ipoib_dev_priv *priv = ipoib_priv(netdev);
 	struct ib_port_attr attr;
-	int ret, speed, width;
+	int ret, width;
+	u32 speed;
 
 	if (!netif_carrier_ok(netdev)) {
 		cmd->base.speed = SPEED_UNKNOWN;
@@ -196,7 +197,7 @@  static int ipoib_get_link_ksettings(struct net_device *netdev,
 	speed = ib_speed_enum_to_int(attr.active_speed);
 	width = ib_width_enum_to_int(attr.active_width);
 
-	if (speed < 0 || width < 0)
+	if (speed == SPEED_UNKNOWN || width < 0)
 		return -EINVAL;
 
 	/* Except the following are set, the other members of
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index f4e2db4..de57fde 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -550,7 +550,7 @@  int mv88e6xxx_update(struct mv88e6xxx_chip *chip, int addr, int reg, u16 update)
 }
 
 int mv88e6xxx_port_setup_mac(struct mv88e6xxx_chip *chip, int port, int link,
-			     int speed, int duplex, int pause,
+			     u32 speed, int duplex, int pause,
 			     phy_interface_t mode)
 {
 	int err;
@@ -760,7 +760,8 @@  static void mv88e6xxx_mac_config(struct dsa_switch *ds, int port,
 				 const struct phylink_link_state *state)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
-	int speed, duplex, link, pause, err;
+	int duplex, link, pause, err;
+	u32 speed;
 
 	if ((mode == MLO_AN_PHY) && mv88e6xxx_phy_is_internal(ds, port))
 		return;
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 6a5de1b7..83b26ef 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -511,7 +511,7 @@  static void mv88e6390_serdes_irq_link_sgmii(struct mv88e6xxx_chip *chip,
 {
 	struct dsa_switch *ds = chip->ds;
 	int duplex = DUPLEX_UNKNOWN;
-	int speed = SPEED_UNKNOWN;
+	u32 speed = SPEED_UNKNOWN;
 	int link, err;
 	u16 status;
 
diff --git a/drivers/net/ethernet/alacritech/slic.h b/drivers/net/ethernet/alacritech/slic.h
index 3add305..b218982 100644
--- a/drivers/net/ethernet/alacritech/slic.h
+++ b/drivers/net/ethernet/alacritech/slic.h
@@ -550,7 +550,7 @@  struct slic_device {
 	/* link configuration lock */
 	spinlock_t link_lock;
 	bool promisc;
-	int speed;
+	u32 speed;
 	unsigned int duplex;
 	bool is_fiber;
 	unsigned char model;
diff --git a/drivers/net/ethernet/alacritech/slicoss.c b/drivers/net/ethernet/alacritech/slicoss.c
index 16477aa..4bdbf65 100644
--- a/drivers/net/ethernet/alacritech/slicoss.c
+++ b/drivers/net/ethernet/alacritech/slicoss.c
@@ -280,7 +280,7 @@  static void slic_configure_mac(struct slic_device *sdev)
 	slic_write(sdev, SLIC_REG_WMCFG, val);
 }
 
-static void slic_configure_link_locked(struct slic_device *sdev, int speed,
+static void slic_configure_link_locked(struct slic_device *sdev, u32 speed,
 				       unsigned int duplex)
 {
 	struct net_device *dev = sdev->netdev;
@@ -306,7 +306,7 @@  static void slic_configure_link_locked(struct slic_device *sdev, int speed,
 	}
 }
 
-static void slic_configure_link(struct slic_device *sdev, int speed,
+static void slic_configure_link(struct slic_device *sdev, u32 speed,
 				unsigned int duplex)
 {
 	spin_lock_bh(&sdev->link_lock);
@@ -639,7 +639,7 @@  static void slic_handle_link_irq(struct slic_device *sdev)
 	struct slic_shmem *sm = &sdev->shmem;
 	struct slic_shmem_data *sm_data = sm->shmem_data;
 	unsigned int duplex;
-	int speed;
+	u32 speed;
 	u32 link;
 
 	link = le32_to_cpu(sm_data->link);
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
index 8a3a60b..9f165af3 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
@@ -1522,7 +1522,7 @@  static void xgbe_dump_phy_registers(struct xgbe_prv_data *pdata)
 	dev_dbg(dev, "\n*************************************************\n");
 }
 
-static int xgbe_phy_best_advertised_speed(struct xgbe_prv_data *pdata)
+static u32 xgbe_phy_best_advertised_speed(struct xgbe_prv_data *pdata)
 {
 	struct ethtool_link_ksettings *lks = &pdata->phy.lks;
 
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v1.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v1.c
index d16eae4..ce7e90c 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v1.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v1.c
@@ -533,7 +533,7 @@  static enum xgbe_mode xgbe_phy_switch_mode(struct xgbe_prv_data *pdata)
 }
 
 static enum xgbe_mode xgbe_phy_get_mode(struct xgbe_prv_data *pdata,
-					int speed)
+					u32 speed)
 {
 	struct xgbe_phy_data *phy_data = pdata->phy_data;
 
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
index 128cd64..7651f49 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
@@ -2204,7 +2204,7 @@  static enum xgbe_mode xgbe_phy_get_baset_mode(struct xgbe_phy_data *phy_data,
 }
 
 static enum xgbe_mode xgbe_phy_get_sfp_mode(struct xgbe_phy_data *phy_data,
-					    int speed)
+					    u32 speed)
 {
 	switch (speed) {
 	case SPEED_100:
@@ -2245,7 +2245,7 @@  static enum xgbe_mode xgbe_phy_get_bp_mode(int speed)
 }
 
 static enum xgbe_mode xgbe_phy_get_mode(struct xgbe_prv_data *pdata,
-					int speed)
+					u32 speed)
 {
 	struct xgbe_phy_data *phy_data = pdata->phy_data;
 
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
index 47bcbcf..18f2ce6 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
@@ -617,7 +617,7 @@  struct xgbe_phy {
 	int address;
 
 	int autoneg;
-	int speed;
+	u32 speed;
 	int duplex;
 
 	int link;
@@ -863,7 +863,7 @@  struct xgbe_phy_impl_if {
 	/* Switch the PHY into various modes */
 	void (*set_mode)(struct xgbe_prv_data *, enum xgbe_mode);
 	/* Retrieve mode needed for a specific speed */
-	enum xgbe_mode (*get_mode)(struct xgbe_prv_data *, int);
+	enum xgbe_mode (*get_mode)(struct xgbe_prv_data *, u32);
 	/* Retrieve new/next mode when trying to auto-negotiate */
 	enum xgbe_mode (*switch_mode)(struct xgbe_prv_data *);
 	/* Retrieve current mode */
@@ -1234,7 +1234,7 @@  struct xgbe_prv_data {
 	/* Current PHY settings */
 	phy_interface_t phy_mode;
 	int phy_link;
-	int phy_speed;
+	u32 phy_speed;
 
 	/* MDIO/PHY related settings */
 	unsigned int phy_started;
diff --git a/drivers/net/ethernet/apm/xgene-v2/main.h b/drivers/net/ethernet/apm/xgene-v2/main.h
index 969b258..25d44db 100644
--- a/drivers/net/ethernet/apm/xgene-v2/main.h
+++ b/drivers/net/ethernet/apm/xgene-v2/main.h
@@ -70,7 +70,7 @@  struct xge_pdata {
 	struct net_device *ndev;
 	struct napi_struct napi;
 	struct xge_stats stats;
-	int phy_speed;
+	u32 phy_speed;
 	u8 nbufs;
 };
 
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
index 9857685..0ace0bc 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
@@ -196,7 +196,7 @@  struct xgene_cle_ops {
 struct xgene_enet_pdata {
 	struct net_device *ndev;
 	struct mii_bus *mdio_bus;
-	int phy_speed;
+	u32 phy_speed;
 	struct clk *clk;
 	struct platform_device *pdev;
 	enum xgene_enet_id enet_id;
diff --git a/drivers/net/ethernet/atheros/alx/hw.h b/drivers/net/ethernet/atheros/alx/hw.h
index e42d7e0..de6d241 100644
--- a/drivers/net/ethernet/atheros/alx/hw.h
+++ b/drivers/net/ethernet/atheros/alx/hw.h
@@ -480,7 +480,7 @@  struct alx_hw {
 
 	u32 smb_timer;
 	/* SPEED_* + DUPLEX_*, SPEED_UNKNOWN if link is down */
-	int link_speed;
+	u32 link_speed;
 	u8 duplex;
 
 	/* auto-neg advertisement or force mode config */
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index e3538ba..5295fd4 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -1277,7 +1277,7 @@  static void alx_check_link(struct alx_priv *alx)
 {
 	struct alx_hw *hw = &alx->hw;
 	unsigned long flags;
-	int old_speed;
+	u32 old_speed;
 	int err;
 
 	/* clear PHY internal interrupt status, otherwise the main
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 40d02fe..0c9cf98 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -512,7 +512,7 @@  struct bgmac {
 	u32 int_mask;
 
 	/* Current MAC state */
-	int mac_speed;
+	u32 mac_speed;
 	int mac_duplex;
 
 	u8 phyaddr;
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 328373e..060a6f3 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -4283,7 +4283,7 @@  static void tg3_power_down(struct tg3 *tp)
 	pci_set_power_state(tp->pdev, PCI_D3hot);
 }
 
-static void tg3_aux_stat_to_speed_duplex(struct tg3 *tp, u32 val, u16 *speed, u8 *duplex)
+static void tg3_aux_stat_to_speed_duplex(struct tg3 *tp, u32 val, u32 *speed, u8 *duplex)
 {
 	switch (val & MII_TG3_AUX_STAT_SPDMASK) {
 	case MII_TG3_AUX_STAT_10HALF:
@@ -4787,7 +4787,7 @@  static int tg3_setup_copper_phy(struct tg3 *tp, bool force_reset)
 	bool current_link_up;
 	u32 bmsr, val;
 	u32 lcl_adv, rmt_adv;
-	u16 current_speed;
+	u32 current_speed;
 	u8 current_duplex;
 	int i, err;
 
@@ -5719,7 +5719,7 @@  static bool tg3_setup_fiber_by_hand(struct tg3 *tp, u32 mac_status)
 static int tg3_setup_fiber_phy(struct tg3 *tp, bool force_reset)
 {
 	u32 orig_pause_cfg;
-	u16 orig_active_speed;
+	u32 orig_active_speed;
 	u8 orig_active_duplex;
 	u32 mac_status;
 	bool current_link_up;
@@ -5823,7 +5823,7 @@  static int tg3_setup_fiber_mii_phy(struct tg3 *tp, bool force_reset)
 {
 	int err = 0;
 	u32 bmsr, bmcr;
-	u16 current_speed = SPEED_UNKNOWN;
+	u32 current_speed = SPEED_UNKNOWN;
 	u8 current_duplex = DUPLEX_UNKNOWN;
 	bool current_link_up = false;
 	u32 local_adv, remote_adv, sgsr;
diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
index a772a33..6953d05 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -2873,7 +2873,7 @@  struct tg3_tx_ring_info {
 struct tg3_link_config {
 	/* Describes what we're trying to get. */
 	u32				advertising;
-	u16				speed;
+	u32				speed;
 	u8				duplex;
 	u8				autoneg;
 	u8				flowctrl;
@@ -2882,7 +2882,7 @@  struct tg3_link_config {
 	u8				active_flowctrl;
 
 	u8				active_duplex;
-	u16				active_speed;
+	u32				active_speed;
 	u32				rmt_adv;
 };
 
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
index 673c57b..81c281a 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
@@ -962,13 +962,13 @@  static void bgx_poll_for_sgmii_link(struct lmac *lmac)
 	lmac->last_duplex = (an_result >> 1) & 0x1;
 	switch (speed) {
 	case 0:
-		lmac->last_speed = 10;
+		lmac->last_speed = SPEED_10;
 		break;
 	case 1:
-		lmac->last_speed = 100;
+		lmac->last_speed = SPEED_100;
 		break;
 	case 2:
-		lmac->last_speed = 1000;
+		lmac->last_speed = SPEED_1000;
 		break;
 	default:
 		lmac->link_up = false;
@@ -1012,10 +1012,10 @@  static void bgx_poll_for_link(struct work_struct *work)
 	    !(smu_link & SMU_RX_CTL_STATUS)) {
 		lmac->link_up = 1;
 		if (lmac->lmac_type == BGX_MODE_XLAUI)
-			lmac->last_speed = 40000;
+			lmac->last_speed = SPEED_40000;
 		else
-			lmac->last_speed = 10000;
-		lmac->last_duplex = 1;
+			lmac->last_speed = SPEED_10000;
+		lmac->last_duplex = DUPLEX_FULL;
 	} else {
 		lmac->link_up = 0;
 		lmac->last_speed = SPEED_UNKNOWN;
@@ -1105,8 +1105,8 @@  static int bgx_lmac_enable(struct bgx *bgx, u8 lmacid)
 			} else {
 				/* Default to below link speed and duplex */
 				lmac->link_up = true;
-				lmac->last_speed = 1000;
-				lmac->last_duplex = 1;
+				lmac->last_speed = SPEED_1000;
+				lmac->last_duplex = DUPLEX_FULL;
 				bgx_sgmii_change_link_state(lmac);
 				return 0;
 			}
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
index 0c443ea..374a4d4 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -497,7 +497,7 @@  struct qlcnic_hardware_context {
 	u16 board_type;
 	u16 supported_type;
 
-	u16 link_speed;
+	u32 link_speed;
 	u16 link_duplex;
 	u16 link_autoneg;
 	u16 module_type;
diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_common.h b/drivers/net/ethernet/samsung/sxgbe/sxgbe_common.h
index c61f260..55b70d0 100644
--- a/drivers/net/ethernet/samsung/sxgbe/sxgbe_common.h
+++ b/drivers/net/ethernet/samsung/sxgbe/sxgbe_common.h
@@ -475,7 +475,7 @@  struct sxgbe_priv_data {
 	spinlock_t stats_lock;	/* lock for tx/rx statatics */
 
 	int oldlink;
-	int speed;
+	u32 speed;
 	int oldduplex;
 	struct mii_bus *mii;
 	int mii_irq[PHY_MAX_ADDR];
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index dd95d95..31a4ecb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -148,7 +148,7 @@  struct stmmac_priv {
 	struct stmmac_channel channel[STMMAC_CH_MAX];
 
 	bool oldlink;
-	int speed;
+	u32 speed;
 	int oldduplex;
 	unsigned int flow_ctrl;
 	unsigned int pause;
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 5016cd5f..63baf95 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -6,7 +6,7 @@ 
 #include <linux/phy.h>
 #include <linux/of.h>
 
-const char *phy_speed_to_str(int speed)
+const char *phy_speed_to_str(u32 speed)
 {
 	switch (speed) {
 	case SPEED_10:
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 3408489..0f54fa4 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -404,7 +404,7 @@  struct phy_device {
 	 * forced speed & duplex (no autoneg)
 	 * partner speed & duplex & pause (autoneg)
 	 */
-	int speed;
+	u32 speed;
 	int duplex;
 	int pause;
 	int asym_pause;
@@ -656,7 +656,7 @@  struct phy_fixup {
 	int (*run)(struct phy_device *phydev);
 };
 
-const char *phy_speed_to_str(int speed);
+const char *phy_speed_to_str(u32 speed);
 const char *phy_duplex_to_str(unsigned int duplex);
 
 /* A structure for mapping a particular speed and duplex
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 6411c62..eec2a32 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -46,7 +46,7 @@  struct phylink_link_state {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising);
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising);
 	phy_interface_t interface;
-	int speed;
+	u32 speed;
 	int duplex;
 	int pause;
 	unsigned int link:1;
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 3652b239..8f98475 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1587,7 +1587,7 @@  enum ethtool_link_mode_bit_indices {
 #define SPEED_100000		100000
 #define SPEED_200000		200000
 
-#define SPEED_UNKNOWN		-1
+#define SPEED_UNKNOWN		UINT_MAX
 
 static inline int ethtool_validate_speed(__u32 speed)
 {