From patchwork Fri Nov 22 12:22:17 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Russell King (Oracle)" X-Patchwork-Id: 13883128 X-Patchwork-Delegate: kuba@kernel.org Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk [78.32.30.218]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E88621D5164 for ; Fri, 22 Nov 2024 12:22:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=78.32.30.218 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732278147; cv=none; b=n6m9SyRnNIJ2wIkhAo0yWA/Lc0roWQzXUlz/6eTrQk4X3CwKoyKqHMwaOAY/HJHwcpJaP8znmFYOQqQbmRvmhTmwZxb5IGEMIa8jLsxXccZIdL4BWJRyeKZVAodEtk1w8h1xheaOXSPlMMWjxmnXko+tn6Z2FrGaqioLFh3Cavs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732278147; c=relaxed/simple; bh=6P9vbsDQcOn2TrVTB/64KzffWt/q15shx9IOFx8viOQ=; h=From:To:Cc:Subject:MIME-Version:Content-Disposition:Content-Type: Message-Id:Date; b=AlAdvME3OXgCm7Hk9pwk6iYKUjLlE99M0b1iBCupVEZEwQATb+lEEOUUdh6ecZL9s/4xAwYKcFhKPY8CKD4flFsFtToafjJMqP4DGPytLZL3kU07+QB3L4/qI4oNJrsNu0BLajcSG9SIy0ayKrt/Zcc4S2tNEGzZ3OUXuzB+6wc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=armlinux.org.uk; spf=none smtp.mailfrom=armlinux.org.uk; dkim=pass (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b=fSaO6vdT; arc=none smtp.client-ip=78.32.30.218 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=armlinux.org.uk Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=armlinux.org.uk Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b="fSaO6vdT" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Date:Sender:Message-Id:Content-Type: Content-Transfer-Encoding:MIME-Version:Subject:Cc:To:From:Reply-To:Content-ID :Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To: Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Id:List-Help: List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=xlfNOBhQj4PaLhDXJw+TWDqraZPwaHPnZ3fn7raguOs=; b=fSaO6vdTT5e5lSn9ri9qjM70OK 8kkRnsH41xoUBqDO3JmLqIwlTNwdB6c8jnHqPU4d2yT5nTDaGJcf2lZ7JHpfl4vpreGaqVCNOMcKl fJC0tXFv+mgVFpyLdxClb4mi5JRvWkL0XS57jdApYjhR5WWW6lZNRBWLoVr9Jaj5RLSUx+thO711K aiCsKhqCjK6+/LIfLqYHXZJzGbE9GY+lBDpma9fOKHlYcKrCihL8oVufh1GWkqayMPUg5ZNUt1GzW /VTQ7TWQNWEngkYc8pimQ8YvKoHFTJLZGPmUhtybw6DqS7ZJY82YxdY3s5y8DgucYa1iaMbRL8N0I OkTYSF7w==; Received: from e0022681537dd.dyn.armlinux.org.uk ([fd8f:7570:feb6:1:222:68ff:fe15:37dd]:33028 helo=rmk-PC.armlinux.org.uk) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1tESfy-0000Ux-1E; Fri, 22 Nov 2024 12:22:18 +0000 Received: from rmk by rmk-PC.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1tESfx-004evI-NH; Fri, 22 Nov 2024 12:22:17 +0000 From: "Russell King (Oracle)" To: Andrew Lunn , Heiner Kallweit Cc: "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , netdev@vger.kernel.org Subject: [PATCH net v2] net: phy: fix phy_ethtool_set_eee() incorrectly enabling LPI Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline Message-Id: Sender: Russell King Date: Fri, 22 Nov 2024 12:22:17 +0000 X-Patchwork-Delegate: kuba@kernel.org When phy_ethtool_set_eee_noneg() detects a change in the LPI parameters, it attempts to update phylib state and trigger the link to cycle so the MAC sees the updated parameters. However, in doing so, it sets phydev->enable_tx_lpi depending on whether the EEE configuration allows the MAC to generate LPI without taking into account the result of negotiation. This can be demonstrated with a 1000base-T FD interface by: # ethtool --set-eee eno0 advertise 8 # cause EEE to be not negotiated # ethtool --set-eee eno0 tx-lpi off # ethtool --set-eee eno0 tx-lpi on This results in being true, despite EEE not having been negotiated and: # ethtool --show-eee eno0 EEE status: enabled - inactive Tx LPI: 250 (us) Supported EEE link modes: 100baseT/Full 1000baseT/Full Advertised EEE link modes: 100baseT/Full Link partner advertised EEE link modes: 100baseT/Full 1000baseT/Full Fix this by keeping track of whether EEE was negotiated via a new eee_active member in struct phy_device, and include this state in the decision whether phydev->enable_tx_lpi should be set. Signed-off-by: Russell King (Oracle) --- drivers/net/phy/phy-c45.c | 2 +- drivers/net/phy/phy.c | 32 ++++++++++++++++++-------------- include/linux/phy.h | 1 + 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c index 96d0b3a5a9d3..944ae98ad110 100644 --- a/drivers/net/phy/phy-c45.c +++ b/drivers/net/phy/phy-c45.c @@ -1530,7 +1530,7 @@ int genphy_c45_ethtool_get_eee(struct phy_device *phydev, return ret; data->eee_enabled = is_enabled; - data->eee_active = ret; + data->eee_active = phydev->eee_active; linkmode_copy(data->supported, phydev->supported_eee); return 0; diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 4f3e742907cb..e174107b96e2 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -990,14 +990,14 @@ static int phy_check_link_status(struct phy_device *phydev) phydev->state = PHY_RUNNING; err = genphy_c45_eee_is_active(phydev, NULL, NULL, NULL); - if (err <= 0) - phydev->enable_tx_lpi = false; - else - phydev->enable_tx_lpi = phydev->eee_cfg.tx_lpi_enabled; + phydev->eee_active = err > 0; + phydev->enable_tx_lpi = phydev->eee_cfg.tx_lpi_enabled && + phydev->eee_active; phy_link_up(phydev); } else if (!phydev->link && phydev->state != PHY_NOLINK) { phydev->state = PHY_NOLINK; + phydev->eee_active = false; phydev->enable_tx_lpi = false; phy_link_down(phydev); } @@ -1685,16 +1685,20 @@ EXPORT_SYMBOL(phy_ethtool_get_eee); static void phy_ethtool_set_eee_noneg(struct phy_device *phydev, struct ethtool_keee *data) { - if (phydev->eee_cfg.tx_lpi_enabled != data->tx_lpi_enabled || - phydev->eee_cfg.tx_lpi_timer != data->tx_lpi_timer) { - eee_to_eeecfg(&phydev->eee_cfg, data); - phydev->enable_tx_lpi = eeecfg_mac_can_tx_lpi(&phydev->eee_cfg); - if (phydev->link) { - phydev->link = false; - phy_link_down(phydev); - phydev->link = true; - phy_link_up(phydev); - } + bool enable_tx_lpi = data->tx_lpi_enabled && + phydev->eee_active; + + eee_to_eeecfg(&phydev->eee_cfg, data); + + if ((phydev->enable_tx_lpi != enable_tx_lpi || + phydev->eee_cfg.tx_lpi_timer != data->tx_lpi_timer) && + phydev->link) { + phydev->enable_tx_lpi = false; + phydev->link = false; + phy_link_down(phydev); + phydev->enable_tx_lpi = enable_tx_lpi; + phydev->link = true; + phy_link_up(phydev); } } diff --git a/include/linux/phy.h b/include/linux/phy.h index 77c6d6451638..6a17cc05f876 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -723,6 +723,7 @@ struct phy_device { /* Energy efficient ethernet modes which should be prohibited */ __ETHTOOL_DECLARE_LINK_MODE_MASK(eee_broken_modes); bool enable_tx_lpi; + bool eee_active; struct eee_config eee_cfg; /* Host supported PHY interface types. Should be ignored if empty. */