Message ID | 20200217144414.409-4-olteanv@gmail.com |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
Series | DT bindings for Felix DSA switch on LS1028A | expand |
Hi Vladimir > + /* Internal port with DSA tagging */ > + mscc_felix_port4: port@4 { > + reg = <4>; > + phy-mode = "gmii"; Is it really using gmii? Often in SoC connections use something else, and phy-mode = "internal" is more appropriate. > + ethernet = <&enetc_port2>; > + > + fixed-link { > + speed = <2500>; > + full-duplex; > + }; gmii and 2500 also don't really go together. Andrew
Hi Andrew, On Mon, 17 Feb 2020 at 17:29, Andrew Lunn <andrew@lunn.ch> wrote: > > Hi Vladimir > > > + /* Internal port with DSA tagging */ > > + mscc_felix_port4: port@4 { > > + reg = <4>; > > + phy-mode = "gmii"; > > Is it really using gmii? Often in SoC connections use something else, > and phy-mode = "internal" is more appropriate. > What would be that "something else"? Given that the host port and the switch are completely different hardware IP blocks, I would assume that a parallel GMII is what's connecting them, no optimizations done. Certainly no serializer. But I don't know for sure. Does it matter, in the end? > > + ethernet = <&enetc_port2>; > > + > > + fixed-link { > > + speed = <2500>; > > + full-duplex; > > + }; > > gmii and 2500 also don't really go together. Not even if you raise the clock frequency? > > Andrew Thanks, -Vladimir
On Mon, 17 Feb 2020 at 17:33, Vladimir Oltean <olteanv@gmail.com> wrote: > > Hi Andrew, > > On Mon, 17 Feb 2020 at 17:29, Andrew Lunn <andrew@lunn.ch> wrote: > > > > Hi Vladimir > > > > > + /* Internal port with DSA tagging */ > > > + mscc_felix_port4: port@4 { > > > + reg = <4>; > > > + phy-mode = "gmii"; > > > > Is it really using gmii? Often in SoC connections use something else, > > and phy-mode = "internal" is more appropriate. > > > > What would be that "something else"? Given that the host port and the > switch are completely different hardware IP blocks, I would assume > that a parallel GMII is what's connecting them, no optimizations done. > Certainly no serializer. But I don't know for sure. > Does it matter, in the end? > To clarify, the reason I'm asking whether it matters is because I'd have to modify PHY_INTERFACE_MODE_GMII in drivers/net/dsa/ocelot/felix_vsc9959.c too, for the internal ports. Then I'm not sure anymore what tree this device tree patch should go in through. > > > + ethernet = <&enetc_port2>; > > > + > > > + fixed-link { > > > + speed = <2500>; > > > + full-duplex; > > > + }; > > > > gmii and 2500 also don't really go together. > > Not even if you raise the clock frequency? > > > > > Andrew > > Thanks, > -Vladimir
Hi Andrew, On Mon, 17 Feb 2020 at 19:24, Vladimir Oltean <olteanv@gmail.com> wrote: > > On Mon, 17 Feb 2020 at 17:33, Vladimir Oltean <olteanv@gmail.com> wrote: > > > > Hi Andrew, > > > > On Mon, 17 Feb 2020 at 17:29, Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > Hi Vladimir > > > > > > > + /* Internal port with DSA tagging */ > > > > + mscc_felix_port4: port@4 { > > > > + reg = <4>; > > > > + phy-mode = "gmii"; > > > > > > Is it really using gmii? Often in SoC connections use something else, > > > and phy-mode = "internal" is more appropriate. > > > > > > > What would be that "something else"? Given that the host port and the > > switch are completely different hardware IP blocks, I would assume > > that a parallel GMII is what's connecting them, no optimizations done. > > Certainly no serializer. But I don't know for sure. > > Does it matter, in the end? > > > > To clarify, the reason I'm asking whether it matters is because I'd > have to modify PHY_INTERFACE_MODE_GMII in > drivers/net/dsa/ocelot/felix_vsc9959.c too, for the internal ports. > Then I'm not sure anymore what tree this device tree patch should go > in through. > > > > > + ethernet = <&enetc_port2>; > > > > + > > > > + fixed-link { > > > > + speed = <2500>; > > > > + full-duplex; > > > > + }; > > > > > > gmii and 2500 also don't really go together. > > > > Not even if you raise the clock frequency? > > > > > > > > Andrew > > > > Thanks, > > -Vladimir Correct me if I'm wrong, but I think that PHY_INTERFACE_MODE_INTERNAL is added by Florian in 2017 as a generalization of the BCM7445 DSA switch bindings with internal PHY ports, and later became "popular" with other DSA drivers (ar9331, lantiq gswip). Of those, ar9331 is actually using phy-mode = "gmii" for the CPU port, and phy-mode = "internal" for the embedded copper PHYs. I hate to be making this sort of non-binary decision. Is it a GMII interface _or_ an internal interface? Prior to 2017, this would have probably been a non-question. The patch series which adds it does not clarify "you should use this mode in situation A, and this mode in situation B" either. Regards, -Vladimir
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi index dfead691e509..b35679dbcaa7 100644 --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi @@ -700,7 +700,9 @@ /* PF1: VF0-1 BAR0 - non-prefetchable memory */ 0x82000000 0x0 0x00000000 0x1 0xf8210000 0x0 0x020000 /* PF1: VF0-1 BAR2 - prefetchable memory */ - 0xc2000000 0x0 0x00000000 0x1 0xf8230000 0x0 0x020000>; + 0xc2000000 0x0 0x00000000 0x1 0xf8230000 0x0 0x020000 + /* BAR4 (PF5) - non-prefetchable memory */ + 0x82000000 0x0 0x00000000 0x1 0xfc000000 0x0 0x400000>; enetc_port0: ethernet@0,0 { compatible = "fsl,enetc"; @@ -710,6 +712,18 @@ compatible = "fsl,enetc"; reg = <0x000100 0 0 0 0>; }; + + enetc_port2: ethernet@0,2 { + compatible = "fsl,enetc"; + reg = <0x000200 0 0 0 0>; + phy-mode = "gmii"; + + fixed-link { + speed = <1000>; + full-duplex; + }; + }; + enetc_mdio_pf3: mdio@0,3 { compatible = "fsl,enetc-mdio"; reg = <0x000300 0 0 0 0>; @@ -722,6 +736,74 @@ clocks = <&clockgen 4 0>; little-endian; }; + + ethernet-switch@0,5 { + reg = <0x000500 0 0 0 0>; + /* IEP INT_B */ + interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + /* external ports */ + mscc_felix_port0: port@0 { + reg = <0>; + status = "disabled"; + }; + + mscc_felix_port1: port@1 { + reg = <1>; + status = "disabled"; + }; + + mscc_felix_port2: port@2 { + reg = <2>; + status = "disabled"; + }; + + mscc_felix_port3: port@3 { + reg = <3>; + status = "disabled"; + }; + + /* Internal port with DSA tagging */ + mscc_felix_port4: port@4 { + reg = <4>; + phy-mode = "gmii"; + ethernet = <&enetc_port2>; + + fixed-link { + speed = <2500>; + full-duplex; + }; + }; + + /* Internal port without DSA tagging */ + mscc_felix_port5: port@5 { + reg = <5>; + phy-mode = "gmii"; + status = "disabled"; + + fixed-link { + speed = <1000>; + full-duplex; + }; + }; + }; + }; + + enetc_port3: ethernet@0,6 { + compatible = "fsl,enetc"; + reg = <0x000600 0 0 0 0>; + status = "disabled"; + phy-mode = "gmii"; + + fixed-link { + speed = <1000>; + full-duplex; + }; + }; }; };