[PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Arnd Bergmann]

The newly introduced mdiobus_register_board_info() function is only available
as part of PHYLIB, so we get a link error when we call that from a board while
phylib is disabled:

arch/arm/plat-orion/common.o: In function `orion_ge00_switch_init':
common.c:(.init.text+0x6a4): undefined reference to `mdiobus_register_board_info'

This adds a workaround that is made up of three parts:

- in plat-orion, the function for declaring the switch is hidden without
  PHYLIB.
- in mach-orion5x, the caller conditionally stubs out the call to
  the removed function, so we can still build other orion5x boards
  without PHYLIB
- For the boards that actually declare the switch, we select PHYLIB
  explicitly from Kconfig if NETDEVICES is set. Without NETDEVICES,
  we cannot enable PHYLIB, but we also wouldn't need it.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mach-orion5x/Kconfig  | 5 +++++
 arch/arm/mach-orion5x/common.c | 4 ++--
 arch/arm/plat-orion/common.c   | 2 ++
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-orion5x/Kconfig b/arch/arm/mach-orion5x/Kconfig
index 468b8cb7fd5f..b98a0057ef66 100644
--- a/arch/arm/mach-orion5x/Kconfig
+++ b/arch/arm/mach-orion5x/Kconfig
@@ -107,6 +107,7 @@ config MACH_TS409
 
 config MACH_WRT350N_V2
 	bool "Linksys WRT350N v2"
+	select PHYLIB if NETDEVICES
 	help
 	  Say 'Y' here if you want your kernel to support the
 	  Linksys WRT350N v2 platform.
@@ -146,24 +147,28 @@ config MACH_MSS2_DT
 
 config MACH_WNR854T
 	bool "Netgear WNR854T"
+	select PHYLIB if NETDEVICES
 	help
 	  Say 'Y' here if you want your kernel to support the
 	  Netgear WNR854T platform.
 
 config MACH_RD88F5181L_GE
 	bool "Marvell Orion-VoIP GE Reference Design"
+	select PHYLIB if NETDEVICES
 	help
 	  Say 'Y' here if you want your kernel to support the
 	  Marvell Orion-VoIP GE (88F5181L) RD.
 
 config MACH_RD88F5181L_FXO
 	bool "Marvell Orion-VoIP FXO Reference Design"
+	select PHYLIB if NETDEVICES
 	help
 	  Say 'Y' here if you want your kernel to support the
 	  Marvell Orion-VoIP FXO (88F5181L) RD.
 
 config MACH_RD88F6183AP_GE
 	bool "Marvell Orion-1-90 AP GE Reference Design"
+	select PHYLIB if NETDEVICES
 	help
 	  Say 'Y' here if you want your kernel to support the
 	  Marvell Orion-1-90 (88F6183) AP GE RD.
diff --git a/arch/arm/mach-orion5x/common.c b/arch/arm/mach-orion5x/common.c
index 83a7ec4c16d0..e712c5adaf87 100644
--- a/arch/arm/mach-orion5x/common.c
+++ b/arch/arm/mach-orion5x/common.c
@@ -107,10 +107,10 @@ void __init orion5x_eth_init(struct mv643xx_eth_platform_data *eth_data)
  ****************************************************************************/
 void __init orion5x_eth_switch_init(struct dsa_chip_data *d)
 {
-	orion_ge00_switch_init(d);
+	if (IS_BUILTIN(CONFIG_PHYLIB))
+		orion_ge00_switch_init(d);
 }
 
-
 /*****************************************************************************
  * I2C
  ****************************************************************************/
diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index 9255b6d67ba5..ab5af56f9b9d 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -471,6 +471,7 @@ void __init orion_ge11_init(struct mv643xx_eth_platform_data *eth_data,
 /*****************************************************************************
  * Ethernet switch
  ****************************************************************************/
+#if IS_BUILTIN(CONFIG_PHYLIB)
 static __initconst const char *orion_ge00_mvmdio_bus_name = "orion-mii";
 static __initdata struct mdio_board_info
 		  orion_ge00_switch_board_info;
@@ -493,6 +494,7 @@ void __init orion_ge00_switch_init(struct dsa_chip_data *d)
 
 	mdiobus_register_board_info(&orion_ge00_switch_board_info, 1);
 }
+#endif
 
 /*****************************************************************************
  * I2C
-- 
2.9.0

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Andrew Lunn]

On Thu, Feb 09, 2017 at 04:08:11PM +0100, Arnd Bergmann wrote:
> The newly introduced mdiobus_register_board_info() function is only available
> as part of PHYLIB, so we get a link error when we call that from a board while
> phylib is disabled:
> 
> arch/arm/plat-orion/common.o: In function `orion_ge00_switch_init':
> common.c:(.init.text+0x6a4): undefined reference to `mdiobus_register_board_info'
> 
> This adds a workaround that is made up of three parts:
> 
> - in plat-orion, the function for declaring the switch is hidden without
>   PHYLIB.
> - in mach-orion5x, the caller conditionally stubs out the call to
>   the removed function, so we can still build other orion5x boards
>   without PHYLIB
> - For the boards that actually declare the switch, we select PHYLIB
>   explicitly from Kconfig if NETDEVICES is set. Without NETDEVICES,
>   we cannot enable PHYLIB, but we also wouldn't need it.

Hi Arnd

Although all correct, would it not be simpler to just select PHYLIB
and NETDEVICES? These devices are all NAS boxes and WiFi access
points. What sense does it make to build a kernel without working
networking for these classes of devices?

	   Andrew

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Arnd Bergmann]

On Thursday, February 9, 2017 4:57:51 PM CET Andrew Lunn wrote:
> On Thu, Feb 09, 2017 at 04:08:11PM +0100, Arnd Bergmann wrote:
> > The newly introduced mdiobus_register_board_info() function is only available
> > as part of PHYLIB, so we get a link error when we call that from a board while
> > phylib is disabled:
> > 
> > arch/arm/plat-orion/common.o: In function `orion_ge00_switch_init':
> > common.c:(.init.text+0x6a4): undefined reference to `mdiobus_register_board_info'
> > 
> > This adds a workaround that is made up of three parts:
> > 
> > - in plat-orion, the function for declaring the switch is hidden without
> >   PHYLIB.
> > - in mach-orion5x, the caller conditionally stubs out the call to
> >   the removed function, so we can still build other orion5x boards
> >   without PHYLIB
> > - For the boards that actually declare the switch, we select PHYLIB
> >   explicitly from Kconfig if NETDEVICES is set. Without NETDEVICES,
> >   we cannot enable PHYLIB, but we also wouldn't need it.
> 
> Hi Arnd
> 
> Although all correct, would it not be simpler to just select PHYLIB
> and NETDEVICES? These devices are all NAS boxes and WiFi access
> points. What sense does it make to build a kernel without working
> networking for these classes of devices?

Adding a 'select' statement to something as broad as NETDEVICES sounds
really bad, it has a significant risk of introducing dependency loops
and may be confusing if you want to build a multiplatform config without
networking support (note that NETDEVICES in turn depends on NET, which
can also be disabled).

One possibility would be to have a special Kconfig symbol that controls
mdiobus_register_board_info() being present and have that symbol
force PHYLIB to never be "=m". Then we can either have no networking
support and no phylib, turning mdiobus_register_board_info() into a
stub, or we have the function built-in and reachable from the board
code.

	Arnd

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Andrew Lunn]

> Adding a 'select' statement to something as broad as NETDEVICES sounds
> really bad, it has a significant risk of introducing dependency loops
> and may be confusing if you want to build a multiplatform config without
> networking support (note that NETDEVICES in turn depends on NET, which
> can also be disabled).

O.K, so overall it is not simple. So lets drop my idea.
 
> One possibility would be to have a special Kconfig symbol that controls
> mdiobus_register_board_info() being present and have that symbol
> force PHYLIB to never be "=m". Then we can either have no networking
> support and no phylib, turning mdiobus_register_board_info() into a
> stub, or we have the function built-in and reachable from the board
> code.

FYI: Florian is working on splitting MDIO out of PHYLIB. There will be
two separate symbols, so it will be possible to have MDIO without
PHYLIB. When this happens, i expect mdiobus_register_board_info() will
be in the MDIO part, not PHYLIB.

   Andrew

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Andrew Lunn]

On Thu, Feb 09, 2017 at 04:08:11PM +0100, Arnd Bergmann wrote:
> The newly introduced mdiobus_register_board_info() function is only available
> as part of PHYLIB, so we get a link error when we call that from a board while
> phylib is disabled:
> 
> arch/arm/plat-orion/common.o: In function `orion_ge00_switch_init':
> common.c:(.init.text+0x6a4): undefined reference to `mdiobus_register_board_info'
> 
> This adds a workaround that is made up of three parts:
> 
> - in plat-orion, the function for declaring the switch is hidden without
>   PHYLIB.
> - in mach-orion5x, the caller conditionally stubs out the call to
>   the removed function, so we can still build other orion5x boards
>   without PHYLIB
> - For the boards that actually declare the switch, we select PHYLIB
>   explicitly from Kconfig if NETDEVICES is set. Without NETDEVICES,
>   we cannot enable PHYLIB, but we also wouldn't need it.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Florian Fainelli]

On 02/09/2017 09:20 AM, Andrew Lunn wrote:
>> Adding a 'select' statement to something as broad as NETDEVICES sounds
>> really bad, it has a significant risk of introducing dependency loops
>> and may be confusing if you want to build a multiplatform config without
>> networking support (note that NETDEVICES in turn depends on NET, which
>> can also be disabled).
> 
> O.K, so overall it is not simple. So lets drop my idea.
>  
>> One possibility would be to have a special Kconfig symbol that controls
>> mdiobus_register_board_info() being present and have that symbol
>> force PHYLIB to never be "=m". Then we can either have no networking
>> support and no phylib, turning mdiobus_register_board_info() into a
>> stub, or we have the function built-in and reachable from the board
>> code.
> 
> FYI: Florian is working on splitting MDIO out of PHYLIB. There will be
> two separate symbols, so it will be possible to have MDIO without
> PHYLIB. When this happens, i expect mdiobus_register_board_info() will
> be in the MDIO part, not PHYLIB.

Yes that would be the plan.
-- 
Florian

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Florian Fainelli]

On 02/09/2017 07:08 AM, Arnd Bergmann wrote:
> The newly introduced mdiobus_register_board_info() function is only available
> as part of PHYLIB, so we get a link error when we call that from a board while
> phylib is disabled:
> 
> arch/arm/plat-orion/common.o: In function `orion_ge00_switch_init':
> common.c:(.init.text+0x6a4): undefined reference to `mdiobus_register_board_info'

There is an empty stub provided in include/linux/phy.h when
CONFIG_PHYLIB is disabled, I am not clear why this did not work here?

I disabled CONFIG_NETDEVICES to force CONFIG_PHY not to be set here, and
I was not able to reproduce this, what am I missing?

> 
> This adds a workaround that is made up of three parts:
> 
> - in plat-orion, the function for declaring the switch is hidden without
>   PHYLIB.
> - in mach-orion5x, the caller conditionally stubs out the call to
>   the removed function, so we can still build other orion5x boards
>   without PHYLIB
> - For the boards that actually declare the switch, we select PHYLIB
>   explicitly from Kconfig if NETDEVICES is set. Without NETDEVICES,
>   we cannot enable PHYLIB, but we also wouldn't need it.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/mach-orion5x/Kconfig  | 5 +++++
>  arch/arm/mach-orion5x/common.c | 4 ++--
>  arch/arm/plat-orion/common.c   | 2 ++
>  3 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-orion5x/Kconfig b/arch/arm/mach-orion5x/Kconfig
> index 468b8cb7fd5f..b98a0057ef66 100644
> --- a/arch/arm/mach-orion5x/Kconfig
> +++ b/arch/arm/mach-orion5x/Kconfig
> @@ -107,6 +107,7 @@ config MACH_TS409
>  
>  config MACH_WRT350N_V2
>  	bool "Linksys WRT350N v2"
> +	select PHYLIB if NETDEVICES
>  	help
>  	  Say 'Y' here if you want your kernel to support the
>  	  Linksys WRT350N v2 platform.
> @@ -146,24 +147,28 @@ config MACH_MSS2_DT
>  
>  config MACH_WNR854T
>  	bool "Netgear WNR854T"
> +	select PHYLIB if NETDEVICES
>  	help
>  	  Say 'Y' here if you want your kernel to support the
>  	  Netgear WNR854T platform.
>  
>  config MACH_RD88F5181L_GE
>  	bool "Marvell Orion-VoIP GE Reference Design"
> +	select PHYLIB if NETDEVICES
>  	help
>  	  Say 'Y' here if you want your kernel to support the
>  	  Marvell Orion-VoIP GE (88F5181L) RD.
>  
>  config MACH_RD88F5181L_FXO
>  	bool "Marvell Orion-VoIP FXO Reference Design"
> +	select PHYLIB if NETDEVICES
>  	help
>  	  Say 'Y' here if you want your kernel to support the
>  	  Marvell Orion-VoIP FXO (88F5181L) RD.
>  
>  config MACH_RD88F6183AP_GE
>  	bool "Marvell Orion-1-90 AP GE Reference Design"
> +	select PHYLIB if NETDEVICES
>  	help
>  	  Say 'Y' here if you want your kernel to support the
>  	  Marvell Orion-1-90 (88F6183) AP GE RD.
> diff --git a/arch/arm/mach-orion5x/common.c b/arch/arm/mach-orion5x/common.c
> index 83a7ec4c16d0..e712c5adaf87 100644
> --- a/arch/arm/mach-orion5x/common.c
> +++ b/arch/arm/mach-orion5x/common.c
> @@ -107,10 +107,10 @@ void __init orion5x_eth_init(struct mv643xx_eth_platform_data *eth_data)
>   ****************************************************************************/
>  void __init orion5x_eth_switch_init(struct dsa_chip_data *d)
>  {
> -	orion_ge00_switch_init(d);
> +	if (IS_BUILTIN(CONFIG_PHYLIB))
> +		orion_ge00_switch_init(d);
>  }
>  
> -
>  /*****************************************************************************
>   * I2C
>   ****************************************************************************/
> diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
> index 9255b6d67ba5..ab5af56f9b9d 100644
> --- a/arch/arm/plat-orion/common.c
> +++ b/arch/arm/plat-orion/common.c
> @@ -471,6 +471,7 @@ void __init orion_ge11_init(struct mv643xx_eth_platform_data *eth_data,
>  /*****************************************************************************
>   * Ethernet switch
>   ****************************************************************************/
> +#if IS_BUILTIN(CONFIG_PHYLIB)
>  static __initconst const char *orion_ge00_mvmdio_bus_name = "orion-mii";
>  static __initdata struct mdio_board_info
>  		  orion_ge00_switch_board_info;
> @@ -493,6 +494,7 @@ void __init orion_ge00_switch_init(struct dsa_chip_data *d)
>  
>  	mdiobus_register_board_info(&orion_ge00_switch_board_info, 1);
>  }
> +#endif
>  
>  /*****************************************************************************
>   * I2C
> 


-- 
Florian

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Arnd Bergmann]

On Thu, Feb 9, 2017 at 7:22 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 02/09/2017 07:08 AM, Arnd Bergmann wrote:
> I disabled CONFIG_NETDEVICES to force CONFIG_PHY not to be set here, and
> I was not able to reproduce this, what am I missing?

In the ARMv5 allmodconfig build, this fails because CONFIG_PHY=m, and
we can't call into it. You could use IS_BUILTIN instead of IS_ENABLED in
the header as a oneline workaround, but I think that would be more confusing
to real users that try to use CONFIG_PHY=m without realizing why they lose
access to their switch.

        Arnd

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Florian Fainelli]

On 02/10/2017 12:20 AM, Arnd Bergmann wrote:
> On Thu, Feb 9, 2017 at 7:22 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 02/09/2017 07:08 AM, Arnd Bergmann wrote:
>> I disabled CONFIG_NETDEVICES to force CONFIG_PHY not to be set here, and
>> I was not able to reproduce this, what am I missing?
> 
> In the ARMv5 allmodconfig build, this fails because CONFIG_PHY=m, and
> we can't call into it. You could use IS_BUILTIN instead of IS_ENABLED in
> the header as a oneline workaround, but I think that would be more confusing
> to real users that try to use CONFIG_PHY=m without realizing why they lose
> access to their switch.

I see, this patch should also help fixing this:

http://patchwork.ozlabs.org/patch/726381/

-- 
Florian

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Arnd Bergmann]

On Friday, February 10, 2017 9:42:21 AM CET Florian Fainelli wrote:
> On 02/10/2017 12:20 AM, Arnd Bergmann wrote:
> > On Thu, Feb 9, 2017 at 7:22 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> >> On 02/09/2017 07:08 AM, Arnd Bergmann wrote:
> >> I disabled CONFIG_NETDEVICES to force CONFIG_PHY not to be set here, and
> >> I was not able to reproduce this, what am I missing?
> > 
> > In the ARMv5 allmodconfig build, this fails because CONFIG_PHY=m, and
> > we can't call into it. You could use IS_BUILTIN instead of IS_ENABLED in
> > the header as a oneline workaround, but I think that would be more confusing
> > to real users that try to use CONFIG_PHY=m without realizing why they lose
> > access to their switch.
> 
> I see, this patch should also help fixing this:
> 
> http://patchwork.ozlabs.org/patch/726381/

I think you still have the same problem, as you can still have the
boardinfo registration in a loadable module.

I have come up with a patch too now and done some randconfig testing
on it (it took me several tries as well), please see below. It does
some of the same things as yours and some others.

The main trick is to have a separate 'MDIO_BOARDINFO' Kconfig symbol
that can be selected regardless of all the other symbols, and that
will lead to the registration being either built-in when it's needed
or not built at all when either no board calls it, or PHYLIB is
disabled.

>From f35e89cacfabdf7b822772013389132605941def Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 27 Apr 2016 11:51:18 +0200
Subject: [PATCH] [RFC] move ethernet PHY config into drivers/phy/Kconfig

Calling mdiobus_register_board_info from builtin code with CONFIG_PHYLIB=m
currently results in a link error:

arch/arm/plat-orion/common.o: In function `orion_ge00_switch_init':
common.c:(.init.text+0x6a4): undefined reference to `mdiobus_register_board_info'

As the long-term strategy is to separate mdio from phylib, and to get generic-phy
and (networking-only) phylib closer together, this performs a first step in that
direction: The Kconfig file for phylib gets logically pulled under the PHY
driver configuration and becomes independent from networking. This lets us
select the new CONFIG_MDIO_BOARDINFO from platforms that need it, and provide
the functions exactly when we need them.

In the same step, we can also split out the MDIO driver configuration from
phylib. This is based on an older experimental patch I had, but it still
requires some code changes in phylib itself to let users actually rely on
MDIO without all of PHYLIB.

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

diff --git a/arch/arm/mach-orion5x/Kconfig b/arch/arm/mach-orion5x/Kconfig
index 468b8cb7fd5f..e1126e1aa3d2 100644
--- a/arch/arm/mach-orion5x/Kconfig
+++ b/arch/arm/mach-orion5x/Kconfig
@@ -4,6 +4,7 @@ menuconfig ARCH_ORION5X
 	select CPU_FEROCEON
 	select GENERIC_CLOCKEVENTS
 	select GPIOLIB
+	select MDIO_BOARDINFO
 	select MVEBU_MBUS
 	select PCI
 	select PLAT_ORION_LEGACY
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index a993cbeb9e0c..9eb15b7518bd 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -378,8 +378,6 @@ config NET_SB1000
 
 	  If you don't have this card, of course say N.
 
-source "drivers/net/phy/Kconfig"
-
 source "drivers/net/plip/Kconfig"
 
 source "drivers/net/ppp/Kconfig"
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 7336cbd3ef5d..3ab87e9f9442 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -17,7 +17,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_PHYLIB) += phy/
+obj-y+= phy/
 obj-$(CONFIG_RIONET) += rionet.o
 obj-$(CONFIG_NET_TEAM) += team/
 obj-$(CONFIG_TUN) += tun.o
diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index 8c08f9deef92..9c4652ae2750 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -11,9 +11,6 @@ menuconfig ETHERNET
 
 if ETHERNET
 
-config MDIO
-	tristate
-
 config SUNGEM_PHY
 	tristate
 
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 8dbd59baa34d..37f5552cc5b3 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -3,8 +3,9 @@
 #
 
 menuconfig PHYLIB
-	tristate "PHY Device support and infrastructure"
+	tristate "Ethernet PHY Device support and infrastructure"
 	depends on NETDEVICES
+	select MDIO
 	help
 	  Ethernet controllers are usually attached to PHY
 	  devices.  This option provides infrastructure for
@@ -248,6 +249,16 @@ config FIXED_PHY
 	  PHYs that are not connected to the real MDIO bus.
 
 	  Currently tested with mpc866ads and mpc8349e-mitx.
+endif # PHYLIB
+
+config MDIO
+	tristate
+	help
+	  The MDIO bus is typically used ethernet PHYs, but can also be
+	  used by other PHY drivers.
+
+menu "MDIO bus drivers"
+	depends on MDIO
 
 config ICPLUS_PHY
 	tristate "ICPlus PHYs"
@@ -340,7 +351,10 @@ config XILINX_GMII2RGMII
          the Reduced Gigabit Media Independent Interface(RGMII) between
          Ethernet physical media devices and the Gigabit Ethernet controller.
 
-endif # PHYLIB
+endmenu # MDIO
+
+config MDIO_BOARDINFO
+	bool
 
 config MICREL_KS8995MA
 	tristate "Micrel KS8995MA 5-ports 10/100 managed Ethernet switch"
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 407b0b601ea8..e60e5d2fae53 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -1,11 +1,13 @@
 # Makefile for Linux PHY drivers and MDIO bus drivers
 
-libphy-y			:= phy.o phy_device.o mdio_bus.o mdio_device.o \
-				   mdio-boardinfo.o
+libphy-y			:= phy.o phy_device.o mdio_bus.o mdio_device.o
 libphy-$(CONFIG_SWPHY)		+= swphy.o
 libphy-$(CONFIG_LED_TRIGGER_PHY)	+= phy_led_triggers.o
 
 obj-$(CONFIG_PHYLIB)		+= libphy.o
+ifdef CONFIG_PHYLIB
+obj-$(CONFIG_MDIO_BOARDINFO)	+= mdio-boardinfo.o
+endif
 
 obj-$(CONFIG_MDIO_BCM_IPROC)	+= mdio-bcm-iproc.o
 obj-$(CONFIG_MDIO_BCM_UNIMAC)	+= mdio-bcm-unimac.o
diff --git a/drivers/net/phy/mdio-boardinfo.c b/drivers/net/phy/mdio-boardinfo.c
index 6b988f77da08..f7cf98093344 100644
--- a/drivers/net/phy/mdio-boardinfo.c
+++ b/drivers/net/phy/mdio-boardinfo.c
@@ -55,6 +55,7 @@ void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus)
 	}
 	mutex_unlock(&mdio_board_lock);
 }
+EXPORT_SYMBOL_GPL(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..3a72bd978114 100644
--- a/drivers/net/phy/mdio-boardinfo.h
+++ b/drivers/net/phy/mdio-boardinfo.h
@@ -14,6 +14,12 @@ struct mdio_board_entry {
 	struct mdio_board_info	board_info;
 };
 
+#ifdef CONFIG_MDIO_BOARDINFO
 void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus);
+#else
+static inline void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus)
+{
+}
+#endif
 
 #endif /* __MDIO_BOARD_INFO_H */
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 519241bc8817..87c581f11297 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -2,7 +2,9 @@
 # PHY
 #
 
-menu "PHY Subsystem"
+menu "PHY drivers"
+
+menu "Generic PHY subsystem"
 
 config GENERIC_PHY
 	bool "PHY Core"
@@ -515,3 +517,7 @@ config PHY_NSP_USB3
 	  Enable this to support the Broadcom Northstar plus USB3 PHY.
 	  If unsure, say N.
 endmenu
+
+source "drivers/net/phy/Kconfig"
+
+endmenu
diff --git a/include/linux/phy.h b/include/linux/phy.h
index d9bdf53e0514..2ec82a4c3b8f 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -893,7 +893,7 @@ struct mdio_board_info {
 	const void	*platform_data;
 };
 
-#if IS_ENABLED(CONFIG_PHYLIB)
+#if IS_ENABLED(CONFIG_PHYLIB) && IS_ENABLED(CONFIG_MDIO_BOARDINFO)
 int mdiobus_register_board_info(const struct mdio_board_info *info,
 				unsigned int n);
 #else

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Florian Fainelli]

On 02/10/2017 12:05 PM, Arnd Bergmann wrote:
> On Friday, February 10, 2017 9:42:21 AM CET Florian Fainelli wrote:
>> On 02/10/2017 12:20 AM, Arnd Bergmann wrote:
>>> On Thu, Feb 9, 2017 at 7:22 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>> On 02/09/2017 07:08 AM, Arnd Bergmann wrote:
>>>> I disabled CONFIG_NETDEVICES to force CONFIG_PHY not to be set here, and
>>>> I was not able to reproduce this, what am I missing?
>>>
>>> In the ARMv5 allmodconfig build, this fails because CONFIG_PHY=m, and
>>> we can't call into it. You could use IS_BUILTIN instead of IS_ENABLED in
>>> the header as a oneline workaround, but I think that would be more confusing
>>> to real users that try to use CONFIG_PHY=m without realizing why they lose
>>> access to their switch.
>>
>> I see, this patch should also help fixing this:
>>
>> http://patchwork.ozlabs.org/patch/726381/
> 
> I think you still have the same problem, as you can still have the
> boardinfo registration in a loadable module.

The patch exports mdiobus_register_board_info() so that should solve
your problem here, and I did verify this with a loadable module that
references mdiobus_register_board_info() in that case.

> 
> I have come up with a patch too now and done some randconfig testing
> on it (it took me several tries as well), please see below. It does
> some of the same things as yours and some others.
> 
> The main trick is to have a separate 'MDIO_BOARDINFO' Kconfig symbol
> that can be selected regardless of all the other symbols, and that
> will lead to the registration being either built-in when it's needed
> or not built at all when either no board calls it, or PHYLIB is
> disabled.

Your patch is fine in premise except that you are making CONFIG_MDIO
encompass both drivers/net/mdio.c and
drivers/net/phy/mdio_{bus,device}.c and these do share the same header
(for better or for worse), but are not quite dealing with MDIO at the
same level. drivers/net/mdio.c is more like PHYLIB for the old-style,
pre mdiobus() drivers helper functions.

I like it that you made MDIO_BOARDINFO separate, and that is probably a
patch I should incorporate in the other patch splitting things up, see
below though for the remainder of the changes.

> 
> From f35e89cacfabdf7b822772013389132605941def Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Wed, 27 Apr 2016 11:51:18 +0200
> Subject: [PATCH] [RFC] move ethernet PHY config into drivers/phy/Kconfig
> 
> Calling mdiobus_register_board_info from builtin code with CONFIG_PHYLIB=m
> currently results in a link error:
> 
> arch/arm/plat-orion/common.o: In function `orion_ge00_switch_init':
> common.c:(.init.text+0x6a4): undefined reference to `mdiobus_register_board_info'
> 
> As the long-term strategy is to separate mdio from phylib, and to get generic-phy
> and (networking-only) phylib closer together, this performs a first step in that
> direction: The Kconfig file for phylib gets logically pulled under the PHY
> driver configuration and becomes independent from networking. This lets us
> select the new CONFIG_MDIO_BOARDINFO from platforms that need it, and provide
> the functions exactly when we need them.

This is too broad, the only part that is worth in drivers/net/phy/ of
pulling out of drivers/net/phy/ is what I tried to extract: mdio bus and
device. There are some bad inter-dependencies between that code and
phy_device.c and phy.c which makes it hard to split and make that part
completely standalone for now.

The only part that is truly valuable to non-Ethernet PHY devices is the
MDIO bus/device registration part, which is available in my patch with
CONFIG_MDIO_DEVICE, and which probably should not depend from
NETDEVICES, so the other part of your patch makes sense too here.

Thanks!


> 
> In the same step, we can also split out the MDIO driver configuration from
> phylib. This is based on an older experimental patch I had, but it still
> requires some code changes in phylib itself to let users actually rely on
> MDIO without all of PHYLIB.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> diff --git a/arch/arm/mach-orion5x/Kconfig b/arch/arm/mach-orion5x/Kconfig
> index 468b8cb7fd5f..e1126e1aa3d2 100644
> --- a/arch/arm/mach-orion5x/Kconfig
> +++ b/arch/arm/mach-orion5x/Kconfig
> @@ -4,6 +4,7 @@ menuconfig ARCH_ORION5X
>  	select CPU_FEROCEON
>  	select GENERIC_CLOCKEVENTS
>  	select GPIOLIB
> +	select MDIO_BOARDINFO
>  	select MVEBU_MBUS
>  	select PCI
>  	select PLAT_ORION_LEGACY
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index a993cbeb9e0c..9eb15b7518bd 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -378,8 +378,6 @@ config NET_SB1000
>  
>  	  If you don't have this card, of course say N.
>  
> -source "drivers/net/phy/Kconfig"
> -
>  source "drivers/net/plip/Kconfig"
>  
>  source "drivers/net/ppp/Kconfig"
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 7336cbd3ef5d..3ab87e9f9442 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -17,7 +17,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_PHYLIB) += phy/
> +obj-y+= phy/
>  obj-$(CONFIG_RIONET) += rionet.o
>  obj-$(CONFIG_NET_TEAM) += team/
>  obj-$(CONFIG_TUN) += tun.o
> diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
> index 8c08f9deef92..9c4652ae2750 100644
> --- a/drivers/net/ethernet/Kconfig
> +++ b/drivers/net/ethernet/Kconfig
> @@ -11,9 +11,6 @@ menuconfig ETHERNET
>  
>  if ETHERNET
>  
> -config MDIO
> -	tristate
> -
>  config SUNGEM_PHY
>  	tristate
>  
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 8dbd59baa34d..37f5552cc5b3 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -3,8 +3,9 @@
>  #
>  
>  menuconfig PHYLIB
> -	tristate "PHY Device support and infrastructure"
> +	tristate "Ethernet PHY Device support and infrastructure"
>  	depends on NETDEVICES
> +	select MDIO
>  	help
>  	  Ethernet controllers are usually attached to PHY
>  	  devices.  This option provides infrastructure for
> @@ -248,6 +249,16 @@ config FIXED_PHY
>  	  PHYs that are not connected to the real MDIO bus.
>  
>  	  Currently tested with mpc866ads and mpc8349e-mitx.
> +endif # PHYLIB
> +
> +config MDIO
> +	tristate
> +	help
> +	  The MDIO bus is typically used ethernet PHYs, but can also be
> +	  used by other PHY drivers.
> +
> +menu "MDIO bus drivers"
> +	depends on MDIO
>  
>  config ICPLUS_PHY
>  	tristate "ICPlus PHYs"
> @@ -340,7 +351,10 @@ config XILINX_GMII2RGMII
>           the Reduced Gigabit Media Independent Interface(RGMII) between
>           Ethernet physical media devices and the Gigabit Ethernet controller.
>  
> -endif # PHYLIB
> +endmenu # MDIO
> +
> +config MDIO_BOARDINFO
> +	bool
>  
>  config MICREL_KS8995MA
>  	tristate "Micrel KS8995MA 5-ports 10/100 managed Ethernet switch"
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 407b0b601ea8..e60e5d2fae53 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -1,11 +1,13 @@
>  # Makefile for Linux PHY drivers and MDIO bus drivers
>  
> -libphy-y			:= phy.o phy_device.o mdio_bus.o mdio_device.o \
> -				   mdio-boardinfo.o
> +libphy-y			:= phy.o phy_device.o mdio_bus.o mdio_device.o
>  libphy-$(CONFIG_SWPHY)		+= swphy.o
>  libphy-$(CONFIG_LED_TRIGGER_PHY)	+= phy_led_triggers.o
>  
>  obj-$(CONFIG_PHYLIB)		+= libphy.o
> +ifdef CONFIG_PHYLIB
> +obj-$(CONFIG_MDIO_BOARDINFO)	+= mdio-boardinfo.o
> +endif
>  
>  obj-$(CONFIG_MDIO_BCM_IPROC)	+= mdio-bcm-iproc.o
>  obj-$(CONFIG_MDIO_BCM_UNIMAC)	+= mdio-bcm-unimac.o
> diff --git a/drivers/net/phy/mdio-boardinfo.c b/drivers/net/phy/mdio-boardinfo.c
> index 6b988f77da08..f7cf98093344 100644
> --- a/drivers/net/phy/mdio-boardinfo.c
> +++ b/drivers/net/phy/mdio-boardinfo.c
> @@ -55,6 +55,7 @@ void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus)
>  	}
>  	mutex_unlock(&mdio_board_lock);
>  }
> +EXPORT_SYMBOL_GPL(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..3a72bd978114 100644
> --- a/drivers/net/phy/mdio-boardinfo.h
> +++ b/drivers/net/phy/mdio-boardinfo.h
> @@ -14,6 +14,12 @@ struct mdio_board_entry {
>  	struct mdio_board_info	board_info;
>  };
>  
> +#ifdef CONFIG_MDIO_BOARDINFO
>  void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus);
> +#else
> +static inline void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus)
> +{
> +}
> +#endif
>  
>  #endif /* __MDIO_BOARD_INFO_H */
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 519241bc8817..87c581f11297 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -2,7 +2,9 @@
>  # PHY
>  #
>  
> -menu "PHY Subsystem"
> +menu "PHY drivers"
> +
> +menu "Generic PHY subsystem"
>  
>  config GENERIC_PHY
>  	bool "PHY Core"
> @@ -515,3 +517,7 @@ config PHY_NSP_USB3
>  	  Enable this to support the Broadcom Northstar plus USB3 PHY.
>  	  If unsure, say N.
>  endmenu
> +
> +source "drivers/net/phy/Kconfig"
> +
> +endmenu
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index d9bdf53e0514..2ec82a4c3b8f 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -893,7 +893,7 @@ struct mdio_board_info {
>  	const void	*platform_data;
>  };
>  
> -#if IS_ENABLED(CONFIG_PHYLIB)
> +#if IS_ENABLED(CONFIG_PHYLIB) && IS_ENABLED(CONFIG_MDIO_BOARDINFO)
>  int mdiobus_register_board_info(const struct mdio_board_info *info,
>  				unsigned int n);
>  #else
> 


-- 
Florian

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Arnd Bergmann]

On Fri, Feb 10, 2017 at 9:57 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 02/10/2017 12:05 PM, Arnd Bergmann wrote:
>> On Friday, February 10, 2017 9:42:21 AM CET Florian Fainelli wrote:
>>> On 02/10/2017 12:20 AM, Arnd Bergmann wrote:
>>>> On Thu, Feb 9, 2017 at 7:22 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>>> On 02/09/2017 07:08 AM, Arnd Bergmann wrote:
>>>>> I disabled CONFIG_NETDEVICES to force CONFIG_PHY not to be set here, and
>>>>> I was not able to reproduce this, what am I missing?
>>>>
>>>> In the ARMv5 allmodconfig build, this fails because CONFIG_PHY=m, and
>>>> we can't call into it. You could use IS_BUILTIN instead of IS_ENABLED in
>>>> the header as a oneline workaround, but I think that would be more confusing
>>>> to real users that try to use CONFIG_PHY=m without realizing why they lose
>>>> access to their switch.
>>>
>>> I see, this patch should also help fixing this:
>>>
>>> http://patchwork.ozlabs.org/patch/726381/
>>
>> I think you still have the same problem, as you can still have the
>> boardinfo registration in a loadable module.
>
> The patch exports mdiobus_register_board_info() so that should solve
> your problem here, and I did verify this with a loadable module that
> references mdiobus_register_board_info() in that case.

No, that's a different problem. What you get with arm allmodconfig
(try it!) is that mdio-bus.ko is a loadable module, but referenced
from built-in code rather than the other way around. Exporting
the symbol doesn't change anything since the module cannot
be loaded by the time we need the symbol.

>>
>> I have come up with a patch too now and done some randconfig testing
>> on it (it took me several tries as well), please see below. It does
>> some of the same things as yours and some others.
>>
>> The main trick is to have a separate 'MDIO_BOARDINFO' Kconfig symbol
>> that can be selected regardless of all the other symbols, and that
>> will lead to the registration being either built-in when it's needed
>> or not built at all when either no board calls it, or PHYLIB is
>> disabled.
>
> Your patch is fine in premise except that you are making CONFIG_MDIO
> encompass both drivers/net/mdio.c and
> drivers/net/phy/mdio_{bus,device}.c and these do share the same header
> (for better or for worse), but are not quite dealing with MDIO at the
> same level. drivers/net/mdio.c is more like PHYLIB for the old-style,
> pre mdiobus() drivers helper functions.

Ah, makes sense. I had missed that part.

> I like it that you made MDIO_BOARDINFO separate, and that is probably a
> patch I should incorporate in the other patch splitting things up, see
> below though for the remainder of the changes.

Ok.

>>
>> From f35e89cacfabdf7b822772013389132605941def Mon Sep 17 00:00:00 2001
>> From: Arnd Bergmann <arnd@arndb.de>
>> Date: Wed, 27 Apr 2016 11:51:18 +0200
>> Subject: [PATCH] [RFC] move ethernet PHY config into drivers/phy/Kconfig
>>
>> Calling mdiobus_register_board_info from builtin code with CONFIG_PHYLIB=m
>> currently results in a link error:
>>
>> arch/arm/plat-orion/common.o: In function `orion_ge00_switch_init':
>> common.c:(.init.text+0x6a4): undefined reference to `mdiobus_register_board_info'
>>
>> As the long-term strategy is to separate mdio from phylib, and to get generic-phy
>> and (networking-only) phylib closer together, this performs a first step in that
>> direction: The Kconfig file for phylib gets logically pulled under the PHY
>> driver configuration and becomes independent from networking. This lets us
>> select the new CONFIG_MDIO_BOARDINFO from platforms that need it, and provide
>> the functions exactly when we need them.
>
> This is too broad, the only part that is worth in drivers/net/phy/ of
> pulling out of drivers/net/phy/ is what I tried to extract: mdio bus and
> device. There are some bad inter-dependencies between that code and
> phy_device.c and phy.c which makes it hard to split and make that part
> completely standalone for now.
>
> The only part that is truly valuable to non-Ethernet PHY devices is the
> MDIO bus/device registration part, which is available in my patch with
> CONFIG_MDIO_DEVICE, and which probably should not depend from
> NETDEVICES, so the other part of your patch makes sense too here.

My patch started out from something I had done a long time ago when
we discussed how the two subsystems (generic-phy and phylib) can
be tied together more. This has two aspects:

- Moving them into a single top-level Kconfig menu (and eventually
  directory) to make it easier to find one of them when you look in
  the wrong place. My patch starts doing that.
- making the MDIO bus available to generic-phy drivers. This is
  what your patch does.

Right now, we only really need part of my patch to fix the link
error, but it makes way more sense once all the parts come together.

    Arnd

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Arnd Bergmann]

On Fri, Feb 10, 2017 at 10:32 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Feb 10, 2017 at 9:57 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 02/10/2017 12:05 PM, Arnd Bergmann wrote:
>>> On Friday, February 10, 2017 9:42:21 AM CET Florian Fainelli wrote:
>>>> On 02/10/2017 12:20 AM, Arnd Bergmann wrote:
>>>>> On Thu, Feb 9, 2017 at 7:22 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>>>> On 02/09/2017 07:08 AM, Arnd Bergmann wrote:
>>>>>> I disabled CONFIG_NETDEVICES to force CONFIG_PHY not to be set here, and
>>>>>> I was not able to reproduce this, what am I missing?
>>>>>
>>>>> In the ARMv5 allmodconfig build, this fails because CONFIG_PHY=m, and
>>>>> we can't call into it. You could use IS_BUILTIN instead of IS_ENABLED in
>>>>> the header as a oneline workaround, but I think that would be more confusing
>>>>> to real users that try to use CONFIG_PHY=m without realizing why they lose
>>>>> access to their switch.
>>>>
>>>> I see, this patch should also help fixing this:
>>>>
>>>> http://patchwork.ozlabs.org/patch/726381/
>>>
>>> I think you still have the same problem, as you can still have the
>>> boardinfo registration in a loadable module.
>>
>> The patch exports mdiobus_register_board_info() so that should solve
>> your problem here, and I did verify this with a loadable module that
>> references mdiobus_register_board_info() in that case.
>
> No, that's a different problem. What you get with arm allmodconfig
> (try it!) is that mdio-bus.ko is a loadable module, but referenced
> from built-in code rather than the other way around. Exporting
> the symbol doesn't change anything since the module cannot
> be loaded by the time we need the symbol.

I got another failure with my patch applied:

drivers/net/phy/mdio-boardinfo.o: In function
`mdiobus_setup_mdiodev_from_board_info':
mdio-boardinfo.c:(.text.mdiobus_setup_mdiodev_from_board_info+0x48):
undefined reference to `mdio_device_create'
mdio-boardinfo.c:(.text.mdiobus_setup_mdiodev_from_board_info+0x78):
undefined reference to `mdio_device_register'
mdio-boardinfo.c:(.text.mdiobus_setup_mdiodev_from_board_info+0x84):
undefined reference to `mdio_device_free'
mdio-boardinfo.c:(.text.mdiobus_setup_mdiodev_from_board_info+0xa8):
undefined reference to `mdio_device_bus_match'

I hadn't realized earlier that the function has to call back into
the mdio layer. This is theoretically fixable too, but it's probably easier
to make all of the MDIO bus layer builtin whenever there is a user of
mdiobus_register_board_info().

      Arnd