Re: [PATCH RFT] of: property: fw_devlink: Add support for the "phy-handle" binding

[Andrew Lunn]

> Let me see if I can hack something up on this board (which has a decent
> dependency tree for testing this stuff) to use the generic phy driver
> instead of the marvell one that it needs and see how that goes. It won't
> *actually* work from a phy perspective, but it will at least test out
> the driver core bits here I think.
> 
> > 
> > But like you said, it's been a while and fw_devlink has improved since
> > then (I think). So please go ahead and give this a shot. If you can
> > help fix any issues this highlights, I'd really appreciate it and I'd
> > be happy to guide you through what I think needs to happen. But I
> > don't think I have the time to fix it myself.
> 
> Sure, I tend to agree. Let me check the generic phy driver path for any
> issues and if that test seems to go okay I too am of the opinion that
> without any solid reasoning against this we enable it and battle through
> (revert and fix after the fact if necessary) any newly identified issues
> that prevent phy-handle and fw_devlink have with each other.

Here is one for you to look at:

https://elixir.bootlin.com/linux/v6.11.1/source/drivers/net/ethernet/freescale/fec_main.c#L2481

I don't know if there is a real issue here, but if the order of the
probe gets swapped, i think the usage of mii_cnt will break.

I don't remember what broke last time, and i'm currently too lazy to
go look. But maybe take a look at devices like this:

arch/arm/boot/dts/nxp/vf/vf610-zii-dev-rev-b.dts

       mdio-mux {
                compatible = "mdio-mux-gpio";
                pinctrl-0 = <&pinctrl_mdio_mux>;
                pinctrl-names = "default";
                gpios = <&gpio0 8  GPIO_ACTIVE_HIGH
                         &gpio0 9  GPIO_ACTIVE_HIGH
                         &gpio0 24 GPIO_ACTIVE_HIGH
                         &gpio0 25 GPIO_ACTIVE_HIGH>;
                mdio-parent-bus = <&mdio1>;
                #address-cells = <1>;
                #size-cells = <0>;

                mdio_mux_1: mdio@1 {
                        reg = <1>;
                        #address-cells = <1>;
                        #size-cells = <0>;

                        switch0: switch@0 {
                                compatible = "marvell,mv88e6085";
                                pinctrl-0 = <&pinctrl_gpio_switch0>;
                                pinctrl-names = "default";
                                reg = <0>;
                                dsa,member = <0 0>;
                                interrupt-parent = <&gpio0>;
                                interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
                                interrupt-controller;
                                #interrupt-cells = <2>;
                                eeprom-length = <512>;

                                ports {
                                        #address-cells = <1>;
                                        #size-cells = <0>;

                                        port@0 {
                                                reg = <0>;
                                                label = "lan0";
                                                phy-handle = <&switch0phy0>;
                                        };

                                        port@1 {
                                                reg = <1>;
                                                label = "lan1";
                                                phy-handle = <&switch0phy1>;
                                        };

                                        port@2 {
                                                reg = <2>;
                                                label = "lan2";
                                                phy-handle = <&switch0phy2>;
                                        };

                                        switch0port5: port@5 {
                                                reg = <5>;
                                                label = "dsa";
                                                phy-mode = "rgmii-txid";
                                                link = <&switch1port6
                                                        &switch2port9>;
                                                fixed-link {
                                                        speed = <1000>;
                                                        full-duplex;
                                                };
                                        };

                                        port@6 {
                                                reg = <6>;
                                                phy-mode = "rmii";
                                                ethernet = <&fec1>;

                                                fixed-link {
                                                        speed = <100>;
                                                        full-duplex;
                                                };
                                        };
                                };
                                mdio {
                                        #address-cells = <1>;
                                        #size-cells = <0>;
                                        switch0phy0: switch0phy0@0 {
                                                reg = <0>;
                                                interrupt-parent = <&switch0>;
                                                interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
                                        };
                                        switch0phy1: switch1phy0@1 {
                                                reg = <1>;
                                                interrupt-parent = <&switch0>;
                                                interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
                                        };
                                        switch0phy2: switch1phy0@2 {
                                                reg = <2>;
                                                interrupt-parent = <&switch0>;
                                                interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
                                        };
                                };
                        };
                };


This would be an example of circular dependencies, with the interrupt
properties closing the loop.

switch -> mdio -> phy
 ^                |
 |----------------+

	Andrew