Re: [PATCH] net: lan743x: LAN743X selects FIXED_PHY to resolve a link error

From: Tom Rix
Date: Sun Feb 26 2023 - 10:26:47 EST



On 2/21/23 8:20 AM, Simon Horman wrote:
+Arnd

On Tue, Feb 21, 2023 at 03:29:39AM +0100, Andrew Lunn wrote:
On Mon, Feb 20, 2023 at 07:46:13AM +0100, Simon Horman wrote:
On Mon, Feb 20, 2023 at 02:19:34AM +0100, Andrew Lunn wrote:
On Sun, Feb 19, 2023 at 07:16:07PM +0100, Simon Horman wrote:
On Sun, Feb 19, 2023 at 10:03:21AM -0500, Tom Rix wrote:
A rand config causes this link error
drivers/net/ethernet/microchip/lan743x_main.o: In function `lan743x_netdev_open':
drivers/net/ethernet/microchip/lan743x_main.c:1512: undefined reference to `fixed_phy_register'

lan743x_netdev_open is controlled by LAN743X
fixed_phy_register is controlled by FIXED_PHY

So LAN743X should also select FIXED_PHY

Signed-off-by: Tom Rix <trix@xxxxxxxxxx>
Hi Tom,

I am a little confused by this.

I did manage to cook up a config with LAN743X=m and FIXED_PHY not set.
But I do not see a build failure, and I believe that is because
when FIXED_PHY is not set then a stub version of fixed_phy_register(),
defined as static inline in include/linux/phy_fixed.h, is used.

Ref: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/include/linux/phy_fixed.h?h=main&id=675f176b4dcc2b75adbcea7ba0e9a649527f53bd#n42
I'n guessing, but it could be that LAN743X is built in, and FIXED_PHY
is a module? What might be needed is

depends on FIXED_PHY || FIXED_PHY=n
Thanks Andrew,

LAN743X=y and FIXED_PHY=m does indeed produce the problem that Tom
describes. And his patch does appear to resolve the problem.
O.K. So the commit message needs updating to describe the actual
problem.
Yes, that would be a good improvement.

Perhaps a fixes tag too?

Unfortunately your proposed solution seems to run foul of a complex
dependency situation.
I was never any good at Kconfig. Arnd is the expert at solving
problems like this.

You want either everything built in, or FIXED_PHY built in and LAN743X
modular, or both modular.
I _think_ the patch, which uses select FIXED_PHY for LAN743X,
achieves that.

I CCed Arnd in case he has any input. Though I think I read
in an recent email from him that he is out most of this week.

I was out last week as well :)

I considered this a cleanup rather than a fix, I can add that fixes tag.

From my point of view this is a linking problem, I don't mind changing,  what commit message should I use ?

Tom