drivers/net/phy/micrel.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)
Since the commit 25c6a5ab151f ("net: phy: micrel: Dynamically control
external clock of KSZ PHY"), the clock of Micrel PHY has been enabled
by phy_driver::resume() and disabled by phy_driver::suspend(). However,
devm_clk_get_optional_enabled() is used in kszphy_probe(), so the clock
will automatically be disabled when the device is unbound from the bus.
Therefore, this could cause the clock to be disabled twice, resulting
in clk driver warnings.
For example, this issue can be reproduced on i.MX6ULL platform, and we
can see the following logs when removing the FEC MAC drivers.
$ echo 2188000.ethernet > /sys/bus/platform/drivers/fec/unbind
$ echo 20b4000.ethernet > /sys/bus/platform/drivers/fec/unbind
[ 109.758207] ------------[ cut here ]------------
[ 109.758240] WARNING: drivers/clk/clk.c:1188 at clk_core_disable+0xb4/0xd0, CPU#0: sh/639
[ 109.771011] enet2_ref already disabled
[ 109.793359] Call trace:
[ 109.822006] clk_core_disable from clk_disable+0x28/0x34
[ 109.827340] clk_disable from clk_disable_unprepare+0xc/0x18
[ 109.833029] clk_disable_unprepare from devm_clk_release+0x1c/0x28
[ 109.839241] devm_clk_release from devres_release_all+0x98/0x100
[ 109.845278] devres_release_all from device_unbind_cleanup+0xc/0x70
[ 109.851571] device_unbind_cleanup from device_release_driver_internal+0x1a4/0x1f4
[ 109.859170] device_release_driver_internal from bus_remove_device+0xbc/0xe4
[ 109.866243] bus_remove_device from device_del+0x140/0x458
[ 109.871757] device_del from phy_mdio_device_remove+0xc/0x24
[ 109.877452] phy_mdio_device_remove from mdiobus_unregister+0x40/0xac
[ 109.883918] mdiobus_unregister from fec_enet_mii_remove+0x40/0x78
[ 109.890125] fec_enet_mii_remove from fec_drv_remove+0x4c/0x158
[ 109.896076] fec_drv_remove from device_release_driver_internal+0x17c/0x1f4
[ 109.962748] WARNING: drivers/clk/clk.c:1047 at clk_core_unprepare+0xfc/0x13c, CPU#0: sh/639
[ 109.975805] enet2_ref already unprepared
[ 110.002866] Call trace:
[ 110.031758] clk_core_unprepare from clk_unprepare+0x24/0x2c
[ 110.037440] clk_unprepare from devm_clk_release+0x1c/0x28
[ 110.042957] devm_clk_release from devres_release_all+0x98/0x100
[ 110.048989] devres_release_all from device_unbind_cleanup+0xc/0x70
[ 110.055280] device_unbind_cleanup from device_release_driver_internal+0x1a4/0x1f4
[ 110.062877] device_release_driver_internal from bus_remove_device+0xbc/0xe4
[ 110.069950] bus_remove_device from device_del+0x140/0x458
[ 110.075469] device_del from phy_mdio_device_remove+0xc/0x24
[ 110.081165] phy_mdio_device_remove from mdiobus_unregister+0x40/0xac
[ 110.087632] mdiobus_unregister from fec_enet_mii_remove+0x40/0x78
[ 110.093836] fec_enet_mii_remove from fec_drv_remove+0x4c/0x158
[ 110.099782] fec_drv_remove from device_release_driver_internal+0x17c/0x1f4
After analyzing the process of removing the FEC driver, as shown below,
it can be seen that the clock was disabled twice by the PHY driver.
fec_drv_remove()
--> fec_enet_close()
--> phy_stop()
--> phy_suspend()
--> kszphy_suspend() #1 The clock is disabled
--> fec_enet_mii_remove()
--> mdiobus_unregister()
--> phy_mdio_device_remove()
--> device_del()
--> devm_clk_release() #2 The clock is disabled again
Therefore, devm_clk_get_optional() is used to fix the above issue. And
to avoid the issue mentioned by the commit 985329462723 ("net: phy:
micrel: use devm_clk_get_optional_enabled for the rmii-ref clock"), the
clock is enabled by clk_prepare_enable() to get the correct clock rate.
Fixes: 25c6a5ab151f ("net: phy: micrel: Dynamically control external clock of KSZ PHY")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
drivers/net/phy/micrel.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 05de68b9f719..8208ecbb575c 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -2643,11 +2643,21 @@ static int kszphy_probe(struct phy_device *phydev)
kszphy_parse_led_mode(phydev);
- clk = devm_clk_get_optional_enabled(&phydev->mdio.dev, "rmii-ref");
+ clk = devm_clk_get_optional(&phydev->mdio.dev, "rmii-ref");
/* NOTE: clk may be NULL if building without CONFIG_HAVE_CLK */
if (!IS_ERR_OR_NULL(clk)) {
- unsigned long rate = clk_get_rate(clk);
bool rmii_ref_clk_sel_25_mhz;
+ unsigned long rate;
+ int err;
+
+ err = clk_prepare_enable(clk);
+ if (err) {
+ phydev_err(phydev, "Failed to enable rmii-ref clock\n");
+ return err;
+ }
+
+ rate = clk_get_rate(clk);
+ clk_disable_unprepare(clk);
if (type)
priv->rmii_ref_clk_sel = type->has_rmii_ref_clk_sel;
@@ -2665,13 +2675,12 @@ static int kszphy_probe(struct phy_device *phydev)
}
} else if (!clk) {
/* unnamed clock from the generic ethernet-phy binding */
- clk = devm_clk_get_optional_enabled(&phydev->mdio.dev, NULL);
+ clk = devm_clk_get_optional(&phydev->mdio.dev, NULL);
}
if (IS_ERR(clk))
return PTR_ERR(clk);
- clk_disable_unprepare(clk);
priv->clk = clk;
if (ksz8041_fiber_mode(phydev))
--
2.34.1
Hi,
On 26/01/2026 09:15, Wei Fang wrote:
> Since the commit 25c6a5ab151f ("net: phy: micrel: Dynamically control
> external clock of KSZ PHY"), the clock of Micrel PHY has been enabled
> by phy_driver::resume() and disabled by phy_driver::suspend(). However,
> devm_clk_get_optional_enabled() is used in kszphy_probe(), so the clock
> will automatically be disabled when the device is unbound from the bus.
> Therefore, this could cause the clock to be disabled twice, resulting
> in clk driver warnings.
>
> For example, this issue can be reproduced on i.MX6ULL platform, and we
> can see the following logs when removing the FEC MAC drivers.
>
> $ echo 2188000.ethernet > /sys/bus/platform/drivers/fec/unbind
> $ echo 20b4000.ethernet > /sys/bus/platform/drivers/fec/unbind
> [ 109.758207] ------------[ cut here ]------------
> [ 109.758240] WARNING: drivers/clk/clk.c:1188 at clk_core_disable+0xb4/0xd0, CPU#0: sh/639
> [ 109.771011] enet2_ref already disabled
> [ 109.793359] Call trace:
> [ 109.822006] clk_core_disable from clk_disable+0x28/0x34
> [ 109.827340] clk_disable from clk_disable_unprepare+0xc/0x18
> [ 109.833029] clk_disable_unprepare from devm_clk_release+0x1c/0x28
> [ 109.839241] devm_clk_release from devres_release_all+0x98/0x100
> [ 109.845278] devres_release_all from device_unbind_cleanup+0xc/0x70
> [ 109.851571] device_unbind_cleanup from device_release_driver_internal+0x1a4/0x1f4
> [ 109.859170] device_release_driver_internal from bus_remove_device+0xbc/0xe4
> [ 109.866243] bus_remove_device from device_del+0x140/0x458
> [ 109.871757] device_del from phy_mdio_device_remove+0xc/0x24
> [ 109.877452] phy_mdio_device_remove from mdiobus_unregister+0x40/0xac
> [ 109.883918] mdiobus_unregister from fec_enet_mii_remove+0x40/0x78
> [ 109.890125] fec_enet_mii_remove from fec_drv_remove+0x4c/0x158
> [ 109.896076] fec_drv_remove from device_release_driver_internal+0x17c/0x1f4
> [ 109.962748] WARNING: drivers/clk/clk.c:1047 at clk_core_unprepare+0xfc/0x13c, CPU#0: sh/639
> [ 109.975805] enet2_ref already unprepared
> [ 110.002866] Call trace:
> [ 110.031758] clk_core_unprepare from clk_unprepare+0x24/0x2c
> [ 110.037440] clk_unprepare from devm_clk_release+0x1c/0x28
> [ 110.042957] devm_clk_release from devres_release_all+0x98/0x100
> [ 110.048989] devres_release_all from device_unbind_cleanup+0xc/0x70
> [ 110.055280] device_unbind_cleanup from device_release_driver_internal+0x1a4/0x1f4
> [ 110.062877] device_release_driver_internal from bus_remove_device+0xbc/0xe4
> [ 110.069950] bus_remove_device from device_del+0x140/0x458
> [ 110.075469] device_del from phy_mdio_device_remove+0xc/0x24
> [ 110.081165] phy_mdio_device_remove from mdiobus_unregister+0x40/0xac
> [ 110.087632] mdiobus_unregister from fec_enet_mii_remove+0x40/0x78
> [ 110.093836] fec_enet_mii_remove from fec_drv_remove+0x4c/0x158
> [ 110.099782] fec_drv_remove from device_release_driver_internal+0x17c/0x1f4
>
> After analyzing the process of removing the FEC driver, as shown below,
> it can be seen that the clock was disabled twice by the PHY driver.
>
> fec_drv_remove()
> --> fec_enet_close()
> --> phy_stop()
> --> phy_suspend()
> --> kszphy_suspend() #1 The clock is disabled
> --> fec_enet_mii_remove()
> --> mdiobus_unregister()
> --> phy_mdio_device_remove()
> --> device_del()
> --> devm_clk_release() #2 The clock is disabled again
>
> Therefore, devm_clk_get_optional() is used to fix the above issue. And
> to avoid the issue mentioned by the commit 985329462723 ("net: phy:
> micrel: use devm_clk_get_optional_enabled for the rmii-ref clock"), the
> clock is enabled by clk_prepare_enable() to get the correct clock rate.
>
> Fixes: 25c6a5ab151f ("net: phy: micrel: Dynamically control external clock of KSZ PHY")
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Maxime
Hi,
On 26/01/2026 09:15, Wei Fang wrote:
> Since the commit 25c6a5ab151f ("net: phy: micrel: Dynamically control
> external clock of KSZ PHY"), the clock of Micrel PHY has been enabled
> by phy_driver::resume() and disabled by phy_driver::suspend(). However,
> devm_clk_get_optional_enabled() is used in kszphy_probe(), so the clock
> will automatically be disabled when the device is unbound from the bus.
> Therefore, this could cause the clock to be disabled twice, resulting
> in clk driver warnings.
>
> For example, this issue can be reproduced on i.MX6ULL platform, and we
> can see the following logs when removing the FEC MAC drivers.
>
> $ echo 2188000.ethernet > /sys/bus/platform/drivers/fec/unbind
> $ echo 20b4000.ethernet > /sys/bus/platform/drivers/fec/unbind
> [ 109.758207] ------------[ cut here ]------------
> [ 109.758240] WARNING: drivers/clk/clk.c:1188 at clk_core_disable+0xb4/0xd0, CPU#0: sh/639
> [ 109.771011] enet2_ref already disabled
> [ 109.793359] Call trace:
> [ 109.822006] clk_core_disable from clk_disable+0x28/0x34
> [ 109.827340] clk_disable from clk_disable_unprepare+0xc/0x18
> [ 109.833029] clk_disable_unprepare from devm_clk_release+0x1c/0x28
> [ 109.839241] devm_clk_release from devres_release_all+0x98/0x100
> [ 109.845278] devres_release_all from device_unbind_cleanup+0xc/0x70
> [ 109.851571] device_unbind_cleanup from device_release_driver_internal+0x1a4/0x1f4
> [ 109.859170] device_release_driver_internal from bus_remove_device+0xbc/0xe4
> [ 109.866243] bus_remove_device from device_del+0x140/0x458
> [ 109.871757] device_del from phy_mdio_device_remove+0xc/0x24
> [ 109.877452] phy_mdio_device_remove from mdiobus_unregister+0x40/0xac
> [ 109.883918] mdiobus_unregister from fec_enet_mii_remove+0x40/0x78
> [ 109.890125] fec_enet_mii_remove from fec_drv_remove+0x4c/0x158
> [ 109.896076] fec_drv_remove from device_release_driver_internal+0x17c/0x1f4
> [ 109.962748] WARNING: drivers/clk/clk.c:1047 at clk_core_unprepare+0xfc/0x13c, CPU#0: sh/639
> [ 109.975805] enet2_ref already unprepared
> [ 110.002866] Call trace:
> [ 110.031758] clk_core_unprepare from clk_unprepare+0x24/0x2c
> [ 110.037440] clk_unprepare from devm_clk_release+0x1c/0x28
> [ 110.042957] devm_clk_release from devres_release_all+0x98/0x100
> [ 110.048989] devres_release_all from device_unbind_cleanup+0xc/0x70
> [ 110.055280] device_unbind_cleanup from device_release_driver_internal+0x1a4/0x1f4
> [ 110.062877] device_release_driver_internal from bus_remove_device+0xbc/0xe4
> [ 110.069950] bus_remove_device from device_del+0x140/0x458
> [ 110.075469] device_del from phy_mdio_device_remove+0xc/0x24
> [ 110.081165] phy_mdio_device_remove from mdiobus_unregister+0x40/0xac
> [ 110.087632] mdiobus_unregister from fec_enet_mii_remove+0x40/0x78
> [ 110.093836] fec_enet_mii_remove from fec_drv_remove+0x4c/0x158
> [ 110.099782] fec_drv_remove from device_release_driver_internal+0x17c/0x1f4
>
> After analyzing the process of removing the FEC driver, as shown below,
> it can be seen that the clock was disabled twice by the PHY driver.
>
> fec_drv_remove()
> --> fec_enet_close()
> --> phy_stop()
> --> phy_suspend()
> --> kszphy_suspend() #1 The clock is disabled
> --> fec_enet_mii_remove()
> --> mdiobus_unregister()
> --> phy_mdio_device_remove()
> --> device_del()
> --> devm_clk_release() #2 The clock is disabled again
>
> Therefore, devm_clk_get_optional() is used to fix the above issue. And
> to avoid the issue mentioned by the commit 985329462723 ("net: phy:
> micrel: use devm_clk_get_optional_enabled for the rmii-ref clock"), the
> clock is enabled by clk_prepare_enable() to get the correct clock rate.
>
> Fixes: 25c6a5ab151f ("net: phy: micrel: Dynamically control external clock of KSZ PHY")
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
This should fix the issue indeed. The dance of enabling/disabling then
re-enabling at resume() again feels odd, but at least it's balanced.
We may be able to avoid that entirely by dealing with the ref_mii_clk
selection only in the .resume() path, as it's also called no long after
config_init(). That would mean reading the clock rate in
kszphy_config_reset(), then performing the kszphy_rmii_clk_sel()
accordingly.
Maxime
© 2016 - 2026 Red Hat, Inc.