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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <675200c3.7b0a0220.236ac3.9edf@mx.google.com>
Date: Thu, 5 Dec 2024 20:36:30 +0100
From: Christian Marangi <ansuelsmth@...il.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	upstream@...oha.com
Subject: Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port
 Gigabit DSA Switch driver

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