linux-kernel - Re: [PATCH net-next] net: phy: add wol config options in phy device

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]

Date: Fri, 3 May 2024 14:52:48 +0530
From: Raju Lakkaraju <Raju.Lakkaraju@...rochip.com>
To: Paolo Abeni <pabeni@...hat.com>
CC: Raju Lakkaraju <Raju.Lakkaraju@...rochip.com>, <netdev@...r.kernel.org>,
	<lxu@...linear.com>, <andrew@...n.ch>, <hkallweit1@...il.com>,
	<linux@...linux.org.uk>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<kuba@...nel.org>, <linux-kernel@...r.kernel.org>,
	<UNGLinuxDriver@...rochip.com>
Subject: Re: [PATCH net-next] net: phy: add wol config options in phy device

Hi Paolo,

Thank you for review comments.

The 05/02/2024 15:49, Paolo Abeni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Tue, 2024-04-30 at 10:36 +0530, Raju Lakkaraju wrote:
> > @@ -725,6 +726,8 @@ static int gpy_set_wol(struct phy_device *phydev,
> >               ret = phy_read(phydev, PHY_ISTAT);
> >               if (ret < 0)
> >                       return ret;
> > +
> > +             phydev->wolopts |= WAKE_MAGIC;
> 
> I'm wondering if 'phydev->wolopts' could/should be handled in the
> common code.

Ok. I will change.

> 
> >       } else {
> >               /* Disable magic packet matching */
> >               ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2,
> > @@ -748,6 +751,8 @@ static int gpy_set_wol(struct phy_device *phydev,
> >               if (ret & (PHY_IMASK_MASK & ~PHY_IMASK_LSTC))
> >                       phy_trigger_machine(phydev);
> >
> > +             phydev->wolopts |= WAKE_PHY;
> > +
> >               return 0;
> >       }
> >
> > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > index 616bd7ba46cb..9740f08ad98e 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -2038,6 +2038,11 @@ int phy_suspend(struct phy_device *phydev)
> >       if (phydev->suspended)
> >               return 0;
> >
> > +     if (phydev->wolopts) {
> > +             wol.wolopts = phydev->wolopts;
> > +             phy_ethtool_set_wol(phydev, &wol);
> 
> The above will fail when the phy does not provide wol operations -
> should never happen when 'wolopts != 0', but possibly worth catching
> the error?

In another mail thread, Andrew gave different comment.
I will address that comment.

> 
> Thanks,
> 
> Paolo
> 

-- 
Thanks,                                                                         
Raju