netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Emanuele Ghidoli <ghidoliemanuele@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Emanuele Ghidoli <emanuele.ghidoli@toradex.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH v1] net: phy: dp83867: Disable EEE support as not implemented
Date: Sat, 25 Oct 2025 10:37:59 +0100	[thread overview]
Message-ID: <aPyad0PUR9lY70rk@shell.armlinux.org.uk> (raw)
In-Reply-To: <aPyWHRphEYOdk2MG@pengutronix.de>

On Sat, Oct 25, 2025 at 11:19:25AM +0200, Oleksij Rempel wrote:
> On Sat, Oct 25, 2025 at 09:44:31AM +0100, Russell King (Oracle) wrote:
> > On Thu, Oct 23, 2025 at 04:48:53PM +0200, Emanuele Ghidoli wrote:
> > > While the DP83867 PHYs report EEE capability through their feature
> > > registers, the actual hardware does not support EEE (see Links).
> > > When the connected MAC enables EEE, it causes link instability and
> > > communication failures.
> > > 
> > > The issue is reproducible with a iMX8MP and relevant stmmac ethernet port.
> > > Since the introduction of phylink-managed EEE support in the stmmac driver,
> > > EEE is now enabled by default, leading to issues on systems using the
> > > DP83867 PHY.
> > 
> > Wasn't it enabled before? See commit 4218647d4556 ("net: stmmac:
> > convert to phylink managed EEE support").
> > 
> > stmmac's mac_link_up() was:
> > 
> > -       if (phy && priv->dma_cap.eee) {
> > -               phy_eee_rx_clock_stop(phy, !(priv->plat->flags &
> > -                                            STMMAC_FLAG_RX_CLK_RUNS_IN_LPI));
> > -               priv->tx_lpi_timer = phy->eee_cfg.tx_lpi_timer;
> > -               stmmac_eee_init(priv, phy->enable_tx_lpi);
> >                 stmmac_set_eee_pls(priv, priv->hw, true);
> > -       }
> > 
> > So, if EEE is enabled in the core synthesis, then EEE will be
> > configured depending on what phylib says.
> > 
> > In stmmac_init_phy(), there was this:
> > 
> > -               if (priv->dma_cap.eee)
> > -                       phy_support_eee(phydev);
> > -
> >                 ret = phylink_connect_phy(priv->phylink, phydev);
> > 
> > So phylib was told to enable EEE support on the PHY if the dwmac
> > core supports EEE.
> > 
> > So, from what I can see, converting to phylink managed EEE didn't
> > change this. So what really did change?
> 
> I suspect it is a change in board designs. iMX8MP EVB variants are using
> Realtek PHYs with the SmartEEE variant. Therefore, the MAC is not able
> to control LPI behavior. Designs based on the EVB design (with the
> Realtek PHY) are not affected. I mean, any bug on the MAC or software
> side will stay invisible.
> 
> Some new designs with special requirements for TSN, for example
> low-latency TI PHYs, are a different story. They promise "Extra low
> latency TX < 90ns, RX < 290ns" and also announce EEE support. These two
> promises are not compatible with each other anyway, and at the same
> time, even if LPI does work, it will most probably fail with the FEC
> driver. I do not know about STMMAC.

What's annoying me is this "we spotted a change in the driver, we're
going to blame that for our problems" attitude that there seems to be
towards phylink.

When I make changes such as when porting a driver to a new facility,
I try to do it with _no_ behavioural change. Yet, people still blame
phylink for their problems. In 99% of cases, it turns out to be
incorrect blame.

This commit description is stating that the conversion of stmmac to
phylink-managed EEE changed the behaviour to default to enabling EEE.
I claim that the driver _already_ defaulted to enabling EEE. So
the commit description is nonsense, and just pulling at straws to
justify the probem.

What I'm asking for is people to _properly_ investigate their problems
rather than just looking at the commit history, and pulling out some
random commit to blame, which invariably seems to be phylink related.

Having one's hard efforts constantly slated in this way is unhelpful.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

  reply	other threads:[~2025-10-25  9:38 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-23 14:48 [PATCH v1] net: phy: dp83867: Disable EEE support as not implemented Emanuele Ghidoli
2025-10-23 17:01 ` Andrew Lunn
2025-10-25  2:20 ` patchwork-bot+netdevbpf
2025-10-25  8:44 ` Russell King (Oracle)
2025-10-25  9:19   ` Oleksij Rempel
2025-10-25  9:37     ` Russell King (Oracle) [this message]
2025-10-26 23:45 ` Andrew Lunn
2025-10-27 12:57   ` Emanuele Ghidoli
2025-10-27 13:25     ` Andrew Lunn
2025-10-27 13:57       ` Oleksij Rempel
2025-10-27 14:35       ` Francesco Dolcini
2025-10-27 14:41         ` Andrew Lunn
2025-10-27 14:53     ` Russell King (Oracle)
2025-10-27 15:34       ` Emanuele Ghidoli
2025-10-27 16:44         ` Russell King (Oracle)
2025-10-27 17:23           ` Russell King (Oracle)
2025-10-27 19:26           ` Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aPyad0PUR9lY70rk@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=emanuele.ghidoli@toradex.com \
    --cc=ghidoliemanuele@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).