[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 05 Dec 2023 10:32:55 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Christian Marangi <ansuelsmth@...il.com>,
Andrew Lunn <andrew@...n.ch>,
Heiner Kallweit <hkallweit1@...il.com>,
Russell King <linux@...linux.org.uk>,
"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 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?
Thanks,
Paolo
Powered by blists - more mailing lists