netdev - RE: [PATCH net-next 1/1] net: phy: marvell10g: Add WAKE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <PH0PR11MB49509E7A82947DCB6BB48203D8FD9@PH0PR11MB4950.namprd11.prod.outlook.com>
Date:   Mon, 16 Aug 2021 08:56:36 +0000
From:   "Song, Yoong Siang" <yoong.siang.song@...el.com>
To:     Russell King <linux@...linux.org.uk>
CC:     Andrew Lunn <andrew@...n.ch>,
        Marek Behún <kabel@...nel.org>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net-next 1/1] net: phy: marvell10g: Add WAKE_PHY support
 to WOL event

> 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!