Re: [PATCH net-next 3/3] net: fec: reset phy on resume after power-up

Hello Andrew,

On Tue, Dec 14, 2021 at 07:54:54PM +0100, Andrew Lunn wrote:

What i don't particularly like about this is that the MAC driver is
doing it. Meaning if this PHY is used with any other MAC, the same
code needs adding there.

This is exactly the same case as phy_reset_after_clk_enable() [1][2], to
me it does not look that bad.

So maybe in the phy driver, add a suspend handler, which asserts the
reset. This call here will take it out of reset, so applying the reset
you need?

Asserting the reset in the phylib in suspend path is a bad idea, in the
general case in which the PHY is powered in suspend the
power-consumption is likely to be higher if the device is in reset
compared to software power-down using the BMCR register (at least for
the PHY datasheet I checked).

What we could do is to call phy_device_reset in the fec driver suspend
path when we know we are going to disable the regulator, I do not like
it, but it would solve the issue.

--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c

@@ -4064,7 +4064,11 @@ static int __maybe_unused fec_suspend(struct device *dev)
        rtnl_unlock();

        if (fep->reg_phy && !(fep->wol_flag & FEC_WOL_FLAG_ENABLE))
+       {
                regulator_disable(fep->reg_phy);
+               phy_device_reset(ndev->phydev, 1);
+       }
+

        /* SOC supply clock to phy, when clock is disabled, phy link down
         * SOC control phy regulator, when regulator is disabled, phy link down

Francesco

[1] https://lore.kernel.org/netdev/20171211121700.10200-1-dev@g0hl1n.net/ (local)
[2] 1b0a83ac04e3 ("net: fec: add phy_reset_after_clk_enable() support")