On Thu, Mar 10, 2022 at 8:50 AM Vladimir Oltean olteanv@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.... [3] https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/mv88e6xxx/chi...
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