| 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: <30ec2581-ab5d-2cf8-e5cb-dc7c99f43d3c@gmail.com>
Date: Thu, 16 Feb 2023 19:48:30 -0800
From: Florian Fainelli <f.fainelli@...il.com>
To: Andrew Lunn <andrew@...n.ch>, netdev <netdev@...r.kernel.org>
Cc: Vladimir Oltean <vladimir.oltean@....com>,
Sean Wang <sean.wang@...iatek.com>,
Landen Chao <Landen.Chao@...iatek.com>,
DENG Qingfang <dqfext@...il.com>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>,
Doug Berger <opendmb@...il.com>,
Broadcom internal kernel review list
<bcm-kernel-feedback-list@...adcom.com>,
Wei Fang <wei.fang@....com>,
Shenwei Wang <shenwei.wang@....com>,
Clark Wang <xiaoning.wang@....com>,
NXP Linux Team <linux-imx@....com>,
UNGLinuxDriver@...rochip.com, Byungho An <bh74.an@...sung.com>,
Giuseppe Cavallaro <peppe.cavallaro@...com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Jose Abreu <joabreu@...opsys.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Heiner Kallweit <hkallweit1@...il.com>,
Russell King <rmk+kernel@...linux.org.uk>,
Woojung Huh <woojung.huh@...rochip.com>,
Oleksij Rempel <linux@...pel-privat.de>
Subject: Re: [PATCH RFC 09/18] net: genet: Fixup EEE
On 2/16/2023 7:42 PM, Andrew Lunn wrote:
> The enabling/disabling of EEE in the MAC should happen as a result of
> auto negotiation. So move the enable/disable into bcmgenet_mii_setup()
> which gets called by phylib when there is a change in link status.
>
> bcmgenet_set_eee() now just writes the LTI timer value to the hardware
> and stores if TX LPI should be enabled. Everything else is passed to
> phylib, so it can correctly setup the PHY.
>
> bcmgenet_get_eee() relies on phylib doing most of the work, the MAC
> driver just adds the LTI timer value from hardware and the stored
> tx_lpi_enabled.
>
> Signed-off-by: Andrew Lunn <andrew@...n.ch>
This looks similar to a number of patches for GENET that I need to
resurrect against net-next, or even submit to net. LGTM at first glance,
I will give you series a test.
> ---
> .../net/ethernet/broadcom/genet/bcmgenet.c | 31 ++++++-------------
> .../net/ethernet/broadcom/genet/bcmgenet.h | 1 +
> drivers/net/ethernet/broadcom/genet/bcmmii.c | 1 +
> 3 files changed, 12 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index d937daa8ee88..2793d94ed32c 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1272,12 +1272,17 @@ static void bcmgenet_get_ethtool_stats(struct net_device *dev,
> }
> }
>
> -static void bcmgenet_eee_enable_set(struct net_device *dev, bool enable)
> +void bcmgenet_eee_enable_set(struct net_device *dev, bool eee_active)
> {
> struct bcmgenet_priv *priv = netdev_priv(dev);
> - u32 off = priv->hw_params->tbuf_offset + TBUF_ENERGY_CTRL;
Seems unnecessary, yes it does not quite abide by the RCT style, but no
need to fix that yet.
> + struct ethtool_eee *p = &priv->eee;
> + bool enable;
> + u32 off;
> u32 reg;
>
> + off = priv->hw_params->tbuf_offset + TBUF_ENERGY_CTRL;
> + enable = eee_active && p->tx_lpi_enabled;
> +
> if (enable && !priv->clk_eee_enabled) {
> clk_prepare_enable(priv->clk_eee);
> priv->clk_eee_enabled = true;
> @@ -1310,9 +1315,6 @@ static void bcmgenet_eee_enable_set(struct net_device *dev, bool enable)
> clk_disable_unprepare(priv->clk_eee);
> priv->clk_eee_enabled = false;
> }
> -
> - priv->eee.eee_enabled = enable;
> - priv->eee.eee_active = enable;
> }
>
> static int bcmgenet_get_eee(struct net_device *dev, struct ethtool_eee *e)
> @@ -1326,8 +1328,7 @@ static int bcmgenet_get_eee(struct net_device *dev, struct ethtool_eee *e)
> if (!dev->phydev)
> return -ENODEV;
>
> - e->eee_enabled = p->eee_enabled;
> - e->eee_active = p->eee_active;
> + e->tx_lpi_enabled = p->tx_lpi_enabled;
> e->tx_lpi_timer = bcmgenet_umac_readl(priv, UMAC_EEE_LPI_TIMER);
>
> return phy_ethtool_get_eee(dev->phydev, e);
> @@ -1337,7 +1338,6 @@ static int bcmgenet_set_eee(struct net_device *dev, struct ethtool_eee *e)
> {
> struct bcmgenet_priv *priv = netdev_priv(dev);
> struct ethtool_eee *p = &priv->eee;
> - int ret = 0;
>
> if (GENET_IS_V1(priv))
> return -EOPNOTSUPP;
> @@ -1345,20 +1345,9 @@ static int bcmgenet_set_eee(struct net_device *dev, struct ethtool_eee *e)
> if (!dev->phydev)
> return -ENODEV;
>
> - p->eee_enabled = e->eee_enabled;
> + p->tx_lpi_enabled = e->tx_lpi_enabled;
>
> - if (!p->eee_enabled) {
> - bcmgenet_eee_enable_set(dev, false);
> - } else {
> - ret = phy_init_eee(dev->phydev, false);
> - if (ret) {
> - netif_err(priv, hw, dev, "EEE initialization failed\n");
> - return ret;
> - }
> -
> - bcmgenet_umac_writel(priv, e->tx_lpi_timer, UMAC_EEE_LPI_TIMER);
> - bcmgenet_eee_enable_set(dev, true);
> - }
> + bcmgenet_umac_writel(priv, e->tx_lpi_timer, UMAC_EEE_LPI_TIMER);
>
> return phy_ethtool_set_eee(dev->phydev, e);
> }
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> index 946f6e283c4e..7458a62afc2c 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> @@ -703,4 +703,5 @@ int bcmgenet_wol_power_down_cfg(struct bcmgenet_priv *priv,
> void bcmgenet_wol_power_up_cfg(struct bcmgenet_priv *priv,
> enum bcmgenet_power_mode mode);
>
> +void bcmgenet_eee_enable_set(struct net_device *dev, bool eee_active);
> #endif /* __BCMGENET_H__ */
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
> index b615176338b2..eb1747503c2e 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
> @@ -100,6 +100,7 @@ void bcmgenet_mii_setup(struct net_device *dev)
>
> if (phydev->link) {
> bcmgenet_mac_config(dev);
> + bcmgenet_eee_enable_set(dev, phydev->eee_active);
> } else {
> reg = bcmgenet_ext_readl(priv, EXT_RGMII_OOB_CTRL);
> reg &= ~RGMII_LINK;
--
Florian
Powered by blists - more mailing lists