Message ID | 20220928193732.998665-9-tharvey@gateworks.com |
---|---|
State | Superseded |
Delegated to: | Ramon Fried |
Headers | show |
Series | Add MV88E6xxx DSA driver and use on gwventana | expand |
Hi Tim, On Wed, Sep 28, 2022 at 4:37 PM Tim Harvey <tharvey@gateworks.com> wrote: > > Add MV88E61XX DSA support: > - update dt: U-Boot dsa driver requires different device-tree syntax > than the linux driver in order to link the dsa ports to the mdio bus. Shouldn't the U-Boot dts version (imx6qdl-gw5904-u-boot.dtsi) be updated instead? Otherwise, this will cause problems when syncing imx6qdl-gw5904.dtsi with Linux.
On Wed, Sep 28, 2022 at 12:56 PM Fabio Estevam <festevam@gmail.com> wrote: > > Hi Tim, > > On Wed, Sep 28, 2022 at 4:37 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > > Add MV88E61XX DSA support: > > - update dt: U-Boot dsa driver requires different device-tree syntax > > than the linux driver in order to link the dsa ports to the mdio bus. > > Shouldn't the U-Boot dts version (imx6qdl-gw5904-u-boot.dtsi) be > updated instead? > > Otherwise, this will cause problems when syncing imx6qdl-gw5904.dtsi > with Linux. Fabio, That is one route to go but I'm hoping to update the Linux dt as well to match so I'm not worried about it at this time. Best Regards, Tim
Hi Tim, On Wed, Sep 28, 2022 at 4:37 PM Tim Harvey <tharvey@gateworks.com> wrote: > diff --git a/configs/gwventana_gw5904_defconfig b/configs/gwventana_gw5904_defconfig > index 8c6baf0ae5e2..33b57c5a1c5d 100644 > --- a/configs/gwventana_gw5904_defconfig > +++ b/configs/gwventana_gw5904_defconfig > @@ -111,11 +111,11 @@ CONFIG_SUPPORT_EMMC_BOOT=y > CONFIG_FSL_USDHC=y > CONFIG_MTD=y > CONFIG_PHYLIB=y > -CONFIG_MV88E61XX_SWITCH=y > -CONFIG_MV88E61XX_CPU_PORT=5 > -CONFIG_MV88E61XX_PHY_PORTS=0xf > -CONFIG_MV88E61XX_FIXED_PORTS=0x0 > +CONFIG_MV88E61XX=y There is a typo here: This one should be CONFIG_MV88E6XXX instead. After fixing this and passing CONFIG_NET_RANDOM_ETHADDR=y to my custom board defconfig: ethernet 0 [ + ] fecmxc | | `-- ethernet@30be0000 bootdev 2 [ ] eth_bootdev | | |-- ethernet@30be0000.bootdev mdio 0 [ + ] fec_mdio | | `-- mdio dsa 0 [ ] mv88e6xxx | | `-- switch@0 ethernet 1 [ ] dsa-port | | |-- lan4 bootdev 3 [ ] eth_bootdev | | | `-- switch@0@0.bootdev ethernet 2 [ ] dsa-port | | |-- lan3 bootdev 4 [ ] eth_bootdev | | | `-- switch@0@1.bootdev ethernet 3 [ ] dsa-port | | |-- lan2 bootdev 5 [ ] eth_bootdev | | | `-- switch@0@2.bootdev ethernet 4 [ ] dsa-port | | |-- lan1 bootdev 6 [ ] eth_bootdev | | | `-- switch@0@3.bootdev mdio 1 [ + ] mv88e6xxx_mdio | | |-- mv88e6xxx-mdio-0 mdio 2 [ ] mv88e6xxx_mdio | | |-- mv88e6xxx-mdio-1 mdio 3 [ ] mv88e6xxx_mdio | | |-- mv88e6xxx-mdio-2 mdio 4 [ ] mv88e6xxx_mdio | | `-- mv88e6xxx-mdio-3 but still not able to get a DHCP address: u-boot=> dhcp BOOTP broadcast 1 BOOTP broadcast 2 BOOTP broadcast 3 BOOTP broadcast 4 Thanks
On Mon, Oct 3, 2022 at 10:13 AM Fabio Estevam <festevam@gmail.com> wrote: > > Hi Tim, > > On Wed, Sep 28, 2022 at 4:37 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > diff --git a/configs/gwventana_gw5904_defconfig b/configs/gwventana_gw5904_defconfig > > index 8c6baf0ae5e2..33b57c5a1c5d 100644 > > --- a/configs/gwventana_gw5904_defconfig > > +++ b/configs/gwventana_gw5904_defconfig > > @@ -111,11 +111,11 @@ CONFIG_SUPPORT_EMMC_BOOT=y > > CONFIG_FSL_USDHC=y > > CONFIG_MTD=y > > CONFIG_PHYLIB=y > > -CONFIG_MV88E61XX_SWITCH=y > > -CONFIG_MV88E61XX_CPU_PORT=5 > > -CONFIG_MV88E61XX_PHY_PORTS=0xf > > -CONFIG_MV88E61XX_FIXED_PORTS=0x0 > > +CONFIG_MV88E61XX=y > > There is a typo here: > > This one should be CONFIG_MV88E6XXX instead. Ooops - thanks for pointing that out. I have another patch on top to merge two defconfigs that fixed that typo introduced with the v4 series that changed MV88E61XX to MV88E6XXX. > > After fixing this and passing CONFIG_NET_RANDOM_ETHADDR=y to my custom > board defconfig: > > ethernet 0 [ + ] fecmxc | | `-- ethernet@30be0000 > bootdev 2 [ ] eth_bootdev | | |-- > ethernet@30be0000.bootdev > mdio 0 [ + ] fec_mdio | | `-- mdio > dsa 0 [ ] mv88e6xxx | | `-- switch@0 > ethernet 1 [ ] dsa-port | | |-- lan4 > bootdev 3 [ ] eth_bootdev | | | > `-- switch@0@0.bootdev > ethernet 2 [ ] dsa-port | | |-- lan3 > bootdev 4 [ ] eth_bootdev | | | > `-- switch@0@1.bootdev > ethernet 3 [ ] dsa-port | | |-- lan2 > bootdev 5 [ ] eth_bootdev | | | > `-- switch@0@2.bootdev > ethernet 4 [ ] dsa-port | | |-- lan1 > bootdev 6 [ ] eth_bootdev | | | > `-- switch@0@3.bootdev > mdio 1 [ + ] mv88e6xxx_mdio | | > |-- mv88e6xxx-mdio-0 > mdio 2 [ ] mv88e6xxx_mdio | | > |-- mv88e6xxx-mdio-1 > mdio 3 [ ] mv88e6xxx_mdio | | > |-- mv88e6xxx-mdio-2 > mdio 4 [ ] mv88e6xxx_mdio | | > `-- mv88e6xxx-mdio-3 > > but still not able to get a DHCP address: > > u-boot=> dhcp > BOOTP broadcast 1 > BOOTP broadcast 2 > BOOTP broadcast 3 > BOOTP broadcast 4 With CONFIG_CMD_NET=y what does 'net list' show? I have the following (with network attached to lan4 port) Net: MV88E61XX eth0: ethernet@2188000, eth1: lan4, eth2: lan3, eth3: lan2, eth 4: lan1 Hit any key to stop autoboot: 0 Ventana > print ethaddr ethaddr=00:d0:12:f3:f2:f5 Ventana > dm tree ... ethernet 0 [ + ] fecmxc | | |-- ethernet@2188000 bootdev 0 [ ] eth_bootdev | | | |-- ethernet@2188000.bootdev mdio 0 [ + ] fec_mdio | | | `-- mdio dsa 0 [ + ] mv88e6xxx | | | `-- switch@0 ethernet 1 [ + ] dsa-port | | | |-- lan4 bootdev 2 [ ] eth_bootdev | | | | `-- switch@0@0.bootdev ethernet 2 [ + ] dsa-port | | | |-- lan3 bootdev 3 [ ] eth_bootdev | | | | `-- switch@0@1.bootdev ethernet 3 [ + ] dsa-port | | | |-- lan2 bootdev 4 [ ] eth_bootdev | | | | `-- switch@0@2.bootdev ethernet 4 [ + ] dsa-port | | | |-- lan1 bootdev 5 [ ] eth_bootdev | | | | `-- switch@0@3.bootdev mdio 1 [ + ] mv88e6xxx_mdio | | | `-- mv88e6xxx-mdio-0 ... Ventana > net list eth0 : ethernet@2188000 00:d0:12:f3:f2:f5 active eth1 : lan4 00:d0:12:f3:f2:f5 eth2 : lan3 00:d0:12:f3:f2:f5 eth3 : lan2 00:d0:12:f3:f2:f5 eth4 : lan1 00:d0:12:f3:f2:f5 Ventana > setenv ethact lan4 Ventana > dhcp BOOTP broadcast 1 BOOTP broadcast 2 BOOTP broadcast 3 DHCP client bound to address 172.24.33.142 (1076 ms) I do not use CONFIG_NET_RANDOM_ETHADDR and have a single MAC address allocated to the board. What board are you using and what does its fec/mdio dt look like? Best Regards, Tim
Hi Tim, On Mon, Oct 3, 2022 at 3:22 PM Tim Harvey <tharvey@gateworks.com> wrote: > With CONFIG_CMD_NET=y what does 'net list' show? > > I have the following (with network attached to lan4 port) > Net: MV88E61XX eth0: ethernet@2188000, eth1: lan4, eth2: lan3, eth3: lan2, eth > 4: lan1 > Hit any key to stop autoboot: 0 > Ventana > print ethaddr > ethaddr=00:d0:12:f3:f2:f5 > Ventana > dm tree > ... > ethernet 0 [ + ] fecmxc | | |-- ethernet@2188000 > bootdev 0 [ ] eth_bootdev | | | |-- > ethernet@2188000.bootdev > mdio 0 [ + ] fec_mdio | | | `-- mdio > dsa 0 [ + ] mv88e6xxx | | | `-- switch@0 > ethernet 1 [ + ] dsa-port | | | |-- lan4 > bootdev 2 [ ] eth_bootdev | | | | > `-- switch@0@0.bootdev > ethernet 2 [ + ] dsa-port | | | |-- lan3 > bootdev 3 [ ] eth_bootdev | | | | > `-- switch@0@1.bootdev > ethernet 3 [ + ] dsa-port | | | |-- lan2 > bootdev 4 [ ] eth_bootdev | | | | > `-- switch@0@2.bootdev > ethernet 4 [ + ] dsa-port | | | |-- lan1 > bootdev 5 [ ] eth_bootdev | | | | > `-- switch@0@3.bootdev > mdio 1 [ + ] mv88e6xxx_mdio | | | > `-- mv88e6xxx-mdio-0 > ... > Ventana > net list > eth0 : ethernet@2188000 00:d0:12:f3:f2:f5 active > eth1 : lan4 00:d0:12:f3:f2:f5 > eth2 : lan3 00:d0:12:f3:f2:f5 > eth3 : lan2 00:d0:12:f3:f2:f5 > eth4 : lan1 00:d0:12:f3:f2:f5 > Ventana > setenv ethact lan4 > Ventana > dhcp > BOOTP broadcast 1 > BOOTP broadcast 2 > BOOTP broadcast 3 > DHCP client bound to address 172.24.33.142 (1076 ms) > > I do not use CONFIG_NET_RANDOM_ETHADDR and have a single MAC address > allocated to the board. Below are the logs: Net: MV88E61XX Warning: ethernet@30be0000 (eth0) using random MAC address - a6:a6:56:1c:28:0a eth0: ethernet@30be0000 Hit any key to stop autoboot: 0 Your log also shows eth1-eth4. Mine does not. Unlike your log, dsa-port is not getting probed here: ethernet 0 [ + ] fecmxc | | `-- ethernet@30be0000 bootdev 2 [ ] eth_bootdev | | |-- ethernet@30be0000.bootdev mdio 0 [ + ] fec_mdio | | `-- mdio dsa 0 [ ] mv88e6xxx | | `-- switch@0 ethernet 1 [ ] dsa-port | | |-- lan4 bootdev 3 [ ] eth_bootdev | | | `-- switch@0@0.bootdev ethernet 2 [ ] dsa-port | | |-- lan3 bootdev 4 [ ] eth_bootdev | | | `-- switch@0@1.bootdev ethernet 3 [ ] dsa-port | | |-- lan2 bootdev 5 [ ] eth_bootdev | | | `-- switch@0@2.bootdev ethernet 4 [ ] dsa-port | | |-- lan1 bootdev 6 [ ] eth_bootdev | | | `-- switch@0@3.bootdev mdio 1 [ + ] mv88e6xxx_mdio | | |-- mv88e6xxx-mdio-0 mdio 2 [ ] mv88e6xxx_mdio | | |-- mv88e6xxx-mdio-1 mdio 3 [ ] mv88e6xxx_mdio | | |-- mv88e6xxx-mdio-2 mdio 4 [ ] mv88e6xxx_mdio | | `-- mv88e6xxx-mdio-3 simple_bus 7 [ ] simple_bus | `-- bus@32c00000 usb 0 [ ] ehci_mx6 | `-- usb@32e40000 phy 0 [ ] nop_phy |-- usbphynop1 regulator 0 [ ] regulator_fixed |-- regulator-usdhc2 sysreset 1 [ ] wdt_reboot |-- wdt-reboot bootstd 0 [ ] bootstd_drv `-- bootstd bootmeth 0 [ ] bootmeth_distro |-- distro bootmeth 1 [ ] bootmeth_efi |-- efi bootmeth 2 [ ] bootmeth_pxe |-- pxe bootmeth 3 [ ] vbe_simple `-- vbe_simple u-boot=> net list eth0 : ethernet@30be0000 a6:a6:56:1c:28:0a active eth1 : lan4 00:00:00:00:00:00 eth2 : lan3 00:00:00:00:00:00 eth3 : lan2 00:00:00:00:00:00 eth4 : lan1 00:00:00:00:00:00 u-boot=> setenv ethact lan4 u-boot=> dhcp BOOTP broadcast 1 BOOTP broadcast 2 BOOTP broadcast 3 BOOTP broadcast 4 BOOTP broadcast 5 BOOTP broadcast 6 BOOTP broadcast 7 BOOTP broadcast 8 (IP is never retrieved) > What board are you using and what does its fec/mdio dt look like? I am using a custom imx8mn board. The FEC/MDIO nodes are like this: https://pastebin.com/raw/0SvLLzxj Thanks, Fabio Estevam
On Mon, Oct 3, 2022 at 11:40 AM Fabio Estevam <festevam@gmail.com> wrote: > > Hi Tim, > > On Mon, Oct 3, 2022 at 3:22 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > With CONFIG_CMD_NET=y what does 'net list' show? > > > > I have the following (with network attached to lan4 port) > > Net: MV88E61XX eth0: ethernet@2188000, eth1: lan4, eth2: lan3, eth3: lan2, eth > > 4: lan1 > > Hit any key to stop autoboot: 0 > > Ventana > print ethaddr > > ethaddr=00:d0:12:f3:f2:f5 > > Ventana > dm tree > > ... > > ethernet 0 [ + ] fecmxc | | |-- ethernet@2188000 > > bootdev 0 [ ] eth_bootdev | | | |-- > > ethernet@2188000.bootdev > > mdio 0 [ + ] fec_mdio | | | `-- mdio > > dsa 0 [ + ] mv88e6xxx | | | `-- switch@0 > > ethernet 1 [ + ] dsa-port | | | |-- lan4 > > bootdev 2 [ ] eth_bootdev | | | | > > `-- switch@0@0.bootdev > > ethernet 2 [ + ] dsa-port | | | |-- lan3 > > bootdev 3 [ ] eth_bootdev | | | | > > `-- switch@0@1.bootdev > > ethernet 3 [ + ] dsa-port | | | |-- lan2 > > bootdev 4 [ ] eth_bootdev | | | | > > `-- switch@0@2.bootdev > > ethernet 4 [ + ] dsa-port | | | |-- lan1 > > bootdev 5 [ ] eth_bootdev | | | | > > `-- switch@0@3.bootdev > > mdio 1 [ + ] mv88e6xxx_mdio | | | > > `-- mv88e6xxx-mdio-0 > > ... > > Ventana > net list > > eth0 : ethernet@2188000 00:d0:12:f3:f2:f5 active > > eth1 : lan4 00:d0:12:f3:f2:f5 > > eth2 : lan3 00:d0:12:f3:f2:f5 > > eth3 : lan2 00:d0:12:f3:f2:f5 > > eth4 : lan1 00:d0:12:f3:f2:f5 > > Ventana > setenv ethact lan4 > > Ventana > dhcp > > BOOTP broadcast 1 > > BOOTP broadcast 2 > > BOOTP broadcast 3 > > DHCP client bound to address 172.24.33.142 (1076 ms) > > > > I do not use CONFIG_NET_RANDOM_ETHADDR and have a single MAC address > > allocated to the board. > > Below are the logs: > > Net: MV88E61XX > Warning: ethernet@30be0000 (eth0) using random MAC address - a6:a6:56:1c:28:0a > eth0: ethernet@30be0000 > Hit any key to stop autoboot: 0 > > Your log also shows eth1-eth4. Mine does not. > > Unlike your log, dsa-port is not getting probed here: > > ethernet 0 [ + ] fecmxc | | `-- ethernet@30be0000 > bootdev 2 [ ] eth_bootdev | | |-- > ethernet@30be0000.bootdev > mdio 0 [ + ] fec_mdio | | `-- mdio > dsa 0 [ ] mv88e6xxx | | `-- switch@0 > ethernet 1 [ ] dsa-port | | |-- lan4 > bootdev 3 [ ] eth_bootdev | | | > `-- switch@0@0.bootdev > ethernet 2 [ ] dsa-port | | |-- lan3 > bootdev 4 [ ] eth_bootdev | | | > `-- switch@0@1.bootdev > ethernet 3 [ ] dsa-port | | |-- lan2 > bootdev 5 [ ] eth_bootdev | | | > `-- switch@0@2.bootdev > ethernet 4 [ ] dsa-port | | |-- lan1 > bootdev 6 [ ] eth_bootdev | | | > `-- switch@0@3.bootdev > mdio 1 [ + ] mv88e6xxx_mdio | | > |-- mv88e6xxx-mdio-0 > mdio 2 [ ] mv88e6xxx_mdio | | > |-- mv88e6xxx-mdio-1 > mdio 3 [ ] mv88e6xxx_mdio | | > |-- mv88e6xxx-mdio-2 > mdio 4 [ ] mv88e6xxx_mdio | | > `-- mv88e6xxx-mdio-3 > simple_bus 7 [ ] simple_bus | `-- bus@32c00000 > usb 0 [ ] ehci_mx6 | `-- usb@32e40000 > phy 0 [ ] nop_phy |-- usbphynop1 > regulator 0 [ ] regulator_fixed |-- regulator-usdhc2 > sysreset 1 [ ] wdt_reboot |-- wdt-reboot > bootstd 0 [ ] bootstd_drv `-- bootstd > bootmeth 0 [ ] bootmeth_distro |-- distro > bootmeth 1 [ ] bootmeth_efi |-- efi > bootmeth 2 [ ] bootmeth_pxe |-- pxe > bootmeth 3 [ ] vbe_simple `-- vbe_simple > Right - so it looks like the 'net: fec: add support for DM_MDIO' patch is not doing its job to probe the mdio bus and ports. Is CONFIG_DM_MDIO enabled and is dm_fec_bind_mdio returning successfully? Is CONFIG_DM_ETH_PHY enabled? I don't use that and if I recall that code never seemed right to me. Tim > u-boot=> net list > eth0 : ethernet@30be0000 a6:a6:56:1c:28:0a active > eth1 : lan4 00:00:00:00:00:00 > eth2 : lan3 00:00:00:00:00:00 > eth3 : lan2 00:00:00:00:00:00 > eth4 : lan1 00:00:00:00:00:00 > u-boot=> setenv ethact lan4 > u-boot=> dhcp > BOOTP broadcast 1 > BOOTP broadcast 2 > BOOTP broadcast 3 > BOOTP broadcast 4 > BOOTP broadcast 5 > BOOTP broadcast 6 > BOOTP broadcast 7 > BOOTP broadcast 8 > (IP is never retrieved) > > > What board are you using and what does its fec/mdio dt look like? > > I am using a custom imx8mn board. > > The FEC/MDIO nodes are like this: > https://pastebin.com/raw/0SvLLzxj > > Thanks, > > Fabio Estevam
On 03/10/2022 16:09, Tim Harvey wrote: > Right - so it looks like the 'net: fec: add support for DM_MDIO' patch > is not doing its job to probe the mdio bus and ports. > > Is CONFIG_DM_MDIO enabled and is dm_fec_bind_mdio returning > successfully? Yes, for both. > Is CONFIG_DM_ETH_PHY enabled? I don't use that and if I recall that > code never seemed right to me. CONFIG_DM_ETH_PHY is not enabled. Regards, Fabio Estevam
On Mon, Oct 3, 2022 at 3:35 PM Fabio Estevam <festevam@denx.de> wrote: > > On 03/10/2022 16:09, Tim Harvey wrote: > > > Right - so it looks like the 'net: fec: add support for DM_MDIO' patch > > is not doing its job to probe the mdio bus and ports. > > > > Is CONFIG_DM_MDIO enabled and is dm_fec_bind_mdio returning > > successfully? > > Yes, for both. > > > > Is CONFIG_DM_ETH_PHY enabled? I don't use that and if I recall that > > code never seemed right to me. > > CONFIG_DM_ETH_PHY is not enabled. > Fabio, I'm not sure why your configuration is behaving differently and think you'll have to instrument code to help understand it. Here is the chain of events I see on my imx6q fec <-> mv88e6085 configuration: fecmxc_probe() calls dm_fec_bind_mdio() which finds the mdio node in dt and binds the 'fec_mdio' driver to it then probes that driver. When it binds via device_bind_driver_to_node() it should end up calling dsa_post_bind() to bind to the dsa_port driver. Can you verify that you have DM_DSA enabled and that dsa_post_bind() gets called? You can enable debug for dsa-uclass.c and perhaps that will explain more. In dsa_post_bind() there is a loop over pdata->num_ports which calls device_bind_driver_to_node() to bind the node to DSA_PORT_CHILD_DRV_NAME - I imagine that isn't happening due to one of the checks prior to this failing silently. Tim
Hi Tim, On Mon, Oct 3, 2022 at 8:39 PM Tim Harvey <tharvey@gateworks.com> wrote: > Fabio, > > I'm not sure why your configuration is behaving differently and think > you'll have to instrument code to help understand it. > > Here is the chain of events I see on my imx6q fec <-> mv88e6085 configuration: The switch I am using is a mv88e6320. I instrumented the code a bit and I notice that I am not able to read the switch ID. mv88e6xxx_get_switch_id() fails because mv88e6xxx_port_read() returns 0xffff. I am investigating this failure. Thanks
On Mon, Oct 3, 2022 at 8:46 PM Fabio Estevam <festevam@gmail.com> wrote: > The switch I am using is a mv88e6320. > > I instrumented the code a bit and I notice that I am not able to read > the switch ID. > > mv88e6xxx_get_switch_id() fails because mv88e6xxx_port_read() returns 0xffff. This was caused by the wrong description of the Ethernet switch reset. With this fixed, dm tree looks a bit better: ethernet 0 [ + ] fecmxc | | `-- ethernet@30be0000 bootdev 2 [ ] eth_bootdev | | |-- ethernet@30be0000.bootdev mdio 0 [ + ] fec_mdio | | `-- mdio dsa 0 [ + ] mv88e6xxx | | `-- switch@0 ethernet 1 [ ] dsa-port | | |-- lan0 bootdev 3 [ ] eth_bootdev | | | `-- switch@0@0.bootdev ethernet 2 [ ] dsa-port | | |-- lan1 bootdev 4 [ ] eth_bootdev | | | `-- switch@0@1.bootdev ethernet 3 [ + ] dsa-port | | |-- lan3 bootdev 5 [ ] eth_bootdev | | | `-- switch@0@3.bootdev mdio 1 [ + ] mv88e6xxx_mdio | | `-- mv88e6xxx-mdio-0 Boot log: Net: MV88E61XX Warning: ethernet@30be0000 (eth0) using random MAC address - 36:6f:7f:0c:61:14 eth0: ethernet@30be0000Could not get PHY for mv88e6xxx-mdio-0: addr 0 Could not get PHY for mv88e6xxx-mdio-0: addr 1 , eth3: lan3 u-boot=> net list eth0 : ethernet@30be0000 36:6f:7f:0c:61:14 active eth1 : lan0 00:00:00:00:00:00 eth2 : lan1 00:00:00:00:00:00 eth3 : lan3 36:6f:7f:0c:61:14 but still not able to get DHCP IP.
On Mon, Oct 3, 2022 at 5:15 PM Fabio Estevam <festevam@gmail.com> wrote: > > On Mon, Oct 3, 2022 at 8:46 PM Fabio Estevam <festevam@gmail.com> wrote: > > > The switch I am using is a mv88e6320. > > > > I instrumented the code a bit and I notice that I am not able to read > > the switch ID. > > > > mv88e6xxx_get_switch_id() fails because mv88e6xxx_port_read() returns 0xffff. > > This was caused by the wrong description of the Ethernet switch reset. Seems like something that needs an error print? Which driver this? > > With this fixed, dm tree looks a bit better: > > ethernet 0 [ + ] fecmxc | | `-- ethernet@30be0000 > bootdev 2 [ ] eth_bootdev | | |-- > ethernet@30be0000.bootdev > mdio 0 [ + ] fec_mdio | | `-- mdio > dsa 0 [ + ] mv88e6xxx | | `-- switch@0 > ethernet 1 [ ] dsa-port | | |-- lan0 > bootdev 3 [ ] eth_bootdev | | | > `-- switch@0@0.bootdev > ethernet 2 [ ] dsa-port | | |-- lan1 > bootdev 4 [ ] eth_bootdev | | | > `-- switch@0@1.bootdev > ethernet 3 [ + ] dsa-port | | |-- lan3 > bootdev 5 [ ] eth_bootdev | | | > `-- switch@0@3.bootdev > mdio 1 [ + ] mv88e6xxx_mdio | | > `-- mv88e6xxx-mdio-0 > > Boot log: > Net: MV88E61XX > Warning: ethernet@30be0000 (eth0) using random MAC address - 36:6f:7f:0c:61:14 > eth0: ethernet@30be0000Could not get PHY for mv88e6xxx-mdio-0: addr 0 > Could not get PHY for mv88e6xxx-mdio-0: addr 1 > , eth3: lan3 This doesn't look correct - why could it not get the phy for lan1 and lan2 yet lan3 was fine? I would assume a DT port/reg mismatch. > > u-boot=> net list > eth0 : ethernet@30be0000 36:6f:7f:0c:61:14 active > eth1 : lan0 00:00:00:00:00:00 > eth2 : lan1 00:00:00:00:00:00 > eth3 : lan3 36:6f:7f:0c:61:14 > > but still not able to get DHCP IP. Where are you setting your active port? By default eth0 is active and thats the cpu uplink. If you have a network connection on lan3 port you would need to 'setenv ethact lan3'. Tim
Hi Tim, On Mon, Oct 3, 2022 at 9:23 PM Tim Harvey <tharvey@gateworks.com> wrote: > Where are you setting your active port? By default eth0 is active and > thats the cpu uplink. If you have a network connection on lan3 port > you would need to 'setenv ethact lan3'. It works now: both 'dhcp' and 'tftp' commands are functional. Thanks a lot for your work on this and for your help! If you send a v5 with the CONFIG_MV88E6XXX=y typo fixed, feel free to add for the whole series: Reviewed-by: Fabio Estevam <festevam@denx.de>
On Mon, Oct 3, 2022 at 5:39 PM Fabio Estevam <festevam@gmail.com> wrote: > > Hi Tim, > > On Mon, Oct 3, 2022 at 9:23 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > Where are you setting your active port? By default eth0 is active and > > thats the cpu uplink. If you have a network connection on lan3 port > > you would need to 'setenv ethact lan3'. > > It works now: both 'dhcp' and 'tftp' commands are functional. > > Thanks a lot for your work on this and for your help! > > If you send a v5 with the CONFIG_MV88E6XXX=y typo fixed, > feel free to add for the whole series: > > Reviewed-by: Fabio Estevam <festevam@denx.de> Fabio, Thanks for testing this. I'll submit a v5 shortly with your rb tag and support for 6320: --- a/drivers/net/mv88e6xxx.c +++ b/drivers/net/mv88e6xxx.c @@ -176,6 +176,7 @@ #define PORT_SWITCH_ID_6220 0x2200 #define PORT_SWITCH_ID_6240 0x2400 #define PORT_SWITCH_ID_6250 0x2500 +#define PORT_SWITCH_ID_6320 0x1150 #define PORT_SWITCH_ID_6352 0x3520 struct mv88e6xxx_priv { @@ -792,6 +793,7 @@ static int mv88e6xxx_probe(struct udevice *dev) case PORT_SWITCH_ID_6071: case PORT_SWITCH_ID_6220: case PORT_SWITCH_ID_6250: + case PORT_SWITCH_ID_6320: priv->port_count = 7; priv->phy_ctrl1_en_det_shift = 14; priv->phy_ctrl1_en_det_width = 1; Was there an error path you found in drivers/net/mv88e6xxx.c that should print an error to explain a failed probe for better future troubleshooting or was that failure somewhere else? It sounds like your switch was held in reset such that register reads were failing. Best Regards, Tim
Hi Tim, On Tue, Oct 4, 2022 at 12:26 PM Tim Harvey <tharvey@gateworks.com> wrote: > Fabio, > > Thanks for testing this. I'll submit a v5 shortly with your rb tag and > support for 6320: Thanks for including the 6320 model! > Was there an error path you found in drivers/net/mv88e6xxx.c that > should print an error to explain a failed probe for better future > troubleshooting or was that failure somewhere else? It sounds like > your switch was held in reset such that register reads were failing. Yes, that was the reason for the register read failure. Initially, I had described the reset GPIO inside using reset-gpios inside the switch node, just like described in the Linux binding document: Documentation/devicetree/bindings/net/dsa/marvell.txt It did not work, so I moved it to phy-reset-gpios inside the fec node. I don't think this is a blocker for your v5 series. We can improve it later so that the binding from Linux could also work in U-Boot. Thanks
Hi Tim, On Tue, Oct 4, 2022 at 1:25 PM Fabio Estevam <festevam@gmail.com> wrote: > > Hi Tim, > > On Tue, Oct 4, 2022 at 12:26 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > Fabio, > > > > Thanks for testing this. I'll submit a v5 shortly with your rb tag and > > support for 6320: > > Thanks for including the 6320 model! > > > Was there an error path you found in drivers/net/mv88e6xxx.c that > > should print an error to explain a failed probe for better future > > troubleshooting or was that failure somewhere else? It sounds like I forgot to reply to this one. Yes, it would help if we print an error like this: --- a/drivers/net/mv88e6xxx.c +++ b/drivers/net/mv88e6xxx.c @@ -410,8 +410,10 @@ static int mv88e6xxx_get_switch_id(struct udevice *dev) int res; res = mv88e6xxx_port_read(dev, 0, PORT_REG_SWITCH_ID); - if (res < 0) + if (res < 0) { + dev_err(dev, "Failed to read switch ID: %d\n", res); return res; + } return res & 0xfff0; } Thanks
On Tue, Oct 4, 2022 at 9:47 AM Fabio Estevam <festevam@gmail.com> wrote: > > Hi Tim, > > On Tue, Oct 4, 2022 at 1:25 PM Fabio Estevam <festevam@gmail.com> wrote: > > > > Hi Tim, > > > > On Tue, Oct 4, 2022 at 12:26 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > > > Fabio, > > > > > > Thanks for testing this. I'll submit a v5 shortly with your rb tag and > > > support for 6320: > > > > Thanks for including the 6320 model! > > > > > Was there an error path you found in drivers/net/mv88e6xxx.c that > > > should print an error to explain a failed probe for better future > > > troubleshooting or was that failure somewhere else? It sounds like > > I forgot to reply to this one. Yes, it would help if we print an error like > this: > > --- a/drivers/net/mv88e6xxx.c > +++ b/drivers/net/mv88e6xxx.c > @@ -410,8 +410,10 @@ static int mv88e6xxx_get_switch_id(struct udevice *dev) > int res; > > res = mv88e6xxx_port_read(dev, 0, PORT_REG_SWITCH_ID); > - if (res < 0) > + if (res < 0) { > + dev_err(dev, "Failed to read switch ID: %d\n", res); > return res; > + } > return res & 0xfff0; > } > > Thanks Fabio, I just sent a v5 before seeing this. I think all we are waiting for on this series is for Vladimir to ok the dt changes as I believe I have finally done the right thing there now. If I need to do a v6 I'll add this in, otherwise I'll send it after it gets accepted and merged. Best Regards, Tim
Hi Tim, On 04/10/2022 13:50, Tim Harvey wrote: > Fabio, > > I just sent a v5 before seeing this. I think all we are waiting for on > this series is for Vladimir to ok the dt changes as I believe I have > finally done the right thing there now. > > If I need to do a v6 I'll add this in, otherwise I'll send it after it > gets accepted and merged. That's totally fine. Thanks!
diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi b/arch/arm/dts/imx6qdl-gw5904.dtsi index 612b6e068e28..487dd7648274 100644 --- a/arch/arm/dts/imx6qdl-gw5904.dtsi +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi @@ -212,6 +212,27 @@ compatible = "marvell,mv88e6085"; reg = <0>; + mdio { + #address-cells = <1>; + #size-cells = <0>; + + sw_phy0: ethernet-phy@0 { + reg = <0x0>; + }; + + sw_phy1: ethernet-phy@1 { + reg = <0x1>; + }; + + sw_phy2: ethernet-phy@2 { + reg = <0x2>; + }; + + sw_phy3: ethernet-phy@3 { + reg = <0x3>; + }; + }; + ports { #address-cells = <1>; #size-cells = <0>; @@ -219,27 +240,37 @@ port@0 { reg = <0>; label = "lan4"; + phy-handle = <&sw_phy0>; }; port@1 { reg = <1>; label = "lan3"; + phy-handle = <&sw_phy1>; }; port@2 { reg = <2>; label = "lan2"; + phy-handle = <&sw_phy2>; }; port@3 { reg = <3>; label = "lan1"; + phy-handle = <&sw_phy3>; }; port@5 { reg = <5>; label = "cpu"; ethernet = <&fec>; + phy-mode = "rgmii-id"; + + fixed-link { + speed = <1000>; + full-duplex; + }; }; }; }; diff --git a/board/gateworks/gw_ventana/gw_ventana.c b/board/gateworks/gw_ventana/gw_ventana.c index 99f52b9953e2..1c3da2c82707 100644 --- a/board/gateworks/gw_ventana/gw_ventana.c +++ b/board/gateworks/gw_ventana/gw_ventana.c @@ -83,44 +83,30 @@ int board_phy_config(struct phy_device *phydev) break; } + /* Fixed PHY: for GW5904/GW5909 this is Marvell 88E6176 GbE Switch */ + if (phydev->phy_id == PHY_FIXED_ID && + (board_type == GW5904 || board_type == GW5909)) { + struct mii_dev *bus = miiphy_get_dev_by_name("mdio"); + + puts("MV88E61XX "); + /* GPIO[0] output CLK125 for RGMII_REFCLK */ + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x62 << 8) | 0xfe); + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x68 << 8) | 7); + + /* Port 0-3 LED configuration: Table 80/82 */ + /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ + bus->write(bus, 0x10, 0, 0x16, 0x8088); + bus->write(bus, 0x11, 0, 0x16, 0x8088); + bus->write(bus, 0x12, 0, 0x16, 0x8088); + bus->write(bus, 0x13, 0, 0x16, 0x8088); + } + if (phydev->drv->config) phydev->drv->config(phydev); return 0; } -#ifdef CONFIG_MV88E61XX_SWITCH -int mv88e61xx_hw_reset(struct phy_device *phydev) -{ - struct mii_dev *bus = phydev->bus; - - /* GPIO[0] output, CLK125 */ - debug("enabling RGMII_REFCLK\n"); - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, - 0x1a /*MV_SCRATCH_MISC*/, - (1 << 15) | (0x62 /*MV_GPIO_DIR*/ << 8) | 0xfe); - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, - 0x1a /*MV_SCRATCH_MISC*/, - (1 << 15) | (0x68 /*MV_GPIO01_CNTL*/ << 8) | 7); - - /* RGMII delay - Physical Control register bit[15:14] */ - debug("setting port%d RGMII rx/tx delay\n", CONFIG_MV88E61XX_CPU_PORT); - /* forced 1000mbps full-duplex link */ - bus->write(bus, 0x10 + CONFIG_MV88E61XX_CPU_PORT, 0, 1, 0xc0fe); - phydev->autoneg = AUTONEG_DISABLE; - phydev->speed = SPEED_1000; - phydev->duplex = DUPLEX_FULL; - - /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ - bus->write(bus, 0x10, 0, 0x16, 0x8088); - bus->write(bus, 0x11, 0, 0x16, 0x8088); - bus->write(bus, 0x12, 0, 0x16, 0x8088); - bus->write(bus, 0x13, 0, 0x16, 0x8088); - - return 0; -} -#endif // CONFIG_MV88E61XX_SWITCH - #if defined(CONFIG_VIDEO_IPUV3) static void enable_hdmi(struct display_info_t const *dev) { diff --git a/configs/gwventana_gw5904_defconfig b/configs/gwventana_gw5904_defconfig index 8c6baf0ae5e2..33b57c5a1c5d 100644 --- a/configs/gwventana_gw5904_defconfig +++ b/configs/gwventana_gw5904_defconfig @@ -111,11 +111,11 @@ CONFIG_SUPPORT_EMMC_BOOT=y CONFIG_FSL_USDHC=y CONFIG_MTD=y CONFIG_PHYLIB=y -CONFIG_MV88E61XX_SWITCH=y -CONFIG_MV88E61XX_CPU_PORT=5 -CONFIG_MV88E61XX_PHY_PORTS=0xf -CONFIG_MV88E61XX_FIXED_PORTS=0x0 +CONFIG_MV88E61XX=y +CONFIG_PHY_FIXED=y +CONFIG_DM_ETH=y CONFIG_DM_MDIO=y +CONFIG_DM_DSA=y CONFIG_E1000=y CONFIG_FEC_MXC=y CONFIG_MII=y diff --git a/drivers/net/mv88e6xxx.c b/drivers/net/mv88e6xxx.c index a3a3f6cf4acc..6064bb73257b 100644 --- a/drivers/net/mv88e6xxx.c +++ b/drivers/net/mv88e6xxx.c @@ -727,31 +727,26 @@ static const struct dsa_ops mv88e6xxx_dsa_ops = { static int mv88e6xxx_probe_mdio(struct udevice *dev) { struct udevice *mdev; - ofnode node, mdios; const char *name; + ofnode node; int ret; - /* bind phy ports of mdios child node to mv88e6xxx_mdio device */ - mdios = dev_read_subnode(dev, "mdios"); - if (!ofnode_valid(mdios)) + /* bind phy ports of mdio child node to mv88e6xxx_mdio device */ + node = dev_read_subnode(dev, "mdio"); + if (!ofnode_valid(node)) return 0; - ofnode_for_each_subnode(node, mdios) { - name = ofnode_get_name(node); - ret = device_bind_driver_to_node(dev, - "mv88e6xxx_mdio", - name, node, NULL); - if (ret) { - dev_err(dev, "failed to bind %s: %d\n", name, ret); - continue; - } - + name = ofnode_get_name(node); + ret = device_bind_driver_to_node(dev, + "mv88e6xxx_mdio", + name, node, NULL); + if (ret) { + dev_err(dev, "failed to bind %s: %d\n", name, ret); + } else { /* need to probe it as there is no compatible to do so */ ret = uclass_get_device_by_ofnode(UCLASS_MDIO, node, &mdev); - if (ret) { + if (ret) dev_err(dev, "failed to probe %s: %d\n", name, ret); - continue; - } } return ret;
Add MV88E61XX DSA support: - update dt: U-Boot dsa driver requires different device-tree syntax than the linux driver in order to link the dsa ports to the mdio bus. - update defconfig - replace mv88e61xx_hw_reset weak override with board_phy_config support for mv88e61xx configuration that is outside the scope of the DSA driver Signed-off-by: Tim Harvey <tharvey@gateworks.com> --- v4: - use standard Linux internal MDIO dt structure - use PHY_FIXED_ID define v3: - move mdio's mdio@0 subnode v2: no changes --- arch/arm/dts/imx6qdl-gw5904.dtsi | 31 +++++++++++++++ board/gateworks/gw_ventana/gw_ventana.c | 50 +++++++++---------------- configs/gwventana_gw5904_defconfig | 8 ++-- drivers/net/mv88e6xxx.c | 29 ++++++-------- 4 files changed, 65 insertions(+), 53 deletions(-)