[PATCH 1/7] ARM: boards: make use of MDIO_MMD register defines

[PATCH 1/7] ARM: boards: make use of MDIO_MMD register defines

[Marco Felsch]

Make use of the register definition instead of having magic numbers. No
functional change.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 arch/arm/boards/datamodul-edm-qmx6/board.c   | 7 ++++---
 arch/arm/boards/embest-marsboard/board.c     | 7 ++++---
 arch/arm/boards/terasic-de0-nano-soc/board.c | 7 ++++---
 arch/arm/boards/terasic-de10-nano/board.c    | 7 ++++---
 arch/arm/boards/tqma6x/board.c               | 7 ++++---
 5 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/arch/arm/boards/datamodul-edm-qmx6/board.c b/arch/arm/boards/datamodul-edm-qmx6/board.c
index 9adb3ee0f8..366b64d35a 100644
--- a/arch/arm/boards/datamodul-edm-qmx6/board.c
+++ b/arch/arm/boards/datamodul-edm-qmx6/board.c
@@ -11,6 +11,7 @@
 #include <gpio.h>
 #include <of.h>
 
+#include <linux/mdio.h>
 #include <linux/micrel_phy.h>
 #include <mfd/stmpe-i2c.h>
 
@@ -48,9 +49,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
 	 * min rx data delay, max rx/tx clock delay,
 	 * min rx/tx control delay
 	 */
-	phy_write_mmd_indirect(dev, 4, 2, 0);
-	phy_write_mmd_indirect(dev, 5, 2, 0);
-	phy_write_mmd_indirect(dev, 8, 2, 0x03ff);
+	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
+	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
+	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x03ff);
 
 	return 0;
 }
diff --git a/arch/arm/boards/embest-marsboard/board.c b/arch/arm/boards/embest-marsboard/board.c
index 7835a9265a..7274595e2a 100644
--- a/arch/arm/boards/embest-marsboard/board.c
+++ b/arch/arm/boards/embest-marsboard/board.c
@@ -9,6 +9,7 @@
 #include <init.h>
 #include <envfs.h>
 #include <mach/imx/bbu.h>
+#include <linux/mdio.h>
 #include <linux/phy.h>
 #include <deep-probe.h>
 
@@ -19,13 +20,13 @@ static int ar8035_phy_fixup(struct phy_device *dev)
 	/* Ar803x phy SmartEEE feature cause link status generates glitch,
 	 * which cause ethernet link down/up issue, so disable SmartEEE
 	 */
-	val = phy_read_mmd_indirect(dev, 0x805d, 0x3);
+	val = phy_read_mmd_indirect(dev, 0x805d, MDIO_MMD_PCS);
 	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
 
-	val = phy_read_mmd_indirect(dev, 0x4003, 0x3);
+	val = phy_read_mmd_indirect(dev, 0x4003, MDIO_MMD_PCS);
 	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
 
-	val = phy_read_mmd_indirect(dev, 0x4007, 0x3);
+	val = phy_read_mmd_indirect(dev, 0x4007, MDIO_MMD_PCS);
 	val &= 0xffe3;
 	val |= 0x18;
 	phy_write(dev, MII_MMD_DATA, val);
diff --git a/arch/arm/boards/terasic-de0-nano-soc/board.c b/arch/arm/boards/terasic-de0-nano-soc/board.c
index 19f74b784c..832160c595 100644
--- a/arch/arm/boards/terasic-de0-nano-soc/board.c
+++ b/arch/arm/boards/terasic-de0-nano-soc/board.c
@@ -5,6 +5,7 @@
 #include <driver.h>
 #include <init.h>
 #include <asm/armlinux.h>
+#include <linux/mdio.h>
 #include <linux/micrel_phy.h>
 #include <linux/phy.h>
 #include <linux/sizes.h>
@@ -18,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
 	 * min rx data delay, max rx/tx clock delay,
 	 * min rx/tx control delay
 	 */
-	phy_write_mmd_indirect(dev, 4, 2, 0);
-	phy_write_mmd_indirect(dev, 5, 2, 0);
-	phy_write_mmd_indirect(dev, 8, 2, 0x003ff);
+	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
+	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
+	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
 	return 0;
 }
 
diff --git a/arch/arm/boards/terasic-de10-nano/board.c b/arch/arm/boards/terasic-de10-nano/board.c
index 580c898012..e47d9ac841 100644
--- a/arch/arm/boards/terasic-de10-nano/board.c
+++ b/arch/arm/boards/terasic-de10-nano/board.c
@@ -5,6 +5,7 @@
 #include <driver.h>
 #include <init.h>
 #include <asm/armlinux.h>
+#include <linux/mdio.h>
 #include <linux/micrel_phy.h>
 #include <linux/phy.h>
 #include <linux/sizes.h>
@@ -18,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
 	 * min rx data delay, max rx/tx clock delay,
 	 * min rx/tx control delay
 	 */
-	phy_write_mmd_indirect(dev, 4, 2, 0);
-	phy_write_mmd_indirect(dev, 5, 2, 0);
-	phy_write_mmd_indirect(dev, 8, 2, 0x003ff);
+	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
+	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
+	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
 	return 0;
 }
 
diff --git a/arch/arm/boards/tqma6x/board.c b/arch/arm/boards/tqma6x/board.c
index 4bb7223a6e..8a91ad652a 100644
--- a/arch/arm/boards/tqma6x/board.c
+++ b/arch/arm/boards/tqma6x/board.c
@@ -11,6 +11,7 @@
 #include <gpio.h>
 #include <of.h>
 
+#include <linux/mdio.h>
 #include <linux/micrel_phy.h>
 #include <mfd/stmpe-i2c.h>
 
@@ -46,9 +47,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
 	 * min rx data delay, max rx/tx clock delay,
 	 * min rx/tx control delay
 	 */
-	phy_write_mmd_indirect(dev, 4, 2, 0);
-	phy_write_mmd_indirect(dev, 5, 2, 0);
-	phy_write_mmd_indirect(dev, 8, 2, 0x003ff);
+	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
+	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
+	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
 
 	return 0;
 }
-- 
2.39.2

[PATCH 2/7] net: phy: micrel: make use of MDIO_MMD register defines

[Marco Felsch]

Make use of the register definition instead of having magic numbers. No
functional change.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/net/phy/micrel.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index ef1f919ae7..02d474c442 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -447,23 +447,27 @@ static int ksz9031_config_rgmii_delay(struct phy_device *phydev)
 		return 0;
 	}
 
-	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CONTROL_PAD_SKEW, 2,
+	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CONTROL_PAD_SKEW,
+			       MDIO_MMD_WIS,
 			       FIELD_PREP(MII_KSZ9031RN_RX_CTL_M, rx) |
 			       FIELD_PREP(MII_KSZ9031RN_TX_CTL_M, tx));
 
-	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_RX_DATA_PAD_SKEW, 2,
+	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_RX_DATA_PAD_SKEW,
+			       MDIO_MMD_WIS,
 			       FIELD_PREP(MII_KSZ9031RN_RXD3, rx) |
 			       FIELD_PREP(MII_KSZ9031RN_RXD2, rx) |
 			       FIELD_PREP(MII_KSZ9031RN_RXD1, rx) |
 			       FIELD_PREP(MII_KSZ9031RN_RXD0, rx));
 
-	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_TX_DATA_PAD_SKEW, 2,
+	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_TX_DATA_PAD_SKEW,
+			       MDIO_MMD_WIS,
 			       FIELD_PREP(MII_KSZ9031RN_TXD3, tx) |
 			       FIELD_PREP(MII_KSZ9031RN_TXD2, tx) |
 			       FIELD_PREP(MII_KSZ9031RN_TXD1, tx) |
 			       FIELD_PREP(MII_KSZ9031RN_TXD0, tx));
 
-	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CLK_PAD_SKEW, 2,
+	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CLK_PAD_SKEW,
+			       MDIO_MMD_WIS,
 			       FIELD_PREP(MII_KSZ9031RN_GTX_CLK, tx_clk) |
 			       FIELD_PREP(MII_KSZ9031RN_RX_CLK, rx_clk));
 	return 0;
-- 
2.39.2

[PATCH 3/7] net: phy: sync phy_{write,read,modify}_mmd_indirect with linux

[Marco Felsch]

Sync the barebox phy_{write,read,modify}_mmd_indirect API with the Linux
phy_{write,read,modify}_mmd API to make it easier and less error prone
to port phy drivers. This also fixes the r8169 driver since this driver
did not adapt the parameters while porting from Linux.

The sync also aligns the function parameter `prtad` as well as the
function documentation.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 arch/arm/boards/datamodul-edm-qmx6/board.c   |  6 +--
 arch/arm/boards/embest-marsboard/board.c     |  6 +--
 arch/arm/boards/terasic-de0-nano-soc/board.c |  6 +--
 arch/arm/boards/terasic-de10-nano/board.c    |  6 +--
 arch/arm/boards/tqma6x/board.c               |  6 +--
 drivers/net/phy/at803x.c                     |  4 +-
 drivers/net/phy/dp83867.c                    | 40 +++++++-------
 drivers/net/phy/micrel.c                     | 24 ++++-----
 drivers/net/phy/phy.c                        | 57 ++++++++++----------
 include/linux/phy.h                          | 10 ++--
 10 files changed, 83 insertions(+), 82 deletions(-)

diff --git a/arch/arm/boards/datamodul-edm-qmx6/board.c b/arch/arm/boards/datamodul-edm-qmx6/board.c
index 366b64d35a..0dc71f2aca 100644
--- a/arch/arm/boards/datamodul-edm-qmx6/board.c
+++ b/arch/arm/boards/datamodul-edm-qmx6/board.c
@@ -49,9 +49,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
 	 * min rx data delay, max rx/tx clock delay,
 	 * min rx/tx control delay
 	 */
-	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
-	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
-	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x03ff);
+	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
+	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
+	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x03ff);
 
 	return 0;
 }
diff --git a/arch/arm/boards/embest-marsboard/board.c b/arch/arm/boards/embest-marsboard/board.c
index 7274595e2a..e70fefec5e 100644
--- a/arch/arm/boards/embest-marsboard/board.c
+++ b/arch/arm/boards/embest-marsboard/board.c
@@ -20,13 +20,13 @@ static int ar8035_phy_fixup(struct phy_device *dev)
 	/* Ar803x phy SmartEEE feature cause link status generates glitch,
 	 * which cause ethernet link down/up issue, so disable SmartEEE
 	 */
-	val = phy_read_mmd_indirect(dev, 0x805d, MDIO_MMD_PCS);
+	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x805d);
 	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
 
-	val = phy_read_mmd_indirect(dev, 0x4003, MDIO_MMD_PCS);
+	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x4003);
 	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
 
-	val = phy_read_mmd_indirect(dev, 0x4007, MDIO_MMD_PCS);
+	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x4007);
 	val &= 0xffe3;
 	val |= 0x18;
 	phy_write(dev, MII_MMD_DATA, val);
diff --git a/arch/arm/boards/terasic-de0-nano-soc/board.c b/arch/arm/boards/terasic-de0-nano-soc/board.c
index 832160c595..fa83498f46 100644
--- a/arch/arm/boards/terasic-de0-nano-soc/board.c
+++ b/arch/arm/boards/terasic-de0-nano-soc/board.c
@@ -19,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
 	 * min rx data delay, max rx/tx clock delay,
 	 * min rx/tx control delay
 	 */
-	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
-	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
-	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
+	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
+	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
+	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
 	return 0;
 }
 
diff --git a/arch/arm/boards/terasic-de10-nano/board.c b/arch/arm/boards/terasic-de10-nano/board.c
index e47d9ac841..7ceb691f71 100644
--- a/arch/arm/boards/terasic-de10-nano/board.c
+++ b/arch/arm/boards/terasic-de10-nano/board.c
@@ -19,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
 	 * min rx data delay, max rx/tx clock delay,
 	 * min rx/tx control delay
 	 */
-	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
-	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
-	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
+	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
+	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
+	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
 	return 0;
 }
 
diff --git a/arch/arm/boards/tqma6x/board.c b/arch/arm/boards/tqma6x/board.c
index 8a91ad652a..3fd2e1db2c 100644
--- a/arch/arm/boards/tqma6x/board.c
+++ b/arch/arm/boards/tqma6x/board.c
@@ -47,9 +47,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
 	 * min rx data delay, max rx/tx clock delay,
 	 * min rx/tx control delay
 	 */
-	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
-	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
-	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
+	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
+	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
+	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
 
 	return 0;
 }
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 18182bffc2..41010c58ad 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -229,14 +229,14 @@ static int at803x_clk_out_config(struct phy_device *phydev)
 	if (!priv->clk_25m_mask)
 		return 0;
 
-	val = phy_read_mmd_indirect(phydev, AT803X_MMD7_CLK25M, MDIO_MMD_AN);
+	val = phy_read_mmd_indirect(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M);
 	if (val < 0)
 		return val;
 
 	val &= ~priv->clk_25m_mask;
 	val |= priv->clk_25m_reg;
 
-	phy_write_mmd_indirect(phydev, AT803X_MMD7_CLK25M, MDIO_MMD_AN, val);
+	phy_write_mmd_indirect(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M, val);
 
 	return 0;
 }
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index d8109172df..85fe5107da 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -154,14 +154,14 @@ static int dp83867_config_port_mirroring(struct phy_device *phydev)
 	struct dp83867_private *dp83867 = phydev->priv;
 	u16 val;
 
-	val = phy_read_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR);
+	val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR, DP83867_CFG4);
 
 	if (dp83867->port_mirroring == DP83867_PORT_MIRROING_EN)
 		val |= DP83867_CFG4_PORT_MIRROR_EN;
 	else
 		val &= ~DP83867_CFG4_PORT_MIRROR_EN;
 
-	phy_write_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR, val);
+	phy_write_mmd_indirect(phydev, DP83867_DEVADDR, DP83867_CFG4, val);
 
 	return 0;
 }
@@ -256,11 +256,11 @@ static int dp83867_config_init(struct phy_device *phydev)
 	phy_write(phydev, DP83867_CTRL, val | DP83867_SW_RESTART);
 
 	if (dp83867->rxctrl_strap_quirk) {
-		val = phy_read_mmd_indirect(phydev, DP83867_CFG4,
-					    DP83867_DEVADDR);
+		val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
+					    DP83867_CFG4);
 		val &= ~BIT(7);
-		phy_write_mmd_indirect(phydev, DP83867_CFG4,
-				       DP83867_DEVADDR, val);
+		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
+				       DP83867_CFG4, val);
 	}
 
 	if (phy_interface_is_rgmii(phydev)) {
@@ -270,8 +270,8 @@ static int dp83867_config_init(struct phy_device *phydev)
 		if (ret)
 			return ret;
 
-		val = phy_read_mmd_indirect(phydev, DP83867_RGMIICTL,
-					    DP83867_DEVADDR);
+		val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
+					    DP83867_RGMIICTL);
 
 		switch (phydev->interface) {
 		case PHY_INTERFACE_MODE_RGMII_ID:
@@ -287,31 +287,31 @@ static int dp83867_config_init(struct phy_device *phydev)
 		default:
 			break;
 		}
-		phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
-				       DP83867_DEVADDR, val);
+		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
+				       DP83867_RGMIICTL, val);
 
 		delay = (dp83867->rx_id_delay |
 			(dp83867->tx_id_delay << DP83867_RGMII_TX_CLK_DELAY_SHIFT));
 
-		phy_write_mmd_indirect(phydev, DP83867_RGMIIDCTL,
-				       DP83867_DEVADDR, delay);
+		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
+				       DP83867_RGMIIDCTL, delay);
 
 		if (dp83867->io_impedance >= 0) {
-			val = phy_read_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
-						    DP83867_DEVADDR);
+			val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
+						    DP83867_IO_MUX_CFG);
 			val &= ~DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
 			val |= (dp83867->io_impedance &
 				DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL);
 
-			phy_write_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
-					       DP83867_DEVADDR, val);
+			phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
+					       DP83867_IO_MUX_CFG, val);
 		}
 	} else if (phy_interface_is_sgmii(phydev)) {
 		phy_write(phydev, MII_BMCR,
 			  BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000);
 
-		phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
-				       DP83867_DEVADDR, 0x0);
+		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
+				       DP83867_RGMIICTL, 0x0);
 
 		val = DP83867_PHYCTRL_SGMIIEN |
 		      DP83867_MDI_CROSSOVER_MDIX << DP83867_MDI_CROSSOVER |
@@ -341,8 +341,8 @@ static int dp83867_config_init(struct phy_device *phydev)
 				DP83867_IO_MUX_CFG_CLK_O_SEL_SHIFT;
 		}
 
-		phy_modify_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
-				DP83867_DEVADDR, mask, val);
+		phy_modify_mmd_indirect(phydev, DP83867_DEVADDR,
+					DP83867_IO_MUX_CFG, mask, val);
 	}
 
 	return 0;
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 02d474c442..892df01d2e 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -387,7 +387,7 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
 		return 0;
 
 	if (matches < numfields)
-		newval = phy_read_mmd_indirect(phydev, reg, MDIO_MMD_WIS);
+		newval = phy_read_mmd_indirect(phydev, MDIO_MMD_WIS, reg);
 	else
 		newval = 0;
 
@@ -401,15 +401,15 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
 				<< (field_sz * i));
 		}
 
-	phy_write_mmd_indirect(phydev, reg, MDIO_MMD_WIS, newval);
+	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS, reg, newval);
 	return 0;
 }
 
 static int ksz9031_center_flp_timing(struct phy_device *phydev)
 {
 	/* Center KSZ9031RNX FLP timing at 16ms. */
-	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_HI, 0, 0x0006);
-	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_LO, 0, 0x1a80);
+	phy_write_mmd_indirect(phydev, 0, MII_KSZ9031RN_FLP_BURST_TX_HI, 0x0006);
+	phy_write_mmd_indirect(phydev, 0, MII_KSZ9031RN_FLP_BURST_TX_LO, 0x1a80);
 
 	return genphy_restart_aneg(phydev);
 }
@@ -447,27 +447,27 @@ static int ksz9031_config_rgmii_delay(struct phy_device *phydev)
 		return 0;
 	}
 
-	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CONTROL_PAD_SKEW,
-			       MDIO_MMD_WIS,
+	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
+			       MII_KSZ9031RN_CONTROL_PAD_SKEW,
 			       FIELD_PREP(MII_KSZ9031RN_RX_CTL_M, rx) |
 			       FIELD_PREP(MII_KSZ9031RN_TX_CTL_M, tx));
 
-	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_RX_DATA_PAD_SKEW,
-			       MDIO_MMD_WIS,
+	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
+			       MII_KSZ9031RN_RX_DATA_PAD_SKEW,
 			       FIELD_PREP(MII_KSZ9031RN_RXD3, rx) |
 			       FIELD_PREP(MII_KSZ9031RN_RXD2, rx) |
 			       FIELD_PREP(MII_KSZ9031RN_RXD1, rx) |
 			       FIELD_PREP(MII_KSZ9031RN_RXD0, rx));
 
-	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_TX_DATA_PAD_SKEW,
-			       MDIO_MMD_WIS,
+	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
+			       MII_KSZ9031RN_TX_DATA_PAD_SKEW,
 			       FIELD_PREP(MII_KSZ9031RN_TXD3, tx) |
 			       FIELD_PREP(MII_KSZ9031RN_TXD2, tx) |
 			       FIELD_PREP(MII_KSZ9031RN_TXD1, tx) |
 			       FIELD_PREP(MII_KSZ9031RN_TXD0, tx));
 
-	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CLK_PAD_SKEW,
-			       MDIO_MMD_WIS,
+	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
+			       MII_KSZ9031RN_CLK_PAD_SKEW,
 			       FIELD_PREP(MII_KSZ9031RN_GTX_CLK, tx_clk) |
 			       FIELD_PREP(MII_KSZ9031RN_RX_CLK, rx_clk));
 	return 0;
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 74381949b4..283e1a3d79 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -819,24 +819,25 @@ int genphy_read_status(struct phy_device *phydev)
 	return 0;
 }
 
-static inline void mmd_phy_indirect(struct phy_device *phydev, int prtad,
-					int devad)
+static inline void mmd_phy_indirect(struct phy_device *phydev, int devad,
+				    u16 regnum)
 {
 	/* Write the desired MMD Devad */
 	phy_write(phydev, MII_MMD_CTRL, devad);
 
 	/* Write the desired MMD register address */
-	phy_write(phydev, MII_MMD_DATA, prtad);
+	phy_write(phydev, MII_MMD_DATA, regnum);
 
 	/* Select the Function : DATA with no post increment */
 	phy_write(phydev, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
 }
 
 /**
- * phy_read_mmd_indirect - reads data from the MMD registers
- * @phy_device: phy device
- * @prtad: MMD Address
- * @devad: MMD DEVAD
+ * phy_read_mmd_indirect - Convenience function for reading a register
+ * from an MMD on a given PHY.
+ * @phydev: The phy_device struct
+ * @devad: The MMD to read from
+ * @regnum: The register on the MMD to read
  *
  * Description: it reads data from the MMD registers (clause 22 to access to
  * clause 45) of the specified phy address.
@@ -846,11 +847,11 @@ static inline void mmd_phy_indirect(struct phy_device *phydev, int prtad,
  * 3) Write reg 13 // MMD Data Command for MMD DEVAD
  * 3) Read  reg 14 // Read MMD data
  */
-int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
+int phy_read_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum)
 {
 	u32 ret;
 
-	mmd_phy_indirect(phydev, prtad, devad);
+	mmd_phy_indirect(phydev, devad, regnum);
 
 	/* Read the content of the MMD's selected register */
 	ret = phy_read(phydev, MII_MMD_DATA);
@@ -859,11 +860,12 @@ int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
 }
 
 /**
- * phy_write_mmd_indirect - writes data to the MMD registers
- * @phy_device: phy device
- * @prtad: MMD Address
- * @devad: MMD DEVAD
- * @data: data to write in the MMD register
+ * phy_write_mmd_indirect - Convenience function for writing a register
+ * on an MMD on a given PHY.
+ * @phydev: The phy_device struct
+ * @devad: The MMD to read from
+ * @regnum: The register on the MMD to read
+ * @val: value to write to @regnum
  *
  * Description: Write data from the MMD registers of the specified
  * phy address.
@@ -873,34 +875,33 @@ int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
  * 3) Write reg 13 // MMD Data Command for MMD DEVAD
  * 3) Write reg 14 // Write MMD data
  */
-void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
-				   u16 data)
+void phy_write_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
+			    u16 val)
 {
-	mmd_phy_indirect(phydev, prtad, devad);
+	mmd_phy_indirect(phydev, devad, regnum);
 
 	/* Write the data into MMD's selected register */
-	phy_write(phydev, MII_MMD_DATA, data);
+	phy_write(phydev, MII_MMD_DATA, val);
 }
 
 /**
- * phy_modify_mmd_indirect - Convenience function for modifying a MMD register
- * @phydev: phy device
- * @prtad: MMD Address
- * @devad: MMD DEVAD
+ * phy_modify_mmd_indirect - Convenience function for modifying a register on MMD
+ * @phydev: the phy_device struct
+ * @devad: the MMD containing register to modify
+ * @regnum: register number to modify
  * @mask: bit mask of bits to clear
- * @set: new value of bits set in @mask
- *
+ * @set: new value of bits set in mask to write to @regnum
  */
-int phy_modify_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
-				   u16 mask, u16 set)
+int phy_modify_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
+			    u16 mask, u16 set)
 {
 	int ret;
 
-	ret = phy_read_mmd_indirect(phydev, prtad, devad);
+	ret = phy_read_mmd_indirect(phydev, devad, regnum);
 	if (ret < 0)
 		return ret;
 
-	phy_write_mmd_indirect(phydev, prtad, devad, (ret & ~mask) | set);
+	phy_write_mmd_indirect(phydev, devad, regnum, (ret & ~mask) | set);
 
 	return 0;
 }
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 509bf72de9..d96c4e97ab 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -406,11 +406,11 @@ int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
 int phy_register_fixup_for_id(const char *bus_id,
 		int (*run)(struct phy_device *));
 int phy_scan_fixups(struct phy_device *phydev);
-int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad);
-void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
-				   u16 data);
-int phy_modify_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
-				    u16 mask, u16 set);
+int phy_read_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum);
+void phy_write_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
+			    u16 val);
+int phy_modify_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
+			    u16 mask, u16 set);
 
 static inline bool phy_acquired(struct phy_device *phydev)
 {
-- 
2.39.2

[PATCH 4/7] net: phy: add phydev_{err,err_probe,info,warn,dbg} macros

[Marco Felsch]

Import Linux macros to make it easier to port drivers to barebox.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 include/linux/phy.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index d96c4e97ab..b0474e87df 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -417,6 +417,21 @@ static inline bool phy_acquired(struct phy_device *phydev)
 	return phydev && phydev->bus && slice_acquired(&phydev->bus->slice);
 }
 
+#define phydev_err(_phydev, format, args...)	\
+	dev_err(&_phydev->dev, format, ##args)
+
+#define phydev_err_probe(_phydev, err, format, args...)	\
+	dev_err_probe(&_phydev->dev, err, format, ##args)
+
+#define phydev_info(_phydev, format, args...)	\
+	dev_info(&_phydev->dev, format, ##args)
+
+#define phydev_warn(_phydev, format, args...)	\
+	dev_warn(&_phydev->dev, format, ##args)
+
+#define phydev_dbg(_phydev, format, args...)	\
+	dev_dbg(&_phydev->dev, format, ##args)
+
 #ifdef CONFIG_PHYLIB
 int phy_register_fixup_for_uid(u32 phy_uid, u32 phy_uid_mask,
 		int (*run)(struct phy_device *));
-- 
2.39.2

[PATCH 5/7] net: phy: at803x: add support for configuring SmartEEE

[Marco Felsch]

This commit port Linux commit:

| commit 390b4cad81484124db2b676ed20a265adc032bae
| Author: Russell King <rmk+kernel@armlinux.org.uk>
| Date:   Thu Jan 14 10:45:49 2021 +0000
|
|     net: phy: at803x: add support for configuring SmartEEE
|
|     SmartEEE for the atheros phy was deemed buggy by Freescale and commits
|     were added to disable it for their boards.
|
|     In initial testing, SolidRun found that the default settings were
|     causing disconnects but by increasing the Tw buffer time we could allow
|     enough time for all parts of the link to come out of a low power state
|     and function properly without causing a disconnect. This allows us to
|     have functional power savings of between 300 and 400mW, rather than
|     disabling the feature altogether.
|
|     This commit adds support for disabling SmartEEE and configuring the Tw
|     parameters for 1G and 100M speeds.
|
|     Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
|     Signed-off-by: Jakub Kicinski <kuba@kernel.org>

I slightly adapted the at803x_config_init() as well to be more in line
with the Linux code base.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/net/phy/at803x.c | 75 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 73 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 41010c58ad..cc425c318f 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -59,6 +59,15 @@
  */
 #define AT8035_CLK_OUT_MASK			GENMASK(4, 3)
 
+#define AT803X_MMD3_SMARTEEE_CTL1		0x805b
+#define AT803X_MMD3_SMARTEEE_CTL3		0x805d
+#define AT803X_MMD3_SMARTEEE_CTL3_LPI_EN	BIT(8)
+
+#define AT803X_DISABLE_SMARTEEE			BIT(1)
+
+/* disable hibernation mode */
+#define AT803X_DISABLE_HIBERNATION_MODE		BIT(2)
+
 #define AT803X_CLK_OUT_STRENGTH_MASK		GENMASK(8, 7)
 #define AT803X_CLK_OUT_STRENGTH_FULL		0
 #define AT803X_CLK_OUT_STRENGTH_HALF		1
@@ -72,8 +81,11 @@
 #define AT8030_PHY_ID_MASK			0xffffffef
 
 struct at803x_priv {
+	int flags;
 	u16 clk_25m_reg;
 	u16 clk_25m_mask;
+	u8 smarteee_lpi_tw_1g;
+	u8 smarteee_lpi_tw_100m;
 };
 
 static int at803x_debug_reg_read(struct phy_device *phydev, u16 reg)
@@ -141,10 +153,29 @@ static int at803x_parse_dt(struct phy_device *phydev)
 	const struct device *dev = &phydev->dev;
 	const struct device_node *node = dev->of_node;
 	struct at803x_priv *priv = phydev->priv;
+	u32 freq, strength, tw;
 	unsigned int sel;
-	u32 freq, strength;
 	int ret;
 
+	if (of_property_read_bool(node, "qca,disable-smarteee"))
+		priv->flags |= AT803X_DISABLE_SMARTEEE;
+
+	if (!of_property_read_u32(node, "qca,smarteee-tw-us-1g", &tw)) {
+		if (!tw || tw > 255) {
+			phydev_err(phydev, "invalid qca,smarteee-tw-us-1g\n");
+			return -EINVAL;
+		}
+		priv->smarteee_lpi_tw_1g = tw;
+	}
+
+	if (!of_property_read_u32(node, "qca,smarteee-tw-us-100m", &tw)) {
+		if (!tw || tw > 255) {
+			phydev_err(phydev, "invalid qca,smarteee-tw-us-100m\n");
+			return -EINVAL;
+		}
+		priv->smarteee_lpi_tw_100m = tw;
+	}
+
 	ret = of_property_read_u32(node, "qca,clk-out-frequency", &freq);
 	if (!ret) {
 		switch (freq) {
@@ -221,6 +252,38 @@ static int at803x_probe(struct phy_device *phydev)
 	return at803x_parse_dt(phydev);
 }
 
+static int at803x_smarteee_config(struct phy_device *phydev)
+{
+	struct at803x_priv *priv = phydev->priv;
+	u16 mask = 0, val = 0;
+	int ret;
+
+	if (priv->flags & AT803X_DISABLE_SMARTEEE)
+		return phy_modify_mmd_indirect(phydev, MDIO_MMD_PCS,
+					       AT803X_MMD3_SMARTEEE_CTL3,
+					       AT803X_MMD3_SMARTEEE_CTL3_LPI_EN, 0);
+
+	if (priv->smarteee_lpi_tw_1g) {
+		mask |= 0xff00;
+		val |= priv->smarteee_lpi_tw_1g << 8;
+	}
+	if (priv->smarteee_lpi_tw_100m) {
+		mask |= 0x00ff;
+		val |= priv->smarteee_lpi_tw_100m;
+	}
+	if (!mask)
+		return 0;
+
+	ret = phy_modify_mmd_indirect(phydev, MDIO_MMD_PCS, AT803X_MMD3_SMARTEEE_CTL1,
+				      mask, val);
+	if (ret)
+		return ret;
+
+	return phy_modify_mmd_indirect(phydev, MDIO_MMD_PCS, AT803X_MMD3_SMARTEEE_CTL3,
+				       AT803X_MMD3_SMARTEEE_CTL3_LPI_EN,
+				       AT803X_MMD3_SMARTEEE_CTL3_LPI_EN);
+}
+
 static int at803x_clk_out_config(struct phy_device *phydev)
 {
 	struct at803x_priv *priv = phydev->priv;
@@ -270,7 +333,15 @@ static int at803x_config_init(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
-	return at803x_clk_out_config(phydev);
+	ret = at803x_smarteee_config(phydev);
+	if (ret < 0)
+		return ret;
+
+	ret = at803x_clk_out_config(phydev);
+	if (ret < 0)
+		return ret;
+
+	return 0;
 }
 
 static struct phy_driver at803x_driver[] = {
-- 
2.39.2

[PATCH 6/7] net: phy: at803x: add disable hibernation mode support

[Marco Felsch]

This commit ports Linux commit:

| commit 9ecf04016c87bcb33b44e24489d33618e2592f41
| Author: Wei Fang <wei.fang@nxp.com>
| Date:   Thu Aug 18 11:00:54 2022 +0800
|
|     net: phy: at803x: add disable hibernation mode support
|
|     When the cable is unplugged, the Atheros AR803x PHYs will enter
|     hibernation mode after about 10 seconds if the hibernation mode
|     is enabled and will not provide any clock to the MAC. But for
|     some MACs, this feature might cause unexpected issues due to the
|     logic of MACs.
|     Taking SYNP MAC (stmmac) as an example, if the cable is unplugged
|     and the "eth0" interface is down, the AR803x PHY will enter
|     hibernation mode. Then perform the "ifconfig eth0 up" operation,
|     the stmmac can't be able to complete the software reset operation
|     and fail to init it's own DMA. Therefore, the "eth0" interface is
|     failed to ifconfig up. Why does it cause this issue? The truth is
|     that the software reset operation of the stmmac is designed to
|     depend on the RX_CLK of PHY.
|     So, this patch offers an option for the user to determine whether
|     to disable the hibernation mode of AR803x PHYs.
|
|     Signed-off-by: Wei Fang <wei.fang@nxp.com>
|     Reviewed-by: Andrew Lunn <andrew@lunn.ch>
|     Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/net/phy/at803x.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index cc425c318f..d8a9c3588f 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -32,6 +32,9 @@
 #define AT803X_DEBUG_REG_5			0x05
 #define AT803X_DEBUG_TX_CLK_DLY_EN		BIT(8)
 
+#define AT803X_DEBUG_REG_HIB_CTRL		0x0b
+#define   AT803X_DEBUG_HIB_CTRL_PS_HIB_EN	BIT(15)
+
 /* AT803x supports either the XTAL input pad, an internal PLL or the
  * DSP as clock reference for the clock output pad. The XTAL reference
  * is only used for 25 MHz output, all other frequencies need the PLL.
@@ -140,6 +143,20 @@ static int at803x_disable_tx_delay(struct phy_device *phydev)
 				     AT803X_DEBUG_TX_CLK_DLY_EN, 0);
 }
 
+static int at803x_hibernation_mode_config(struct phy_device *phydev)
+{
+	struct at803x_priv *priv = phydev->priv;
+
+	/* The default after hardware reset is hibernation mode enabled. After
+	 * software reset, the value is retained.
+	 */
+	if (!(priv->flags & AT803X_DISABLE_HIBERNATION_MODE))
+		return 0;
+
+	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_HIB_CTRL,
+				     AT803X_DEBUG_HIB_CTRL_PS_HIB_EN, 0);
+}
+
 static bool at803x_match_phy_id(struct phy_device *phydev, u32 phy_id)
 {
 	struct phy_driver *drv = to_phy_driver(phydev->dev.driver);
@@ -160,6 +177,9 @@ static int at803x_parse_dt(struct phy_device *phydev)
 	if (of_property_read_bool(node, "qca,disable-smarteee"))
 		priv->flags |= AT803X_DISABLE_SMARTEEE;
 
+	if (of_property_read_bool(node, "qca,disable-hibernation-mode"))
+		priv->flags |= AT803X_DISABLE_HIBERNATION_MODE;
+
 	if (!of_property_read_u32(node, "qca,smarteee-tw-us-1g", &tw)) {
 		if (!tw || tw > 255) {
 			phydev_err(phydev, "invalid qca,smarteee-tw-us-1g\n");
@@ -341,6 +361,10 @@ static int at803x_config_init(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
+	ret = at803x_hibernation_mode_config(phydev);
+	if (ret < 0)
+		return ret;
+
 	return 0;
 }
 
-- 
2.39.2

[PATCH 7/7] net: phy: at803x: disable extended next page bit

[Marco Felsch]

This commit ports Linux commit:

| commit 3c51fa5d2afe7a4909b53af5019635326389dd29
| Author: Russell King <rmk+kernel@armlinux.org.uk>
| Date:   Tue Jan 12 22:59:43 2021 +0000
|
|     net: phy: ar803x: disable extended next page bit
|
|     This bit is enabled by default and advertises support for extended
|     next page support.  XNP is only needed for 10GBase-T and MultiGig
|     support which is not supported. Additionally, Cisco MultiGig switches
|     will read this bit and attempt 10Gb negotiation even though Next Page
|     support is disabled. This will cause timeouts when the interface is
|     forced to 100Mbps and auto-negotiation will fail. The interfaces are
|     only 1000Base-T and supporting auto-negotiation for this only requires
|     the Next Page bit to be set.
|
|     Taken from:
|     https://github.com/SolidRun/linux-stable/commit/7406c5244b7ea6bc17a2afe8568277a8c4b126a9
|     and adapted to mainline kernels by rmk.
|
|     Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
|     Reviewed-by: Andrew Lunn <andrew@lunn.ch>
|     Link: https://lore.kernel.org/r/E1kzSdb-000417-FJ@rmk-PC.armlinux.org.uk
|     Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/net/phy/at803x.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index d8a9c3588f..577bdf1244 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -365,7 +365,13 @@ static int at803x_config_init(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
-	return 0;
+	/* Ar803x extended next page bit is enabled by default. Cisco
+	 * multigig switches read this bit and attempt to negotiate 10Gbps
+	 * rates even if the next page bit is disabled. This is incorrect
+	 * behaviour but we still need to accommodate it. XNP is only needed
+	 * for 10Gbps support, so disable XNP.
+	 */
+	return phy_modify(phydev, MII_ADVERTISE, MDIO_AN_CTRL1_XNP, 0);
 }
 
 static struct phy_driver at803x_driver[] = {
-- 
2.39.2

Re: [PATCH 1/7] ARM: boards: make use of MDIO_MMD register defines

[Ahmad Fatoum]

On 10.07.23 08:36, Marco Felsch wrote:
> Make use of the register definition instead of having magic numbers. No
> functional change.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  arch/arm/boards/datamodul-edm-qmx6/board.c   | 7 ++++---
>  arch/arm/boards/embest-marsboard/board.c     | 7 ++++---
>  arch/arm/boards/terasic-de0-nano-soc/board.c | 7 ++++---
>  arch/arm/boards/terasic-de10-nano/board.c    | 7 ++++---
>  arch/arm/boards/tqma6x/board.c               | 7 ++++---
>  5 files changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/boards/datamodul-edm-qmx6/board.c b/arch/arm/boards/datamodul-edm-qmx6/board.c
> index 9adb3ee0f8..366b64d35a 100644
> --- a/arch/arm/boards/datamodul-edm-qmx6/board.c
> +++ b/arch/arm/boards/datamodul-edm-qmx6/board.c
> @@ -11,6 +11,7 @@
>  #include <gpio.h>
>  #include <of.h>
>  
> +#include <linux/mdio.h>
>  #include <linux/micrel_phy.h>
>  #include <mfd/stmpe-i2c.h>
>  
> @@ -48,9 +49,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
>  	 * min rx data delay, max rx/tx clock delay,
>  	 * min rx/tx control delay
>  	 */
> -	phy_write_mmd_indirect(dev, 4, 2, 0);
> -	phy_write_mmd_indirect(dev, 5, 2, 0);
> -	phy_write_mmd_indirect(dev, 8, 2, 0x03ff);
> +	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> +	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> +	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x03ff);
>  
>  	return 0;
>  }
> diff --git a/arch/arm/boards/embest-marsboard/board.c b/arch/arm/boards/embest-marsboard/board.c
> index 7835a9265a..7274595e2a 100644
> --- a/arch/arm/boards/embest-marsboard/board.c
> +++ b/arch/arm/boards/embest-marsboard/board.c
> @@ -9,6 +9,7 @@
>  #include <init.h>
>  #include <envfs.h>
>  #include <mach/imx/bbu.h>
> +#include <linux/mdio.h>
>  #include <linux/phy.h>
>  #include <deep-probe.h>
>  
> @@ -19,13 +20,13 @@ static int ar8035_phy_fixup(struct phy_device *dev)
>  	/* Ar803x phy SmartEEE feature cause link status generates glitch,
>  	 * which cause ethernet link down/up issue, so disable SmartEEE
>  	 */
> -	val = phy_read_mmd_indirect(dev, 0x805d, 0x3);
> +	val = phy_read_mmd_indirect(dev, 0x805d, MDIO_MMD_PCS);
>  	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
>  
> -	val = phy_read_mmd_indirect(dev, 0x4003, 0x3);
> +	val = phy_read_mmd_indirect(dev, 0x4003, MDIO_MMD_PCS);
>  	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
>  
> -	val = phy_read_mmd_indirect(dev, 0x4007, 0x3);
> +	val = phy_read_mmd_indirect(dev, 0x4007, MDIO_MMD_PCS);
>  	val &= 0xffe3;
>  	val |= 0x18;
>  	phy_write(dev, MII_MMD_DATA, val);
> diff --git a/arch/arm/boards/terasic-de0-nano-soc/board.c b/arch/arm/boards/terasic-de0-nano-soc/board.c
> index 19f74b784c..832160c595 100644
> --- a/arch/arm/boards/terasic-de0-nano-soc/board.c
> +++ b/arch/arm/boards/terasic-de0-nano-soc/board.c
> @@ -5,6 +5,7 @@
>  #include <driver.h>
>  #include <init.h>
>  #include <asm/armlinux.h>
> +#include <linux/mdio.h>
>  #include <linux/micrel_phy.h>
>  #include <linux/phy.h>
>  #include <linux/sizes.h>
> @@ -18,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
>  	 * min rx data delay, max rx/tx clock delay,
>  	 * min rx/tx control delay
>  	 */
> -	phy_write_mmd_indirect(dev, 4, 2, 0);
> -	phy_write_mmd_indirect(dev, 5, 2, 0);
> -	phy_write_mmd_indirect(dev, 8, 2, 0x003ff);
> +	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> +	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> +	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
>  	return 0;
>  }
>  
> diff --git a/arch/arm/boards/terasic-de10-nano/board.c b/arch/arm/boards/terasic-de10-nano/board.c
> index 580c898012..e47d9ac841 100644
> --- a/arch/arm/boards/terasic-de10-nano/board.c
> +++ b/arch/arm/boards/terasic-de10-nano/board.c
> @@ -5,6 +5,7 @@
>  #include <driver.h>
>  #include <init.h>
>  #include <asm/armlinux.h>
> +#include <linux/mdio.h>
>  #include <linux/micrel_phy.h>
>  #include <linux/phy.h>
>  #include <linux/sizes.h>
> @@ -18,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
>  	 * min rx data delay, max rx/tx clock delay,
>  	 * min rx/tx control delay
>  	 */
> -	phy_write_mmd_indirect(dev, 4, 2, 0);
> -	phy_write_mmd_indirect(dev, 5, 2, 0);
> -	phy_write_mmd_indirect(dev, 8, 2, 0x003ff);
> +	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> +	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> +	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
>  	return 0;
>  }
>  
> diff --git a/arch/arm/boards/tqma6x/board.c b/arch/arm/boards/tqma6x/board.c
> index 4bb7223a6e..8a91ad652a 100644
> --- a/arch/arm/boards/tqma6x/board.c
> +++ b/arch/arm/boards/tqma6x/board.c
> @@ -11,6 +11,7 @@
>  #include <gpio.h>
>  #include <of.h>
>  
> +#include <linux/mdio.h>
>  #include <linux/micrel_phy.h>
>  #include <mfd/stmpe-i2c.h>
>  
> @@ -46,9 +47,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
>  	 * min rx data delay, max rx/tx clock delay,
>  	 * min rx/tx control delay
>  	 */
> -	phy_write_mmd_indirect(dev, 4, 2, 0);
> -	phy_write_mmd_indirect(dev, 5, 2, 0);
> -	phy_write_mmd_indirect(dev, 8, 2, 0x003ff);
> +	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> +	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> +	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
>  
>  	return 0;
>  }

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 2/7] net: phy: micrel: make use of MDIO_MMD register defines

[Ahmad Fatoum]

On 10.07.23 08:36, Marco Felsch wrote:
> Make use of the register definition instead of having magic numbers. No
> functional change.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

FYI, the code is taken from Linux (with arguments swapped).

Change is ok though:

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  drivers/net/phy/micrel.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index ef1f919ae7..02d474c442 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -447,23 +447,27 @@ static int ksz9031_config_rgmii_delay(struct phy_device *phydev)
>  		return 0;
>  	}
>  
> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CONTROL_PAD_SKEW, 2,
> +	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CONTROL_PAD_SKEW,
> +			       MDIO_MMD_WIS,
>  			       FIELD_PREP(MII_KSZ9031RN_RX_CTL_M, rx) |
>  			       FIELD_PREP(MII_KSZ9031RN_TX_CTL_M, tx));
>  
> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_RX_DATA_PAD_SKEW, 2,
> +	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_RX_DATA_PAD_SKEW,
> +			       MDIO_MMD_WIS,
>  			       FIELD_PREP(MII_KSZ9031RN_RXD3, rx) |
>  			       FIELD_PREP(MII_KSZ9031RN_RXD2, rx) |
>  			       FIELD_PREP(MII_KSZ9031RN_RXD1, rx) |
>  			       FIELD_PREP(MII_KSZ9031RN_RXD0, rx));
>  
> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_TX_DATA_PAD_SKEW, 2,
> +	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_TX_DATA_PAD_SKEW,
> +			       MDIO_MMD_WIS,
>  			       FIELD_PREP(MII_KSZ9031RN_TXD3, tx) |
>  			       FIELD_PREP(MII_KSZ9031RN_TXD2, tx) |
>  			       FIELD_PREP(MII_KSZ9031RN_TXD1, tx) |
>  			       FIELD_PREP(MII_KSZ9031RN_TXD0, tx));
>  
> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CLK_PAD_SKEW, 2,
> +	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CLK_PAD_SKEW,
> +			       MDIO_MMD_WIS,
>  			       FIELD_PREP(MII_KSZ9031RN_GTX_CLK, tx_clk) |
>  			       FIELD_PREP(MII_KSZ9031RN_RX_CLK, rx_clk));
>  	return 0;

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 3/7] net: phy: sync phy_{write,read,modify}_mmd_indirect with linux

[Ahmad Fatoum]

On 10.07.23 08:36, Marco Felsch wrote:
> Sync the barebox phy_{write,read,modify}_mmd_indirect API with the Linux
> phy_{write,read,modify}_mmd API to make it easier and less error prone
> to port phy drivers. This also fixes the r8169 driver since this driver
> did not adapt the parameters while porting from Linux.
> 
> The sync also aligns the function parameter `prtad` as well as the
> function documentation.

This breaks out of tree board code in a subtle and annoying manner.
We are in luck that the kernel function has a different name than
the barebox function, so I'd rather suggest:

 - Add phy_read_mmd/phy_write_mmd functions with same argument
   order as Linux
 - Switch all users in barebox to the new functions
 - Mark the old _indirect function deprecated with a suggestion
   to use phy_read_mmd/phy_write_mmd and swap address arguments.

> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  arch/arm/boards/datamodul-edm-qmx6/board.c   |  6 +--
>  arch/arm/boards/embest-marsboard/board.c     |  6 +--
>  arch/arm/boards/terasic-de0-nano-soc/board.c |  6 +--
>  arch/arm/boards/terasic-de10-nano/board.c    |  6 +--
>  arch/arm/boards/tqma6x/board.c               |  6 +--
>  drivers/net/phy/at803x.c                     |  4 +-
>  drivers/net/phy/dp83867.c                    | 40 +++++++-------
>  drivers/net/phy/micrel.c                     | 24 ++++-----
>  drivers/net/phy/phy.c                        | 57 ++++++++++----------
>  include/linux/phy.h                          | 10 ++--
>  10 files changed, 83 insertions(+), 82 deletions(-)
> 
> diff --git a/arch/arm/boards/datamodul-edm-qmx6/board.c b/arch/arm/boards/datamodul-edm-qmx6/board.c
> index 366b64d35a..0dc71f2aca 100644
> --- a/arch/arm/boards/datamodul-edm-qmx6/board.c
> +++ b/arch/arm/boards/datamodul-edm-qmx6/board.c
> @@ -49,9 +49,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
>  	 * min rx data delay, max rx/tx clock delay,
>  	 * min rx/tx control delay
>  	 */
> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x03ff);
> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x03ff);
>  
>  	return 0;
>  }
> diff --git a/arch/arm/boards/embest-marsboard/board.c b/arch/arm/boards/embest-marsboard/board.c
> index 7274595e2a..e70fefec5e 100644
> --- a/arch/arm/boards/embest-marsboard/board.c
> +++ b/arch/arm/boards/embest-marsboard/board.c
> @@ -20,13 +20,13 @@ static int ar8035_phy_fixup(struct phy_device *dev)
>  	/* Ar803x phy SmartEEE feature cause link status generates glitch,
>  	 * which cause ethernet link down/up issue, so disable SmartEEE
>  	 */
> -	val = phy_read_mmd_indirect(dev, 0x805d, MDIO_MMD_PCS);
> +	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x805d);
>  	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
>  
> -	val = phy_read_mmd_indirect(dev, 0x4003, MDIO_MMD_PCS);
> +	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x4003);
>  	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
>  
> -	val = phy_read_mmd_indirect(dev, 0x4007, MDIO_MMD_PCS);
> +	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x4007);
>  	val &= 0xffe3;
>  	val |= 0x18;
>  	phy_write(dev, MII_MMD_DATA, val);
> diff --git a/arch/arm/boards/terasic-de0-nano-soc/board.c b/arch/arm/boards/terasic-de0-nano-soc/board.c
> index 832160c595..fa83498f46 100644
> --- a/arch/arm/boards/terasic-de0-nano-soc/board.c
> +++ b/arch/arm/boards/terasic-de0-nano-soc/board.c
> @@ -19,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
>  	 * min rx data delay, max rx/tx clock delay,
>  	 * min rx/tx control delay
>  	 */
> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
>  	return 0;
>  }
>  
> diff --git a/arch/arm/boards/terasic-de10-nano/board.c b/arch/arm/boards/terasic-de10-nano/board.c
> index e47d9ac841..7ceb691f71 100644
> --- a/arch/arm/boards/terasic-de10-nano/board.c
> +++ b/arch/arm/boards/terasic-de10-nano/board.c
> @@ -19,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
>  	 * min rx data delay, max rx/tx clock delay,
>  	 * min rx/tx control delay
>  	 */
> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
>  	return 0;
>  }
>  
> diff --git a/arch/arm/boards/tqma6x/board.c b/arch/arm/boards/tqma6x/board.c
> index 8a91ad652a..3fd2e1db2c 100644
> --- a/arch/arm/boards/tqma6x/board.c
> +++ b/arch/arm/boards/tqma6x/board.c
> @@ -47,9 +47,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
>  	 * min rx data delay, max rx/tx clock delay,
>  	 * min rx/tx control delay
>  	 */
> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
>  
>  	return 0;
>  }
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 18182bffc2..41010c58ad 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -229,14 +229,14 @@ static int at803x_clk_out_config(struct phy_device *phydev)
>  	if (!priv->clk_25m_mask)
>  		return 0;
>  
> -	val = phy_read_mmd_indirect(phydev, AT803X_MMD7_CLK25M, MDIO_MMD_AN);
> +	val = phy_read_mmd_indirect(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M);
>  	if (val < 0)
>  		return val;
>  
>  	val &= ~priv->clk_25m_mask;
>  	val |= priv->clk_25m_reg;
>  
> -	phy_write_mmd_indirect(phydev, AT803X_MMD7_CLK25M, MDIO_MMD_AN, val);
> +	phy_write_mmd_indirect(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M, val);
>  
>  	return 0;
>  }
> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> index d8109172df..85fe5107da 100644
> --- a/drivers/net/phy/dp83867.c
> +++ b/drivers/net/phy/dp83867.c
> @@ -154,14 +154,14 @@ static int dp83867_config_port_mirroring(struct phy_device *phydev)
>  	struct dp83867_private *dp83867 = phydev->priv;
>  	u16 val;
>  
> -	val = phy_read_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR);
> +	val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR, DP83867_CFG4);
>  
>  	if (dp83867->port_mirroring == DP83867_PORT_MIRROING_EN)
>  		val |= DP83867_CFG4_PORT_MIRROR_EN;
>  	else
>  		val &= ~DP83867_CFG4_PORT_MIRROR_EN;
>  
> -	phy_write_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR, val);
> +	phy_write_mmd_indirect(phydev, DP83867_DEVADDR, DP83867_CFG4, val);
>  
>  	return 0;
>  }
> @@ -256,11 +256,11 @@ static int dp83867_config_init(struct phy_device *phydev)
>  	phy_write(phydev, DP83867_CTRL, val | DP83867_SW_RESTART);
>  
>  	if (dp83867->rxctrl_strap_quirk) {
> -		val = phy_read_mmd_indirect(phydev, DP83867_CFG4,
> -					    DP83867_DEVADDR);
> +		val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
> +					    DP83867_CFG4);
>  		val &= ~BIT(7);
> -		phy_write_mmd_indirect(phydev, DP83867_CFG4,
> -				       DP83867_DEVADDR, val);
> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> +				       DP83867_CFG4, val);
>  	}
>  
>  	if (phy_interface_is_rgmii(phydev)) {
> @@ -270,8 +270,8 @@ static int dp83867_config_init(struct phy_device *phydev)
>  		if (ret)
>  			return ret;
>  
> -		val = phy_read_mmd_indirect(phydev, DP83867_RGMIICTL,
> -					    DP83867_DEVADDR);
> +		val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
> +					    DP83867_RGMIICTL);
>  
>  		switch (phydev->interface) {
>  		case PHY_INTERFACE_MODE_RGMII_ID:
> @@ -287,31 +287,31 @@ static int dp83867_config_init(struct phy_device *phydev)
>  		default:
>  			break;
>  		}
> -		phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
> -				       DP83867_DEVADDR, val);
> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> +				       DP83867_RGMIICTL, val);
>  
>  		delay = (dp83867->rx_id_delay |
>  			(dp83867->tx_id_delay << DP83867_RGMII_TX_CLK_DELAY_SHIFT));
>  
> -		phy_write_mmd_indirect(phydev, DP83867_RGMIIDCTL,
> -				       DP83867_DEVADDR, delay);
> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> +				       DP83867_RGMIIDCTL, delay);
>  
>  		if (dp83867->io_impedance >= 0) {
> -			val = phy_read_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
> -						    DP83867_DEVADDR);
> +			val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
> +						    DP83867_IO_MUX_CFG);
>  			val &= ~DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
>  			val |= (dp83867->io_impedance &
>  				DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL);
>  
> -			phy_write_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
> -					       DP83867_DEVADDR, val);
> +			phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> +					       DP83867_IO_MUX_CFG, val);
>  		}
>  	} else if (phy_interface_is_sgmii(phydev)) {
>  		phy_write(phydev, MII_BMCR,
>  			  BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000);
>  
> -		phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
> -				       DP83867_DEVADDR, 0x0);
> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> +				       DP83867_RGMIICTL, 0x0);
>  
>  		val = DP83867_PHYCTRL_SGMIIEN |
>  		      DP83867_MDI_CROSSOVER_MDIX << DP83867_MDI_CROSSOVER |
> @@ -341,8 +341,8 @@ static int dp83867_config_init(struct phy_device *phydev)
>  				DP83867_IO_MUX_CFG_CLK_O_SEL_SHIFT;
>  		}
>  
> -		phy_modify_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
> -				DP83867_DEVADDR, mask, val);
> +		phy_modify_mmd_indirect(phydev, DP83867_DEVADDR,
> +					DP83867_IO_MUX_CFG, mask, val);
>  	}
>  
>  	return 0;
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 02d474c442..892df01d2e 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -387,7 +387,7 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
>  		return 0;
>  
>  	if (matches < numfields)
> -		newval = phy_read_mmd_indirect(phydev, reg, MDIO_MMD_WIS);
> +		newval = phy_read_mmd_indirect(phydev, MDIO_MMD_WIS, reg);
>  	else
>  		newval = 0;
>  
> @@ -401,15 +401,15 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
>  				<< (field_sz * i));
>  		}
>  
> -	phy_write_mmd_indirect(phydev, reg, MDIO_MMD_WIS, newval);
> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS, reg, newval);
>  	return 0;
>  }
>  
>  static int ksz9031_center_flp_timing(struct phy_device *phydev)
>  {
>  	/* Center KSZ9031RNX FLP timing at 16ms. */
> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_HI, 0, 0x0006);
> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_LO, 0, 0x1a80);
> +	phy_write_mmd_indirect(phydev, 0, MII_KSZ9031RN_FLP_BURST_TX_HI, 0x0006);
> +	phy_write_mmd_indirect(phydev, 0, MII_KSZ9031RN_FLP_BURST_TX_LO, 0x1a80);
>  
>  	return genphy_restart_aneg(phydev);
>  }
> @@ -447,27 +447,27 @@ static int ksz9031_config_rgmii_delay(struct phy_device *phydev)
>  		return 0;
>  	}
>  
> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CONTROL_PAD_SKEW,
> -			       MDIO_MMD_WIS,
> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> +			       MII_KSZ9031RN_CONTROL_PAD_SKEW,
>  			       FIELD_PREP(MII_KSZ9031RN_RX_CTL_M, rx) |
>  			       FIELD_PREP(MII_KSZ9031RN_TX_CTL_M, tx));
>  
> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_RX_DATA_PAD_SKEW,
> -			       MDIO_MMD_WIS,
> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> +			       MII_KSZ9031RN_RX_DATA_PAD_SKEW,
>  			       FIELD_PREP(MII_KSZ9031RN_RXD3, rx) |
>  			       FIELD_PREP(MII_KSZ9031RN_RXD2, rx) |
>  			       FIELD_PREP(MII_KSZ9031RN_RXD1, rx) |
>  			       FIELD_PREP(MII_KSZ9031RN_RXD0, rx));
>  
> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_TX_DATA_PAD_SKEW,
> -			       MDIO_MMD_WIS,
> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> +			       MII_KSZ9031RN_TX_DATA_PAD_SKEW,
>  			       FIELD_PREP(MII_KSZ9031RN_TXD3, tx) |
>  			       FIELD_PREP(MII_KSZ9031RN_TXD2, tx) |
>  			       FIELD_PREP(MII_KSZ9031RN_TXD1, tx) |
>  			       FIELD_PREP(MII_KSZ9031RN_TXD0, tx));
>  
> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CLK_PAD_SKEW,
> -			       MDIO_MMD_WIS,
> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> +			       MII_KSZ9031RN_CLK_PAD_SKEW,
>  			       FIELD_PREP(MII_KSZ9031RN_GTX_CLK, tx_clk) |
>  			       FIELD_PREP(MII_KSZ9031RN_RX_CLK, rx_clk));
>  	return 0;
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 74381949b4..283e1a3d79 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -819,24 +819,25 @@ int genphy_read_status(struct phy_device *phydev)
>  	return 0;
>  }
>  
> -static inline void mmd_phy_indirect(struct phy_device *phydev, int prtad,
> -					int devad)
> +static inline void mmd_phy_indirect(struct phy_device *phydev, int devad,
> +				    u16 regnum)
>  {
>  	/* Write the desired MMD Devad */
>  	phy_write(phydev, MII_MMD_CTRL, devad);
>  
>  	/* Write the desired MMD register address */
> -	phy_write(phydev, MII_MMD_DATA, prtad);
> +	phy_write(phydev, MII_MMD_DATA, regnum);
>  
>  	/* Select the Function : DATA with no post increment */
>  	phy_write(phydev, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
>  }
>  
>  /**
> - * phy_read_mmd_indirect - reads data from the MMD registers
> - * @phy_device: phy device
> - * @prtad: MMD Address
> - * @devad: MMD DEVAD
> + * phy_read_mmd_indirect - Convenience function for reading a register
> + * from an MMD on a given PHY.
> + * @phydev: The phy_device struct
> + * @devad: The MMD to read from
> + * @regnum: The register on the MMD to read
>   *
>   * Description: it reads data from the MMD registers (clause 22 to access to
>   * clause 45) of the specified phy address.
> @@ -846,11 +847,11 @@ static inline void mmd_phy_indirect(struct phy_device *phydev, int prtad,
>   * 3) Write reg 13 // MMD Data Command for MMD DEVAD
>   * 3) Read  reg 14 // Read MMD data
>   */
> -int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
> +int phy_read_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum)
>  {
>  	u32 ret;
>  
> -	mmd_phy_indirect(phydev, prtad, devad);
> +	mmd_phy_indirect(phydev, devad, regnum);
>  
>  	/* Read the content of the MMD's selected register */
>  	ret = phy_read(phydev, MII_MMD_DATA);
> @@ -859,11 +860,12 @@ int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
>  }
>  
>  /**
> - * phy_write_mmd_indirect - writes data to the MMD registers
> - * @phy_device: phy device
> - * @prtad: MMD Address
> - * @devad: MMD DEVAD
> - * @data: data to write in the MMD register
> + * phy_write_mmd_indirect - Convenience function for writing a register
> + * on an MMD on a given PHY.
> + * @phydev: The phy_device struct
> + * @devad: The MMD to read from
> + * @regnum: The register on the MMD to read
> + * @val: value to write to @regnum
>   *
>   * Description: Write data from the MMD registers of the specified
>   * phy address.
> @@ -873,34 +875,33 @@ int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
>   * 3) Write reg 13 // MMD Data Command for MMD DEVAD
>   * 3) Write reg 14 // Write MMD data
>   */
> -void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> -				   u16 data)
> +void phy_write_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> +			    u16 val)
>  {
> -	mmd_phy_indirect(phydev, prtad, devad);
> +	mmd_phy_indirect(phydev, devad, regnum);
>  
>  	/* Write the data into MMD's selected register */
> -	phy_write(phydev, MII_MMD_DATA, data);
> +	phy_write(phydev, MII_MMD_DATA, val);
>  }
>  
>  /**
> - * phy_modify_mmd_indirect - Convenience function for modifying a MMD register
> - * @phydev: phy device
> - * @prtad: MMD Address
> - * @devad: MMD DEVAD
> + * phy_modify_mmd_indirect - Convenience function for modifying a register on MMD
> + * @phydev: the phy_device struct
> + * @devad: the MMD containing register to modify
> + * @regnum: register number to modify
>   * @mask: bit mask of bits to clear
> - * @set: new value of bits set in @mask
> - *
> + * @set: new value of bits set in mask to write to @regnum
>   */
> -int phy_modify_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> -				   u16 mask, u16 set)
> +int phy_modify_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> +			    u16 mask, u16 set)
>  {
>  	int ret;
>  
> -	ret = phy_read_mmd_indirect(phydev, prtad, devad);
> +	ret = phy_read_mmd_indirect(phydev, devad, regnum);
>  	if (ret < 0)
>  		return ret;
>  
> -	phy_write_mmd_indirect(phydev, prtad, devad, (ret & ~mask) | set);
> +	phy_write_mmd_indirect(phydev, devad, regnum, (ret & ~mask) | set);
>  
>  	return 0;
>  }
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 509bf72de9..d96c4e97ab 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -406,11 +406,11 @@ int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
>  int phy_register_fixup_for_id(const char *bus_id,
>  		int (*run)(struct phy_device *));
>  int phy_scan_fixups(struct phy_device *phydev);
> -int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad);
> -void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> -				   u16 data);
> -int phy_modify_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> -				    u16 mask, u16 set);
> +int phy_read_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum);
> +void phy_write_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> +			    u16 val);
> +int phy_modify_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> +			    u16 mask, u16 set);
>  
>  static inline bool phy_acquired(struct phy_device *phydev)
>  {

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 4/7] net: phy: add phydev_{err,err_probe,info,warn,dbg} macros

[Ahmad Fatoum]

On 10.07.23 08:36, Marco Felsch wrote:
> Import Linux macros to make it easier to port drivers to barebox.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  include/linux/phy.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index d96c4e97ab..b0474e87df 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -417,6 +417,21 @@ static inline bool phy_acquired(struct phy_device *phydev)
>  	return phydev && phydev->bus && slice_acquired(&phydev->bus->slice);
>  }
>  
> +#define phydev_err(_phydev, format, args...)	\
> +	dev_err(&_phydev->dev, format, ##args)
> +
> +#define phydev_err_probe(_phydev, err, format, args...)	\
> +	dev_err_probe(&_phydev->dev, err, format, ##args)
> +
> +#define phydev_info(_phydev, format, args...)	\
> +	dev_info(&_phydev->dev, format, ##args)
> +
> +#define phydev_warn(_phydev, format, args...)	\
> +	dev_warn(&_phydev->dev, format, ##args)
> +
> +#define phydev_dbg(_phydev, format, args...)	\
> +	dev_dbg(&_phydev->dev, format, ##args)
> +
>  #ifdef CONFIG_PHYLIB
>  int phy_register_fixup_for_uid(u32 phy_uid, u32 phy_uid_mask,
>  		int (*run)(struct phy_device *));

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 7/7] net: phy: at803x: disable extended next page bit

[Ahmad Fatoum]

On 10.07.23 08:36, Marco Felsch wrote:
> This commit ports Linux commit:
> 
> | commit 3c51fa5d2afe7a4909b53af5019635326389dd29
> | Author: Russell King <rmk+kernel@armlinux.org.uk>
> | Date:   Tue Jan 12 22:59:43 2021 +0000
> |
> |     net: phy: ar803x: disable extended next page bit
> |
> |     This bit is enabled by default and advertises support for extended
> |     next page support.  XNP is only needed for 10GBase-T and MultiGig
> |     support which is not supported. Additionally, Cisco MultiGig switches
> |     will read this bit and attempt 10Gb negotiation even though Next Page
> |     support is disabled. This will cause timeouts when the interface is
> |     forced to 100Mbps and auto-negotiation will fail. The interfaces are
> |     only 1000Base-T and supporting auto-negotiation for this only requires
> |     the Next Page bit to be set.
> |
> |     Taken from:
> |     https://github.com/SolidRun/linux-stable/commit/7406c5244b7ea6bc17a2afe8568277a8c4b126a9
> |     and adapted to mainline kernels by rmk.
> |
> |     Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> |     Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> |     Link: https://lore.kernel.org/r/E1kzSdb-000417-FJ@rmk-PC.armlinux.org.uk
> |     Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  drivers/net/phy/at803x.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index d8a9c3588f..577bdf1244 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -365,7 +365,13 @@ static int at803x_config_init(struct phy_device *phydev)
>  	if (ret < 0)
>  		return ret;
>  
> -	return 0;
> +	/* Ar803x extended next page bit is enabled by default. Cisco
> +	 * multigig switches read this bit and attempt to negotiate 10Gbps
> +	 * rates even if the next page bit is disabled. This is incorrect
> +	 * behaviour but we still need to accommodate it. XNP is only needed
> +	 * for 10Gbps support, so disable XNP.
> +	 */
> +	return phy_modify(phydev, MII_ADVERTISE, MDIO_AN_CTRL1_XNP, 0);
>  }
>  
>  static struct phy_driver at803x_driver[] = {

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 6/7] net: phy: at803x: add disable hibernation mode support

[Ahmad Fatoum]

On 10.07.23 08:36, Marco Felsch wrote:
> This commit ports Linux commit:
> 
> | commit 9ecf04016c87bcb33b44e24489d33618e2592f41
> | Author: Wei Fang <wei.fang@nxp.com>
> | Date:   Thu Aug 18 11:00:54 2022 +0800
> |
> |     net: phy: at803x: add disable hibernation mode support
> |
> |     When the cable is unplugged, the Atheros AR803x PHYs will enter
> |     hibernation mode after about 10 seconds if the hibernation mode
> |     is enabled and will not provide any clock to the MAC. But for
> |     some MACs, this feature might cause unexpected issues due to the
> |     logic of MACs.
> |     Taking SYNP MAC (stmmac) as an example, if the cable is unplugged
> |     and the "eth0" interface is down, the AR803x PHY will enter
> |     hibernation mode. Then perform the "ifconfig eth0 up" operation,
> |     the stmmac can't be able to complete the software reset operation
> |     and fail to init it's own DMA. Therefore, the "eth0" interface is
> |     failed to ifconfig up. Why does it cause this issue? The truth is
> |     that the software reset operation of the stmmac is designed to
> |     depend on the RX_CLK of PHY.
> |     So, this patch offers an option for the user to determine whether
> |     to disable the hibernation mode of AR803x PHYs.
> |
> |     Signed-off-by: Wei Fang <wei.fang@nxp.com>
> |     Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> |     Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

IMO we should just makes AT803X_DISABLE_HIBERNATION_MODE the default and
never hibernate the PHY. Let Linux worry about that.

> ---
>  drivers/net/phy/at803x.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index cc425c318f..d8a9c3588f 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -32,6 +32,9 @@
>  #define AT803X_DEBUG_REG_5			0x05
>  #define AT803X_DEBUG_TX_CLK_DLY_EN		BIT(8)
>  
> +#define AT803X_DEBUG_REG_HIB_CTRL		0x0b
> +#define   AT803X_DEBUG_HIB_CTRL_PS_HIB_EN	BIT(15)
> +
>  /* AT803x supports either the XTAL input pad, an internal PLL or the
>   * DSP as clock reference for the clock output pad. The XTAL reference
>   * is only used for 25 MHz output, all other frequencies need the PLL.
> @@ -140,6 +143,20 @@ static int at803x_disable_tx_delay(struct phy_device *phydev)
>  				     AT803X_DEBUG_TX_CLK_DLY_EN, 0);
>  }
>  
> +static int at803x_hibernation_mode_config(struct phy_device *phydev)
> +{
> +	struct at803x_priv *priv = phydev->priv;
> +
> +	/* The default after hardware reset is hibernation mode enabled. After
> +	 * software reset, the value is retained.
> +	 */
> +	if (!(priv->flags & AT803X_DISABLE_HIBERNATION_MODE))
> +		return 0;
> +
> +	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_HIB_CTRL,
> +				     AT803X_DEBUG_HIB_CTRL_PS_HIB_EN, 0);
> +}
> +
>  static bool at803x_match_phy_id(struct phy_device *phydev, u32 phy_id)
>  {
>  	struct phy_driver *drv = to_phy_driver(phydev->dev.driver);
> @@ -160,6 +177,9 @@ static int at803x_parse_dt(struct phy_device *phydev)
>  	if (of_property_read_bool(node, "qca,disable-smarteee"))
>  		priv->flags |= AT803X_DISABLE_SMARTEEE;
>  
> +	if (of_property_read_bool(node, "qca,disable-hibernation-mode"))
> +		priv->flags |= AT803X_DISABLE_HIBERNATION_MODE;
> +
>  	if (!of_property_read_u32(node, "qca,smarteee-tw-us-1g", &tw)) {
>  		if (!tw || tw > 255) {
>  			phydev_err(phydev, "invalid qca,smarteee-tw-us-1g\n");
> @@ -341,6 +361,10 @@ static int at803x_config_init(struct phy_device *phydev)
>  	if (ret < 0)
>  		return ret;
>  
> +	ret = at803x_hibernation_mode_config(phydev);
> +	if (ret < 0)
> +		return ret;
> +
>  	return 0;
>  }
>  

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 5/7] net: phy: at803x: add support for configuring SmartEEE

[Ahmad Fatoum]

On 10.07.23 08:36, Marco Felsch wrote:
> This commit port Linux commit:
> 
> | commit 390b4cad81484124db2b676ed20a265adc032bae
> | Author: Russell King <rmk+kernel@armlinux.org.uk>
> | Date:   Thu Jan 14 10:45:49 2021 +0000
> |
> |     net: phy: at803x: add support for configuring SmartEEE
> |
> |     SmartEEE for the atheros phy was deemed buggy by Freescale and commits
> |     were added to disable it for their boards.
> |
> |     In initial testing, SolidRun found that the default settings were
> |     causing disconnects but by increasing the Tw buffer time we could allow
> |     enough time for all parts of the link to come out of a low power state
> |     and function properly without causing a disconnect. This allows us to
> |     have functional power savings of between 300 and 400mW, rather than
> |     disabling the feature altogether.
> |
> |     This commit adds support for disabling SmartEEE and configuring the Tw
> |     parameters for 1G and 100M speeds.
> |
> |     Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> |     Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> 
> I slightly adapted the at803x_config_init() as well to be more in line
> with the Linux code base.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Same question as in follow-up commit: Can't we just disable it unconditionally?

> ---
>  drivers/net/phy/at803x.c | 75 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 73 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 41010c58ad..cc425c318f 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -59,6 +59,15 @@
>   */
>  #define AT8035_CLK_OUT_MASK			GENMASK(4, 3)
>  
> +#define AT803X_MMD3_SMARTEEE_CTL1		0x805b
> +#define AT803X_MMD3_SMARTEEE_CTL3		0x805d
> +#define AT803X_MMD3_SMARTEEE_CTL3_LPI_EN	BIT(8)
> +
> +#define AT803X_DISABLE_SMARTEEE			BIT(1)
> +
> +/* disable hibernation mode */
> +#define AT803X_DISABLE_HIBERNATION_MODE		BIT(2)
> +
>  #define AT803X_CLK_OUT_STRENGTH_MASK		GENMASK(8, 7)
>  #define AT803X_CLK_OUT_STRENGTH_FULL		0
>  #define AT803X_CLK_OUT_STRENGTH_HALF		1
> @@ -72,8 +81,11 @@
>  #define AT8030_PHY_ID_MASK			0xffffffef
>  
>  struct at803x_priv {
> +	int flags;
>  	u16 clk_25m_reg;
>  	u16 clk_25m_mask;
> +	u8 smarteee_lpi_tw_1g;
> +	u8 smarteee_lpi_tw_100m;
>  };
>  
>  static int at803x_debug_reg_read(struct phy_device *phydev, u16 reg)
> @@ -141,10 +153,29 @@ static int at803x_parse_dt(struct phy_device *phydev)
>  	const struct device *dev = &phydev->dev;
>  	const struct device_node *node = dev->of_node;
>  	struct at803x_priv *priv = phydev->priv;
> +	u32 freq, strength, tw;
>  	unsigned int sel;
> -	u32 freq, strength;
>  	int ret;
>  
> +	if (of_property_read_bool(node, "qca,disable-smarteee"))
> +		priv->flags |= AT803X_DISABLE_SMARTEEE;
> +
> +	if (!of_property_read_u32(node, "qca,smarteee-tw-us-1g", &tw)) {
> +		if (!tw || tw > 255) {
> +			phydev_err(phydev, "invalid qca,smarteee-tw-us-1g\n");
> +			return -EINVAL;
> +		}
> +		priv->smarteee_lpi_tw_1g = tw;
> +	}
> +
> +	if (!of_property_read_u32(node, "qca,smarteee-tw-us-100m", &tw)) {
> +		if (!tw || tw > 255) {
> +			phydev_err(phydev, "invalid qca,smarteee-tw-us-100m\n");
> +			return -EINVAL;
> +		}
> +		priv->smarteee_lpi_tw_100m = tw;
> +	}
> +
>  	ret = of_property_read_u32(node, "qca,clk-out-frequency", &freq);
>  	if (!ret) {
>  		switch (freq) {
> @@ -221,6 +252,38 @@ static int at803x_probe(struct phy_device *phydev)
>  	return at803x_parse_dt(phydev);
>  }
>  
> +static int at803x_smarteee_config(struct phy_device *phydev)
> +{
> +	struct at803x_priv *priv = phydev->priv;
> +	u16 mask = 0, val = 0;
> +	int ret;
> +
> +	if (priv->flags & AT803X_DISABLE_SMARTEEE)
> +		return phy_modify_mmd_indirect(phydev, MDIO_MMD_PCS,
> +					       AT803X_MMD3_SMARTEEE_CTL3,
> +					       AT803X_MMD3_SMARTEEE_CTL3_LPI_EN, 0);
> +
> +	if (priv->smarteee_lpi_tw_1g) {
> +		mask |= 0xff00;
> +		val |= priv->smarteee_lpi_tw_1g << 8;
> +	}
> +	if (priv->smarteee_lpi_tw_100m) {
> +		mask |= 0x00ff;
> +		val |= priv->smarteee_lpi_tw_100m;
> +	}
> +	if (!mask)
> +		return 0;
> +
> +	ret = phy_modify_mmd_indirect(phydev, MDIO_MMD_PCS, AT803X_MMD3_SMARTEEE_CTL1,
> +				      mask, val);
> +	if (ret)
> +		return ret;
> +
> +	return phy_modify_mmd_indirect(phydev, MDIO_MMD_PCS, AT803X_MMD3_SMARTEEE_CTL3,
> +				       AT803X_MMD3_SMARTEEE_CTL3_LPI_EN,
> +				       AT803X_MMD3_SMARTEEE_CTL3_LPI_EN);
> +}
> +
>  static int at803x_clk_out_config(struct phy_device *phydev)
>  {
>  	struct at803x_priv *priv = phydev->priv;
> @@ -270,7 +333,15 @@ static int at803x_config_init(struct phy_device *phydev)
>  	if (ret < 0)
>  		return ret;
>  
> -	return at803x_clk_out_config(phydev);
> +	ret = at803x_smarteee_config(phydev);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = at803x_clk_out_config(phydev);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
>  }
>  
>  static struct phy_driver at803x_driver[] = {

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 2/7] net: phy: micrel: make use of MDIO_MMD register defines

[Marco Felsch]

On 23-07-10, Ahmad Fatoum wrote:
> On 10.07.23 08:36, Marco Felsch wrote:
> > Make use of the register definition instead of having magic numbers. No
> > functional change.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> 
> FYI, the code is taken from Linux (with arguments swapped).

Patch opportunities ^^ thanks for pointing that out.

Regards,
  Marco

> Change is ok though:
> 
> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> > ---
> >  drivers/net/phy/micrel.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> > index ef1f919ae7..02d474c442 100644
> > --- a/drivers/net/phy/micrel.c
> > +++ b/drivers/net/phy/micrel.c
> > @@ -447,23 +447,27 @@ static int ksz9031_config_rgmii_delay(struct phy_device *phydev)
> >  		return 0;
> >  	}
> >  
> > -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CONTROL_PAD_SKEW, 2,
> > +	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CONTROL_PAD_SKEW,
> > +			       MDIO_MMD_WIS,
> >  			       FIELD_PREP(MII_KSZ9031RN_RX_CTL_M, rx) |
> >  			       FIELD_PREP(MII_KSZ9031RN_TX_CTL_M, tx));
> >  
> > -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_RX_DATA_PAD_SKEW, 2,
> > +	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_RX_DATA_PAD_SKEW,
> > +			       MDIO_MMD_WIS,
> >  			       FIELD_PREP(MII_KSZ9031RN_RXD3, rx) |
> >  			       FIELD_PREP(MII_KSZ9031RN_RXD2, rx) |
> >  			       FIELD_PREP(MII_KSZ9031RN_RXD1, rx) |
> >  			       FIELD_PREP(MII_KSZ9031RN_RXD0, rx));
> >  
> > -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_TX_DATA_PAD_SKEW, 2,
> > +	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_TX_DATA_PAD_SKEW,
> > +			       MDIO_MMD_WIS,
> >  			       FIELD_PREP(MII_KSZ9031RN_TXD3, tx) |
> >  			       FIELD_PREP(MII_KSZ9031RN_TXD2, tx) |
> >  			       FIELD_PREP(MII_KSZ9031RN_TXD1, tx) |
> >  			       FIELD_PREP(MII_KSZ9031RN_TXD0, tx));
> >  
> > -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CLK_PAD_SKEW, 2,
> > +	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CLK_PAD_SKEW,
> > +			       MDIO_MMD_WIS,
> >  			       FIELD_PREP(MII_KSZ9031RN_GTX_CLK, tx_clk) |
> >  			       FIELD_PREP(MII_KSZ9031RN_RX_CLK, rx_clk));
> >  	return 0;
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 

Re: [PATCH 3/7] net: phy: sync phy_{write,read,modify}_mmd_indirect with linux

[Marco Felsch]

On 23-07-10, Ahmad Fatoum wrote:
> On 10.07.23 08:36, Marco Felsch wrote:
> > Sync the barebox phy_{write,read,modify}_mmd_indirect API with the Linux
> > phy_{write,read,modify}_mmd API to make it easier and less error prone
> > to port phy drivers. This also fixes the r8169 driver since this driver
> > did not adapt the parameters while porting from Linux.
> > 
> > The sync also aligns the function parameter `prtad` as well as the
> > function documentation.
> 
> This breaks out of tree board code in a subtle and annoying manner.

Do we really need to care about out all-of-tree drivers/boards? If so,
we need to ensure to not break any public function.

> We are in luck that the kernel function has a different name than
> the barebox function, so I'd rather suggest:
> 
>  - Add phy_read_mmd/phy_write_mmd functions with same argument
>    order as Linux
>  - Switch all users in barebox to the new functions
>  - Mark the old _indirect function deprecated with a suggestion
>    to use phy_read_mmd/phy_write_mmd and swap address arguments.

I was think about such approach, then I checked that the Linux API does
handle C45 as well, so we would need least at least $some minimal
support like:

if (c45)
	pr_err("C45 phys are not supported yet\n");
	return -EINVAL;

and therefore I stuck with the _indirect functions.

Regards,
  Marco

> 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  arch/arm/boards/datamodul-edm-qmx6/board.c   |  6 +--
> >  arch/arm/boards/embest-marsboard/board.c     |  6 +--
> >  arch/arm/boards/terasic-de0-nano-soc/board.c |  6 +--
> >  arch/arm/boards/terasic-de10-nano/board.c    |  6 +--
> >  arch/arm/boards/tqma6x/board.c               |  6 +--
> >  drivers/net/phy/at803x.c                     |  4 +-
> >  drivers/net/phy/dp83867.c                    | 40 +++++++-------
> >  drivers/net/phy/micrel.c                     | 24 ++++-----
> >  drivers/net/phy/phy.c                        | 57 ++++++++++----------
> >  include/linux/phy.h                          | 10 ++--
> >  10 files changed, 83 insertions(+), 82 deletions(-)
> > 
> > diff --git a/arch/arm/boards/datamodul-edm-qmx6/board.c b/arch/arm/boards/datamodul-edm-qmx6/board.c
> > index 366b64d35a..0dc71f2aca 100644
> > --- a/arch/arm/boards/datamodul-edm-qmx6/board.c
> > +++ b/arch/arm/boards/datamodul-edm-qmx6/board.c
> > @@ -49,9 +49,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
> >  	 * min rx data delay, max rx/tx clock delay,
> >  	 * min rx/tx control delay
> >  	 */
> > -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> > -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> > -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x03ff);
> > +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> > +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> > +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x03ff);
> >  
> >  	return 0;
> >  }
> > diff --git a/arch/arm/boards/embest-marsboard/board.c b/arch/arm/boards/embest-marsboard/board.c
> > index 7274595e2a..e70fefec5e 100644
> > --- a/arch/arm/boards/embest-marsboard/board.c
> > +++ b/arch/arm/boards/embest-marsboard/board.c
> > @@ -20,13 +20,13 @@ static int ar8035_phy_fixup(struct phy_device *dev)
> >  	/* Ar803x phy SmartEEE feature cause link status generates glitch,
> >  	 * which cause ethernet link down/up issue, so disable SmartEEE
> >  	 */
> > -	val = phy_read_mmd_indirect(dev, 0x805d, MDIO_MMD_PCS);
> > +	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x805d);
> >  	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
> >  
> > -	val = phy_read_mmd_indirect(dev, 0x4003, MDIO_MMD_PCS);
> > +	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x4003);
> >  	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
> >  
> > -	val = phy_read_mmd_indirect(dev, 0x4007, MDIO_MMD_PCS);
> > +	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x4007);
> >  	val &= 0xffe3;
> >  	val |= 0x18;
> >  	phy_write(dev, MII_MMD_DATA, val);
> > diff --git a/arch/arm/boards/terasic-de0-nano-soc/board.c b/arch/arm/boards/terasic-de0-nano-soc/board.c
> > index 832160c595..fa83498f46 100644
> > --- a/arch/arm/boards/terasic-de0-nano-soc/board.c
> > +++ b/arch/arm/boards/terasic-de0-nano-soc/board.c
> > @@ -19,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
> >  	 * min rx data delay, max rx/tx clock delay,
> >  	 * min rx/tx control delay
> >  	 */
> > -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> > -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> > -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
> > +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> > +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> > +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
> >  	return 0;
> >  }
> >  
> > diff --git a/arch/arm/boards/terasic-de10-nano/board.c b/arch/arm/boards/terasic-de10-nano/board.c
> > index e47d9ac841..7ceb691f71 100644
> > --- a/arch/arm/boards/terasic-de10-nano/board.c
> > +++ b/arch/arm/boards/terasic-de10-nano/board.c
> > @@ -19,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
> >  	 * min rx data delay, max rx/tx clock delay,
> >  	 * min rx/tx control delay
> >  	 */
> > -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> > -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> > -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
> > +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> > +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> > +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
> >  	return 0;
> >  }
> >  
> > diff --git a/arch/arm/boards/tqma6x/board.c b/arch/arm/boards/tqma6x/board.c
> > index 8a91ad652a..3fd2e1db2c 100644
> > --- a/arch/arm/boards/tqma6x/board.c
> > +++ b/arch/arm/boards/tqma6x/board.c
> > @@ -47,9 +47,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
> >  	 * min rx data delay, max rx/tx clock delay,
> >  	 * min rx/tx control delay
> >  	 */
> > -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> > -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> > -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
> > +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> > +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> > +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
> >  
> >  	return 0;
> >  }
> > diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> > index 18182bffc2..41010c58ad 100644
> > --- a/drivers/net/phy/at803x.c
> > +++ b/drivers/net/phy/at803x.c
> > @@ -229,14 +229,14 @@ static int at803x_clk_out_config(struct phy_device *phydev)
> >  	if (!priv->clk_25m_mask)
> >  		return 0;
> >  
> > -	val = phy_read_mmd_indirect(phydev, AT803X_MMD7_CLK25M, MDIO_MMD_AN);
> > +	val = phy_read_mmd_indirect(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M);
> >  	if (val < 0)
> >  		return val;
> >  
> >  	val &= ~priv->clk_25m_mask;
> >  	val |= priv->clk_25m_reg;
> >  
> > -	phy_write_mmd_indirect(phydev, AT803X_MMD7_CLK25M, MDIO_MMD_AN, val);
> > +	phy_write_mmd_indirect(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M, val);
> >  
> >  	return 0;
> >  }
> > diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> > index d8109172df..85fe5107da 100644
> > --- a/drivers/net/phy/dp83867.c
> > +++ b/drivers/net/phy/dp83867.c
> > @@ -154,14 +154,14 @@ static int dp83867_config_port_mirroring(struct phy_device *phydev)
> >  	struct dp83867_private *dp83867 = phydev->priv;
> >  	u16 val;
> >  
> > -	val = phy_read_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR);
> > +	val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR, DP83867_CFG4);
> >  
> >  	if (dp83867->port_mirroring == DP83867_PORT_MIRROING_EN)
> >  		val |= DP83867_CFG4_PORT_MIRROR_EN;
> >  	else
> >  		val &= ~DP83867_CFG4_PORT_MIRROR_EN;
> >  
> > -	phy_write_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR, val);
> > +	phy_write_mmd_indirect(phydev, DP83867_DEVADDR, DP83867_CFG4, val);
> >  
> >  	return 0;
> >  }
> > @@ -256,11 +256,11 @@ static int dp83867_config_init(struct phy_device *phydev)
> >  	phy_write(phydev, DP83867_CTRL, val | DP83867_SW_RESTART);
> >  
> >  	if (dp83867->rxctrl_strap_quirk) {
> > -		val = phy_read_mmd_indirect(phydev, DP83867_CFG4,
> > -					    DP83867_DEVADDR);
> > +		val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
> > +					    DP83867_CFG4);
> >  		val &= ~BIT(7);
> > -		phy_write_mmd_indirect(phydev, DP83867_CFG4,
> > -				       DP83867_DEVADDR, val);
> > +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> > +				       DP83867_CFG4, val);
> >  	}
> >  
> >  	if (phy_interface_is_rgmii(phydev)) {
> > @@ -270,8 +270,8 @@ static int dp83867_config_init(struct phy_device *phydev)
> >  		if (ret)
> >  			return ret;
> >  
> > -		val = phy_read_mmd_indirect(phydev, DP83867_RGMIICTL,
> > -					    DP83867_DEVADDR);
> > +		val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
> > +					    DP83867_RGMIICTL);
> >  
> >  		switch (phydev->interface) {
> >  		case PHY_INTERFACE_MODE_RGMII_ID:
> > @@ -287,31 +287,31 @@ static int dp83867_config_init(struct phy_device *phydev)
> >  		default:
> >  			break;
> >  		}
> > -		phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
> > -				       DP83867_DEVADDR, val);
> > +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> > +				       DP83867_RGMIICTL, val);
> >  
> >  		delay = (dp83867->rx_id_delay |
> >  			(dp83867->tx_id_delay << DP83867_RGMII_TX_CLK_DELAY_SHIFT));
> >  
> > -		phy_write_mmd_indirect(phydev, DP83867_RGMIIDCTL,
> > -				       DP83867_DEVADDR, delay);
> > +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> > +				       DP83867_RGMIIDCTL, delay);
> >  
> >  		if (dp83867->io_impedance >= 0) {
> > -			val = phy_read_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
> > -						    DP83867_DEVADDR);
> > +			val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
> > +						    DP83867_IO_MUX_CFG);
> >  			val &= ~DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
> >  			val |= (dp83867->io_impedance &
> >  				DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL);
> >  
> > -			phy_write_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
> > -					       DP83867_DEVADDR, val);
> > +			phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> > +					       DP83867_IO_MUX_CFG, val);
> >  		}
> >  	} else if (phy_interface_is_sgmii(phydev)) {
> >  		phy_write(phydev, MII_BMCR,
> >  			  BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000);
> >  
> > -		phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
> > -				       DP83867_DEVADDR, 0x0);
> > +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> > +				       DP83867_RGMIICTL, 0x0);
> >  
> >  		val = DP83867_PHYCTRL_SGMIIEN |
> >  		      DP83867_MDI_CROSSOVER_MDIX << DP83867_MDI_CROSSOVER |
> > @@ -341,8 +341,8 @@ static int dp83867_config_init(struct phy_device *phydev)
> >  				DP83867_IO_MUX_CFG_CLK_O_SEL_SHIFT;
> >  		}
> >  
> > -		phy_modify_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
> > -				DP83867_DEVADDR, mask, val);
> > +		phy_modify_mmd_indirect(phydev, DP83867_DEVADDR,
> > +					DP83867_IO_MUX_CFG, mask, val);
> >  	}
> >  
> >  	return 0;
> > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> > index 02d474c442..892df01d2e 100644
> > --- a/drivers/net/phy/micrel.c
> > +++ b/drivers/net/phy/micrel.c
> > @@ -387,7 +387,7 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
> >  		return 0;
> >  
> >  	if (matches < numfields)
> > -		newval = phy_read_mmd_indirect(phydev, reg, MDIO_MMD_WIS);
> > +		newval = phy_read_mmd_indirect(phydev, MDIO_MMD_WIS, reg);
> >  	else
> >  		newval = 0;
> >  
> > @@ -401,15 +401,15 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
> >  				<< (field_sz * i));
> >  		}
> >  
> > -	phy_write_mmd_indirect(phydev, reg, MDIO_MMD_WIS, newval);
> > +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS, reg, newval);
> >  	return 0;
> >  }
> >  
> >  static int ksz9031_center_flp_timing(struct phy_device *phydev)
> >  {
> >  	/* Center KSZ9031RNX FLP timing at 16ms. */
> > -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_HI, 0, 0x0006);
> > -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_LO, 0, 0x1a80);
> > +	phy_write_mmd_indirect(phydev, 0, MII_KSZ9031RN_FLP_BURST_TX_HI, 0x0006);
> > +	phy_write_mmd_indirect(phydev, 0, MII_KSZ9031RN_FLP_BURST_TX_LO, 0x1a80);
> >  
> >  	return genphy_restart_aneg(phydev);
> >  }
> > @@ -447,27 +447,27 @@ static int ksz9031_config_rgmii_delay(struct phy_device *phydev)
> >  		return 0;
> >  	}
> >  
> > -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CONTROL_PAD_SKEW,
> > -			       MDIO_MMD_WIS,
> > +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> > +			       MII_KSZ9031RN_CONTROL_PAD_SKEW,
> >  			       FIELD_PREP(MII_KSZ9031RN_RX_CTL_M, rx) |
> >  			       FIELD_PREP(MII_KSZ9031RN_TX_CTL_M, tx));
> >  
> > -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_RX_DATA_PAD_SKEW,
> > -			       MDIO_MMD_WIS,
> > +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> > +			       MII_KSZ9031RN_RX_DATA_PAD_SKEW,
> >  			       FIELD_PREP(MII_KSZ9031RN_RXD3, rx) |
> >  			       FIELD_PREP(MII_KSZ9031RN_RXD2, rx) |
> >  			       FIELD_PREP(MII_KSZ9031RN_RXD1, rx) |
> >  			       FIELD_PREP(MII_KSZ9031RN_RXD0, rx));
> >  
> > -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_TX_DATA_PAD_SKEW,
> > -			       MDIO_MMD_WIS,
> > +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> > +			       MII_KSZ9031RN_TX_DATA_PAD_SKEW,
> >  			       FIELD_PREP(MII_KSZ9031RN_TXD3, tx) |
> >  			       FIELD_PREP(MII_KSZ9031RN_TXD2, tx) |
> >  			       FIELD_PREP(MII_KSZ9031RN_TXD1, tx) |
> >  			       FIELD_PREP(MII_KSZ9031RN_TXD0, tx));
> >  
> > -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CLK_PAD_SKEW,
> > -			       MDIO_MMD_WIS,
> > +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> > +			       MII_KSZ9031RN_CLK_PAD_SKEW,
> >  			       FIELD_PREP(MII_KSZ9031RN_GTX_CLK, tx_clk) |
> >  			       FIELD_PREP(MII_KSZ9031RN_RX_CLK, rx_clk));
> >  	return 0;
> > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > index 74381949b4..283e1a3d79 100644
> > --- a/drivers/net/phy/phy.c
> > +++ b/drivers/net/phy/phy.c
> > @@ -819,24 +819,25 @@ int genphy_read_status(struct phy_device *phydev)
> >  	return 0;
> >  }
> >  
> > -static inline void mmd_phy_indirect(struct phy_device *phydev, int prtad,
> > -					int devad)
> > +static inline void mmd_phy_indirect(struct phy_device *phydev, int devad,
> > +				    u16 regnum)
> >  {
> >  	/* Write the desired MMD Devad */
> >  	phy_write(phydev, MII_MMD_CTRL, devad);
> >  
> >  	/* Write the desired MMD register address */
> > -	phy_write(phydev, MII_MMD_DATA, prtad);
> > +	phy_write(phydev, MII_MMD_DATA, regnum);
> >  
> >  	/* Select the Function : DATA with no post increment */
> >  	phy_write(phydev, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
> >  }
> >  
> >  /**
> > - * phy_read_mmd_indirect - reads data from the MMD registers
> > - * @phy_device: phy device
> > - * @prtad: MMD Address
> > - * @devad: MMD DEVAD
> > + * phy_read_mmd_indirect - Convenience function for reading a register
> > + * from an MMD on a given PHY.
> > + * @phydev: The phy_device struct
> > + * @devad: The MMD to read from
> > + * @regnum: The register on the MMD to read
> >   *
> >   * Description: it reads data from the MMD registers (clause 22 to access to
> >   * clause 45) of the specified phy address.
> > @@ -846,11 +847,11 @@ static inline void mmd_phy_indirect(struct phy_device *phydev, int prtad,
> >   * 3) Write reg 13 // MMD Data Command for MMD DEVAD
> >   * 3) Read  reg 14 // Read MMD data
> >   */
> > -int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
> > +int phy_read_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum)
> >  {
> >  	u32 ret;
> >  
> > -	mmd_phy_indirect(phydev, prtad, devad);
> > +	mmd_phy_indirect(phydev, devad, regnum);
> >  
> >  	/* Read the content of the MMD's selected register */
> >  	ret = phy_read(phydev, MII_MMD_DATA);
> > @@ -859,11 +860,12 @@ int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
> >  }
> >  
> >  /**
> > - * phy_write_mmd_indirect - writes data to the MMD registers
> > - * @phy_device: phy device
> > - * @prtad: MMD Address
> > - * @devad: MMD DEVAD
> > - * @data: data to write in the MMD register
> > + * phy_write_mmd_indirect - Convenience function for writing a register
> > + * on an MMD on a given PHY.
> > + * @phydev: The phy_device struct
> > + * @devad: The MMD to read from
> > + * @regnum: The register on the MMD to read
> > + * @val: value to write to @regnum
> >   *
> >   * Description: Write data from the MMD registers of the specified
> >   * phy address.
> > @@ -873,34 +875,33 @@ int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
> >   * 3) Write reg 13 // MMD Data Command for MMD DEVAD
> >   * 3) Write reg 14 // Write MMD data
> >   */
> > -void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> > -				   u16 data)
> > +void phy_write_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> > +			    u16 val)
> >  {
> > -	mmd_phy_indirect(phydev, prtad, devad);
> > +	mmd_phy_indirect(phydev, devad, regnum);
> >  
> >  	/* Write the data into MMD's selected register */
> > -	phy_write(phydev, MII_MMD_DATA, data);
> > +	phy_write(phydev, MII_MMD_DATA, val);
> >  }
> >  
> >  /**
> > - * phy_modify_mmd_indirect - Convenience function for modifying a MMD register
> > - * @phydev: phy device
> > - * @prtad: MMD Address
> > - * @devad: MMD DEVAD
> > + * phy_modify_mmd_indirect - Convenience function for modifying a register on MMD
> > + * @phydev: the phy_device struct
> > + * @devad: the MMD containing register to modify
> > + * @regnum: register number to modify
> >   * @mask: bit mask of bits to clear
> > - * @set: new value of bits set in @mask
> > - *
> > + * @set: new value of bits set in mask to write to @regnum
> >   */
> > -int phy_modify_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> > -				   u16 mask, u16 set)
> > +int phy_modify_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> > +			    u16 mask, u16 set)
> >  {
> >  	int ret;
> >  
> > -	ret = phy_read_mmd_indirect(phydev, prtad, devad);
> > +	ret = phy_read_mmd_indirect(phydev, devad, regnum);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	phy_write_mmd_indirect(phydev, prtad, devad, (ret & ~mask) | set);
> > +	phy_write_mmd_indirect(phydev, devad, regnum, (ret & ~mask) | set);
> >  
> >  	return 0;
> >  }
> > diff --git a/include/linux/phy.h b/include/linux/phy.h
> > index 509bf72de9..d96c4e97ab 100644
> > --- a/include/linux/phy.h
> > +++ b/include/linux/phy.h
> > @@ -406,11 +406,11 @@ int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
> >  int phy_register_fixup_for_id(const char *bus_id,
> >  		int (*run)(struct phy_device *));
> >  int phy_scan_fixups(struct phy_device *phydev);
> > -int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad);
> > -void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> > -				   u16 data);
> > -int phy_modify_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> > -				    u16 mask, u16 set);
> > +int phy_read_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum);
> > +void phy_write_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> > +			    u16 val);
> > +int phy_modify_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> > +			    u16 mask, u16 set);
> >  
> >  static inline bool phy_acquired(struct phy_device *phydev)
> >  {
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 

Re: [PATCH 3/7] net: phy: sync phy_{write,read,modify}_mmd_indirect with linux

[Ahmad Fatoum]

Hello Marco,

On 10.07.23 12:49, Marco Felsch wrote:
> On 23-07-10, Ahmad Fatoum wrote:
>> On 10.07.23 08:36, Marco Felsch wrote:
>>> Sync the barebox phy_{write,read,modify}_mmd_indirect API with the Linux
>>> phy_{write,read,modify}_mmd API to make it easier and less error prone
>>> to port phy drivers. This also fixes the r8169 driver since this driver
>>> did not adapt the parameters while porting from Linux.
>>>
>>> The sync also aligns the function parameter `prtad` as well as the
>>> function documentation.
>>
>> This breaks out of tree board code in a subtle and annoying manner.
> 
> Do we really need to care about out all-of-tree drivers/boards? If so,
> we need to ensure to not break any public function.

Out-of-tree drivers not too much probably, but out-of-tree boards
are unfortunately the norm. Breaking the build can be justified,
but silent breaking by swapping arguments is not nice.

>> We are in luck that the kernel function has a different name than
>> the barebox function, so I'd rather suggest:
>>
>>  - Add phy_read_mmd/phy_write_mmd functions with same argument
>>    order as Linux
>>  - Switch all users in barebox to the new functions
>>  - Mark the old _indirect function deprecated with a suggestion
>>    to use phy_read_mmd/phy_write_mmd and swap address arguments.
> 
> I was think about such approach, then I checked that the Linux API does
> handle C45 as well, so we would need least at least $some minimal
> support like:
> 
> if (c45)
> 	pr_err("C45 phys are not supported yet\n");
> 	return -EINVAL;

Sounds good IMO. Maybe return -ENOSYS instead.

> 
> and therefore I stuck with the _indirect functions.
> 
> Regards,
>   Marco
> 
>>
>>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>>> ---
>>>  arch/arm/boards/datamodul-edm-qmx6/board.c   |  6 +--
>>>  arch/arm/boards/embest-marsboard/board.c     |  6 +--
>>>  arch/arm/boards/terasic-de0-nano-soc/board.c |  6 +--
>>>  arch/arm/boards/terasic-de10-nano/board.c    |  6 +--
>>>  arch/arm/boards/tqma6x/board.c               |  6 +--
>>>  drivers/net/phy/at803x.c                     |  4 +-
>>>  drivers/net/phy/dp83867.c                    | 40 +++++++-------
>>>  drivers/net/phy/micrel.c                     | 24 ++++-----
>>>  drivers/net/phy/phy.c                        | 57 ++++++++++----------
>>>  include/linux/phy.h                          | 10 ++--
>>>  10 files changed, 83 insertions(+), 82 deletions(-)
>>>
>>> diff --git a/arch/arm/boards/datamodul-edm-qmx6/board.c b/arch/arm/boards/datamodul-edm-qmx6/board.c
>>> index 366b64d35a..0dc71f2aca 100644
>>> --- a/arch/arm/boards/datamodul-edm-qmx6/board.c
>>> +++ b/arch/arm/boards/datamodul-edm-qmx6/board.c
>>> @@ -49,9 +49,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
>>>  	 * min rx data delay, max rx/tx clock delay,
>>>  	 * min rx/tx control delay
>>>  	 */
>>> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
>>> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
>>> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x03ff);
>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x03ff);
>>>  
>>>  	return 0;
>>>  }
>>> diff --git a/arch/arm/boards/embest-marsboard/board.c b/arch/arm/boards/embest-marsboard/board.c
>>> index 7274595e2a..e70fefec5e 100644
>>> --- a/arch/arm/boards/embest-marsboard/board.c
>>> +++ b/arch/arm/boards/embest-marsboard/board.c
>>> @@ -20,13 +20,13 @@ static int ar8035_phy_fixup(struct phy_device *dev)
>>>  	/* Ar803x phy SmartEEE feature cause link status generates glitch,
>>>  	 * which cause ethernet link down/up issue, so disable SmartEEE
>>>  	 */
>>> -	val = phy_read_mmd_indirect(dev, 0x805d, MDIO_MMD_PCS);
>>> +	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x805d);
>>>  	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
>>>  
>>> -	val = phy_read_mmd_indirect(dev, 0x4003, MDIO_MMD_PCS);
>>> +	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x4003);
>>>  	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
>>>  
>>> -	val = phy_read_mmd_indirect(dev, 0x4007, MDIO_MMD_PCS);
>>> +	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x4007);
>>>  	val &= 0xffe3;
>>>  	val |= 0x18;
>>>  	phy_write(dev, MII_MMD_DATA, val);
>>> diff --git a/arch/arm/boards/terasic-de0-nano-soc/board.c b/arch/arm/boards/terasic-de0-nano-soc/board.c
>>> index 832160c595..fa83498f46 100644
>>> --- a/arch/arm/boards/terasic-de0-nano-soc/board.c
>>> +++ b/arch/arm/boards/terasic-de0-nano-soc/board.c
>>> @@ -19,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
>>>  	 * min rx data delay, max rx/tx clock delay,
>>>  	 * min rx/tx control delay
>>>  	 */
>>> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
>>> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
>>> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
>>>  	return 0;
>>>  }
>>>  
>>> diff --git a/arch/arm/boards/terasic-de10-nano/board.c b/arch/arm/boards/terasic-de10-nano/board.c
>>> index e47d9ac841..7ceb691f71 100644
>>> --- a/arch/arm/boards/terasic-de10-nano/board.c
>>> +++ b/arch/arm/boards/terasic-de10-nano/board.c
>>> @@ -19,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
>>>  	 * min rx data delay, max rx/tx clock delay,
>>>  	 * min rx/tx control delay
>>>  	 */
>>> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
>>> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
>>> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
>>>  	return 0;
>>>  }
>>>  
>>> diff --git a/arch/arm/boards/tqma6x/board.c b/arch/arm/boards/tqma6x/board.c
>>> index 8a91ad652a..3fd2e1db2c 100644
>>> --- a/arch/arm/boards/tqma6x/board.c
>>> +++ b/arch/arm/boards/tqma6x/board.c
>>> @@ -47,9 +47,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
>>>  	 * min rx data delay, max rx/tx clock delay,
>>>  	 * min rx/tx control delay
>>>  	 */
>>> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
>>> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
>>> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
>>>  
>>>  	return 0;
>>>  }
>>> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
>>> index 18182bffc2..41010c58ad 100644
>>> --- a/drivers/net/phy/at803x.c
>>> +++ b/drivers/net/phy/at803x.c
>>> @@ -229,14 +229,14 @@ static int at803x_clk_out_config(struct phy_device *phydev)
>>>  	if (!priv->clk_25m_mask)
>>>  		return 0;
>>>  
>>> -	val = phy_read_mmd_indirect(phydev, AT803X_MMD7_CLK25M, MDIO_MMD_AN);
>>> +	val = phy_read_mmd_indirect(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M);
>>>  	if (val < 0)
>>>  		return val;
>>>  
>>>  	val &= ~priv->clk_25m_mask;
>>>  	val |= priv->clk_25m_reg;
>>>  
>>> -	phy_write_mmd_indirect(phydev, AT803X_MMD7_CLK25M, MDIO_MMD_AN, val);
>>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M, val);
>>>  
>>>  	return 0;
>>>  }
>>> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
>>> index d8109172df..85fe5107da 100644
>>> --- a/drivers/net/phy/dp83867.c
>>> +++ b/drivers/net/phy/dp83867.c
>>> @@ -154,14 +154,14 @@ static int dp83867_config_port_mirroring(struct phy_device *phydev)
>>>  	struct dp83867_private *dp83867 = phydev->priv;
>>>  	u16 val;
>>>  
>>> -	val = phy_read_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR);
>>> +	val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR, DP83867_CFG4);
>>>  
>>>  	if (dp83867->port_mirroring == DP83867_PORT_MIRROING_EN)
>>>  		val |= DP83867_CFG4_PORT_MIRROR_EN;
>>>  	else
>>>  		val &= ~DP83867_CFG4_PORT_MIRROR_EN;
>>>  
>>> -	phy_write_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR, val);
>>> +	phy_write_mmd_indirect(phydev, DP83867_DEVADDR, DP83867_CFG4, val);
>>>  
>>>  	return 0;
>>>  }
>>> @@ -256,11 +256,11 @@ static int dp83867_config_init(struct phy_device *phydev)
>>>  	phy_write(phydev, DP83867_CTRL, val | DP83867_SW_RESTART);
>>>  
>>>  	if (dp83867->rxctrl_strap_quirk) {
>>> -		val = phy_read_mmd_indirect(phydev, DP83867_CFG4,
>>> -					    DP83867_DEVADDR);
>>> +		val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
>>> +					    DP83867_CFG4);
>>>  		val &= ~BIT(7);
>>> -		phy_write_mmd_indirect(phydev, DP83867_CFG4,
>>> -				       DP83867_DEVADDR, val);
>>> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
>>> +				       DP83867_CFG4, val);
>>>  	}
>>>  
>>>  	if (phy_interface_is_rgmii(phydev)) {
>>> @@ -270,8 +270,8 @@ static int dp83867_config_init(struct phy_device *phydev)
>>>  		if (ret)
>>>  			return ret;
>>>  
>>> -		val = phy_read_mmd_indirect(phydev, DP83867_RGMIICTL,
>>> -					    DP83867_DEVADDR);
>>> +		val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
>>> +					    DP83867_RGMIICTL);
>>>  
>>>  		switch (phydev->interface) {
>>>  		case PHY_INTERFACE_MODE_RGMII_ID:
>>> @@ -287,31 +287,31 @@ static int dp83867_config_init(struct phy_device *phydev)
>>>  		default:
>>>  			break;
>>>  		}
>>> -		phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
>>> -				       DP83867_DEVADDR, val);
>>> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
>>> +				       DP83867_RGMIICTL, val);
>>>  
>>>  		delay = (dp83867->rx_id_delay |
>>>  			(dp83867->tx_id_delay << DP83867_RGMII_TX_CLK_DELAY_SHIFT));
>>>  
>>> -		phy_write_mmd_indirect(phydev, DP83867_RGMIIDCTL,
>>> -				       DP83867_DEVADDR, delay);
>>> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
>>> +				       DP83867_RGMIIDCTL, delay);
>>>  
>>>  		if (dp83867->io_impedance >= 0) {
>>> -			val = phy_read_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
>>> -						    DP83867_DEVADDR);
>>> +			val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
>>> +						    DP83867_IO_MUX_CFG);
>>>  			val &= ~DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
>>>  			val |= (dp83867->io_impedance &
>>>  				DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL);
>>>  
>>> -			phy_write_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
>>> -					       DP83867_DEVADDR, val);
>>> +			phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
>>> +					       DP83867_IO_MUX_CFG, val);
>>>  		}
>>>  	} else if (phy_interface_is_sgmii(phydev)) {
>>>  		phy_write(phydev, MII_BMCR,
>>>  			  BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000);
>>>  
>>> -		phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
>>> -				       DP83867_DEVADDR, 0x0);
>>> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
>>> +				       DP83867_RGMIICTL, 0x0);
>>>  
>>>  		val = DP83867_PHYCTRL_SGMIIEN |
>>>  		      DP83867_MDI_CROSSOVER_MDIX << DP83867_MDI_CROSSOVER |
>>> @@ -341,8 +341,8 @@ static int dp83867_config_init(struct phy_device *phydev)
>>>  				DP83867_IO_MUX_CFG_CLK_O_SEL_SHIFT;
>>>  		}
>>>  
>>> -		phy_modify_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
>>> -				DP83867_DEVADDR, mask, val);
>>> +		phy_modify_mmd_indirect(phydev, DP83867_DEVADDR,
>>> +					DP83867_IO_MUX_CFG, mask, val);
>>>  	}
>>>  
>>>  	return 0;
>>> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
>>> index 02d474c442..892df01d2e 100644
>>> --- a/drivers/net/phy/micrel.c
>>> +++ b/drivers/net/phy/micrel.c
>>> @@ -387,7 +387,7 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
>>>  		return 0;
>>>  
>>>  	if (matches < numfields)
>>> -		newval = phy_read_mmd_indirect(phydev, reg, MDIO_MMD_WIS);
>>> +		newval = phy_read_mmd_indirect(phydev, MDIO_MMD_WIS, reg);
>>>  	else
>>>  		newval = 0;
>>>  
>>> @@ -401,15 +401,15 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
>>>  				<< (field_sz * i));
>>>  		}
>>>  
>>> -	phy_write_mmd_indirect(phydev, reg, MDIO_MMD_WIS, newval);
>>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS, reg, newval);
>>>  	return 0;
>>>  }
>>>  
>>>  static int ksz9031_center_flp_timing(struct phy_device *phydev)
>>>  {
>>>  	/* Center KSZ9031RNX FLP timing at 16ms. */
>>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_HI, 0, 0x0006);
>>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_LO, 0, 0x1a80);
>>> +	phy_write_mmd_indirect(phydev, 0, MII_KSZ9031RN_FLP_BURST_TX_HI, 0x0006);
>>> +	phy_write_mmd_indirect(phydev, 0, MII_KSZ9031RN_FLP_BURST_TX_LO, 0x1a80);
>>>  
>>>  	return genphy_restart_aneg(phydev);
>>>  }
>>> @@ -447,27 +447,27 @@ static int ksz9031_config_rgmii_delay(struct phy_device *phydev)
>>>  		return 0;
>>>  	}
>>>  
>>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CONTROL_PAD_SKEW,
>>> -			       MDIO_MMD_WIS,
>>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
>>> +			       MII_KSZ9031RN_CONTROL_PAD_SKEW,
>>>  			       FIELD_PREP(MII_KSZ9031RN_RX_CTL_M, rx) |
>>>  			       FIELD_PREP(MII_KSZ9031RN_TX_CTL_M, tx));
>>>  
>>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_RX_DATA_PAD_SKEW,
>>> -			       MDIO_MMD_WIS,
>>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
>>> +			       MII_KSZ9031RN_RX_DATA_PAD_SKEW,
>>>  			       FIELD_PREP(MII_KSZ9031RN_RXD3, rx) |
>>>  			       FIELD_PREP(MII_KSZ9031RN_RXD2, rx) |
>>>  			       FIELD_PREP(MII_KSZ9031RN_RXD1, rx) |
>>>  			       FIELD_PREP(MII_KSZ9031RN_RXD0, rx));
>>>  
>>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_TX_DATA_PAD_SKEW,
>>> -			       MDIO_MMD_WIS,
>>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
>>> +			       MII_KSZ9031RN_TX_DATA_PAD_SKEW,
>>>  			       FIELD_PREP(MII_KSZ9031RN_TXD3, tx) |
>>>  			       FIELD_PREP(MII_KSZ9031RN_TXD2, tx) |
>>>  			       FIELD_PREP(MII_KSZ9031RN_TXD1, tx) |
>>>  			       FIELD_PREP(MII_KSZ9031RN_TXD0, tx));
>>>  
>>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CLK_PAD_SKEW,
>>> -			       MDIO_MMD_WIS,
>>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
>>> +			       MII_KSZ9031RN_CLK_PAD_SKEW,
>>>  			       FIELD_PREP(MII_KSZ9031RN_GTX_CLK, tx_clk) |
>>>  			       FIELD_PREP(MII_KSZ9031RN_RX_CLK, rx_clk));
>>>  	return 0;
>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>> index 74381949b4..283e1a3d79 100644
>>> --- a/drivers/net/phy/phy.c
>>> +++ b/drivers/net/phy/phy.c
>>> @@ -819,24 +819,25 @@ int genphy_read_status(struct phy_device *phydev)
>>>  	return 0;
>>>  }
>>>  
>>> -static inline void mmd_phy_indirect(struct phy_device *phydev, int prtad,
>>> -					int devad)
>>> +static inline void mmd_phy_indirect(struct phy_device *phydev, int devad,
>>> +				    u16 regnum)
>>>  {
>>>  	/* Write the desired MMD Devad */
>>>  	phy_write(phydev, MII_MMD_CTRL, devad);
>>>  
>>>  	/* Write the desired MMD register address */
>>> -	phy_write(phydev, MII_MMD_DATA, prtad);
>>> +	phy_write(phydev, MII_MMD_DATA, regnum);
>>>  
>>>  	/* Select the Function : DATA with no post increment */
>>>  	phy_write(phydev, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
>>>  }
>>>  
>>>  /**
>>> - * phy_read_mmd_indirect - reads data from the MMD registers
>>> - * @phy_device: phy device
>>> - * @prtad: MMD Address
>>> - * @devad: MMD DEVAD
>>> + * phy_read_mmd_indirect - Convenience function for reading a register
>>> + * from an MMD on a given PHY.
>>> + * @phydev: The phy_device struct
>>> + * @devad: The MMD to read from
>>> + * @regnum: The register on the MMD to read
>>>   *
>>>   * Description: it reads data from the MMD registers (clause 22 to access to
>>>   * clause 45) of the specified phy address.
>>> @@ -846,11 +847,11 @@ static inline void mmd_phy_indirect(struct phy_device *phydev, int prtad,
>>>   * 3) Write reg 13 // MMD Data Command for MMD DEVAD
>>>   * 3) Read  reg 14 // Read MMD data
>>>   */
>>> -int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
>>> +int phy_read_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum)
>>>  {
>>>  	u32 ret;
>>>  
>>> -	mmd_phy_indirect(phydev, prtad, devad);
>>> +	mmd_phy_indirect(phydev, devad, regnum);
>>>  
>>>  	/* Read the content of the MMD's selected register */
>>>  	ret = phy_read(phydev, MII_MMD_DATA);
>>> @@ -859,11 +860,12 @@ int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
>>>  }
>>>  
>>>  /**
>>> - * phy_write_mmd_indirect - writes data to the MMD registers
>>> - * @phy_device: phy device
>>> - * @prtad: MMD Address
>>> - * @devad: MMD DEVAD
>>> - * @data: data to write in the MMD register
>>> + * phy_write_mmd_indirect - Convenience function for writing a register
>>> + * on an MMD on a given PHY.
>>> + * @phydev: The phy_device struct
>>> + * @devad: The MMD to read from
>>> + * @regnum: The register on the MMD to read
>>> + * @val: value to write to @regnum
>>>   *
>>>   * Description: Write data from the MMD registers of the specified
>>>   * phy address.
>>> @@ -873,34 +875,33 @@ int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
>>>   * 3) Write reg 13 // MMD Data Command for MMD DEVAD
>>>   * 3) Write reg 14 // Write MMD data
>>>   */
>>> -void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
>>> -				   u16 data)
>>> +void phy_write_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
>>> +			    u16 val)
>>>  {
>>> -	mmd_phy_indirect(phydev, prtad, devad);
>>> +	mmd_phy_indirect(phydev, devad, regnum);
>>>  
>>>  	/* Write the data into MMD's selected register */
>>> -	phy_write(phydev, MII_MMD_DATA, data);
>>> +	phy_write(phydev, MII_MMD_DATA, val);
>>>  }
>>>  
>>>  /**
>>> - * phy_modify_mmd_indirect - Convenience function for modifying a MMD register
>>> - * @phydev: phy device
>>> - * @prtad: MMD Address
>>> - * @devad: MMD DEVAD
>>> + * phy_modify_mmd_indirect - Convenience function for modifying a register on MMD
>>> + * @phydev: the phy_device struct
>>> + * @devad: the MMD containing register to modify
>>> + * @regnum: register number to modify
>>>   * @mask: bit mask of bits to clear
>>> - * @set: new value of bits set in @mask
>>> - *
>>> + * @set: new value of bits set in mask to write to @regnum
>>>   */
>>> -int phy_modify_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
>>> -				   u16 mask, u16 set)
>>> +int phy_modify_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
>>> +			    u16 mask, u16 set)
>>>  {
>>>  	int ret;
>>>  
>>> -	ret = phy_read_mmd_indirect(phydev, prtad, devad);
>>> +	ret = phy_read_mmd_indirect(phydev, devad, regnum);
>>>  	if (ret < 0)
>>>  		return ret;
>>>  
>>> -	phy_write_mmd_indirect(phydev, prtad, devad, (ret & ~mask) | set);
>>> +	phy_write_mmd_indirect(phydev, devad, regnum, (ret & ~mask) | set);
>>>  
>>>  	return 0;
>>>  }
>>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>>> index 509bf72de9..d96c4e97ab 100644
>>> --- a/include/linux/phy.h
>>> +++ b/include/linux/phy.h
>>> @@ -406,11 +406,11 @@ int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
>>>  int phy_register_fixup_for_id(const char *bus_id,
>>>  		int (*run)(struct phy_device *));
>>>  int phy_scan_fixups(struct phy_device *phydev);
>>> -int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad);
>>> -void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
>>> -				   u16 data);
>>> -int phy_modify_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
>>> -				    u16 mask, u16 set);
>>> +int phy_read_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum);
>>> +void phy_write_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
>>> +			    u16 val);
>>> +int phy_modify_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
>>> +			    u16 mask, u16 set);
>>>  
>>>  static inline bool phy_acquired(struct phy_device *phydev)
>>>  {
>>
>> -- 
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 6/7] net: phy: at803x: add disable hibernation mode support

[Marco Felsch]

On 23-07-10, Ahmad Fatoum wrote:
> On 10.07.23 08:36, Marco Felsch wrote:
> > This commit ports Linux commit:
> > 
> > | commit 9ecf04016c87bcb33b44e24489d33618e2592f41
> > | Author: Wei Fang <wei.fang@nxp.com>
> > | Date:   Thu Aug 18 11:00:54 2022 +0800
> > |
> > |     net: phy: at803x: add disable hibernation mode support
> > |
> > |     When the cable is unplugged, the Atheros AR803x PHYs will enter
> > |     hibernation mode after about 10 seconds if the hibernation mode
> > |     is enabled and will not provide any clock to the MAC. But for
> > |     some MACs, this feature might cause unexpected issues due to the
> > |     logic of MACs.
> > |     Taking SYNP MAC (stmmac) as an example, if the cable is unplugged
> > |     and the "eth0" interface is down, the AR803x PHY will enter
> > |     hibernation mode. Then perform the "ifconfig eth0 up" operation,
> > |     the stmmac can't be able to complete the software reset operation
> > |     and fail to init it's own DMA. Therefore, the "eth0" interface is
> > |     failed to ifconfig up. Why does it cause this issue? The truth is
> > |     that the software reset operation of the stmmac is designed to
> > |     depend on the RX_CLK of PHY.
> > |     So, this patch offers an option for the user to determine whether
> > |     to disable the hibernation mode of AR803x PHYs.
> > |
> > |     Signed-off-by: Wei Fang <wei.fang@nxp.com>
> > |     Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > |     Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> 
> IMO we should just makes AT803X_DISABLE_HIBERNATION_MODE the default and
> never hibernate the PHY. Let Linux worry about that.

Got your point and I'm not sure. At least for drivers I like the sources
to bin in sync with Linux which allows porting fixes/features more
easily. Also since the API (dt-bindings) are there, so why not just
copy'n'paste from Linux?

Regards,
  Marco

> > ---
> >  drivers/net/phy/at803x.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> > 
> > diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> > index cc425c318f..d8a9c3588f 100644
> > --- a/drivers/net/phy/at803x.c
> > +++ b/drivers/net/phy/at803x.c
> > @@ -32,6 +32,9 @@
> >  #define AT803X_DEBUG_REG_5			0x05
> >  #define AT803X_DEBUG_TX_CLK_DLY_EN		BIT(8)
> >  
> > +#define AT803X_DEBUG_REG_HIB_CTRL		0x0b
> > +#define   AT803X_DEBUG_HIB_CTRL_PS_HIB_EN	BIT(15)
> > +
> >  /* AT803x supports either the XTAL input pad, an internal PLL or the
> >   * DSP as clock reference for the clock output pad. The XTAL reference
> >   * is only used for 25 MHz output, all other frequencies need the PLL.
> > @@ -140,6 +143,20 @@ static int at803x_disable_tx_delay(struct phy_device *phydev)
> >  				     AT803X_DEBUG_TX_CLK_DLY_EN, 0);
> >  }
> >  
> > +static int at803x_hibernation_mode_config(struct phy_device *phydev)
> > +{
> > +	struct at803x_priv *priv = phydev->priv;
> > +
> > +	/* The default after hardware reset is hibernation mode enabled. After
> > +	 * software reset, the value is retained.
> > +	 */
> > +	if (!(priv->flags & AT803X_DISABLE_HIBERNATION_MODE))
> > +		return 0;
> > +
> > +	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_HIB_CTRL,
> > +				     AT803X_DEBUG_HIB_CTRL_PS_HIB_EN, 0);
> > +}
> > +
> >  static bool at803x_match_phy_id(struct phy_device *phydev, u32 phy_id)
> >  {
> >  	struct phy_driver *drv = to_phy_driver(phydev->dev.driver);
> > @@ -160,6 +177,9 @@ static int at803x_parse_dt(struct phy_device *phydev)
> >  	if (of_property_read_bool(node, "qca,disable-smarteee"))
> >  		priv->flags |= AT803X_DISABLE_SMARTEEE;
> >  
> > +	if (of_property_read_bool(node, "qca,disable-hibernation-mode"))
> > +		priv->flags |= AT803X_DISABLE_HIBERNATION_MODE;
> > +
> >  	if (!of_property_read_u32(node, "qca,smarteee-tw-us-1g", &tw)) {
> >  		if (!tw || tw > 255) {
> >  			phydev_err(phydev, "invalid qca,smarteee-tw-us-1g\n");
> > @@ -341,6 +361,10 @@ static int at803x_config_init(struct phy_device *phydev)
> >  	if (ret < 0)
> >  		return ret;
> >  
> > +	ret = at803x_hibernation_mode_config(phydev);
> > +	if (ret < 0)
> > +		return ret;
> > +
> >  	return 0;
> >  }
> >  
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 

Re: [PATCH 3/7] net: phy: sync phy_{write,read,modify}_mmd_indirect with linux

[Marco Felsch]

On 23-07-10, Ahmad Fatoum wrote:
> Hello Marco,
> 
> On 10.07.23 12:49, Marco Felsch wrote:
> > On 23-07-10, Ahmad Fatoum wrote:
> >> On 10.07.23 08:36, Marco Felsch wrote:
> >>> Sync the barebox phy_{write,read,modify}_mmd_indirect API with the Linux
> >>> phy_{write,read,modify}_mmd API to make it easier and less error prone
> >>> to port phy drivers. This also fixes the r8169 driver since this driver
> >>> did not adapt the parameters while porting from Linux.
> >>>
> >>> The sync also aligns the function parameter `prtad` as well as the
> >>> function documentation.
> >>
> >> This breaks out of tree board code in a subtle and annoying manner.
> > 
> > Do we really need to care about out all-of-tree drivers/boards? If so,
> > we need to ensure to not break any public function.
> 
> Out-of-tree drivers not too much probably, but out-of-tree boards
> are unfortunately the norm. Breaking the build can be justified,
> but silent breaking by swapping arguments is not nice.

And how do you ensure that a public API is only used by drivers? Don't
get me wrong I got your point but if you update an out-of-tree board to
a newer barebox version it may happen that something break. Therefore
manufacturers should upstream there code to reduce the maintenance
effort to a minimum :) I know we don't live in an ideal world :/

Regards,
  Marco

> >> We are in luck that the kernel function has a different name than
> >> the barebox function, so I'd rather suggest:
> >>
> >>  - Add phy_read_mmd/phy_write_mmd functions with same argument
> >>    order as Linux
> >>  - Switch all users in barebox to the new functions
> >>  - Mark the old _indirect function deprecated with a suggestion
> >>    to use phy_read_mmd/phy_write_mmd and swap address arguments.
> > 
> > I was think about such approach, then I checked that the Linux API does
> > handle C45 as well, so we would need least at least $some minimal
> > support like:
> > 
> > if (c45)
> > 	pr_err("C45 phys are not supported yet\n");
> > 	return -EINVAL;
> 
> Sounds good IMO. Maybe return -ENOSYS instead.
> 
> > 
> > and therefore I stuck with the _indirect functions.
> > 
> > Regards,
> >   Marco
> > 
> >>
> >>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> >>> ---
> >>>  arch/arm/boards/datamodul-edm-qmx6/board.c   |  6 +--
> >>>  arch/arm/boards/embest-marsboard/board.c     |  6 +--
> >>>  arch/arm/boards/terasic-de0-nano-soc/board.c |  6 +--
> >>>  arch/arm/boards/terasic-de10-nano/board.c    |  6 +--
> >>>  arch/arm/boards/tqma6x/board.c               |  6 +--
> >>>  drivers/net/phy/at803x.c                     |  4 +-
> >>>  drivers/net/phy/dp83867.c                    | 40 +++++++-------
> >>>  drivers/net/phy/micrel.c                     | 24 ++++-----
> >>>  drivers/net/phy/phy.c                        | 57 ++++++++++----------
> >>>  include/linux/phy.h                          | 10 ++--
> >>>  10 files changed, 83 insertions(+), 82 deletions(-)
> >>>
> >>> diff --git a/arch/arm/boards/datamodul-edm-qmx6/board.c b/arch/arm/boards/datamodul-edm-qmx6/board.c
> >>> index 366b64d35a..0dc71f2aca 100644
> >>> --- a/arch/arm/boards/datamodul-edm-qmx6/board.c
> >>> +++ b/arch/arm/boards/datamodul-edm-qmx6/board.c
> >>> @@ -49,9 +49,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
> >>>  	 * min rx data delay, max rx/tx clock delay,
> >>>  	 * min rx/tx control delay
> >>>  	 */
> >>> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> >>> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> >>> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x03ff);
> >>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> >>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> >>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x03ff);
> >>>  
> >>>  	return 0;
> >>>  }
> >>> diff --git a/arch/arm/boards/embest-marsboard/board.c b/arch/arm/boards/embest-marsboard/board.c
> >>> index 7274595e2a..e70fefec5e 100644
> >>> --- a/arch/arm/boards/embest-marsboard/board.c
> >>> +++ b/arch/arm/boards/embest-marsboard/board.c
> >>> @@ -20,13 +20,13 @@ static int ar8035_phy_fixup(struct phy_device *dev)
> >>>  	/* Ar803x phy SmartEEE feature cause link status generates glitch,
> >>>  	 * which cause ethernet link down/up issue, so disable SmartEEE
> >>>  	 */
> >>> -	val = phy_read_mmd_indirect(dev, 0x805d, MDIO_MMD_PCS);
> >>> +	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x805d);
> >>>  	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
> >>>  
> >>> -	val = phy_read_mmd_indirect(dev, 0x4003, MDIO_MMD_PCS);
> >>> +	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x4003);
> >>>  	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
> >>>  
> >>> -	val = phy_read_mmd_indirect(dev, 0x4007, MDIO_MMD_PCS);
> >>> +	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x4007);
> >>>  	val &= 0xffe3;
> >>>  	val |= 0x18;
> >>>  	phy_write(dev, MII_MMD_DATA, val);
> >>> diff --git a/arch/arm/boards/terasic-de0-nano-soc/board.c b/arch/arm/boards/terasic-de0-nano-soc/board.c
> >>> index 832160c595..fa83498f46 100644
> >>> --- a/arch/arm/boards/terasic-de0-nano-soc/board.c
> >>> +++ b/arch/arm/boards/terasic-de0-nano-soc/board.c
> >>> @@ -19,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
> >>>  	 * min rx data delay, max rx/tx clock delay,
> >>>  	 * min rx/tx control delay
> >>>  	 */
> >>> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> >>> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> >>> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
> >>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> >>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> >>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
> >>>  	return 0;
> >>>  }
> >>>  
> >>> diff --git a/arch/arm/boards/terasic-de10-nano/board.c b/arch/arm/boards/terasic-de10-nano/board.c
> >>> index e47d9ac841..7ceb691f71 100644
> >>> --- a/arch/arm/boards/terasic-de10-nano/board.c
> >>> +++ b/arch/arm/boards/terasic-de10-nano/board.c
> >>> @@ -19,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
> >>>  	 * min rx data delay, max rx/tx clock delay,
> >>>  	 * min rx/tx control delay
> >>>  	 */
> >>> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> >>> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> >>> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
> >>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> >>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> >>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
> >>>  	return 0;
> >>>  }
> >>>  
> >>> diff --git a/arch/arm/boards/tqma6x/board.c b/arch/arm/boards/tqma6x/board.c
> >>> index 8a91ad652a..3fd2e1db2c 100644
> >>> --- a/arch/arm/boards/tqma6x/board.c
> >>> +++ b/arch/arm/boards/tqma6x/board.c
> >>> @@ -47,9 +47,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
> >>>  	 * min rx data delay, max rx/tx clock delay,
> >>>  	 * min rx/tx control delay
> >>>  	 */
> >>> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
> >>> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
> >>> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
> >>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
> >>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
> >>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
> >>>  
> >>>  	return 0;
> >>>  }
> >>> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> >>> index 18182bffc2..41010c58ad 100644
> >>> --- a/drivers/net/phy/at803x.c
> >>> +++ b/drivers/net/phy/at803x.c
> >>> @@ -229,14 +229,14 @@ static int at803x_clk_out_config(struct phy_device *phydev)
> >>>  	if (!priv->clk_25m_mask)
> >>>  		return 0;
> >>>  
> >>> -	val = phy_read_mmd_indirect(phydev, AT803X_MMD7_CLK25M, MDIO_MMD_AN);
> >>> +	val = phy_read_mmd_indirect(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M);
> >>>  	if (val < 0)
> >>>  		return val;
> >>>  
> >>>  	val &= ~priv->clk_25m_mask;
> >>>  	val |= priv->clk_25m_reg;
> >>>  
> >>> -	phy_write_mmd_indirect(phydev, AT803X_MMD7_CLK25M, MDIO_MMD_AN, val);
> >>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M, val);
> >>>  
> >>>  	return 0;
> >>>  }
> >>> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> >>> index d8109172df..85fe5107da 100644
> >>> --- a/drivers/net/phy/dp83867.c
> >>> +++ b/drivers/net/phy/dp83867.c
> >>> @@ -154,14 +154,14 @@ static int dp83867_config_port_mirroring(struct phy_device *phydev)
> >>>  	struct dp83867_private *dp83867 = phydev->priv;
> >>>  	u16 val;
> >>>  
> >>> -	val = phy_read_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR);
> >>> +	val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR, DP83867_CFG4);
> >>>  
> >>>  	if (dp83867->port_mirroring == DP83867_PORT_MIRROING_EN)
> >>>  		val |= DP83867_CFG4_PORT_MIRROR_EN;
> >>>  	else
> >>>  		val &= ~DP83867_CFG4_PORT_MIRROR_EN;
> >>>  
> >>> -	phy_write_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR, val);
> >>> +	phy_write_mmd_indirect(phydev, DP83867_DEVADDR, DP83867_CFG4, val);
> >>>  
> >>>  	return 0;
> >>>  }
> >>> @@ -256,11 +256,11 @@ static int dp83867_config_init(struct phy_device *phydev)
> >>>  	phy_write(phydev, DP83867_CTRL, val | DP83867_SW_RESTART);
> >>>  
> >>>  	if (dp83867->rxctrl_strap_quirk) {
> >>> -		val = phy_read_mmd_indirect(phydev, DP83867_CFG4,
> >>> -					    DP83867_DEVADDR);
> >>> +		val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
> >>> +					    DP83867_CFG4);
> >>>  		val &= ~BIT(7);
> >>> -		phy_write_mmd_indirect(phydev, DP83867_CFG4,
> >>> -				       DP83867_DEVADDR, val);
> >>> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> >>> +				       DP83867_CFG4, val);
> >>>  	}
> >>>  
> >>>  	if (phy_interface_is_rgmii(phydev)) {
> >>> @@ -270,8 +270,8 @@ static int dp83867_config_init(struct phy_device *phydev)
> >>>  		if (ret)
> >>>  			return ret;
> >>>  
> >>> -		val = phy_read_mmd_indirect(phydev, DP83867_RGMIICTL,
> >>> -					    DP83867_DEVADDR);
> >>> +		val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
> >>> +					    DP83867_RGMIICTL);
> >>>  
> >>>  		switch (phydev->interface) {
> >>>  		case PHY_INTERFACE_MODE_RGMII_ID:
> >>> @@ -287,31 +287,31 @@ static int dp83867_config_init(struct phy_device *phydev)
> >>>  		default:
> >>>  			break;
> >>>  		}
> >>> -		phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
> >>> -				       DP83867_DEVADDR, val);
> >>> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> >>> +				       DP83867_RGMIICTL, val);
> >>>  
> >>>  		delay = (dp83867->rx_id_delay |
> >>>  			(dp83867->tx_id_delay << DP83867_RGMII_TX_CLK_DELAY_SHIFT));
> >>>  
> >>> -		phy_write_mmd_indirect(phydev, DP83867_RGMIIDCTL,
> >>> -				       DP83867_DEVADDR, delay);
> >>> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> >>> +				       DP83867_RGMIIDCTL, delay);
> >>>  
> >>>  		if (dp83867->io_impedance >= 0) {
> >>> -			val = phy_read_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
> >>> -						    DP83867_DEVADDR);
> >>> +			val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
> >>> +						    DP83867_IO_MUX_CFG);
> >>>  			val &= ~DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
> >>>  			val |= (dp83867->io_impedance &
> >>>  				DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL);
> >>>  
> >>> -			phy_write_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
> >>> -					       DP83867_DEVADDR, val);
> >>> +			phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> >>> +					       DP83867_IO_MUX_CFG, val);
> >>>  		}
> >>>  	} else if (phy_interface_is_sgmii(phydev)) {
> >>>  		phy_write(phydev, MII_BMCR,
> >>>  			  BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000);
> >>>  
> >>> -		phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
> >>> -				       DP83867_DEVADDR, 0x0);
> >>> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
> >>> +				       DP83867_RGMIICTL, 0x0);
> >>>  
> >>>  		val = DP83867_PHYCTRL_SGMIIEN |
> >>>  		      DP83867_MDI_CROSSOVER_MDIX << DP83867_MDI_CROSSOVER |
> >>> @@ -341,8 +341,8 @@ static int dp83867_config_init(struct phy_device *phydev)
> >>>  				DP83867_IO_MUX_CFG_CLK_O_SEL_SHIFT;
> >>>  		}
> >>>  
> >>> -		phy_modify_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
> >>> -				DP83867_DEVADDR, mask, val);
> >>> +		phy_modify_mmd_indirect(phydev, DP83867_DEVADDR,
> >>> +					DP83867_IO_MUX_CFG, mask, val);
> >>>  	}
> >>>  
> >>>  	return 0;
> >>> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> >>> index 02d474c442..892df01d2e 100644
> >>> --- a/drivers/net/phy/micrel.c
> >>> +++ b/drivers/net/phy/micrel.c
> >>> @@ -387,7 +387,7 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
> >>>  		return 0;
> >>>  
> >>>  	if (matches < numfields)
> >>> -		newval = phy_read_mmd_indirect(phydev, reg, MDIO_MMD_WIS);
> >>> +		newval = phy_read_mmd_indirect(phydev, MDIO_MMD_WIS, reg);
> >>>  	else
> >>>  		newval = 0;
> >>>  
> >>> @@ -401,15 +401,15 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
> >>>  				<< (field_sz * i));
> >>>  		}
> >>>  
> >>> -	phy_write_mmd_indirect(phydev, reg, MDIO_MMD_WIS, newval);
> >>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS, reg, newval);
> >>>  	return 0;
> >>>  }
> >>>  
> >>>  static int ksz9031_center_flp_timing(struct phy_device *phydev)
> >>>  {
> >>>  	/* Center KSZ9031RNX FLP timing at 16ms. */
> >>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_HI, 0, 0x0006);
> >>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_LO, 0, 0x1a80);
> >>> +	phy_write_mmd_indirect(phydev, 0, MII_KSZ9031RN_FLP_BURST_TX_HI, 0x0006);
> >>> +	phy_write_mmd_indirect(phydev, 0, MII_KSZ9031RN_FLP_BURST_TX_LO, 0x1a80);
> >>>  
> >>>  	return genphy_restart_aneg(phydev);
> >>>  }
> >>> @@ -447,27 +447,27 @@ static int ksz9031_config_rgmii_delay(struct phy_device *phydev)
> >>>  		return 0;
> >>>  	}
> >>>  
> >>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CONTROL_PAD_SKEW,
> >>> -			       MDIO_MMD_WIS,
> >>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> >>> +			       MII_KSZ9031RN_CONTROL_PAD_SKEW,
> >>>  			       FIELD_PREP(MII_KSZ9031RN_RX_CTL_M, rx) |
> >>>  			       FIELD_PREP(MII_KSZ9031RN_TX_CTL_M, tx));
> >>>  
> >>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_RX_DATA_PAD_SKEW,
> >>> -			       MDIO_MMD_WIS,
> >>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> >>> +			       MII_KSZ9031RN_RX_DATA_PAD_SKEW,
> >>>  			       FIELD_PREP(MII_KSZ9031RN_RXD3, rx) |
> >>>  			       FIELD_PREP(MII_KSZ9031RN_RXD2, rx) |
> >>>  			       FIELD_PREP(MII_KSZ9031RN_RXD1, rx) |
> >>>  			       FIELD_PREP(MII_KSZ9031RN_RXD0, rx));
> >>>  
> >>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_TX_DATA_PAD_SKEW,
> >>> -			       MDIO_MMD_WIS,
> >>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> >>> +			       MII_KSZ9031RN_TX_DATA_PAD_SKEW,
> >>>  			       FIELD_PREP(MII_KSZ9031RN_TXD3, tx) |
> >>>  			       FIELD_PREP(MII_KSZ9031RN_TXD2, tx) |
> >>>  			       FIELD_PREP(MII_KSZ9031RN_TXD1, tx) |
> >>>  			       FIELD_PREP(MII_KSZ9031RN_TXD0, tx));
> >>>  
> >>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CLK_PAD_SKEW,
> >>> -			       MDIO_MMD_WIS,
> >>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
> >>> +			       MII_KSZ9031RN_CLK_PAD_SKEW,
> >>>  			       FIELD_PREP(MII_KSZ9031RN_GTX_CLK, tx_clk) |
> >>>  			       FIELD_PREP(MII_KSZ9031RN_RX_CLK, rx_clk));
> >>>  	return 0;
> >>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> >>> index 74381949b4..283e1a3d79 100644
> >>> --- a/drivers/net/phy/phy.c
> >>> +++ b/drivers/net/phy/phy.c
> >>> @@ -819,24 +819,25 @@ int genphy_read_status(struct phy_device *phydev)
> >>>  	return 0;
> >>>  }
> >>>  
> >>> -static inline void mmd_phy_indirect(struct phy_device *phydev, int prtad,
> >>> -					int devad)
> >>> +static inline void mmd_phy_indirect(struct phy_device *phydev, int devad,
> >>> +				    u16 regnum)
> >>>  {
> >>>  	/* Write the desired MMD Devad */
> >>>  	phy_write(phydev, MII_MMD_CTRL, devad);
> >>>  
> >>>  	/* Write the desired MMD register address */
> >>> -	phy_write(phydev, MII_MMD_DATA, prtad);
> >>> +	phy_write(phydev, MII_MMD_DATA, regnum);
> >>>  
> >>>  	/* Select the Function : DATA with no post increment */
> >>>  	phy_write(phydev, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
> >>>  }
> >>>  
> >>>  /**
> >>> - * phy_read_mmd_indirect - reads data from the MMD registers
> >>> - * @phy_device: phy device
> >>> - * @prtad: MMD Address
> >>> - * @devad: MMD DEVAD
> >>> + * phy_read_mmd_indirect - Convenience function for reading a register
> >>> + * from an MMD on a given PHY.
> >>> + * @phydev: The phy_device struct
> >>> + * @devad: The MMD to read from
> >>> + * @regnum: The register on the MMD to read
> >>>   *
> >>>   * Description: it reads data from the MMD registers (clause 22 to access to
> >>>   * clause 45) of the specified phy address.
> >>> @@ -846,11 +847,11 @@ static inline void mmd_phy_indirect(struct phy_device *phydev, int prtad,
> >>>   * 3) Write reg 13 // MMD Data Command for MMD DEVAD
> >>>   * 3) Read  reg 14 // Read MMD data
> >>>   */
> >>> -int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
> >>> +int phy_read_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum)
> >>>  {
> >>>  	u32 ret;
> >>>  
> >>> -	mmd_phy_indirect(phydev, prtad, devad);
> >>> +	mmd_phy_indirect(phydev, devad, regnum);
> >>>  
> >>>  	/* Read the content of the MMD's selected register */
> >>>  	ret = phy_read(phydev, MII_MMD_DATA);
> >>> @@ -859,11 +860,12 @@ int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
> >>>  }
> >>>  
> >>>  /**
> >>> - * phy_write_mmd_indirect - writes data to the MMD registers
> >>> - * @phy_device: phy device
> >>> - * @prtad: MMD Address
> >>> - * @devad: MMD DEVAD
> >>> - * @data: data to write in the MMD register
> >>> + * phy_write_mmd_indirect - Convenience function for writing a register
> >>> + * on an MMD on a given PHY.
> >>> + * @phydev: The phy_device struct
> >>> + * @devad: The MMD to read from
> >>> + * @regnum: The register on the MMD to read
> >>> + * @val: value to write to @regnum
> >>>   *
> >>>   * Description: Write data from the MMD registers of the specified
> >>>   * phy address.
> >>> @@ -873,34 +875,33 @@ int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
> >>>   * 3) Write reg 13 // MMD Data Command for MMD DEVAD
> >>>   * 3) Write reg 14 // Write MMD data
> >>>   */
> >>> -void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> >>> -				   u16 data)
> >>> +void phy_write_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> >>> +			    u16 val)
> >>>  {
> >>> -	mmd_phy_indirect(phydev, prtad, devad);
> >>> +	mmd_phy_indirect(phydev, devad, regnum);
> >>>  
> >>>  	/* Write the data into MMD's selected register */
> >>> -	phy_write(phydev, MII_MMD_DATA, data);
> >>> +	phy_write(phydev, MII_MMD_DATA, val);
> >>>  }
> >>>  
> >>>  /**
> >>> - * phy_modify_mmd_indirect - Convenience function for modifying a MMD register
> >>> - * @phydev: phy device
> >>> - * @prtad: MMD Address
> >>> - * @devad: MMD DEVAD
> >>> + * phy_modify_mmd_indirect - Convenience function for modifying a register on MMD
> >>> + * @phydev: the phy_device struct
> >>> + * @devad: the MMD containing register to modify
> >>> + * @regnum: register number to modify
> >>>   * @mask: bit mask of bits to clear
> >>> - * @set: new value of bits set in @mask
> >>> - *
> >>> + * @set: new value of bits set in mask to write to @regnum
> >>>   */
> >>> -int phy_modify_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> >>> -				   u16 mask, u16 set)
> >>> +int phy_modify_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> >>> +			    u16 mask, u16 set)
> >>>  {
> >>>  	int ret;
> >>>  
> >>> -	ret = phy_read_mmd_indirect(phydev, prtad, devad);
> >>> +	ret = phy_read_mmd_indirect(phydev, devad, regnum);
> >>>  	if (ret < 0)
> >>>  		return ret;
> >>>  
> >>> -	phy_write_mmd_indirect(phydev, prtad, devad, (ret & ~mask) | set);
> >>> +	phy_write_mmd_indirect(phydev, devad, regnum, (ret & ~mask) | set);
> >>>  
> >>>  	return 0;
> >>>  }
> >>> diff --git a/include/linux/phy.h b/include/linux/phy.h
> >>> index 509bf72de9..d96c4e97ab 100644
> >>> --- a/include/linux/phy.h
> >>> +++ b/include/linux/phy.h
> >>> @@ -406,11 +406,11 @@ int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
> >>>  int phy_register_fixup_for_id(const char *bus_id,
> >>>  		int (*run)(struct phy_device *));
> >>>  int phy_scan_fixups(struct phy_device *phydev);
> >>> -int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad);
> >>> -void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> >>> -				   u16 data);
> >>> -int phy_modify_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
> >>> -				    u16 mask, u16 set);
> >>> +int phy_read_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum);
> >>> +void phy_write_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> >>> +			    u16 val);
> >>> +int phy_modify_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
> >>> +			    u16 mask, u16 set);
> >>>  
> >>>  static inline bool phy_acquired(struct phy_device *phydev)
> >>>  {
> >>
> >> -- 
> >> Pengutronix e.K.                           |                             |
> >> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> >> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> >> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> >>
> >>
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 

Re: [PATCH 3/7] net: phy: sync phy_{write,read,modify}_mmd_indirect with linux

[Ahmad Fatoum]

On 10.07.23 13:01, Marco Felsch wrote:
> On 23-07-10, Ahmad Fatoum wrote:
>> Hello Marco,
>>
>> On 10.07.23 12:49, Marco Felsch wrote:
>>> On 23-07-10, Ahmad Fatoum wrote:
>>>> On 10.07.23 08:36, Marco Felsch wrote:
>>>>> Sync the barebox phy_{write,read,modify}_mmd_indirect API with the Linux
>>>>> phy_{write,read,modify}_mmd API to make it easier and less error prone
>>>>> to port phy drivers. This also fixes the r8169 driver since this driver
>>>>> did not adapt the parameters while porting from Linux.
>>>>>
>>>>> The sync also aligns the function parameter `prtad` as well as the
>>>>> function documentation.
>>>>
>>>> This breaks out of tree board code in a subtle and annoying manner.
>>>
>>> Do we really need to care about out all-of-tree drivers/boards? If so,
>>> we need to ensure to not break any public function.
>>
>> Out-of-tree drivers not too much probably, but out-of-tree boards
>> are unfortunately the norm. Breaking the build can be justified,
>> but silent breaking by swapping arguments is not nice.
> 
> And how do you ensure that a public API is only used by drivers? Don't
> get me wrong I got your point but if you update an out-of-tree board to
> a newer barebox version it may happen that something break. Therefore
> manufacturers should upstream there code to reduce the maintenance
> effort to a minimum :) I know we don't live in an ideal world :/

Some breakage is ultimately unavoidable, but we can try to not needlessly
introduce silent breakage. Here we have a nice way out: We import the
Linux function with the Linux name and use that for upstream code. The old
function is marked deprecated and removed later on. Do you see an issue
with that?

Cheers,
Ahmad

> 
> Regards,
>   Marco
> 
>>>> We are in luck that the kernel function has a different name than
>>>> the barebox function, so I'd rather suggest:
>>>>
>>>>  - Add phy_read_mmd/phy_write_mmd functions with same argument
>>>>    order as Linux
>>>>  - Switch all users in barebox to the new functions
>>>>  - Mark the old _indirect function deprecated with a suggestion
>>>>    to use phy_read_mmd/phy_write_mmd and swap address arguments.
>>>
>>> I was think about such approach, then I checked that the Linux API does
>>> handle C45 as well, so we would need least at least $some minimal
>>> support like:
>>>
>>> if (c45)
>>> 	pr_err("C45 phys are not supported yet\n");
>>> 	return -EINVAL;
>>
>> Sounds good IMO. Maybe return -ENOSYS instead.
>>
>>>
>>> and therefore I stuck with the _indirect functions.
>>>
>>> Regards,
>>>   Marco
>>>
>>>>
>>>>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>>>>> ---
>>>>>  arch/arm/boards/datamodul-edm-qmx6/board.c   |  6 +--
>>>>>  arch/arm/boards/embest-marsboard/board.c     |  6 +--
>>>>>  arch/arm/boards/terasic-de0-nano-soc/board.c |  6 +--
>>>>>  arch/arm/boards/terasic-de10-nano/board.c    |  6 +--
>>>>>  arch/arm/boards/tqma6x/board.c               |  6 +--
>>>>>  drivers/net/phy/at803x.c                     |  4 +-
>>>>>  drivers/net/phy/dp83867.c                    | 40 +++++++-------
>>>>>  drivers/net/phy/micrel.c                     | 24 ++++-----
>>>>>  drivers/net/phy/phy.c                        | 57 ++++++++++----------
>>>>>  include/linux/phy.h                          | 10 ++--
>>>>>  10 files changed, 83 insertions(+), 82 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/boards/datamodul-edm-qmx6/board.c b/arch/arm/boards/datamodul-edm-qmx6/board.c
>>>>> index 366b64d35a..0dc71f2aca 100644
>>>>> --- a/arch/arm/boards/datamodul-edm-qmx6/board.c
>>>>> +++ b/arch/arm/boards/datamodul-edm-qmx6/board.c
>>>>> @@ -49,9 +49,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
>>>>>  	 * min rx data delay, max rx/tx clock delay,
>>>>>  	 * min rx/tx control delay
>>>>>  	 */
>>>>> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
>>>>> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
>>>>> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x03ff);
>>>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
>>>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
>>>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x03ff);
>>>>>  
>>>>>  	return 0;
>>>>>  }
>>>>> diff --git a/arch/arm/boards/embest-marsboard/board.c b/arch/arm/boards/embest-marsboard/board.c
>>>>> index 7274595e2a..e70fefec5e 100644
>>>>> --- a/arch/arm/boards/embest-marsboard/board.c
>>>>> +++ b/arch/arm/boards/embest-marsboard/board.c
>>>>> @@ -20,13 +20,13 @@ static int ar8035_phy_fixup(struct phy_device *dev)
>>>>>  	/* Ar803x phy SmartEEE feature cause link status generates glitch,
>>>>>  	 * which cause ethernet link down/up issue, so disable SmartEEE
>>>>>  	 */
>>>>> -	val = phy_read_mmd_indirect(dev, 0x805d, MDIO_MMD_PCS);
>>>>> +	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x805d);
>>>>>  	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
>>>>>  
>>>>> -	val = phy_read_mmd_indirect(dev, 0x4003, MDIO_MMD_PCS);
>>>>> +	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x4003);
>>>>>  	phy_write(dev, MII_MMD_DATA, val & ~(1 << 8));
>>>>>  
>>>>> -	val = phy_read_mmd_indirect(dev, 0x4007, MDIO_MMD_PCS);
>>>>> +	val = phy_read_mmd_indirect(dev, MDIO_MMD_PCS, 0x4007);
>>>>>  	val &= 0xffe3;
>>>>>  	val |= 0x18;
>>>>>  	phy_write(dev, MII_MMD_DATA, val);
>>>>> diff --git a/arch/arm/boards/terasic-de0-nano-soc/board.c b/arch/arm/boards/terasic-de0-nano-soc/board.c
>>>>> index 832160c595..fa83498f46 100644
>>>>> --- a/arch/arm/boards/terasic-de0-nano-soc/board.c
>>>>> +++ b/arch/arm/boards/terasic-de0-nano-soc/board.c
>>>>> @@ -19,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
>>>>>  	 * min rx data delay, max rx/tx clock delay,
>>>>>  	 * min rx/tx control delay
>>>>>  	 */
>>>>> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
>>>>> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
>>>>> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
>>>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
>>>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
>>>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
>>>>>  	return 0;
>>>>>  }
>>>>>  
>>>>> diff --git a/arch/arm/boards/terasic-de10-nano/board.c b/arch/arm/boards/terasic-de10-nano/board.c
>>>>> index e47d9ac841..7ceb691f71 100644
>>>>> --- a/arch/arm/boards/terasic-de10-nano/board.c
>>>>> +++ b/arch/arm/boards/terasic-de10-nano/board.c
>>>>> @@ -19,9 +19,9 @@ static int phy_fixup(struct phy_device *dev)
>>>>>  	 * min rx data delay, max rx/tx clock delay,
>>>>>  	 * min rx/tx control delay
>>>>>  	 */
>>>>> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
>>>>> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
>>>>> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
>>>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
>>>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
>>>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
>>>>>  	return 0;
>>>>>  }
>>>>>  
>>>>> diff --git a/arch/arm/boards/tqma6x/board.c b/arch/arm/boards/tqma6x/board.c
>>>>> index 8a91ad652a..3fd2e1db2c 100644
>>>>> --- a/arch/arm/boards/tqma6x/board.c
>>>>> +++ b/arch/arm/boards/tqma6x/board.c
>>>>> @@ -47,9 +47,9 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
>>>>>  	 * min rx data delay, max rx/tx clock delay,
>>>>>  	 * min rx/tx control delay
>>>>>  	 */
>>>>> -	phy_write_mmd_indirect(dev, 4, MDIO_MMD_WIS, 0);
>>>>> -	phy_write_mmd_indirect(dev, 5, MDIO_MMD_WIS, 0);
>>>>> -	phy_write_mmd_indirect(dev, 8, MDIO_MMD_WIS, 0x003ff);
>>>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 4, 0);
>>>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 5, 0);
>>>>> +	phy_write_mmd_indirect(dev, MDIO_MMD_WIS, 8, 0x003ff);
>>>>>  
>>>>>  	return 0;
>>>>>  }
>>>>> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
>>>>> index 18182bffc2..41010c58ad 100644
>>>>> --- a/drivers/net/phy/at803x.c
>>>>> +++ b/drivers/net/phy/at803x.c
>>>>> @@ -229,14 +229,14 @@ static int at803x_clk_out_config(struct phy_device *phydev)
>>>>>  	if (!priv->clk_25m_mask)
>>>>>  		return 0;
>>>>>  
>>>>> -	val = phy_read_mmd_indirect(phydev, AT803X_MMD7_CLK25M, MDIO_MMD_AN);
>>>>> +	val = phy_read_mmd_indirect(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M);
>>>>>  	if (val < 0)
>>>>>  		return val;
>>>>>  
>>>>>  	val &= ~priv->clk_25m_mask;
>>>>>  	val |= priv->clk_25m_reg;
>>>>>  
>>>>> -	phy_write_mmd_indirect(phydev, AT803X_MMD7_CLK25M, MDIO_MMD_AN, val);
>>>>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M, val);
>>>>>  
>>>>>  	return 0;
>>>>>  }
>>>>> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
>>>>> index d8109172df..85fe5107da 100644
>>>>> --- a/drivers/net/phy/dp83867.c
>>>>> +++ b/drivers/net/phy/dp83867.c
>>>>> @@ -154,14 +154,14 @@ static int dp83867_config_port_mirroring(struct phy_device *phydev)
>>>>>  	struct dp83867_private *dp83867 = phydev->priv;
>>>>>  	u16 val;
>>>>>  
>>>>> -	val = phy_read_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR);
>>>>> +	val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR, DP83867_CFG4);
>>>>>  
>>>>>  	if (dp83867->port_mirroring == DP83867_PORT_MIRROING_EN)
>>>>>  		val |= DP83867_CFG4_PORT_MIRROR_EN;
>>>>>  	else
>>>>>  		val &= ~DP83867_CFG4_PORT_MIRROR_EN;
>>>>>  
>>>>> -	phy_write_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR, val);
>>>>> +	phy_write_mmd_indirect(phydev, DP83867_DEVADDR, DP83867_CFG4, val);
>>>>>  
>>>>>  	return 0;
>>>>>  }
>>>>> @@ -256,11 +256,11 @@ static int dp83867_config_init(struct phy_device *phydev)
>>>>>  	phy_write(phydev, DP83867_CTRL, val | DP83867_SW_RESTART);
>>>>>  
>>>>>  	if (dp83867->rxctrl_strap_quirk) {
>>>>> -		val = phy_read_mmd_indirect(phydev, DP83867_CFG4,
>>>>> -					    DP83867_DEVADDR);
>>>>> +		val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
>>>>> +					    DP83867_CFG4);
>>>>>  		val &= ~BIT(7);
>>>>> -		phy_write_mmd_indirect(phydev, DP83867_CFG4,
>>>>> -				       DP83867_DEVADDR, val);
>>>>> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
>>>>> +				       DP83867_CFG4, val);
>>>>>  	}
>>>>>  
>>>>>  	if (phy_interface_is_rgmii(phydev)) {
>>>>> @@ -270,8 +270,8 @@ static int dp83867_config_init(struct phy_device *phydev)
>>>>>  		if (ret)
>>>>>  			return ret;
>>>>>  
>>>>> -		val = phy_read_mmd_indirect(phydev, DP83867_RGMIICTL,
>>>>> -					    DP83867_DEVADDR);
>>>>> +		val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
>>>>> +					    DP83867_RGMIICTL);
>>>>>  
>>>>>  		switch (phydev->interface) {
>>>>>  		case PHY_INTERFACE_MODE_RGMII_ID:
>>>>> @@ -287,31 +287,31 @@ static int dp83867_config_init(struct phy_device *phydev)
>>>>>  		default:
>>>>>  			break;
>>>>>  		}
>>>>> -		phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
>>>>> -				       DP83867_DEVADDR, val);
>>>>> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
>>>>> +				       DP83867_RGMIICTL, val);
>>>>>  
>>>>>  		delay = (dp83867->rx_id_delay |
>>>>>  			(dp83867->tx_id_delay << DP83867_RGMII_TX_CLK_DELAY_SHIFT));
>>>>>  
>>>>> -		phy_write_mmd_indirect(phydev, DP83867_RGMIIDCTL,
>>>>> -				       DP83867_DEVADDR, delay);
>>>>> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
>>>>> +				       DP83867_RGMIIDCTL, delay);
>>>>>  
>>>>>  		if (dp83867->io_impedance >= 0) {
>>>>> -			val = phy_read_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
>>>>> -						    DP83867_DEVADDR);
>>>>> +			val = phy_read_mmd_indirect(phydev, DP83867_DEVADDR,
>>>>> +						    DP83867_IO_MUX_CFG);
>>>>>  			val &= ~DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
>>>>>  			val |= (dp83867->io_impedance &
>>>>>  				DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL);
>>>>>  
>>>>> -			phy_write_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
>>>>> -					       DP83867_DEVADDR, val);
>>>>> +			phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
>>>>> +					       DP83867_IO_MUX_CFG, val);
>>>>>  		}
>>>>>  	} else if (phy_interface_is_sgmii(phydev)) {
>>>>>  		phy_write(phydev, MII_BMCR,
>>>>>  			  BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000);
>>>>>  
>>>>> -		phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
>>>>> -				       DP83867_DEVADDR, 0x0);
>>>>> +		phy_write_mmd_indirect(phydev, DP83867_DEVADDR,
>>>>> +				       DP83867_RGMIICTL, 0x0);
>>>>>  
>>>>>  		val = DP83867_PHYCTRL_SGMIIEN |
>>>>>  		      DP83867_MDI_CROSSOVER_MDIX << DP83867_MDI_CROSSOVER |
>>>>> @@ -341,8 +341,8 @@ static int dp83867_config_init(struct phy_device *phydev)
>>>>>  				DP83867_IO_MUX_CFG_CLK_O_SEL_SHIFT;
>>>>>  		}
>>>>>  
>>>>> -		phy_modify_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
>>>>> -				DP83867_DEVADDR, mask, val);
>>>>> +		phy_modify_mmd_indirect(phydev, DP83867_DEVADDR,
>>>>> +					DP83867_IO_MUX_CFG, mask, val);
>>>>>  	}
>>>>>  
>>>>>  	return 0;
>>>>> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
>>>>> index 02d474c442..892df01d2e 100644
>>>>> --- a/drivers/net/phy/micrel.c
>>>>> +++ b/drivers/net/phy/micrel.c
>>>>> @@ -387,7 +387,7 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
>>>>>  		return 0;
>>>>>  
>>>>>  	if (matches < numfields)
>>>>> -		newval = phy_read_mmd_indirect(phydev, reg, MDIO_MMD_WIS);
>>>>> +		newval = phy_read_mmd_indirect(phydev, MDIO_MMD_WIS, reg);
>>>>>  	else
>>>>>  		newval = 0;
>>>>>  
>>>>> @@ -401,15 +401,15 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
>>>>>  				<< (field_sz * i));
>>>>>  		}
>>>>>  
>>>>> -	phy_write_mmd_indirect(phydev, reg, MDIO_MMD_WIS, newval);
>>>>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS, reg, newval);
>>>>>  	return 0;
>>>>>  }
>>>>>  
>>>>>  static int ksz9031_center_flp_timing(struct phy_device *phydev)
>>>>>  {
>>>>>  	/* Center KSZ9031RNX FLP timing at 16ms. */
>>>>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_HI, 0, 0x0006);
>>>>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_LO, 0, 0x1a80);
>>>>> +	phy_write_mmd_indirect(phydev, 0, MII_KSZ9031RN_FLP_BURST_TX_HI, 0x0006);
>>>>> +	phy_write_mmd_indirect(phydev, 0, MII_KSZ9031RN_FLP_BURST_TX_LO, 0x1a80);
>>>>>  
>>>>>  	return genphy_restart_aneg(phydev);
>>>>>  }
>>>>> @@ -447,27 +447,27 @@ static int ksz9031_config_rgmii_delay(struct phy_device *phydev)
>>>>>  		return 0;
>>>>>  	}
>>>>>  
>>>>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CONTROL_PAD_SKEW,
>>>>> -			       MDIO_MMD_WIS,
>>>>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
>>>>> +			       MII_KSZ9031RN_CONTROL_PAD_SKEW,
>>>>>  			       FIELD_PREP(MII_KSZ9031RN_RX_CTL_M, rx) |
>>>>>  			       FIELD_PREP(MII_KSZ9031RN_TX_CTL_M, tx));
>>>>>  
>>>>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_RX_DATA_PAD_SKEW,
>>>>> -			       MDIO_MMD_WIS,
>>>>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
>>>>> +			       MII_KSZ9031RN_RX_DATA_PAD_SKEW,
>>>>>  			       FIELD_PREP(MII_KSZ9031RN_RXD3, rx) |
>>>>>  			       FIELD_PREP(MII_KSZ9031RN_RXD2, rx) |
>>>>>  			       FIELD_PREP(MII_KSZ9031RN_RXD1, rx) |
>>>>>  			       FIELD_PREP(MII_KSZ9031RN_RXD0, rx));
>>>>>  
>>>>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_TX_DATA_PAD_SKEW,
>>>>> -			       MDIO_MMD_WIS,
>>>>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
>>>>> +			       MII_KSZ9031RN_TX_DATA_PAD_SKEW,
>>>>>  			       FIELD_PREP(MII_KSZ9031RN_TXD3, tx) |
>>>>>  			       FIELD_PREP(MII_KSZ9031RN_TXD2, tx) |
>>>>>  			       FIELD_PREP(MII_KSZ9031RN_TXD1, tx) |
>>>>>  			       FIELD_PREP(MII_KSZ9031RN_TXD0, tx));
>>>>>  
>>>>> -	phy_write_mmd_indirect(phydev, MII_KSZ9031RN_CLK_PAD_SKEW,
>>>>> -			       MDIO_MMD_WIS,
>>>>> +	phy_write_mmd_indirect(phydev, MDIO_MMD_WIS,
>>>>> +			       MII_KSZ9031RN_CLK_PAD_SKEW,
>>>>>  			       FIELD_PREP(MII_KSZ9031RN_GTX_CLK, tx_clk) |
>>>>>  			       FIELD_PREP(MII_KSZ9031RN_RX_CLK, rx_clk));
>>>>>  	return 0;
>>>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>>>> index 74381949b4..283e1a3d79 100644
>>>>> --- a/drivers/net/phy/phy.c
>>>>> +++ b/drivers/net/phy/phy.c
>>>>> @@ -819,24 +819,25 @@ int genphy_read_status(struct phy_device *phydev)
>>>>>  	return 0;
>>>>>  }
>>>>>  
>>>>> -static inline void mmd_phy_indirect(struct phy_device *phydev, int prtad,
>>>>> -					int devad)
>>>>> +static inline void mmd_phy_indirect(struct phy_device *phydev, int devad,
>>>>> +				    u16 regnum)
>>>>>  {
>>>>>  	/* Write the desired MMD Devad */
>>>>>  	phy_write(phydev, MII_MMD_CTRL, devad);
>>>>>  
>>>>>  	/* Write the desired MMD register address */
>>>>> -	phy_write(phydev, MII_MMD_DATA, prtad);
>>>>> +	phy_write(phydev, MII_MMD_DATA, regnum);
>>>>>  
>>>>>  	/* Select the Function : DATA with no post increment */
>>>>>  	phy_write(phydev, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
>>>>>  }
>>>>>  
>>>>>  /**
>>>>> - * phy_read_mmd_indirect - reads data from the MMD registers
>>>>> - * @phy_device: phy device
>>>>> - * @prtad: MMD Address
>>>>> - * @devad: MMD DEVAD
>>>>> + * phy_read_mmd_indirect - Convenience function for reading a register
>>>>> + * from an MMD on a given PHY.
>>>>> + * @phydev: The phy_device struct
>>>>> + * @devad: The MMD to read from
>>>>> + * @regnum: The register on the MMD to read
>>>>>   *
>>>>>   * Description: it reads data from the MMD registers (clause 22 to access to
>>>>>   * clause 45) of the specified phy address.
>>>>> @@ -846,11 +847,11 @@ static inline void mmd_phy_indirect(struct phy_device *phydev, int prtad,
>>>>>   * 3) Write reg 13 // MMD Data Command for MMD DEVAD
>>>>>   * 3) Read  reg 14 // Read MMD data
>>>>>   */
>>>>> -int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
>>>>> +int phy_read_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum)
>>>>>  {
>>>>>  	u32 ret;
>>>>>  
>>>>> -	mmd_phy_indirect(phydev, prtad, devad);
>>>>> +	mmd_phy_indirect(phydev, devad, regnum);
>>>>>  
>>>>>  	/* Read the content of the MMD's selected register */
>>>>>  	ret = phy_read(phydev, MII_MMD_DATA);
>>>>> @@ -859,11 +860,12 @@ int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
>>>>>  }
>>>>>  
>>>>>  /**
>>>>> - * phy_write_mmd_indirect - writes data to the MMD registers
>>>>> - * @phy_device: phy device
>>>>> - * @prtad: MMD Address
>>>>> - * @devad: MMD DEVAD
>>>>> - * @data: data to write in the MMD register
>>>>> + * phy_write_mmd_indirect - Convenience function for writing a register
>>>>> + * on an MMD on a given PHY.
>>>>> + * @phydev: The phy_device struct
>>>>> + * @devad: The MMD to read from
>>>>> + * @regnum: The register on the MMD to read
>>>>> + * @val: value to write to @regnum
>>>>>   *
>>>>>   * Description: Write data from the MMD registers of the specified
>>>>>   * phy address.
>>>>> @@ -873,34 +875,33 @@ int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
>>>>>   * 3) Write reg 13 // MMD Data Command for MMD DEVAD
>>>>>   * 3) Write reg 14 // Write MMD data
>>>>>   */
>>>>> -void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
>>>>> -				   u16 data)
>>>>> +void phy_write_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
>>>>> +			    u16 val)
>>>>>  {
>>>>> -	mmd_phy_indirect(phydev, prtad, devad);
>>>>> +	mmd_phy_indirect(phydev, devad, regnum);
>>>>>  
>>>>>  	/* Write the data into MMD's selected register */
>>>>> -	phy_write(phydev, MII_MMD_DATA, data);
>>>>> +	phy_write(phydev, MII_MMD_DATA, val);
>>>>>  }
>>>>>  
>>>>>  /**
>>>>> - * phy_modify_mmd_indirect - Convenience function for modifying a MMD register
>>>>> - * @phydev: phy device
>>>>> - * @prtad: MMD Address
>>>>> - * @devad: MMD DEVAD
>>>>> + * phy_modify_mmd_indirect - Convenience function for modifying a register on MMD
>>>>> + * @phydev: the phy_device struct
>>>>> + * @devad: the MMD containing register to modify
>>>>> + * @regnum: register number to modify
>>>>>   * @mask: bit mask of bits to clear
>>>>> - * @set: new value of bits set in @mask
>>>>> - *
>>>>> + * @set: new value of bits set in mask to write to @regnum
>>>>>   */
>>>>> -int phy_modify_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
>>>>> -				   u16 mask, u16 set)
>>>>> +int phy_modify_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
>>>>> +			    u16 mask, u16 set)
>>>>>  {
>>>>>  	int ret;
>>>>>  
>>>>> -	ret = phy_read_mmd_indirect(phydev, prtad, devad);
>>>>> +	ret = phy_read_mmd_indirect(phydev, devad, regnum);
>>>>>  	if (ret < 0)
>>>>>  		return ret;
>>>>>  
>>>>> -	phy_write_mmd_indirect(phydev, prtad, devad, (ret & ~mask) | set);
>>>>> +	phy_write_mmd_indirect(phydev, devad, regnum, (ret & ~mask) | set);
>>>>>  
>>>>>  	return 0;
>>>>>  }
>>>>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>>>>> index 509bf72de9..d96c4e97ab 100644
>>>>> --- a/include/linux/phy.h
>>>>> +++ b/include/linux/phy.h
>>>>> @@ -406,11 +406,11 @@ int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
>>>>>  int phy_register_fixup_for_id(const char *bus_id,
>>>>>  		int (*run)(struct phy_device *));
>>>>>  int phy_scan_fixups(struct phy_device *phydev);
>>>>> -int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad);
>>>>> -void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
>>>>> -				   u16 data);
>>>>> -int phy_modify_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
>>>>> -				    u16 mask, u16 set);
>>>>> +int phy_read_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum);
>>>>> +void phy_write_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
>>>>> +			    u16 val);
>>>>> +int phy_modify_mmd_indirect(struct phy_device *phydev, int devad, u32 regnum,
>>>>> +			    u16 mask, u16 set);
>>>>>  
>>>>>  static inline bool phy_acquired(struct phy_device *phydev)
>>>>>  {
>>>>
>>>> -- 
>>>> Pengutronix e.K.                           |                             |
>>>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>>>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>>>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>>>
>>>>
>>>
>>
>> -- 
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 6/7] net: phy: at803x: add disable hibernation mode support

[Ahmad Fatoum]

On 10.07.23 12:53, Marco Felsch wrote:
> On 23-07-10, Ahmad Fatoum wrote:
>> On 10.07.23 08:36, Marco Felsch wrote:
>>> This commit ports Linux commit:
>>>
>>> | commit 9ecf04016c87bcb33b44e24489d33618e2592f41
>>> | Author: Wei Fang <wei.fang@nxp.com>
>>> | Date:   Thu Aug 18 11:00:54 2022 +0800
>>> |
>>> |     net: phy: at803x: add disable hibernation mode support
>>> |
>>> |     When the cable is unplugged, the Atheros AR803x PHYs will enter
>>> |     hibernation mode after about 10 seconds if the hibernation mode
>>> |     is enabled and will not provide any clock to the MAC. But for
>>> |     some MACs, this feature might cause unexpected issues due to the
>>> |     logic of MACs.
>>> |     Taking SYNP MAC (stmmac) as an example, if the cable is unplugged
>>> |     and the "eth0" interface is down, the AR803x PHY will enter
>>> |     hibernation mode. Then perform the "ifconfig eth0 up" operation,
>>> |     the stmmac can't be able to complete the software reset operation
>>> |     and fail to init it's own DMA. Therefore, the "eth0" interface is
>>> |     failed to ifconfig up. Why does it cause this issue? The truth is
>>> |     that the software reset operation of the stmmac is designed to
>>> |     depend on the RX_CLK of PHY.
>>> |     So, this patch offers an option for the user to determine whether
>>> |     to disable the hibernation mode of AR803x PHYs.
>>> |
>>> |     Signed-off-by: Wei Fang <wei.fang@nxp.com>
>>> |     Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>>> |     Signed-off-by: Jakub Kicinski <kuba@kernel.org>
>>>
>>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>>
>> IMO we should just makes AT803X_DISABLE_HIBERNATION_MODE the default and
>> never hibernate the PHY. Let Linux worry about that.
> 
> Got your point and I'm not sure. At least for drivers I like the sources
> to bin in sync with Linux which allows porting fixes/features more
> easily.

You can keep code mostly as-is and just replace the DT property check
with a comment why we do this unconditionally.

> Also since the API (dt-bindings) are there, so why not just
> copy'n'paste from Linux?

We don't care much for power saving during barebox runtime. We turn on
whatever clocks are necessary and leave it to Linux to power down
unneeded peripherals. Same thing should apply to the PHY. If the PHY
has a power saving mode that breaks some MACs, why not just disable
the power saving mode? It's better for code size anyway.

Cheers,
Ahmad

> 
> Regards,
>   Marco
> 
>>> ---
>>>  drivers/net/phy/at803x.c | 24 ++++++++++++++++++++++++
>>>  1 file changed, 24 insertions(+)
>>>
>>> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
>>> index cc425c318f..d8a9c3588f 100644
>>> --- a/drivers/net/phy/at803x.c
>>> +++ b/drivers/net/phy/at803x.c
>>> @@ -32,6 +32,9 @@
>>>  #define AT803X_DEBUG_REG_5			0x05
>>>  #define AT803X_DEBUG_TX_CLK_DLY_EN		BIT(8)
>>>  
>>> +#define AT803X_DEBUG_REG_HIB_CTRL		0x0b
>>> +#define   AT803X_DEBUG_HIB_CTRL_PS_HIB_EN	BIT(15)
>>> +
>>>  /* AT803x supports either the XTAL input pad, an internal PLL or the
>>>   * DSP as clock reference for the clock output pad. The XTAL reference
>>>   * is only used for 25 MHz output, all other frequencies need the PLL.
>>> @@ -140,6 +143,20 @@ static int at803x_disable_tx_delay(struct phy_device *phydev)
>>>  				     AT803X_DEBUG_TX_CLK_DLY_EN, 0);
>>>  }
>>>  
>>> +static int at803x_hibernation_mode_config(struct phy_device *phydev)
>>> +{
>>> +	struct at803x_priv *priv = phydev->priv;
>>> +
>>> +	/* The default after hardware reset is hibernation mode enabled. After
>>> +	 * software reset, the value is retained.
>>> +	 */
>>> +	if (!(priv->flags & AT803X_DISABLE_HIBERNATION_MODE))
>>> +		return 0;
>>> +
>>> +	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_HIB_CTRL,
>>> +				     AT803X_DEBUG_HIB_CTRL_PS_HIB_EN, 0);
>>> +}
>>> +
>>>  static bool at803x_match_phy_id(struct phy_device *phydev, u32 phy_id)
>>>  {
>>>  	struct phy_driver *drv = to_phy_driver(phydev->dev.driver);
>>> @@ -160,6 +177,9 @@ static int at803x_parse_dt(struct phy_device *phydev)
>>>  	if (of_property_read_bool(node, "qca,disable-smarteee"))
>>>  		priv->flags |= AT803X_DISABLE_SMARTEEE;
>>>  
>>> +	if (of_property_read_bool(node, "qca,disable-hibernation-mode"))
>>> +		priv->flags |= AT803X_DISABLE_HIBERNATION_MODE;
>>> +
>>>  	if (!of_property_read_u32(node, "qca,smarteee-tw-us-1g", &tw)) {
>>>  		if (!tw || tw > 255) {
>>>  			phydev_err(phydev, "invalid qca,smarteee-tw-us-1g\n");
>>> @@ -341,6 +361,10 @@ static int at803x_config_init(struct phy_device *phydev)
>>>  	if (ret < 0)
>>>  		return ret;
>>>  
>>> +	ret = at803x_hibernation_mode_config(phydev);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>>  	return 0;
>>>  }
>>>  
>>
>> -- 
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 5/7] net: phy: at803x: add support for configuring SmartEEE

[Oleksij Rempel]

Am 10.07.23 um 12:10 schrieb Ahmad Fatoum:
> On 10.07.23 08:36, Marco Felsch wrote:
>> This commit port Linux commit:
>>
>> | commit 390b4cad81484124db2b676ed20a265adc032bae
>> | Author: Russell King <rmk+kernel@armlinux.org.uk>
>> | Date:   Thu Jan 14 10:45:49 2021 +0000
>> |
>> |     net: phy: at803x: add support for configuring SmartEEE
>> |
>> |     SmartEEE for the atheros phy was deemed buggy by Freescale and commits
>> |     were added to disable it for their boards.
>> |
>> |     In initial testing, SolidRun found that the default settings were
>> |     causing disconnects but by increasing the Tw buffer time we could allow
>> |     enough time for all parts of the link to come out of a low power state
>> |     and function properly without causing a disconnect. This allows us to
>> |     have functional power savings of between 300 and 400mW, rather than
>> |     disabling the feature altogether.
>> |
>> |     This commit adds support for disabling SmartEEE and configuring the Tw
>> |     parameters for 1G and 100M speeds.
>> |
>> |     Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>> |     Signed-off-by: Jakub Kicinski <kuba@kernel.org>
>>
>> I slightly adapted the at803x_config_init() as well to be more in line
>> with the Linux code base.
>>
>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>
> Same question as in follow-up commit: Can't we just disable it unconditionally?

Most of PHYs with EEE support have EEE advertisement register on the same offset. I would recommend
to create a common function for all PHYs drivers to clear all EEE advertising bits and call it the
PHY framework. EEE should be never advertised by barebox. As a side effect, if Atheros PHY do not
advertise EEE, SmartEEE will not be activated.

--
Regards,
Oleksij

Re: [PATCH 3/7] net: phy: sync phy_{write,read,modify}_mmd_indirect with linux

[Marco Felsch]

On 23-07-10, Ahmad Fatoum wrote:
> On 10.07.23 13:01, Marco Felsch wrote:
> > On 23-07-10, Ahmad Fatoum wrote:
> >> Hello Marco,
> >>
> >> On 10.07.23 12:49, Marco Felsch wrote:
> >>> On 23-07-10, Ahmad Fatoum wrote:
> >>>> On 10.07.23 08:36, Marco Felsch wrote:
> >>>>> Sync the barebox phy_{write,read,modify}_mmd_indirect API with the Linux
> >>>>> phy_{write,read,modify}_mmd API to make it easier and less error prone
> >>>>> to port phy drivers. This also fixes the r8169 driver since this driver
> >>>>> did not adapt the parameters while porting from Linux.
> >>>>>
> >>>>> The sync also aligns the function parameter `prtad` as well as the
> >>>>> function documentation.
> >>>>
> >>>> This breaks out of tree board code in a subtle and annoying manner.
> >>>
> >>> Do we really need to care about out all-of-tree drivers/boards? If so,
> >>> we need to ensure to not break any public function.
> >>
> >> Out-of-tree drivers not too much probably, but out-of-tree boards
> >> are unfortunately the norm. Breaking the build can be justified,
> >> but silent breaking by swapping arguments is not nice.
> > 
> > And how do you ensure that a public API is only used by drivers? Don't
> > get me wrong I got your point but if you update an out-of-tree board to
> > a newer barebox version it may happen that something break. Therefore
> > manufacturers should upstream there code to reduce the maintenance
> > effort to a minimum :) I know we don't live in an ideal world :/
> 
> Some breakage is ultimately unavoidable, but we can try to not needlessly
> introduce silent breakage. Here we have a nice way out: We import the
> Linux function with the Linux name and use that for upstream code. The old
> function is marked deprecated and removed later on. Do you see an issue
> with that?

The only problem I see is, that the Linux API does support C45 and C22
and we don't. As said, we can implement it and make the C45 support a
stub. From user perspective I would assume that the new API is working
for both classes (like Linux). Therefore I kept the _indirect suffix
since this makes it clear from API pov.

Regards,
  Marco

Re: [PATCH 3/7] net: phy: sync phy_{write,read,modify}_mmd_indirect with linux

[Ahmad Fatoum]

On 10.07.23 14:43, Marco Felsch wrote:
> On 23-07-10, Ahmad Fatoum wrote:
>> On 10.07.23 13:01, Marco Felsch wrote:
>>> On 23-07-10, Ahmad Fatoum wrote:
>>>> Hello Marco,
>>>>
>>>> On 10.07.23 12:49, Marco Felsch wrote:
>>>>> On 23-07-10, Ahmad Fatoum wrote:
>>>>>> On 10.07.23 08:36, Marco Felsch wrote:
>>>>>>> Sync the barebox phy_{write,read,modify}_mmd_indirect API with the Linux
>>>>>>> phy_{write,read,modify}_mmd API to make it easier and less error prone
>>>>>>> to port phy drivers. This also fixes the r8169 driver since this driver
>>>>>>> did not adapt the parameters while porting from Linux.
>>>>>>>
>>>>>>> The sync also aligns the function parameter `prtad` as well as the
>>>>>>> function documentation.
>>>>>>
>>>>>> This breaks out of tree board code in a subtle and annoying manner.
>>>>>
>>>>> Do we really need to care about out all-of-tree drivers/boards? If so,
>>>>> we need to ensure to not break any public function.
>>>>
>>>> Out-of-tree drivers not too much probably, but out-of-tree boards
>>>> are unfortunately the norm. Breaking the build can be justified,
>>>> but silent breaking by swapping arguments is not nice.
>>>
>>> And how do you ensure that a public API is only used by drivers? Don't
>>> get me wrong I got your point but if you update an out-of-tree board to
>>> a newer barebox version it may happen that something break. Therefore
>>> manufacturers should upstream there code to reduce the maintenance
>>> effort to a minimum :) I know we don't live in an ideal world :/
>>
>> Some breakage is ultimately unavoidable, but we can try to not needlessly
>> introduce silent breakage. Here we have a nice way out: We import the
>> Linux function with the Linux name and use that for upstream code. The old
>> function is marked deprecated and removed later on. Do you see an issue
>> with that?
> 
> The only problem I see is, that the Linux API does support C45 and C22
> and we don't. As said, we can implement it and make the C45 support a
> stub. From user perspective I would assume that the new API is working
> for both classes (like Linux). Therefore I kept the _indirect suffix
> since this makes it clear from API pov.

Error message on C45 sounds good to me. Swapping parameters without
function rename doesn't.

Cheers,
Ahmad

> 
> Regards,
>   Marco
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |