Re: [PATCH] net: phy: micrel: reconfigure the phy on resume

On 13.01.2021 10:29, Claudiu.Beznea@microchip.com wrote:

Hi Heiner,

On 08.01.2021 18:31, Heiner Kallweit wrote:

quoted

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

On 08.01.2021 16:45, Claudiu Beznea wrote:

quoted

KSZ9131 is used in setups with SAMA7G5. SAMA7G5 supports a special
power saving mode (backup mode) that cuts the power for almost all
parts of the SoC. The rail powering the ethernet PHY is also cut off.
When resuming, in case the PHY has been configured on probe with
slew rate or DLL settings these needs to be restored thus call
driver's config_init() on resume.

When would the SoC enter this backup mode?

It could enter in this mode based on request for standby or suspend-to-mem:
echo mem > /sys/power/state
echo standby > /sys/power/state

What I didn't mentioned previously is that the RAM remains in self-refresh
while the rest of the SoC is powered down.

This leaves the question which driver sets backup mode in the SoC.
Whatever/whoever wakes the SoC later would have to take care that basically
everything that was switched off is reconfigured (incl. calling phy_init_hw()). 
So it' more or less the same as waking up from hibernation. Therefore I think
the .restore of all subsystems would have to be executed, incl. .restore of
the MDIO bus. Having said that I don't think that change belongs into the
PHY driver.
Just imagine tomorrow another PHY type is used in a SAMA7G5 setup.
Then you would have to do same change in another PHY driver.

quoted

And would it suspend the
MDIO bus before cutting power to the PHY?

SAMA7G5 embeds Cadence macb driver which has a integrated MDIO bus. Inside
macb driver the bus is registered with of_mdiobus_register() or
mdiobus_register() based on the PHY devices present in DT or not. On macb
suspend()/resume() functions there are calls to
phylink_stop()/phylink_start() before cutting/after enabling the power to
the PHY.

quoted

I'm asking because in mdio_bus_phy_restore() we call phy_init_hw()
already (that calls the driver's config_init).

As far as I can see from documentation the .restore API of dev_pm_ops is
hibernation specific (please correct me if I'm wrong). On transitions to
backup mode the suspend()/resume() PM APIs are called on the drivers.

Thank you,
Claudiu Beznea

quoted

quoted

Signed-off-by: Claudiu Beznea <redacted>
---
 drivers/net/phy/micrel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 3fe552675dd2..52d3a0480158 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c

@@ -1077,7 +1077,7 @@ static int kszphy_resume(struct phy_device *phydev)
       */
      usleep_range(1000, 2000);

-     ret = kszphy_config_reset(phydev);
+     ret = phydev->drv->config_init(phydev);
      if (ret)
              return ret;