dsa driver for mv88e61xx

[Vladimir Oltean]

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 at 0 {
>                         compatible = "marvell,mv88e6085";
>                         reg = <0>;
> 
>                         ports {
>                                 #address-cells = <1>;
>                                 #size-cells = <0>;
> 
>                                 port at 0 {
>                                         reg = <0>;
>                                         label = "lan4";
>                                 };
> 
>                                 port at 1 {
>                                         reg = <1>;
>                                         label = "lan3";
>                                 };
> 
>                                 port at 2 {
>                                         reg = <2>;
>                                         label = "lan2";
>                                 };
> 
>                                 port at 3 {
>                                         reg = <3>;
>                                         label = "lan1";
>                                 };
> 
>                                 port at 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?