[PATCH 0/5] smsc95xx updates

[PATCH 0/5] smsc95xx updates

[Steve Glendinning]

This patchset contains all my pending smsc95xx development patches.

please consider 1 - 4 for net-next.

1 is a bugfix so please also consider it for net and stable.

5 is for review and comment only, it's not ready for inclusion yet.

Thanks,

Steve Glendinning (5):
  smsc95xx: fix error checking of usbnet_resume
  smsc95xx: detect chip revision specific features
  smsc95xx: support PHY wakeup source
  smsc95xx: refactor entering suspend modes
  smsc95xx: enable dynamic autosuspend (RFC)

 drivers/net/usb/smsc95xx.c |  405 +++++++++++++++++++++++++++++++++++++-------
 drivers/net/usb/smsc95xx.h |   20 +++
 2 files changed, 363 insertions(+), 62 deletions(-)

-- 
1.7.10.4

[PATCH 1/5] smsc95xx: fix error checking of usbnet_resume

[Steve Glendinning]

without this patch the two lines below won't ever execute

Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
---
 drivers/net/usb/smsc95xx.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index e083f53..e8465fe 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1304,7 +1304,7 @@ static int smsc95xx_resume(struct usb_interface *intf)
 		check_warn_return(ret, "Error writing PM_CTRL");
 	}
 
-	return usbnet_resume(intf);
+	ret = usbnet_resume(intf);
 	check_warn_return(ret, "usbnet_resume error");
 
 	return 0;
-- 
1.7.10.4

[PATCH 2/5] smsc95xx: detect chip revision specific features

[Steve Glendinning]

Instead of storing the number of wake-up frame filter registers
in the pdata structure, this patch changes the driver to detect
the type of device we have and store its available features.

The new two features will be used in future patches.

This patch is intended to have no change in behaviour.

Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
---
 drivers/net/usb/smsc95xx.c |   29 ++++++++++++++++++++---------
 drivers/net/usb/smsc95xx.h |    3 +++
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index e8465fe..3bacb41 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -51,6 +51,10 @@
 #define SUPPORTED_WAKE			(WAKE_UCAST | WAKE_BCAST | \
 					 WAKE_MCAST | WAKE_ARP | WAKE_MAGIC)
 
+#define FEATURE_8_WAKEUP_FILTERS	(0x01)
+#define FEATURE_PHY_NLP_CROSSOVER	(0x02)
+#define FEATURE_AUTOSUSPEND		(0x04)
+
 #define check_warn(ret, fmt, args...) \
 	({ if (ret < 0) netdev_warn(dev->net, fmt, ##args); })
 
@@ -66,7 +70,7 @@ struct smsc95xx_priv {
 	u32 hash_lo;
 	u32 wolopts;
 	spinlock_t mac_cr_lock;
-	int wuff_filter_count;
+	u8 features;
 };
 
 static bool turbo_mode = true;
@@ -1031,10 +1035,14 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 	ret = smsc95xx_read_reg(dev, ID_REV, &val);
 	check_warn_return(ret, "Failed to read ID_REV: %d\n", ret);
 	val >>= 16;
-	if ((val == ID_REV_CHIP_ID_9500A_) || (val == ID_REV_CHIP_ID_9512_))
-		pdata->wuff_filter_count = LAN9500A_WUFF_NUM;
-	else
-		pdata->wuff_filter_count = LAN9500_WUFF_NUM;
+
+	if ((val == ID_REV_CHIP_ID_9500A_) || (val == ID_REV_CHIP_ID_9530_) ||
+	    (val == ID_REV_CHIP_ID_89530_) || (val == ID_REV_CHIP_ID_9730_))
+		pdata->features = (FEATURE_8_WAKEUP_FILTERS |
+			FEATURE_PHY_NLP_CROSSOVER |
+			FEATURE_AUTOSUSPEND);
+	else if (val == ID_REV_CHIP_ID_9512_)
+		pdata->features = FEATURE_8_WAKEUP_FILTERS;
 
 	dev->net->netdev_ops = &smsc95xx_netdev_ops;
 	dev->net->ethtool_ops = &smsc95xx_ethtool_ops;
@@ -1109,6 +1117,9 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 		u32 command[2];
 		u32 offset[2];
 		u32 crc[4];
+		int wuff_filter_count =
+			(pdata->features & FEATURE_8_WAKEUP_FILTERS) ?
+			LAN9500A_WUFF_NUM : LAN9500_WUFF_NUM;
 		int i, filter = 0;
 
 		memset(command, 0, sizeof(command));
@@ -1166,7 +1177,7 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 			filter++;
 		}
 
-		for (i = 0; i < (pdata->wuff_filter_count * 4); i++) {
+		for (i = 0; i < (wuff_filter_count * 4); i++) {
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, filter_mask[i]);
 			if (ret < 0)
 				kfree(filter_mask);
@@ -1174,17 +1185,17 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 		}
 		kfree(filter_mask);
 
-		for (i = 0; i < (pdata->wuff_filter_count / 4); i++) {
+		for (i = 0; i < (wuff_filter_count / 4); i++) {
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, command[i]);
 			check_warn_return(ret, "Error writing WUFF");
 		}
 
-		for (i = 0; i < (pdata->wuff_filter_count / 4); i++) {
+		for (i = 0; i < (wuff_filter_count / 4); i++) {
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, offset[i]);
 			check_warn_return(ret, "Error writing WUFF");
 		}
 
-		for (i = 0; i < (pdata->wuff_filter_count / 2); i++) {
+		for (i = 0; i < (wuff_filter_count / 2); i++) {
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, crc[i]);
 			check_warn_return(ret, "Error writing WUFF");
 		}
diff --git a/drivers/net/usb/smsc95xx.h b/drivers/net/usb/smsc95xx.h
index 1f86269..99f04a2 100644
--- a/drivers/net/usb/smsc95xx.h
+++ b/drivers/net/usb/smsc95xx.h
@@ -55,6 +55,9 @@
 #define ID_REV_CHIP_ID_9500_		(0x9500)
 #define ID_REV_CHIP_ID_9500A_		(0x9E00)
 #define ID_REV_CHIP_ID_9512_		(0xEC00)
+#define ID_REV_CHIP_ID_9530_		(0x9530)
+#define ID_REV_CHIP_ID_89530_		(0x9E08)
+#define ID_REV_CHIP_ID_9730_		(0x9730)
 
 #define INT_STS				(0x08)
 #define INT_STS_TX_STOP_		(0x00020000)
-- 
1.7.10.4

[PATCH 3/5] smsc95xx: support PHY wakeup source

[Steve Glendinning]

This patch enables LAN9500 family devices to wake from suspend
on either link up or link down events

It also adds _nopm versions of mdio access functions, so we can
safely call them from suspend and resume functions

Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
---
 drivers/net/usb/smsc95xx.c |  164 +++++++++++++++++++++++++++++++++++++++-----
 drivers/net/usb/smsc95xx.h |   17 +++++
 2 files changed, 164 insertions(+), 17 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 3bacb41..e98ff8c 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -48,7 +48,7 @@
 #define SMSC95XX_INTERNAL_PHY_ID	(1)
 #define SMSC95XX_TX_OVERHEAD		(8)
 #define SMSC95XX_TX_OVERHEAD_CSUM	(12)
-#define SUPPORTED_WAKE			(WAKE_UCAST | WAKE_BCAST | \
+#define SUPPORTED_WAKE			(WAKE_PHY | WAKE_UCAST | WAKE_BCAST | \
 					 WAKE_MCAST | WAKE_ARP | WAKE_MAGIC)
 
 #define FEATURE_8_WAKEUP_FILTERS	(0x01)
@@ -176,14 +176,15 @@ static int smsc95xx_clear_feature(struct usbnet *dev, u32 feature)
 
 /* Loop until the read is completed with timeout
  * called with phy_mutex held */
-static int __must_check smsc95xx_phy_wait_not_busy(struct usbnet *dev)
+static int __must_check __smsc95xx_phy_wait_not_busy(struct usbnet *dev,
+						     int in_pm)
 {
 	unsigned long start_time = jiffies;
 	u32 val;
 	int ret;
 
 	do {
-		ret = smsc95xx_read_reg(dev, MII_ADDR, &val);
+		ret = __smsc95xx_read_reg(dev, MII_ADDR, &val, in_pm);
 		check_warn_return(ret, "Error reading MII_ACCESS");
 		if (!(val & MII_BUSY_))
 			return 0;
@@ -192,7 +193,8 @@ static int __must_check smsc95xx_phy_wait_not_busy(struct usbnet *dev)
 	return -EIO;
 }
 
-static int smsc95xx_mdio_read(struct net_device *netdev, int phy_id, int idx)
+static int __smsc95xx_mdio_read(struct net_device *netdev, int phy_id, int idx,
+				int in_pm)
 {
 	struct usbnet *dev = netdev_priv(netdev);
 	u32 val, addr;
@@ -201,20 +203,20 @@ static int smsc95xx_mdio_read(struct net_device *netdev, int phy_id, int idx)
 	mutex_lock(&dev->phy_mutex);
 
 	/* confirm MII not busy */
-	ret = smsc95xx_phy_wait_not_busy(dev);
+	ret = __smsc95xx_phy_wait_not_busy(dev, in_pm);
 	check_warn_goto_done(ret, "MII is busy in smsc95xx_mdio_read");
 
 	/* set the address, index & direction (read from PHY) */
 	phy_id &= dev->mii.phy_id_mask;
 	idx &= dev->mii.reg_num_mask;
 	addr = (phy_id << 11) | (idx << 6) | MII_READ_ | MII_BUSY_;
-	ret = smsc95xx_write_reg(dev, MII_ADDR, addr);
+	ret = __smsc95xx_write_reg(dev, MII_ADDR, addr, in_pm);
 	check_warn_goto_done(ret, "Error writing MII_ADDR");
 
-	ret = smsc95xx_phy_wait_not_busy(dev);
+	ret = __smsc95xx_phy_wait_not_busy(dev, in_pm);
 	check_warn_goto_done(ret, "Timed out reading MII reg %02X", idx);
 
-	ret = smsc95xx_read_reg(dev, MII_DATA, &val);
+	ret = __smsc95xx_read_reg(dev, MII_DATA, &val, in_pm);
 	check_warn_goto_done(ret, "Error reading MII_DATA");
 
 	ret = (u16)(val & 0xFFFF);
@@ -224,8 +226,8 @@ done:
 	return ret;
 }
 
-static void smsc95xx_mdio_write(struct net_device *netdev, int phy_id, int idx,
-				int regval)
+static void __smsc95xx_mdio_write(struct net_device *netdev, int phy_id,
+				  int idx, int regval, int in_pm)
 {
 	struct usbnet *dev = netdev_priv(netdev);
 	u32 val, addr;
@@ -234,27 +236,50 @@ static void smsc95xx_mdio_write(struct net_device *netdev, int phy_id, int idx,
 	mutex_lock(&dev->phy_mutex);
 
 	/* confirm MII not busy */
-	ret = smsc95xx_phy_wait_not_busy(dev);
+	ret = __smsc95xx_phy_wait_not_busy(dev, in_pm);
 	check_warn_goto_done(ret, "MII is busy in smsc95xx_mdio_write");
 
 	val = regval;
-	ret = smsc95xx_write_reg(dev, MII_DATA, val);
+	ret = __smsc95xx_write_reg(dev, MII_DATA, val, in_pm);
 	check_warn_goto_done(ret, "Error writing MII_DATA");
 
 	/* set the address, index & direction (write to PHY) */
 	phy_id &= dev->mii.phy_id_mask;
 	idx &= dev->mii.reg_num_mask;
 	addr = (phy_id << 11) | (idx << 6) | MII_WRITE_ | MII_BUSY_;
-	ret = smsc95xx_write_reg(dev, MII_ADDR, addr);
+	ret = __smsc95xx_write_reg(dev, MII_ADDR, addr, in_pm);
 	check_warn_goto_done(ret, "Error writing MII_ADDR");
 
-	ret = smsc95xx_phy_wait_not_busy(dev);
+	ret = __smsc95xx_phy_wait_not_busy(dev, in_pm);
 	check_warn_goto_done(ret, "Timed out writing MII reg %02X", idx);
 
 done:
 	mutex_unlock(&dev->phy_mutex);
 }
 
+static int smsc95xx_mdio_read_nopm(struct net_device *netdev, int phy_id,
+				   int idx)
+{
+	return __smsc95xx_mdio_read(netdev, phy_id, idx, 1);
+}
+
+static void smsc95xx_mdio_write_nopm(struct net_device *netdev, int phy_id,
+				     int idx, int regval)
+{
+	__smsc95xx_mdio_write(netdev, phy_id, idx, regval, 1);
+}
+
+static int smsc95xx_mdio_read(struct net_device *netdev, int phy_id, int idx)
+{
+	return __smsc95xx_mdio_read(netdev, phy_id, idx, 0);
+}
+
+static void smsc95xx_mdio_write(struct net_device *netdev, int phy_id, int idx,
+				int regval)
+{
+	__smsc95xx_mdio_write(netdev, phy_id, idx, regval, 0);
+}
+
 static int __must_check smsc95xx_wait_eeprom(struct usbnet *dev)
 {
 	unsigned long start_time = jiffies;
@@ -1068,18 +1093,61 @@ static u16 smsc_crc(const u8 *buffer, size_t len, int filter)
 	return bitrev16(crc16(0xFFFF, buffer, len)) << ((filter % 2) * 16);
 }
 
+static int smsc95xx_enable_phy_wakeup_interrupts(struct usbnet *dev, u16 mask)
+{
+	struct mii_if_info *mii = &dev->mii;
+	int ret;
+
+	netdev_dbg(dev->net, "enabling PHY wakeup interrupts");
+
+	/* read to clear */
+	ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, PHY_INT_SRC);
+	check_warn_return(ret, "Error reading PHY_INT_SRC");
+
+	/* enable interrupt source */
+	ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, PHY_INT_MASK);
+	check_warn_return(ret, "Error reading PHY_INT_MASK");
+
+	ret |= mask;
+
+	smsc95xx_mdio_write_nopm(dev->net, mii->phy_id, PHY_INT_MASK, ret);
+
+	return 0;
+}
+
+static int smsc95xx_link_ok_nopm(struct usbnet *dev)
+{
+	struct mii_if_info *mii = &dev->mii;
+	int ret;
+
+	/* first, a dummy read, needed to latch some MII phys */
+	ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, MII_BMSR);
+	check_warn_return(ret, "Error reading MII_BMSR");
+
+	ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, MII_BMSR);
+	check_warn_return(ret, "Error reading MII_BMSR");
+
+	return !!(ret & BMSR_LSTATUS);
+}
+
 static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct usbnet *dev = usb_get_intfdata(intf);
 	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	u32 val, link_up;
 	int ret;
-	u32 val;
 
 	ret = usbnet_suspend(intf, message);
 	check_warn_return(ret, "usbnet_suspend error");
 
-	/* if no wol options set, enter lowest power SUSPEND2 mode */
-	if (!(pdata->wolopts & SUPPORTED_WAKE)) {
+	/* determine if link is up using only _nopm functions */
+	link_up = smsc95xx_link_ok_nopm(dev);
+
+	/* if no wol options set, or if link is down and we're not waking on
+	 * PHY activity, enter lowest power SUSPEND2 mode
+	 */
+	if (!(pdata->wolopts & SUPPORTED_WAKE) ||
+		!(link_up || (pdata->wolopts & WAKE_PHY))) {
 		netdev_info(dev->net, "entering SUSPEND2 mode");
 
 		/* disable energy detect (link up) & wake up events */
@@ -1112,6 +1180,59 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 		return 0;
 	}
 
+	if (pdata->wolopts & WAKE_PHY) {
+		ret = smsc95xx_enable_phy_wakeup_interrupts(dev,
+			(PHY_INT_MASK_ANEG_COMP_ | PHY_INT_MASK_LINK_DOWN_));
+		check_warn_return(ret, "error enabling PHY wakeup ints");
+
+		/* if link is down then configure EDPD and enter SUSPEND1,
+		 * otherwise enter SUSPEND0 below
+		 */
+		if (!link_up) {
+			struct mii_if_info *mii = &dev->mii;
+			netdev_info(dev->net, "entering SUSPEND1 mode");
+
+			/* reconfigure link pulse detection timing for
+			 * compatibility with non-standard link partners
+			 */
+			if (pdata->features & FEATURE_PHY_NLP_CROSSOVER)
+				smsc95xx_mdio_write_nopm(dev->net, mii->phy_id,
+					PHY_EDPD_CONFIG,
+					PHY_EDPD_CONFIG_DEFAULT);
+
+			/* enable energy detect power-down mode */
+			ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id,
+				PHY_MODE_CTRL_STS);
+			check_warn_return(ret, "Error reading PHY_MODE_CTRL_STS");
+
+			ret |= MODE_CTRL_STS_EDPWRDOWN_;
+
+			smsc95xx_mdio_write_nopm(dev->net, mii->phy_id,
+				PHY_MODE_CTRL_STS, ret);
+
+			/* enter SUSPEND1 mode */
+			ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
+			check_warn_return(ret, "Error reading PM_CTRL");
+
+			val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
+			val |= PM_CTL_SUS_MODE_1;
+
+			ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+			check_warn_return(ret, "Error writing PM_CTRL");
+
+			/* clear wol status, enable energy detection */
+			val &= ~PM_CTL_WUPS_;
+			val |= (PM_CTL_WUPS_ED_ | PM_CTL_ED_EN_);
+
+			ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+			check_warn_return(ret, "Error writing PM_CTRL");
+
+			smsc95xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
+
+			return 0;
+		}
+	}
+
 	if (pdata->wolopts & (WAKE_BCAST | WAKE_MCAST | WAKE_ARP | WAKE_UCAST)) {
 		u32 *filter_mask = kzalloc(32, GFP_KERNEL);
 		u32 command[2];
@@ -1250,6 +1371,10 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 
 	val |= PM_CTL_WOL_EN_;
 
+	/* phy energy detect wakeup source */
+	if (pdata->wolopts & WAKE_PHY)
+		val |= PM_CTL_ED_EN_;
+
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
 	check_warn_return(ret, "Error writing PM_CTRL");
 
@@ -1271,6 +1396,11 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 	/* clear wol status */
 	val &= ~PM_CTL_WUPS_;
 	val |= PM_CTL_WUPS_WOL_;
+
+	/* enable energy detection */
+	if (pdata->wolopts & WAKE_PHY)
+		val |= PM_CTL_WUPS_ED_;
+
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
 	check_warn_return(ret, "Error writing PM_CTRL");
 
diff --git a/drivers/net/usb/smsc95xx.h b/drivers/net/usb/smsc95xx.h
index 99f04a2..f360ee3 100644
--- a/drivers/net/usb/smsc95xx.h
+++ b/drivers/net/usb/smsc95xx.h
@@ -226,6 +226,23 @@
 
 /* Vendor-specific PHY Definitions */
 
+/* EDPD NLP / crossover time configuration (LAN9500A only) */
+#define PHY_EDPD_CONFIG			(16)
+#define PHY_EDPD_CONFIG_TX_NLP_EN_	((u16)0x8000)
+#define PHY_EDPD_CONFIG_TX_NLP_1000_	((u16)0x0000)
+#define PHY_EDPD_CONFIG_TX_NLP_768_	((u16)0x2000)
+#define PHY_EDPD_CONFIG_TX_NLP_512_	((u16)0x4000)
+#define PHY_EDPD_CONFIG_TX_NLP_256_	((u16)0x6000)
+#define PHY_EDPD_CONFIG_RX_1_NLP_	((u16)0x1000)
+#define PHY_EDPD_CONFIG_RX_NLP_64_	((u16)0x0000)
+#define PHY_EDPD_CONFIG_RX_NLP_256_	((u16)0x0400)
+#define PHY_EDPD_CONFIG_RX_NLP_512_	((u16)0x0800)
+#define PHY_EDPD_CONFIG_RX_NLP_1000_	((u16)0x0C00)
+#define PHY_EDPD_CONFIG_EXT_CROSSOVER_	((u16)0x0001)
+#define PHY_EDPD_CONFIG_DEFAULT		(PHY_EDPD_CONFIG_TX_NLP_EN_ | \
+					 PHY_EDPD_CONFIG_TX_NLP_768_ | \
+					 PHY_EDPD_CONFIG_RX_1_NLP_)
+
 /* Mode Control/Status Register */
 #define PHY_MODE_CTRL_STS		(17)
 #define MODE_CTRL_STS_EDPWRDOWN_	((u16)0x2000)
-- 
1.7.10.4

[PATCH 4/5] smsc95xx: refactor entering suspend modes

[Steve Glendinning]

This patch splits out the logic for entering suspend modes
to separate functions, to reduce the complexity of the
smsc95xx_suspend function.

Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
---
 drivers/net/usb/smsc95xx.c |  178 ++++++++++++++++++++++++--------------------
 1 file changed, 99 insertions(+), 79 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index e98ff8c..bf88543 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1130,6 +1130,102 @@ static int smsc95xx_link_ok_nopm(struct usbnet *dev)
 	return !!(ret & BMSR_LSTATUS);
 }
 
+static int smsc95xx_enter_suspend0(struct usbnet *dev)
+{
+	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	u32 val;
+	int ret;
+
+	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
+	check_warn_return(ret, "Error reading PM_CTRL");
+
+	val &= (~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_));
+	val |= PM_CTL_SUS_MODE_0;
+
+	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+	check_warn_return(ret, "Error writing PM_CTRL");
+
+	/* clear wol status */
+	val &= ~PM_CTL_WUPS_;
+	val |= PM_CTL_WUPS_WOL_;
+
+	/* enable energy detection */
+	if (pdata->wolopts & WAKE_PHY)
+		val |= PM_CTL_WUPS_ED_;
+
+	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+	check_warn_return(ret, "Error writing PM_CTRL");
+
+	/* read back PM_CTRL */
+	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
+	check_warn_return(ret, "Error reading PM_CTRL");
+
+	smsc95xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
+
+	return 0;
+}
+
+static int smsc95xx_enter_suspend1(struct usbnet *dev)
+{
+	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct mii_if_info *mii = &dev->mii;
+	u32 val;
+	int ret;
+
+	/* reconfigure link pulse detection timing for
+	 * compatibility with non-standard link partners
+	 */
+	if (pdata->features & FEATURE_PHY_NLP_CROSSOVER)
+		smsc95xx_mdio_write_nopm(dev->net, mii->phy_id,	PHY_EDPD_CONFIG,
+			PHY_EDPD_CONFIG_DEFAULT);
+
+	/* enable energy detect power-down mode */
+	ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, PHY_MODE_CTRL_STS);
+	check_warn_return(ret, "Error reading PHY_MODE_CTRL_STS");
+
+	ret |= MODE_CTRL_STS_EDPWRDOWN_;
+
+	smsc95xx_mdio_write_nopm(dev->net, mii->phy_id, PHY_MODE_CTRL_STS, ret);
+
+	/* enter SUSPEND1 mode */
+	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
+	check_warn_return(ret, "Error reading PM_CTRL");
+
+	val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
+	val |= PM_CTL_SUS_MODE_1;
+
+	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+	check_warn_return(ret, "Error writing PM_CTRL");
+
+	/* clear wol status, enable energy detection */
+	val &= ~PM_CTL_WUPS_;
+	val |= (PM_CTL_WUPS_ED_ | PM_CTL_ED_EN_);
+
+	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+	check_warn_return(ret, "Error writing PM_CTRL");
+
+	smsc95xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
+
+	return 0;
+}
+
+static int smsc95xx_enter_suspend2(struct usbnet *dev)
+{
+	u32 val;
+	int ret;
+
+	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
+	check_warn_return(ret, "Error reading PM_CTRL");
+
+	val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
+	val |= PM_CTL_SUS_MODE_2;
+
+	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+	check_warn_return(ret, "Error writing PM_CTRL");
+
+	return 0;
+}
+
 static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct usbnet *dev = usb_get_intfdata(intf);
@@ -1167,17 +1263,7 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 		ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
 		check_warn_return(ret, "Error writing PM_CTRL");
 
-		/* enter suspend2 mode */
-		ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-		check_warn_return(ret, "Error reading PM_CTRL");
-
-		val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
-		val |= PM_CTL_SUS_MODE_2;
-
-		ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-		check_warn_return(ret, "Error writing PM_CTRL");
-
-		return 0;
+		return smsc95xx_enter_suspend2(dev);
 	}
 
 	if (pdata->wolopts & WAKE_PHY) {
@@ -1189,47 +1275,8 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 		 * otherwise enter SUSPEND0 below
 		 */
 		if (!link_up) {
-			struct mii_if_info *mii = &dev->mii;
 			netdev_info(dev->net, "entering SUSPEND1 mode");
-
-			/* reconfigure link pulse detection timing for
-			 * compatibility with non-standard link partners
-			 */
-			if (pdata->features & FEATURE_PHY_NLP_CROSSOVER)
-				smsc95xx_mdio_write_nopm(dev->net, mii->phy_id,
-					PHY_EDPD_CONFIG,
-					PHY_EDPD_CONFIG_DEFAULT);
-
-			/* enable energy detect power-down mode */
-			ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id,
-				PHY_MODE_CTRL_STS);
-			check_warn_return(ret, "Error reading PHY_MODE_CTRL_STS");
-
-			ret |= MODE_CTRL_STS_EDPWRDOWN_;
-
-			smsc95xx_mdio_write_nopm(dev->net, mii->phy_id,
-				PHY_MODE_CTRL_STS, ret);
-
-			/* enter SUSPEND1 mode */
-			ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-			check_warn_return(ret, "Error reading PM_CTRL");
-
-			val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
-			val |= PM_CTL_SUS_MODE_1;
-
-			ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-			check_warn_return(ret, "Error writing PM_CTRL");
-
-			/* clear wol status, enable energy detection */
-			val &= ~PM_CTL_WUPS_;
-			val |= (PM_CTL_WUPS_ED_ | PM_CTL_ED_EN_);
-
-			ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-			check_warn_return(ret, "Error writing PM_CTRL");
-
-			smsc95xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
-
-			return 0;
+			return smsc95xx_enter_suspend1(dev);
 		}
 	}
 
@@ -1383,34 +1430,7 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 
 	/* some wol options are enabled, so enter SUSPEND0 */
 	netdev_info(dev->net, "entering SUSPEND0 mode");
-
-	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-	check_warn_return(ret, "Error reading PM_CTRL");
-
-	val &= (~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_));
-	val |= PM_CTL_SUS_MODE_0;
-
-	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-	check_warn_return(ret, "Error writing PM_CTRL");
-
-	/* clear wol status */
-	val &= ~PM_CTL_WUPS_;
-	val |= PM_CTL_WUPS_WOL_;
-
-	/* enable energy detection */
-	if (pdata->wolopts & WAKE_PHY)
-		val |= PM_CTL_WUPS_ED_;
-
-	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-	check_warn_return(ret, "Error writing PM_CTRL");
-
-	/* read back PM_CTRL */
-	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-	check_warn_return(ret, "Error reading PM_CTRL");
-
-	smsc95xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
-
-	return 0;
+	return smsc95xx_enter_suspend0(dev);
 }
 
 static int smsc95xx_resume(struct usb_interface *intf)
-- 
1.7.10.4

[PATCH 5/5][RFC] smsc95xx: enable dynamic autosuspend (RFC)

[Steve Glendinning]

This is a work in-progress patch.  It's not yet complete but
I thought I'd share it for comments, feedback and testing.

This patch enables dynamic autosuspend for all devices
supported by the driver, but it will only actually work on
LAN9500A (as this has a new SUSPEND3 mode for this purpose).
Unfortunately we don't know if the connected device supports
this feature until we query its ID register at runtime.

On unsupported devices (LAN9500/9512/9514) this patch claims
to support the feature but if enabled it will always return
failure to the autosuspend call (and fill up the kernel log
with a message every 2s).

Suggestions on how best to indicate this capability at runtime
instead of compile-time would be appreciated, so we don't have
to repeatedly fail if accidentally enabled.  Or maybe this is
actually the best way?

We should also be able to identify devices supporting
autosuspend from the USB VID/PID if this would help.

Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
---
 drivers/net/usb/smsc95xx.c |  124 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 122 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index bf88543..463571c 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -55,6 +55,14 @@
 #define FEATURE_PHY_NLP_CROSSOVER	(0x02)
 #define FEATURE_AUTOSUSPEND		(0x04)
 
+#define SUSPEND_SUSPEND0		(0x01)
+#define SUSPEND_SUSPEND1		(0x02)
+#define SUSPEND_SUSPEND2		(0x04)
+#define SUSPEND_SUSPEND3		(0x08)
+#define SUSPEND_REMOTEWAKE		(0x10)
+#define SUSPEND_ALLMODES		(SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \
+					 SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3)
+
 #define check_warn(ret, fmt, args...) \
 	({ if (ret < 0) netdev_warn(dev->net, fmt, ##args); })
 
@@ -71,6 +79,7 @@ struct smsc95xx_priv {
 	u32 wolopts;
 	spinlock_t mac_cr_lock;
 	u8 features;
+	u8 suspend_flags;
 };
 
 static bool turbo_mode = true;
@@ -1162,6 +1171,8 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev)
 
 	smsc95xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
 
+	pdata->suspend_flags |= SUSPEND_SUSPEND0 | SUSPEND_REMOTEWAKE;
+
 	return 0;
 }
 
@@ -1206,11 +1217,14 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev)
 
 	smsc95xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
 
+	pdata->suspend_flags |= SUSPEND_SUSPEND1 | SUSPEND_REMOTEWAKE;
+
 	return 0;
 }
 
 static int smsc95xx_enter_suspend2(struct usbnet *dev)
 {
+	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
 	u32 val;
 	int ret;
 
@@ -1223,9 +1237,80 @@ static int smsc95xx_enter_suspend2(struct usbnet *dev)
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
 	check_warn_return(ret, "Error writing PM_CTRL");
 
+	pdata->suspend_flags |= SUSPEND_SUSPEND2;
+
+	return 0;
+}
+
+static int smsc95xx_enter_suspend3(struct usbnet *dev)
+{
+	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	u32 val;
+	int ret;
+
+	ret = smsc95xx_read_reg_nopm(dev, RX_FIFO_INF, &val);
+	check_warn_return(ret, "Error reading RX_FIFO_INF");
+
+	if (val & 0xFFFF) {
+		netdev_info(dev->net, "rx fifo not empty in autosuspend");
+		return -EBUSY;
+	}
+
+	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
+	check_warn_return(ret, "Error reading PM_CTRL");
+
+	val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
+	val |= PM_CTL_SUS_MODE_3 | PM_CTL_RES_CLR_WKP_STS;
+
+	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+	check_warn_return(ret, "Error writing PM_CTRL");
+
+	/* clear wol status */
+	val &= ~PM_CTL_WUPS_;
+	val |= PM_CTL_WUPS_WOL_;
+
+	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+	check_warn_return(ret, "Error writing PM_CTRL");
+
+	smsc95xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
+
+	pdata->suspend_flags |= SUSPEND_SUSPEND3 | SUSPEND_REMOTEWAKE;
+
 	return 0;
 }
 
+static int smsc95xx_autosuspend(struct usbnet *dev, u32 link_up)
+{
+	int ret;
+
+	if (!netif_running(dev->net)) {
+		/* interface is ifconfig down so fully power down hw */
+		netdev_dbg(dev->net, "autosuspend entering SUSPEND2");
+		return smsc95xx_enter_suspend2(dev);
+	}
+
+	if (!link_up) {
+		/* link is down so enter EDPD mode */
+		netdev_dbg(dev->net, "autosuspend entering SUSPEND1");
+
+		/* enable PHY wakeup events for if cable is attached */
+		ret = smsc95xx_enable_phy_wakeup_interrupts(dev,
+			PHY_INT_MASK_ANEG_COMP_);
+		check_warn_return(ret, "error enabling PHY wakeup ints");
+
+		netdev_info(dev->net, "entering SUSPEND1 mode");
+		return smsc95xx_enter_suspend1(dev);
+	}
+
+	/* enable PHY wakeup events so we remote wakeup if cable is pulled */
+	ret = smsc95xx_enable_phy_wakeup_interrupts(dev,
+		PHY_INT_MASK_LINK_DOWN_);
+	check_warn_return(ret, "error enabling PHY wakeup ints");
+
+	netdev_dbg(dev->net, "autosuspend entering SUSPEND3");
+	return smsc95xx_enter_suspend3(dev);
+}
+
 static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct usbnet *dev = usb_get_intfdata(intf);
@@ -1233,12 +1318,30 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 	u32 val, link_up;
 	int ret;
 
+	/* TODO: don't indicate this feature to usb framework if
+	 * our current hardware doesn't have the capability
+	 */
+	if ((message.event == PM_EVENT_AUTO_SUSPEND) &&
+	    (!(pdata->features & FEATURE_AUTOSUSPEND))) {
+		netdev_warn(dev->net, "autosuspend not supported");
+		return -EBUSY;
+	}
+
 	ret = usbnet_suspend(intf, message);
 	check_warn_return(ret, "usbnet_suspend error");
 
+	if (pdata->suspend_flags) {
+		netdev_warn(dev->net, "error during last resume");
+		pdata->suspend_flags = 0;
+	}
+
 	/* determine if link is up using only _nopm functions */
 	link_up = smsc95xx_link_ok_nopm(dev);
 
+	if (message.event == PM_EVENT_AUTO_SUSPEND)
+		return smsc95xx_autosuspend(dev, link_up);
+
+	/* if we get this far we're not autosuspending */
 	/* if no wol options set, or if link is down and we're not waking on
 	 * PHY activity, enter lowest power SUSPEND2 mode
 	 */
@@ -1437,14 +1540,23 @@ static int smsc95xx_resume(struct usb_interface *intf)
 {
 	struct usbnet *dev = usb_get_intfdata(intf);
 	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	u8 suspend_flags = pdata->suspend_flags;
 	int ret;
 	u32 val;
 
 	BUG_ON(!dev);
 
-	if (pdata->wolopts) {
-		smsc95xx_clear_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
+	netdev_dbg(dev->net, "resume suspend_flags=0x%02x", suspend_flags);
 
+	/* do this first to ensure it's cleared even in error case */
+	pdata->suspend_flags = 0;
+
+	if (suspend_flags & SUSPEND_REMOTEWAKE) {
+		ret = smsc95xx_clear_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
+		check_warn_return(ret, "Error disabling remote wakeup");
+	}
+
+	if (suspend_flags & SUSPEND_ALLMODES) {
 		/* clear wake-up sources */
 		ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
 		check_warn_return(ret, "Error reading WUCSR");
@@ -1623,6 +1735,12 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
 	return skb;
 }
 
+static int smsc95xx_manage_power(struct usbnet *dev, int on)
+{
+	dev->intf->needs_remote_wakeup = on;
+	return 0;
+}
+
 static const struct driver_info smsc95xx_info = {
 	.description	= "smsc95xx USB 2.0 Ethernet",
 	.bind		= smsc95xx_bind,
@@ -1632,6 +1750,7 @@ static const struct driver_info smsc95xx_info = {
 	.rx_fixup	= smsc95xx_rx_fixup,
 	.tx_fixup	= smsc95xx_tx_fixup,
 	.status		= smsc95xx_status,
+	.manage_power	= smsc95xx_manage_power,
 	.flags		= FLAG_ETHER | FLAG_SEND_ZLP | FLAG_LINK_INTR,
 };
 
@@ -1739,6 +1858,7 @@ static struct usb_driver smsc95xx_driver = {
 	.reset_resume	= smsc95xx_resume,
 	.disconnect	= usbnet_disconnect,
 	.disable_hub_initiated_lpm = 1,
+	.supports_autosuspend = 1,
 };
 
 module_usb_driver(smsc95xx_driver);
-- 
1.7.10.4

Re: [PATCH 1/5] smsc95xx: fix error checking of usbnet_resume

[David Miller]

I only see patches #1 and #2, where are the other 3?

If I'm not mistaken this isn't the first time your patch set length
didn't match the number of patches you actually posted :-)

Re: [PATCH 1/5] smsc95xx: fix error checking of usbnet_resume

[Steve Glendinning]

On 22 November 2012 20:32, David Miller <davem@davemloft.net> wrote:
>
> I only see patches #1 and #2, where are the other 3?

That's odd, they seem to have all made it to patchwork so they must have sent:

http://patchwork.ozlabs.org/project/netdev/list/

Spam filtering problem maybe?  Let me know if you need me to re-send.

Re: [PATCH 0/5] smsc95xx updates

[David Miller]

From: Steve Glendinning <steve.glendinning@shawell.net>
Date: Thu, 22 Nov 2012 18:05:21 +0000

> please consider 1 - 4 for net-next.

Done.

Re: [PATCH 4/5] smsc95xx: refactor entering suspend modes

[Bjørn Mork]

[adding linux-usb to CC as this is very USB specific]

Steve Glendinning <steve.glendinning-nksJyM/082jR7s880joybQ@public.gmane.org> writes:

> +	smsc95xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP);

That does look a bit strange to me.  This is a USB interface driver.
The USB device is handled by the generic "usb" USB device driver, which
will DTRT for you.  I don't think you need to set any USB device
features here.

Sorry for not commenting on this earlier.... It took me a while to
understand why that part surprised me.


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH][RFC] smsc95xx: enable dynamic autosuspend (RFC)

[Steve Glendinning]

This is a work in-progress patch.  It's not yet complete but
I thought I'd share it for comments, feedback and testing.

This patch enables dynamic autosuspend for all devices
supported by the driver, but it will only actually work on
LAN9500A (as this has a new SUSPEND3 mode for this purpose).
Unfortunately we don't know if the connected device supports
this feature until we query its ID register at runtime.

On unsupported devices (LAN9500/9512/9514) this patch claims
to support the feature but if enabled it will always return
failure to the autosuspend call (and fill up the kernel log
with a message every 2s).

Suggestions on how best to indicate this capability at runtime
instead of compile-time would be appreciated, so we don't have
to repeatedly fail if accidentally enabled.  Or maybe this is
actually the best way?

We should also be able to identify devices supporting
autosuspend from the USB VID/PID if this would help.

UPDATE: reposting this to a wider audience due to lack of
feedback last time round

Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
---
 drivers/net/usb/smsc95xx.c |  136 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 135 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 9b73670..d9c6674 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -55,6 +55,13 @@
 #define FEATURE_PHY_NLP_CROSSOVER	(0x02)
 #define FEATURE_AUTOSUSPEND		(0x04)
 
+#define SUSPEND_SUSPEND0		(0x01)
+#define SUSPEND_SUSPEND1		(0x02)
+#define SUSPEND_SUSPEND2		(0x04)
+#define SUSPEND_SUSPEND3		(0x08)
+#define SUSPEND_ALLMODES		(SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \
+					 SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3)
+
 struct smsc95xx_priv {
 	u32 mac_cr;
 	u32 hash_hi;
@@ -62,6 +69,7 @@ struct smsc95xx_priv {
 	u32 wolopts;
 	spinlock_t mac_cr_lock;
 	u8 features;
+	u8 suspend_flags;
 };
 
 static bool turbo_mode = true;
@@ -1341,6 +1349,8 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev)
 	if (ret < 0)
 		netdev_warn(dev->net, "Error reading PM_CTRL\n");
 
+	pdata->suspend_flags |= SUSPEND_SUSPEND0;
+
 	return ret;
 }
 
@@ -1393,11 +1403,14 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev)
 	if (ret < 0)
 		netdev_warn(dev->net, "Error writing PM_CTRL\n");
 
+	pdata->suspend_flags |= SUSPEND_SUSPEND1;
+
 	return ret;
 }
 
 static int smsc95xx_enter_suspend2(struct usbnet *dev)
 {
+	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
 	u32 val;
 	int ret;
 
@@ -1414,9 +1427,96 @@ static int smsc95xx_enter_suspend2(struct usbnet *dev)
 	if (ret < 0)
 		netdev_warn(dev->net, "Error writing PM_CTRL\n");
 
+	pdata->suspend_flags |= SUSPEND_SUSPEND2;
+
 	return ret;
 }
 
+static int smsc95xx_enter_suspend3(struct usbnet *dev)
+{
+	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	u32 val;
+	int ret;
+
+	ret = smsc95xx_read_reg_nopm(dev, RX_FIFO_INF, &val);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading RX_FIFO_INF");
+		return ret;
+	}
+
+	if (val & 0xFFFF) {
+		netdev_info(dev->net, "rx fifo not empty in autosuspend");
+		return -EBUSY;
+	}
+
+	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PM_CTRL");
+		return ret;
+	}
+
+	val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
+	val |= PM_CTL_SUS_MODE_3 | PM_CTL_RES_CLR_WKP_STS;
+
+	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing PM_CTRL");
+		return ret;
+	}
+
+	/* clear wol status */
+	val &= ~PM_CTL_WUPS_;
+	val |= PM_CTL_WUPS_WOL_;
+
+	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing PM_CTRL");
+		return ret;
+	}
+
+	pdata->suspend_flags |= SUSPEND_SUSPEND3;
+
+	return 0;
+}
+
+static int smsc95xx_autosuspend(struct usbnet *dev, u32 link_up)
+{
+	int ret;
+
+	if (!netif_running(dev->net)) {
+		/* interface is ifconfig down so fully power down hw */
+		netdev_dbg(dev->net, "autosuspend entering SUSPEND2");
+		return smsc95xx_enter_suspend2(dev);
+	}
+
+	if (!link_up) {
+		/* link is down so enter EDPD mode */
+		netdev_dbg(dev->net, "autosuspend entering SUSPEND1");
+
+		/* enable PHY wakeup events for if cable is attached */
+		ret = smsc95xx_enable_phy_wakeup_interrupts(dev,
+			PHY_INT_MASK_ANEG_COMP_);
+		if (ret < 0) {
+			netdev_warn(dev->net, "error enabling PHY wakeup ints");
+			return ret;
+		}
+
+		netdev_info(dev->net, "entering SUSPEND1 mode");
+		return smsc95xx_enter_suspend1(dev);
+	}
+
+	/* enable PHY wakeup events so we remote wakeup if cable is pulled */
+	ret = smsc95xx_enable_phy_wakeup_interrupts(dev,
+		PHY_INT_MASK_LINK_DOWN_);
+	if (ret < 0) {
+		netdev_warn(dev->net, "error enabling PHY wakeup ints");
+		return ret;
+	}
+
+	netdev_dbg(dev->net, "autosuspend entering SUSPEND3");
+	return smsc95xx_enter_suspend3(dev);
+}
+
 static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct usbnet *dev = usb_get_intfdata(intf);
@@ -1424,15 +1524,35 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 	u32 val, link_up;
 	int ret;
 
+	/* TODO: don't indicate this feature to usb framework if
+	 * our current hardware doesn't have the capability
+	 */
+	if ((message.event == PM_EVENT_AUTO_SUSPEND) &&
+	    (!(pdata->features & FEATURE_AUTOSUSPEND))) {
+		netdev_warn(dev->net, "autosuspend not supported");
+		return -EBUSY;
+	}
+
 	ret = usbnet_suspend(intf, message);
 	if (ret < 0) {
 		netdev_warn(dev->net, "usbnet_suspend error\n");
 		return ret;
 	}
 
+	if (pdata->suspend_flags) {
+		netdev_warn(dev->net, "error during last resume");
+		pdata->suspend_flags = 0;
+	}
+
 	/* determine if link is up using only _nopm functions */
 	link_up = smsc95xx_link_ok_nopm(dev);
 
+	if (message.event == PM_EVENT_AUTO_SUSPEND) {
+		ret = smsc95xx_autosuspend(dev, link_up);
+		goto done;
+	}
+
+	/* if we get this far we're not autosuspending */
 	/* if no wol options set, or if link is down and we're not waking on
 	 * PHY activity, enter lowest power SUSPEND2 mode
 	 */
@@ -1694,12 +1814,18 @@ static int smsc95xx_resume(struct usb_interface *intf)
 {
 	struct usbnet *dev = usb_get_intfdata(intf);
 	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	u8 suspend_flags = pdata->suspend_flags;
 	int ret;
 	u32 val;
 
 	BUG_ON(!dev);
 
-	if (pdata->wolopts) {
+	netdev_dbg(dev->net, "resume suspend_flags=0x%02x", suspend_flags);
+
+	/* do this first to ensure it's cleared even in error case */
+	pdata->suspend_flags = 0;
+
+	if (suspend_flags & SUSPEND_ALLMODES) {
 		/* clear wake-up sources */
 		ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
 		if (ret < 0) {
@@ -1891,6 +2017,12 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
 	return skb;
 }
 
+static int smsc95xx_manage_power(struct usbnet *dev, int on)
+{
+	dev->intf->needs_remote_wakeup = on;
+	return 0;
+}
+
 static const struct driver_info smsc95xx_info = {
 	.description	= "smsc95xx USB 2.0 Ethernet",
 	.bind		= smsc95xx_bind,
@@ -1900,6 +2032,7 @@ static const struct driver_info smsc95xx_info = {
 	.rx_fixup	= smsc95xx_rx_fixup,
 	.tx_fixup	= smsc95xx_tx_fixup,
 	.status		= smsc95xx_status,
+	.manage_power	= smsc95xx_manage_power,
 	.flags		= FLAG_ETHER | FLAG_SEND_ZLP | FLAG_LINK_INTR,
 };
 
@@ -2007,6 +2140,7 @@ static struct usb_driver smsc95xx_driver = {
 	.reset_resume	= smsc95xx_resume,
 	.disconnect	= usbnet_disconnect,
 	.disable_hub_initiated_lpm = 1,
+	.supports_autosuspend = 1,
 };
 
 module_usb_driver(smsc95xx_driver);
-- 
1.7.10.4

Re: [PATCH][RFC] smsc95xx: enable dynamic autosuspend (RFC)

[Oliver Neukum]

On Monday 10 December 2012 11:51:41 Steve Glendinning wrote:
> This is a work in-progress patch.  It's not yet complete but
> I thought I'd share it for comments, feedback and testing.
> 
> This patch enables dynamic autosuspend for all devices
> supported by the driver, but it will only actually work on
> LAN9500A (as this has a new SUSPEND3 mode for this purpose).

So this is a problem with remote wakeup on older hardware?

> Unfortunately we don't know if the connected device supports
> this feature until we query its ID register at runtime.
> 
> On unsupported devices (LAN9500/9512/9514) this patch claims
> to support the feature but if enabled it will always return
> failure to the autosuspend call (and fill up the kernel log
> with a message every 2s).
> 
> Suggestions on how best to indicate this capability at runtime
> instead of compile-time would be appreciated, so we don't have
> to repeatedly fail if accidentally enabled.  Or maybe this is
> actually the best way?

If this is a problem with remote wakeup, you should up the
pm counter (usb_autopm_get_noresume()) in .manage_power
That was the reason I implemented this is a callback and not as
a helper in usbnet.

	Regards
		Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH][RFC] smsc95xx: enable dynamic autosuspend (RFC)

[Ming Lei]

On Mon, Dec 10, 2012 at 7:51 PM, Steve Glendinning
<steve.glendinning-nksJyM/082jR7s880joybQ@public.gmane.org> wrote:
> This is a work in-progress patch.  It's not yet complete but
> I thought I'd share it for comments, feedback and testing.
>
> This patch enables dynamic autosuspend for all devices
> supported by the driver, but it will only actually work on
> LAN9500A (as this has a new SUSPEND3 mode for this purpose).
> Unfortunately we don't know if the connected device supports
> this feature until we query its ID register at runtime.
>
> On unsupported devices (LAN9500/9512/9514) this patch claims
> to support the feature but if enabled it will always return
> failure to the autosuspend call (and fill up the kernel log
> with a message every 2s).
>
> Suggestions on how best to indicate this capability at runtime
> instead of compile-time would be appreciated, so we don't have
> to repeatedly fail if accidentally enabled.  Or maybe this is
> actually the best way?

The ID register can be read inside bind(), so you may set
smsc95xx_info.manage_power as smsc95xx_manage_power
only for LAN9500A devices.

One disadvantage of above idea is that the link down can't trigger
runtime suspend via mange_power way(USB auto-suspend), but
we still can introduce explicit link change based runtime suspend for
non-LAN9500A devices.

>
> We should also be able to identify devices supporting
> autosuspend from the USB VID/PID if this would help.
>
> UPDATE: reposting this to a wider audience due to lack of
> feedback last time round
>
> Signed-off-by: Steve Glendinning <steve.glendinning-nksJyM/082jR7s880joybQ@public.gmane.org>
> ---
>  drivers/net/usb/smsc95xx.c |  136 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 135 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index 9b73670..d9c6674 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -55,6 +55,13 @@
>  #define FEATURE_PHY_NLP_CROSSOVER      (0x02)
>  #define FEATURE_AUTOSUSPEND            (0x04)
>
> +#define SUSPEND_SUSPEND0               (0x01)
> +#define SUSPEND_SUSPEND1               (0x02)
> +#define SUSPEND_SUSPEND2               (0x04)
> +#define SUSPEND_SUSPEND3               (0x08)
> +#define SUSPEND_ALLMODES               (SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \
> +                                        SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3)
> +
>  struct smsc95xx_priv {
>         u32 mac_cr;
>         u32 hash_hi;
> @@ -62,6 +69,7 @@ struct smsc95xx_priv {
>         u32 wolopts;
>         spinlock_t mac_cr_lock;
>         u8 features;
> +       u8 suspend_flags;
>  };
>
>  static bool turbo_mode = true;
> @@ -1341,6 +1349,8 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev)
>         if (ret < 0)
>                 netdev_warn(dev->net, "Error reading PM_CTRL\n");
>
> +       pdata->suspend_flags |= SUSPEND_SUSPEND0;
> +
>         return ret;
>  }
>
> @@ -1393,11 +1403,14 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev)
>         if (ret < 0)
>                 netdev_warn(dev->net, "Error writing PM_CTRL\n");
>
> +       pdata->suspend_flags |= SUSPEND_SUSPEND1;
> +
>         return ret;
>  }
>
>  static int smsc95xx_enter_suspend2(struct usbnet *dev)
>  {
> +       struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
>         u32 val;
>         int ret;
>
> @@ -1414,9 +1427,96 @@ static int smsc95xx_enter_suspend2(struct usbnet *dev)
>         if (ret < 0)
>                 netdev_warn(dev->net, "Error writing PM_CTRL\n");
>
> +       pdata->suspend_flags |= SUSPEND_SUSPEND2;
> +
>         return ret;
>  }
>
> +static int smsc95xx_enter_suspend3(struct usbnet *dev)
> +{
> +       struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
> +       u32 val;
> +       int ret;
> +
> +       ret = smsc95xx_read_reg_nopm(dev, RX_FIFO_INF, &val);
> +       if (ret < 0) {
> +               netdev_warn(dev->net, "Error reading RX_FIFO_INF");
> +               return ret;
> +       }
> +
> +       if (val & 0xFFFF) {
> +               netdev_info(dev->net, "rx fifo not empty in autosuspend");
> +               return -EBUSY;
> +       }
> +
> +       ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
> +       if (ret < 0) {
> +               netdev_warn(dev->net, "Error reading PM_CTRL");
> +               return ret;
> +       }
> +
> +       val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
> +       val |= PM_CTL_SUS_MODE_3 | PM_CTL_RES_CLR_WKP_STS;
> +
> +       ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
> +       if (ret < 0) {
> +               netdev_warn(dev->net, "Error writing PM_CTRL");
> +               return ret;
> +       }
> +
> +       /* clear wol status */
> +       val &= ~PM_CTL_WUPS_;
> +       val |= PM_CTL_WUPS_WOL_;
> +
> +       ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
> +       if (ret < 0) {
> +               netdev_warn(dev->net, "Error writing PM_CTRL");
> +               return ret;
> +       }
> +
> +       pdata->suspend_flags |= SUSPEND_SUSPEND3;
> +
> +       return 0;
> +}
> +
> +static int smsc95xx_autosuspend(struct usbnet *dev, u32 link_up)
> +{
> +       int ret;
> +
> +       if (!netif_running(dev->net)) {
> +               /* interface is ifconfig down so fully power down hw */
> +               netdev_dbg(dev->net, "autosuspend entering SUSPEND2");
> +               return smsc95xx_enter_suspend2(dev);
> +       }
> +
> +       if (!link_up) {
> +               /* link is down so enter EDPD mode */
> +               netdev_dbg(dev->net, "autosuspend entering SUSPEND1");
> +
> +               /* enable PHY wakeup events for if cable is attached */
> +               ret = smsc95xx_enable_phy_wakeup_interrupts(dev,
> +                       PHY_INT_MASK_ANEG_COMP_);
> +               if (ret < 0) {
> +                       netdev_warn(dev->net, "error enabling PHY wakeup ints");
> +                       return ret;
> +               }
> +
> +               netdev_info(dev->net, "entering SUSPEND1 mode");
> +               return smsc95xx_enter_suspend1(dev);
> +       }
> +
> +       /* enable PHY wakeup events so we remote wakeup if cable is pulled */
> +       ret = smsc95xx_enable_phy_wakeup_interrupts(dev,
> +               PHY_INT_MASK_LINK_DOWN_);
> +       if (ret < 0) {
> +               netdev_warn(dev->net, "error enabling PHY wakeup ints");
> +               return ret;
> +       }
> +
> +       netdev_dbg(dev->net, "autosuspend entering SUSPEND3");
> +       return smsc95xx_enter_suspend3(dev);
> +}
> +
>  static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
>  {
>         struct usbnet *dev = usb_get_intfdata(intf);
> @@ -1424,15 +1524,35 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
>         u32 val, link_up;
>         int ret;
>
> +       /* TODO: don't indicate this feature to usb framework if
> +        * our current hardware doesn't have the capability
> +        */
> +       if ((message.event == PM_EVENT_AUTO_SUSPEND) &&
> +           (!(pdata->features & FEATURE_AUTOSUSPEND))) {
> +               netdev_warn(dev->net, "autosuspend not supported");
> +               return -EBUSY;
> +       }
> +
>         ret = usbnet_suspend(intf, message);
>         if (ret < 0) {
>                 netdev_warn(dev->net, "usbnet_suspend error\n");
>                 return ret;
>         }
>
> +       if (pdata->suspend_flags) {
> +               netdev_warn(dev->net, "error during last resume");
> +               pdata->suspend_flags = 0;
> +       }
> +
>         /* determine if link is up using only _nopm functions */
>         link_up = smsc95xx_link_ok_nopm(dev);
>
> +       if (message.event == PM_EVENT_AUTO_SUSPEND) {
> +               ret = smsc95xx_autosuspend(dev, link_up);
> +               goto done;
> +       }
> +
> +       /* if we get this far we're not autosuspending */
>         /* if no wol options set, or if link is down and we're not waking on
>          * PHY activity, enter lowest power SUSPEND2 mode
>          */
> @@ -1694,12 +1814,18 @@ static int smsc95xx_resume(struct usb_interface *intf)
>  {
>         struct usbnet *dev = usb_get_intfdata(intf);
>         struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
> +       u8 suspend_flags = pdata->suspend_flags;
>         int ret;
>         u32 val;
>
>         BUG_ON(!dev);
>
> -       if (pdata->wolopts) {
> +       netdev_dbg(dev->net, "resume suspend_flags=0x%02x", suspend_flags);
> +
> +       /* do this first to ensure it's cleared even in error case */
> +       pdata->suspend_flags = 0;
> +
> +       if (suspend_flags & SUSPEND_ALLMODES) {
>                 /* clear wake-up sources */
>                 ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
>                 if (ret < 0) {
> @@ -1891,6 +2017,12 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
>         return skb;
>  }
>
> +static int smsc95xx_manage_power(struct usbnet *dev, int on)
> +{
> +       dev->intf->needs_remote_wakeup = on;
> +       return 0;
> +}
> +
>  static const struct driver_info smsc95xx_info = {
>         .description    = "smsc95xx USB 2.0 Ethernet",
>         .bind           = smsc95xx_bind,
> @@ -1900,6 +2032,7 @@ static const struct driver_info smsc95xx_info = {
>         .rx_fixup       = smsc95xx_rx_fixup,
>         .tx_fixup       = smsc95xx_tx_fixup,
>         .status         = smsc95xx_status,
> +       .manage_power   = smsc95xx_manage_power,
>         .flags          = FLAG_ETHER | FLAG_SEND_ZLP | FLAG_LINK_INTR,
>  };
>
> @@ -2007,6 +2140,7 @@ static struct usb_driver smsc95xx_driver = {
>         .reset_resume   = smsc95xx_resume,
>         .disconnect     = usbnet_disconnect,
>         .disable_hub_initiated_lpm = 1,
> +       .supports_autosuspend = 1,
>  };
>
>  module_usb_driver(smsc95xx_driver);
> --
> 1.7.10.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH][RFC] smsc95xx: enable dynamic autosuspend (RFC)

[Steve Glendinning]

On 10 December 2012 12:09, Oliver Neukum <oliver@neukum.org> wrote:
> So this is a problem with remote wakeup on older hardware?

Exactly, the older hardware revisions can't reliably do it.

>> Unfortunately we don't know if the connected device supports
>> this feature until we query its ID register at runtime.
>>
>> Suggestions on how best to indicate this capability at runtime
>> instead of compile-time would be appreciated, so we don't have
>> to repeatedly fail if accidentally enabled.  Or maybe this is
>> actually the best way?
>
> If this is a problem with remote wakeup, you should up the
> pm counter (usb_autopm_get_noresume()) in .manage_power
> That was the reason I implemented this is a callback and not as
> a helper in usbnet.

Thanks, so something like this should do the job?

static int smsc95xx_manage_power(struct usbnet *dev, int on)
{
struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);

dev->intf->needs_remote_wakeup = on;

if (pdata->features & FEATURE_AUTOSUSPEND)
return 0;

/* this chip revision doesn't support autosuspend */
netdev_info(dev->net, "hardware doesn't support USB autosuspend\n");

if (on)
usb_autopm_get_interface_no_resume(dev->intf);
else
usb_autopm_put_interface_no_suspend(dev->intf);

return 0;
}

Re: [PATCH][RFC] smsc95xx: enable dynamic autosuspend (RFC)

[Ming Lei]

On Mon, Dec 10, 2012 at 10:18 PM, Steve Glendinning <steve-nksJyM/082jR7s880joybQ@public.gmane.org> wrote:
> On 10 December 2012 12:09, Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org> wrote:
>> So this is a problem with remote wakeup on older hardware?
>
> Exactly, the older hardware revisions can't reliably do it.
>
>>> Unfortunately we don't know if the connected device supports
>>> this feature until we query its ID register at runtime.
>>>
>>> Suggestions on how best to indicate this capability at runtime
>>> instead of compile-time would be appreciated, so we don't have
>>> to repeatedly fail if accidentally enabled.  Or maybe this is
>>> actually the best way?
>>
>> If this is a problem with remote wakeup, you should up the
>> pm counter (usb_autopm_get_noresume()) in .manage_power
>> That was the reason I implemented this is a callback and not as
>> a helper in usbnet.
>
> Thanks, so something like this should do the job?

This will do, but not simple as clearing .manage_power function
pointer in bind(), and still disable runtime suspend for link off case
since these devices which don't support suspend 3 can generate
remote wakeup for link change event.

I suggest to introduce link-off triggered runtime suspend for these
usbnet devices(non-LAN9500A device, devices which don't support
USB auto-suspend), and I have posted one patch set before[1].
If no one objects that, I'd like to post them again with some fix and
update for checking link after link_reset().

[1], http://marc.info/?w=2&r=1&s=usbnet%3A+support+runtime+PM+triggered+by+&q=t

>
> static int smsc95xx_manage_power(struct usbnet *dev, int on)
> {
> struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
>
> dev->intf->needs_remote_wakeup = on;
>
> if (pdata->features & FEATURE_AUTOSUSPEND)
> return 0;
>
> /* this chip revision doesn't support autosuspend */
> netdev_info(dev->net, "hardware doesn't support USB autosuspend\n");
>
> if (on)
> usb_autopm_get_interface_no_resume(dev->intf);
> else
> usb_autopm_put_interface_no_suspend(dev->intf);
>
> return 0;
> }

Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH][RFC] smsc95xx: enable dynamic autosuspend (RFC)

[Oliver Neukum]

On Tuesday 11 December 2012 10:24:57 Ming Lei wrote:
> On Mon, Dec 10, 2012 at 10:18 PM, Steve Glendinning <steve@shawell.net> wrote:

> > Thanks, so something like this should do the job?
> 
> This will do, but not simple as clearing .manage_power function
> pointer in bind(), and still disable runtime suspend for link off case
> since these devices which don't support suspend 3 can generate
> remote wakeup for link change event.

So they can autosuspend if the interface is up and no cable is plugged
in?

> I suggest to introduce link-off triggered runtime suspend for these
> usbnet devices(non-LAN9500A device, devices which don't support
> USB auto-suspend), and I have posted one patch set before[1].
> If no one objects that, I'd like to post them again with some fix and
> update for checking link after link_reset().

If you can get rid of a periodic work this would be great.

	Regards
		Oliver

Re: [PATCH][RFC] smsc95xx: enable dynamic autosuspend (RFC)

[Ming Lei]

On Tue, Dec 11, 2012 at 6:27 PM, Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org> wrote:
> On Tuesday 11 December 2012 10:24:57 Ming Lei wrote:
>> On Mon, Dec 10, 2012 at 10:18 PM, Steve Glendinning <steve-nksJyM/082jR7s880joybQ@public.gmane.org> wrote:
>
>> > Thanks, so something like this should do the job?
>>
>> This will do, but not simple as clearing .manage_power function
>> pointer in bind(), and still disable runtime suspend for link off case
>> since these devices which don't support suspend 3 can generate
>> remote wakeup for link change event.
>
> So they can autosuspend if the interface is up and no cable is plugged
> in?

>From the open datasheet, that is the suspend 1 mode, which is supported
by all LAN95xx devices. Steve, correct me if I am wrong.

>
>> I suggest to introduce link-off triggered runtime suspend for these
>> usbnet devices(non-LAN9500A device, devices which don't support
>> USB auto-suspend), and I have posted one patch set before[1].
>> If no one objects that, I'd like to post them again with some fix and
>> update for checking link after link_reset().
>
> If you can get rid of a periodic work this would be great.

For the LAN95xx devices, the periodic work isn't needed because
they may generate remote wakeup when link change is detected.

In fact, I have test data which can show a much power save
on OMAP3 based beagle board plus asix usbnet device with
the periodic work. IMO, the power save after introducing periodic
timer depends on the arch or platform, there should be much power
save if the CPU power consumption is very less. So how about letting
module parameter switch on/off the periodic work?


Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH][RFC] smsc95xx: enable dynamic autosuspend (RFC)

[Steve Glendinning]

On 11 December 2012 12:53, Ming Lei <ming.lei@canonical.com> wrote:
> On Tue, Dec 11, 2012 at 6:27 PM, Oliver Neukum <oliver@neukum.org> wrote:
>> So they can autosuspend if the interface is up and no cable is plugged
>> in?
>
> From the open datasheet, that is the suspend 1 mode, which is supported
> by all LAN95xx devices. Steve, correct me if I am wrong.

All parts support SUSPEND1, but some parts can't 100% reliably wake on
ENERGYON - some link partners will wake them but others won't.  The
driver already detects parts that work reliably with all link partners
and sets the FEATURE_PHY_NLP_CROSSOVER feature flag.

I didn't block these devices from configuring WOL, because they do
work in *some* cases and the user is explicitly requesting to wake the
system so we try to do that (and sometimes succeed).

>>> I suggest to introduce link-off triggered runtime suspend for these
>>> usbnet devices(non-LAN9500A device, devices which don't support
>>> USB auto-suspend), and I have posted one patch set before[1].
>>> If no one objects that, I'd like to post them again with some fix and
>>> update for checking link after link_reset().
>>
>> If you can get rid of a periodic work this would be great.
>
> For the LAN95xx devices, the periodic work isn't needed because
> they may generate remote wakeup when link change is detected.

As above, some parts will do this but some will not.  I think we
should only consider sleeping the part if we're sure it'll wake up
when a cable is connected!

-- 
Steve Glendinning

Re: [PATCH][RFC] smsc95xx: enable dynamic autosuspend (RFC)

[Oliver Neukum]

On Tuesday 11 December 2012 20:53:19 Ming Lei wrote:
> In fact, I have test data which can show a much power save
> on OMAP3 based beagle board plus asix usbnet device with
> the periodic work. IMO, the power save after introducing periodic
> timer depends on the arch or platform, there should be much power
> save if the CPU power consumption is very less. So how about letting
> module parameter switch on/off the periodic work?

You could ask on linux-power and netdev. But there would be an
obvious question: Why kernel space?

	Regards
		Oliver

Re: [PATCH][RFC] smsc95xx: enable dynamic autosuspend (RFC)

[Ming Lei]

CC linux-power

On Tue, Dec 11, 2012 at 11:19 PM, Oliver Neukum <oliver@neukum.org> wrote:
> On Tuesday 11 December 2012 20:53:19 Ming Lei wrote:
>> In fact, I have test data which can show a much power save
>> on OMAP3 based beagle board plus asix usbnet device with
>> the periodic work. IMO, the power save after introducing periodic
>> timer depends on the arch or platform, there should be much power
>> save if the CPU power consumption is very less. So how about letting
>> module parameter switch on/off the periodic work?
>
> You could ask on linux-power and netdev. But there would be an
> obvious question: Why kernel space?

How does user space utility know one interface doesn't support remote
wakeup for link change? and how to do it in user space?  Or could we
persuade user space guys to do it? Or could the previous user space
utility can support power save on these devices?

At least, one advantage of doing it in kernel space is that we can let
current and previous user space utility support power save on these
devices when the link is off.

Also, suppose user space utility may close interface automatically when
the link becomes off, some configurations(such as IP address) of the
network interface will be lost after it is brought up again next time once
the link becomes on. The problem might break some application.

Thanks,
--
Ming Lei