Re: [PATCH net] net: phy: micrel: fix clk warning when removing the driver

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

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