[PATCH 0/8] alx fixes/improvements

[PATCH 0/8] alx fixes/improvements

[Johannes Berg]

Unfortunately WoL is broken and the system always wakes up
immediately, and the code also looks rather odd to me. As I
can't seem to fix it (with admittedly mostly random hacking)
I'm removing it, sorry. Having the system come back from
sleep immediately doesn't seem very useful though, I'm sure
it can be added back by somebody more interested than me.

johannes

[PATCH 1/8] alx: treat flow control correctly in alx_set_pauseparam()

[Johannes Berg]

Even when alx_setup_speed_duplex() is called, we still
need to call alx_cfg_mac_flowcontrol() and set hw->flowctrl
if flow control changed.

This was a bug I accidentally introduced while simplifying
the original driver.

Reported-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 drivers/net/ethernet/atheros/alx/ethtool.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/atheros/alx/ethtool.c b/drivers/net/ethernet/atheros/alx/ethtool.c
index 6fa2aec..50a91d0 100644
--- a/drivers/net/ethernet/atheros/alx/ethtool.c
+++ b/drivers/net/ethernet/atheros/alx/ethtool.c
@@ -187,7 +187,8 @@ static int alx_set_pauseparam(struct net_device *netdev,
 
 	if (reconfig_phy) {
 		err = alx_setup_speed_duplex(hw, hw->adv_cfg, fc);
-		return err;
+		if (err)
+			return err;
 	}
 
 	/* flow control on mac */
-- 
1.8.0

[PATCH 2/8] alx: fix 100mbit/half duplex speed translation

[Johannes Berg]

100mbit half duplex is ADVERTISED_100baseT_Half, not
ADVERTISED_10baseT_Half.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 drivers/net/ethernet/atheros/alx/hw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/atheros/alx/hw.c b/drivers/net/ethernet/atheros/alx/hw.c
index 220a16a..dc71cfb 100644
--- a/drivers/net/ethernet/atheros/alx/hw.c
+++ b/drivers/net/ethernet/atheros/alx/hw.c
@@ -1134,7 +1134,7 @@ static inline u32 alx_speed_to_ethadv(int speed)
 	case SPEED_100 + DUPLEX_FULL:
 		return ADVERTISED_100baseT_Full;
 	case SPEED_100 + DUPLEX_HALF:
-		return ADVERTISED_10baseT_Half;
+		return ADVERTISED_100baseT_Half;
 	case SPEED_10 + DUPLEX_FULL:
 		return ADVERTISED_10baseT_Full;
 	case SPEED_10 + DUPLEX_HALF:
-- 
1.8.0

[PATCH 3/8] alx: remove NET_CORE Kconfig select

[Johannes Berg]

That select doesn't make any sense, remove it.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 drivers/net/ethernet/atheros/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/atheros/Kconfig b/drivers/net/ethernet/atheros/Kconfig
index ad6aa1e9..802f6a3 100644
--- a/drivers/net/ethernet/atheros/Kconfig
+++ b/drivers/net/ethernet/atheros/Kconfig
@@ -71,7 +71,6 @@ config ALX
 	tristate "Qualcomm Atheros AR816x/AR817x support"
 	depends on PCI
 	select CRC32
-	select NET_CORE
 	select MDIO
 	help
 	  This driver supports the Qualcomm Atheros L1F ethernet adapter,
-- 
1.8.0

[PATCH 4/8] alx: make sizes unsigned

[Johannes Berg]

The ring sizes should be unsigned, pointed out by Ben Hutchings.

Reported-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 drivers/net/ethernet/atheros/alx/alx.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/alx.h b/drivers/net/ethernet/atheros/alx/alx.h
index 50b3ae2..d71103d 100644
--- a/drivers/net/ethernet/atheros/alx/alx.h
+++ b/drivers/net/ethernet/atheros/alx/alx.h
@@ -85,16 +85,16 @@ struct alx_priv {
 	struct {
 		dma_addr_t dma;
 		void *virt;
-		int size;
+		unsigned int size;
 	} descmem;
 
 	/* protect int_mask updates */
 	spinlock_t irq_lock;
 	u32 int_mask;
 
-	int tx_ringsz;
-	int rx_ringsz;
-	int rxbuf_size;
+	unsigned int tx_ringsz;
+	unsigned int rx_ringsz;
+	unsigned int rxbuf_size;
 
 	struct napi_struct napi;
 	struct alx_tx_queue txq;
-- 
1.8.0

[PATCH 5/8] alx: separate link speed/duplex fields

[Johannes Berg]

As suggested by Ben Hutchings, use separate fields to track
current link speed and duplex setting.

Reported-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 drivers/net/ethernet/atheros/alx/ethtool.c |  31 ++-----
 drivers/net/ethernet/atheros/alx/hw.c      | 139 +++++++++++++----------------
 drivers/net/ethernet/atheros/alx/hw.h      |  24 ++++-
 drivers/net/ethernet/atheros/alx/main.c    |  37 ++++----
 4 files changed, 106 insertions(+), 125 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/ethtool.c b/drivers/net/ethernet/atheros/alx/ethtool.c
index 50a91d0..5e19e08 100644
--- a/drivers/net/ethernet/atheros/alx/ethtool.c
+++ b/drivers/net/ethernet/atheros/alx/ethtool.c
@@ -85,14 +85,8 @@ static int alx_get_settings(struct net_device *netdev, struct ethtool_cmd *ecmd)
 		}
 	}
 
-	if (hw->link_speed != SPEED_UNKNOWN) {
-		ethtool_cmd_speed_set(ecmd,
-				      hw->link_speed - hw->link_speed % 10);
-		ecmd->duplex = hw->link_speed % 10;
-	} else {
-		ethtool_cmd_speed_set(ecmd, SPEED_UNKNOWN);
-		ecmd->duplex = DUPLEX_UNKNOWN;
-	}
+	ethtool_cmd_speed_set(ecmd, hw->link_speed);
+	ecmd->duplex = hw->duplex;
 
 	return 0;
 }
@@ -110,24 +104,11 @@ static int alx_set_settings(struct net_device *netdev, struct ethtool_cmd *ecmd)
 			return -EINVAL;
 		adv_cfg = ecmd->advertising | ADVERTISED_Autoneg;
 	} else {
-		int speed = ethtool_cmd_speed(ecmd);
-
-		switch (speed + ecmd->duplex) {
-		case SPEED_10 + DUPLEX_HALF:
-			adv_cfg = ADVERTISED_10baseT_Half;
-			break;
-		case SPEED_10 + DUPLEX_FULL:
-			adv_cfg = ADVERTISED_10baseT_Full;
-			break;
-		case SPEED_100 + DUPLEX_HALF:
-			adv_cfg = ADVERTISED_100baseT_Half;
-			break;
-		case SPEED_100 + DUPLEX_FULL:
-			adv_cfg = ADVERTISED_100baseT_Full;
-			break;
-		default:
+		adv_cfg = alx_speed_to_ethadv(ethtool_cmd_speed(ecmd),
+					      ecmd->duplex);
+
+		if (!adv_cfg || adv_cfg == ADVERTISED_1000baseT_Full)
 			return -EINVAL;
-		}
 	}
 
 	hw->adv_cfg = adv_cfg;
diff --git a/drivers/net/ethernet/atheros/alx/hw.c b/drivers/net/ethernet/atheros/alx/hw.c
index dc71cfb..aed48a7 100644
--- a/drivers/net/ethernet/atheros/alx/hw.c
+++ b/drivers/net/ethernet/atheros/alx/hw.c
@@ -624,12 +624,12 @@ void alx_start_mac(struct alx_hw *hw)
 	alx_write_mem32(hw, ALX_TXQ0, txq | ALX_TXQ0_EN);
 
 	mac = hw->rx_ctrl;
-	if (hw->link_speed % 10 == DUPLEX_FULL)
+	if (hw->duplex == DUPLEX_FULL)
 		mac |= ALX_MAC_CTRL_FULLD;
 	else
 		mac &= ~ALX_MAC_CTRL_FULLD;
 	ALX_SET_FIELD(mac, ALX_MAC_CTRL_SPEED,
-		      hw->link_speed >= SPEED_1000 ? ALX_MAC_CTRL_SPEED_1000 :
+		      hw->link_speed == SPEED_1000 ? ALX_MAC_CTRL_SPEED_1000 :
 						     ALX_MAC_CTRL_SPEED_10_100);
 	mac |= ALX_MAC_CTRL_TX_EN | ALX_MAC_CTRL_RX_EN;
 	hw->rx_ctrl = mac;
@@ -790,28 +790,22 @@ void alx_post_phy_link(struct alx_hw *hw)
 	u16 phy_val, len, agc;
 	u8 revid = alx_hw_revision(hw);
 	bool adj_th = revid == ALX_REV_B0;
-	int speed;
-
-	if (hw->link_speed == SPEED_UNKNOWN)
-		speed = SPEED_UNKNOWN;
-	else
-		speed = hw->link_speed - hw->link_speed % 10;
 
 	if (revid != ALX_REV_B0 && !alx_is_rev_a(revid))
 		return;
 
 	/* 1000BT/AZ, wrong cable length */
-	if (speed != SPEED_UNKNOWN) {
+	if (hw->link_speed != SPEED_UNKNOWN) {
 		alx_read_phy_ext(hw, ALX_MIIEXT_PCS, ALX_MIIEXT_CLDCTRL6,
 				 &phy_val);
 		len = ALX_GET_FIELD(phy_val, ALX_CLDCTRL6_CAB_LEN);
 		alx_read_phy_dbg(hw, ALX_MIIDBG_AGC, &phy_val);
 		agc = ALX_GET_FIELD(phy_val, ALX_AGC_2_VGA);
 
-		if ((speed == SPEED_1000 &&
+		if ((hw->link_speed == SPEED_1000 &&
 		     (len > ALX_CLDCTRL6_CAB_LEN_SHORT1G ||
 		      (len == 0 && agc > ALX_AGC_LONG1G_LIMT))) ||
-		    (speed == SPEED_100 &&
+		    (hw->link_speed == SPEED_100 &&
 		     (len > ALX_CLDCTRL6_CAB_LEN_SHORT100M ||
 		      (len == 0 && agc > ALX_AGC_LONG100M_LIMT)))) {
 			alx_write_phy_dbg(hw, ALX_MIIDBG_AZ_ANADECT,
@@ -831,10 +825,10 @@ void alx_post_phy_link(struct alx_hw *hw)
 
 		/* threshold adjust */
 		if (adj_th && hw->lnk_patch) {
-			if (speed == SPEED_100) {
+			if (hw->link_speed == SPEED_100) {
 				alx_write_phy_dbg(hw, ALX_MIIDBG_MSE16DB,
 						  ALX_MSE16DB_UP);
-			} else if (speed == SPEED_1000) {
+			} else if (hw->link_speed == SPEED_1000) {
 				/*
 				 * Giga link threshold, raise the tolerance of
 				 * noise 50%
@@ -869,7 +863,7 @@ void alx_post_phy_link(struct alx_hw *hw)
  *    1. phy link must be established before calling this function
  *    2. wol option (pattern,magic,link,etc.) is configed before call it.
  */
-int alx_pre_suspend(struct alx_hw *hw, int speed)
+int alx_pre_suspend(struct alx_hw *hw, int speed, u8 duplex)
 {
 	u32 master, mac, phy, val;
 	int err = 0;
@@ -897,9 +891,9 @@ int alx_pre_suspend(struct alx_hw *hw, int speed)
 			mac |= ALX_MAC_CTRL_RX_EN | ALX_MAC_CTRL_BRD_EN;
 		if (hw->sleep_ctrl & ALX_SLEEP_CIFS)
 			mac |= ALX_MAC_CTRL_TX_EN;
-		if (speed % 10 == DUPLEX_FULL)
+		if (duplex == DUPLEX_FULL)
 			mac |= ALX_MAC_CTRL_FULLD;
-		if (speed >= SPEED_1000)
+		if (speed == SPEED_1000)
 			ALX_SET_FIELD(mac, ALX_MAC_CTRL_SPEED,
 				      ALX_MAC_CTRL_SPEED_1000);
 		phy |= ALX_PHY_CTRL_DSPRST_OUT;
@@ -938,7 +932,7 @@ bool alx_phy_configured(struct alx_hw *hw)
 	return cfg == hw_cfg;
 }
 
-int alx_get_phy_link(struct alx_hw *hw, int *speed)
+int alx_read_phy_link(struct alx_hw *hw)
 {
 	struct pci_dev *pdev = hw->pdev;
 	u16 bmsr, giga;
@@ -953,7 +947,8 @@ int alx_get_phy_link(struct alx_hw *hw, int *speed)
 		return err;
 
 	if (!(bmsr & BMSR_LSTATUS)) {
-		*speed = SPEED_UNKNOWN;
+		hw->link_speed = SPEED_UNKNOWN;
+		hw->duplex = DUPLEX_UNKNOWN;
 		return 0;
 	}
 
@@ -967,20 +962,20 @@ int alx_get_phy_link(struct alx_hw *hw, int *speed)
 
 	switch (giga & ALX_GIGA_PSSR_SPEED) {
 	case ALX_GIGA_PSSR_1000MBS:
-		*speed = SPEED_1000;
+		hw->link_speed = SPEED_1000;
 		break;
 	case ALX_GIGA_PSSR_100MBS:
-		*speed = SPEED_100;
+		hw->link_speed = SPEED_100;
 		break;
 	case ALX_GIGA_PSSR_10MBS:
-		*speed = SPEED_10;
+		hw->link_speed = SPEED_10;
 		break;
 	default:
 		goto wrong_speed;
 	}
 
-	*speed += (giga & ALX_GIGA_PSSR_DPLX) ? DUPLEX_FULL : DUPLEX_HALF;
-	return 1;
+	hw->duplex = (giga & ALX_GIGA_PSSR_DPLX) ? DUPLEX_FULL : DUPLEX_HALF;
+	return 0;
 
 wrong_speed:
 	dev_err(&pdev->dev, "invalid PHY speed/duplex: 0x%x\n", giga);
@@ -1126,81 +1121,67 @@ void alx_configure_basic(struct alx_hw *hw)
 	alx_write_mem32(hw, ALX_WRR, val);
 }
 
-static inline u32 alx_speed_to_ethadv(int speed)
-{
-	switch (speed) {
-	case SPEED_1000 + DUPLEX_FULL:
-		return ADVERTISED_1000baseT_Full;
-	case SPEED_100 + DUPLEX_FULL:
-		return ADVERTISED_100baseT_Full;
-	case SPEED_100 + DUPLEX_HALF:
-		return ADVERTISED_100baseT_Half;
-	case SPEED_10 + DUPLEX_FULL:
-		return ADVERTISED_10baseT_Full;
-	case SPEED_10 + DUPLEX_HALF:
-		return ADVERTISED_10baseT_Half;
-	default:
-		return 0;
-	}
-}
-
-int alx_select_powersaving_speed(struct alx_hw *hw, int *speed)
+int alx_select_powersaving_speed(struct alx_hw *hw, int *speed, u8 *duplex)
 {
-	int i, err, spd;
+	int i, err;
 	u16 lpa;
 
-	err = alx_get_phy_link(hw, &spd);
-	if (err < 0)
+	err = alx_read_phy_link(hw);
+	if (err)
 		return err;
 
-	if (spd == SPEED_UNKNOWN)
+	if (hw->link_speed == SPEED_UNKNOWN) {
+		*speed = SPEED_UNKNOWN;
+		*duplex = DUPLEX_UNKNOWN;
 		return 0;
+	}
 
 	err = alx_read_phy_reg(hw, MII_LPA, &lpa);
 	if (err)
 		return err;
 
 	if (!(lpa & LPA_LPACK)) {
-		*speed = spd;
+		*speed = hw->link_speed;
 		return 0;
 	}
 
-	if (lpa & LPA_10FULL)
-		*speed = SPEED_10 + DUPLEX_FULL;
-	else if (lpa & LPA_10HALF)
-		*speed = SPEED_10 + DUPLEX_HALF;
-	else if (lpa & LPA_100FULL)
-		*speed = SPEED_100 + DUPLEX_FULL;
-	else
-		*speed = SPEED_100 + DUPLEX_HALF;
-
-	if (*speed != spd) {
-		err = alx_write_phy_reg(hw, ALX_MII_IER, 0);
-		if (err)
-			return err;
-		err = alx_setup_speed_duplex(hw,
-					     alx_speed_to_ethadv(*speed) |
-					     ADVERTISED_Autoneg,
-					     ALX_FC_ANEG | ALX_FC_RX |
-					     ALX_FC_TX);
-		if (err)
-			return err;
+	if (lpa & LPA_10FULL) {
+		*speed = SPEED_10;
+		*duplex = DUPLEX_FULL;
+	} else if (lpa & LPA_10HALF) {
+		*speed = SPEED_10;
+		*duplex = DUPLEX_HALF;
+	} else if (lpa & LPA_100FULL) {
+		*speed = SPEED_100;
+		*duplex = DUPLEX_FULL;
+	} else {
+		*speed = SPEED_100;
+		*duplex = DUPLEX_HALF;
+	}
 
-		/* wait for linkup */
-		for (i = 0; i < ALX_MAX_SETUP_LNK_CYCLE; i++) {
-			int speed2;
+	if (*speed == hw->link_speed && *duplex == hw->duplex)
+		return 0;
+	err = alx_write_phy_reg(hw, ALX_MII_IER, 0);
+	if (err)
+		return err;
+	err = alx_setup_speed_duplex(hw, alx_speed_to_ethadv(*speed, *duplex) |
+					 ADVERTISED_Autoneg, ALX_FC_ANEG |
+					 ALX_FC_RX | ALX_FC_TX);
+	if (err)
+		return err;
 
-			msleep(100);
+	/* wait for linkup */
+	for (i = 0; i < ALX_MAX_SETUP_LNK_CYCLE; i++) {
+		msleep(100);
 
-			err = alx_get_phy_link(hw, &speed2);
-			if (err < 0)
-				return err;
-			if (speed2 != SPEED_UNKNOWN)
-				break;
-		}
-		if (i == ALX_MAX_SETUP_LNK_CYCLE)
-			return -ETIMEDOUT;
+		err = alx_read_phy_link(hw);
+		if (err < 0)
+			return err;
+		if (hw->link_speed != SPEED_UNKNOWN)
+			break;
 	}
+	if (i == ALX_MAX_SETUP_LNK_CYCLE)
+		return -ETIMEDOUT;
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/atheros/alx/hw.h b/drivers/net/ethernet/atheros/alx/hw.h
index 65e723d..a60e35c 100644
--- a/drivers/net/ethernet/atheros/alx/hw.h
+++ b/drivers/net/ethernet/atheros/alx/hw.h
@@ -412,10 +412,11 @@ struct alx_hw {
 	u32 smb_timer;
 	/* SPEED_* + DUPLEX_*, SPEED_UNKNOWN if link is down */
 	int link_speed;
+	u8 duplex;
 
 	/* auto-neg advertisement or force mode config */
-	u32 adv_cfg;
 	u8 flowctrl;
+	u32 adv_cfg;
 
 	u32 sleep_ctrl;
 
@@ -478,12 +479,12 @@ void alx_reset_pcie(struct alx_hw *hw);
 void alx_enable_aspm(struct alx_hw *hw, bool l0s_en, bool l1_en);
 int alx_setup_speed_duplex(struct alx_hw *hw, u32 ethadv, u8 flowctrl);
 void alx_post_phy_link(struct alx_hw *hw);
-int alx_pre_suspend(struct alx_hw *hw, int speed);
+int alx_pre_suspend(struct alx_hw *hw, int speed, u8 duplex);
 int alx_read_phy_reg(struct alx_hw *hw, u16 reg, u16 *phy_data);
 int alx_write_phy_reg(struct alx_hw *hw, u16 reg, u16 phy_data);
 int alx_read_phy_ext(struct alx_hw *hw, u8 dev, u16 reg, u16 *pdata);
 int alx_write_phy_ext(struct alx_hw *hw, u8 dev, u16 reg, u16 data);
-int alx_get_phy_link(struct alx_hw *hw, int *speed);
+int alx_read_phy_link(struct alx_hw *hw);
 int alx_clear_phy_intr(struct alx_hw *hw);
 int alx_config_wol(struct alx_hw *hw);
 void alx_cfg_mac_flowcontrol(struct alx_hw *hw, u8 fc);
@@ -493,7 +494,22 @@ void alx_set_macaddr(struct alx_hw *hw, const u8 *addr);
 bool alx_phy_configured(struct alx_hw *hw);
 void alx_configure_basic(struct alx_hw *hw);
 void alx_disable_rss(struct alx_hw *hw);
-int alx_select_powersaving_speed(struct alx_hw *hw, int *speed);
+int alx_select_powersaving_speed(struct alx_hw *hw, int *speed, u8 *duplex);
 bool alx_get_phy_info(struct alx_hw *hw);
 
+static inline u32 alx_speed_to_ethadv(int speed, u8 duplex)
+{
+	if (speed == SPEED_1000 && duplex == DUPLEX_FULL)
+		return ADVERTISED_1000baseT_Full;
+	if (speed == SPEED_100 && duplex == DUPLEX_FULL)
+		return ADVERTISED_100baseT_Full;
+	if (speed == SPEED_100 && duplex== DUPLEX_HALF)
+		return ADVERTISED_100baseT_Half;
+	if (speed == SPEED_10 && duplex == DUPLEX_FULL)
+		return ADVERTISED_10baseT_Full;
+	if (speed == SPEED_10 && duplex == DUPLEX_HALF)
+		return ADVERTISED_10baseT_Half;
+	return 0;
+}
+
 #endif
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index 418de8b..148b4b9 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -712,6 +712,7 @@ static int alx_init_sw(struct alx_priv *alx)
 	hw->dma_chnl = hw->max_dma_chnl;
 	hw->ith_tpd = alx->tx_ringsz / 3;
 	hw->link_speed = SPEED_UNKNOWN;
+	hw->duplex = DUPLEX_UNKNOWN;
 	hw->adv_cfg = ADVERTISED_Autoneg |
 		      ADVERTISED_10baseT_Half |
 		      ADVERTISED_10baseT_Full |
@@ -758,6 +759,7 @@ static void alx_halt(struct alx_priv *alx)
 
 	alx_netif_stop(alx);
 	hw->link_speed = SPEED_UNKNOWN;
+	hw->duplex = DUPLEX_UNKNOWN;
 
 	alx_reset_mac(hw);
 
@@ -869,18 +871,18 @@ static void __alx_stop(struct alx_priv *alx)
 	alx_free_rings(alx);
 }
 
-static const char *alx_speed_desc(u16 speed)
+static const char *alx_speed_desc(struct alx_hw *hw)
 {
-	switch (speed) {
-	case SPEED_1000 + DUPLEX_FULL:
+	switch (alx_speed_to_ethadv(hw->link_speed, hw->duplex)) {
+	case ADVERTISED_1000baseT_Full:
 		return "1 Gbps Full";
-	case SPEED_100 + DUPLEX_FULL:
+	case ADVERTISED_100baseT_Full:
 		return "100 Mbps Full";
-	case SPEED_100 + DUPLEX_HALF:
+	case ADVERTISED_100baseT_Half:
 		return "100 Mbps Half";
-	case SPEED_10 + DUPLEX_FULL:
+	case ADVERTISED_10baseT_Full:
 		return "10 Mbps Full";
-	case SPEED_10 + DUPLEX_HALF:
+	case ADVERTISED_10baseT_Half:
 		return "10 Mbps Half";
 	default:
 		return "Unknown speed";
@@ -891,7 +893,8 @@ static void alx_check_link(struct alx_priv *alx)
 {
 	struct alx_hw *hw = &alx->hw;
 	unsigned long flags;
-	int speed, old_speed;
+	int old_speed;
+	u8 old_duplex;
 	int err;
 
 	/* clear PHY internal interrupt status, otherwise the main
@@ -899,7 +902,9 @@ static void alx_check_link(struct alx_priv *alx)
 	 */
 	alx_clear_phy_intr(hw);
 
-	err = alx_get_phy_link(hw, &speed);
+	old_speed = hw->link_speed;
+	old_duplex = hw->duplex;
+	err = alx_read_phy_link(hw);
 	if (err < 0)
 		goto reset;
 
@@ -908,15 +913,12 @@ static void alx_check_link(struct alx_priv *alx)
 	alx_write_mem32(hw, ALX_IMR, alx->int_mask);
 	spin_unlock_irqrestore(&alx->irq_lock, flags);
 
-	old_speed = hw->link_speed;
-
-	if (old_speed == speed)
+	if (old_speed == hw->link_speed)
 		return;
-	hw->link_speed = speed;
 
-	if (speed != SPEED_UNKNOWN) {
+	if (hw->link_speed != SPEED_UNKNOWN) {
 		netif_info(alx, link, alx->dev,
-			   "NIC Up: %s\n", alx_speed_desc(speed));
+			   "NIC Up: %s\n", alx_speed_desc(hw));
 		alx_post_phy_link(hw);
 		alx_enable_aspm(hw, true, true);
 		alx_start_mac(hw);
@@ -965,6 +967,7 @@ static int __alx_shutdown(struct pci_dev *pdev, bool *wol_en)
 	struct net_device *netdev = alx->dev;
 	struct alx_hw *hw = &alx->hw;
 	int err, speed;
+	u8 duplex;
 
 	netif_device_detach(netdev);
 
@@ -977,13 +980,13 @@ static int __alx_shutdown(struct pci_dev *pdev, bool *wol_en)
 		return err;
 #endif
 
-	err = alx_select_powersaving_speed(hw, &speed);
+	err = alx_select_powersaving_speed(hw, &speed, &duplex);
 	if (err)
 		return err;
 	err = alx_clear_phy_intr(hw);
 	if (err)
 		return err;
-	err = alx_pre_suspend(hw, speed);
+	err = alx_pre_suspend(hw, speed, duplex);
 	if (err)
 		return err;
 	err = alx_config_wol(hw);
-- 
1.8.0

[PATCH 6/8] alx: fix MAC address alignment problem

[Johannes Berg]

In two places, parts of MAC addresses are used as u32/u16
values. This can cause alignment problems, use put_unaligned
and get_unaligned to fix this.

Reported-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 drivers/net/ethernet/atheros/alx/hw.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/hw.c b/drivers/net/ethernet/atheros/alx/hw.c
index aed48a7..ea99e5d 100644
--- a/drivers/net/ethernet/atheros/alx/hw.c
+++ b/drivers/net/ethernet/atheros/alx/hw.c
@@ -282,8 +282,8 @@ static bool alx_read_macaddr(struct alx_hw *hw, u8 *addr)
 	mac1 = alx_read_mem32(hw, ALX_STAD1);
 
 	/* addr should be big-endian */
-	*(__be32 *)(addr + 2) = cpu_to_be32(mac0);
-	*(__be16 *)addr = cpu_to_be16(mac1);
+	put_unaligned(cpu_to_be32(mac0), (__be32 *)(addr + 2));
+	put_unaligned(cpu_to_be16(mac1), (__be16 *)addr);
 
 	return is_valid_ether_addr(addr);
 }
@@ -326,9 +326,9 @@ void alx_set_macaddr(struct alx_hw *hw, const u8 *addr)
 	u32 val;
 
 	/* for example: 00-0B-6A-F6-00-DC * STAD0=6AF600DC, STAD1=000B */
-	val = be32_to_cpu(*(__be32 *)(addr + 2));
+	val = be32_to_cpu(get_unaligned((__be32 *)(addr + 2)));
 	alx_write_mem32(hw, ALX_STAD0, val);
-	val = be16_to_cpu(*(__be16 *)addr);
+	val = be16_to_cpu(get_unaligned((__be16 *)addr));
 	alx_write_mem32(hw, ALX_STAD1, val);
 }
 
-- 
1.8.0

[PATCH 7/8] alx: fix ethtool support code

[Johannes Berg]

A number of places treated features wrongly, listing not-supported
features instead of supported ones. Also, the get_drvinfo ethtool
callback isn't needed, and alx_get_pauseparam can be simplified.

Reported-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 drivers/net/ethernet/atheros/alx/ethtool.c | 64 ++++++++++++++----------------
 1 file changed, 29 insertions(+), 35 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/ethtool.c b/drivers/net/ethernet/atheros/alx/ethtool.c
index 5e19e08..9261006 100644
--- a/drivers/net/ethernet/atheros/alx/ethtool.c
+++ b/drivers/net/ethernet/atheros/alx/ethtool.c
@@ -46,21 +46,37 @@
 #include "reg.h"
 #include "hw.h"
 
+static u32 alx_get_supported_speeds(struct alx_hw *hw)
+{
+	u32 supported = SUPPORTED_10baseT_Half |
+			SUPPORTED_10baseT_Full |
+			SUPPORTED_100baseT_Half |
+			SUPPORTED_100baseT_Full;
+
+	if (alx_hw_giga(hw))
+		supported |= SUPPORTED_1000baseT_Full;
+
+	BUILD_BUG_ON(SUPPORTED_10baseT_Half != ADVERTISED_10baseT_Half);
+	BUILD_BUG_ON(SUPPORTED_10baseT_Full != ADVERTISED_10baseT_Full);
+	BUILD_BUG_ON(SUPPORTED_100baseT_Half != ADVERTISED_100baseT_Half);
+	BUILD_BUG_ON(SUPPORTED_100baseT_Full != ADVERTISED_100baseT_Full);
+	BUILD_BUG_ON(SUPPORTED_1000baseT_Full != ADVERTISED_1000baseT_Full);
+
+	return supported;
+}
 
 static int alx_get_settings(struct net_device *netdev, struct ethtool_cmd *ecmd)
 {
 	struct alx_priv *alx = netdev_priv(netdev);
 	struct alx_hw *hw = &alx->hw;
 
-	ecmd->supported = SUPPORTED_10baseT_Half |
-			  SUPPORTED_10baseT_Full |
-			  SUPPORTED_100baseT_Half |
-			  SUPPORTED_100baseT_Full |
-			  SUPPORTED_Autoneg |
+	ecmd->supported = SUPPORTED_Autoneg |
 			  SUPPORTED_TP |
-			  SUPPORTED_Pause;
+			  SUPPORTED_Pause |
+			  SUPPORTED_Asym_Pause;
 	if (alx_hw_giga(hw))
 		ecmd->supported |= SUPPORTED_1000baseT_Full;
+	ecmd->supported |= alx_get_supported_speeds(hw);
 
 	ecmd->advertising = ADVERTISED_TP;
 	if (hw->adv_cfg & ADVERTISED_Autoneg)
@@ -68,6 +84,7 @@ static int alx_get_settings(struct net_device *netdev, struct ethtool_cmd *ecmd)
 
 	ecmd->port = PORT_TP;
 	ecmd->phy_address = 0;
+
 	if (hw->adv_cfg & ADVERTISED_Autoneg)
 		ecmd->autoneg = AUTONEG_ENABLE;
 	else
@@ -100,7 +117,7 @@ static int alx_set_settings(struct net_device *netdev, struct ethtool_cmd *ecmd)
 	ASSERT_RTNL();
 
 	if (ecmd->autoneg == AUTONEG_ENABLE) {
-		if (ecmd->advertising & ADVERTISED_1000baseT_Half)
+		if (ecmd->advertising & ~alx_get_supported_speeds(hw))
 			return -EINVAL;
 		adv_cfg = ecmd->advertising | ADVERTISED_Autoneg;
 	} else {
@@ -121,21 +138,10 @@ static void alx_get_pauseparam(struct net_device *netdev,
 	struct alx_priv *alx = netdev_priv(netdev);
 	struct alx_hw *hw = &alx->hw;
 
-	if (hw->flowctrl & ALX_FC_ANEG &&
-	    hw->adv_cfg & ADVERTISED_Autoneg)
-		pause->autoneg = AUTONEG_ENABLE;
-	else
-		pause->autoneg = AUTONEG_DISABLE;
-
-	if (hw->flowctrl & ALX_FC_TX)
-		pause->tx_pause = 1;
-	else
-		pause->tx_pause = 0;
-
-	if (hw->flowctrl & ALX_FC_RX)
-		pause->rx_pause = 1;
-	else
-		pause->rx_pause = 0;
+	pause->autoneg = !!(hw->flowctrl & ALX_FC_ANEG &&
+			    hw->adv_cfg & ADVERTISED_Autoneg);
+	pause->tx_pause = !!(hw->flowctrl & ALX_FC_TX);
+	pause->rx_pause = !!(hw->flowctrl & ALX_FC_RX);
 }
 
 
@@ -214,8 +220,7 @@ static int alx_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
 	struct alx_priv *alx = netdev_priv(netdev);
 	struct alx_hw *hw = &alx->hw;
 
-	if (wol->wolopts & (WAKE_ARP | WAKE_MAGICSECURE |
-			    WAKE_UCAST | WAKE_BCAST | WAKE_MCAST))
+	if (wol->wolopts & ~(WAKE_MAGIC | WAKE_PHY))
 		return -EOPNOTSUPP;
 
 	hw->sleep_ctrl = 0;
@@ -230,22 +235,11 @@ static int alx_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
 	return 0;
 }
 
-static void alx_get_drvinfo(struct net_device *netdev,
-			    struct ethtool_drvinfo *drvinfo)
-{
-	struct alx_priv *alx = netdev_priv(netdev);
-
-	strlcpy(drvinfo->driver, alx_drv_name, sizeof(drvinfo->driver));
-	strlcpy(drvinfo->bus_info, pci_name(alx->hw.pdev),
-		sizeof(drvinfo->bus_info));
-}
-
 const struct ethtool_ops alx_ethtool_ops = {
 	.get_settings	= alx_get_settings,
 	.set_settings	= alx_set_settings,
 	.get_pauseparam	= alx_get_pauseparam,
 	.set_pauseparam	= alx_set_pauseparam,
-	.get_drvinfo	= alx_get_drvinfo,
 	.get_msglevel	= alx_get_msglevel,
 	.set_msglevel	= alx_set_msglevel,
 	.get_wol	= alx_get_wol,
-- 
1.8.0

[PATCH 8/8] alx: remove WoL support

[Johannes Berg]

Unfortunately, WoL is broken and the system will immediately
resume after suspending, and I can't seem to figure out why.
Remove WoL support until the issue can be found.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 drivers/net/ethernet/atheros/alx/ethtool.c |  36 -------
 drivers/net/ethernet/atheros/alx/hw.c      | 155 -----------------------------
 drivers/net/ethernet/atheros/alx/hw.h      |   5 -
 drivers/net/ethernet/atheros/alx/main.c    | 144 ---------------------------
 4 files changed, 340 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/ethtool.c b/drivers/net/ethernet/atheros/alx/ethtool.c
index 9261006..45b3650 100644
--- a/drivers/net/ethernet/atheros/alx/ethtool.c
+++ b/drivers/net/ethernet/atheros/alx/ethtool.c
@@ -201,40 +201,6 @@ static void alx_set_msglevel(struct net_device *netdev, u32 data)
 	alx->msg_enable = data;
 }
 
-static void alx_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
-{
-	struct alx_priv *alx = netdev_priv(netdev);
-	struct alx_hw *hw = &alx->hw;
-
-	wol->supported = WAKE_MAGIC | WAKE_PHY;
-	wol->wolopts = 0;
-
-	if (hw->sleep_ctrl & ALX_SLEEP_WOL_MAGIC)
-		wol->wolopts |= WAKE_MAGIC;
-	if (hw->sleep_ctrl & ALX_SLEEP_WOL_PHY)
-		wol->wolopts |= WAKE_PHY;
-}
-
-static int alx_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
-{
-	struct alx_priv *alx = netdev_priv(netdev);
-	struct alx_hw *hw = &alx->hw;
-
-	if (wol->wolopts & ~(WAKE_MAGIC | WAKE_PHY))
-		return -EOPNOTSUPP;
-
-	hw->sleep_ctrl = 0;
-
-	if (wol->wolopts & WAKE_MAGIC)
-		hw->sleep_ctrl |= ALX_SLEEP_WOL_MAGIC;
-	if (wol->wolopts & WAKE_PHY)
-		hw->sleep_ctrl |= ALX_SLEEP_WOL_PHY;
-
-	device_set_wakeup_enable(&alx->hw.pdev->dev, hw->sleep_ctrl);
-
-	return 0;
-}
-
 const struct ethtool_ops alx_ethtool_ops = {
 	.get_settings	= alx_get_settings,
 	.set_settings	= alx_set_settings,
@@ -242,7 +208,5 @@ const struct ethtool_ops alx_ethtool_ops = {
 	.set_pauseparam	= alx_set_pauseparam,
 	.get_msglevel	= alx_get_msglevel,
 	.set_msglevel	= alx_set_msglevel,
-	.get_wol	= alx_get_wol,
-	.set_wol	= alx_set_wol,
 	.get_link	= ethtool_op_get_link,
 };
diff --git a/drivers/net/ethernet/atheros/alx/hw.c b/drivers/net/ethernet/atheros/alx/hw.c
index ea99e5d..1e8c24a 100644
--- a/drivers/net/ethernet/atheros/alx/hw.c
+++ b/drivers/net/ethernet/atheros/alx/hw.c
@@ -332,16 +332,6 @@ void alx_set_macaddr(struct alx_hw *hw, const u8 *addr)
 	alx_write_mem32(hw, ALX_STAD1, val);
 }
 
-static void alx_enable_osc(struct alx_hw *hw)
-{
-	u32 val;
-
-	/* rising edge */
-	val = alx_read_mem32(hw, ALX_MISC);
-	alx_write_mem32(hw, ALX_MISC, val & ~ALX_MISC_INTNLOSC_OPEN);
-	alx_write_mem32(hw, ALX_MISC, val | ALX_MISC_INTNLOSC_OPEN);
-}
-
 static void alx_reset_osc(struct alx_hw *hw, u8 rev)
 {
 	u32 val, val2;
@@ -858,66 +848,6 @@ void alx_post_phy_link(struct alx_hw *hw)
 	}
 }
 
-
-/* NOTE:
- *    1. phy link must be established before calling this function
- *    2. wol option (pattern,magic,link,etc.) is configed before call it.
- */
-int alx_pre_suspend(struct alx_hw *hw, int speed, u8 duplex)
-{
-	u32 master, mac, phy, val;
-	int err = 0;
-
-	master = alx_read_mem32(hw, ALX_MASTER);
-	master &= ~ALX_MASTER_PCLKSEL_SRDS;
-	mac = hw->rx_ctrl;
-	/* 10/100 half */
-	ALX_SET_FIELD(mac, ALX_MAC_CTRL_SPEED,  ALX_MAC_CTRL_SPEED_10_100);
-	mac &= ~(ALX_MAC_CTRL_FULLD | ALX_MAC_CTRL_RX_EN | ALX_MAC_CTRL_TX_EN);
-
-	phy = alx_read_mem32(hw, ALX_PHY_CTRL);
-	phy &= ~(ALX_PHY_CTRL_DSPRST_OUT | ALX_PHY_CTRL_CLS);
-	phy |= ALX_PHY_CTRL_RST_ANALOG | ALX_PHY_CTRL_HIB_PULSE |
-	       ALX_PHY_CTRL_HIB_EN;
-
-	/* without any activity  */
-	if (!(hw->sleep_ctrl & ALX_SLEEP_ACTIVE)) {
-		err = alx_write_phy_reg(hw, ALX_MII_IER, 0);
-		if (err)
-			return err;
-		phy |= ALX_PHY_CTRL_IDDQ | ALX_PHY_CTRL_POWER_DOWN;
-	} else {
-		if (hw->sleep_ctrl & (ALX_SLEEP_WOL_MAGIC | ALX_SLEEP_CIFS))
-			mac |= ALX_MAC_CTRL_RX_EN | ALX_MAC_CTRL_BRD_EN;
-		if (hw->sleep_ctrl & ALX_SLEEP_CIFS)
-			mac |= ALX_MAC_CTRL_TX_EN;
-		if (duplex == DUPLEX_FULL)
-			mac |= ALX_MAC_CTRL_FULLD;
-		if (speed == SPEED_1000)
-			ALX_SET_FIELD(mac, ALX_MAC_CTRL_SPEED,
-				      ALX_MAC_CTRL_SPEED_1000);
-		phy |= ALX_PHY_CTRL_DSPRST_OUT;
-		err = alx_write_phy_ext(hw, ALX_MIIEXT_ANEG,
-					ALX_MIIEXT_S3DIG10,
-					ALX_MIIEXT_S3DIG10_SL);
-		if (err)
-			return err;
-	}
-
-	alx_enable_osc(hw);
-	hw->rx_ctrl = mac;
-	alx_write_mem32(hw, ALX_MASTER, master);
-	alx_write_mem32(hw, ALX_MAC_CTRL, mac);
-	alx_write_mem32(hw, ALX_PHY_CTRL, phy);
-
-	/* set val of PDLL D3PLLOFF */
-	val = alx_read_mem32(hw, ALX_PDLL_TRNS1);
-	val |= ALX_PDLL_TRNS1_D3PLLOFF_EN;
-	alx_write_mem32(hw, ALX_PDLL_TRNS1, val);
-
-	return 0;
-}
-
 bool alx_phy_configured(struct alx_hw *hw)
 {
 	u32 cfg, hw_cfg;
@@ -990,26 +920,6 @@ int alx_clear_phy_intr(struct alx_hw *hw)
 	return alx_read_phy_reg(hw, ALX_MII_ISR, &isr);
 }
 
-int alx_config_wol(struct alx_hw *hw)
-{
-	u32 wol = 0;
-	int err = 0;
-
-	/* turn on magic packet event */
-	if (hw->sleep_ctrl & ALX_SLEEP_WOL_MAGIC)
-		wol |= ALX_WOL0_MAGIC_EN | ALX_WOL0_PME_MAGIC_EN;
-
-	/* turn on link up event */
-	if (hw->sleep_ctrl & ALX_SLEEP_WOL_PHY) {
-		wol |=  ALX_WOL0_LINK_EN | ALX_WOL0_PME_LINK;
-		/* only link up can wake up */
-		err = alx_write_phy_reg(hw, ALX_MII_IER, ALX_IER_LINK_UP);
-	}
-	alx_write_mem32(hw, ALX_WOL0, wol);
-
-	return err;
-}
-
 void alx_disable_rss(struct alx_hw *hw)
 {
 	u32 ctrl = alx_read_mem32(hw, ALX_RXQ0);
@@ -1121,71 +1031,6 @@ void alx_configure_basic(struct alx_hw *hw)
 	alx_write_mem32(hw, ALX_WRR, val);
 }
 
-int alx_select_powersaving_speed(struct alx_hw *hw, int *speed, u8 *duplex)
-{
-	int i, err;
-	u16 lpa;
-
-	err = alx_read_phy_link(hw);
-	if (err)
-		return err;
-
-	if (hw->link_speed == SPEED_UNKNOWN) {
-		*speed = SPEED_UNKNOWN;
-		*duplex = DUPLEX_UNKNOWN;
-		return 0;
-	}
-
-	err = alx_read_phy_reg(hw, MII_LPA, &lpa);
-	if (err)
-		return err;
-
-	if (!(lpa & LPA_LPACK)) {
-		*speed = hw->link_speed;
-		return 0;
-	}
-
-	if (lpa & LPA_10FULL) {
-		*speed = SPEED_10;
-		*duplex = DUPLEX_FULL;
-	} else if (lpa & LPA_10HALF) {
-		*speed = SPEED_10;
-		*duplex = DUPLEX_HALF;
-	} else if (lpa & LPA_100FULL) {
-		*speed = SPEED_100;
-		*duplex = DUPLEX_FULL;
-	} else {
-		*speed = SPEED_100;
-		*duplex = DUPLEX_HALF;
-	}
-
-	if (*speed == hw->link_speed && *duplex == hw->duplex)
-		return 0;
-	err = alx_write_phy_reg(hw, ALX_MII_IER, 0);
-	if (err)
-		return err;
-	err = alx_setup_speed_duplex(hw, alx_speed_to_ethadv(*speed, *duplex) |
-					 ADVERTISED_Autoneg, ALX_FC_ANEG |
-					 ALX_FC_RX | ALX_FC_TX);
-	if (err)
-		return err;
-
-	/* wait for linkup */
-	for (i = 0; i < ALX_MAX_SETUP_LNK_CYCLE; i++) {
-		msleep(100);
-
-		err = alx_read_phy_link(hw);
-		if (err < 0)
-			return err;
-		if (hw->link_speed != SPEED_UNKNOWN)
-			break;
-	}
-	if (i == ALX_MAX_SETUP_LNK_CYCLE)
-		return -ETIMEDOUT;
-
-	return 0;
-}
-
 bool alx_get_phy_info(struct alx_hw *hw)
 {
 	u16  devs1, devs2;
diff --git a/drivers/net/ethernet/atheros/alx/hw.h b/drivers/net/ethernet/atheros/alx/hw.h
index a60e35c..96f3b43 100644
--- a/drivers/net/ethernet/atheros/alx/hw.h
+++ b/drivers/net/ethernet/atheros/alx/hw.h
@@ -418,8 +418,6 @@ struct alx_hw {
 	u8 flowctrl;
 	u32 adv_cfg;
 
-	u32 sleep_ctrl;
-
 	spinlock_t mdio_lock;
 	struct mdio_if_info mdio;
 	u16 phy_id[2];
@@ -479,14 +477,12 @@ void alx_reset_pcie(struct alx_hw *hw);
 void alx_enable_aspm(struct alx_hw *hw, bool l0s_en, bool l1_en);
 int alx_setup_speed_duplex(struct alx_hw *hw, u32 ethadv, u8 flowctrl);
 void alx_post_phy_link(struct alx_hw *hw);
-int alx_pre_suspend(struct alx_hw *hw, int speed, u8 duplex);
 int alx_read_phy_reg(struct alx_hw *hw, u16 reg, u16 *phy_data);
 int alx_write_phy_reg(struct alx_hw *hw, u16 reg, u16 phy_data);
 int alx_read_phy_ext(struct alx_hw *hw, u8 dev, u16 reg, u16 *pdata);
 int alx_write_phy_ext(struct alx_hw *hw, u8 dev, u16 reg, u16 data);
 int alx_read_phy_link(struct alx_hw *hw);
 int alx_clear_phy_intr(struct alx_hw *hw);
-int alx_config_wol(struct alx_hw *hw);
 void alx_cfg_mac_flowcontrol(struct alx_hw *hw, u8 fc);
 void alx_start_mac(struct alx_hw *hw);
 int alx_reset_mac(struct alx_hw *hw);
@@ -494,7 +490,6 @@ void alx_set_macaddr(struct alx_hw *hw, const u8 *addr);
 bool alx_phy_configured(struct alx_hw *hw);
 void alx_configure_basic(struct alx_hw *hw);
 void alx_disable_rss(struct alx_hw *hw);
-int alx_select_powersaving_speed(struct alx_hw *hw, int *speed, u8 *duplex);
 bool alx_get_phy_info(struct alx_hw *hw);
 
 static inline u32 alx_speed_to_ethadv(int speed, u8 duplex)
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index 148b4b9..e2f4178 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -706,7 +706,6 @@ static int alx_init_sw(struct alx_priv *alx)
 	alx->rxbuf_size = ALIGN(ALX_RAW_MTU(hw->mtu), 8);
 	alx->tx_ringsz = 256;
 	alx->rx_ringsz = 512;
-	hw->sleep_ctrl = ALX_SLEEP_WOL_MAGIC | ALX_SLEEP_WOL_PHY;
 	hw->imt = 200;
 	alx->int_mask = ALX_ISR_MISC;
 	hw->dma_chnl = hw->max_dma_chnl;
@@ -961,66 +960,6 @@ static int alx_stop(struct net_device *netdev)
 	return 0;
 }
 
-static int __alx_shutdown(struct pci_dev *pdev, bool *wol_en)
-{
-	struct alx_priv *alx = pci_get_drvdata(pdev);
-	struct net_device *netdev = alx->dev;
-	struct alx_hw *hw = &alx->hw;
-	int err, speed;
-	u8 duplex;
-
-	netif_device_detach(netdev);
-
-	if (netif_running(netdev))
-		__alx_stop(alx);
-
-#ifdef CONFIG_PM_SLEEP
-	err = pci_save_state(pdev);
-	if (err)
-		return err;
-#endif
-
-	err = alx_select_powersaving_speed(hw, &speed, &duplex);
-	if (err)
-		return err;
-	err = alx_clear_phy_intr(hw);
-	if (err)
-		return err;
-	err = alx_pre_suspend(hw, speed, duplex);
-	if (err)
-		return err;
-	err = alx_config_wol(hw);
-	if (err)
-		return err;
-
-	*wol_en = false;
-	if (hw->sleep_ctrl & ALX_SLEEP_ACTIVE) {
-		netif_info(alx, wol, netdev,
-			   "wol: ctrl=%X, speed=%X\n",
-			   hw->sleep_ctrl, speed);
-		device_set_wakeup_enable(&pdev->dev, true);
-		*wol_en = true;
-	}
-
-	pci_disable_device(pdev);
-
-	return 0;
-}
-
-static void alx_shutdown(struct pci_dev *pdev)
-{
-	int err;
-	bool wol_en;
-
-	err = __alx_shutdown(pdev, &wol_en);
-	if (!err) {
-		pci_wake_from_d3(pdev, wol_en);
-		pci_set_power_state(pdev, PCI_D3hot);
-	} else {
-		dev_err(&pdev->dev, "shutdown fail %d\n", err);
-	}
-}
-
 static void alx_link_check(struct work_struct *work)
 {
 	struct alx_priv *alx;
@@ -1399,8 +1338,6 @@ static int alx_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto out_unmap;
 	}
 
-	device_set_wakeup_enable(&pdev->dev, hw->sleep_ctrl);
-
 	netdev_info(netdev,
 		    "Qualcomm Atheros AR816x/AR817x Ethernet [%pM]\n",
 		    netdev->dev_addr);
@@ -1441,76 +1378,6 @@ static void alx_remove(struct pci_dev *pdev)
 	free_netdev(alx->dev);
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int alx_suspend(struct device *dev)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-	int err;
-	bool wol_en;
-
-	err = __alx_shutdown(pdev, &wol_en);
-	if (err) {
-		dev_err(&pdev->dev, "shutdown fail in suspend %d\n", err);
-		return err;
-	}
-
-	if (wol_en) {
-		pci_prepare_to_sleep(pdev);
-	} else {
-		pci_wake_from_d3(pdev, false);
-		pci_set_power_state(pdev, PCI_D3hot);
-	}
-
-	return 0;
-}
-
-static int alx_resume(struct device *dev)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-	struct alx_priv *alx = pci_get_drvdata(pdev);
-	struct net_device *netdev = alx->dev;
-	struct alx_hw *hw = &alx->hw;
-	int err;
-
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
-	pci_save_state(pdev);
-
-	pci_enable_wake(pdev, PCI_D3hot, 0);
-	pci_enable_wake(pdev, PCI_D3cold, 0);
-
-	hw->link_speed = SPEED_UNKNOWN;
-	alx->int_mask = ALX_ISR_MISC;
-
-	alx_reset_pcie(hw);
-	alx_reset_phy(hw);
-
-	err = alx_reset_mac(hw);
-	if (err) {
-		netif_err(alx, hw, alx->dev,
-			  "resume:reset_mac fail %d\n", err);
-		return -EIO;
-	}
-
-	err = alx_setup_speed_duplex(hw, hw->adv_cfg, hw->flowctrl);
-	if (err) {
-		netif_err(alx, hw, alx->dev,
-			  "resume:setup_speed_duplex fail %d\n", err);
-		return -EIO;
-	}
-
-	if (netif_running(netdev)) {
-		err = __alx_open(alx, true);
-		if (err)
-			return err;
-	}
-
-	netif_device_attach(netdev);
-
-	return err;
-}
-#endif
-
 static pci_ers_result_t alx_pci_error_detected(struct pci_dev *pdev,
 					       pci_channel_state_t state)
 {
@@ -1553,8 +1420,6 @@ static pci_ers_result_t alx_pci_error_slot_reset(struct pci_dev *pdev)
 	}
 
 	pci_set_master(pdev);
-	pci_enable_wake(pdev, PCI_D3hot, 0);
-	pci_enable_wake(pdev, PCI_D3cold, 0);
 
 	alx_reset_pcie(hw);
 	if (!alx_reset_mac(hw))
@@ -1590,13 +1455,6 @@ static const struct pci_error_handlers alx_err_handlers = {
 	.resume         = alx_pci_error_resume,
 };
 
-#ifdef CONFIG_PM_SLEEP
-static SIMPLE_DEV_PM_OPS(alx_pm_ops, alx_suspend, alx_resume);
-#define ALX_PM_OPS      (&alx_pm_ops)
-#else
-#define ALX_PM_OPS      NULL
-#endif
-
 static DEFINE_PCI_DEVICE_TABLE(alx_pci_tbl) = {
 	{ PCI_VDEVICE(ATTANSIC, ALX_DEV_ID_AR8161),
 	  .driver_data = ALX_DEV_QUIRK_MSI_INTX_DISABLE_BUG },
@@ -1614,9 +1472,7 @@ static struct pci_driver alx_driver = {
 	.id_table    = alx_pci_tbl,
 	.probe       = alx_probe,
 	.remove      = alx_remove,
-	.shutdown    = alx_shutdown,
 	.err_handler = &alx_err_handlers,
-	.driver.pm   = ALX_PM_OPS,
 };
 
 module_pci_driver(alx_driver);
-- 
1.8.0

Re: [PATCH 8/8] alx: remove WoL support

[Johannes Stezenbach]

On Sat, Jun 29, 2013 at 07:23:20PM +0200, Johannes Berg wrote:
> Unfortunately, WoL is broken and the system will immediately
> resume after suspending, and I can't seem to figure out why.
> Remove WoL support until the issue can be found.
...
> -#ifdef CONFIG_PM_SLEEP
> -static SIMPLE_DEV_PM_OPS(alx_pm_ops, alx_suspend, alx_resume);
> -#define ALX_PM_OPS      (&alx_pm_ops)
> -#else
> -#define ALX_PM_OPS      NULL
> -#endif
> -
>  static DEFINE_PCI_DEVICE_TABLE(alx_pci_tbl) = {
>  	{ PCI_VDEVICE(ATTANSIC, ALX_DEV_ID_AR8161),
>  	  .driver_data = ALX_DEV_QUIRK_MSI_INTX_DISABLE_BUG },
> @@ -1614,9 +1472,7 @@ static struct pci_driver alx_driver = {
>  	.id_table    = alx_pci_tbl,
>  	.probe       = alx_probe,
>  	.remove      = alx_remove,
> -	.shutdown    = alx_shutdown,
>  	.err_handler = &alx_err_handlers,
> -	.driver.pm   = ALX_PM_OPS,
>  };

Not sure but doesn't that mean the driver breaks after suspend?
I think you removed too much, not just WOL support but all pm handling.


Johannes

Re: [PATCH 8/8] alx: remove WoL support

[Johannes Berg]

> > -	.driver.pm   = ALX_PM_OPS,
> >  };
> 
> Not sure but doesn't that mean the driver breaks after suspend?
> I think you removed too much, not just WOL support but all pm handling.

*sigh*, yes, you're probably right. Probably should stop/restart if the
interface is up. I'll try to poke at it tomorrow.

johannes

Re: [PATCH 8/8] alx: remove WoL support

[Johannes Stezenbach]

Hi Johannes,

I applied patches 1-7 of your series and the patch below
on top of it.  I had added some debug prints and found
that today alx_select_powersaving_speed() always found
a valid link after __alx_stop(), even though in
previous testing I had apparently unintialized data
in my logs from the "wol: ctrl=%X, speed=%d\n" message, no idea why.

The patch below seems to fix the wakeup issue, but WOL is
still mostly broken (immediately wakes up with
"ethtool -s eth0 wol g", but "wol p" can be used
to wake up when the cable is plugged in,
i.e. unplug -> suspend -> plug -> wake-up).
I just had one spurious wake-up after "wol p" ->
wake-up -> "wol d" -> spurious wake-up -> try again -> works.

So, if you still want to rip out WOL handling, go ahead.
My patch is just the minimal change to make the driver
behave well for my needs.

BTW, one issue that bugs me is that the link is still
up after "ifconfig eth0 down" (and it is up after loading
the driver before configuring the interface).  It seems
the ALX_PHY_CTRL_POWER_DOWN in alx_pre_suspend would
do the job, but I'm not sure and now out of time to do
more experiments.  I'm not sure how much power is wasted
by an unused PHY link.


Thanks,
Johannes


>From 1e2abe93f8b05deaeea485637f100d347a308aba Mon Sep 17 00:00:00 2001
From: Johannes Stezenbach <js@sig21.net>
Date: Sun, 30 Jun 2013 16:23:53 +0200
Subject: [PATCH] alx: disable WOL by default and fix immediate wakeups

WOL is still broken, but at least the driver doesn't
cause immediate wakeups anymore when it is disabled,
and the link is down when the system is suspended.

Signed-off-by: Johannes Stezenbach <js@sig21.net>

diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index 148b4b9..6311acc 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -706,7 +706,7 @@ static int alx_init_sw(struct alx_priv *alx)
 	alx->rxbuf_size = ALIGN(ALX_RAW_MTU(hw->mtu), 8);
 	alx->tx_ringsz = 256;
 	alx->rx_ringsz = 512;
-	hw->sleep_ctrl = ALX_SLEEP_WOL_MAGIC | ALX_SLEEP_WOL_PHY;
+	hw->sleep_ctrl = 0;
 	hw->imt = 200;
 	alx->int_mask = ALX_ISR_MISC;
 	hw->dma_chnl = hw->max_dma_chnl;
@@ -983,20 +983,20 @@ static int __alx_shutdown(struct pci_dev *pdev, bool *wol_en)
 	err = alx_select_powersaving_speed(hw, &speed, &duplex);
 	if (err)
 		return err;
-	err = alx_clear_phy_intr(hw);
+	err = alx_config_wol(hw);
 	if (err)
 		return err;
 	err = alx_pre_suspend(hw, speed, duplex);
 	if (err)
 		return err;
-	err = alx_config_wol(hw);
+	err = alx_clear_phy_intr(hw);
 	if (err)
 		return err;
 
 	*wol_en = false;
 	if (hw->sleep_ctrl & ALX_SLEEP_ACTIVE) {
 		netif_info(alx, wol, netdev,
-			   "wol: ctrl=%X, speed=%X\n",
+			   "wol: ctrl=%X, speed=%d\n",
 			   hw->sleep_ctrl, speed);
 		device_set_wakeup_enable(&pdev->dev, true);
 		*wol_en = true;

Re: [PATCH 0/8] alx fixes/improvements

[David Miller]

From: Johannes Berg <johannes@sipsolutions.net>
Date: Sat, 29 Jun 2013 19:23:12 +0200

> Unfortunately WoL is broken and the system always wakes up
> immediately, and the code also looks rather odd to me. As I
> can't seem to fix it (with admittedly mostly random hacking)
> I'm removing it, sorry. Having the system come back from
> sleep immediately doesn't seem very useful though, I'm sure
> it can be added back by somebody more interested than me.

More work seems to be needed on patch #8, so I applied just
#1 to #7 to net-next.

Thanks.