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

[Oleksij Rempel]

On Tue, Oct 29, 2024 at 09:35:48PM +0100, Andrew Lunn wrote:
> > > 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@abcd {
> > > 		ethernet-phy@2 {
> > > 			reg = <2>;
> > > 		};
> > > 	};
> > > 
> > > 	child_bus: mdio@efgh {
> > > 		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) ?

Yes, this is correct. This can be implemented purely by parsing the
devicetree. Based on previous experience, I expected you to suggest me
to implement the validation so i jumped directly to a part of this step.

Should I implement it based on the devicetree information and validate
based on HW strapping?

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |