| Message ID | 20240625-icc_bw_voting_from_ethqos-v2-3-eaa7cf9060f0@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add interconnect support for stmmac driver. | expand |
On Tue, Jun 25, 2024 at 04:49:30PM GMT, Sagar Cheluvegowda wrote: > When mac link goes down we don't need to mainitain the clocks to operate > at higher frequencies, as an optimized solution to save power when > the link goes down we are trying to bring down the clocks to the > frequencies corresponding to the lowest speed possible. > > Signed-off-by: Sagar Cheluvegowda <quic_scheluve@quicinc.com> > --- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index ec7c61ee44d4..f0166f0bc25f 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -996,6 +996,9 @@ static void stmmac_mac_link_down(struct phylink_config *config, > { > struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev)); > > + if (priv->plat->fix_mac_speed) > + priv->plat->fix_mac_speed(priv->plat->bsp_priv, SPEED_10, mode); > + > stmmac_mac_set(priv, priv->ioaddr, false); > priv->eee_active = false; > priv->tx_lpi_enabled = false; > @@ -1004,6 +1007,11 @@ static void stmmac_mac_link_down(struct phylink_config *config, > > if (priv->dma_cap.fpesel) > stmmac_fpe_link_state_handle(priv, false); > + > + stmmac_set_icc_bw(priv, SPEED_10); > + > + if (priv->plat->fix_mac_speed) > + priv->plat->fix_mac_speed(priv->plat->bsp_priv, SPEED_10, mode); I think you're doing this at the beginning and end of stmmac_mac_link_down(), is that intentional? I'm still curious if any of the netdev folks have any opinion on scaling things down like this on link down.
> I'm still curious if any of the netdev folks have any opinion on scaling > things down like this on link down. It does make sense, in that there are no frames to process, so the clock can be reduced. But i also think it is a bit of a workaround for poor hardware design. Often you can tell the MAC the link is down, and it can shut down a lot more, and even turn all the clocks off. I also wounder if there are going to be any side effects of this. Some Ethernet MACs export a clock to the PHY. Is that clock going to change? I don't think it will, because we are changing to a valid MAC speed, not 0. So the PHY has to work at this speed clock. But to make it easier to find issues like this, open() should probably set the clocks to a low speed until the link is up. That way, if there are going to be problems, the link should never come up, as opposed to the link never comes up after being lost the first time... Andrew
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index ec7c61ee44d4..f0166f0bc25f 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -996,6 +996,9 @@ static void stmmac_mac_link_down(struct phylink_config *config, { struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev)); + if (priv->plat->fix_mac_speed) + priv->plat->fix_mac_speed(priv->plat->bsp_priv, SPEED_10, mode); + stmmac_mac_set(priv, priv->ioaddr, false); priv->eee_active = false; priv->tx_lpi_enabled = false; @@ -1004,6 +1007,11 @@ static void stmmac_mac_link_down(struct phylink_config *config, if (priv->dma_cap.fpesel) stmmac_fpe_link_state_handle(priv, false); + + stmmac_set_icc_bw(priv, SPEED_10); + + if (priv->plat->fix_mac_speed) + priv->plat->fix_mac_speed(priv->plat->bsp_priv, SPEED_10, mode); } static void stmmac_mac_link_up(struct phylink_config *config,
When mac link goes down we don't need to mainitain the clocks to operate at higher frequencies, as an optimized solution to save power when the link goes down we are trying to bring down the clocks to the frequencies corresponding to the lowest speed possible. Signed-off-by: Sagar Cheluvegowda <quic_scheluve@quicinc.com> --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++++++++ 1 file changed, 8 insertions(+)