Received-SPF: pass (google.com: domain of linux-kernel+bounces-180312-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <19db38dd-b265-45d3-af1c-85d4cb8d075a@gmail.com>
Date: Wed, 15 May 2024 12:20:55 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems
From: Florian Fainelli <f.fainelli@gmail.com>
To: Andrew Lunn <andrew@lunn.ch>,
 Stephen Langstaff <stephenlangstaff1@gmail.com>,
 Alexander Lobakin <aleksander.lobakin@intel.com>,
 Masahiro Yamada <masahiroy@kernel.org>,
 Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>
Cc: linux-kernel@vger.kernel.org, OlteanV@gmail.com
References: <212a9464-d52b-4730-95b9-5a0aebd38c91@gmail.com>
 <CAHx5RXCWW5M-eW5v65bAkQWZemsU2NTvDv3jA9_XKz=+YP56Qg@mail.gmail.com>
 <688e54ec-3b29-4e3b-a2c3-f2c83b9c97b7@lunn.ch>
 <CAHx5RXBFdzsgKXR94gdZd2b=uz8PJDg4OjLPJxKtsdhcjJq3Qw@mail.gmail.com>
 <e307a237-68e3-40c9-be31-4fe3d560ada2@lunn.ch>
 <CAHx5RXCF0=Soz_k88RGvJFGrajaxn=mVnqpb99GAQ=b7XOcWiw@mail.gmail.com>
 <732d8bb2-1d4f-4958-b130-0bd15a407271@gmail.com>
 <CAHx5RXDaweFTF_Qt0GdBH4nBeMqwL4VVto7xzHBvFgFL5n=Ebg@mail.gmail.com>
 <c8c01e53-0a45-4319-88ff-bfb0caba150c@lunn.ch>
 <CAHx5RXDzN93WaYFe2bk6m2TmMC+A9vsmhodRFmZi17cFY5CrWQ@mail.gmail.com>
 <949fcbea-23dc-44c1-9146-c358b15b9253@lunn.ch>
 <b7d6fdcb-4b01-4bc1-8e4b-3cf4ccb951e3@gmail.com>
Content-Language: en-US
In-Reply-To: <b7d6fdcb-4b01-4bc1-8e4b-3cf4ccb951e3@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Adding Olek and Masahiro,

On 5/14/24 21:05, Florian Fainelli wrote:
> 
> 
> On 5/14/2024 9:17 AM, Andrew Lunn wrote:
>> On Tue, May 14, 2024 at 05:08:24PM +0100, Stephen Langstaff wrote:
>>> On Tue, May 14, 2024 at 1:32 PM Andrew Lunn <andrew@lunn.ch> wrote:
>>>
>>>> So try to making FIXED_PHY = m, and load it after dsa_loop_bdinfo.ko.
>>>
>>> In my configuration FIXED_PHY is selected by several other modules:
>>>    │ Selected by [y]:
>>>    │   - FSL_DPAA_ETH [=y] && NETDEVICES [=y] && ETHERNET [=y] &&
>>> NET_VENDOR_FREESCALE [=y] && FSL_DPAA [=y] && FSL_FMAN [=y]
>>>    │   - FWNODE_MDIO [=y] && NETDEVICES [=y] && MDIO_DEVICE [=y] &&
>>> (ACPI [=y] || OF [=y] || COMPILE_TEST [=n])
>>>    │   - OF_MDIO [=y] && NETDEVICES [=y] && MDIO_DEVICE [=y] && OF [=y]
>>> && PHYLIB [=y]
>>>
>>> ...so it looks pretty tied up with the MDIO support which I guess I
>>> will need for the real PHY!
>>>
>>> If I sorted out building the dsa_loop_bdinfo.c code as a built-in do
>>> you think that would solve the ordering issue?
> 
> I have re-created the issue with CONFIG_FIXED_PHY=y and for a reason I 
> do not yet understand the following rule:
> 
> obj-$(CONFIG_FIXED_PHY)                += dsa_loop_bdinfo.o
> 
> does not result in the kernel image containing the dsa_loop_bdinfo.o 
> object symbols. I am fairly sure this worked when this was submitted 
> back then, so give me a day or two to figure out why. AFAICT the make 
> rule is simply not executed.

Bisection landed on 227d72063fccb2d19b30fb4197fba478514f7d83 ("dsa: 
simplify Kconfig symbols and dependencies") which appeared in v5.13 and 
specifically this hunk being reverted back to how it was before gets us 
the build results we want:

diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 7ffd2d03efaf..5da6424bc6f8 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -45,7 +45,7 @@ obj-$(CONFIG_ARCNET) += arcnet/
  obj-$(CONFIG_DEV_APPLETALK) += appletalk/
  obj-$(CONFIG_CAIF) += caif/
  obj-$(CONFIG_CAN) += can/
-obj-$(CONFIG_NET_DSA) += dsa/
+obj-y += dsa/
  obj-$(CONFIG_ETHERNET) += ethernet/
  obj-$(CONFIG_FDDI) += fddi/
  obj-$(CONFIG_HIPPI) += hippi/

Masahiro, for context in drivers/net/dsa/Makefile we have this bit:

obj-$(CONFIG_NET_DSA_LOOP)      += dsa_loop.o
ifdef CONFIG_NET_DSA_LOOP
obj-$(CONFIG_FIXED_PHY)         += dsa_loop_bdinfo.o
endif

whereby we want dsa_loop.o to follow the value of CONFIG_NET_DSA_LOOP, 
and we want dsa_loop_bdinfo.o to be either built as a module or built 
into the kernel and we want to follow the value of CONFIG_FIXED_PHY 
because there is a functional dependency between the two objects.

Prior to Olek's change this would work just fine because we would always 
descend into drivers/net/dsa/ but after his change, and assuming that 
CONFIG_NET_DSA=m which is the case, then we no longer get 
dsa_loop_bdinfo.o to be built at all when CONFIG_FIXED_PHY=y. 
Essentially only obj-m rules are being processed, obj-y rules are not.

That does not really seem intuitive to me as to why any suggestions on 
how to fix that, short of unconditionally descending into the tree like 
we used to?

Reproducer configuration available here:

https://gist.github.com/ffainelli/2650a832803b502b94b2d247209f61b1

rm drivers/net/dsa/*.o
ARCH=x86 make drivers/net/dsa/
ls drivers/net/dsa/dsa_loop_bdinfo.o

Thanks!
-- 
Florian