Messages in this thread | | | Subject | Re: [PATCH net 1/2] net: phy: DP83822: Fix WoL in config init to be disabled | From | Heiner Kallweit <> | Date | Thu, 23 Apr 2020 19:02:44 +0200 |
| |
On 23.04.2020 18:39, Dan Murphy wrote: > The WoL feature should be disabled when config_init is called and the > feature should turned on or off when set_wol is called. > > In addition updated the calls to modify the registers to use the set_bit > and clear_bit function calls. > > Fixes: 3b427751a9d0 ("net: phy: DP83822 initial driver submission") > Signed-off-by: Dan Murphy <dmurphy@ti.com> > --- > drivers/net/phy/dp83822.c | 30 ++++++++++++++++-------------- > 1 file changed, 16 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c > index fe9aa3ad52a7..40fdfd043947 100644 > --- a/drivers/net/phy/dp83822.c > +++ b/drivers/net/phy/dp83822.c > @@ -137,16 +137,19 @@ static int dp83822_set_wol(struct phy_device *phydev, > value &= ~DP83822_WOL_SECURE_ON; > } > > - value |= (DP83822_WOL_EN | DP83822_WOL_INDICATION_SEL | > - DP83822_WOL_CLR_INDICATION); > - phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG, > - value); > + /* Clear any pending WoL interrupt */ > + phy_read(phydev, MII_DP83822_MISR2); > + > + value |= DP83822_WOL_EN | DP83822_WOL_INDICATION_SEL | > + DP83822_WOL_CLR_INDICATION; > + > + return phy_set_bits_mmd(phydev, DP83822_DEVADDR, > + MII_DP83822_WOL_CFG, value);
The switch to phy_set_bits_mmd() doesn't seem to be correct here. So far bit DP83822_WOL_MAGIC_EN is cleared if WAKE_MAGIC isn't set. Similar for bit DP83822_WOL_SECURE_ON. With your change they don't get cleared any longer.
> } else { > - value = phy_read_mmd(phydev, DP83822_DEVADDR, > - MII_DP83822_WOL_CFG); > - value &= ~DP83822_WOL_EN; > - phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG, > - value); > + value = DP83822_WOL_EN | DP83822_WOL_CLR_INDICATION; > + You clear one bit more than before. The reason for this may be worth a note in the commit message.
> + return phy_clear_bits_mmd(phydev, DP83822_DEVADDR, > + MII_DP83822_WOL_CFG, value); > } > > return 0; > @@ -258,12 +261,11 @@ static int dp83822_config_intr(struct phy_device *phydev) > > static int dp83822_config_init(struct phy_device *phydev) > { > - int value; > - > - value = DP83822_WOL_MAGIC_EN | DP83822_WOL_SECURE_ON | DP83822_WOL_EN; > + int value = DP83822_WOL_EN | DP83822_WOL_MAGIC_EN | > + DP83822_WOL_SECURE_ON; > > - return phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG, > - value); > + return phy_clear_bits_mmd(phydev, DP83822_DEVADDR, > + MII_DP83822_WOL_CFG, value); > } > > static int dp83822_phy_reset(struct phy_device *phydev) >
| |