| Message ID | 20180323201117.8416-7-alexandre.belloni@bootlin.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | Microsemi Ocelot switch support | expand |
On 03/23/2018 01:11 PM, Alexandre Belloni wrote: > Ocelot has an integrated switch, add support for it. > > Cc: James Hogan <jhogan@kernel.org> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > --- > arch/mips/boot/dts/mscc/ocelot.dtsi | 84 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 84 insertions(+) > > diff --git a/arch/mips/boot/dts/mscc/ocelot.dtsi b/arch/mips/boot/dts/mscc/ocelot.dtsi > index dd239cab2f9d..22a86373b1c9 100644 > --- a/arch/mips/boot/dts/mscc/ocelot.dtsi > +++ b/arch/mips/boot/dts/mscc/ocelot.dtsi > @@ -91,6 +91,69 @@ > status = "disabled"; > }; > > + switch@1010000 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "mscc,ocelot-switch"; > + reg = <0x1010000 0x10000>, > + <0x1030000 0x10000>, > + <0x1080000 0x100>, > + <0x10d0000 0x10000>, > + <0x11e0000 0x100>, > + <0x11f0000 0x100>, > + <0x1200000 0x100>, > + <0x1210000 0x100>, > + <0x1220000 0x100>, > + <0x1230000 0x100>, > + <0x1240000 0x100>, > + <0x1250000 0x100>, > + <0x1260000 0x100>, > + <0x1270000 0x100>, > + <0x1280000 0x100>, > + <0x1800000 0x80000>, > + <0x1880000 0x10000>; > + reg-names = "sys", "rew", "qs", "hsio", "port0", > + "port1", "port2", "port3", "port4", "port5", > + "port6", "port7", "port8", "port9", "port10", > + "qsys", "ana"; > + interrupts = <21 22>; > + interrupt-names = "xtr", "inj"; See my comment about the binding patch, this should be moved to a ports subnode so it is conforming to the existing DSA binding and makes it a lot easier to have all ports disabled by default at the .dsti level by not defini > + > + port0: port@0 { > + reg = <0>; > + }; > + port1: port@1 { > + reg = <1>; > + }; > + port2: port@2 { > + reg = <2>; > + }; > + port3: port@3 { > + reg = <3>; > + }; > + port4: port@4 { > + reg = <4>; > + }; > + port5: port@5 { > + reg = <5>; > + }; > + port6: port@6 { > + reg = <6>; > + }; > + port7: port@7 { > + reg = <7>; > + }; > + port8: port@8 { > + reg = <8>; > + }; > + port9: port@9 { > + reg = <9>; > + }; > + port10: port@10 { > + reg = <10>; > + }; > + }; > + > reset@1070008 { > compatible = "mscc,ocelot-chip-reset"; > reg = <0x1070008 0x4>; > @@ -113,5 +176,26 @@ > function = "uart2"; > }; > }; > + > + mdio0: mdio@107009c { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "mscc,ocelot-miim"; > + reg = <0x107009c 0x36>, <0x10700f0 0x8>; > + interrupts = <14>; status = "disabled" by default? > + > + phy0: ethernet-phy@0 { > + reg = <0>; > + }; > + phy1: ethernet-phy@1 { > + reg = <1>; > + }; > + phy2: ethernet-phy@2 { > + reg = <2>; > + }; > + phy3: ethernet-phy@3 { > + reg = <3>; > + }; These PHYs should be defined at the board DTS level.
On 23/03/2018 at 14:17:48 -0700, Florian Fainelli wrote: > On 03/23/2018 01:11 PM, Alexandre Belloni wrote: > > + > > + phy0: ethernet-phy@0 { > > + reg = <0>; > > + }; > > + phy1: ethernet-phy@1 { > > + reg = <1>; > > + }; > > + phy2: ethernet-phy@2 { > > + reg = <2>; > > + }; > > + phy3: ethernet-phy@3 { > > + reg = <3>; > > + }; > > These PHYs should be defined at the board DTS level. Those are internal PHYs, present on the SoC, I doubt anyone will have anything different while using the same SoC.
On Fri, Mar 23, 2018 at 10:22:30PM +0100, Alexandre Belloni wrote: > On 23/03/2018 at 14:17:48 -0700, Florian Fainelli wrote: > > On 03/23/2018 01:11 PM, Alexandre Belloni wrote: > > > + > > > + phy0: ethernet-phy@0 { > > > + reg = <0>; > > > + }; > > > + phy1: ethernet-phy@1 { > > > + reg = <1>; > > > + }; > > > + phy2: ethernet-phy@2 { > > > + reg = <2>; > > > + }; > > > + phy3: ethernet-phy@3 { > > > + reg = <3>; > > > + }; > > > > These PHYs should be defined at the board DTS level. > > Those are internal PHYs, present on the SoC, I doubt anyone will have > anything different while using the same SoC. With DSA, there is no need to list internal PHYs. That is the trade off of having a standalone MDIO bus driver. Maybe add a phandle to the internal MDIO bus? The switch driver could then follow the phandle, and direct connect the internal PHYs? Andrew
On 03/23/2018 02:33 PM, Andrew Lunn wrote: > On Fri, Mar 23, 2018 at 10:22:30PM +0100, Alexandre Belloni wrote: >> On 23/03/2018 at 14:17:48 -0700, Florian Fainelli wrote: >>> On 03/23/2018 01:11 PM, Alexandre Belloni wrote: >>>> + >>>> + phy0: ethernet-phy@0 { >>>> + reg = <0>; >>>> + }; >>>> + phy1: ethernet-phy@1 { >>>> + reg = <1>; >>>> + }; >>>> + phy2: ethernet-phy@2 { >>>> + reg = <2>; >>>> + }; >>>> + phy3: ethernet-phy@3 { >>>> + reg = <3>; >>>> + }; >>> >>> These PHYs should be defined at the board DTS level. >> >> Those are internal PHYs, present on the SoC, I doubt anyone will have >> anything different while using the same SoC. > > With DSA, there is no need to list internal PHYs. > > That is the trade off of having a standalone MDIO bus driver. Maybe > add a phandle to the internal MDIO bus? The switch driver could then > follow the phandle, and direct connect the internal PHYs? This is more or less what patch 7 does, right?
> > That is the trade off of having a standalone MDIO bus driver. Maybe > > add a phandle to the internal MDIO bus? The switch driver could then > > follow the phandle, and direct connect the internal PHYs? > > This is more or less what patch 7 does, right? Patch 7 does it in DT. I'm suggesting it could be done in C. It is hard wired, so there is no need to describe it in DT. Use the phandle to get the mdio bus, mdiobus_get_phy(, port) to get the phydev and then use phy_connect(). Andrew
On 03/23/2018 03:06 PM, Andrew Lunn wrote: >>> That is the trade off of having a standalone MDIO bus driver. Maybe >>> add a phandle to the internal MDIO bus? The switch driver could then >>> follow the phandle, and direct connect the internal PHYs? >> >> This is more or less what patch 7 does, right? > > Patch 7 does it in DT. I'm suggesting it could be done in C. It is > hard wired, so there is no need to describe it in DT. Use the phandle > to get the mdio bus, mdiobus_get_phy(, port) to get the phydev and > then use phy_connect(). That does not sound like a great idea. And to go back to your example about DSA, it is partially true, you will see some switch bindings defining the internal PHYs (e.g: qca8k), and most not doing it (b53, mv88e6xxx, etc.). In either case, this resolves to the same thing though. Being able to parse a phy-handle property is a lot more flexible, and if it does matter that the PHY truly is internal, then the 'phy-mode' property can help reflect that.
On Fri, Mar 23, 2018 at 03:11:23PM -0700, Florian Fainelli wrote: > On 03/23/2018 03:06 PM, Andrew Lunn wrote: > >>> That is the trade off of having a standalone MDIO bus driver. Maybe > >>> add a phandle to the internal MDIO bus? The switch driver could then > >>> follow the phandle, and direct connect the internal PHYs? > >> > >> This is more or less what patch 7 does, right? > > > > Patch 7 does it in DT. I'm suggesting it could be done in C. It is > > hard wired, so there is no need to describe it in DT. Use the phandle > > to get the mdio bus, mdiobus_get_phy(, port) to get the phydev and > > then use phy_connect(). > > That does not sound like a great idea. And to go back to your example > about DSA, it is partially true, you will see some switch bindings > defining the internal PHYs (e.g: qca8k), and most not doing it (b53, > mv88e6xxx, etc.). In either case, this resolves to the same thing > though. Being able to parse a phy-handle property is a lot more > flexible, and if it does matter that the PHY truly is internal, then the > 'phy-mode' property can help reflect that. Hi Florian With DSA, you can always provide a phy-handle. It is only when there is nothing specified that the fallback case is used to map internal PHYs to ports. Putting internal PHYs in DT is fine, but it is a nice simplification if it is not needed. Andrew
diff --git a/arch/mips/boot/dts/mscc/ocelot.dtsi b/arch/mips/boot/dts/mscc/ocelot.dtsi index dd239cab2f9d..22a86373b1c9 100644 --- a/arch/mips/boot/dts/mscc/ocelot.dtsi +++ b/arch/mips/boot/dts/mscc/ocelot.dtsi @@ -91,6 +91,69 @@ status = "disabled"; }; + switch@1010000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "mscc,ocelot-switch"; + reg = <0x1010000 0x10000>, + <0x1030000 0x10000>, + <0x1080000 0x100>, + <0x10d0000 0x10000>, + <0x11e0000 0x100>, + <0x11f0000 0x100>, + <0x1200000 0x100>, + <0x1210000 0x100>, + <0x1220000 0x100>, + <0x1230000 0x100>, + <0x1240000 0x100>, + <0x1250000 0x100>, + <0x1260000 0x100>, + <0x1270000 0x100>, + <0x1280000 0x100>, + <0x1800000 0x80000>, + <0x1880000 0x10000>; + reg-names = "sys", "rew", "qs", "hsio", "port0", + "port1", "port2", "port3", "port4", "port5", + "port6", "port7", "port8", "port9", "port10", + "qsys", "ana"; + interrupts = <21 22>; + interrupt-names = "xtr", "inj"; + + port0: port@0 { + reg = <0>; + }; + port1: port@1 { + reg = <1>; + }; + port2: port@2 { + reg = <2>; + }; + port3: port@3 { + reg = <3>; + }; + port4: port@4 { + reg = <4>; + }; + port5: port@5 { + reg = <5>; + }; + port6: port@6 { + reg = <6>; + }; + port7: port@7 { + reg = <7>; + }; + port8: port@8 { + reg = <8>; + }; + port9: port@9 { + reg = <9>; + }; + port10: port@10 { + reg = <10>; + }; + }; + reset@1070008 { compatible = "mscc,ocelot-chip-reset"; reg = <0x1070008 0x4>; @@ -113,5 +176,26 @@ function = "uart2"; }; }; + + mdio0: mdio@107009c { + #address-cells = <1>; + #size-cells = <0>; + compatible = "mscc,ocelot-miim"; + reg = <0x107009c 0x36>, <0x10700f0 0x8>; + interrupts = <14>; + + phy0: ethernet-phy@0 { + reg = <0>; + }; + phy1: ethernet-phy@1 { + reg = <1>; + }; + phy2: ethernet-phy@2 { + reg = <2>; + }; + phy3: ethernet-phy@3 { + reg = <3>; + }; + }; }; };
Ocelot has an integrated switch, add support for it. Cc: James Hogan <jhogan@kernel.org> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> --- arch/mips/boot/dts/mscc/ocelot.dtsi | 84 +++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+)