[PATCHv2 net-next] dsa: Support multiple MDIO busses

[PATCHv2 net-next] dsa: Support multiple MDIO busses

[Andrew Lunn]

When using a cluster of switches, some topologies will have an MDIO
bus per switch, not one for the whole cluster. Allow this to be
represented in the device tree, by adding an optional mii-bus property
at the switch level.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---

v2: Fix documentation, which placed the properties documentation in
    the wrong place.
---
 Documentation/devicetree/bindings/net/dsa/dsa.txt |  5 +++++
 net/dsa/dsa.c                                     | 12 +++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.txt b/Documentation/devicetree/bindings/net/dsa/dsa.txt
index f0b4cd72411d..fc06f4a7c788 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa.txt
+++ b/Documentation/devicetree/bindings/net/dsa/dsa.txt
@@ -32,6 +32,10 @@ A switch child node has the following optional property:
 			  the presence and/or size of a connected EEPROM,
 			  otherwise optional.
 
+- mii-bus		: Should be a phandle to a valid MDIO bus device node.
+			  This mii-bus will be used in preference to the
+			  global dsa,mii-bus defined above, for this switch.
+
 A switch may have multiple "port" children nodes
 
 Each port children node must have the following mandatory properties:
@@ -107,6 +111,7 @@ Example:
 			#address-cells = <1>;
 			#size-cells = <0>;
 			reg = <17 1>;	/* MDIO address 17, switch 1 in tree */
+			mii-bus = <&mii_bus1>;
 
 			switch1uplink: port@0 {
 				reg = <0>;
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index b445d492c115..78d4ac97aae3 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -574,7 +574,7 @@ static int dsa_of_probe(struct device *dev)
 {
 	struct device_node *np = dev->of_node;
 	struct device_node *child, *mdio, *ethernet, *port, *link;
-	struct mii_bus *mdio_bus;
+	struct mii_bus *mdio_bus, *mdio_bus_switch;
 	struct net_device *ethernet_dev;
 	struct dsa_platform_data *pd;
 	struct dsa_chip_data *cd;
@@ -636,6 +636,16 @@ static int dsa_of_probe(struct device *dev)
 		if (!of_property_read_u32(child, "eeprom-length", &eeprom_len))
 			cd->eeprom_len = eeprom_len;
 
+		mdio = of_parse_phandle(child, "mii-bus", 0);
+		if (mdio) {
+			mdio_bus_switch = of_mdio_find_bus(mdio);
+			if (!mdio_bus_switch) {
+				ret = -EPROBE_DEFER;
+				goto out_free_chip;
+			}
+			cd->host_dev = &mdio_bus_switch->dev;
+		}
+
 		for_each_available_child_of_node(child, port) {
 			port_reg = of_get_property(port, "reg", NULL);
 			if (!port_reg)
-- 
2.5.0

Re: [PATCHv2 net-next] dsa: Support multiple MDIO busses

[Sergei Shtylyov]

Hello.

On 8/9/2015 4:29 AM, Andrew Lunn wrote:

> When using a cluster of switches, some topologies will have an MDIO
> bus per switch, not one for the whole cluster. Allow this to be
> represented in the device tree, by adding an optional mii-bus property
> at the switch level.

> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>
> v2: Fix documentation, which placed the properties documentation in
>      the wrong place.
> ---
>   Documentation/devicetree/bindings/net/dsa/dsa.txt |  5 +++++
>   net/dsa/dsa.c                                     | 12 +++++++++++-
>   2 files changed, 16 insertions(+), 1 deletion(-)

> diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.txt b/Documentation/devicetree/bindings/net/dsa/dsa.txt
> index f0b4cd72411d..fc06f4a7c788 100644
> --- a/Documentation/devicetree/bindings/net/dsa/dsa.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/dsa.txt
> @@ -32,6 +32,10 @@ A switch child node has the following optional property:
>   			  the presence and/or size of a connected EEPROM,
>   			  otherwise optional.
>
> +- mii-bus		: Should be a phandle to a valid MDIO bus device node.

    Why not call it "mdio-bus"?

> +			  This mii-bus will be used in preference to the
> +			  global dsa,mii-bus defined above, for this switch.
> +
>   A switch may have multiple "port" children nodes
>
>   Each port children node must have the following mandatory properties:
[...]

MBR, Sergei

Re: [PATCHv2 net-next] dsa: Support multiple MDIO busses

[Andrew Lunn]

On Sun, Aug 09, 2015 at 03:39:25PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 8/9/2015 4:29 AM, Andrew Lunn wrote:
> 
> >When using a cluster of switches, some topologies will have an MDIO
> >bus per switch, not one for the whole cluster. Allow this to be
> >represented in the device tree, by adding an optional mii-bus property
> >at the switch level.
> 
> >Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> >Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> >---
> >
> >v2: Fix documentation, which placed the properties documentation in
> >     the wrong place.
> >---
> >  Documentation/devicetree/bindings/net/dsa/dsa.txt |  5 +++++
> >  net/dsa/dsa.c                                     | 12 +++++++++++-
> >  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> >diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.txt b/Documentation/devicetree/bindings/net/dsa/dsa.txt
> >index f0b4cd72411d..fc06f4a7c788 100644
> >--- a/Documentation/devicetree/bindings/net/dsa/dsa.txt
> >+++ b/Documentation/devicetree/bindings/net/dsa/dsa.txt
> >@@ -32,6 +32,10 @@ A switch child node has the following optional property:
> >  			  the presence and/or size of a connected EEPROM,
> >  			  otherwise optional.
> >
> >+- mii-bus		: Should be a phandle to a valid MDIO bus device node.
> 
>    Why not call it "mdio-bus"?

Hi Sergei

What you cannot see in this hunk, but can in the file is:

Marvell Distributed Switch Architecture Device Tree Bindings
------------------------------------------------------------

Required properties:
- compatible            : Should be "marvell,dsa"
- #address-cells        : Must be 2, first cell is the address on the MDIO bus
                          and second cell is the address in the switch tree.
                          Second cell is used only when cascading/chaining.
- #size-cells           : Must be 0
- dsa,ethernet          : Should be a phandle to a valid Ethernet device node
- dsa,mii-bus           : Should be a phandle to a valid MDIO bus device node

The optional mii-bus properties i'm adding should be used in
preference to this global one, for this switch. Keeping the names the
same makes sense. But i did drop the dsa prefix to indicate it is
local, not global scope, and all the other properties at switch level
do not use the dsa prefix.

    Andrew

Re: [PATCHv2 net-next] dsa: Support multiple MDIO busses

[Sergei Shtylyov]

On 8/9/2015 5:20 PM, Andrew Lunn wrote:

>>> When using a cluster of switches, some topologies will have an MDIO
>>> bus per switch, not one for the whole cluster. Allow this to be
>>> represented in the device tree, by adding an optional mii-bus property
>>> at the switch level.

>>> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
>>> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>>> ---

>>> v2: Fix documentation, which placed the properties documentation in
>>>      the wrong place.
>>> ---
>>>   Documentation/devicetree/bindings/net/dsa/dsa.txt |  5 +++++
>>>   net/dsa/dsa.c                                     | 12 +++++++++++-
>>>   2 files changed, 16 insertions(+), 1 deletion(-)

>>> diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.txt b/Documentation/devicetree/bindings/net/dsa/dsa.txt
>>> index f0b4cd72411d..fc06f4a7c788 100644
>>> --- a/Documentation/devicetree/bindings/net/dsa/dsa.txt
>>> +++ b/Documentation/devicetree/bindings/net/dsa/dsa.txt
>>> @@ -32,6 +32,10 @@ A switch child node has the following optional property:
>>>   			  the presence and/or size of a connected EEPROM,
>>>   			  otherwise optional.
>>>
>>> +- mii-bus		: Should be a phandle to a valid MDIO bus device node.

>>     Why not call it "mdio-bus"?

> Hi Sergei

> What you cannot see in this hunk, but can in the file is:

> Marvell Distributed Switch Architecture Device Tree Bindings
> ------------------------------------------------------------

> Required properties:
> - compatible            : Should be "marvell,dsa"
> - #address-cells        : Must be 2, first cell is the address on the MDIO bus
>                            and second cell is the address in the switch tree.
>                            Second cell is used only when cascading/chaining.
> - #size-cells           : Must be 0
> - dsa,ethernet          : Should be a phandle to a valid Ethernet device node
> - dsa,mii-bus           : Should be a phandle to a valid MDIO bus device node
>
> The optional mii-bus properties i'm adding should be used in
> preference to this global one, for this switch. Keeping the names the
> same makes sense. But i did drop the dsa prefix to indicate it is
> local, not global scope,

    Dropping a vendor prefix usually means that it's (standardized) common 
property name. I don't know where you took the "local" scope thing...

> and all the other properties at switch level
> do not use the dsa prefix.

>      Andrew

MBR, Sergei

Re: [PATCHv2 net-next] dsa: Support multiple MDIO busses

[Andrew Lunn]

> >The optional mii-bus properties i'm adding should be used in
> >preference to this global one, for this switch. Keeping the names the
> >same makes sense. But i did drop the dsa prefix to indicate it is
> >local, not global scope,
> 
>    Dropping a vendor prefix usually means that it's (standardized)
> common property name. I don't know where you took the "local" scope
> thing...

DSA has at least three levels of hierarchy. The following is the
example from the binding documentation. There is a 'global' scope
dsa,mii-bus = <&mii_bus0> in the top level, which is mandatory. This
patch allows optional 'local' scope mii bus to be specified at an
individual switch level. The 'local' value, if present, will override
the 'global' value.

Maybe more properties should use the dsa prefix? But dsa is not a
vendor prefix, it just refers to "Distributed Switch Architecture",
the scheme for accessing a number of Ethernet switches connected
together in a cluster. But as the binding has grown, this prefix has
not been used with new properties.

    Andrew

        dsa@0 {
                compatible = "marvell,dsa";
                #address-cells = <2>;
                #size-cells = <0>;

                interrupts = <10>;
                dsa,ethernet = <&ethernet0>;
                dsa,mii-bus = <&mii_bus0>;

                switch@0 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        reg = <16 0>;   /* MDIO address 16, switch 0 in tree */

                        port@0 {
                                reg = <0>;
                                label = "lan1";
                                phy-handle = <&phy0>;
                        };

                        port@1 {
                                reg = <1>;
                                label = "lan2";
                        };

                        port@5 {
                                reg = <5>;
                                label = "cpu";
                        };

                        switch0port6: port@6 {
                                reg = <6>;
                                label = "dsa";
                                link = <&switch1port0
                                        &switch2port0>;
                        };
                };

                switch@1 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        reg = <17 1>;   /* MDIO address 17, switch 1 in tree */
                        mii-bus = <&mii_bus1>;

                        switch1port0: port@0 {
                                reg = <0>;
                                label = "dsa";
                                link = <&switch0port6>;
                        };
                        switch1port1: port@1 {
                                reg = <1>;
                                label = "dsa";
                                link = <&switch2port1>;
                        };
                };

                switch@2 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        reg = <18 2>;   /* MDIO address 18, switch 2 in tree */
                        mii-bus = <&mii_bus1>;

                        switch2port0: port@0 {
                                reg = <0>;
                                label = "dsa";
                                link = <&switch1port1
                                        &switch0port6>;
                        };
                };
        };

Re: [PATCHv2 net-next] dsa: Support multiple MDIO busses

[Florian Fainelli]

Le 08/09/15 10:35, Andrew Lunn a écrit :
>>> The optional mii-bus properties i'm adding should be used in
>>> preference to this global one, for this switch. Keeping the names the
>>> same makes sense. But i did drop the dsa prefix to indicate it is
>>> local, not global scope,
>>
>>    Dropping a vendor prefix usually means that it's (standardized)
>> common property name. I don't know where you took the "local" scope
>> thing...
> 
> DSA has at least three levels of hierarchy. The following is the
> example from the binding documentation. There is a 'global' scope
> dsa,mii-bus = <&mii_bus0> in the top level, which is mandatory. This
> patch allows optional 'local' scope mii bus to be specified at an
> individual switch level. The 'local' value, if present, will override
> the 'global' value.
> 
> Maybe more properties should use the dsa prefix? But dsa is not a
> vendor prefix, it just refers to "Distributed Switch Architecture",
> the scheme for accessing a number of Ethernet switches connected
> together in a cluster. But as the binding has grown, this prefix has
> not been used with new properties.

Maybe it was not very smart to use the "dsa," prefixing initially, but I
agree with Andrew's naming of things here, we are already local to the
switch tree so dropping the prefix sounds reasonable.

> 
>     Andrew
> 
>         dsa@0 {
>                 compatible = "marvell,dsa";
>                 #address-cells = <2>;
>                 #size-cells = <0>;
> 
>                 interrupts = <10>;
>                 dsa,ethernet = <&ethernet0>;
>                 dsa,mii-bus = <&mii_bus0>;
> 
>                 switch@0 {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>                         reg = <16 0>;   /* MDIO address 16, switch 0 in tree */
> 
>                         port@0 {
>                                 reg = <0>;
>                                 label = "lan1";
>                                 phy-handle = <&phy0>;
>                         };
> 
>                         port@1 {
>                                 reg = <1>;
>                                 label = "lan2";
>                         };
> 
>                         port@5 {
>                                 reg = <5>;
>                                 label = "cpu";
>                         };
> 
>                         switch0port6: port@6 {
>                                 reg = <6>;
>                                 label = "dsa";
>                                 link = <&switch1port0
>                                         &switch2port0>;
>                         };
>                 };
> 
>                 switch@1 {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>                         reg = <17 1>;   /* MDIO address 17, switch 1 in tree */
>                         mii-bus = <&mii_bus1>;
> 
>                         switch1port0: port@0 {
>                                 reg = <0>;
>                                 label = "dsa";
>                                 link = <&switch0port6>;
>                         };
>                         switch1port1: port@1 {
>                                 reg = <1>;
>                                 label = "dsa";
>                                 link = <&switch2port1>;
>                         };
>                 };
> 
>                 switch@2 {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>                         reg = <18 2>;   /* MDIO address 18, switch 2 in tree */
>                         mii-bus = <&mii_bus1>;
> 
>                         switch2port0: port@0 {
>                                 reg = <0>;
>                                 label = "dsa";
>                                 link = <&switch1port1
>                                         &switch0port6>;
>                         };
>                 };
>         };
> 


-- 
Florian