Re: [PATCH net-next 5/6] net: stmmac: move PHY handling out of __stmmac_open()/release()

Russell King (Oracle)
2026-04-16T13:48:13Z

On Thu, Apr 16, 2026 at 01:13:00PM +0100, Russell King (Oracle) wrote:
> On Thu, Apr 16, 2026 at 02:02:53PM +0200, Alexander Stein wrote:
> > Hi Russel,
> >
> > Am Donnerstag, 16. April 2026, 12:49:25 CEST schrieb Russell King (Oracle):
> > > On Thu, Apr 16, 2026 at 08:20:13AM +0200, Alexander Stein wrote:
> > > > Am Mittwoch, 15. April 2026, 14:59:32 CEST schrieb Russell King (Oracle):
> > > > > On Wed, Apr 15, 2026 at 08:08:40AM +0200, Alexander Stein wrote:
> > > > > > Hi,
> > > > > >
> > > > > > Am Dienstag, 23. September 2025, 13:26:19 CEST schrieb Russell King (Oracle):
> > > > > > > Move the PHY attachment/detachment from the network driver out of
> > > > > > > __stmmac_open() and __stmmac_release() into stmmac_open() and
> > > > > > > stmmac_release() where these actions will only happen when the
> > > > > > > interface is administratively brought up or down. It does not make
> > > > > > > sense to detach and re-attach the PHY during a change of MTU.
> > > > > >
> > > > > > Sorry for coming up now. But I recently noticed this commit breaks changing
> > > > > > the MTU on i.MX8MP. Once I simply change the MTU I run into some DMA error:
> > > > > > $ ip link set dev end1 mtu 1400
> > > > > > imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-0
> > > > > > imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-1
> > > > > > imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-2
> > > > > > imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-3
> > > > > > imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-4
> > > > > > imx-dwmac 30bf0000.ethernet end1: Link is Down
> > > > > > imx-dwmac 30bf0000.ethernet end1: Failed to reset the dma
> > > > > > imx-dwmac 30bf0000.ethernet end1: stmmac_hw_setup: DMA engine initialization failed
> > > > >
> > > > > This basically means that a clock is missing. Please provide more
> > > > > information:
> > > > >
> > > > > - what kernel version are you using?
> > > >
> > > > Currently I am using v6.18.22.
> > > > $ ethtool -i end1
> > > > driver: st_gmac
> > > > version: 6.18.22
> > > > firmware-version:
> > > > expansion-rom-version:
> > > > bus-info: 30bf0000.ethernet
> > > > supports-statistics: yes
> > > > supports-test: no
> > > > supports-eeprom-access: no
> > > > supports-register-dump: yes
> > > > supports-priv-flags: no
> > > >
> > > > > - has EEE been negotiated?
> > > >
> > > > No. It is marked as not supported
> > > >
> > > > $ ethtool --show-eee end1
> > > > EEE settings for end1:
> > > > EEE status: not supported
> > > >
> > > > > - does the problem persist when EEE is disabled?
> > > >
> > > > As EEE is not supported the problem occurs even with EEE disabled.
> > > >
> > > > > - which PHY is attached to stmmac?
> > > >
> > > > It is a TI DP83867.
> > > >
> > > > imx-dwmac 30bf0000.ethernet eth1: PHY [stmmac-1:03] driver [TI DP83867] (irq=136)
> > > >
> > > > > - which PHY interface mode is being used to connect the PHY to stmmac?
> > > >
> > > > For this interface
> > > > > phy-mode = "rgmii-id";
> > > > is set.
> > > >
> > > > In case it is helpful. My platform is arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl.dts
> > > > Thanks for assisting. If there a further questions, don't hesitate to ask.
> > >
> > > Thanks.
> > >
> > > So, as best I can determine at the moment, we end up with the following
> > > sequence:
> > >
> > > stmmac_change_mtu()
> > > __stmmac_release()
> > > phylink_stop()
> > > phy_stop()
> > > phy->state = PHY_HALTED
> > > _phy_state_machine() returns PHY_STATE_WORK_SUSPEND
> > > _phy_state_machine_post_work()
> > > phy_suspend()
> > > genphy_suspend()
> > > phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN)
> > >
> > > With the DP83867, this causes most of the PHY to be powered down, thus
> > > stopping the clocks, and this causes the stmmac reset to time out.
> > >
> > > Prior to this commit, we would have called phylink_disconnect_phy()
> > > immediately after phylink_stop(), but I can see nothing that would
> > > be affected by this change there (since that also calls
> > > phy_suspend(), but as the PHY is already suspended, this becomes a
> > > no-op.)
> > >
> > > However, __stmmac_open() would have called stmmac_init_phy(), which
> > > would reattach the PHY. This would have called phy_init_hw(),
> > > resetting the PHY, and phy_resume() which would ensure that the
> > > PDOWN bit is clear - thus clocks would be running.
> > >
> > > As a hack, please can you try calling phylink_prepare_resume()
> > > between the __stmmac_release() and __stmmac_open() in
> > > stmmac_change_mtu(). This should resume the PHY, thus restoring the
> > > clocks necessary for stmmac to reset.
> >
> > I tried the following patch. This works as you suspected.
>
> Brilliant, thanks for proving the theory why it broke.
>
> I'll have a think about the best way to solve this, because
> phylink_prepare_resume() is supposed to be paired with phylink_resume()
> and that isn't the case here.
>
> Please bear with me as my availability for looking at the kernel is
> very unpredictable at present (family health issues.)

I have some patches which passed build testing, but more chaos means
I can't post them nor test them. I'll do something when I'm next able
to, whenever that will be.

The next problem will be netdev's policy over reviews vs patches
balance which I'm already in deficit, and I have *NO* *TIME*
what so ever to review patches - let alone propose patches to
fix people's problems.

So I'm going to say this plainly: if netdev wants to enforce that
rule, then I won't be fixing people's problems.

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