All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Jon Hunter <jonathanh@nvidia.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	Bryan Whitehead <bryan.whitehead@microchip.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-stm32@st-md-mailman.stormreply.com,
	Marcin Wojtas <marcin.s.wojtas@gmail.com>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	netdev@vger.kernel.org, Paolo Abeni <pabeni@redhat.com>,
	UNGLinuxDriver@microchip.com,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>
Subject: Re: [PATCH net-next 9/9] net: stmmac: convert to phylink managed EEE support
Date: Wed, 19 Feb 2025 19:13:30 +0000	[thread overview]
Message-ID: <Z7YtWmkVl0rWFvQO@shell.armlinux.org.uk> (raw)
In-Reply-To: <203871c2-c673-4a98-a0a3-299d1cf71cf0@nvidia.com>

On Wed, Feb 19, 2025 at 05:52:34PM +0000, Jon Hunter wrote:
> On 19/02/2025 15:36, Russell King (Oracle) wrote:
> > So clearly the phylink resolver is racing with the rest of the stmmac
> > resume path - which doesn't surprise me in the least. I believe I raised
> > the fact that calling phylink_resume() before the hardware was ready to
> > handle link-up is a bad idea precisely because of races like this.
> > 
> > The reason stmmac does this is because of it's quirk that it needs the
> > receive clock from the PHY in order for stmmac_reset() to work.
> 
> I do see the reset fail infrequently on previous kernels with this device
> and when it does I see these messages ...
> 
>  dwc-eth-dwmac 2490000.ethernet: Failed to reset the dma
>  dwc-eth-dwmac 2490000.ethernet eth0: stmmac_hw_setup: DMA engine
>   initialization failed

I wonder whether it's also racing with phylib, but phylink_resume()
calling phylink_start() going in to call phy_start() is all synchronous.
That causes __phy_resume() to be called.

Which PHY device/driver is being used?

> > So, my preference would be to move phylink_resume() later, removing
> > the race condition. If there's any regressions, then we need to
> > _properly_ solve them by ensuring that the PHY keeps the RX clock
> > running by honouring PHY_F_RXC_ALWAYS_ON. That's going to need
> > everyone to test their stmmac platforms to find all the cases that
> > need fixing...
> 
> Thanks for the in-depth analysis and feedback. We have 3 SoCs that use this
> driver and so I will do some testing with this change on all of them.

Thanks!

-- 
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-02-19 19:27 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-15 20:42 [PATCH net-next 0/9] net: add phylink managed EEE support Russell King (Oracle)
2025-01-15 20:42 ` [PATCH net-next 1/9] net: mdio: add definition for clock stop capable bit Russell King (Oracle)
2025-01-15 20:42 ` [PATCH net-next 2/9] net: phy: add support for querying PHY clock stop capability Russell King (Oracle)
2025-01-15 20:42 ` [PATCH net-next 3/9] net: phylink: add phylink_link_is_up() helper Russell King (Oracle)
2025-01-15 20:42 ` [PATCH net-next 4/9] net: phylink: add EEE management Russell King (Oracle)
2025-01-15 20:42 ` [PATCH net-next 5/9] net: mvneta: convert to phylink EEE implementation Russell King (Oracle)
2025-01-15 20:42 ` [PATCH net-next 6/9] net: mvpp2: add " Russell King (Oracle)
2025-01-16  8:27   ` Maxime Chevallier
2025-01-15 20:42 ` [PATCH net-next 7/9] net: lan743x: use netdev in lan743x_phylink_mac_link_down() Russell King (Oracle)
2025-01-15 20:43 ` [PATCH net-next 8/9] net: lan743x: convert to phylink managed EEE Russell King (Oracle)
2025-01-15 20:43 ` [PATCH net-next 9/9] net: stmmac: convert to phylink managed EEE support Russell King (Oracle)
2025-02-13 11:05   ` Jon Hunter
2025-02-13 11:37     ` Russell King (Oracle)
2025-02-13 12:00       ` Russell King (Oracle)
2025-02-14 10:58         ` Jon Hunter
2025-02-14 11:21           ` Russell King (Oracle)
2025-02-14 17:03             ` Jon Hunter
2025-02-19 14:01             ` Jon Hunter
2025-02-19 15:36               ` Russell King (Oracle)
2025-02-19 17:52                 ` Jon Hunter
2025-02-19 19:13                   ` Russell King (Oracle) [this message]
2025-02-19 20:05                     ` Jon Hunter
2025-02-19 20:57                       ` Russell King (Oracle)
2025-02-25 14:21                         ` Jon Hunter
2025-02-26 10:02                           ` Russell King (Oracle)
2025-02-26 10:11                             ` Jon Hunter
2025-02-26 10:59                               ` Russell King (Oracle)
2025-02-26 15:55                                 ` Jon Hunter
2025-02-26 16:00                                   ` Russell King (Oracle)
2025-02-26 16:06                                     ` Jon Hunter
2025-02-26 11:37                               ` Russell King (Oracle)
2025-02-26 17:24                                 ` Jon Hunter
2025-01-16  0:40 ` [PATCH net-next 0/9] net: add " Jacob Keller
2025-01-17  1:40 ` patchwork-bot+netdevbpf
2025-01-17  8:56 ` Jiawen Wu
2025-01-17  9:05   ` Russell King (Oracle)
2025-01-17 10:17     ` Jiawen Wu
2025-01-17 12:23       ` Russell King (Oracle)
2025-01-20  1:51         ` Jiawen Wu
2025-01-20  9:54           ` Russell King (Oracle)
2025-01-20  9:59             ` Jiawen Wu

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=Z7YtWmkVl0rWFvQO@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=alexandre.torgue@foss.st.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=andrew@lunn.ch \
    --cc=bryan.whitehead@microchip.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=jonathanh@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=linux-tegra@vger.kernel.org \
    --cc=marcin.s.wojtas@gmail.com \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.