linux-kernel - Re: [PATCH net-next v2 2/5] dt-bindings: net: dsa: ksz: add mdio-parent-bus property for internal MDIO

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: <2b03f429-9ae2-4c7a-9cec-0bc2f3c6e816@lunn.ch>
Date: Tue, 29 Oct 2024 21:35:48 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Oleksij Rempel <o.rempel@...gutronix.de>
Cc: Vladimir Oltean <olteanv@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Florian Fainelli <f.fainelli@...il.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Woojung Huh <woojung.huh@...rochip.com>,
	Arun Ramadoss <arun.ramadoss@...rochip.com>,
	Conor Dooley <conor+dt@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Rob Herring <robh+dt@...nel.org>, Rob Herring <robh@...nel.org>,
	kernel@...gutronix.de, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, UNGLinuxDriver@...rochip.com,
	"Russell King (Oracle)" <linux@...linux.org.uk>,
	devicetree@...r.kernel.org, Marek Vasut <marex@...x.de>
Subject: Re: [PATCH net-next v2 2/5] dt-bindings: net: dsa: ksz: add
 mdio-parent-bus property for internal MDIO

> > I'm not saying whether this is good or bad, I'm just worried about
> > mixing quantities having different measurement units into the same
> > address space.
> > 
> > Just like in the case of an mdio-mux, there is no address space isolation
> > between the parent bus and the child bus. AKA you can't have this,
> > because there would be clashes:
> > 
> > 	host_bus: mdio@...d {
> > 		ethernet-phy@2 {
> > 			reg = <2>;
> > 		};
> > 	};
> > 
> > 	child_bus: mdio@...h {
> > 		mdio-parent-bus = <&host_bus>;
> > 
> > 		ethernet-phy@2 {
> > 			reg = <2>;
> > 		};
> > 	};
> > 
> > But there is a big difference. With an mdio-mux, you could statically
> > detect address space clashes by inspecting the PHY addresses on the 2
> > buses. But with the lan937x child MDIO bus, in this design, you can't,
> > because the "reg" values don't represent MDIO addresses, but switch port
> > numbers (this is kind of important, but I don't see it mentioned in the
> > dt-binding).
> 
> In current state, the driver still require properly configured addresses
> in the devicetree. So, it will be visible in the DT.

This is not what i was expecting, especially from mv88e6xxx
perspective. The older generation of devices had the PHYs available on
the 'host bus', as well as the 'child bus', using a 1:1 address
mapping. You could in theory even skip the 'child bus' and list the
PHYs on the 'host bus' and phy-handle would make it work. However i
see from a later comment that does not work here, you need some
configuration done over SPI, which mv88e6xx does not need. 

> 
> > These are translated by lan937x_create_phy_addr_map() using
> > the CASCADE_ID/VPHY_ADD pin strapping information read over SPI.
> > I.e. with the same device tree, you may or may not have address space
> > clashes depending on pin strapping. No way to tell.
> 
> The PHY address to port mapping in the driver is needed to make the
> internal switch interrupt controller assign interrupts to proper PHYs.

You are talking about:

			ds->user_mii_bus->irq[phy] = irq;

in ksz_irq_phy_setup.

I naively expect 'phy' to be the 'reg' value in DT, and the 'dev'
value which passed to mdiobus_read_nested(bus, dev, reg) ?

	Andrew