| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20210823135229.36581-6-john.efstathiades@pebblebay.com>
Date: Mon, 23 Aug 2021 14:52:24 +0100
From: John Efstathiades <john.efstathiades@...blebay.com>
To: unlisted-recipients:; (no To-header on input)
Cc: UNGLinuxDriver@...rochip.com, woojung.huh@...rochip.com,
davem@...emloft.net, netdev@...r.kernel.org,
john.efstathiades@...blebay.com
Subject: [PATCH net-next 05/10] lan78xx: Disable USB3 link power state transitions
Disable USB3 link power state transitions from U0 (Fully Powered) to
U1 (Standby with Fast Recovery) or U2 (Standby with Slow Recovery).
The device can initiate U1 and U2 state transitions when there is no
activity on the bus which can save power. However, testing with some
USB3 hosts and hubs showed erratic ping response time due to the time
required to transition back to U0 state.
In the following example the outgoing packets were delayed until the
device transitioned from U2 back to U0 giving the misleading
response time.
console:/data/local # ping 192.168.73.1
PING 192.168.73.1 (192.168.73.1) 56(84) bytes of data.
64 bytes from 192.168.73.1: icmp_seq=1 ttl=64 time=466 ms
64 bytes from 192.168.73.1: icmp_seq=2 ttl=64 time=225 ms
64 bytes from 192.168.73.1: icmp_seq=3 ttl=64 time=155 ms
64 bytes from 192.168.73.1: icmp_seq=4 ttl=64 time=7.07 ms
64 bytes from 192.168.73.1: icmp_seq=5 ttl=64 time=141 ms
64 bytes from 192.168.73.1: icmp_seq=6 ttl=64 time=152 ms
64 bytes from 192.168.73.1: icmp_seq=7 ttl=64 time=51.9 ms
64 bytes from 192.168.73.1: icmp_seq=8 ttl=64 time=136 ms
The following shows the behaviour when the U1 and U2 transitions
were disabled.
console:/data/local # ping 192.168.73.1
PING 192.168.73.1 (192.168.73.1) 56(84) bytes of data.
64 bytes from 192.168.73.1: icmp_seq=1 ttl=64 time=6.66 ms
64 bytes from 192.168.73.1: icmp_seq=2 ttl=64 time=2.97 ms
64 bytes from 192.168.73.1: icmp_seq=3 ttl=64 time=2.02 ms
64 bytes from 192.168.73.1: icmp_seq=4 ttl=64 time=2.42 ms
64 bytes from 192.168.73.1: icmp_seq=5 ttl=64 time=2.47 ms
64 bytes from 192.168.73.1: icmp_seq=6 ttl=64 time=2.55 ms
64 bytes from 192.168.73.1: icmp_seq=7 ttl=64 time=2.43 ms
64 bytes from 192.168.73.1: icmp_seq=8 ttl=64 time=2.13 ms
Signed-off-by: John Efstathiades <john.efstathiades@...blebay.com>
---
drivers/net/usb/lan78xx.c | 44 ++++++++++++++++++++++++++++++++++-----
1 file changed, 39 insertions(+), 5 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 746aeeaa9d6e..3181753b1621 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -430,6 +430,12 @@ struct lan78xx_net {
#define PHY_LAN8835 (0x0007C130)
#define PHY_KSZ9031RNX (0x00221620)
+/* Enabling link power state transitions will reduce power consumption
+ * when the link is idle. However, this can cause problems with some
+ * USB3 hubs resulting in erratic packet flow.
+ */
+static bool enable_link_power_states;
+
/* use ethtool to change the level for any given device */
static int msg_level = -1;
module_param(msg_level, int, 0);
@@ -1173,7 +1179,7 @@ static int lan78xx_link_reset(struct lan78xx_net *dev)
/* clear LAN78xx interrupt status */
ret = lan78xx_write_reg(dev, INT_STS, INT_STS_PHY_INT_);
if (unlikely(ret < 0))
- return -EIO;
+ return ret;
mutex_lock(&phydev->lock);
phy_read_status(phydev);
@@ -1186,11 +1192,11 @@ static int lan78xx_link_reset(struct lan78xx_net *dev)
/* reset MAC */
ret = lan78xx_read_reg(dev, MAC_CR, &buf);
if (unlikely(ret < 0))
- return -EIO;
+ return ret;
buf |= MAC_CR_RST_;
ret = lan78xx_write_reg(dev, MAC_CR, buf);
if (unlikely(ret < 0))
- return -EIO;
+ return ret;
del_timer(&dev->stat_monitor);
} else if (link && !dev->link_on) {
@@ -1198,23 +1204,49 @@ static int lan78xx_link_reset(struct lan78xx_net *dev)
phy_ethtool_ksettings_get(phydev, &ecmd);
- if (dev->udev->speed == USB_SPEED_SUPER) {
+ if (enable_link_power_states &&
+ dev->udev->speed == USB_SPEED_SUPER) {
if (ecmd.base.speed == 1000) {
/* disable U2 */
ret = lan78xx_read_reg(dev, USB_CFG1, &buf);
+ if (ret < 0)
+ return ret;
buf &= ~USB_CFG1_DEV_U2_INIT_EN_;
ret = lan78xx_write_reg(dev, USB_CFG1, buf);
+ if (ret < 0)
+ return ret;
/* enable U1 */
ret = lan78xx_read_reg(dev, USB_CFG1, &buf);
+ if (ret < 0)
+ return ret;
buf |= USB_CFG1_DEV_U1_INIT_EN_;
ret = lan78xx_write_reg(dev, USB_CFG1, buf);
+ if (ret < 0)
+ return ret;
} else {
/* enable U1 & U2 */
ret = lan78xx_read_reg(dev, USB_CFG1, &buf);
+ if (ret < 0)
+ return ret;
buf |= USB_CFG1_DEV_U2_INIT_EN_;
buf |= USB_CFG1_DEV_U1_INIT_EN_;
ret = lan78xx_write_reg(dev, USB_CFG1, buf);
+ if (ret < 0)
+ return ret;
}
+ } else {
+ /* Disabling initiation of U1 and U2 transitions
+ * prevents erratic ping times when connected to
+ * some USB3 hubs.
+ */
+ ret = lan78xx_read_reg(dev, USB_CFG1, &buf);
+ if (ret < 0)
+ return ret;
+ buf &= ~USB_CFG1_DEV_U2_INIT_EN_;
+ buf &= ~USB_CFG1_DEV_U1_INIT_EN_;
+ ret = lan78xx_write_reg(dev, USB_CFG1, buf);
+ if (ret < 0)
+ return ret;
}
ladv = phy_read(phydev, MII_ADVERTISE);
@@ -1231,6 +1263,8 @@ static int lan78xx_link_reset(struct lan78xx_net *dev)
ret = lan78xx_update_flowcontrol(dev, ecmd.base.duplex, ladv,
radv);
+ if (ret < 0)
+ return ret;
if (!timer_pending(&dev->stat_monitor)) {
dev->delta = 1;
@@ -1241,7 +1275,7 @@ static int lan78xx_link_reset(struct lan78xx_net *dev)
tasklet_schedule(&dev->bh);
}
- return ret;
+ return 0;
}
/* some work can't be done in tasklets, so we use keventd
--
2.25.1
Powered by blists - more mailing lists