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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4826747e-0dd5-4ab9-af02-9d17a1ab7358@broadcom.com>
Date: Tue, 9 Apr 2024 13:41:43 -0700
From: Florian Fainelli <florian.fainelli@...adcom.com>
To: Andrew Lunn <andrew@...n.ch>, Doug Berger <opendmb@...il.com>,
 Broadcom internal kernel review list
 <bcm-kernel-feedback-list@...adcom.com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Oleksij Rempel <o.rempel@...gutronix.de>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: genet: Fixup EEE

On 4/8/24 17:54, 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 LPI timer value to the
> hardware.  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 LPI timer value from hardware.
> 
> Signed-off-by: Andrew Lunn <andrew@...n.ch>
> ---
>   drivers/net/ethernet/broadcom/genet/bcmgenet.c | 26 ++++++--------------------
>   drivers/net/ethernet/broadcom/genet/bcmgenet.h |  6 +-----
>   drivers/net/ethernet/broadcom/genet/bcmmii.c   |  7 +------
>   3 files changed, 8 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index b1f84b37032a..c090b519255a 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1272,13 +1272,14 @@ static void bcmgenet_get_ethtool_stats(struct net_device *dev,
>   	}
>   }
>   
> -void bcmgenet_eee_enable_set(struct net_device *dev, bool enable,
> -			     bool tx_lpi_enabled)
> +void bcmgenet_eee_enable_set(struct net_device *dev, bool enable)
>   {
>   	struct bcmgenet_priv *priv = netdev_priv(dev);
> -	u32 off = priv->hw_params->tbuf_offset + TBUF_ENERGY_CTRL;
> +	u32 off;
>   	u32 reg;
>   
> +	off = priv->hw_params->tbuf_offset + TBUF_ENERGY_CTRL;
> +
>   	if (enable && !priv->clk_eee_enabled) {
>   		clk_prepare_enable(priv->clk_eee);
>   		priv->clk_eee_enabled = true;
> @@ -1293,7 +1294,7 @@ void bcmgenet_eee_enable_set(struct net_device *dev, bool enable,
>   
>   	/* Enable EEE and switch to a 27Mhz clock automatically */
>   	reg = bcmgenet_readl(priv->base + off);
> -	if (tx_lpi_enabled)
> +	if (enable)
>   		reg |= TBUF_EEE_EN | TBUF_PM_EN;
>   	else
>   		reg &= ~(TBUF_EEE_EN | TBUF_PM_EN);
> @@ -1311,15 +1312,11 @@ 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.tx_lpi_enabled = tx_lpi_enabled;
>   }
>   
>   static int bcmgenet_get_eee(struct net_device *dev, struct ethtool_keee *e)
>   {
>   	struct bcmgenet_priv *priv = netdev_priv(dev);
> -	struct ethtool_keee *p = &priv->eee;
>   
>   	if (GENET_IS_V1(priv))
>   		return -EOPNOTSUPP;
> @@ -1327,7 +1324,6 @@ static int bcmgenet_get_eee(struct net_device *dev, struct ethtool_keee *e)
>   	if (!dev->phydev)
>   		return -ENODEV;
>   
> -	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);
> @@ -1336,8 +1332,6 @@ static int bcmgenet_get_eee(struct net_device *dev, struct ethtool_keee *e)
>   static int bcmgenet_set_eee(struct net_device *dev, struct ethtool_keee *e)
>   {
>   	struct bcmgenet_priv *priv = netdev_priv(dev);
> -	struct ethtool_keee *p = &priv->eee;
> -	bool active;
>   
>   	if (GENET_IS_V1(priv))
>   		return -EOPNOTSUPP;
> @@ -1345,15 +1339,7 @@ static int bcmgenet_set_eee(struct net_device *dev, struct ethtool_keee *e)
>   	if (!dev->phydev)
>   		return -ENODEV;
>   
> -	p->eee_enabled = e->eee_enabled;
> -
> -	if (!p->eee_enabled) {
> -		bcmgenet_eee_enable_set(dev, false, false);
> -	} else {
> -		active = phy_init_eee(dev->phydev, false) >= 0;
> -		bcmgenet_umac_writel(priv, e->tx_lpi_timer, UMAC_EEE_LPI_TIMER);
> -		bcmgenet_eee_enable_set(dev, active, e->tx_lpi_enabled);
> -	}
> +	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 7523b60b3c1c..bb82ecb2e220 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> @@ -644,8 +644,6 @@ struct bcmgenet_priv {
>   	bool wol_active;
>   
>   	struct bcmgenet_mib_counters mib;
> -
> -	struct ethtool_keee eee;
>   };
>   
>   #define GENET_IO_MACRO(name, offset)					\
> @@ -703,7 +701,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 enable,
> -			     bool tx_lpi_enabled);
> -
> +void bcmgenet_eee_enable_set(struct net_device *dev, bool enable);
>   #endif /* __BCMGENET_H__ */
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
> index 9ada89355747..25eeea4c1965 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
> @@ -30,7 +30,6 @@ static void bcmgenet_mac_config(struct net_device *dev)
>   	struct bcmgenet_priv *priv = netdev_priv(dev);
>   	struct phy_device *phydev = dev->phydev;
>   	u32 reg, cmd_bits = 0;
> -	bool active;
>   
>   	/* speed */
>   	if (phydev->speed == SPEED_1000)
> @@ -88,11 +87,6 @@ static void bcmgenet_mac_config(struct net_device *dev)
>   		reg |= CMD_TX_EN | CMD_RX_EN;
>   	}
>   	bcmgenet_umac_writel(priv, reg, UMAC_CMD);
> -
> -	active = phy_init_eee(phydev, 0) >= 0;
> -	bcmgenet_eee_enable_set(dev,
> -				priv->eee.eee_enabled && active,
> -				priv->eee.tx_lpi_enabled);

You can keep the call to bcmgenet_eee_enable_set() where it currently is 
and just change the arguments?
-- 
Florian


Download attachment "smime.p7s" of type "application/pkcs7-signature" (4221 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ