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

[Raju Lakkaraju <Raju.Lakkaraju@microchip.com>]

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