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?