lkml.org 
[lkml]   [2021]   [Jun]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH net-next] net: phy: marvell10g: enable WoL for mv2110
On Wed, Jun 23, 2021 at 09:09:29PM +0800, Ling Pei Lee wrote:
> +static void mv2110_get_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
> +{
> + int ret = 0;

This initialiser doesn't do anything.

> +
> + wol->supported = WAKE_MAGIC;
> + wol->wolopts = 0;
> +
> + ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MV_V2_WOL_CTRL);
> +
> + if (ret & MV_V2_WOL_MAGIC_PKT_EN)
> + wol->wolopts |= WAKE_MAGIC;

You need to check whether "ret" is a negative number - if phy_read_mmd()
returns an error, this test could be true or false. It would be better
to have well defined behaviour (e.g. reporting that WOL is disabled?)

> + /* Reset the clear WOL status bit as it does not self-clear */
> + ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2,
> + MV_V2_WOL_CTRL,
> + MV_V2_WOL_CLEAR_STS);
> +
> + if (ret < 0)
> + return ret;
> + } else {
> + /* Disable magic packet matching & reset WOL status bit */
> + ret = phy_modify_mmd(phydev, MDIO_MMD_VEND2,
> + MV_V2_WOL_CTRL,
> + MV_V2_WOL_MAGIC_PKT_EN,
> + MV_V2_WOL_CLEAR_STS);
> +
> + if (ret < 0)
> + return ret;
> +
> + ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2,
> + MV_V2_WOL_CTRL,
> + MV_V2_WOL_CLEAR_STS);
> +
> + if (ret < 0)
> + return ret;

This phy_clear_bits_mmd() is the same as the tail end of the other part
of the if() clause. Consider moving it after the if () { } else { }
statement...

> + }
> +
> + return ret;

and as all paths return "ret" just do:

return phy_clear_bits_mmd(...

I will also need to check whether this is the same as the 88x3310, but
I'm afraid I don't have the energy this evening - please email me a
remind to look at this tomorrow. Thanks.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

\
 
 \ /
  Last update: 2021-06-23 22:06    [W:0.061 / U:0.704 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site