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

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

[Russell King (Oracle)]

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.

v2: fix build warnings in patch2, add Andrew's RB to patch 1

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

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

[PATCH v2 net-next 1/2] net: mdio: add mdiobus_modify_changed()

[Russell King (Oracle)]

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.

iReviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 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

[PATCH v2 net-next 2/2] net: phylink: use mdiobus_modify_changed() helper

[Russell King (Oracle)]

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

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
v2: fix build warnings

 drivers/net/phy/phylink.c | 29 ++++-------------------------
 1 file changed, 4 insertions(+), 25 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index b32774fd65f8..16240f2dd161 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -2596,7 +2596,6 @@ int phylink_mii_c22_pcs_set_advertisement(struct mdio_device *pcs,
 {
 	struct mii_bus *bus = pcs->bus;
 	int addr = pcs->addr;
-	int val, ret;
 	u16 adv;
 
 	switch (interface) {
@@ -2610,32 +2609,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

Re: [PATCH v2 net-next 2/2] net: phylink: use mdiobus_modify_changed() helper

[Andrew Lunn]

On Wed, Oct 06, 2021 at 01:19:25PM +0100, Russell King (Oracle) wrote:
> Use the mdiobus_modify_changed() helper in the C22 PCS advertisement
> helper.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

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

    Andrew

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

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Wed, 6 Oct 2021 13:18:41 +0100 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [v2,net-next,1/2] net: mdio: add mdiobus_modify_changed()
    applied by Jakub Kicinski <kuba@kernel.org>
    https://git.kernel.org/netdev/net-next/c/79365f36d1de
  - [v2,net-next,2/2] net: phylink: use mdiobus_modify_changed() helper
    applied by Jakub Kicinski <kuba@kernel.org>
    https://git.kernel.org/netdev/net-next/c/078e0b5363db

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

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

[Russell King (Oracle)]

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!