| Message ID | E1tpt2t-005UNB-MC@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: stmmac: approach 2 to solve EEE LPI reset issues | expand |
Hello Russell, On Wed, 05 Mar 2025 18:00:39 +0000 "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk> wrote: > Some MACs require the PHY receive clock to be running to complete setup > actions. This may fail if the PHY has negotiated EEE, the MAC supports > receive clock stop, and the link has entered LPI state. Provide a pair > of APIs that MAC drivers can use to temporarily block the PHY disabling > the receive clock. > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> I only have comments on the implementation, see below :) > --- > drivers/net/phy/phylink.c | 50 +++++++++++++++++++++++++++++++++++++++ > include/linux/phylink.h | 3 +++ > 2 files changed, 53 insertions(+) > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > index a3b186ab3854..8f93b597d019 100644 > --- a/drivers/net/phy/phylink.c > +++ b/drivers/net/phy/phylink.c > @@ -88,6 +88,7 @@ struct phylink { > bool mac_enable_tx_lpi; > bool mac_tx_clk_stop; > u32 mac_tx_lpi_timer; > + u8 mac_rx_clk_stop_blocked; > > struct sfp_bus *sfp_bus; > bool sfp_may_have_phy; > @@ -2592,6 +2593,55 @@ void phylink_stop(struct phylink *pl) > } > EXPORT_SYMBOL_GPL(phylink_stop); > > + > +void phylink_rx_clk_stop_block(struct phylink *pl) > +{ > + ASSERT_RTNL(); > + > + if (pl->mac_rx_clk_stop_blocked == U8_MAX) { > + phylink_warn(pl, "%s called too many times - ignoring\n", > + __func__); > + dump_stack(); > + return; > + } > + > + /* Disable PHY receive clock stop if this is the first time this > + * function has been called and clock-stop was previously enabled. > + */ > + if (pl->mac_rx_clk_stop_blocked++ == 0 && > + pl->mac_supports_eee_ops && pl->phydev) > + pl->config->eee_rx_clk_stop_enable) Looks like there's an extra closing ')' here > + phy_eee_rx_clock_stop(pl->phydev, false); > +} Do you need an EXPORT_SYMBOL_GPL here as this will be used by MAC drivers? > + > +/** > + * phylink_rx_clk_stop_unblock() - unblock PHY ability to stop receive clock > + * @pl: a pointer to a &struct phylink returned from phylink_create() > + * > + * All calls to phylink_rx_clk_stop_block() must be balanced with a > + * corresponding call to phylink_rx_clk_stop_unblock() to restore the PHYs > + * clock stop ability. > + */ > +void phylink_rx_clk_stop_unblock(struct phylink *pl) > +{ > + ASSERT_RTNL(); > + > + if (pl->mac_rx_clk_stop_blocked == 0) { > + phylink_warn(pl, "%s called too many times - ignoring\n", > + __func__); > + dump_stack(); > + return; > + } > + > + /* Re-enable PHY receive clock stop if the number of unblocks matches > + * the number of calls to the block function above. > + */ > + if (--pl->mac_rx_clk_stop_blocked == 0 && > + pl->mac_supports_eee_ops && pl->phydev && > + pl->config->eee_rx_clk_stop_enable) > + phy_eee_rx_clock_stop(pl->phydev, true); > +} Same for the EXPORT_SYMBOL_GPL Thanks, Maxime
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index a3b186ab3854..8f93b597d019 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -88,6 +88,7 @@ struct phylink { bool mac_enable_tx_lpi; bool mac_tx_clk_stop; u32 mac_tx_lpi_timer; + u8 mac_rx_clk_stop_blocked; struct sfp_bus *sfp_bus; bool sfp_may_have_phy; @@ -2592,6 +2593,55 @@ void phylink_stop(struct phylink *pl) } EXPORT_SYMBOL_GPL(phylink_stop); + +void phylink_rx_clk_stop_block(struct phylink *pl) +{ + ASSERT_RTNL(); + + if (pl->mac_rx_clk_stop_blocked == U8_MAX) { + phylink_warn(pl, "%s called too many times - ignoring\n", + __func__); + dump_stack(); + return; + } + + /* Disable PHY receive clock stop if this is the first time this + * function has been called and clock-stop was previously enabled. + */ + if (pl->mac_rx_clk_stop_blocked++ == 0 && + pl->mac_supports_eee_ops && pl->phydev) + pl->config->eee_rx_clk_stop_enable) + phy_eee_rx_clock_stop(pl->phydev, false); +} + +/** + * phylink_rx_clk_stop_unblock() - unblock PHY ability to stop receive clock + * @pl: a pointer to a &struct phylink returned from phylink_create() + * + * All calls to phylink_rx_clk_stop_block() must be balanced with a + * corresponding call to phylink_rx_clk_stop_unblock() to restore the PHYs + * clock stop ability. + */ +void phylink_rx_clk_stop_unblock(struct phylink *pl) +{ + ASSERT_RTNL(); + + if (pl->mac_rx_clk_stop_blocked == 0) { + phylink_warn(pl, "%s called too many times - ignoring\n", + __func__); + dump_stack(); + return; + } + + /* Re-enable PHY receive clock stop if the number of unblocks matches + * the number of calls to the block function above. + */ + if (--pl->mac_rx_clk_stop_blocked == 0 && + pl->mac_supports_eee_ops && pl->phydev && + pl->config->eee_rx_clk_stop_enable) + phy_eee_rx_clock_stop(pl->phydev, true); +} + /** * phylink_suspend() - handle a network device suspend event * @pl: a pointer to a &struct phylink returned from phylink_create() diff --git a/include/linux/phylink.h b/include/linux/phylink.h index 08df65f6867a..249c437d6b7b 100644 --- a/include/linux/phylink.h +++ b/include/linux/phylink.h @@ -698,6 +698,9 @@ int phylink_pcs_pre_init(struct phylink *pl, struct phylink_pcs *pcs); void phylink_start(struct phylink *); void phylink_stop(struct phylink *); +void phylink_rx_clk_stop_block(struct phylink *); +void phylink_rx_clk_stop_unblock(struct phylink *); + void phylink_suspend(struct phylink *pl, bool mac_wol); void phylink_resume(struct phylink *pl);
Some MACs require the PHY receive clock to be running to complete setup actions. This may fail if the PHY has negotiated EEE, the MAC supports receive clock stop, and the link has entered LPI state. Provide a pair of APIs that MAC drivers can use to temporarily block the PHY disabling the receive clock. Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- drivers/net/phy/phylink.c | 50 +++++++++++++++++++++++++++++++++++++++ include/linux/phylink.h | 3 +++ 2 files changed, 53 insertions(+)