| Message ID | 1606731399-8772-1-git-send-email-yejune.deng@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | net: phy: marvell: replace __phy_modify() | expand |
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.deng@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 > >
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);
a set of phy_set_bits() looks more neater Signed-off-by: Yejune Deng <yejune.deng@gmail.com> --- drivers/net/phy/marvell.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)