net: phy: micrel: fix clk warning when removing the driver
Subsystems (4)
PATCH 1/1 net: phy: micrel: fix clk warning when removing the driver
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 againTherefore, 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.
--- drivers/net/phy/micrel.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)
drivers/net/phy/micrel.c +13 -4
@@ -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))
Signature · 1 line
2.34.1
Hi,
Quoted 60 lines · Wei Fang
Since the commit
25c6a5ab151f("net: phy: micrel: Dynamically control external clock of KSZ PHY"), the clock of Micrel PHY has been enabled byphy_driver::resume()and disabled byphy_driver::suspend(). However,devm_clk_get_optional_enabled()is used inkszphy_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/0x1f4After 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
Quoted 7 lines
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: usedevm_clk_get_optional_enabledfor the rmii-ref clock"), the clock is enabled byclk_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 <[email protected]>
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
Quoted 82 lines · Wei Fang
Quoted 8 lines
Since the commit
25c6a5ab151f("net: phy: micrel: Dynamically control external clock of KSZ PHY"), the clock of Micrel PHY has been enabled byphy_driver::resume()and disabled byphy_driver::suspend(). However,devm_clk_get_optional_enabled()is used inkszphy_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.Quoted 7 lines
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/639Quoted 8 lines
[ 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/0x1f4Quoted 1 line
[ 109.859170] device_release_driver_internal frombus_remove_device+0xbc/0xe4 > [ 109.866243] bus_remove_device from device_del+0x140/0x458 > [ 109.871757] device_del from phy_mdio_device_remove+0xc/0x24Quoted 1 line
[ 109.877452] phy_mdio_device_remove from
mdiobus_unregister+0x40/0xacQuoted 1 line
[ 109.883918] mdiobus_unregister fromfec_enet_mii_remove+0x40/0x78 > [ 109.890125] fec_enet_mii_remove from fec_drv_remove+0x4c/0x158Quoted 1 line
[ 109.896076] fec_drv_remove from
device_release_driver_internal+0x17c/0x1f4Quoted 1 line
[ 109.962748] WARNING: drivers/clk/clk.c:1047 at
clk_core_unprepare+0xfc/0x13c, CPU#0: sh/639Quoted 7 lines
[ 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/0x1f4Quoted 1 line
[ 110.062877] device_release_driver_internal frombus_remove_device+0xbc/0xe4 > [ 110.069950] bus_remove_device from device_del+0x140/0x458 > [ 110.075469] device_del from phy_mdio_device_remove+0xc/0x24Quoted 1 line
[ 110.081165] phy_mdio_device_remove from
mdiobus_unregister+0x40/0xacQuoted 1 line
[ 110.087632] mdiobus_unregister fromfec_enet_mii_remove+0x40/0x78 > [ 110.093836] fec_enet_mii_remove from fec_drv_remove+0x4c/0x158Quoted 1 line
[ 110.099782] fec_drv_remove from
device_release_driver_internal+0x17c/0x1f4Quoted 15 lines
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 againQuoted 6 lines
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: usedevm_clk_get_optional_enabledfor the rmii-ref clock"), the clock is enabled byclk_prepare_enable()to get the correct clock rate.Fixes 25c6a5ab151f ("net: phy: micrel: Dynamically control external clock ofKSZ PHY")
Quoted 1 line
Signed-off-by Wei Fang <[email protected]>
Quoted 9 lines
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_clkselection only in the .resume() path, as it's also called no long afterconfig_init(). That would mean reading the clock rate inkszphy_config_reset(), then performing thekszphy_rmii_clk_sel()accordingly.
It seems rather strange to get the clock rate every time the .resume() is called. Otherwise, we would need to add some additional logic to kszphy_rmii_clk_sel() to ensure that the clock rate is only obtained once. IMO, it would be more reasonable to only get the clock rate in the .probe() during the lifetime of the PHY driver.
Furthermore, this seems to break the previous logic: when the clock rate is incorrect, the .probe() will return an error, meaning the PHY driver will not be loaded.
Quoted 98 lines · Wei Fang
Quoted 82 lines · Wei Fang
Quoted 8 lines
Since the commit
25c6a5ab151f("net: phy: micrel: Dynamically control external clock of KSZ PHY"), the clock of Micrel PHY has been enabled byphy_driver::resume()and disabled byphy_driver::suspend(). However,devm_clk_get_optional_enabled()is used inkszphy_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.Quoted 7 lines
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/639Quoted 8 lines
[ 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/0x1f4Quoted 1 line
[ 109.859170] device_release_driver_internal frombus_remove_device+0xbc/0xe4 > [ 109.866243] bus_remove_device from device_del+0x140/0x458 > [ 109.871757] device_del from phy_mdio_device_remove+0xc/0x24Quoted 1 line
[ 109.877452] phy_mdio_device_remove from
mdiobus_unregister+0x40/0xacQuoted 1 line
[ 109.883918] mdiobus_unregister fromfec_enet_mii_remove+0x40/0x78 > [ 109.890125] fec_enet_mii_remove from fec_drv_remove+0x4c/0x158Quoted 1 line
[ 109.896076] fec_drv_remove from
device_release_driver_internal+0x17c/0x1f4Quoted 1 line
[ 109.962748] WARNING: drivers/clk/clk.c:1047 at
clk_core_unprepare+0xfc/0x13c, CPU#0: sh/639Quoted 7 lines
[ 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/0x1f4Quoted 1 line
[ 110.062877] device_release_driver_internal frombus_remove_device+0xbc/0xe4 > [ 110.069950] bus_remove_device from device_del+0x140/0x458 > [ 110.075469] device_del from phy_mdio_device_remove+0xc/0x24Quoted 1 line
[ 110.081165] phy_mdio_device_remove from
mdiobus_unregister+0x40/0xacQuoted 1 line
[ 110.087632] mdiobus_unregister fromfec_enet_mii_remove+0x40/0x78 > [ 110.093836] fec_enet_mii_remove from fec_drv_remove+0x4c/0x158Quoted 1 line
[ 110.099782] fec_drv_remove from
device_release_driver_internal+0x17c/0x1f4Quoted 15 lines
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 againQuoted 6 lines
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: usedevm_clk_get_optional_enabledfor the rmii-ref clock"), the clock is enabled byclk_prepare_enable()to get the correct clock rate.Fixes 25c6a5ab151f ("net: phy: micrel: Dynamically control external clock ofKSZ PHY")
Quoted 1 line
Signed-off-by Wei Fang <[email protected]>Quoted 9 lines
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_clkselection only in the .resume() path, as it's also called no long afterconfig_init(). That would mean reading the clock rate inkszphy_config_reset(), then performing thekszphy_rmii_clk_sel()accordingly.It seems rather strange to get the clock rate every time the .resume() is called. Otherwise, we would need to add some additional logic to
kszphy_rmii_clk_sel()to ensure that the clock rate is only obtained once. IMO, it would be more reasonable to only get the clock rate in the .probe() during the lifetime of the PHY driver.
Quoted 4 lines
Furthermore, this seems to break the previous logic: when the clock rate is incorrect, the .probe() will return an error, meaning the PHY driver will not be loaded.
Makes sense to validate it early indeed :) All in all, this seems ok then.
Maxime
Hi,
Quoted 60 lines · Wei Fang
Since the commit
25c6a5ab151f("net: phy: micrel: Dynamically control external clock of KSZ PHY"), the clock of Micrel PHY has been enabled byphy_driver::resume()and disabled byphy_driver::suspend(). However,devm_clk_get_optional_enabled()is used inkszphy_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/0x1f4After 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
Quoted 7 lines
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: usedevm_clk_get_optional_enabledfor the rmii-ref clock"), the clock is enabled byclk_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 <[email protected]>
Maxime
This patch was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:
Quoted 9 lines · On Mon, 26 Jan 2026 16:15:44 +0800 you wrote:
Since the commit
25c6a5ab151f("net: phy: micrel: Dynamically control external clock of KSZ PHY"), the clock of Micrel PHY has been enabled byphy_driver::resume()and disabled byphy_driver::suspend(). However,devm_clk_get_optional_enabled()is used inkszphy_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.[...]
Here is the summary with links:
- [net] net: phy: micrel: fix clk warning when removing the driver
https://git.kernel.org/netdev/net/c/2aa1545ba8d4
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html