| Message ID | 20171105231909.5599-4-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | RFC: Realtek 83xx SMI driver core | expand |
> I have the phy-handle in the ethernet controller. This RTL8366RB > thing is just one big PHY as far as I know. Hi Linus We don't model switches as PHYs. They are their own device type. And the internal or external PHYs are just normal PHYs in the linux model. Meaning their interrupt properties goes in the PHY node in device tree, as documented in the phy.txt binding documentation. Andrew
On Wed, Nov 29, 2017 at 4:56 PM, Andrew Lunn <andrew@lunn.ch> wrote: >> I have the phy-handle in the ethernet controller. This RTL8366RB >> thing is just one big PHY as far as I know. > > We don't model switches as PHYs. They are their own device type. And > the internal or external PHYs are just normal PHYs in the linux > model. Meaning their interrupt properties goes in the PHY node in > device tree, as documented in the phy.txt binding documentation. I do model the PHYs on the switch as PHYs. They are using the driver in drivers/phy/realtek.c. The interrupts are assigned to the PHYs not to the Switch. Just that the PHYs are on the MDIO bus inside the switch, of course. The switch however provides an irqchip to demux the interrupts. I think there is some misunderstanding in what I'm trying to do.. I have tried learning the DSA ideas by reading e.g. your paper: https://www.netdevconf.org/2.1/papers/distributed-switch-architecture.pdf So I try my best to conform with these ideas. I however have a hard time testing things since I don't really have a system to compare to. What would be useful is to know how commands like "ip" and "ifconfig" are used on a typical say home router. Yours, Linus Walleij
On Wed, Nov 29, 2017 at 10:56 PM, Andrew Lunn <andrew@lunn.ch> wrote: > I think the problem might be, you are using the DSA provided MDIO bus. > The Marvell switches has a similar setup in terms of interrupts. The > PHY interrupts appear within the switch. So i implemented an interrupt > controller, just the same as you. > > The problem is, the DSA provided MDIO bus is not linked to device > tree. So you cannot have phy nodes in device tree associated to it. > > What i did for the Marvell driver is that driver itself implements an > MDIO bus (two actually in some chips), and the internal or external > PHYs are placed on the switch drivers MDIO bus, rather than the DSA > MDIO bus. The switch driver MDIO bus links to an mdio node in device > tree. I can then have interrupt properties in the phys on this MDIO > bus in device tree. > > What actually might make sense, is to have the DSA MDIO bus look > inside the switches device tree node and see if there is an mdio > node. If so allow dsa_switch_setup() to use of_mdiobus_register() > instead of mdiobus_register(). Aha I think I see where my thinking went wrong. I have been assuming (thought it was intuitive...) that ports and PHYs are mapped 1:1. So I assumed the port with reg = <N> is also the PHY with reg = <N> So naturally I added the PHY interrupt to the port node. So you are saying that the PHY and the port are two very disparate things in DSA terminology? I guess all ports except the CPU port actually have a 1:1 mapped PHY though, am I right? Or are there in pracice things such that reg is different on the port and the PHY connected to it? Then it makes much sense to put an MDIO bus inside the switch DT node and populate the PHY interrupts from there as you say. I can take a stab at fixing that if that is what we want. Yours, Linus Walleij
On 11/29/2017 03:19 PM, Linus Walleij wrote: > On Wed, Nov 29, 2017 at 10:56 PM, Andrew Lunn <andrew@lunn.ch> wrote: > >> I think the problem might be, you are using the DSA provided MDIO bus. >> The Marvell switches has a similar setup in terms of interrupts. The >> PHY interrupts appear within the switch. So i implemented an interrupt >> controller, just the same as you. >> >> The problem is, the DSA provided MDIO bus is not linked to device >> tree. So you cannot have phy nodes in device tree associated to it. >> >> What i did for the Marvell driver is that driver itself implements an >> MDIO bus (two actually in some chips), and the internal or external >> PHYs are placed on the switch drivers MDIO bus, rather than the DSA >> MDIO bus. The switch driver MDIO bus links to an mdio node in device >> tree. I can then have interrupt properties in the phys on this MDIO >> bus in device tree. >> >> What actually might make sense, is to have the DSA MDIO bus look >> inside the switches device tree node and see if there is an mdio >> node. If so allow dsa_switch_setup() to use of_mdiobus_register() >> instead of mdiobus_register(). > > Aha I think I see where my thinking went wrong. > > I have been assuming (thought it was intuitive...) that ports and > PHYs are mapped 1:1. > > So I assumed the port with reg = <N> is also the PHY with > reg = <N> > > So naturally I added the PHY interrupt to the port node. > > So you are saying that the PHY and the port are two > very disparate things in DSA terminology? Yes, because the port is some sort of simplified Ethernet MAC, whereas the PHY is the PHY, and it usually exists in the same shape and size irrespective of whether it's integrated into a switch, being external, or being internal to a proper Ethernet NIC. > > I guess all ports except the CPU port actually have > a 1:1 mapped PHY though, am I right? This is the typical case, but is not universally true. > > Or are there in pracice things such that reg is different > on the port and the PHY connected to it? Then it makes > much sense to put an MDIO bus inside the switch DT > node and populate the PHY interrupts from there as you > say. Yes, I have such systems here, Port 0 has its PHY at MDIO address 5 for instance. > > I can take a stab at fixing that if that is what we want. While Andrew's suggestion to use of_mdiobus_register() even for the built-in DSA created slave_mii_bus makes sense, I would rather recommend you instantiate your own bus (ala mv88e6xxx), such that your DT will likely look like: switch@0 { compatible = "acme,switch"; #address-cells = <1>; #size-cells = <0>; ports { port@0 { reg = <0>; phy-handle = <&phy0>; }; port@1 { reg = <1>; phy-handle = <&phy1>; }; port@8 { reg = <8>; ethernet = = <ð0>; }; }; mdio { compatible = "acme,switch-mdio"; phy@0 { reg = <0>; }; phy@1 { reg = <1>; }; }; }; That way it's clear which port maps to which PHY, and that the MDIO controller is internal within the switch (and so are the PHYs). -- Florian
On Thu, Nov 30, 2017 at 12:26 AM, Florian Fainelli <f.fainelli@gmail.com> wrote: > On 11/29/2017 03:19 PM, Linus Walleij wrote: >> Or are there in pracice things such that reg is different >> on the port and the PHY connected to it? Then it makes >> much sense to put an MDIO bus inside the switch DT >> node and populate the PHY interrupts from there as you >> say. > > Yes, I have such systems here, Port 0 has its PHY at MDIO address 5 for > instance. That explains it. > switch@0 { > compatible = "acme,switch"; > #address-cells = <1>; > #size-cells = <0>; > > ports { > > port@0 { > reg = <0>; > phy-handle = <&phy0>; > }; > > port@1 { > reg = <1>; > phy-handle = <&phy1>; > }; > > port@8 { > reg = <8>; > ethernet = = <ð0>; > }; > }; > > mdio { > compatible = "acme,switch-mdio"; > > phy@0 { > reg = <0>; > }; > > phy@1 { > reg = <1>; > }; > }; > }; > > That way it's clear which port maps to which PHY, and that the MDIO > controller is internal within the switch (and so are the PHYs). So why not: switch@0 { compatible = "acme,switch"; #address-cells = <1>; #size-cells = <0>; ports { port@0 { reg = <0>; phy@0 { reg = <0>; }; }; port@1 { reg = <1>; phy@1 { reg = <1>; }; }; port@8 { reg = <8>; ethernet = = <ð0>; }; }; This avoids the cross-referencing of phandles. Yours, Linus Walleii
diff --git a/Documentation/devicetree/bindings/net/dsa/realtek-smi.txt b/Documentation/devicetree/bindings/net/dsa/realtek-smi.txt new file mode 100644 index 000000000000..95e96d49c0be --- /dev/null +++ b/Documentation/devicetree/bindings/net/dsa/realtek-smi.txt @@ -0,0 +1,104 @@ +Realtek SMI-based Switches +========================== + +The SMI "Simple Management Interface" is a two-wire protocol using +bit-banged GPIO that while it reuses the MDIO lines MCK and MDIO does +not use the MDIO protocol. This binding defines how to specify the +SMI-based Realtek devices. + +Required properties: + +- compatible: must be exactly one of: + "realtek,rtl8366" + "realtek,rtl8369" + "realtek,rtl8366rb" + "realtek,rtl8366s" + "realtek,rtl8367" + "realtek,rtl8367b" + +Required subnode: + +- interrupt-controller + + This defines an interrupt controller with an IRQ line (typically + a GPIO) that will demultiplex and handle the interrupt from the single + interrupt line coming out of one of the SMI-based chips. It most + importantly provides link up/down interrupts to the PHY blocks inside + the ASIC. + +Required properties of interrupt-controller: + +- interrupt: parent interrupt, see interrupt-controller/interrupts.txt +- interrupt-controller: see interrupt-controller/interrupts.txt +- #address-cells: should be <0> +- #interrupt-cells: should be <1> + +See net/dsa/dsa.txt for a list of additional required and optional properties +and subnodes. + + +Examples: + +switch { + compatible = "realtek,rtl8366rb"; + reg = <0>; + /* 22 = MDIO (has input reads), 21 = MDC (clock, output only) */ + mdc-gpios = <&gpio0 21 GPIO_ACTIVE_HIGH>; + mdio-gpios = <&gpio0 22 GPIO_ACTIVE_HIGH>; + reset-gpios = <&gpio0 14 GPIO_ACTIVE_LOW>; + + switch_intc: interrupt-controller { + /* GPIO 15 provides the interrupt */ + interrupt-parent = <&gpio0>; + interrupts = <15 IRQ_TYPE_LEVEL_LOW>; + interrupt-controller; + #address-cells = <0>; + #interrupt-cells = <1>; + }; + + ports { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + port@0 { + reg = <0>; + label = "lan0"; + interrupt-parent = <&switch_intc>; + interrupts = <0>; + }; + port@1 { + reg = <1>; + label = "lan1"; + interrupt-parent = <&switch_intc>; + interrupts = <1>; + }; + port@2 { + reg = <2>; + label = "lan2"; + interrupt-parent = <&switch_intc>; + interrupts = <2>; + }; + port@3 { + reg = <3>; + label = "lan3"; + interrupt-parent = <&switch_intc>; + interrupts = <3>; + }; + port@4 { + reg = <4>; + label = "wan"; + interrupt-parent = <&switch_intc>; + interrupts = <4>; + }; + phy0: port@5 { + reg = <5>; + label = "cpu"; + ethernet = <&gmac0>; + phy-mode = "rgmii"; + fixed-link { + speed = <1000>; + full-duplex; + }; + }; + }; +};
The Realtek SMI family is a set of DSA chips that provide switching in routers. This binding just follows the pattern set by other switches but with the introduction of an embedded irqchip to demux and handle the interrupts fired by the single line from the chip. This interrupt construction is similar to how we handle interrupt controllers inside PCI bridges etc. Cc: Antti Seppälä <a.seppala@gmail.com> Cc: Roman Yeryomin <roman@advem.lv> Cc: Colin Leitner <colin.leitner@googlemail.com> Cc: Gabor Juhos <juhosg@openwrt.org> Cc: Florian Fainelli <f.fainelli@gmail.com> Cc: devicetree@vger.kernel.org Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- .../devicetree/bindings/net/dsa/realtek-smi.txt | 104 +++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/dsa/realtek-smi.txt -- 2.13.6