On Mon, 23 Aug 2021 14:52:24 +0100 John Efstathiades wrote:
> 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@xxxxxxxxxxxxx>
> ---
> 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;
How is the user supposed to control this? Are similar issues not
addressed at the USB layer? There used to be a "no autosuspend"
flag that all netdev drivers set..
Was linux-usb consulted? Adding the list to Cc.
> /* 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;
Please split the ret code changes to a separate, earlier patch.
> del_timer(&dev->stat_monitor);
> } else if (link && !dev->link_on) {
