On Thu, May 16, 2024 at 12:35 PM Florian Fainelli <f.fainelli@xxxxxxxxx> wrote:
>
>
>
> On 5/15/2024 7:17 PM, Masahiro Yamada wrote:
> > On Thu, May 16, 2024 at 4:21 AM Florian Fainelli <f.fainelli@xxxxxxxxx> wrote:
> >>
> >> 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@xxxxxxx> 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?
> >
> >
> >
> > "obj-m += dsa/" means everything under dsa/ must be modular.
> >
> >
> >
> > If there is a built-in object under dsa/ with CONFIG_NET_DSA=m,
> > you cannot do "obj-$(CONFIG_NET_DSA) += dsa/".
> >
> >
> > You need to change it back to "obj-y += dsa/".
>
> Thanks, posted a patch doing that! Is there anyway that Kbuild could be
> warning about such a situation?
I posted such a patch a few years ago.
https://lore.kernel.org/lkml/20190912162254.9603-2-yamada.masahiro@xxxxxxxxxxxxx/#t
but I noticed some corner cases where false alarms are reported.
I can revisit it when I have spare time.
--
Best Regards
Masahiro Yamada
