On 1/5/21 6:38 PM, Andrew Lunn wrote:
+static void smsc_link_change_notify(struct phy_device *phydev)
+{
+       struct smsc_phy_priv *priv = phydev->priv;
+
+       if (!priv->vddio)
+               return;
+
+       if (phydev->state == PHY_HALTED)
+               regulator_disable(priv->vddio);
+
+       if (phydev->state == PHY_NOLINK)
+               regulator_enable(priv->vddio);

NOLINK is an interesting choice. Could you explain that please.

It's the first state after interface is up.

I fear this is not going to be very robust to state machine
changes. And since it is hidden away in a driver, it is going to be
forgotten about. You might want to think about making it more robust.

I marked the patch as RFC because I would like input on how to implement this properly. Note that since the regulator supplies the magnetics, which might be shared between multiple ports with different PHYs, I don't think this code should even be in the PHY driver, but somewhere else -- but I don't know where.

Reply via email to