drivers/net/ethernet/freescale/fec_main.c | 41 ----------------------- drivers/net/phy/phy_device.c | 27 ++------------- include/linux/phy.h | 1 - 3 files changed, 3 insertions(+), 66 deletions(-)
The current implementation of phy_reset_after_clk_enable requires MAC drivers
(like fec_main.c) to manually track PHY probing states and trigger resets
at specific points in their clock management flow and was created just for a SoC
vendor. This leads to fragile code in the Ethernet driver, involving complex
checks to see if the PHY device is bound.
This patch proposes moving the handling of the PHY_RST_AFTER_CLK_EN flag directly
into the generic phy_init_hw() function.
Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
I need to test better some boards that need this reset, but
I need to confirm if the general approach is ok
---
drivers/net/ethernet/freescale/fec_main.c | 41 -----------------------
drivers/net/phy/phy_device.c | 27 ++-------------
include/linux/phy.h | 1 -
3 files changed, 3 insertions(+), 66 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index e2b75d1970ae..ea112385e77e 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2364,28 +2364,6 @@ static int fec_enet_mdio_write_c45(struct mii_bus *bus, int mii_id,
return ret;
}
-static void fec_enet_phy_reset_after_clk_enable(struct net_device *ndev)
-{
- struct fec_enet_private *fep = netdev_priv(ndev);
- struct phy_device *phy_dev = ndev->phydev;
-
- if (phy_dev) {
- phy_reset_after_clk_enable(phy_dev);
- } else if (fep->phy_node) {
- /*
- * If the PHY still is not bound to the MAC, but there is
- * OF PHY node and a matching PHY device instance already,
- * use the OF PHY node to obtain the PHY device instance,
- * and then use that PHY device instance when triggering
- * the PHY reset.
- */
- phy_dev = of_phy_find_device(fep->phy_node);
- phy_reset_after_clk_enable(phy_dev);
- if (phy_dev)
- put_device(&phy_dev->mdio.dev);
- }
-}
-
static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
{
struct fec_enet_private *fep = netdev_priv(ndev);
@@ -2416,7 +2394,6 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
if (ret)
goto failed_clk_2x_txclk;
- fec_enet_phy_reset_after_clk_enable(ndev);
} else {
clk_disable_unprepare(fep->clk_enet_out);
if (fep->clk_ptp) {
@@ -3554,7 +3531,6 @@ fec_enet_open(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
int ret;
- bool reset_again;
ret = pm_runtime_resume_and_get(&fep->pdev->dev);
if (ret < 0)
@@ -3565,17 +3541,6 @@ fec_enet_open(struct net_device *ndev)
if (ret)
goto clk_enable;
- /* During the first fec_enet_open call the PHY isn't probed at this
- * point. Therefore the phy_reset_after_clk_enable() call within
- * fec_enet_clk_enable() fails. As we need this reset in order to be
- * sure the PHY is working correctly we check if we need to reset again
- * later when the PHY is probed
- */
- if (ndev->phydev && ndev->phydev->drv)
- reset_again = false;
- else
- reset_again = true;
-
/* I should reset the ring buffers here, but I don't yet know
* a simple way to do that.
*/
@@ -3587,12 +3552,6 @@ fec_enet_open(struct net_device *ndev)
/* Init MAC prior to mii bus probe */
fec_restart(ndev);
- /* Call phy_reset_after_clk_enable() again if it failed during
- * phy_reset_after_clk_enable() before because the PHY wasn't probed.
- */
- if (reset_again)
- fec_enet_phy_reset_after_clk_enable(ndev);
-
/* Probe and connect to PHY when open the interface */
ret = fec_enet_mii_probe(ndev);
if (ret)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 7a67c900e79a..3af652cbd7d2 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1376,6 +1376,9 @@ int phy_init_hw(struct phy_device *phydev)
{
int ret = 0;
+ if (phydev->drv->flags & PHY_RST_AFTER_CLK_EN)
+ phy_device_reset(phydev, 1);
+
/* Deassert the reset signal */
phy_device_reset(phydev, 0);
@@ -1956,30 +1959,6 @@ int phy_resume(struct phy_device *phydev)
}
EXPORT_SYMBOL(phy_resume);
-/**
- * phy_reset_after_clk_enable - perform a PHY reset if needed
- * @phydev: target phy_device struct
- *
- * Description: Some PHYs are known to need a reset after their refclk was
- * enabled. This function evaluates the flags and perform the reset if it's
- * needed. Returns < 0 on error, 0 if the phy wasn't reset and 1 if the phy
- * was reset.
- */
-int phy_reset_after_clk_enable(struct phy_device *phydev)
-{
- if (!phydev || !phydev->drv)
- return -ENODEV;
-
- if (phydev->drv->flags & PHY_RST_AFTER_CLK_EN) {
- phy_device_reset(phydev, 1);
- phy_device_reset(phydev, 0);
- return 1;
- }
-
- return 0;
-}
-EXPORT_SYMBOL(phy_reset_after_clk_enable);
-
/* Generic PHY support and helper functions */
/**
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 0bc00a4cceb2..d843415e65a6 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1876,7 +1876,6 @@ int phy_speed_up(struct phy_device *phydev);
bool phy_check_valid(int speed, int duplex, unsigned long *features);
int phy_restart_aneg(struct phy_device *phydev);
-int phy_reset_after_clk_enable(struct phy_device *phydev);
#if IS_ENABLED(CONFIG_PHYLIB)
int phy_start_cable_test(struct phy_device *phydev,
--
2.51.0On Wed, Jan 28, 2026 at 10:46:41AM +0100, Michael Trimarchi wrote:
> The current implementation of phy_reset_after_clk_enable requires MAC drivers
> (like fec_main.c) to manually track PHY probing states and trigger resets
> at specific points in their clock management flow and was created just for a SoC
> vendor.
We try to keep workarounds for specific SoC vendors out of the core,
when possible. It just makes the core more complex for an edge case
which most people don't care about. It is better to hide the
complexity away in the driver which needs it.
> This leads to fragile code in the Ethernet driver, involving complex
> checks to see if the PHY device is bound.
Have you found an actual issue with the code in the driver?
Andrew
Hi Andrew On Wed, Jan 28, 2026 at 2:25 PM Andrew Lunn <andrew@lunn.ch> wrote: > > On Wed, Jan 28, 2026 at 10:46:41AM +0100, Michael Trimarchi wrote: > > The current implementation of phy_reset_after_clk_enable requires MAC drivers > > (like fec_main.c) to manually track PHY probing states and trigger resets > > at specific points in their clock management flow and was created just for a SoC > > vendor. > > We try to keep workarounds for specific SoC vendors out of the core, > when possible. It just makes the core more complex for an edge case > which most people don't care about. It is better to hide the > complexity away in the driver which needs it. We should not merge things in the core that are for one Soc and after d65af21842f8a7821fc3b28dc043c2f92b2b312c, I need to anyway to do: net: phy: smsc: Fix functionality of lan8710 in combination with NXP fec Revert "net: phy: smsc: LAN8710/20: remove PHY_RST_AFTER_CLK_EN flag" d65af21842 The way the reset is sent using phy library is not compatible with the phy reset logic. For now we need to revert this patch to all the phy to go back working Change-Id: Ic1b0fde04188c7cbfd8c94f9fa1a26d058e07d1d Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> > > > This leads to fragile code in the Ethernet driver, involving complex > > checks to see if the PHY device is bound. > > Have you found an actual issue with the code in the driver? Yes, and I think that the flag should be used properly in the state machine of the phy and not in the fec. Is possible that is the only one SoC has this problem? Michael > > Andrew -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 michael@amarulasolutions.com __________________________________ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 info@amarulasolutions.com www.amarulasolutions.com
On Wed, Jan 28, 2026 at 02:33:24PM +0100, Michael Nazzareno Trimarchi wrote: > Hi Andrew > > On Wed, Jan 28, 2026 at 2:25 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > On Wed, Jan 28, 2026 at 10:46:41AM +0100, Michael Trimarchi wrote: > > > The current implementation of phy_reset_after_clk_enable requires MAC drivers > > > (like fec_main.c) to manually track PHY probing states and trigger resets > > > at specific points in their clock management flow and was created just for a SoC > > > vendor. > > > > We try to keep workarounds for specific SoC vendors out of the core, > > when possible. It just makes the core more complex for an edge case > > which most people don't care about. It is better to hide the > > complexity away in the driver which needs it. > > We should not merge things in the core that are for one Soc and > after d65af21842f8a7821fc3b28dc043c2f92b2b312c, I need to anyway to do: > > net: phy: smsc: Fix functionality of lan8710 in combination with NXP fec > > Revert "net: phy: smsc: LAN8710/20: remove PHY_RST_AFTER_CLK_EN flag" > d65af21842 That commit is an example of a poor commit description. It doesn't describe the problem, nor which hardware this affects, so when we end up with this kind of situation, we're lacking the context behind why the change was made. So, how do we know that reverting that commit is not going to cause a regression for whatever platform the patch was proposed for? If you read the preceeding commit (which is referenced by the commit you intend to revert) itgives more information about the problem, and I would expect that reverting it will cause a regression with interrupts. Adding Marco to this thread for comment. Also, maybe you could clearly state what the issue that you are trying to address - why do you need special reset handling for the combination of FEC + this PHY. Consider including that description in the commit message for any proposed solution so that - as is the case here looking back at Macro's commit - we can see _why_ the change is being made. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On 26-01-28, Russell King (Oracle) wrote: > On Wed, Jan 28, 2026 at 02:33:24PM +0100, Michael Nazzareno Trimarchi wrote: > > Hi Andrew > > > > On Wed, Jan 28, 2026 at 2:25 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > On Wed, Jan 28, 2026 at 10:46:41AM +0100, Michael Trimarchi wrote: > > > > The current implementation of phy_reset_after_clk_enable requires MAC drivers > > > > (like fec_main.c) to manually track PHY probing states and trigger resets > > > > at specific points in their clock management flow and was created just for a SoC > > > > vendor. > > > > > > We try to keep workarounds for specific SoC vendors out of the core, > > > when possible. It just makes the core more complex for an edge case > > > which most people don't care about. It is better to hide the > > > complexity away in the driver which needs it. > > > > We should not merge things in the core that are for one Soc and > > after d65af21842f8a7821fc3b28dc043c2f92b2b312c, I need to anyway to do: > > > > net: phy: smsc: Fix functionality of lan8710 in combination with NXP fec > > > > Revert "net: phy: smsc: LAN8710/20: remove PHY_RST_AFTER_CLK_EN flag" > > d65af21842 > > That commit is an example of a poor commit description. It doesn't > describe the problem, nor which hardware this affects, so when we end > up with this kind of situation, we're lacking the context behind why > the change was made. I need to say that this commit lacks a bit of context which is part of my previous commit. I thought, that I made that clear with: | The same behaviour can be archived now by specifying the refclk. To be honest, I should have made it more clear. > So, how do we know that reverting that commit is not going to cause a > regression for whatever platform the patch was proposed for? If I get the Michael's patch correct, he tries to move the quirk handling into the core. I really don't know why Michael requires the speical reset handling at all. The commit adding the PHY_RST_AFTER_CLK_EN flag says: | phylib: add reset after clk enable support | | Some PHYs need the refclk to be a continuous clock. Therefore they don't | allow turning it off and on again during operation. Nonetheless such a | clock switching is performed by some ETH drivers (namely FEC [1]) for | power saving reasons. An example for an affected PHY is the | SMSC/Microchip LAN8720 in "REF_CLK In Mode". This can be achieved with commit bedd8d78aba300860cec3f85d6ff549b3b7f2679 if specified accordingly. > If you read the preceeding commit (which is referenced by the commit > you intend to revert) itgives more information about the problem, and > I would expect that reverting it will cause a regression with > interrupts. Yes, my bad I thought this reference is enough. > Adding Marco to this thread for comment. Thanks. The issue was with the out-of-band reset coming from the FEC driver which doesn't honor the phy state-machine. It shouldn't cause a regression (TM) if it would be part of the state-machine (aka this RFC patch) but it needs to be tested! As said above, I'm not sure why PHY_RST_AFTER_CLK_EN is required in the first place. > Also, maybe you could clearly state what the issue that you are trying > to address - why do you need special reset handling for the combination > of FEC + this PHY. Consider including that description in the commit > message for any proposed solution so that - as is the case here looking > back at Macro's commit - we can see _why_ the change is being made. With the context of the previous commit it should be clear. Regards, Marco > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! -- #gernperDu #CallMeByMyFirstName Pengutronix e.K. | | Steuerwalder Str. 21 | https://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
On Wed, Jan 28, 2026 at 09:26:34PM +0100, Marco Felsch wrote: > | Some PHYs need the refclk to be a continuous clock. Therefore they don't > | allow turning it off and on again during operation. Nonetheless such a > | clock switching is performed by some ETH drivers (namely FEC [1]) for > | power saving reasons. An example for an affected PHY is the > | SMSC/Microchip LAN8720 in "REF_CLK In Mode". > > This can be achieved with commit > bedd8d78aba300860cec3f85d6ff549b3b7f2679 if specified accordingly. I don't think this is unusual. I've just checked the 88E151x documentation, and that also requires: 10ms after power and 10 clock cycles on XTAL_IN before deasserting reset. The timing diagram indicates that subsequent hardware resets must be 10ms in duration, and show the XTAL_IN clock running during the reset. Looking at AR8035, it is similar - the clock must be running before releasing reset and while reset is asserted. So, to say that LAN8710 is somehow special seems wrong to me - it isn't just these PHYs that have the requirement to be clocked before reset is released nor while reset is asserted. I'm guessing the problem here is that, most cases the clock is provided by a crystal, in these cases it's being sourced from the SoC, and that puts the responsibility on the SoC parts to guarantee the reset timing required for the PHY in some way. In other words, by sourcing it from the SoC, it's not phylib nor the PHY driver's problem (the PHY driver comes along way too late) but the responsibility for the boot firmware / DT description / network driver to detect this situation and give the PHY what it requires to reset properly before registering the MDIO bus. The boot firmware route is how this is handled with SolidRun i.MX6 based boards - the PHY reset is a GPIO, but that GPIO is *not* told to the kernel except as a hog. u-boot takes care of setting up the 25MHz clock for the PHY and releasing its reset - it actually does two resets due to running off the ANATOP provided clock. This is an AR8035 PHY, which as I've mentioned above, requires the clock running before releasing reset, just like the LAN8710-like PHY that you're discussing here. Given all the complexity we've had with systems with two FECs, where the PHY for one is connected via the MDIO bus on the other FEC, it seems to me that pushing the clocking and reset handling into the kernel is just asking for lots of pain. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On 26-01-28, Russell King (Oracle) wrote:
> On Wed, Jan 28, 2026 at 09:26:34PM +0100, Marco Felsch wrote:
> > | Some PHYs need the refclk to be a continuous clock. Therefore they don't
> > | allow turning it off and on again during operation. Nonetheless such a
> > | clock switching is performed by some ETH drivers (namely FEC [1]) for
> > | power saving reasons. An example for an affected PHY is the
> > | SMSC/Microchip LAN8720 in "REF_CLK In Mode".
> >
> > This can be achieved with commit
> > bedd8d78aba300860cec3f85d6ff549b3b7f2679 if specified accordingly.
>
> I don't think this is unusual. I've just checked the 88E151x
> documentation, and that also requires: 10ms after power and 10 clock
> cycles on XTAL_IN before deasserting reset. The timing diagram indicates
> that subsequent hardware resets must be 10ms in duration, and show the
> XTAL_IN clock running during the reset.
>
> Looking at AR8035, it is similar - the clock must be running before
> releasing reset and while reset is asserted.
This is like every external IC or internal IP ;) What's special is the
phy generic compatible handling... (please see below)
> So, to say that LAN8710 is somehow special seems wrong to me - it isn't
> just these PHYs that have the requirement to be clocked before reset is
> released nor while reset is asserted.
IIRC the Linux phy framework does have this hard dependency, that a
phy needs to be accessible to make use of the generic phy compatible
probing to read-out the phy-id registers.
This requires that $someone e.g. the bootloader already did the phy
power-up sequencing for us so the phy-id can be read.
Once it comes to the FEC the situation is like this:
- The FEC can provide the PHY clock via ("enet_out"), no external
crystal required.
- If the phy clk is specified the FEC will turn on this clock during
the probe() and issues the MII bus probe incl. the phy probe().
- Afterwards the clock is gets disabled
My patch added the support to keep to request the clock during
phy.probe() and keep phy-clock running till the phy.remove() callback
gets called. Therefore no such local quirk like PHY_RST_AFTER_CLK_EN is
required.
> I'm guessing the problem here is that, most cases the clock is provided
> by a crystal, in these cases it's being sourced from the SoC, and that
> puts the responsibility on the SoC parts to guarantee the reset timing
> required for the PHY in some way.
This was exactly my use-case too and it's beeing addressed just fine
with no PHY_RST_AFTER_CLK_EN but the clock and reset specified within
the DTS.
> In other words, by sourcing it from the SoC, it's not phylib nor the
> PHY driver's problem (the PHY driver comes along way too late) but the
> responsibility for the boot firmware / DT description / network driver
> to detect this situation and give the PHY what it requires to reset
> properly before registering the MDIO bus.
Yes but I don't see a problem at all if everything was specified
accordingly.
> The boot firmware route is how this is handled with SolidRun i.MX6
> based boards - the PHY reset is a GPIO, but that GPIO is *not* told to
> the kernel except as a hog. u-boot takes care of setting up the 25MHz
> clock for the PHY and releasing its reset - it actually does two
> resets due to running off the ANATOP provided clock. This is an AR8035
> PHY, which as I've mentioned above, requires the clock running before
> releasing reset, just like the LAN8710-like PHY that you're discussing
> here.
Yes, it's very common that the bootloader does the initial work since
sometimes strapping pins are involved which need to be pulled too for
the reset sequence which are later on re-used for $other purpose.
As said, I don't see a problem with the LAN8710. But while checking the
Linux source code, I saw commit
57ec5a8735dc5dccd1ee68afdb1114956a3fce0d which re-introduced the reset
handling :-/
> Given all the complexity we've had with systems with two FECs, where
> the PHY for one is connected via the MDIO bus on the other FEC, it
> seems to me that pushing the clocking and reset handling into the
> kernel is just asking for lots of pain.
I wouldn't recommend such setups at all. It's like asking for troubles
;)
Regards,
Marco
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
>
--
#gernperDu
#CallMeByMyFirstName
Pengutronix e.K. | |
Steuerwalder Str. 21 | https://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
On Thu, Jan 29, 2026 at 11:41:23AM +0100, Marco Felsch wrote: > As said, I don't see a problem with the LAN8710. But while checking the > Linux source code, I saw commit > 57ec5a8735dc5dccd1ee68afdb1114956a3fce0d which re-introduced the reset > handling :-/ So now we get to the bottom of it. This is a regression, right? The addition of that patch has caused previously working boards to stop working, correct? That means it needs to be reverted. We also need to get out of our heads the idea that LAN8710A is somehow special in needing this. It isn't. Thus, there should be no special flags to "work around" this "problem". -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On 26-01-29, Russell King (Oracle) wrote: > On Thu, Jan 29, 2026 at 11:41:23AM +0100, Marco Felsch wrote: > > As said, I don't see a problem with the LAN8710. But while checking the > > Linux source code, I saw commit > > 57ec5a8735dc5dccd1ee68afdb1114956a3fce0d which re-introduced the reset > > handling :-/ > > So now we get to the bottom of it. This is a regression, right? The > addition of that patch has caused previously working boards to stop > working, correct? The commit is part of 6.17 and the system I've added the support for the clk handling is currently running on 6.14. I have to ask a colleague of mine to very this. But yes, it seems like a regression. > That means it needs to be reverted. Yes. > We also need to get out of our heads the idea that LAN8710A is somehow > special in needing this. It isn't. Thus, there should be no special > flags to "work around" this "problem". IMHO this flag should be removed completely because it makes the phy-driver mac-driver dependent. Since the FEC is the only driver checking this flag. Regards, Marco > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! > -- #gernperDu #CallMeByMyFirstName Pengutronix e.K. | | Steuerwalder Str. 21 | https://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
On Thu, Jan 29, 2026 at 12:17:26PM +0100, Marco Felsch wrote: > IMHO this flag should be removed completely because it makes the > phy-driver mac-driver dependent. Since the FEC is the only driver > checking this flag. From what I can see, the _only_ caller of phy_reset_after_clk_enable() is the FEC driver. However, we seem to have DP83848C, DP83848C, DP83620, TLK10X, LAN8710, LAN8720, LAN8740, and LAN8742 all needing this flag. As also pointed out, other PHYs also require their XTAL based clock to be running before reset is released. This reconfirms my statements that this is *not* a PHY issue, but an issue that afflicts FEC based systems because, on these systems, there seems to be a tendency for hardware designers to omit the PHY's crystal and source the PHY clock from the SoC. My guess is that this doesn't happen anywhere else (since no workaround is necessary.) So, this backs up my stance that "this PHY requires a special reset sequence" is incorrect. Hence, I think PHY_RST_AFTER_CLK_EN + phy_reset_after_clk_enable() should be entirely removed. Another possible solution beyond those I've already stated, given that this only afflicts the FEC driver, ould be for the FEC MDIO driver to walk the child nodes, checking to see whether they require any clocks, and ensuring that those are properly initialised before registering the MDIO bus. Since the MDIO bus layer can release the PHY reset just before probing the driver, this seems to me to be the only way to guarantee that, where boot firmware does not deal with this, the PHY manufacturers specification for the initial release of reset can be met while keeping this FEC specific behaviour out of the core MDIO/phylib layers. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Thu, Jan 29, 2026 at 01:12:41PM +0000, Russell King (Oracle) wrote: > Another possible solution beyond those I've already stated, given that > this only afflicts the FEC driver, ould be for the FEC MDIO driver to > walk the child nodes, checking to see whether they require any clocks, > and ensuring that those are properly initialised before registering the > MDIO bus. > > Since the MDIO bus layer can release the PHY reset just before probing > the driver, this seems to me to be the only way to guarantee that, > where boot firmware does not deal with this, the PHY manufacturers > specification for the initial release of reset can be met while keeping > this FEC specific behaviour out of the core MDIO/phylib layers. I'll also add... as kernel developers, we're not very good at insisting on generic names for things like clocks (see the mess that is stmmac, where the dwmac's various clock signals are named differently in the many platform glues.) So, I guess that FEC MDIO may need to have a list of clock names to look for when inspecting the PHY nodes if the clock is even specified there. If it isn't, then some other way of discovering that the PHY needs a clock (looking at the pinctrl configuration to see whether the pin is configured to output a clock to the PHY?) would need to be added. However, care needs to be taken in the case where the PHY has already been setup by boot firmware, that the clock signal is not disturbed, as interrupting it could provoke PHY problems - especially if the platform doesn't have the PHY reset specified in DT. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Thu, Jan 29, 2026 at 01:19:28PM +0000, Russell King (Oracle) wrote: > On Thu, Jan 29, 2026 at 01:12:41PM +0000, Russell King (Oracle) wrote: > > Another possible solution beyond those I've already stated, given that > > this only afflicts the FEC driver, ould be for the FEC MDIO driver to > > walk the child nodes, checking to see whether they require any clocks, > > and ensuring that those are properly initialised before registering the > > MDIO bus. > > > > Since the MDIO bus layer can release the PHY reset just before probing > > the driver, this seems to me to be the only way to guarantee that, > > where boot firmware does not deal with this, the PHY manufacturers > > specification for the initial release of reset can be met while keeping > > this FEC specific behaviour out of the core MDIO/phylib layers. > > I'll also add... as kernel developers, we're not very good at insisting > on generic names for things like clocks (see the mess that is stmmac, > where the dwmac's various clock signals are named differently in the > many platform glues.) > > So, I guess that FEC MDIO may need to have a list of clock names to > look for when inspecting the PHY nodes if the clock is even specified > there. If it isn't, then some other way of discovering that the PHY > needs a clock (looking at the pinctrl configuration to see whether > the pin is configured to output a clock to the PHY?) would need to be > added. > > However, care needs to be taken in the case where the PHY has already > been setup by boot firmware, that the clock signal is not disturbed, > as interrupting it could provoke PHY problems - especially if the > platform doesn't have the PHY reset specified in DT. A further issue to add following on from that last paragraph is whether one is indeed permitted to stop the PHYs XTAL/reference clock or not after the PHY has been setup. This would depend on the design of the PHY. The safe thing to do is not to stop the clock, but if it is desirable to do so with PHYs that allow it, and provided they do not lose settings as a result, then we would need some way to inform the MDIO bus layer when it is safe to stop those clocks. That would require phylib/mdio code changes to have the driver indicate that this PHY permits the clock to be stopped. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> The issue was with the out-of-band reset coming from the FEC driver > which doesn't honor the phy state-machine. Could you explain this is more details. Is the FEC doing something wrong? Andrew
Hi On Wed, Jan 28, 2026 at 9:51 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > The issue was with the out-of-band reset coming from the FEC driver > > which doesn't honor the phy state-machine. > > Could you explain this is more details. Is the FEC doing something > wrong? The fact that the phy should be reset when the clk is provided to it, is not connected at all with the fec. I think that fec_main does not register itself as clock provider, You "should" define the phy to have a clock if this is needed during the restoration and we should not give any "magic" at controller level. * Marco * Your commit bedd8d78aba300860cec3f85d6ff549b3b7f2679 is clear and I have tried to use it but if I remember the problem is the clock is provided by the ethernet controller. Michael > > Andrew -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 michael@amarulasolutions.com __________________________________ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 info@amarulasolutions.com www.amarulasolutions.com
On Wed, Jan 28, 2026 at 10:04:03PM +0100, Michael Nazzareno Trimarchi wrote:
> Hi
>
> On Wed, Jan 28, 2026 at 9:51 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > The issue was with the out-of-band reset coming from the FEC driver
> > > which doesn't honor the phy state-machine.
> >
> > Could you explain this is more details. Is the FEC doing something
> > wrong?
>
> The fact that the phy should be reset when the clk is provided to it, is not
> connected at all with the fec. I think that fec_main does not register itself
> as clock provider, You "should" define the phy to have a clock if this is needed
> during the restoration and we should not give any "magic" at controller level.
Do we know which clock the PHY is consuming?
The FEC has code for the clock "enet_out" which it enables and
disables. If the PHY is also consuming this clock, we could also make
it a consumer. The CCF reference counts enables/disables of clocks, so
if the PHY enables it, the MAC won't be able to disable it.
Andrew
On Wed, Jan 28, 2026 at 10:45:32PM +0100, Andrew Lunn wrote: > On Wed, Jan 28, 2026 at 10:04:03PM +0100, Michael Nazzareno Trimarchi wrote: > > Hi > > > > On Wed, Jan 28, 2026 at 9:51 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > > The issue was with the out-of-band reset coming from the FEC driver > > > > which doesn't honor the phy state-machine. > > > > > > Could you explain this is more details. Is the FEC doing something > > > wrong? > > > > The fact that the phy should be reset when the clk is provided to it, is not > > connected at all with the fec. I think that fec_main does not register itself > > as clock provider, You "should" define the phy to have a clock if this is needed > > during the restoration and we should not give any "magic" at controller level. > > Do we know which clock the PHY is consuming? > > The FEC has code for the clock "enet_out" which it enables and > disables. If the PHY is also consuming this clock, we could also make > it a consumer. The CCF reference counts enables/disables of clocks, so > if the PHY enables it, the MAC won't be able to disable it. As I've just mentioned earlier in this thread, there seems to be nothing special about the LAN8710-like PHYs requiring their XTAL clock to be running for reset to be functional. The same is true of AR8035 used on i.MX6 SolidRun platforms, and Marvell 88E151x PHYs that I've looked at. I would suggest that, in general, _all_ PHYs require their XTAL clock to be running. Therefore, this is not a work of the PHY at all. It is a quirk of the board design to provide the PHY's XTAL clock from the SoC, and this _seems_ to be common with FEC based systems, probably because there's an easy way to get the clock from the FEC, thus saving the cost of a crystal. I stated how SolidRun handles this quirk entirely in uboot so the kernel doesn't have to care about this at all, and the kernel doesn't get to even know that the PHY has a reset GPIO. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Wed, Jan 28, 2026 at 10:17:41PM +0000, Russell King (Oracle) wrote: > As I've just mentioned earlier in this thread, there seems to be nothing > special about the LAN8710-like PHYs requiring their XTAL clock to be > running for reset to be functional. The same is true of AR8035 used > on i.MX6 SolidRun platforms, and Marvell 88E151x PHYs that I've looked > at. I would suggest that, in general, _all_ PHYs require their XTAL > clock to be running. > > Therefore, this is not a work of the PHY at all. > > It is a quirk of the board design to provide the PHY's XTAL clock from > the SoC, and this _seems_ to be common with FEC based systems, probably > because there's an easy way to get the clock from the FEC, thus saving > the cost of a crystal. > > I stated how SolidRun handles this quirk entirely in uboot so the kernel > doesn't have to care about this at all, and the kernel doesn't get to > even know that the PHY has a reset GPIO. I've just been looking at the ZII dev rev B board, which uses a VF610 with a KSZ8041 PHY. It sources its clock from the SoC as well. However, looking at the PHY datasheet, no clock is specified required during reset. However, from what I can see, similar to the SolidRun platforms, the clock for the PHY isn't described in DT. It goes further though - nor is the PHY described, nor is the PHYs reset signal (which comes from io-expander@20). -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi
On Wed, Jan 28, 2026 at 10:04 PM Michael Nazzareno Trimarchi
<michael@amarulasolutions.com> wrote:
>
> Hi
>
> On Wed, Jan 28, 2026 at 9:51 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > The issue was with the out-of-band reset coming from the FEC driver
> > > which doesn't honor the phy state-machine.
> >
> > Could you explain this is more details. Is the FEC doing something
> > wrong?
>
> The fact that the phy should be reset when the clk is provided to it, is not
> connected at all with the fec. I think that fec_main does not register itself
> as clock provider, You "should" define the phy to have a clock if this is needed
> during the restoration and we should not give any "magic" at controller level.
>
> * Marco * Your commit bedd8d78aba300860cec3f85d6ff549b3b7f2679 is clear
> and I have tried to use it but if I remember the problem is the clock
> is provided
> by the ethernet controller.
I have created this patch at that time but not solving it:
panicking@panicking:~/work/wandh/distro/linux-wh$ git show
bfe2e98b653637526213ba06dee92d2cf1f678b4
commit bfe2e98b653637526213ba06dee92d2cf1f678b4
Author: Michael Trimarchi <michael@amarulasolutions.com>
Date: Sat Aug 23 09:34:22 2025 +0200
ARM: dts: imx6ull-mrmmi: Describe the Ethernet PHY clock
From d65af21842f8 ("net: phy: smsc: LAN8710/20: remove
PHY_RST_AFTER_CLK_EN flag"),
the lan phy is not always reset the correct way and let time to time phy
not configured.
Change-Id: I21549981bdca7581dc65acf06007136c24be5d56
Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
diff --git a/arch/arm/boot/dts/nxp/imx/imx6ull-mrmmi.dtsi
b/arch/arm/boot/dts/nxp/imx/imx6ull-mrmmi.dtsi
index 03a870e317cd..0a1618ced6aa 100644
--- a/arch/arm/boot/dts/nxp/imx/imx6ull-mrmmi.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx6ull-mrmmi.dtsi
@@ -237,6 +237,8 @@ mdio {
ethphy0: ethernet-phy@0 {
compatible = "ethernet-phy-ieee802.3-c22";
reg = <0>;
+ clocks = <&clks IMX6UL_CLK_ENET_REF>;
+ clock-names = "rmii-ref";
reset-gpios = <&gpio5 9 GPIO_ACTIVE_LOW>;
reset-assert-us = <4000>;
reset-deassert-us = <4000>;
Michael
>
> Michael
>
>
> >
> > Andrew
>
>
>
> --
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> michael@amarulasolutions.com
> __________________________________
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info@amarulasolutions.com
> www.amarulasolutions.com
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com
Hi Russell On Wed, Jan 28, 2026 at 2:58 PM Russell King (Oracle) <linux@armlinux.org.uk> wrote: > > On Wed, Jan 28, 2026 at 02:33:24PM +0100, Michael Nazzareno Trimarchi wrote: > > Hi Andrew > > > > On Wed, Jan 28, 2026 at 2:25 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > On Wed, Jan 28, 2026 at 10:46:41AM +0100, Michael Trimarchi wrote: > > > > The current implementation of phy_reset_after_clk_enable requires MAC drivers > > > > (like fec_main.c) to manually track PHY probing states and trigger resets > > > > at specific points in their clock management flow and was created just for a SoC > > > > vendor. > > > > > > We try to keep workarounds for specific SoC vendors out of the core, > > > when possible. It just makes the core more complex for an edge case > > > which most people don't care about. It is better to hide the > > > complexity away in the driver which needs it. > > > > We should not merge things in the core that are for one Soc and > > after d65af21842f8a7821fc3b28dc043c2f92b2b312c, I need to anyway to do: > > > > net: phy: smsc: Fix functionality of lan8710 in combination with NXP fec > > > > Revert "net: phy: smsc: LAN8710/20: remove PHY_RST_AFTER_CLK_EN flag" > > d65af21842 > > That commit is an example of a poor commit description. It doesn't > describe the problem, nor which hardware this affects, so when we end > up with this kind of situation, we're lacking the context behind why > the change was made. > > So, how do we know that reverting that commit is not going to cause a > regression for whatever platform the patch was proposed for? > > If you read the preceeding commit (which is referenced by the commit > you intend to revert) itgives more information about the problem, and > I would expect that reverting it will cause a regression with > interrupts. First the PHY_RST_AFTER_CLK_EN was introduced just for the phy that needs a specific reset sequence and we need to guarantee that the clock to the phy is active during the reset. Now phy clock can be provided even by the fec_main and the code there was written really to support in my opinion some reference design they had at that time. The entire code that I suggest to re-think is on top of this use case. For the platform I get this problem was an imx6ull and reverting that patch that triggered the nxp code was the only way to have the reset properly across the clock reset sequence. During this revert I found this code and I have tried to figure out it was possible to just let this specific flag used only on the phy level. > > Adding Marco to this thread for comment. > > Also, maybe you could clearly state what the issue that you are trying > to address - why do you need special reset handling for the combination > of FEC + this PHY. Consider including that description in the commit > message for any proposed solution so that - as is the case here looking > back at Macro's commit - we can see _why_ the change is being made. > Michael > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 michael@amarulasolutions.com __________________________________ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 info@amarulasolutions.com www.amarulasolutions.com
© 2016 - 2026 Red Hat, Inc.