[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZW86hcgt8nhbSpfq@shell.armlinux.org.uk>
Date: Tue, 5 Dec 2023 14:58:13 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Paolo Abeni <pabeni@...hat.com>
Cc: Christian Marangi <ansuelsmth@...il.com>,
Andrew Lunn <andrew@...n.ch>,
Heiner Kallweit <hkallweit1@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [net-next PATCH v2 08/12] net: phy: at803x: move specific at8031
WOL bits to dedicated function
On Tue, Dec 05, 2023 at 10:32:55AM +0100, Paolo Abeni wrote:
> On Fri, 2023-12-01 at 01:14 +0100, Christian Marangi wrote:
> > Move specific at8031 WOL enable/disable to dedicated function to make
> > at803x_set_wol more generic.
> >
> > This is needed in preparation for PHY driver split as qca8081 share the
> > same function to toggle WOL settings.
> >
> > In this new implementation WOL module in at8031 is enabled after the
> > generic interrupt is setup. This should not cause any problem as the
> > WOL_INT has a separate implementation and only relay on MAC bits.
> >
> > Signed-off-by: Christian Marangi <ansuelsmth@...il.com>
> > ---
> > drivers/net/phy/at803x.c | 42 ++++++++++++++++++++++++----------------
> > 1 file changed, 25 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> > index 02ac71f98466..2de7a59c0faa 100644
> > --- a/drivers/net/phy/at803x.c
> > +++ b/drivers/net/phy/at803x.c
> > @@ -466,27 +466,11 @@ static int at803x_set_wol(struct phy_device *phydev,
> > phy_write_mmd(phydev, MDIO_MMD_PCS, offsets[i],
> > mac[(i * 2) + 1] | (mac[(i * 2)] << 8));
> >
> > - /* Enable WOL function for 1588 */
> > - if (phydev->drv->phy_id == ATH8031_PHY_ID) {
> > - ret = phy_modify_mmd(phydev, MDIO_MMD_PCS,
> > - AT803X_PHY_MMD3_WOL_CTRL,
> > - 0, AT803X_WOL_EN);
> > - if (ret)
> > - return ret;
> > - }
> > /* Enable WOL interrupt */
> > ret = phy_modify(phydev, AT803X_INTR_ENABLE, 0, AT803X_INTR_ENABLE_WOL);
> > if (ret)
> > return ret;
> > } else {
> > - /* Disable WoL function for 1588 */
> > - if (phydev->drv->phy_id == ATH8031_PHY_ID) {
> > - ret = phy_modify_mmd(phydev, MDIO_MMD_PCS,
> > - AT803X_PHY_MMD3_WOL_CTRL,
> > - AT803X_WOL_EN, 0);
> > - if (ret)
> > - return ret;
> > - }
> > /* Disable WOL interrupt */
> > ret = phy_modify(phydev, AT803X_INTR_ENABLE, AT803X_INTR_ENABLE_WOL, 0);
> > if (ret)
> > @@ -1611,6 +1595,30 @@ static int at8031_config_init(struct phy_device *phydev)
> > return at803x_config_init(phydev);
> > }
> >
> > +static int at8031_set_wol(struct phy_device *phydev,
> > + struct ethtool_wolinfo *wol)
> > +{
> > + int ret;
> > +
> > + /* First setup MAC address and enable WOL interrupt */
> > + ret = at803x_set_wol(phydev, wol);
> > + if (ret)
> > + return ret;
> > +
> > + if (wol->wolopts & WAKE_MAGIC)
> > + /* Enable WOL function for 1588 */
> > + ret = phy_modify_mmd(phydev, MDIO_MMD_PCS,
> > + AT803X_PHY_MMD3_WOL_CTRL,
> > + 0, AT803X_WOL_EN);
> > + else
> > + /* Disable WoL function for 1588 */
> > + ret = phy_modify_mmd(phydev, MDIO_MMD_PCS,
> > + AT803X_PHY_MMD3_WOL_CTRL,
> > + AT803X_WOL_EN, 0);
> > +
> > + return ret;
>
> If I read correctly, the above changes the order of some WoL
> initialization steps: now WOL_CTRL is touched after
> AT803X_INTR_ENABLE_WOL. Is that correct?
It is fine.
AT803X_INTR_ENABLE_WOL enables or disables whether the INT pin (which
is used for any interrupt from the PHY) is used to signal WOL - it's
the interrupt enable for the WoL function.
The MMD3 WOL_EN bit controls whether the WoL function is enabled, and
thus whether the WOL_INT pin will signal WoL. WOL_EN should not be
set until we have initialised the WoL function, and thus that needs
to happen _after_ the MAC address has been programmed.
Clearing WOL_EN afterwards is not a problem because it will already
have been setup, or is in its power-on default state.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists