Message ID | 20161221102047.28036-1-uwe@kleine-koenig.org |
---|---|
State | New |
Headers | show |
On Wed, Dec 21, 2016 at 11:20:47AM +0100, Uwe Kleine-König wrote: > The Turris Omnia features a Marvell MV88E7176 ethernet switch. Add it to > the dts. > > Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> > --- > Hello, > > when the MAC is operated in rgmii mode and the switch in rgmii-id > communication between them works fine. The other way round it doesn't work. > The fixed-link nodes in the cpu port description is necessary to let the > mv88e6xxx driver use the phy-mode description. Is this a bug? It is somewhat deliberate. Normally, there is no phy at all for a cpu port or a dsa port. If there is no phy, it makes no sense to set the phy-mode. With fixed-link, there is a phy connected to the MAC, in terms of how the network stack sees the devices. It is a somewhat special phy, in fact its bandwidth is fixed, but it does support phy-handle, and a few other phy properties and callbacks. > Regarding the binding, I think the label in the port nodes is strange. > > label = "lan2" > > for an external port is fine. But > > label = "cpu" > > for a port facing a CPU MAC looks wrong, still more as the Turris Omnia has > two ports connected to the SoC and so there are two ports with the same > label. This is somewhat historical. When DSA was designed, only one CPU port was expected. There are in fact quite a few devices with two CPU ports. I have some old proof of concept patches to support this, with some basic load balancing. In the last few weeks, John Crispin has been working on them, and has duel CPU ports working for the qca8k. So i expect early next year we can make the Marvell chips support dual CPU ports as well. > I would suggest to use a separate property for that, e.g. > > cpu-port; > > and no label. I would suggest keeping this idea for DSA v3, otherwise we end up with messy code for little gain. Andrew
> /* Switch MV88E7176 at address 0x10 */ > + switch@10 { > + compatible = "marvell,mv88e6085"; > + #address-cells = <1>; > + #size-cells = <0>; > + dsa,member = <0 0>; > + > + reg = <0x10>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + ports@0 { > + reg = <0>; > + label = "lan0"; > + phy-handle = <&lan0phy>; snip > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + > + lan0phy: ethernet-phy@0 { > + compatible = "ethernet-phy-ieee802.3-c22"; > + reg = <0>; > + }; You probably don't need the phy-handle's and the whole mdio node. So long as there is a simple mapping, port 0 uses the phy at address 0, it should just work. You only need the mdio node when you want to support PHY interrupts, or there is an odd mapping between port and PHY, like the new switch chip added recently. Andrew
Hello Andrew, On Wed, Dec 21, 2016 at 04:18:14PM +0100, Andrew Lunn wrote: > On Wed, Dec 21, 2016 at 11:20:47AM +0100, Uwe Kleine-König wrote: > > when the MAC is operated in rgmii mode and the switch in rgmii-id > > communication between them works fine. The other way round it doesn't work. > > The fixed-link nodes in the cpu port description is necessary to let the > > mv88e6xxx driver use the phy-mode description. Is this a bug? > > It is somewhat deliberate. Normally, there is no phy at all for a cpu > port or a dsa port. If there is no phy, it makes no sense to set the > phy-mode. With fixed-link, there is a phy connected to the MAC, in > terms of how the network stack sees the devices. It is a somewhat > special phy, in fact its bandwidth is fixed, but it does support > phy-handle, and a few other phy properties and callbacks. Here the switch is the phy, and in this case it is necessary to correctly configure it's physical properties. fixed-link is conceptual wrong here because (IIUC) this is for MACs only. So (maybe also for DSA v3 or a general phy cleanup) a standard binding to specify this which doesn't formalise the switch (or general: phy) as MAC. Best regards Uwe
diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts index ab49acb2d452..6722d3243f9b 100644 --- a/arch/arm/boot/dts/armada-385-turris-omnia.dts +++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts @@ -122,7 +122,7 @@ pinctrl-names = "default"; pinctrl-0 = <&ge0_rgmii_pins>; status = "okay"; - phy-mode = "rgmii-id"; + phy-mode = "rgmii"; fixed-link { speed = <1000>; @@ -135,7 +135,7 @@ pinctrl-names = "default"; pinctrl-0 = <&ge1_rgmii_pins>; status = "okay"; - phy-mode = "rgmii-id"; + phy-mode = "rgmii"; fixed-link { speed = <1000>; @@ -274,6 +274,103 @@ }; /* Switch MV88E7176 at address 0x10 */ + switch@10 { + compatible = "marvell,mv88e6085"; + #address-cells = <1>; + #size-cells = <0>; + dsa,member = <0 0>; + + reg = <0x10>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + ports@0 { + reg = <0>; + label = "lan0"; + phy-handle = <&lan0phy>; + }; + + ports@1 { + reg = <1>; + label = "lan1"; + phy-handle = <&lan1phy>; + }; + + ports@2 { + reg = <2>; + label = "lan2"; + phy-handle = <&lan2phy>; + }; + + ports@3 { + reg = <3>; + label = "lan3"; + phy-handle = <&lan3phy>; + }; + + ports@4 { + reg = <4>; + label = "lan4"; + phy-handle = <&lan4phy>; + }; + + ports@5 { + reg = <5>; + label = "cpu"; + ethernet = <ð1>; + phy-mode = "rgmii-id"; + + fixed-link { + speed = <1000>; + full-duplex; + }; + }; + + ports@6 { + reg = <6>; + label = "cpu"; + ethernet = <ð0>; + phy-mode = "rgmii-id"; + + fixed-link { + speed = <1000>; + full-duplex; + }; + }; + }; + + mdio { + #address-cells = <1>; + #size-cells = <0>; + + lan0phy: ethernet-phy@0 { + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <0>; + }; + + lan1phy: ethernet-phy@1 { + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <1>; + }; + + lan2phy: ethernet-phy@2 { + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <2>; + }; + + lan3phy: ethernet-phy@3 { + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <3>; + }; + + lan4phy: ethernet-phy@4 { + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <4>; + }; + }; + }; }; &pinctrl {
The Turris Omnia features a Marvell MV88E7176 ethernet switch. Add it to the dts. Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> --- Hello, when the MAC is operated in rgmii mode and the switch in rgmii-id communication between them works fine. The other way round it doesn't work. The fixed-link nodes in the cpu port description is necessary to let the mv88e6xxx driver use the phy-mode description. Is this a bug? Regarding the binding, I think the label in the port nodes is strange. label = "lan2" for an external port is fine. But label = "cpu" for a port facing a CPU MAC looks wrong, still more as the Turris Omnia has two ports connected to the SoC and so there are two ports with the same label. I would suggest to use a separate property for that, e.g. cpu-port; and no label. Best regards Uwe arch/arm/boot/dts/armada-385-turris-omnia.dts | 101 +++++++++++++++++++++++++- 1 file changed, 99 insertions(+), 2 deletions(-)