[PATCH net-next] net: phy: move mdiobus_setup_mdiodev_from_board_info to mdio_bus_provider.c

[PATCH net-next] net: phy: move mdiobus_setup_mdiodev_from_board_info to mdio_bus_provider.c

[Heiner Kallweit]

Move mdiobus_setup_mdiodev_from_board_info() to mdio_bus_provider.c.
Benefits are:
- The function doesn't have to be exported any longer and can be made
  static.
- We can call mdiobus_create_device() directly instead of passing it
  as a callback.

Only drawback is that now list and mutex have to be exported.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/mdio-boardinfo.c    | 41 +++--------------------------
 drivers/net/phy/mdio-boardinfo.h    |  9 +++----
 drivers/net/phy/mdio_bus_provider.c | 26 +++++++++++++++++-
 3 files changed, 33 insertions(+), 43 deletions(-)

diff --git a/drivers/net/phy/mdio-boardinfo.c b/drivers/net/phy/mdio-boardinfo.c
index 2de679a68..a52eaefe2 100644
--- a/drivers/net/phy/mdio-boardinfo.c
+++ b/drivers/net/phy/mdio-boardinfo.c
@@ -11,43 +11,10 @@

 #include "mdio-boardinfo.h"
 
-static LIST_HEAD(mdio_board_list);
-static DEFINE_MUTEX(mdio_board_lock);
-
-/**
- * mdiobus_setup_mdiodev_from_board_info - create and setup MDIO devices
- * from pre-collected board specific MDIO information
- * @bus: Bus the board_info belongs to
- * @cb: Callback to create device on bus
- * Context: can sleep
- */
-void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus,
-					   int (*cb)
-					   (struct mii_bus *bus,
-					    struct mdio_board_info *bi))
-{
-	struct mdio_board_entry *be;
-	struct mdio_board_entry *tmp;
-	struct mdio_board_info *bi;
-	int ret;
-
-	mutex_lock(&mdio_board_lock);
-	list_for_each_entry_safe(be, tmp, &mdio_board_list, list) {
-		bi = &be->board_info;
-
-		if (strcmp(bus->id, bi->bus_id))
-			continue;
-
-		mutex_unlock(&mdio_board_lock);
-		ret = cb(bus, bi);
-		mutex_lock(&mdio_board_lock);
-		if (ret)
-			continue;
-
-	}
-	mutex_unlock(&mdio_board_lock);
-}
-EXPORT_SYMBOL(mdiobus_setup_mdiodev_from_board_info);
+LIST_HEAD(mdio_board_list);
+EXPORT_SYMBOL_GPL(mdio_board_list);
+DEFINE_MUTEX(mdio_board_lock);
+EXPORT_SYMBOL_GPL(mdio_board_lock);
 
 /**
  * mdiobus_register_board_info - register MDIO devices for a given board
diff --git a/drivers/net/phy/mdio-boardinfo.h b/drivers/net/phy/mdio-boardinfo.h
index 773bb5139..7488e3de2 100644
--- a/drivers/net/phy/mdio-boardinfo.h
+++ b/drivers/net/phy/mdio-boardinfo.h
@@ -7,17 +7,16 @@
 #ifndef __MDIO_BOARD_INFO_H
 #define __MDIO_BOARD_INFO_H
 
-#include <linux/phy.h>
+#include <linux/list.h>
 #include <linux/mutex.h>
+#include <linux/phy.h>
 
 struct mdio_board_entry {
 	struct list_head	list;
 	struct mdio_board_info	board_info;
 };
 
-void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus,
-					   int (*cb)
-					   (struct mii_bus *bus,
-					    struct mdio_board_info *bi));
+extern struct list_head mdio_board_list;
+extern struct mutex mdio_board_lock;
 
 #endif /* __MDIO_BOARD_INFO_H */
diff --git a/drivers/net/phy/mdio_bus_provider.c b/drivers/net/phy/mdio_bus_provider.c
index 48dc4bf85..dd3219969 100644
--- a/drivers/net/phy/mdio_bus_provider.c
+++ b/drivers/net/phy/mdio_bus_provider.c
@@ -161,6 +161,30 @@ static int mdiobus_create_device(struct mii_bus *bus,
 	return ret;
 }
 
+/**
+ * mdiobus_setup_mdiodev_from_board_info - create and setup MDIO devices
+ * from pre-collected board specific MDIO information
+ * @bus: Bus the board_info belongs to
+ * Context: can sleep
+ */
+static void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus)
+{
+	struct mdio_board_entry *be, *tmp;
+
+	mutex_lock(&mdio_board_lock);
+	list_for_each_entry_safe(be, tmp, &mdio_board_list, list) {
+		struct mdio_board_info *bi = &be->board_info;
+
+		if (strcmp(bus->id, bi->bus_id))
+			continue;
+
+		mutex_unlock(&mdio_board_lock);
+		mdiobus_create_device(bus, bi);
+		mutex_lock(&mdio_board_lock);
+	}
+	mutex_unlock(&mdio_board_lock);
+}
+
 static struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr, bool c45)
 {
 	struct phy_device *phydev = ERR_PTR(-ENODEV);
@@ -404,7 +428,7 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
 			goto error;
 	}
 
-	mdiobus_setup_mdiodev_from_board_info(bus, mdiobus_create_device);
+	mdiobus_setup_mdiodev_from_board_info(bus);
 
 	bus->state = MDIOBUS_REGISTERED;
 	dev_dbg(&bus->dev, "probed\n");
-- 
2.49.0

Re: [PATCH net-next] net: phy: move mdiobus_setup_mdiodev_from_board_info to mdio_bus_provider.c

[Paolo Abeni]

On 5/15/25 10:11 PM, Heiner Kallweit wrote:
> Move mdiobus_setup_mdiodev_from_board_info() to mdio_bus_provider.c.
> Benefits are:
> - The function doesn't have to be exported any longer and can be made
>   static.
> - We can call mdiobus_create_device() directly instead of passing it
>   as a callback.
> 
> Only drawback is that now list and mutex have to be exported.

... so the total exports count actually increases, and I personally
think that exporting a function is preferable to exporting a variable.

@Andrew, Russell: WDYT?

Thanks,

Paolo

Re: [PATCH net-next] net: phy: move mdiobus_setup_mdiodev_from_board_info to mdio_bus_provider.c

[Heiner Kallweit]

On Tue, May 20, 2025 at 12:45 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 5/15/25 10:11 PM, Heiner Kallweit wrote:
> > Move mdiobus_setup_mdiodev_from_board_info() to mdio_bus_provider.c.
> > Benefits are:
> > - The function doesn't have to be exported any longer and can be made
> >   static.
> > - We can call mdiobus_create_device() directly instead of passing it
> >   as a callback.
> >
> > Only drawback is that now list and mutex have to be exported.
>
> ... so the total exports count actually increases, and I personally
> think that exporting a function is preferable to exporting a variable.
>
Current call chain is:

__mdio_bus_register()    // in mdio_bus_provider.c (module or built-in)
  mdiobus_setup_mdiodev_from_board_info()   // in mdio-boardinfo.c (built-in)
    mdiobus_create_device()    // in mdio_bus_provider.c, currently
passed to mdiobus_setup_mdiodev_from_board_info as function pointer

Having this call chain in one source file and not having to pass
mdiobus_create_device
as a function pointer outweighs the drawback of having to export list/mutex IMO.
But as always YMMV

> @Andrew, Russell: WDYT?
>
+1

> Thanks,
>
> Paolo
>
Thanks, Heiner

Re: [PATCH net-next] net: phy: move mdiobus_setup_mdiodev_from_board_info to mdio_bus_provider.c

[Heiner Kallweit]

On 20.05.2025 12:45, Paolo Abeni wrote:
> On 5/15/25 10:11 PM, Heiner Kallweit wrote:
>> Move mdiobus_setup_mdiodev_from_board_info() to mdio_bus_provider.c.
>> Benefits are:
>> - The function doesn't have to be exported any longer and can be made
>>   static.
>> - We can call mdiobus_create_device() directly instead of passing it
>>   as a callback.
>>
>> Only drawback is that now list and mutex have to be exported.
> 
> ... so the total exports count actually increases, and I personally
> think that exporting a function is preferable to exporting a variable.
> 
> @Andrew, Russell: WDYT?
> 
> Thanks,
> 
> Paolo
> 
--
pw-bot: cr