On Mon, Nov 30, 2020 at 06:16:39PM +0800, Yejune Deng wrote: > a set of phy_set_bits() looks more neater > > Signed-off-by: Yejune Deng <yejune.d...@gmail.com>
Sorry, but NAK. You seem to be doing a mechanical code change without first understanding the code, as the patch shows no sign of an understanding of the difference between phy_modify() and __phy_modify(). This means you are introducing new bugs with this change. Please investigate the differences between the two variants of phy_modify() and post a more correct patch. Thanks. > --- > drivers/net/phy/marvell.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c > index 587930a..f402e7f 100644 > --- a/drivers/net/phy/marvell.c > +++ b/drivers/net/phy/marvell.c > @@ -1132,8 +1132,8 @@ static int m88e1510_config_init(struct phy_device > *phydev) > return err; > > /* PHY reset is necessary after changing MODE[2:0] */ > - err = phy_modify(phydev, MII_88E1510_GEN_CTRL_REG_1, 0, > - MII_88E1510_GEN_CTRL_REG_1_RESET); > + err = phy_set_bits(phydev, MII_88E1510_GEN_CTRL_REG_1, > + MII_88E1510_GEN_CTRL_REG_1_RESET); > if (err < 0) > return err; > > @@ -1725,7 +1725,7 @@ static int m88e1318_set_wol(struct phy_device *phydev, > __phy_read(phydev, MII_M1011_IEVENT); > > /* Enable the WOL interrupt */ > - err = __phy_modify(phydev, MII_88E1318S_PHY_CSIER, 0, > + err = phy_set_bits(phydev, MII_88E1318S_PHY_CSIER, > MII_88E1318S_PHY_CSIER_WOL_EIE); > if (err < 0) > goto error; > @@ -1735,10 +1735,10 @@ static int m88e1318_set_wol(struct phy_device *phydev, > goto error; > > /* Setup LED[2] as interrupt pin (active low) */ > - err = __phy_modify(phydev, MII_88E1318S_PHY_LED_TCR, > - MII_88E1318S_PHY_LED_TCR_FORCE_INT, > - MII_88E1318S_PHY_LED_TCR_INTn_ENABLE | > - MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW); > + err = phy_modify(phydev, MII_88E1318S_PHY_LED_TCR, > + MII_88E1318S_PHY_LED_TCR_FORCE_INT, > + MII_88E1318S_PHY_LED_TCR_INTn_ENABLE | > + MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW); > if (err < 0) > goto error; > > @@ -1764,7 +1764,7 @@ static int m88e1318_set_wol(struct phy_device *phydev, > goto error; > > /* Clear WOL status and enable magic packet matching */ > - err = __phy_modify(phydev, MII_88E1318S_PHY_WOL_CTRL, 0, > + err = phy_set_bits(phydev, MII_88E1318S_PHY_WOL_CTRL, > MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS | > > MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE); > if (err < 0) > @@ -1775,9 +1775,9 @@ static int m88e1318_set_wol(struct phy_device *phydev, > goto error; > > /* Clear WOL status and disable magic packet matching */ > - err = __phy_modify(phydev, MII_88E1318S_PHY_WOL_CTRL, > - > MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE, > - MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS); > + err = phy_modify(phydev, MII_88E1318S_PHY_WOL_CTRL, > + > MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE, > + MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS); > if (err < 0) > goto error; > } > @@ -1995,7 +1995,7 @@ static int marvell_cable_test_start_common(struct > phy_device *phydev) > return bmsr; > > if (bmcr & BMCR_ANENABLE) { > - ret = phy_modify(phydev, MII_BMCR, BMCR_ANENABLE, 0); > + ret = phy_clear_bits(phydev, MII_BMCR, BMCR_ANENABLE); > if (ret < 0) > return ret; > ret = genphy_soft_reset(phydev); > -- > 1.9.1 > > -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!