Re: [lunn:v6.3-rc2-net-next-phy-leds 5/15] ld.lld: error: undefined symbol: devm_mdiobus_alloc_size

Re: [lunn:v6.3-rc2-net-next-phy-leds 5/15] ld.lld: error: undefined symbol: devm_mdiobus_alloc_size

[Arnd Bergmann]

On Fri, Mar 24, 2023, at 12:36, kernel test robot wrote:
>>> ld.lld: error: undefined symbol: devm_mdiobus_alloc_size
>    >>> referenced by phy.h:458 (include/linux/phy.h:458)
>    >>>               
> drivers/net/ethernet/microchip/lan743x_main.o:(lan743x_pcidev_probe) in 
> archive vmlinux.a
>    >>> referenced by phy.h:458 (include/linux/phy.h:458)
>    >>>               drivers/net/ethernet/ni/nixge.o:(nixge_probe) in 
> archive vmlinux.a
>
> Kconfig warnings: (for reference only)
>    WARNING: unmet direct dependencies detected for PHYLIB

It looks like this has hit linux-next now, I'm seeing the same problem in
my own randconfig builds after Andrew's 01e5b728e9e4 ("net: phy: Add a
binding for PHY LEDs").

>    Depends on [m]: NETDEVICES [=y] && (LEDS_CLASS [=m] || LEDS_CLASS 
> [=m]=n)
>    Selected by [y]:
>    - PHYLINK [=y] && NETDEVICES [=y]
>    - TIGON3 [=y] && NETDEVICES [=y] && ETHERNET [=y] && 
> NET_VENDOR_BROADCOM [=y] && PCI [=y] && PTP_1588_CLOCK_OPTIONAL [=y]
>    - LAN743X [=y] && NETDEVICES [=y] && ETHERNET [=y] && 
> NET_VENDOR_MICROCHIP [=y] && PCI [=y] && PTP_1588_CLOCK_OPTIONAL [=y]
>    - NI_XGE_MANAGEMENT_ENET [=y] && NETDEVICES [=y] && ETHERNET [=y] && 
> NET_VENDOR_NI [=y] && HAS_IOMEM [=y] && HAS_DMA [=y]
>    - XILINX_EMACLITE [=y] && NETDEVICES [=y] && ETHERNET [=y] && 
> NET_VENDOR_XILINX [=y] && HAS_IOMEM [=y]
>    Selected by [m]:
>    - ALTERA_TSE [=m] && NETDEVICES [=y] && ETHERNET [=y] && HAS_DMA [=y]
>    - DNET [=m] && NETDEVICES [=y] && ETHERNET [=y] && HAS_IOMEM [=y]
>    - B44 [=m] && NETDEVICES [=y] && ETHERNET [=y] && 
> NET_VENDOR_BROADCOM [=y] && SSB_POSSIBLE [=y] && HAS_DMA [=y]
>    - KS8851_MLL [=m] && NETDEVICES [=y] && ETHERNET [=y] && 
> NET_VENDOR_MICREL [=y] && HAS_IOMEM [=y] && PTP_1588_CLOCK_OPTIONAL [=y]
>    - SMSC911X [=m] && NETDEVICES [=y] && ETHERNET [=y] && 
> NET_VENDOR_SMSC [=y] && HAS_IOMEM [=y]
>    - SMSC9420 [=m] && NETDEVICES [=y] && ETHERNET [=y] && 
> NET_VENDOR_SMSC [=y] && PCI [=y]
>    - XILINX_LL_TEMAC [=m] && NETDEVICES [=y] && ETHERNET [=y] && 
> NET_VENDOR_XILINX [=y] && HAS_IOMEM [=y]
>    - USB_NET_AX88179_178A [=m] && NETDEVICES [=y] && USB_NET_DRIVERS 
> [=m] && USB_USBNET [=m]

The problem here is that both PHYLIB and LEDS_CLASS are user-visible
tristate symbols that are referenced from other Kconfig symbols with
both 'depends on' and 'select'. Having the two interact introduces a
number of ways that lead to circular dependencies.

It might be ok to use 'select LEDS_CLASS' from PHYLIB, but I have not
tried that yet and I expect this will result in other build failures.

A better solution would be to change all drivers that currently use
'select PHYLIB' to 'depends on PHYLIB' and have PHYLIB itself
'default ETHERNET' to avoid most of the regressions, but doing this
for 6.4 is a bit risky and can cause other problems.

     Arnd

Re: [lunn:v6.3-rc2-net-next-phy-leds 5/15] ld.lld: error: undefined symbol: devm_mdiobus_alloc_size

[Andrew Lunn]

On Thu, Apr 20, 2023 at 08:31:17AM +0200, Arnd Bergmann wrote:
> On Fri, Mar 24, 2023, at 12:36, kernel test robot wrote:
> >>> ld.lld: error: undefined symbol: devm_mdiobus_alloc_size
> >    >>> referenced by phy.h:458 (include/linux/phy.h:458)
> >    >>>               
> > drivers/net/ethernet/microchip/lan743x_main.o:(lan743x_pcidev_probe) in 
> > archive vmlinux.a
> >    >>> referenced by phy.h:458 (include/linux/phy.h:458)
> >    >>>               drivers/net/ethernet/ni/nixge.o:(nixge_probe) in 
> > archive vmlinux.a
> >
> > Kconfig warnings: (for reference only)
> >    WARNING: unmet direct dependencies detected for PHYLIB
> 
> It looks like this has hit linux-next now, I'm seeing the same problem in
> my own randconfig builds after Andrew's 01e5b728e9e4 ("net: phy: Add a
> binding for PHY LEDs").

Hi Arnd

I tried to fix this with:

commit 37f9b2a6c086bb28487a0682b8098f907861c4a1
Author: Andrew Lunn <andrew@lunn.ch>
Date:   Thu Mar 30 20:13:29 2023 -0500

    net: ethernet: Add missing depends on MDIO_DEVRES
    
    A number of MDIO drivers make use of devm_mdiobus_alloc_size(). This
    is only available when CONFIG_MDIO_DEVRES is enabled. Add missing
    depends or selects, depending on if there are circular dependencies or
    not. This avoids linker errors, especially for randconfig builds.

All the failures i've seen have CONFIG_MDIO_DEVRES set to disabled. So
i added either depends on, or select to the drivers which use it. I
missed the LAN743x.

> The problem here is that both PHYLIB and LEDS_CLASS are user-visible
> tristate symbols that are referenced from other Kconfig symbols with
> both 'depends on' and 'select'. Having the two interact introduces a
> number of ways that lead to circular dependencies.

I was getting circular dependencies with first versions of the above
patch. I initially tried depends on everywhere, and then had to drop
back to select for a few cases.

> It might be ok to use 'select LEDS_CLASS' from PHYLIB, but I have not
> tried that yet and I expect this will result in other build failures.
> 
> A better solution would be to change all drivers that currently use
> 'select PHYLIB' to 'depends on PHYLIB' and have PHYLIB itself
> 'default ETHERNET' to avoid most of the regressions, but doing this
> for 6.4 is a bit risky and can cause other problems.

For 6.4, will adding more depend on/select MDIO_DEVRES help? That
should be low risk.

Thanks
	Andrew

Re: [lunn:v6.3-rc2-net-next-phy-leds 5/15] ld.lld: error: undefined symbol: devm_mdiobus_alloc_size

[Arnd Bergmann]

On Thu, Apr 20, 2023, at 15:07, Andrew Lunn wrote:
> On Thu, Apr 20, 2023 at 08:31:17AM +0200, Arnd Bergmann wrote:
>> 
>> It looks like this has hit linux-next now, I'm seeing the same problem in
>> my own randconfig builds after Andrew's 01e5b728e9e4 ("net: phy: Add a
>> binding for PHY LEDs").
>
> Hi Arnd
>
> I tried to fix this with:
>
> commit 37f9b2a6c086bb28487a0682b8098f907861c4a1
> Author: Andrew Lunn <andrew@lunn.ch>
> Date:   Thu Mar 30 20:13:29 2023 -0500
>
>     net: ethernet: Add missing depends on MDIO_DEVRES
>    
>     A number of MDIO drivers make use of devm_mdiobus_alloc_size(). This
>     is only available when CONFIG_MDIO_DEVRES is enabled. Add missing
>     depends or selects, depending on if there are circular dependencies or
>     not. This avoids linker errors, especially for randconfig builds.
>
> All the failures i've seen have CONFIG_MDIO_DEVRES set to disabled. So
> i added either depends on, or select to the drivers which use it. I
> missed the LAN743x.

I've sent a different patch now. From what I can tell, your patch does
not address the actual problem, it just works around one of the symptoms:

When you have a driver that selects PHYLIB, but PHYLIB itself
depends on something that is not enabled, Kconfig just gives up
and stops handling any other 'select' statements from that chain.

selecting MDIO_DEVRES from each of the drivers works around this
one thing, but it does not solve the actual dependency, for that you
would have to add 'depends on LED_CLASS' on each of the network
drivers as well, which is clearly not the intention.

>> The problem here is that both PHYLIB and LEDS_CLASS are user-visible
>> tristate symbols that are referenced from other Kconfig symbols with
>> both 'depends on' and 'select'. Having the two interact introduces a
>> number of ways that lead to circular dependencies.
>
> I was getting circular dependencies with first versions of the above
> patch. I initially tried depends on everywhere, and then had to drop
> back to select for a few cases.
>
>> It might be ok to use 'select LEDS_CLASS' from PHYLIB, but I have not
>> tried that yet and I expect this will result in other build failures.
>> 
>> A better solution would be to change all drivers that currently use
>> 'select PHYLIB' to 'depends on PHYLIB' and have PHYLIB itself
>> 'default ETHERNET' to avoid most of the regressions, but doing this
>> for 6.4 is a bit risky and can cause other problems.
>
> For 6.4, will adding more depend on/select MDIO_DEVRES help? That
> should be low risk.

I think the best way is to drop your MDIO_DEVRES patch and instead
apply mine (or some variation of that) from

https://lore.kernel.org/lkml/20230420084624.3005701-1-arnd@kernel.org/

Once any missing or recursive dependencies are handled, the devres
problem should be fixed as well. I have completed around 150
randconfig builds with that patch and have not seen any further
problems.

     Arnd

Re: [lunn:v6.3-rc2-net-next-phy-leds 5/15] ld.lld: error: undefined symbol: devm_mdiobus_alloc_size

[Andrew Lunn]

> I think the best way is to drop your MDIO_DEVRES patch and instead
> apply mine (or some variation of that) from
> 
> https://lore.kernel.org/lkml/20230420084624.3005701-1-arnd@kernel.org/
> 
> Once any missing or recursive dependencies are handled, the devres
> problem should be fixed as well. I have completed around 150
> randconfig builds with that patch and have not seen any further
> problems.

Hi Arnd

Is this on top of my patch? Or does it require mine is reverted?  I
can send a revert if it is needed.

   Andrew

Re: [lunn:v6.3-rc2-net-next-phy-leds 5/15] ld.lld: error: undefined symbol: devm_mdiobus_alloc_size

[Arnd Bergmann]

On Thu, Apr 20, 2023, at 17:26, Andrew Lunn wrote:
>> I think the best way is to drop your MDIO_DEVRES patch and instead
>> apply mine (or some variation of that) from
>> 
>> https://lore.kernel.org/lkml/20230420084624.3005701-1-arnd@kernel.org/
>> 
>> Once any missing or recursive dependencies are handled, the devres
>> problem should be fixed as well. I have completed around 150
>> randconfig builds with that patch and have not seen any further
>> problems.
>
> Is this on top of my patch? Or does it require mine is reverted?  I
> can send a revert if it is needed.

The two are independent. I think your patch does nothing once
mine is applied, since the indirect 'select MDIO_DEVRES' will
work again. Reverting it might help avoid some confusion though.

     Arnd