Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver

[Christian Marangi]

On Thu, Dec 05, 2024 at 08:50:37PM +0200, Vladimir Oltean wrote:
> On Thu, Dec 05, 2024 at 07:29:53PM +0100, Christian Marangi wrote:
> > Ohhhh ok, wasn't clear to me the MFD driver had to be placed in the mdio
> > node.
> > 
> > To make it clear this would be an implementation.
> > 
> > mdio_bus: mdio-bus {
> > 	#address-cells = <1>;
> > 	#size-cells = <0>;
> > 
> > 	...
> > 
> > 	mfd@1 {
> > 		compatible = "airoha,an8855-mfd";
> > 		reg = <1>;
> > 
> > 		nvmem_node {
> > 			...
> > 		};
> > 
> > 		switch_node {
> > 			...
> > 		};
> > 	};
> > };
> 
> I mean, I did mention Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml
> in my initial reply, which has an example with exactly this layout...
> 
> > The difficulties I found (and maybe is very easy to solve and I'm
> > missing something here) is that switch and internal PHY port have the
> > same address and conflicts.
> > 
> > Switch will be at address 1 (or 2 3 4 5... every port can access switch
> > register with page 0x4)
> > 
> > DSA port 0 will be at address 1, that is already occupied by the switch.
> > 
> > Defining the DSA port node on the host MDIO bus works correctly for
> > every port but for port 0 (the one at address 1), the kernel complains
> > and is not init. (as it does conflict with the switch that is at the
> > same address) (can't remember the exact warning)
> 
> Can any of these MDIO addresses (switch or ports) be changed through registers?

No, it can only be changed the BASE address that change the address of
each port.

port 0 is base address
port 1 is base address + 1
port 2 is base address + 2...

> 
> I guess the non-hack solution would be to permit MDIO buses to have
> #size-cells = 1, and MDIO devices to acquire a range of the address
> space, rather than just one address. Though take this with a grain of
> salt, I have a lot more to learn.

I remember this was an idea when PHY Package API were proposed and was
rejected as we wanted PHY to be single reg.

> 
> If neither of those are options, in principle the hack with just
> selecting, randomly, one of the N internal PHY addresses as the central
> MDIO address should work equally fine regardless of whether we are
> talking about the DSA switch's MDIO address here, or the MFD device's
> MDIO address.
> 
> With MFD you still have the option of creating a fake MDIO controller
> child device, which has mdio-parent-bus = <&host_bus>, and redirecting
> all user port phy-handles to children of this bus. Since all regmap I/O
> of this fake MDIO bus goes to the MFD driver, you can implement there
> your hacks with page switching etc etc, and it should be equally
> safe.

I wonder if a node like this would be more consistent and descriptive?

mdio_bus: mdio-bus {
    #address-cells = <1>;
    #size-cells = <0>;

    ...

    mfd@1 {
            compatible = "airoha,an8855-mfd";
            reg = <1>;

            nvmem_node {
                    ...
            };

            switch_node {
                ports {
                        port@0 {
                                phy-handle = <&phy>;
                        };

                        port@1 {
                                phy-handle = <&phy_2>;
                        }
                };
            };

            phy: phy_node {

            };
    };

    phy_2: phy@2 {
        reg = <2>;
    }

    phy@3 {
        reg = <3>;
    }

    ..
};

No idea how to register that single phy in mfd... I guess a fake mdio is
needed anyway... What do you think of this node example? Or not worth it
and better have the fake MDIO with all the switch PHY in it?

-- 
	Ansuel