Re: [PATCH RFC 00/28] DSA: Restructure probing

[Florian Fainelli <f.fainelli@gmail.com>]

Le 24/12/2015 04:58, Andrew Lunn a écrit :
>> I do not think you need the platform device because ultimately what the
>> DSA platform device does is bind some data to the master network device
>> the DSA switch tree is hanging off. Sure you need some piece of code
>> that is resident in kernel or module space to be able to parse and
>> allocate that data structure and bind multiple switch drivers together,
>> but that could be a consequence of probing switch driver using their bus
>> probe function.
>>
>> The way I would imagine this in a cluster configuration is that you
>> probe switches in the order in which they should appear in the final
>> switch tree (if this order cannot be guaranteed, then defer until it
>> is), and as you parse Device Tree for these switches you allocate their
>> resources and update the dsa_switch_tree structure "live".
> 
> How do you get this ordering? You cannot control the probe order in
> Linux.

Fair enough, then the ordering is not that important after all, the
cases could be something like these:

- all switch drivers built into the kernel, whichever gets probed first
creates the dsa_switch_tree and attaches its to the master network device
- subsequent switch drivers probed locate that master network device,
use the dsa_switch_tree and insert themselves at the correct index

If you need a working switch tree to boot your system from the network,
then all of this probing will eventually converge thanks to deferred
probing. If you build everything as a module, then the expectation as an
end-user could be that the switch tree becomes only fully functional
when *all* modules, and consequently switch devices are loaded/created.
Whether the switch tree is functional in between is something that
mostly depends on the default switch configuration left by
bootloader/reset defaults, so your mileage may vary.

> 
>> If we are using Device Tree this is relatively easy since we can lookup
>> the entire Device Tree to know the switch tree topology whenever we
>> probe a switch device driver. If we are using platform data, then, we
>> should have a way to associate a given MDIO bus address with
>> supplementary information, very much like what this patch does:
> 
> One of my aims is to abstract away the MDIO bus from DSA. How you talk
> to the switch is a switch device property. SF2 has a different way to
> talk to the switch, memory mapped IO, etc.

I appreciate you trying to see my point of view and mentioning the
driver that I care about, but I am not just after SF2 here, just any
kind of switch in general: PCIe, SPI, I2C, GPIO, MDIO, MMIO is precisely
what I would like to solve instead of narrow MDIO and distributed case
that DSA currently deals with, and MMIO with some help from the of_*
routines which do not need any kind of bus probing and struct device
creation (most often).

> 
> Anyway, who do you think the device tree binding will look? Maybe take
> the .dts file in patches 2 and 20 to build an example?

The binding would look pretty much the same as what the current DSA and
your proposed binding look like, except that instead of encoding the
MDIO address *and* the switch index in tree in the same "reg" properties
(which was wrong along, but I did not really knew it back in 2009 or
so), the two would be different properties.

When you parse the Device Tree you can determine what is the position of
the switch in the switch tree by looking at its ports sub-nodes and see
if they have a "link" property which links them to another device,
eventually, in case where there is not a one to one, but one to many
connection, an additional property can help you figure out how to
flatten things out.

So in practical terms, this could wind up looking like this:

mdio_bus@deadbeef {
	compatible = "acme,mdiobus";
	..
	switch0@0 {
		compatible = "marvell,mv88e6131";
		reg = <0>;
		dsa,addr = <1>;

		switch0port5: port@5 {
				reg = <5>;
				label = "dsa";
				link = <&switch1port9>;
			};
		};
	};
};

soc-bus@cafebabe {
	compatible = "simple-bus";

	switch1@cafedead {
		compatible = "marvell,armada-xp-switch";
		reg = <0xcafedead 0x1000>;
		dsa,addr = <0>;

		
		switch1port9: port9 {
			reg = <9>;
			label = "dsa";
			link = <&switch0port5>;
		};
	};
};

That way you could even intertwine foreign bus switches, and that would
not make a slight difference in how they are managed by DSA.

Your changes in patch 20 make your switches appear out of any busing
hierarchy whereas they are actually child devices of their respective
mdio-mux bus nodes and should appear as such imho.
-- 
Florian