Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Stephen Langstaff]

[Corrected emails for f.fainelli and olteanv]

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Florian Fainelli]

Hello,

On 5/13/2024 8:33 AM, Stephen Langstaff wrote:
> [Corrected emails for f.fainelli and olteanv]

What is the build problem?
-- 
Florian

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Stephen Langstaff]

dsa_loop_bdinfo.ko is not created as a module.

On Mon, May 13, 2024 at 4:34 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> Hello,
>
> On 5/13/2024 8:33 AM, Stephen Langstaff wrote:
> > [Corrected emails for f.fainelli and olteanv]
>
> What is the build problem?
> --
> Florian

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Florian Fainelli]

On 5/13/2024 8:36 AM, Stephen Langstaff wrote:
> dsa_loop_bdinfo.ko is not created as a module.

The Makefile has this:

obj-$(CONFIG_FIXED_PHY)         += dsa_loop_bdinfo.o

so it follows the value of CONFIG_FIXED_PHY, what is this configuration 
symbol is to in your kernel configuration?

> 
> On Mon, May 13, 2024 at 4:34 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>> Hello,
>>
>> On 5/13/2024 8:33 AM, Stephen Langstaff wrote:
>>> [Corrected emails for f.fainelli and olteanv]
>>
>> What is the build problem?
>> --
>> Florian

-- 
Florian

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Stephen Langstaff]

On Mon, May 13, 2024 at 4:40 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> The Makefile has this:
>
> obj-$(CONFIG_FIXED_PHY)         += dsa_loop_bdinfo.o
>
> so it follows the value of CONFIG_FIXED_PHY, what is this configuration
> symbol is to in your kernel configuration?

CONFIG_FIXED_PHY=y

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Florian Fainelli]

On 5/13/2024 8:45 AM, Stephen Langstaff wrote:
> On Mon, May 13, 2024 at 4:40 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>> The Makefile has this:
>>
>> obj-$(CONFIG_FIXED_PHY)         += dsa_loop_bdinfo.o
>>
>> so it follows the value of CONFIG_FIXED_PHY, what is this configuration
>> symbol is to in your kernel configuration?
> 
> CONFIG_FIXED_PHY=y

Then dsa_loop_bdinfo is going to be built into your kernel image, and it 
will register a mdio_board_info structure which will create fixed PHY 
devices and it will assume that the DSA conduit network device name is 
"eth0". If this is not correct for your system, that needs changing.

What are you trying to achieve? Can you be a tad more verbose about what 
you have tried and what is not working?
-- 
Florian

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Stephen Langstaff]

On Mon, May 13, 2024 at 4:50 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> Then dsa_loop_bdinfo is going to be built into your kernel image, and it
> will register a mdio_board_info structure which will create fixed PHY
> devices and it will assume that the DSA conduit network device name is
> "eth0". If this is not correct for your system, that needs changing.
>
> What are you trying to achieve? Can you be a tad more verbose about what
> you have tried and what is not working?

Sure, sorry I was being a bit terse.

In a 6.1.55 Linux kernel build, after a full make, if I touch a file
marked as "=y" in the .config (e.g. net/devres.c) I see that the
compiler rebuilds the corresponding .o file, as expected:

user@machine:/opt/linux-imx$ touch net/devres.c
user@machine:/opt/linux-imx$ make
  CALL    scripts/checksyscalls.sh
  CC      net/devres.o
  AR      net/built-in.a
...

If I do the same for drivers/net/dsa/dsa_loop_bdinfo.c nothing is rebuilt!

user@machine:/opt/linux-imx$ touch drivers/net/dsa/dsa_loop_bdinfo.c
user@machine:/opt/linux-imx$ make
  CALL    scripts/checksyscalls.sh
user@machine:/opt/linux-imx$

What I am trying to achieve is to use dsa_loop to connect a CPU
running Linux to an Ethernet switch via a "conduit" Ethernet link
(e.g. eth0) using the port-based Ethertype DSA (Marvell) frame tagging
protocol, so that the Linux kernel will present the userspace with
separate Ethernet links for each of the switch's user ports (e.g.
lan1, lan2, lan3).

As I understand it this is usually done via a fixed phy-less conduit
(e.g. xMII) and an MDIO link between the processor and the switch so
that the processor can configure and control the physical user ports.

I want to do some software development ahead of receiving the
hardware, so I want to "fake" the MDIO link, or in some other way
configure the kernel, such that it uses port-based EDSA frame tagging
over an ordinary (PHY-equipped) Ethernet port and it looks to me like
dsa_loop should be capable of doing this.

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Andrew Lunn]

> What I am trying to achieve is to use dsa_loop to connect a CPU
> running Linux to an Ethernet switch via a "conduit" Ethernet link
> (e.g. eth0) using the port-based Ethertype DSA (Marvell) frame tagging
> protocol, so that the Linux kernel will present the userspace with
> separate Ethernet links for each of the switch's user ports (e.g.
> lan1, lan2, lan3).
> 
> As I understand it this is usually done via a fixed phy-less conduit
> (e.g. xMII) and an MDIO link between the processor and the switch so
> that the processor can configure and control the physical user ports.
> 
> I want to do some software development ahead of receiving the
> hardware, so I want to "fake" the MDIO link, or in some other way
> configure the kernel, such that it uses port-based EDSA frame tagging
> over an ordinary (PHY-equipped) Ethernet port and it looks to me like
> dsa_loop should be capable of doing this.

That is an interesting use case. I don't know of anybody doing that
before.

As Florian pointed out, dsa_loop_bdinfo.o should be built into your
kernel. When you insmod dsa_loop.ko, it should match the data in
dsa_loop_bdinfo to the dsa_loop driver an instantiate an instance of
it. Do you see that happening?

You could hack dsa_loop_get_protocol() to return DSA_TAG_PROTO_EDSA,
in order to use the EDSA frame tagger. dsa_loop does not have an
implementation of .change_tag_protocol, so i don't think you can
change it at runtime. However an implementation of it should be
trivial.

   Andrew

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Stephen Langstaff]

On Mon, May 13, 2024 at 5:54 PM Andrew Lunn <andrew@lunn.ch> wrote:
> As Florian pointed out, dsa_loop_bdinfo.o should be built into your
> kernel. When you insmod dsa_loop.ko, it should match the data in
> dsa_loop_bdinfo to the dsa_loop driver an instantiate an instance of
> it. Do you see that happening?

Well, if I fiddle with the Makefiles to get dsa_loop_bdinfo built as a
module (as opposed to not building at all) then I see the following
behaviour:

root@machine:~/dsa_test# ls
dsa_loop.ko  dsa_loop_bdinfo.ko
root@machine:~/dsa_test# modprobe dsa_core
root@machine:~/dsa_test# insmod dsa_loop_bdinfo.ko
root@machine:~/dsa_test# insmod dsa_loop.ko
root@machine:~/dsa_test# lsmod
Module                  Size  Used by
dsa_loop               16384  0
dsa_loop_bdinfo        16384  0
dsa_core              118784  1 dsa_loop
...

...and I see no new interfaces reported by ifconfig -a
[just the existing eth0, eth1 and lo0 interfaces]

I will revert my changes to the Makefiles and confirm that
dsa_loop_bdinfo.o does not get built into the kernel - that's an
overnight build for me so bye for now!

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Andrew Lunn]

On Mon, May 13, 2024 at 06:40:19PM +0100, Stephen Langstaff wrote:
> On Mon, May 13, 2024 at 5:54 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > As Florian pointed out, dsa_loop_bdinfo.o should be built into your
> > kernel. When you insmod dsa_loop.ko, it should match the data in
> > dsa_loop_bdinfo to the dsa_loop driver an instantiate an instance of
> > it. Do you see that happening?
> 
> Well, if I fiddle with the Makefiles to get dsa_loop_bdinfo built as a
> module (as opposed to not building at all) then I see the following
> behaviour:
> 
> root@machine:~/dsa_test# ls
> dsa_loop.ko  dsa_loop_bdinfo.ko
> root@machine:~/dsa_test# modprobe dsa_core
> root@machine:~/dsa_test# insmod dsa_loop_bdinfo.ko
> root@machine:~/dsa_test# insmod dsa_loop.ko
> root@machine:~/dsa_test# lsmod
> Module                  Size  Used by
> dsa_loop               16384  0
> dsa_loop_bdinfo        16384  0
> dsa_core              118784  1 dsa_loop
> ...
> 
> ...and I see no new interfaces reported by ifconfig -a
> [just the existing eth0, eth1 and lo0 interfaces]
> 
> I will revert my changes to the Makefiles and confirm that
> dsa_loop_bdinfo.o does not get built into the kernel - that's an
> overnight build for me so bye for now!

I think most developer cross compile the kernel on a big machine. I
also tend to build in everything i need, so its all in one file, which
i get uboot to tftpboot. It makes the whole build/boot/crash cycle
much master.

     Andrew

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 1889 bytes --]

On 5/13/24 11:55, Stephen Langstaff wrote:
> 
> OK, just confirmed that dsa_loop_bdinfo.c is not compiled to either a .o 
> or a .ko file when using the unmodified Makefiles.

Attached is a configuration that I use on a x86-64 VM for testing that 
is started with:

qemu-system-x86_64 \
         -s \
         -cpu "max" \
         -smp 4 \
         -kernel ~/dev/linux/arch/x86/boot/bzImage \
         -device pci-bridge,id=pci_bridge1,bus=pci.0,chassis_nr=1 \
         -drive 
file=debian.img,if=none,id=drive-virtio-disk0,format=qcow2 -device 
virtio-blk-pci,scsi=off,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 
\
         -nographic \
         -append "root=/dev/vda1 console=ttyS0,115200 mitigations=off" \
         -net nic,model=e1000 -net tap,ifname=tap0

...

ip link set dev enp0s3 name eth0
modprobe dsa_loop_bdinfo
modprobe dsa_loop

ip link
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode 
DEFAULT group default qlen 1000
     link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: eth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode 
DEFAULT group default qlen 1000
     link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
     altname enp0s3
3: lan1@eth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state 
DOWN mode DEFAULT group default qlen 1000
     link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
4: lan2@eth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state 
DOWN mode DEFAULT group default qlen 1000
     link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
5: lan3@eth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state 
DOWN mode DEFAULT group default qlen 1000
     link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
6: lan4@eth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state 
DOWN mode DEFAULT group default qlen 1000
     link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
-- 
Florian

[-- Attachment #2: x86-64_dsa.config --]
[-- Type: text/plain, Size: 5197 bytes --]

CONFIG_KERNEL_XZ=y
CONFIG_SYSVIPC=y
CONFIG_POSIX_MQUEUE=y
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y
CONFIG_IRQ_TIME_ACCOUNTING=y
CONFIG_LOG_BUF_SHIFT=16
CONFIG_CGROUPS=y
CONFIG_RELAY=y
CONFIG_BLK_DEV_INITRD=y
CONFIG_SMP=y
CONFIG_NR_CPUS=4
# CONFIG_SCHED_MC is not set
CONFIG_HZ_1000=y
# CONFIG_RANDOMIZE_BASE is not set
# CONFIG_SUSPEND is not set
# CONFIG_ACPI is not set
CONFIG_IA32_EMULATION=y
CONFIG_KVM=y
CONFIG_KVM_INTEL=y
CONFIG_KVM_AMD=y
# CONFIG_KVM_HYPERV is not set
# CONFIG_SECCOMP is not set
CONFIG_MODULES=y
CONFIG_MODULE_FORCE_LOAD=y
CONFIG_MODULE_UNLOAD=y
CONFIG_MODVERSIONS=y
CONFIG_PARTITION_ADVANCED=y
CONFIG_CMA=y
CONFIG_NET=y
CONFIG_PACKET=y
CONFIG_UNIX=y
CONFIG_INET=y
CONFIG_IP_MULTICAST=y
CONFIG_IP_PNP=y
CONFIG_IP_PNP_DHCP=y
CONFIG_IP_PNP_BOOTP=y
CONFIG_IP_PNP_RARP=y
CONFIG_IP_MROUTE=y
CONFIG_IP_PIMSM_V1=y
CONFIG_IP_PIMSM_V2=y
CONFIG_INET_UDP_DIAG=y
CONFIG_TCP_CONG_ADVANCED=y
CONFIG_TCP_CONG_BIC=y
# CONFIG_TCP_CONG_WESTWOOD is not set
# CONFIG_TCP_CONG_HTCP is not set
# CONFIG_IPV6 is not set
CONFIG_BRIDGE=m
CONFIG_BRIDGE_VLAN_FILTERING=y
CONFIG_NET_DSA=m
CONFIG_VLAN_8021Q=m
CONFIG_CFG80211=y
CONFIG_PCI=y
CONFIG_PCIEPORTBUS=y
CONFIG_PCI_MSI=y
CONFIG_DEVTMPFS=y
CONFIG_DEVTMPFS_MOUNT=y
CONFIG_FW_CFG_SYSFS=y
CONFIG_FW_CFG_SYSFS_CMDLINE=y
CONFIG_MTD=y
CONFIG_MTD_CMDLINE_PARTS=y
CONFIG_MTD_BLOCK=y
CONFIG_MTD_CFI=y
CONFIG_MTD_JEDECPROBE=y
CONFIG_MTD_CFI_INTELEXT=y
CONFIG_MTD_CFI_AMDSTD=y
CONFIG_MTD_CFI_STAA=y
CONFIG_MTD_ROM=y
CONFIG_MTD_ABSENT=y
CONFIG_MTD_UBI=y
CONFIG_MTD_UBI_GLUEBI=y
CONFIG_BLK_DEV_LOOP=y
CONFIG_BLK_DEV_RAM=y
CONFIG_BLK_DEV_RAM_SIZE=8192
CONFIG_VIRTIO_BLK=y
CONFIG_EEPROM_93CX6=y
CONFIG_BLK_DEV_SD=y
CONFIG_BLK_DEV_SR=y
CONFIG_CHR_DEV_SG=y
CONFIG_SCSI_VIRTIO=y
CONFIG_ATA=y
CONFIG_SATA_AHCI=y
CONFIG_SATA_AHCI_PLATFORM=y
CONFIG_ATA_PIIX=y
CONFIG_NETDEVICES=y
CONFIG_NET_DSA_LOOP=m
# CONFIG_NET_VENDOR_3COM is not set
# CONFIG_NET_VENDOR_ADAPTEC is not set
# CONFIG_NET_VENDOR_AGERE is not set
# CONFIG_NET_VENDOR_ALACRITECH is not set
# CONFIG_NET_VENDOR_ALTEON is not set
# CONFIG_NET_VENDOR_AMAZON is not set
# CONFIG_NET_VENDOR_AMD is not set
# CONFIG_NET_VENDOR_AQUANTIA is not set
# CONFIG_NET_VENDOR_ARC is not set
# CONFIG_NET_VENDOR_ATHEROS is not set
# CONFIG_NET_VENDOR_BROADCOM is not set
# CONFIG_NET_VENDOR_CAVIUM is not set
# CONFIG_NET_VENDOR_CHELSIO is not set
# CONFIG_NET_VENDOR_CISCO is not set
# CONFIG_NET_VENDOR_DEC is not set
# CONFIG_NET_VENDOR_DLINK is not set
# CONFIG_NET_VENDOR_EMULEX is not set
# CONFIG_NET_VENDOR_EZCHIP is not set
# CONFIG_NET_VENDOR_I825XX is not set
CONFIG_E1000=y
# CONFIG_NET_VENDOR_MARVELL is not set
# CONFIG_NET_VENDOR_MELLANOX is not set
# CONFIG_NET_VENDOR_MICREL is not set
# CONFIG_NET_VENDOR_MYRI is not set
# CONFIG_NET_VENDOR_NATSEMI is not set
# CONFIG_NET_VENDOR_NETRONOME is not set
# CONFIG_NET_VENDOR_NVIDIA is not set
# CONFIG_NET_VENDOR_OKI is not set
# CONFIG_NET_VENDOR_QLOGIC is not set
# CONFIG_NET_VENDOR_BROCADE is not set
# CONFIG_NET_VENDOR_QUALCOMM is not set
# CONFIG_NET_VENDOR_RDC is not set
# CONFIG_NET_VENDOR_REALTEK is not set
# CONFIG_NET_VENDOR_RENESAS is not set
# CONFIG_NET_VENDOR_ROCKER is not set
# CONFIG_NET_VENDOR_SAMSUNG is not set
# CONFIG_NET_VENDOR_SEEQ is not set
# CONFIG_NET_VENDOR_SILAN is not set
# CONFIG_NET_VENDOR_SIS is not set
# CONFIG_NET_VENDOR_SOLARFLARE is not set
# CONFIG_NET_VENDOR_SMSC is not set
# CONFIG_NET_VENDOR_STMICRO is not set
# CONFIG_NET_VENDOR_SUN is not set
# CONFIG_NET_VENDOR_TEHUTI is not set
# CONFIG_NET_VENDOR_TI is not set
# CONFIG_NET_VENDOR_VIA is not set
# CONFIG_NET_VENDOR_WIZNET is not set
CONFIG_MDIO_BITBANG=m
CONFIG_MDIO_BCM_UNIMAC=m
CONFIG_MDIO_MSCC_MIIM=m
CONFIG_MDIO_THUNDER=m
# CONFIG_WLAN is not set
CONFIG_INPUT_MOUSEDEV=y
CONFIG_INPUT_MOUSEDEV_PSAUX=y
CONFIG_INPUT_EVDEV=y
CONFIG_MOUSE_PS2_ELANTECH=y
CONFIG_INPUT_MISC=y
CONFIG_INPUT_UINPUT=y
# CONFIG_SERIO_SERPORT is not set
CONFIG_VT_HW_CONSOLE_BINDING=y
CONFIG_SERIAL_8250=y
CONFIG_SERIAL_8250_CONSOLE=y
CONFIG_SERIAL_8250_DW=y
CONFIG_HW_RANDOM=y
CONFIG_I2C=y
CONFIG_I2C_CHARDEV=y
CONFIG_I2C_MUX=y
# CONFIG_HWMON is not set
CONFIG_MFD_SYSCON=y
# CONFIG_HID_SUPPORT is not set
# CONFIG_USB_SUPPORT is not set
CONFIG_UIO=m
CONFIG_VIRTIO_PCI=y
CONFIG_VIRTIO_BALLOON=y
CONFIG_VIRTIO_INPUT=y
CONFIG_VIRTIO_MMIO=y
CONFIG_VIRTIO_MMIO_CMDLINE_DEVICES=y
CONFIG_VHOST_NET=y
# CONFIG_SURFACE_PLATFORMS is not set
# CONFIG_IOMMU_SUPPORT is not set
CONFIG_GENERIC_PHY=y
# CONFIG_NVMEM is not set
CONFIG_EXT4_FS=y
CONFIG_JBD2_DEBUG=y
CONFIG_FUSE_FS=y
CONFIG_CUSE=y
CONFIG_ISO9660_FS=y
CONFIG_JOLIET=y
CONFIG_ZISOFS=y
CONFIG_UDF_FS=y
CONFIG_MSDOS_FS=y
CONFIG_VFAT_FS=y
CONFIG_TMPFS=y
CONFIG_CONFIGFS_FS=y
CONFIG_JFFS2_FS=y
CONFIG_UBIFS_FS=y
CONFIG_CRAMFS=y
CONFIG_SQUASHFS=y
CONFIG_SQUASHFS_LZO=y
CONFIG_SQUASHFS_XZ=y
CONFIG_NFS_FS=y
CONFIG_NFS_V3_ACL=y
CONFIG_NFS_V4=y
CONFIG_NFS_V4_1=y
CONFIG_NFS_V4_2=y
CONFIG_ROOT_NFS=y
CONFIG_NLS_CODEPAGE_437=y
CONFIG_NLS_ISO8859_1=y
# CONFIG_CRYPTO_HW is not set
CONFIG_CRC_CCITT=y
CONFIG_LIBCRC32C=m
CONFIG_DMA_CMA=y
CONFIG_CMA_SIZE_MBYTES=16
CONFIG_CMA_ALIGNMENT=9
CONFIG_PRINTK_TIME=y
CONFIG_DYNAMIC_DEBUG=y
CONFIG_FRAME_WARN=1024
CONFIG_DEBUG_FS=y
CONFIG_DEBUG_RODATA_TEST=y
# CONFIG_STRICT_DEVMEM is not set

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Stephen Langstaff]

On Mon, May 13, 2024 at 8:05 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> Attached is a configuration that I use on a x86-64 VM for testing that
> is started with:

That's kind of what I expect to be happening on my target, but it's not.

I notice that your configuration does not define CONFIG_FIXED_PHY as
either a module or built-in - could that explain both the fact that my
dsa_loop_bdinfo.c does not get compiled when using the default
Makefiles, and that if I force compilation to a module it does not
appear to create the expected lanX@eth0 interfaces?

I have added some printk debug lines to the dsa_loop_init and
dsa_loop_drv_probe functions, and only see the one from the init
function, which makes me suspect that something is stopping the probe
from happening - I assume this should happen when the dsa_loop_bdinfo
module is inserted?

root@machine:~/dsa_test# ls
dsa_loop.ko  dsa_loop_bdinfo.ko

root@machine:~/dsa_test# modprobe dsa_core
root@machine:~/dsa_test#

root@machine:~/dsa_test# insmod dsa_loop_bdinfo.ko
[   51.742367] dsa_loop_bdinfo_init

root@machine:~/dsa_test# insmod dsa_loop.ko
[   58.485104] dsa_loop_init

root@machine:~/dsa_test# ip link
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN
mode DEFAULT group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP
mode DEFAULT group default qlen 1000
    link/ether xx:xx:xx:xx:xx:xx brd ff:ff:ff:ff:ff:ff
3: eth1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc mq state
DOWN mode DEFAULT group default qlen 1000
    link/ether xx:xx:xx:xx:xx:xx brd ff:ff:ff:ff:ff:ff
4: can0: <NOARP,ECHO> mtu 16 qdisc noop state DOWN mode DEFAULT group
default qlen 10
    link/can

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Andrew Lunn]

On Tue, May 14, 2024 at 10:28:55AM +0100, Stephen Langstaff wrote:
> On Mon, May 13, 2024 at 8:05 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > Attached is a configuration that I use on a x86-64 VM for testing that
> > is started with:
> 
> That's kind of what I expect to be happening on my target, but it's not.
> 
> I notice that your configuration does not define CONFIG_FIXED_PHY as
> either a module or built-in - could that explain both the fact that my
> dsa_loop_bdinfo.c does not get compiled when using the default
> Makefiles, and that if I force compilation to a module it does not
> appear to create the expected lanX@eth0 interfaces?
> 
> I have added some printk debug lines to the dsa_loop_init and
> dsa_loop_drv_probe functions, and only see the one from the init
> function, which makes me suspect that something is stopping the probe
> from happening - I assume this should happen when the dsa_loop_bdinfo
> module is inserted?
> 
> root@machine:~/dsa_test# ls
> dsa_loop.ko  dsa_loop_bdinfo.ko
> 
> root@machine:~/dsa_test# modprobe dsa_core
> root@machine:~/dsa_test#
> 
> root@machine:~/dsa_test# insmod dsa_loop_bdinfo.ko
> [   51.742367] dsa_loop_bdinfo_init
> 
> root@machine:~/dsa_test# insmod dsa_loop.ko
> [   58.485104] dsa_loop_init

This is probably an ordering problem.

dsa_loop_bdinfo calls mdiobus_register_board_info() with the bus_id
"fixed-0". That call adds it to a linked list, but does nothing else.

mdiobus_setup_mdiodev_from_board_info() is what walks the linked list
and sees if an MDIO bus matches the ID, and if so, calls the callback
to add a device to the bus. mdiobus_setup_mdiodev_from_board_info() is
only called from __mdiobus_register(). So the boardinfo needs to be
registered before the bus is registers. The other way around does not
work.

So try to making FIXED_PHY = m, and load it after dsa_loop_bdinfo.ko.

   Andrew

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Stephen Langstaff]

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?

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Andrew Lunn]

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?

Probably.

Florian test setup shows it can work.

This mdio board info stuff is based on the same concept for I2C
devices. However, it is simplified a bit, since it is not really meant
for production use. What i suspect the I2C version does is that when a
new board info is registers, it walks all the existing I2C busses and
sees if there is a match. The MDIO version is missing this, it only
runs the match when a bus is added. So it is less forgiving of order.

     Andrew

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Florian Fainelli]

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.

> 
> Probably.
> 
> Florian test setup shows it can work.
> 
> This mdio board info stuff is based on the same concept for I2C
> devices. However, it is simplified a bit, since it is not really meant
> for production use. What i suspect the I2C version does is that when a
> new board info is registers, it walks all the existing I2C busses and
> sees if there is a match. The MDIO version is missing this, it only
> runs the match when a bus is added. So it is less forgiving of order.

Yes that is fair enough, there could be improvements in that area. Or 
nowadays, given that software node(s) are a thing and that we can apply 
overlays to PCI(e) devices, we might consider doing that and ditch 
dsa_loop_bdinfo.c entirely.
-- 
Florian

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Florian Fainelli]

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

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Masahiro Yamada]

On Thu, May 16, 2024 at 4:21 AM Florian Fainelli <f.fainelli@gmail.com> 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@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?



"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/".


>
> 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
>


-- 
Best Regards
Masahiro Yamada

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Florian Fainelli]

On 5/15/2024 7:17 PM, Masahiro Yamada wrote:
> On Thu, May 16, 2024 at 4:21 AM Florian Fainelli <f.fainelli@gmail.com> 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@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?
> 
> 
> 
> "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?
-- 
Florian

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Masahiro Yamada]

On Thu, May 16, 2024 at 12:35 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 5/15/2024 7:17 PM, Masahiro Yamada wrote:
> > On Thu, May 16, 2024 at 4:21 AM Florian Fainelli <f.fainelli@gmail.com> 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@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?
> >
> >
> >
> > "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@socionext.com/#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

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Stephen Langstaff]

On Tue, May 14, 2024 at 5:17 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Tue, May 14, 2024 at 05:08:24PM +0100, Stephen Langstaff wrote:
> > If I sorted out building the dsa_loop_bdinfo.c code as a built-in do
> > you think that would solve the ordering issue?
>
> Probably.

Well, some minor progress... when dsa_loop and dsa_loop_bdinfo are
built-in to the kernel then it appears that, at least for this
configuration, the driver probe function is called:

[    0.053068] dsa_loop_bdinfo_init
[    1.509255] dsa_loop_init
[    1.513178] dsa_loop_drv_probe
[    2.633673] dsa_loop_drv_probe
...
[    3.273496] dsa_loop_drv_probe
[    3.276589] dsa_loop_get_protocol
[    3.281608] dsa-loop fixed-0:1f: skipping link registration for CPU port 5
[    3.288558]  (null): phylink: error: empty supported_interfaces
[    3.294500] error creating PHYLINK: -22
[    3.298343] dsa-loop fixed-0:1f lan1 (uninitialized): error -22
setting up PHY for tree 0, switch 0, port 0
[    3.308145]  (null): phylink: error: empty supported_interfaces
[    3.314070] error creating PHYLINK: -22
[    3.317907] dsa-loop fixed-0:1f lan2 (uninitialized): error -22
setting up PHY for tree 0, switch 0, port 1
[    3.327677]  (null): phylink: error: empty supported_interfaces
[    3.333609] error creating PHYLINK: -22
[    3.337449] dsa-loop fixed-0:1f lan3 (uninitialized): error -22
setting up PHY for tree 0, switch 0, port 2
[    3.347223]  (null): phylink: error: empty supported_interfaces
[    3.353145] error creating PHYLINK: -22
[    3.356983] dsa-loop fixed-0:1f lan4 (uninitialized): error -22
setting up PHY for tree 0, switch 0, port 3
[    3.366855] device eth0 entered promiscuous mode
[    3.371497] DSA: tree 0 setup
[    3.374474] dsa-loop fixed-0:1f: DSA mockup driver: 0x1f

The -22 error appears to be related to the issues mentioned here:
https://www.spinics.net/lists/netdev/msg922961.html so I will now try
to apply the suggested short-term fix.

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Florian Fainelli]

On 5/15/24 02:23, Stephen Langstaff wrote:
> On Tue, May 14, 2024 at 5:17 PM Andrew Lunn <andrew@lunn.ch> wrote:
>>
>> On Tue, May 14, 2024 at 05:08:24PM +0100, Stephen Langstaff wrote:
>>> If I sorted out building the dsa_loop_bdinfo.c code as a built-in do
>>> you think that would solve the ordering issue?
>>
>> Probably.
> 
> Well, some minor progress... when dsa_loop and dsa_loop_bdinfo are
> built-in to the kernel then it appears that, at least for this
> configuration, the driver probe function is called:
> 
> [    0.053068] dsa_loop_bdinfo_init
> [    1.509255] dsa_loop_init
> [    1.513178] dsa_loop_drv_probe
> [    2.633673] dsa_loop_drv_probe
> ...
> [    3.273496] dsa_loop_drv_probe
> [    3.276589] dsa_loop_get_protocol
> [    3.281608] dsa-loop fixed-0:1f: skipping link registration for CPU port 5
> [    3.288558]  (null): phylink: error: empty supported_interfaces
> [    3.294500] error creating PHYLINK: -22
> [    3.298343] dsa-loop fixed-0:1f lan1 (uninitialized): error -22
> setting up PHY for tree 0, switch 0, port 0
> [    3.308145]  (null): phylink: error: empty supported_interfaces
> [    3.314070] error creating PHYLINK: -22
> [    3.317907] dsa-loop fixed-0:1f lan2 (uninitialized): error -22
> setting up PHY for tree 0, switch 0, port 1
> [    3.327677]  (null): phylink: error: empty supported_interfaces
> [    3.333609] error creating PHYLINK: -22
> [    3.337449] dsa-loop fixed-0:1f lan3 (uninitialized): error -22
> setting up PHY for tree 0, switch 0, port 2
> [    3.347223]  (null): phylink: error: empty supported_interfaces
> [    3.353145] error creating PHYLINK: -22
> [    3.356983] dsa-loop fixed-0:1f lan4 (uninitialized): error -22
> setting up PHY for tree 0, switch 0, port 3
> [    3.366855] device eth0 entered promiscuous mode
> [    3.371497] DSA: tree 0 setup
> [    3.374474] dsa-loop fixed-0:1f: DSA mockup driver: 0x1f
> 
> The -22 error appears to be related to the issues mentioned here:
> https://www.spinics.net/lists/netdev/msg922961.html so I will now try
> to apply the suggested short-term fix.

What changes have you done to the dsa_loop.c file for this error to show 
up? Currently the driver does the following:

static void dsa_loop_phylink_get_caps(struct dsa_switch *dsa, int port,
                                         struct phylink_config *config)
   {
           bitmap_fill(config->supported_interfaces, 
PHY_INTERFACE_MODE_MAX);
           __clear_bit(PHY_INTERFACE_MODE_NA, config->supported_interfaces);
           config->mac_capabilities = ~0;
   }

which is basically to say: I support everything, except 
PHY_INTERFACE_MDOE_NA.
-- 
Florian

Re: drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Stephen Langstaff]

On Wed, May 15, 2024 at 8:27 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> What changes have you done to the dsa_loop.c file for this error to show
> up? Currently the driver does the following:
>
> static void dsa_loop_phylink_get_caps(struct dsa_switch *dsa, int port,
>                                          struct phylink_config *config)
>    {
>            bitmap_fill(config->supported_interfaces,
> PHY_INTERFACE_MODE_MAX);
>            __clear_bit(PHY_INTERFACE_MODE_NA, config->supported_interfaces);
>            config->mac_capabilities = ~0;
>    }
>
> which is basically to say: I support everything, except
> PHY_INTERFACE_MDOE_NA.

I am currently on kernel version 6.1.55 which does not appear to have
this caps code in, and neither does 6.1.90.

The following commit to net/dsa/port.c, backported to the 6.1 tree,
appears to fix the issue
https://github.com/torvalds/linux/commit/9945c1fb03a3c9f7e0dcf9aa17041a70e551387a

Steve.

drivers/net/dsa/dsa_loop_bdinfo.c build problems

[Stephen Langstaff]

Hi there.

I have come across an issue in the Linux kernel (6.8.7 and others) and
wondered whether I am doing something wrong?

In trying to test the dsa_loop module, I have found that the
dsa_loop_bdinfo module is not built when the following combination of
configuration items is set:

CONFIG_NET_DSA=y
CONFIG_NET_DSA_LOOP=m
CONFIG_FIXED_PHY=y

This combination can easily be obtained using make menuconfig.

It seems that driver/net/Makefile’s inclusion of the dsa directory as
an obj-m precludes the building of dsa_loop_bdinfo.c (which is an
obj-y):

driver/net/Makefile:
…
obj-$(CONFIG_NET_DSA) += dsa/
…

driver/net/dsa/Makefile:
…
ifdef CONFIG_NET_DSA_LOOP
obj-$(CONFIG_FIXED_PHY)         += dsa_loop_bdinfo.o
endif
…