| Message ID | 1439083745-14169-1-git-send-email-andrew@lunn.ch |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> >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 = <ðernet0>; 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>; }; }; }; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 = <ðernet0>; > 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>; > }; > }; > }; >
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)