[PATCH net-next] net: phy: Allow building mdio-boardinfo into the kernel

[PATCH net-next] net: phy: Allow building mdio-boardinfo into the kernel

[Florian Fainelli]

mdio-boardinfo contains code that is helpful for platforms to register
specific MDIO bus devices independent of how CONFIG_MDIO_DEVICE or
CONFIG_PHYLIB will be selected (modular or built-in). In order to make
that possible, let's do the following:

- descend into drivers/net/phy/ unconditionally

- make mdiobus_setup_mdiodev_from_board_info() take a callback argument
  which allows us not to expose the internal MDIO board info list and
  mutex, yet maintain the logic within the same file

- relocate the code that creates a MDIO device into
  drivers/net/phy/mdio_bus.c

- build mdio-boardinfo.o into the kernel as soon as MDIO_DEVICE is
  defined (y or m)

Fixes: 90eff9096c01 ("net: phy: Allow splitting MDIO bus/device support from PHYs")
Fixes: 648ea0134069 ("net: phy: Allow pre-declaration of MDIO devices")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/Makefile             |  2 +-
 drivers/net/phy/Makefile         |  6 +++++-
 drivers/net/phy/mdio-boardinfo.c | 21 +++++++--------------
 drivers/net/phy/mdio-boardinfo.h |  5 ++++-
 drivers/net/phy/mdio_bus.c       | 32 +++++++++++++++++++++++++++++++-
 5 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 55f75aea283c..57fc47ad5ab3 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -18,7 +18,7 @@ obj-$(CONFIG_MII) += mii.o
 obj-$(CONFIG_MDIO) += mdio.o
 obj-$(CONFIG_NET) += Space.o loopback.o
 obj-$(CONFIG_NETCONSOLE) += netconsole.o
-obj-$(CONFIG_MDIO_DEVICE) += phy/
+obj-y += phy/
 obj-$(CONFIG_RIONET) += rionet.o
 obj-$(CONFIG_NET_TEAM) += team/
 obj-$(CONFIG_TUN) += tun.o
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 0e1ec0438c23..e36db9a2ba38 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -1,7 +1,11 @@
 # Makefile for Linux PHY drivers and MDIO bus drivers
 
 libphy-y			:= phy.o phy-core.o phy_device.o
-mdio-bus-y			+= mdio_bus.o mdio_device.o mdio-boardinfo.o
+mdio-bus-y			+= mdio_bus.o mdio_device.o
+
+ifdef CONFIG_MDIO_DEVICE
+obj-y				+= mdio-boardinfo.o
+endif
 
 # PHYLIB implies MDIO_DEVICE, in that case, we have a bunch of circular
 # dependencies that does not make it possible to split mdio-bus objects into a
diff --git a/drivers/net/phy/mdio-boardinfo.c b/drivers/net/phy/mdio-boardinfo.c
index 61941e29daae..1861f387820d 100644
--- a/drivers/net/phy/mdio-boardinfo.c
+++ b/drivers/net/phy/mdio-boardinfo.c
@@ -24,10 +24,12 @@ static DEFINE_MUTEX(mdio_board_lock);
  * @mdiodev: MDIO device pointer
  * Context: can sleep
  */
-void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus)
+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_device *mdiodev;
 	struct mdio_board_info *bi;
 	int ret;
 
@@ -38,23 +40,14 @@ void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus)
 		if (strcmp(bus->id, bi->bus_id))
 			continue;
 
-		mdiodev = mdio_device_create(bus, bi->mdio_addr);
-		if (IS_ERR(mdiodev))
+		ret = cb(bus, bi);
+		if (ret)
 			continue;
 
-		strncpy(mdiodev->modalias, bi->modalias,
-			sizeof(mdiodev->modalias));
-		mdiodev->bus_match = mdio_device_bus_match;
-		mdiodev->dev.platform_data = (void *)bi->platform_data;
-
-		ret = mdio_device_register(mdiodev);
-		if (ret) {
-			mdio_device_free(mdiodev);
-			continue;
-		}
 	}
 	mutex_unlock(&mdio_board_lock);
 }
+EXPORT_SYMBOL(mdiobus_setup_mdiodev_from_board_info);
 
 /**
  * mdio_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 00f98163e90e..3a7f143904e8 100644
--- a/drivers/net/phy/mdio-boardinfo.h
+++ b/drivers/net/phy/mdio-boardinfo.h
@@ -14,6 +14,9 @@ struct mdio_board_entry {
 	struct mdio_board_info	board_info;
 };
 
-void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus);
+void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus,
+					   int (*cb)
+					   (struct mii_bus *bus,
+					    struct mdio_board_info *bi));
 
 #endif /* __MDIO_BOARD_INFO_H */
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 46b468eb6e12..5a214f3b8671 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -290,6 +290,36 @@ static inline void of_mdiobus_link_mdiodev(struct mii_bus *mdio,
 #endif
 
 /**
+ * mdiobus_create_device_from_board_info - create a full MDIO device given
+ * a mdio_board_info structure
+ * @bus: MDIO bus to create the devices on
+ * @bi: mdio_board_info structure describing the devices
+ *
+ * Returns 0 on success or < 0 on error.
+ */
+static int mdiobus_create_device(struct mii_bus *bus,
+				 struct mdio_board_info *bi)
+{
+	struct mdio_device *mdiodev;
+	int ret = 0;
+
+	mdiodev = mdio_device_create(bus, bi->mdio_addr);
+	if (IS_ERR(mdiodev))
+		return -ENODEV;
+
+	strncpy(mdiodev->modalias, bi->modalias,
+		sizeof(mdiodev->modalias));
+	mdiodev->bus_match = mdio_device_bus_match;
+	mdiodev->dev.platform_data = (void *)bi->platform_data;
+
+	ret = mdio_device_register(mdiodev);
+	if (ret)
+		mdio_device_free(mdiodev);
+
+	return ret;
+}
+
+/**
  * __mdiobus_register - bring up all the PHYs on a given bus and attach them to bus
  * @bus: target mii_bus
  * @owner: module containing bus accessor functions
@@ -345,7 +375,7 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
 		}
 	}
 
-	mdiobus_setup_mdiodev_from_board_info(bus);
+	mdiobus_setup_mdiodev_from_board_info(bus, mdiobus_create_device);
 
 	bus->state = MDIOBUS_REGISTERED;
 	pr_info("%s: probed\n", bus->name);
-- 
2.9.3

Re: [PATCH net-next] net: phy: Allow building mdio-boardinfo into the kernel

[Arnd Bergmann]

On Tue, Mar 28, 2017 at 9:57 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> mdio-boardinfo contains code that is helpful for platforms to register
> specific MDIO bus devices independent of how CONFIG_MDIO_DEVICE or
> CONFIG_PHYLIB will be selected (modular or built-in). In order to make
> that possible, let's do the following:
>
> - descend into drivers/net/phy/ unconditionally
>
> - make mdiobus_setup_mdiodev_from_board_info() take a callback argument
>   which allows us not to expose the internal MDIO board info list and
>   mutex, yet maintain the logic within the same file
>
> - relocate the code that creates a MDIO device into
>   drivers/net/phy/mdio_bus.c
>
> - build mdio-boardinfo.o into the kernel as soon as MDIO_DEVICE is
>   defined (y or m)
>
> Fixes: 90eff9096c01 ("net: phy: Allow splitting MDIO bus/device support from PHYs")
> Fixes: 648ea0134069 ("net: phy: Allow pre-declaration of MDIO devices")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Looks good at first glance. I've added this to my randconfig build setup,
let's see if there are any remaining corner cases I can hit.

        Arnd

Re: [PATCH net-next] net: phy: Allow building mdio-boardinfo into the kernel

[Arnd Bergmann]

On Tue, Mar 28, 2017 at 9:57 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> mdio-boardinfo contains code that is helpful for platforms to register
> specific MDIO bus devices independent of how CONFIG_MDIO_DEVICE or
> CONFIG_PHYLIB will be selected (modular or built-in). In order to make
> that possible, let's do the following:
>
> - descend into drivers/net/phy/ unconditionally
>
> - make mdiobus_setup_mdiodev_from_board_info() take a callback argument
>   which allows us not to expose the internal MDIO board info list and
>   mutex, yet maintain the logic within the same file
>
> - relocate the code that creates a MDIO device into
>   drivers/net/phy/mdio_bus.c
>
> - build mdio-boardinfo.o into the kernel as soon as MDIO_DEVICE is
>   defined (y or m)
>
> Fixes: 90eff9096c01 ("net: phy: Allow splitting MDIO bus/device support from PHYs")
> Fixes: 648ea0134069 ("net: phy: Allow pre-declaration of MDIO devices")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

It survived the overnight randconfig build,

Tested-by: Arnd Bergmann <arnd@arndb.de>

On a related note, I ran into one more case of a network driver selecting a
particular PHY:

drivers/net/built-in.o: In function `octeon_mdiobus_remove':
wilink_platform_data.c:(.text+0xe58): undefined reference to
`mdiobus_unregister'
wilink_platform_data.c:(.text+0xe60): undefined reference to `mdiobus_free'
drivers/net/built-in.o: In function `octeon_mdiobus_probe':
wilink_platform_data.c:(.text+0xec8): undefined reference to
`devm_mdiobus_alloc_size'
wilink_platform_data.c:(.text+0x1090): undefined reference to
`of_mdiobus_register'
wilink_platform_data.c:(.text+0x10d0): undefined reference to `mdiobus_free'

Building with this hack fixes the three instances I found so far, but my
current workaround seems rather fragile:

@@ -28,7 +28,7 @@ config MDIO_BCM_UNIMAC

 config MDIO_BITBANG
        tristate "Bitbanged MDIO buses"
-       depends on !(MDIO_DEVICE=y && PHYLIB=m)
+       depends on m || !(MDIO_DEVICE=y && PHYLIB=m)
        help
          This module implements the MDIO bus protocol in software,
          for use by low level drivers that export the ability to
@@ -118,6 +118,7 @@ config MDIO_OCTEON
 config MDIO_SUN4I
        tristate "Allwinner sun4i MDIO interface support"
        depends on ARCH_SUNXI
+       depends on m || !(MDIO_DEVICE=y && PHYLIB=m)
        help
          This driver supports the MDIO interface found in the network
          interface units of the Allwinner SoC that have an EMAC (A10,
@@ -109,6 +109,7 @@ config MDIO_OCTEON
        tristate "Octeon and some ThunderX SOCs MDIO buses"
        depends on 64BIT
        depends on HAS_IOMEM
+       depends on m || !(MDIO_DEVICE=y && PHYLIB=m)
        select MDIO_CAVIUM
        help
          This module provides a driver for the Octeon and ThunderX MDIO

The configuration causing it is something like this:

CONFIG_MDIO_OCTEON=y
CONFIG_MDIO_DEVICE=y
CONFIG_PHYLIB=m

This is what I'm trying now:

--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -7,7 +7,16 @@ menuconfig MDIO_DEVICE
        help
           MDIO devices and driver infrastructure code.

-if MDIO_DEVICE
+config MDIO_BUS
+       tristate
+       default m if PHYLIB=m
+       default MDIO_DEVICE
+       help
+         This internal symbol is used for link time dependencies and it
+         reflects whether the mdio_bus/mdio_device code is built as a
+         loadable module or built-in.
+
+if MDIO_BUS

 config MDIO_BCM_IPROC
        tristate "Broadcom iProc MDIO bus controller"
@@ -28,7 +37,6 @@ config MDIO_BCM_UNIMAC

 config MDIO_BITBANG
        tristate "Bitbanged MDIO buses"
-       depends on m || !(MDIO_DEVICE=y && PHYLIB=m)
        help
          This module implements the MDIO bus protocol in software,
          for use by low level drivers that export the ability to
@@ -109,7 +117,6 @@ config MDIO_OCTEON
        tristate "Octeon and some ThunderX SOCs MDIO buses"
        depends on 64BIT
        depends on HAS_IOMEM
-       depends on m || !(MDIO_DEVICE=y && PHYLIB=m)
        select MDIO_CAVIUM
        help
          This module provides a driver for the Octeon and ThunderX MDIO
@@ -119,7 +126,6 @@ config MDIO_OCTEON
 config MDIO_SUN4I
        tristate "Allwinner sun4i MDIO interface support"
        depends on ARCH_SUNXI
-       depends on m || !(MDIO_DEVICE=y && PHYLIB=m)
        help
          This driver supports the MDIO interface found in the network
          interface units of the Allwinner SoC that have an EMAC (A10,


    Arnd

Re: [PATCH net-next] net: phy: Allow building mdio-boardinfo into the kernel

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 28 Mar 2017 12:57:09 -0700

> mdio-boardinfo contains code that is helpful for platforms to register
> specific MDIO bus devices independent of how CONFIG_MDIO_DEVICE or
> CONFIG_PHYLIB will be selected (modular or built-in). In order to make
> that possible, let's do the following:
> 
> - descend into drivers/net/phy/ unconditionally
> 
> - make mdiobus_setup_mdiodev_from_board_info() take a callback argument
>   which allows us not to expose the internal MDIO board info list and
>   mutex, yet maintain the logic within the same file
> 
> - relocate the code that creates a MDIO device into
>   drivers/net/phy/mdio_bus.c
> 
> - build mdio-boardinfo.o into the kernel as soon as MDIO_DEVICE is
>   defined (y or m)
> 
> Fixes: 90eff9096c01 ("net: phy: Allow splitting MDIO bus/device support from PHYs")
> Fixes: 648ea0134069 ("net: phy: Allow pre-declaration of MDIO devices")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Applied, thanks.

Re: [PATCH net-next] net: phy: Allow building mdio-boardinfo into the kernel

[Florian Fainelli]

On 03/29/2017 01:09 AM, Arnd Bergmann wrote:
> On Tue, Mar 28, 2017 at 9:57 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> mdio-boardinfo contains code that is helpful for platforms to register
>> specific MDIO bus devices independent of how CONFIG_MDIO_DEVICE or
>> CONFIG_PHYLIB will be selected (modular or built-in). In order to make
>> that possible, let's do the following:
>>
>> - descend into drivers/net/phy/ unconditionally
>>
>> - make mdiobus_setup_mdiodev_from_board_info() take a callback argument
>>   which allows us not to expose the internal MDIO board info list and
>>   mutex, yet maintain the logic within the same file
>>
>> - relocate the code that creates a MDIO device into
>>   drivers/net/phy/mdio_bus.c
>>
>> - build mdio-boardinfo.o into the kernel as soon as MDIO_DEVICE is
>>   defined (y or m)
>>
>> Fixes: 90eff9096c01 ("net: phy: Allow splitting MDIO bus/device support from PHYs")
>> Fixes: 648ea0134069 ("net: phy: Allow pre-declaration of MDIO devices")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> It survived the overnight randconfig build,
> 
> Tested-by: Arnd Bergmann <arnd@arndb.de>
> 
> On a related note, I ran into one more case of a network driver selecting a
> particular PHY:
> 
> drivers/net/built-in.o: In function `octeon_mdiobus_remove':
> wilink_platform_data.c:(.text+0xe58): undefined reference to
> `mdiobus_unregister'
> wilink_platform_data.c:(.text+0xe60): undefined reference to `mdiobus_free'
> drivers/net/built-in.o: In function `octeon_mdiobus_probe':
> wilink_platform_data.c:(.text+0xec8): undefined reference to
> `devm_mdiobus_alloc_size'
> wilink_platform_data.c:(.text+0x1090): undefined reference to
> `of_mdiobus_register'
> wilink_platform_data.c:(.text+0x10d0): undefined reference to `mdiobus_free'
> 
> Building with this hack fixes the three instances I found so far, but my
> current workaround seems rather fragile:
> 
> @@ -28,7 +28,7 @@ config MDIO_BCM_UNIMAC
> 
>  config MDIO_BITBANG
>         tristate "Bitbanged MDIO buses"
> -       depends on !(MDIO_DEVICE=y && PHYLIB=m)
> +       depends on m || !(MDIO_DEVICE=y && PHYLIB=m)
>         help
>           This module implements the MDIO bus protocol in software,
>           for use by low level drivers that export the ability to
> @@ -118,6 +118,7 @@ config MDIO_OCTEON
>  config MDIO_SUN4I
>         tristate "Allwinner sun4i MDIO interface support"
>         depends on ARCH_SUNXI
> +       depends on m || !(MDIO_DEVICE=y && PHYLIB=m)
>         help
>           This driver supports the MDIO interface found in the network
>           interface units of the Allwinner SoC that have an EMAC (A10,
> @@ -109,6 +109,7 @@ config MDIO_OCTEON
>         tristate "Octeon and some ThunderX SOCs MDIO buses"
>         depends on 64BIT
>         depends on HAS_IOMEM
> +       depends on m || !(MDIO_DEVICE=y && PHYLIB=m)
>         select MDIO_CAVIUM
>         help
>           This module provides a driver for the Octeon and ThunderX MDIO
> 
> The configuration causing it is something like this:
> 
> CONFIG_MDIO_OCTEON=y
> CONFIG_MDIO_DEVICE=y
> CONFIG_PHYLIB=m
> 
> This is what I'm trying now:

This change below looks a lot more scalable, thanks a lot for running
this through your randconfig tests.

> 
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -7,7 +7,16 @@ menuconfig MDIO_DEVICE
>         help
>            MDIO devices and driver infrastructure code.
> 
> -if MDIO_DEVICE
> +config MDIO_BUS
> +       tristate
> +       default m if PHYLIB=m
> +       default MDIO_DEVICE
> +       help
> +         This internal symbol is used for link time dependencies and it
> +         reflects whether the mdio_bus/mdio_device code is built as a
> +         loadable module or built-in.
> +
> +if MDIO_BUS
> 
>  config MDIO_BCM_IPROC
>         tristate "Broadcom iProc MDIO bus controller"
> @@ -28,7 +37,6 @@ config MDIO_BCM_UNIMAC
> 
>  config MDIO_BITBANG
>         tristate "Bitbanged MDIO buses"
> -       depends on m || !(MDIO_DEVICE=y && PHYLIB=m)
>         help
>           This module implements the MDIO bus protocol in software,
>           for use by low level drivers that export the ability to
> @@ -109,7 +117,6 @@ config MDIO_OCTEON
>         tristate "Octeon and some ThunderX SOCs MDIO buses"
>         depends on 64BIT
>         depends on HAS_IOMEM
> -       depends on m || !(MDIO_DEVICE=y && PHYLIB=m)
>         select MDIO_CAVIUM
>         help
>           This module provides a driver for the Octeon and ThunderX MDIO
> @@ -119,7 +126,6 @@ config MDIO_OCTEON
>  config MDIO_SUN4I
>         tristate "Allwinner sun4i MDIO interface support"
>         depends on ARCH_SUNXI
> -       depends on m || !(MDIO_DEVICE=y && PHYLIB=m)
>         help
>           This driver supports the MDIO interface found in the network
>           interface units of the Allwinner SoC that have an EMAC (A10,
> 
> 
>     Arnd
> 


-- 
Florian