[PATCH net-next v4 1/3] dt-bindings: net: document Microchip PIC64-HPSC/HX MDIO controller

[PATCH net-next v4 1/3] dt-bindings: net: document Microchip PIC64-HPSC/HX MDIO controller

[Charles Perry]

This MDIO hardware is based on a Microsemi design supported in Linux by
mdio-mscc-miim.c. However, The register interface is completely different
with pic64hpsc, hence the need for separate documentation.

The hardware supports C22 and C45.

The documentation recommends an input clock of 156.25MHz and a prescaler
of 39, which yields an MDIO clock of 1.95MHz.

The hardware supports an interrupt pin to signal transaction completion
which is not strictly needed as the software can also poll a "TRIGGER"
bit for this.

Signed-off-by: Charles Perry <charles.perry@microchip.com>
---

Notes:
    Changes in v4: none
    Changes in v3: none
    
    Changes in v2:
      - Make "clocks" and "interrupts" required (Andrew)
      - Add a default value to "clock-frequency" (Andrew)

 .../net/microchip,pic64hpsc-mdio.yaml         | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/microchip,pic64hpsc-mdio.yaml

diff --git a/Documentation/devicetree/bindings/net/microchip,pic64hpsc-mdio.yaml b/Documentation/devicetree/bindings/net/microchip,pic64hpsc-mdio.yaml
new file mode 100644
index 000000000000..d690afe3d3cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/microchip,pic64hpsc-mdio.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/microchip,pic64hpsc-mdio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PIC64-HPSC/HX MDIO controller
+
+maintainers:
+  - Charles Perry <charles.perry@microchip.com>
+
+description: |
+  Microchip PIC64-HPSC/HX SoCs have two MDIO bus controller. This MDIO bus
+  controller supports C22 and C45 register access. It is named "MDIO Initiator"
+  in the documentation.
+
+allOf:
+  - $ref: mdio.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - const: microchip,pic64hpsc-mdio
+      - items:
+          - const: microchip,pic64hx-mdio
+          - const: microchip,pic64hpsc-mdio
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-frequency:
+    default: 2500000
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    bus {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        mdio@4000C21E000 {
+            compatible = "microchip,pic64hpsc-mdio";
+            reg = <0x400 0x0C21E000 0x0 0x1000>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+            clocks = <&svc_clk>;
+            interrupt-parent = <&saplic0>;
+            interrupts = <168 IRQ_TYPE_LEVEL_HIGH>;
+
+            phy0: ethernet-phy@0 {
+                reg = <0>;
+            };
+        };
+    };
-- 
2.47.3

[PATCH net-next v4 2/3] net: mdio: add a driver for PIC64-HPSC/HX MDIO controller

[Charles Perry]

This adds an MDIO driver for PIC64-HPSC/HX. The hardware supports C22
and C45 but only C22 is implemented in this commit.

This MDIO hardware is based on a Microsemi design supported in Linux by
mdio-mscc-miim.c. However, The register interface is completely
different with pic64hpsc, hence the need for a separate driver.

The documentation recommends an input clock of 156.25MHz and a prescaler
of 39, which yields an MDIO clock of 1.95MHz.

The hardware supports an interrupt pin or a "TRIGGER" bit that can be
polled to signal transaction completion. This commit uses polling.

This was tested on Microchip HB1301 evalkit with a VSC8574 and a
VSC8541.

Signed-off-by: Charles Perry <charles.perry@microchip.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---

Notes:
    Changes in v4:
      - read: return FIELD_GET() directly instead of using "ret" (Russell)
    
    Changes in v3:
      - Add a MAINTAINERS entry (Jakub)
    
    Changes in v2:
      - Remove #define for unused registers (Maxime)
      - Add "c22" to clause 22 read/write ops (Maxime)
      - Remove the call to platform_set_drvdata() (Andrew)
      - Make the clock mandatory (Andrew)
      - Use 2.5MHz if no clock-frequency was specified (Andrew)
      - Change the error message for bad clock-frequency (Andrew)
      - Fix a use without initialization on bus_freq (Andrew)

 MAINTAINERS                       |   6 +
 drivers/net/mdio/Kconfig          |   7 ++
 drivers/net/mdio/Makefile         |   1 +
 drivers/net/mdio/mdio-pic64hpsc.c | 190 ++++++++++++++++++++++++++++++
 4 files changed, 204 insertions(+)
 create mode 100644 drivers/net/mdio/mdio-pic64hpsc.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e08767323763..9297c46dba91 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17216,6 +17216,12 @@ L:	linux-serial@vger.kernel.org
 S:	Maintained
 F:	drivers/tty/serial/8250/8250_pci1xxxx.c
 
+MICROCHIP PIC64-HPSC/HX DRIVERS
+M:	Charles Perry <charles.perry@microchip.com>
+S:	Supported
+F:	Documentation/devicetree/bindings/net/microchip,pic64hpsc-mdio.yaml
+F:	drivers/net/mdio/mdio-pic64hpsc.c
+
 MICROCHIP POLARFIRE FPGA DRIVERS
 M:	Conor Dooley <conor.dooley@microchip.com>
 L:	linux-fpga@vger.kernel.org
diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
index 44380378911b..7bdba8c3ddef 100644
--- a/drivers/net/mdio/Kconfig
+++ b/drivers/net/mdio/Kconfig
@@ -146,6 +146,13 @@ config MDIO_OCTEON
 	  buses. It is required by the Octeon and ThunderX ethernet device
 	  drivers on some systems.
 
+config MDIO_PIC64HPSC
+	tristate "PIC64-HPSC/HX MDIO interface support"
+	depends on HAS_IOMEM && OF_MDIO
+	help
+	  This driver supports the MDIO interface found on the PIC64-HPSC/HX
+	  SoCs.
+
 config MDIO_IPQ4019
 	tristate "Qualcomm IPQ4019 MDIO interface support"
 	depends on HAS_IOMEM && OF_MDIO
diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
index fbec636700e7..048586746026 100644
--- a/drivers/net/mdio/Makefile
+++ b/drivers/net/mdio/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_MDIO_MOXART)		+= mdio-moxart.o
 obj-$(CONFIG_MDIO_MSCC_MIIM)		+= mdio-mscc-miim.o
 obj-$(CONFIG_MDIO_MVUSB)		+= mdio-mvusb.o
 obj-$(CONFIG_MDIO_OCTEON)		+= mdio-octeon.o
+obj-$(CONFIG_MDIO_PIC64HPSC)		+= mdio-pic64hpsc.o
 obj-$(CONFIG_MDIO_REALTEK_RTL9300)	+= mdio-realtek-rtl9300.o
 obj-$(CONFIG_MDIO_REGMAP)		+= mdio-regmap.o
 obj-$(CONFIG_MDIO_SUN4I)		+= mdio-sun4i.o
diff --git a/drivers/net/mdio/mdio-pic64hpsc.c b/drivers/net/mdio/mdio-pic64hpsc.c
new file mode 100644
index 000000000000..28be77ad6cf7
--- /dev/null
+++ b/drivers/net/mdio/mdio-pic64hpsc.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Microchip PIC64-HPSC/HX MDIO controller driver
+ *
+ * Copyright (c) 2026 Microchip Technology Inc. and its subsidiaries.
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_mdio.h>
+#include <linux/platform_device.h>
+
+#define MDIO_REG_PRESCALER     0x20
+#define MDIO_CFG_PRESCALE_MASK GENMASK(7, 0)
+
+#define MDIO_REG_FRAME_CFG_1 0x24
+#define MDIO_WDATA_MASK	     GENMASK(15, 0)
+
+#define MDIO_REG_FRAME_CFG_2	 0x28
+#define MDIO_TRIGGER_BIT	 BIT(31)
+#define MDIO_REG_DEV_ADDR_MASK	 GENMASK(20, 16)
+#define MDIO_PHY_PRT_ADDR_MASK	 GENMASK(8, 4)
+#define MDIO_OPERATION_MASK	 GENMASK(3, 2)
+#define MDIO_START_OF_FRAME_MASK GENMASK(1, 0)
+
+/* Possible value of MDIO_OPERATION_MASK */
+#define MDIO_OPERATION_WRITE BIT(0)
+#define MDIO_OPERATION_READ  BIT(1)
+
+#define MDIO_REG_FRAME_STATUS 0x2C
+#define MDIO_READOK_BIT	      BIT(24)
+#define MDIO_RDATA_MASK	      GENMASK(15, 0)
+
+struct pic64hpsc_mdio_dev {
+	void __iomem *regs;
+};
+
+static int pic64hpsc_mdio_wait_trigger(struct mii_bus *bus)
+{
+	struct pic64hpsc_mdio_dev *priv = bus->priv;
+	u32 val;
+	int ret;
+
+	/* The MDIO_TRIGGER bit returns 0 when a transaction has completed. */
+	ret = readl_poll_timeout(priv->regs + MDIO_REG_FRAME_CFG_2, val,
+				 !(val & MDIO_TRIGGER_BIT), 50, 10000);
+
+	if (ret < 0)
+		dev_dbg(&bus->dev, "TRIGGER bit timeout: %x\n", val);
+
+	return ret;
+}
+
+static int pic64hpsc_mdio_c22_read(struct mii_bus *bus, int mii_id, int regnum)
+{
+	struct pic64hpsc_mdio_dev *priv = bus->priv;
+	u32 val;
+	int ret;
+
+	ret = pic64hpsc_mdio_wait_trigger(bus);
+	if (ret)
+		return ret;
+
+	writel(MDIO_TRIGGER_BIT | FIELD_PREP(MDIO_REG_DEV_ADDR_MASK, regnum) |
+		       FIELD_PREP(MDIO_PHY_PRT_ADDR_MASK, mii_id) |
+		       FIELD_PREP(MDIO_OPERATION_MASK, MDIO_OPERATION_READ) |
+		       FIELD_PREP(MDIO_START_OF_FRAME_MASK, 1),
+	       priv->regs + MDIO_REG_FRAME_CFG_2);
+
+	ret = pic64hpsc_mdio_wait_trigger(bus);
+	if (ret)
+		return ret;
+
+	val = readl(priv->regs + MDIO_REG_FRAME_STATUS);
+
+	/* The MDIO_READOK is a 1-bit value reflecting the inverse of the MDIO
+	 * bus value captured during the 2nd TA cycle. A PHY/Port should drive
+	 * the MDIO bus with a logic 0 on the 2nd TA cycle, however, the
+	 * PHY/Port could optionally drive a logic 1, to communicate a read
+	 * failure. This feature is optional, not defined by the 802.3 standard
+	 * and not supported in standard external PHYs.
+	 */
+	if (!(bus->phy_ignore_ta_mask & 1 << mii_id) &&
+	    !FIELD_GET(MDIO_READOK_BIT, val)) {
+		dev_dbg(&bus->dev, "READOK bit cleared\n");
+		return -EIO;
+	}
+
+	return FIELD_GET(MDIO_RDATA_MASK, val);
+}
+
+static int pic64hpsc_mdio_c22_write(struct mii_bus *bus, int mii_id, int regnum,
+				    u16 value)
+{
+	struct pic64hpsc_mdio_dev *priv = bus->priv;
+	int ret;
+
+	ret = pic64hpsc_mdio_wait_trigger(bus);
+	if (ret < 0)
+		return ret;
+
+	writel(FIELD_PREP(MDIO_WDATA_MASK, value),
+	       priv->regs + MDIO_REG_FRAME_CFG_1);
+
+	writel(MDIO_TRIGGER_BIT | FIELD_PREP(MDIO_REG_DEV_ADDR_MASK, regnum) |
+		       FIELD_PREP(MDIO_PHY_PRT_ADDR_MASK, mii_id) |
+		       FIELD_PREP(MDIO_OPERATION_MASK, MDIO_OPERATION_WRITE) |
+		       FIELD_PREP(MDIO_START_OF_FRAME_MASK, 1),
+	       priv->regs + MDIO_REG_FRAME_CFG_2);
+
+	return 0;
+}
+
+static int pic64hpsc_mdio_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	struct pic64hpsc_mdio_dev *priv;
+	struct mii_bus *bus;
+	unsigned long rate;
+	struct clk *clk;
+	u32 bus_freq;
+	u32 div;
+	int ret;
+
+	bus = devm_mdiobus_alloc_size(dev, sizeof(*priv));
+	if (!bus)
+		return -ENOMEM;
+
+	priv = bus->priv;
+
+	priv->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->regs))
+		return PTR_ERR(priv->regs);
+
+	bus->name = KBUILD_MODNAME;
+	bus->read = pic64hpsc_mdio_c22_read;
+	bus->write = pic64hpsc_mdio_c22_write;
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
+	bus->parent = dev;
+
+	clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	if (of_property_read_u32(np, "clock-frequency", &bus_freq))
+		bus_freq = 2500000;
+
+	rate = clk_get_rate(clk);
+
+	div = DIV_ROUND_UP(rate, 2 * bus_freq) - 1;
+	if (div == 0 || div & ~MDIO_CFG_PRESCALE_MASK) {
+		dev_err(dev, "MDIO clock-frequency out of range\n");
+		return -EINVAL;
+	}
+
+	dev_dbg(dev, "rate=%lu bus_freq=%u real_bus_freq=%lu div=%u\n", rate,
+		bus_freq, rate / (2 * (1 + div)), div);
+	writel(div, priv->regs + MDIO_REG_PRESCALER);
+
+	ret = devm_of_mdiobus_register(dev, bus, np);
+	if (ret) {
+		dev_err(dev, "Cannot register MDIO bus (%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id pic64hpsc_mdio_match[] = {
+	{ .compatible = "microchip,pic64hpsc-mdio" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, pic64hpsc_mdio_match);
+
+static struct platform_driver pic64hpsc_mdio_driver = {
+	.probe = pic64hpsc_mdio_probe,
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = pic64hpsc_mdio_match,
+	},
+};
+module_platform_driver(pic64hpsc_mdio_driver);
+
+MODULE_AUTHOR("Charles Perry <charles.perry@microchip.com>");
+MODULE_DESCRIPTION("Microchip PIC64-HPSC/HX MDIO driver");
+MODULE_LICENSE("GPL");
-- 
2.47.3

[PATCH net-next v4 3/3] net: phy: add a PHY write barrier when disabling interrupts

[Charles Perry]

MDIO bus controllers are not required to wait for write transactions to
complete before returning as synchronization is often achieved by polling
status bits.

This can cause issues when disabling interrupts since an interrupt could
fire before the interrupt handler is unregistered and there's no status
bit to poll.

Add a phy_write_barrier() function and use it in phy_disable_interrupts()
to fix this issue. The write barrier just reads an MII register and
discards the value, which is enough to guarantee that previous writes have
completed.

Signed-off-by: Charles Perry <charles.perry@microchip.com>
---

Notes:
    Changes in v4:
      - Add this patch (Russell, Andrew)

 drivers/net/phy/phy.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 13dd1691886d..2be0b90e9947 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1368,14 +1368,39 @@ void phy_error(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_error);
 
+/**
+ * phy_write_barrier - ensure the last write completed for this PHY device
+ * @phydev: target phy_device struct
+ *
+ * MDIO bus controllers are not required to wait for write transactions to
+ * complete before returning. Calling this function ensures that the previous
+ * write has completed.
+ */
+static int phy_write_barrier(struct phy_device *phydev)
+{
+	int err;
+
+	err = phy_read(phydev, MII_PHYSID1);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
 /**
  * phy_disable_interrupts - Disable the PHY interrupts from the PHY side
  * @phydev: target phy_device struct
  */
 int phy_disable_interrupts(struct phy_device *phydev)
 {
+	int err;
+
 	/* Disable PHY interrupts */
-	return phy_config_interrupt(phydev, PHY_INTERRUPT_DISABLED);
+	err = phy_config_interrupt(phydev, PHY_INTERRUPT_DISABLED);
+	if (err)
+		return err;
+
+	return phy_write_barrier(phydev);
 }
 
 /**
-- 
2.47.3

Re: [PATCH net-next v4 3/3] net: phy: add a PHY write barrier when disabling interrupts

[Andrew Lunn]

> +static int phy_write_barrier(struct phy_device *phydev)
> +{
> +	int err;
> +
> +	err = phy_read(phydev, MII_PHYSID1);
> +	if (err < 0)
> +		return err;

There are a small number of MDIO busses which don't implement C22,
only C45. You are likely to get -EIO or maybe ENODEV, -EOPNOTSUPP,
-EINVAL for such a read. Returning the error than makes
phy_disable_interrupts() fail, etc.

This is why i suggested throwing away the return value.

Maybe call phydev_info() if there is an error, but i suggest making
this a void function.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next v4 3/3] net: phy: add a PHY write barrier when disabling interrupts

[Charles Perry]

On Thu, Apr 02, 2026 at 03:31:52PM +0200, Andrew Lunn wrote:
> > +static int phy_write_barrier(struct phy_device *phydev)
> > +{
> > +	int err;
> > +
> > +	err = phy_read(phydev, MII_PHYSID1);
> > +	if (err < 0)
> > +		return err;
> 
> There are a small number of MDIO busses which don't implement C22,
> only C45. You are likely to get -EIO or maybe ENODEV, -EOPNOTSUPP,
> -EINVAL for such a read. Returning the error than makes
> phy_disable_interrupts() fail, etc.

Ok, I didn't know about this. Do you think there's any merit in handling
the case where phydev->mdio.bus->read is NULL and doing a C45 access in
that case?

> 
> This is why i suggested throwing away the return value.

Ok

Thanks,
Charles

> 
> Maybe call phydev_info() if there is an error, but i suggest making
> this a void function.
> 
>     Andrew
> 
> ---
> pw-bot: cr

Re: [PATCH net-next v4 3/3] net: phy: add a PHY write barrier when disabling interrupts

[Russell King (Oracle)]

On Thu, Apr 02, 2026 at 03:31:52PM +0200, Andrew Lunn wrote:
> > +static int phy_write_barrier(struct phy_device *phydev)
> > +{
> > +	int err;
> > +
> > +	err = phy_read(phydev, MII_PHYSID1);
> > +	if (err < 0)
> > +		return err;
> 
> There are a small number of MDIO busses which don't implement C22,
> only C45. You are likely to get -EIO or maybe ENODEV, -EOPNOTSUPP,
> -EINVAL for such a read. Returning the error than makes
> phy_disable_interrupts() fail, etc.

If it returns -EOPNOTSUPP (meaning, at least, that bus->read is not
implemented), do we want to issue a C45 read instead?

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

Re: [PATCH net-next v4 3/3] net: phy: add a PHY write barrier when disabling interrupts

[Andrew Lunn]

On Thu, Apr 02, 2026 at 07:03:42PM +0100, Russell King (Oracle) wrote:
> On Thu, Apr 02, 2026 at 03:31:52PM +0200, Andrew Lunn wrote:
> > > +static int phy_write_barrier(struct phy_device *phydev)
> > > +{
> > > +	int err;
> > > +
> > > +	err = phy_read(phydev, MII_PHYSID1);
> > > +	if (err < 0)
> > > +		return err;
> > 
> > There are a small number of MDIO busses which don't implement C22,
> > only C45. You are likely to get -EIO or maybe ENODEV, -EOPNOTSUPP,
> > -EINVAL for such a read. Returning the error than makes
> > phy_disable_interrupts() fail, etc.
> 
> If it returns -EOPNOTSUPP (meaning, at least, that bus->read is not
> implemented), do we want to issue a C45 read instead?

Maybe.

get_phy_c45_ids() first reads MDIO_MMD_PMAPMD : MII_PHYSID1. That
seems like a safe option.

      Andrew

Re: [PATCH net-next v4 3/3] net: phy: add a PHY write barrier when disabling interrupts

[Charles Perry]

On Thu, Apr 02, 2026 at 08:09:06PM +0200, Andrew Lunn wrote:
> On Thu, Apr 02, 2026 at 07:03:42PM +0100, Russell King (Oracle) wrote:
> > On Thu, Apr 02, 2026 at 03:31:52PM +0200, Andrew Lunn wrote:
> > > > +static int phy_write_barrier(struct phy_device *phydev)
> > > > +{
> > > > +	int err;
> > > > +
> > > > +	err = phy_read(phydev, MII_PHYSID1);
> > > > +	if (err < 0)
> > > > +		return err;
> > > 
> > > There are a small number of MDIO busses which don't implement C22,
> > > only C45. You are likely to get -EIO or maybe ENODEV, -EOPNOTSUPP,
> > > -EINVAL for such a read. Returning the error than makes
> > > phy_disable_interrupts() fail, etc.
> > 
> > If it returns -EOPNOTSUPP (meaning, at least, that bus->read is not
> > implemented), do we want to issue a C45 read instead?
> 
> Maybe.
> 
> get_phy_c45_ids() first reads MDIO_MMD_PMAPMD : MII_PHYSID1. That
> seems like a safe option.

Here's where I'm at with this:

static void phy_write_barrier(struct phy_device *phydev)
{
       int err;
 
       err = mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, MII_PHYSID1);
       if (err == -EOPNOTSUPP)
               mdiobus_c45_read(phydev->mdio.bus, phydev->mdio.addr,
                                __ffs(phydev->c45_ids.mmds_present),
                                MII_PHYSID1);
}

In part inspired by phylink_phy_read() in phylink.c

Do you think there's any way I can test this on my VSC8574 or VSC8541? It
supports some C45 registers for EEE but not the device discovery part.

Thanks,
Charles

Re: [PATCH net-next v4 3/3] net: phy: add a PHY write barrier when disabling interrupts

[Andrew Lunn]

> static void phy_write_barrier(struct phy_device *phydev)
> {
>        int err;
>  
>        err = mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, MII_PHYSID1);
>        if (err == -EOPNOTSUPP)
>                mdiobus_c45_read(phydev->mdio.bus, phydev->mdio.addr,
>                                 __ffs(phydev->c45_ids.mmds_present),
>                                 MII_PHYSID1);
> }

Using __ffs() is maybe more complex than needed.

All you are trying to do is ensure the last write happened, by doing a
read. Any read should work, even if the device does not respond. We
just need to be careful not to read a register which might clear on
read, such as an interrupt status register, or the link status, which
latches. MII_PHYID1 is safe. Since we throw away the value, we don't
care if the MMD is not present, the read will still flush the previous
write. So i would replace the _ffs() with a hard coded value. KISS.

> Do you think there's any way I can test this on my VSC8574 or VSC8541? It
> supports some C45 registers for EEE but not the device discovery part.

It is not so easy to do. You need to hack the C22 read so that is
returns EOPNOTSUPP, but you also need the first few reads to return a
valid value otherwise the probe will fail.

I think this is one of the cases that if the reviewers thinks its
looks O.K, we can accept it without extensive testing.

      Andrew

Re: [PATCH net-next v4 3/3] net: phy: add a PHY write barrier when disabling interrupts

[Charles Perry]

On Tue, Apr 07, 2026 at 06:59:00PM +0200, Andrew Lunn wrote:
> > static void phy_write_barrier(struct phy_device *phydev)
> > {
> >        int err;
> >  
> >        err = mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, MII_PHYSID1);
> >        if (err == -EOPNOTSUPP)
> >                mdiobus_c45_read(phydev->mdio.bus, phydev->mdio.addr,
> >                                 __ffs(phydev->c45_ids.mmds_present),
> >                                 MII_PHYSID1);
> > }
> 
> Using __ffs() is maybe more complex than needed.
> 
> All you are trying to do is ensure the last write happened, by doing a
> read. Any read should work, even if the device does not respond. We
> just need to be careful not to read a register which might clear on
> read, such as an interrupt status register, or the link status, which
> latches. MII_PHYID1 is safe. Since we throw away the value, we don't
> care if the MMD is not present, the read will still flush the previous
> write. So i would replace the _ffs() with a hard coded value. KISS.

Ok, I'll use device 1 (MDIO_MMD_PMAPMD).

Thanks for the explanations.

> 
> > Do you think there's any way I can test this on my VSC8574 or VSC8541? It
> > supports some C45 registers for EEE but not the device discovery part.
> 
> It is not so easy to do. You need to hack the C22 read so that is
> returns EOPNOTSUPP, but you also need the first few reads to return a
> valid value otherwise the probe will fail.
> 
> I think this is one of the cases that if the reviewers thinks its
> looks O.K, we can accept it without extensive testing.
> 

Ok

Thanks,
Charles