[PATCH net-next v1 0/5] net: mdiobus: HIde ACPI implementation

[PATCH net-next v1 0/5] net: mdiobus: HIde ACPI implementation

[Andy Shevchenko]

This mini-series is dedicated to hiding ACPI implementation details
from the wider users as they (as of today) do not need to know that.
That also introduces the agnostic API that is preferred over OF /
ACPI / et cetera one.

I haven't tried to unify __*_mdiobus_register() as it seems using
something that may differ in ACPI vs. OF cases, it can be done later
on, if one knows how to do that with all the details.

The change has been compile tested (including configurations with
CONFIG_ACPI=n).

Andy Shevchenko (5):
  net: mdiobus: Provide fwnode_mdiobus_register()
  net: mvmdio: Switch to using fwnode_mdiobus_register()
  net/fsl: xgmac_mdio: Switch to using fwnode_mdiobus_register()
  net/fsl: xgmac_mdio: Reuse existing pointer to fwnode
  net: mdiobus: Hide acpi_mdio.h

 drivers/net/ethernet/freescale/xgmac_mdio.c | 23 +++++---------
 drivers/net/ethernet/marvell/mvmdio.c       | 11 ++-----
 drivers/net/mdio/acpi_mdio.c                |  3 +-
 drivers/net/mdio/fwnode_mdio.c              | 16 ++++++++++
 drivers/net/mdio/mdio.h                     |  4 +++
 include/linux/acpi_mdio.h                   | 33 ---------------------
 include/linux/fwnode_mdio.h                 | 17 +++++++++++
 7 files changed, 49 insertions(+), 58 deletions(-)
 create mode 100644 drivers/net/mdio/mdio.h
 delete mode 100644 include/linux/acpi_mdio.h

-- 
2.50.1

[PATCH net-next v1 1/5] net: mdiobus: Provide fwnode_mdiobus_register()

[Andy Shevchenko]

Provide an agnostic helper to register MDIO bus independently on
the firmware node provider.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/mdio/fwnode_mdio.c | 15 +++++++++++++++
 include/linux/fwnode_mdio.h    | 17 +++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index ba7091518265..7541df43ba6d 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -7,9 +7,11 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/acpi_mdio.h>
 #include <linux/dev_printk.h>
 #include <linux/fwnode_mdio.h>
 #include <linux/of.h>
+#include <linux/of_mdio.h>
 #include <linux/phy.h>
 #include <linux/pse-pd/pse.h>
 
@@ -187,3 +189,16 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 	return rc;
 }
 EXPORT_SYMBOL(fwnode_mdiobus_register_phy);
+
+int __fwnode_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode,
+			      struct module *owner)
+{
+	if (is_of_node(fwnode))
+		return __of_mdiobus_register(mdio, to_of_node(fwnode), owner);
+
+	if (is_acpi_node(fwnode))
+		return __acpi_mdiobus_register(mdio, fwnode, owner);
+
+	return mdiobus_register(mdio);
+}
+EXPORT_SYMBOL(__fwnode_mdiobus_register);
diff --git a/include/linux/fwnode_mdio.h b/include/linux/fwnode_mdio.h
index faf603c48c86..c316b738333a 100644
--- a/include/linux/fwnode_mdio.h
+++ b/include/linux/fwnode_mdio.h
@@ -16,6 +16,14 @@ int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
 int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 				struct fwnode_handle *child, u32 addr);
 
+int __fwnode_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode,
+			      struct module *owner);
+
+static inline int
+fwnode_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode)
+{
+	return __fwnode_mdiobus_register(mdio, fwnode, THIS_MODULE);
+}
 #else /* CONFIG_FWNODE_MDIO */
 int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
 				       struct phy_device *phy,
@@ -30,6 +38,15 @@ static inline int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 {
 	return -EINVAL;
 }
+
+static inline int fwnode_mdiobus_register(struct mii_bus *mdio, struct
+					  fwnode_handle *fwnode)
+{
+	/* Fall back to mdiobus_register() function to register a bus.
+	 * This way, we don't have to keep compat bits around in drivers.
+	 */
+	return mdiobus_register(mdio);
+}
 #endif
 
 #endif /* __LINUX_FWNODE_MDIO_H */
-- 
2.50.1

[PATCH net-next v1 2/5] net: mvmdio: Switch to using fwnode_mdiobus_register()

[Andy Shevchenko]

Replace custom implementation of fwnode_mdiobus_register().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/marvell/mvmdio.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 3f4447e68888..62fbe22c01cb 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -18,16 +18,15 @@
  */
 
 #include <linux/acpi.h>
-#include <linux/acpi_mdio.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/fwnode_mdio.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
-#include <linux/of_mdio.h>
 #include <linux/phy.h>
 #include <linux/platform_device.h>
 #include <linux/sched.h>
@@ -379,13 +378,7 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		goto out_mdio;
 	}
 
-	/* For the platforms not supporting DT/ACPI fall-back
-	 * to mdiobus_register via of_mdiobus_register.
-	 */
-	if (is_acpi_node(pdev->dev.fwnode))
-		ret = acpi_mdiobus_register(bus, pdev->dev.fwnode);
-	else
-		ret = of_mdiobus_register(bus, pdev->dev.of_node);
+	ret = fwnode_mdiobus_register(bus, dev_fwnode(&pdev->dev));
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
 		goto out_mdio;
-- 
2.50.1

[PATCH net-next v1 3/5] net/fsl: xgmac_mdio: Switch to using fwnode_mdiobus_register()

[Andy Shevchenko]

Replace custom implementation of fwnode_mdiobus_register().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/freescale/xgmac_mdio.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
index 65dc07d0df0f..b63c8a3d6daf 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -13,14 +13,13 @@
  */
 
 #include <linux/acpi.h>
-#include <linux/acpi_mdio.h>
 #include <linux/clk.h>
+#include <linux/fwnode_mdio.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/mdio.h>
 #include <linux/module.h>
 #include <linux/of.h>
-#include <linux/of_mdio.h>
 #include <linux/phy.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -370,7 +369,7 @@ static void xgmac_mdio_set_suppress_preamble(struct mii_bus *bus)
 
 static int xgmac_mdio_probe(struct platform_device *pdev)
 {
-	struct fwnode_handle *fwnode;
+	struct fwnode_handle *fwnode = dev_fwnode(&pdev->dev);
 	struct mdio_fsl_priv *priv;
 	struct resource *res;
 	struct mii_bus *bus;
@@ -422,13 +421,7 @@ static int xgmac_mdio_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	fwnode = dev_fwnode(&pdev->dev);
-	if (is_of_node(fwnode))
-		ret = of_mdiobus_register(bus, to_of_node(fwnode));
-	else if (is_acpi_node(fwnode))
-		ret = acpi_mdiobus_register(bus, fwnode);
-	else
-		ret = -EINVAL;
+	ret = fwnode_mdiobus_register(bus, fwnode);
 	if (ret) {
 		dev_err(&pdev->dev, "cannot register MDIO bus\n");
 		return ret;
-- 
2.50.1

[PATCH net-next v1 4/5] net/fsl: xgmac_mdio: Reuse existing pointer to fwnode

[Andy Shevchenko]

The probe function has already been using pointer to fwnode.
Reuse it instead of doing this several times.

While at it, amend the header inclusions regarding to the used APIs.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/freescale/xgmac_mdio.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
index b63c8a3d6daf..f31c36b8ec98 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -12,16 +12,16 @@
  * kind, whether express or implied.
  */
 
-#include <linux/acpi.h>
 #include <linux/clk.h>
 #include <linux/fwnode_mdio.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/mdio.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
-#include <linux/of.h>
 #include <linux/phy.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 
 /* Number of microseconds to wait for a register to respond */
@@ -407,12 +407,12 @@ static int xgmac_mdio_probe(struct platform_device *pdev)
 	/* For both ACPI and DT cases, endianness of MDIO controller
 	 * needs to be specified using "little-endian" property.
 	 */
-	priv->is_little_endian = device_property_read_bool(&pdev->dev,
+	priv->is_little_endian = fwnode_property_read_bool(fwnode,
 							   "little-endian");
 
-	priv->has_a009885 = device_property_read_bool(&pdev->dev,
+	priv->has_a009885 = fwnode_property_read_bool(fwnode,
 						      "fsl,erratum-a009885");
-	priv->has_a011043 = device_property_read_bool(&pdev->dev,
+	priv->has_a011043 = fwnode_property_read_bool(fwnode,
 						      "fsl,erratum-a011043");
 
 	xgmac_mdio_set_suppress_preamble(bus);
-- 
2.50.1

[PATCH net-next v1 5/5] net: mdiobus: Hide acpi_mdio.h

[Andy Shevchenko]

No more users outside of MDIO framework.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/mdio/acpi_mdio.c   |  3 ++-
 drivers/net/mdio/fwnode_mdio.c |  3 ++-
 drivers/net/mdio/mdio.h        |  4 ++++
 include/linux/acpi_mdio.h      | 33 ---------------------------------
 4 files changed, 8 insertions(+), 35 deletions(-)
 create mode 100644 drivers/net/mdio/mdio.h
 delete mode 100644 include/linux/acpi_mdio.h

diff --git a/drivers/net/mdio/acpi_mdio.c b/drivers/net/mdio/acpi_mdio.c
index 5d0f11f280cf..15c3fc05fad5 100644
--- a/drivers/net/mdio/acpi_mdio.c
+++ b/drivers/net/mdio/acpi_mdio.c
@@ -7,13 +7,14 @@
  */
 
 #include <linux/acpi.h>
-#include <linux/acpi_mdio.h>
 #include <linux/bits.h>
 #include <linux/dev_printk.h>
 #include <linux/fwnode_mdio.h>
 #include <linux/module.h>
 #include <linux/types.h>
 
+#include "mdio.h"
+
 MODULE_AUTHOR("Calvin Johnson <calvin.johnson@oss.nxp.com>");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("ACPI MDIO bus (Ethernet PHY) accessors");
diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index 7541df43ba6d..610753d55262 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -7,7 +7,6 @@
  */
 
 #include <linux/acpi.h>
-#include <linux/acpi_mdio.h>
 #include <linux/dev_printk.h>
 #include <linux/fwnode_mdio.h>
 #include <linux/of.h>
@@ -15,6 +14,8 @@
 #include <linux/phy.h>
 #include <linux/pse-pd/pse.h>
 
+#include "mdio.h"
+
 MODULE_AUTHOR("Calvin Johnson <calvin.johnson@oss.nxp.com>");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("FWNODE MDIO bus (Ethernet PHY) accessors");
diff --git a/drivers/net/mdio/mdio.h b/drivers/net/mdio/mdio.h
new file mode 100644
index 000000000000..c053bb05368a
--- /dev/null
+++ b/drivers/net/mdio/mdio.h
@@ -0,0 +1,4 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+int __acpi_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode,
+			    struct module *owner);
diff --git a/include/linux/acpi_mdio.h b/include/linux/acpi_mdio.h
deleted file mode 100644
index 8e2eefa9fbc0..000000000000
--- a/include/linux/acpi_mdio.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * ACPI helper for the MDIO (Ethernet PHY) API
- */
-
-#ifndef __LINUX_ACPI_MDIO_H
-#define __LINUX_ACPI_MDIO_H
-
-#include <linux/phy.h>
-
-#if IS_ENABLED(CONFIG_ACPI_MDIO)
-int __acpi_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode,
-			    struct module *owner);
-
-static inline int
-acpi_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *handle)
-{
-	return __acpi_mdiobus_register(mdio, handle, THIS_MODULE);
-}
-#else /* CONFIG_ACPI_MDIO */
-static inline int
-acpi_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode)
-{
-	/*
-	 * Fall back to mdiobus_register() function to register a bus.
-	 * This way, we don't have to keep compat bits around in drivers.
-	 */
-
-	return mdiobus_register(mdio);
-}
-#endif
-
-#endif /* __LINUX_ACPI_MDIO_H */
-- 
2.50.1

Re: [PATCH net-next v1 0/5] net: mdiobus: HIde ACPI implementation

[Andrew Lunn]

On Mon, May 04, 2026 at 09:29:51AM +0200, Andy Shevchenko wrote:
> This mini-series is dedicated to hiding ACPI implementation details
> from the wider users as they (as of today) do not need to know that.

Please could you expand on that. ACPI != OF. They have different
bindings, so you need different implementations.

	Andrew

Re: [PATCH net-next v1 0/5] net: mdiobus: HIde ACPI implementation

[Andy Shevchenko]

On Mon, May 04, 2026 at 03:02:54PM +0200, Andrew Lunn wrote:
> On Mon, May 04, 2026 at 09:29:51AM +0200, Andy Shevchenko wrote:
> > This mini-series is dedicated to hiding ACPI implementation details
> > from the wider users as they (as of today) do not need to know that.
> 
> Please could you expand on that. ACPI != OF. They have different
> bindings, so you need different implementations.

As of today the users that want ACPI also have the OF support. Even without
that if the device is pure ACPI supported one (and somehow never going to DT)
the proposed API (see the first patch) will be no-op in case when CONFIG_ACPI=n
or when it's a non-ACPI platform with no support of the device. Hence, the
pure ACPI (and actually OF as well) do not need to be exposed. The decision is
made based on the type of firmware node.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net-next v1 0/5] net: mdiobus: HIde ACPI implementation

[Andrew Lunn]

On Mon, May 04, 2026 at 04:49:35PM +0300, Andy Shevchenko wrote:
> On Mon, May 04, 2026 at 03:02:54PM +0200, Andrew Lunn wrote:
> > On Mon, May 04, 2026 at 09:29:51AM +0200, Andy Shevchenko wrote:
> > > This mini-series is dedicated to hiding ACPI implementation details
> > > from the wider users as they (as of today) do not need to know that.
> > 
> > Please could you expand on that. ACPI != OF. They have different
> > bindings, so you need different implementations.
> 
> As of today the users that want ACPI also have the OF support. Even without
> that if the device is pure ACPI supported one (and somehow never going to DT)
> the proposed API (see the first patch) will be no-op in case when CONFIG_ACPI=n
> or when it's a non-ACPI platform with no support of the device. Hence, the
> pure ACPI (and actually OF as well) do not need to be exposed. The decision is
> made based on the type of firmware node.

I see two different things here:

You are refactoring code into a helper, so reducing the amount of
duplicated code. That in itself is good.

However, we have the problem in general that developers think that OF
and ACPI do the same thing. And that is not true. OF MDIO busses have
support for a GPIO used as a reset. In retrospect, that was a bad
idea. The documented ACPI binding does not have this, and if anybody
was to suggest adding it, i want to NACK it.

Having OF code and ACPI code to instantiate the MDIO bus makes it
clearer they are different things. fwnode gives the impression they
are the same, which is not true. Hence i don't like fwnode at this
level.

Where fwnode makes sense is deeper down in the implementation of the
bindings, for properties which are the same in both bindings.  Then
you can use fwnode_ for the shared properties, of_ for the OF only
properties, and acpi_ for the ACPI only properties.

So although i like the reduction in duplicated code, i think overall
it makes the code worse because developers are more likely to wrongly
understand it.

	    Andrew

Re: [PATCH net-next v1 0/5] net: mdiobus: HIde ACPI implementation

[Andy Shevchenko]

On Mon, May 04, 2026 at 05:16:57PM +0200, Andrew Lunn wrote:
> On Mon, May 04, 2026 at 04:49:35PM +0300, Andy Shevchenko wrote:
> > On Mon, May 04, 2026 at 03:02:54PM +0200, Andrew Lunn wrote:
> > > On Mon, May 04, 2026 at 09:29:51AM +0200, Andy Shevchenko wrote:
> > > > This mini-series is dedicated to hiding ACPI implementation details
> > > > from the wider users as they (as of today) do not need to know that.
> > > 
> > > Please could you expand on that. ACPI != OF. They have different
> > > bindings, so you need different implementations.
> > 
> > As of today the users that want ACPI also have the OF support. Even without
> > that if the device is pure ACPI supported one (and somehow never going to DT)
> > the proposed API (see the first patch) will be no-op in case when CONFIG_ACPI=n
> > or when it's a non-ACPI platform with no support of the device. Hence, the
> > pure ACPI (and actually OF as well) do not need to be exposed. The decision is
> > made based on the type of firmware node.
> 
> I see two different things here:
> 
> You are refactoring code into a helper, so reducing the amount of
> duplicated code. That in itself is good.
> 
> However, we have the problem in general that developers think that OF
> and ACPI do the same thing. And that is not true. OF MDIO busses have
> support for a GPIO used as a reset. In retrospect, that was a bad
> idea. The documented ACPI binding does not have this, and if anybody
> was to suggest adding it, i want to NACK it.
> 
> Having OF code and ACPI code to instantiate the MDIO bus makes it
> clearer they are different things. fwnode gives the impression they
> are the same, which is not true. Hence i don't like fwnode at this
> level.
> 
> Where fwnode makes sense is deeper down in the implementation of the
> bindings, for properties which are the same in both bindings.  Then
> you can use fwnode_ for the shared properties, of_ for the OF only
> properties, and acpi_ for the ACPI only properties.
> 
> So although i like the reduction in duplicated code, i think overall
> it makes the code worse because developers are more likely to wrongly
> understand it.

OK.
Thanks for review.

-- 
With Best Regards,
Andy Shevchenko