On Thu, Mar 10, 2022 at 3:18 PM Vladimir Oltean olteanv@gmail.com wrote:
On Thu, Mar 10, 2022 at 02:35:21PM -0800, Tim Harvey wrote:
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?
You should definitely use a fixed-link and not a phy-handle to describe the MAC-to-MAC connection between the FEC and the switch's CPU port. If the MDIO bus udevice cannot get created/bound to a driver in lack of a compatible string, maybe you need to force a call to device_bind_driver_to_node() from the FEC driver for its "mdio" subnode?
Yes, the FEC MAC's internal MDIO lacks its own compatible string so I can in the fec driver bind the MDIO driver to the mdio node and probe it via uclass_get_device_by_ofnode(UCLASS_MDIO, node, &mdiodev).
Now the UCLASS_MDIO driver is probed and it works with boards that have MAC->PHY (non-fixed PHY's) but for a MAC<->MAC fixed-link phy_connect calls phy_connect_fixed with a null bus and does not scan the mdio bus and the dsa driver still does not probe. I'm still back to not understanding how to get the dsa switch driver to probe when it's a child of an mdio node.
Maybe the answer is similar to your above answer in that I need to make the fec driver continue to probe all children in the mdio bus node. I feel like this is quite a burden on the mac driver and that functionality should be handled in uclass-mdio somehow? I'm not sure if there is a push to convert MDIO drivers to UCLASS_MDIO. It seems like there is only a handful compared to all the drivers that call mdio_register so perhaps this issue just hasn't come up yet. Are we sure that in order to use UCLASS_DSA you must have a UCLASS_MDIO mdio bus?
Best Regards,
Tim
&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