On Thu, Mar 10, 2022 at 8:50 AM Vladimir Oltean <olte...@gmail.com> wrote: > > Hello Tim, > > On Thu, Mar 10, 2022 at 08:16:13AM -0800, Tim Harvey wrote: > > Greetings, > > > > I wanted to take a stab at adding dsa support for the mv88e61xx which > > currently has a driver in drivers/net/phy [1]. The board I have > > available to me is the gw5904 which has a mv88e6085 with the upstream > > port connected to the IMX6 FEC MAC over RGMII. This currently works > > with the existing mv88e61xx driver with the following defined in > > gwventana_gw5904_defconfig: > > > > CONFIG_MV88E61XX_SWITCH=y > > CONFIG_MV88E61XX_CPU_PORT=5 > > CONFIG_MV88E61XX_PHY_PORTS=0xf > > CONFIG_MV88E61XX_FIXED_PORTS=0x0 > > > > The device-tree is arch/arm/dts/imx6qdl-gw5904.dtsi [2] which I > > believe is proper and works in Linux with the Linux driver in > > drivers/net/dsa/mv88e6xxx [3]. > > > > &fec { > > pinctrl-names = "default"; > > pinctrl-0 = <&pinctrl_enet>; > > phy-mode = "rgmii-id"; > > phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>; > > phy-reset-duration = <10>; > > phy-reset-post-delay = <100>; > > status = "okay"; > > > > fixed-link { > > speed = <1000>; > > full-duplex; > > }; > > > > mdio { > > #address-cells = <1>; > > #size-cells = <0>; > > > > switch@0 { > > compatible = "marvell,mv88e6085"; > > reg = <0>; > > > > ports { > > #address-cells = <1>; > > #size-cells = <0>; > > > > port@0 { > > reg = <0>; > > label = "lan4"; > > }; > > > > port@1 { > > reg = <1>; > > label = "lan3"; > > }; > > > > port@2 { > > reg = <2>; > > label = "lan2"; > > }; > > > > port@3 { > > reg = <3>; > > label = "lan1"; > > }; > > > > port@5 { > > reg = <5>; > > label = "cpu"; > > ethernet = <&fec>; > > }; > > }; > > }; > > }; > > }; > > > > My motivation for doing this is to be able to drop > > gwventana_gw5904_defconfig as it is the same defconfig as > > gwventana_emmc_defconfig with the switch added and with get_phy_id > > overridden by the current mv88e61xx driver that config won't work with > > boards that lack the switch. > > > > My first approach was to just put a #if !defined(CONFIG_DM_DSA) around > > mv88e61xx get_phy_id and add a skeleton driver with an of_match of > > compatible = "marvell,mv88e6085" but the driver does not probe with > > the above dt fragment. > > > > Any ideas why the driver won't probe and advise on how I should > > proceed with this? I'm not clear yet if I can just modify the existing > > driver or if I should create a new one. > > > > Best Regards, > > > > Tim > > [1] > > https://elixir.bootlin.com/u-boot/latest/source/drivers/net/phy/mv88e61xx.c > > [2] > > https://elixir.bootlin.com/u-boot/latest/source/arch/arm/dts/imx6qdl-gw5904.dtsi > > [3] > > https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/mv88e6xxx/chip.c > > I spent about 3 minutes looking at mv88e61xx.c, so take what I'm about > to say with a grain of salt. > > The biggest issue with reusing that code seems to be that it uses struct > phy_device throughout. A DM_DSA driver would need to work around a > struct udevice. I'd probably create a new driver, make it easy for > others to use, then delete old driver. I see there are 5 occurrences of > CONFIG_MV88E61XX_PHY_PORTS in defconfigs, all added within the past 4 > years. One is yours. Could have been a lot worse. > > As to why your driver is not probing. I think the fec_mxc parent MDIO > bus must be a udevice as well, registered using DM_MDIO?
Vladimir, Ok, I see that even though fec_mxc is DM_ETH it doesn't use DM_MDIO. I suppose my ksz9477 dsa driver probed because that switch was hanging off I2C and DM_I2C was in use as opposed to this time the switch is hanging off of MDIO. With changes to fec_mxc to use DM_MDIO/UCLASS_MDIO for the its mdio bus, I see my UCLASS_MDIO bus getting probed and registering the bus but the call to dm_eth_phy_connect(fec) fails. For the situation I have where the fec_mxc provides the MDIO bus connected to the switch as well as the MAC that's connected via rgmii to the upstream port I believe I need a 'phy-handle' in my fec node vs a 'fixed-link' subnode otherwise dm_eth_connect_phy_handle() won't probe my mdio device. But then I find that dm_eth_phy_connect(fec) calls dm_eth_connect_phy_handle() which probes my UCLASS_MDIO but the call to dm_mdio_phy_connect() fails and my UCLASS_DSA device never gets probed. So I guess I'm confused if I should be using fixed-link or not and if so, how does the UCLASS_MDIO device get probed? &fec { pinctrl-names = "default" #if 1 // required for dm_eth_connect_phy_handle to probe the UCLASS_MDIO device phy-handle = "switch"; #endif phy-mode = "rgmii-id"; status = "okay"; #if 0 // if present phy_connect is called immediate in dm_eth_connect_phy_handle and the UCLASS_MDIO device is not probed fixed-link { speed = <1000>; full-duplex; }; #endif mdio { #address-cells = <1>; #size-cells = <0>; switch: switch@0 { compatible = "marvell,mv88e6085"; reg = <0>; ports { #address-cells = <1>; #size-cells = <0>; port@0 { reg = <0>; label = "lan4"; phy-mode = "internal"; }; ... port@5 { reg = <5>; label = "cpu"; ethernet = <&fec>; phy-mode = "rgmii-id"; fixed-link { speed = <1000>; full-duplex; }; }; }; }; }; }; Best regards, Tim