[PATCH RFC net-next 02/23] net: phy: fix phy_ethtool_set_eee() incorrectly enabling LPI
Oleksij Rempel
o.rempel at pengutronix.de
Wed Nov 27 04:20:09 PST 2024
On Wed, Nov 27, 2024 at 11:12:28AM +0000, Russell King (Oracle) wrote:
> On Tue, Nov 26, 2024 at 12:52:21PM +0000, Russell King (Oracle) wrote:
> > @@ -1685,15 +1685,21 @@ EXPORT_SYMBOL(phy_ethtool_get_eee);
> > static void phy_ethtool_set_eee_noneg(struct phy_device *phydev,
> > const struct eee_config *old_cfg)
> > {
> > - if (phydev->eee_cfg.tx_lpi_enabled != old_cfg->tx_lpi_enabled ||
> > + bool enable_tx_lpi;
> > +
> > + if (!phydev->link)
> > + return;
> > +
> > + enable_tx_lpi = phydev->eee_cfg.tx_lpi_enabled && phydev->eee_active;
> > +
> > + if (phydev->enable_tx_lpi != enable_tx_lpi ||
> > phydev->eee_cfg.tx_lpi_timer != old_cfg->tx_lpi_timer) {
>
> I'm wondering whether this should be:
>
> if (phydev->enable_tx_lpi != enable_tx_lpi ||
> (phydev->enable_tx_lpi &&
> phydev->eee_cfg.tx_lpi_timer != old_cfg->tx_lpi_timer)) {
>
> The argument for this change would be to avoid cycling the link when the
> LPI timer changes but we're not using LPI.
>
> The argument against this change would be that then we don't program the
> hardware, and if the driver reads the initial value from hardware and
> is unbound/rebound, we'll lose that update whereas before the phylib
> changes, it would have been preserved.
>
> The problem, however, are drivers where the LPI timer is dependent on
> the speed.
>
> Any thoughts?
So far, i was not able to find any. But, like I said before, to get
optimal performance and power saving balance, the lpi_timer should be
link speed specific... at least, some day with appropriate user
interface.
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
More information about the linux-arm-kernel
mailing list