RE: [PATCH net-next 1/1] net: phy: marvell10g: Add WAKE_PHY support to WOL event

[Song, Yoong Siang]

> On Mon, Aug 16, 2021 at 08:03:59AM +0000, Song, Yoong Siang wrote:
> > Thanks for your explanation. I understand your concern better now.
> >
> > In the case of WoL hasn't been enabled through a set_wol call, the PHY
> > will be suspended, so we no need worry the link change interrupt will
> > create an undesired WoL event.
> >
> > In the case of set_wol is called to disable WAKE_PHY event, we can
> > keep the link change interrupt enable, so that it won't affect the
> > interrupt support.
> 
> I think you're missing the point. In your get_wol method for this
> PHY:
> 
> +       ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
> MV_PCS_INTR_ENABLE);
> +       if (ret < 0)
> +               return;
> +
> +       if (ret & MV_PCS_INTR_ENABLE_LSC)
> +               wol->wolopts |= WAKE_PHY;
> 
> If the link change interrupt is enabled because we want to use interrupt
> support, the above code has the effect of reporting to userspace that WoL is
> enabled, even when nothing has requested WoL to be enabled.
> 
> This also has the effect of preventing the PHY being suspended (see
> phy_suspend()) and in effect means that WoL is enabled, even though
> set_wol() was not called.
> 
Yes, you are right. I missed the effect of get_wol.
Is it needed in future to implement link change interrupt in phy driver?
Cause I dint see much phy driver implement link change interrupt.
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!