[PATCH net-next 0/2] Add mdiobus_modify_changed() helper

Hi,

Sean Anderson's recent patch series is introducing more read-write
operations on the MDIO bus that only need to happen if a change is
being made.

We have similar logic in __mdiobus_modify_changed(), but we didn't
add its correponding locked variant mdiobus_modify_changed() as we
had very few users. Now that we are getting more, let's add the
helper.

 drivers/net/phy/mdio_bus.c | 22 ++++++++++++++++++++++
 drivers/net/phy/phylink.c  | 28 ++++------------------------
 include/linux/mdio.h       |  2 ++
 3 files changed, 28 insertions(+), 24 deletions(-)

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

Add mdiobus_modify_changed() helper to reflect the phylib and similar
equivalents. This will avoid this functionality being open-coded, as
has already happened in phylink, and it looks like other users will be
appearing soon.

Signed-off-by: Russell King (Oracle) <redacted>
---
 drivers/net/phy/mdio_bus.c | 22 ++++++++++++++++++++++
 include/linux/mdio.h       |  2 ++
 2 files changed, 24 insertions(+)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index d25411ae1796..8c59eb6a2b68 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c

@@ -926,6 +926,28 @@ int mdiobus_modify(struct mii_bus *bus, int addr, u32 regnum, u16 mask, u16 set)
 }
 EXPORT_SYMBOL_GPL(mdiobus_modify);
 
+/**
+ * mdiobus_modify_changed - Convenience function for modifying a given mdio
+ *	device register and returning if it changed
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @regnum: register number to write
+ * @mask: bit mask of bits to clear
+ * @set: bit mask of bits to set
+ */
+int mdiobus_modify_changed(struct mii_bus *bus, int addr, u32 regnum,
+			   u16 mask, u16 set)
+{
+	int err;
+
+	mutex_lock(&bus->mdio_lock);
+	err = __mdiobus_modify_changed(bus, addr, regnum, mask, set);
+	mutex_unlock(&bus->mdio_lock);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(mdiobus_modify_changed);
+
 /**
  * mdio_bus_match - determine if given MDIO driver supports the given
  *		    MDIO device

diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index 5e6dc38f418e..f622888a4ba8 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h

@@ -349,6 +349,8 @@ int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
 int mdiobus_write_nested(struct mii_bus *bus, int addr, u32 regnum, u16 val);
 int mdiobus_modify(struct mii_bus *bus, int addr, u32 regnum, u16 mask,
 		   u16 set);
+int mdiobus_modify_changed(struct mii_bus *bus, int addr, u32 regnum,
+			   u16 mask, u16 set);
 
 static inline u32 mdiobus_c45_addr(int devad, u16 regnum)
 {

-- 
2.30.2

Use the mdiobus_modify_changed() helper in the C22 PCS advertisement
helper.

Signed-off-by: Russell King (Oracle) <redacted>
---
 drivers/net/phy/phylink.c | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index b32774fd65f8..d76362028752 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c

@@ -2610,32 +2610,12 @@ int phylink_mii_c22_pcs_set_advertisement(struct mdio_device *pcs,
 				      advertising))
 			adv |= ADVERTISE_1000XPSE_ASYM;
 
-		val = mdiobus_read(bus, addr, MII_ADVERTISE);
-		if (val < 0)
-			return val;
-
-		if (val == adv)
-			return 0;
-
-		ret = mdiobus_write(bus, addr, MII_ADVERTISE, adv);
-		if (ret < 0)
-			return ret;
-
-		return 1;
+		return mdiobus_modify_changed(bus, addr, MII_ADVERTISE,
+					      0xffff, adv);
 
 	case PHY_INTERFACE_MODE_SGMII:
-		val = mdiobus_read(bus, addr, MII_ADVERTISE);
-		if (val < 0)
-			return val;
-
-		if (val == 0x0001)
-			return 0;
-
-		ret = mdiobus_write(bus, addr, MII_ADVERTISE, 0x0001);
-		if (ret < 0)
-			return ret;
-
-		return 1;
+		return mdiobus_modify_changed(bus, addr, MII_ADVERTISE,
+					      0xffff, 0x0001);
 
 	default:
 		/* Nothing to do for other modes */

-- 
2.30.2

On Tue, Oct 05, 2021 at 04:33:57PM +0100, Russell King (Oracle) wrote:

Add mdiobus_modify_changed() helper to reflect the phylib and similar
equivalents. This will avoid this functionality being open-coded, as
has already happened in phylink, and it looks like other users will be
appearing soon.

Signed-off-by: Russell King (Oracle) <redacted>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

On Tue, Oct 05, 2021 at 04:34:02PM +0100, Russell King (Oracle) wrote:

Use the mdiobus_modify_changed() helper in the C22 PCS advertisement
helper.

Signed-off-by: Russell King (Oracle) <redacted>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

On Tue, 05 Oct 2021 16:34:02 +0100 Russell King (Oracle) wrote:

drivers/net/phy/phylink.c: In function ‘phylink_mii_c22_pcs_set_advertisement’:
drivers/net/phy/phylink.c:2599:11: warning: unused variable ‘ret’ [-Wunused-variable]
 2599 |  int val, ret;
      |           ^~~
drivers/net/phy/phylink.c:2599:6: warning: unused variable ‘val’ [-Wunused-variable]
 2599 |  int val, ret;
      |      ^~~