[PATCH 2/4 v1] net: dsa: Add bindings for Realtek SMI DSAs

[PATCH 2/4 v1] net: dsa: Add bindings for Realtek SMI DSAs

[Linus Walleij]

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>
---
ChangeLog RFCv2->v1:
- No changes, we agree on these bindings.
ChangeLog RFCv1->RFCv2:
- Switch to Andrew's suggestion to have a local MDIO bus
  definition inside of the DSA device node
- Add realtek,disabled-leds
- Correct WAN IRQ to 12 in the example
---
 .../bindings/net/dsa/realtek-smi.txt          | 153 ++++++++++++++++++
 1 file changed, 153 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/dsa/realtek-smi.txt

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..b6ae8541bd55
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dsa/realtek-smi.txt
@@ -0,0 +1,153 @@
+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,rtl8366rb" (4+1 ports)
+      "realtek,rtl8366s"  (4+1 ports)
+      "realtek,rtl8367"
+      "realtek,rtl8367b"
+      "realtek,rtl8368s"  (8 port)
+      "realtek,rtl8369"
+      "realtek,rtl8370"   (8 port)
+
+Required properties:
+- mdc-gpios: GPIO line for the MDC clock line.
+- mdio-gpios: GPIO line for the MDIO data line.
+- reset-gpios: GPIO line for the reset signal.
+
+Optional properties:
+- realtek,disable-leds: if the LED drivers are not used in the
+  hardware design this will disable them so they are not turned on
+  and wasting power.
+
+Required subnodes:
+
+- 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>
+
+- mdio
+
+  This defines the internal MDIO bus of the SMI device, mostly for the
+  purpose of being able to hook the interrupts to the right PHY and
+  the right PHY to the corresponding port.
+
+Required properties of mdio:
+
+- compatible: should be set to "realtek,smi-mdio" for all SMI devices
+
+See net/mdio.txt for additional MDIO bus properties.
+
+See net/dsa/dsa.txt for a list of additional required and optional properties
+and subnodes of DSA switches.
+
+Examples:
+
+switch {
+	compatible = "realtek,rtl8366rb";
+	/* 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";
+			phy-handle = <&phy0>;
+		};
+		port@1 {
+			reg = <1>;
+			label = "lan1";
+			phy-handle = <&phy1>;
+		};
+		port@2 {
+			reg = <2>;
+			label = "lan2";
+			phy-handle = <&phy2>;
+		};
+		port@3 {
+			reg = <3>;
+			label = "lan3";
+			phy-handle = <&phy3>;
+		};
+		port@4 {
+			reg = <4>;
+			label = "wan";
+			phy-handle = <&phy4>;
+		};
+		port@5 {
+			reg = <5>;
+			label = "cpu";
+			ethernet = <&gmac0>;
+			phy-mode = "rgmii";
+			fixed-link {
+				speed = <1000>;
+				full-duplex;
+			};
+		};
+	};
+
+	mdio {
+		compatible = "realtek,smi-mdio", "dsa-mdio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		phy0: phy@0 {
+			reg = <0>;
+			interrupt-parent = <&switch_intc>;
+			interrupts = <0>;
+		};
+		phy1: phy@1 {
+			reg = <1>;
+			interrupt-parent = <&switch_intc>;
+			interrupts = <1>;
+		};
+		phy2: phy@2 {
+			reg = <2>;
+			interrupt-parent = <&switch_intc>;
+			interrupts = <2>;
+		};
+		phy3: phy@3 {
+			reg = <3>;
+			interrupt-parent = <&switch_intc>;
+			interrupts = <3>;
+		};
+		phy4: phy@4 {
+			reg = <4>;
+			interrupt-parent = <&switch_intc>;
+			interrupts = <12>;
+		};
+	};
+};
-- 
2.17.1

Re: [PATCH 2/4 v1] net: dsa: Add bindings for Realtek SMI DSAs

[Rob Herring]

On Sat, Jul 14, 2018 at 11:45:54AM +0200, Linus Walleij wrote:
> 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>
> ---
> ChangeLog RFCv2->v1:
> - No changes, we agree on these bindings.
> ChangeLog RFCv1->RFCv2:
> - Switch to Andrew's suggestion to have a local MDIO bus
>   definition inside of the DSA device node
> - Add realtek,disabled-leds
> - Correct WAN IRQ to 12 in the example
> ---
>  .../bindings/net/dsa/realtek-smi.txt          | 153 ++++++++++++++++++
>  1 file changed, 153 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/dsa/realtek-smi.txt
> 
> 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..b6ae8541bd55
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/realtek-smi.txt
> @@ -0,0 +1,153 @@
> +Realtek SMI-based Switches
> +==========================
> +
> +The SMI "Simple Management Interface" is a two-wire protocol using

At least for some other Realtek chips, the documentation I find says the 
S stands for Serial. And Wikipedia says SMI is the same thing as MDIO.

Just want to make sure we don't define GPIOs directly when there should 
be a layer of abstraction like mdio-gpio.

> +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,rtl8366rb" (4+1 ports)
> +      "realtek,rtl8366s"  (4+1 ports)
> +      "realtek,rtl8367"
> +      "realtek,rtl8367b"
> +      "realtek,rtl8368s"  (8 port)
> +      "realtek,rtl8369"
> +      "realtek,rtl8370"   (8 port)
> +
> +Required properties:
> +- mdc-gpios: GPIO line for the MDC clock line.
> +- mdio-gpios: GPIO line for the MDIO data line.
> +- reset-gpios: GPIO line for the reset signal.
> +
> +Optional properties:
> +- realtek,disable-leds: if the LED drivers are not used in the
> +  hardware design this will disable them so they are not turned on
> +  and wasting power.
> +
> +Required subnodes:
> +
> +- 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>
> +
> +- mdio
> +
> +  This defines the internal MDIO bus of the SMI device, mostly for the
> +  purpose of being able to hook the interrupts to the right PHY and
> +  the right PHY to the corresponding port.
> +
> +Required properties of mdio:
> +
> +- compatible: should be set to "realtek,smi-mdio" for all SMI devices
> +
> +See net/mdio.txt for additional MDIO bus properties.
> +
> +See net/dsa/dsa.txt for a list of additional required and optional properties
> +and subnodes of DSA switches.
> +
> +Examples:
> +
> +switch {
> +	compatible = "realtek,rtl8366rb";
> +	/* 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";
> +			phy-handle = <&phy0>;
> +		};
> +		port@1 {
> +			reg = <1>;
> +			label = "lan1";
> +			phy-handle = <&phy1>;
> +		};
> +		port@2 {
> +			reg = <2>;
> +			label = "lan2";
> +			phy-handle = <&phy2>;
> +		};
> +		port@3 {
> +			reg = <3>;
> +			label = "lan3";
> +			phy-handle = <&phy3>;
> +		};
> +		port@4 {
> +			reg = <4>;
> +			label = "wan";
> +			phy-handle = <&phy4>;
> +		};
> +		port@5 {
> +			reg = <5>;
> +			label = "cpu";
> +			ethernet = <&gmac0>;
> +			phy-mode = "rgmii";
> +			fixed-link {
> +				speed = <1000>;
> +				full-duplex;
> +			};
> +		};
> +	};
> +
> +	mdio {
> +		compatible = "realtek,smi-mdio", "dsa-mdio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		phy0: phy@0 {
> +			reg = <0>;
> +			interrupt-parent = <&switch_intc>;
> +			interrupts = <0>;
> +		};
> +		phy1: phy@1 {
> +			reg = <1>;
> +			interrupt-parent = <&switch_intc>;
> +			interrupts = <1>;
> +		};
> +		phy2: phy@2 {
> +			reg = <2>;
> +			interrupt-parent = <&switch_intc>;
> +			interrupts = <2>;
> +		};
> +		phy3: phy@3 {
> +			reg = <3>;
> +			interrupt-parent = <&switch_intc>;
> +			interrupts = <3>;
> +		};
> +		phy4: phy@4 {
> +			reg = <4>;
> +			interrupt-parent = <&switch_intc>;
> +			interrupts = <12>;
> +		};
> +	};
> +};
> -- 
> 2.17.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4 v1] net: dsa: Add bindings for Realtek SMI DSAs

[Andrew Lunn]

> > +Realtek SMI-based Switches
> > +==========================
> > +
> > +The SMI "Simple Management Interface" is a two-wire protocol using
> 
> At least for some other Realtek chips, the documentation I find says the 
> S stands for Serial. And Wikipedia says SMI is the same thing as MDIO.
> 
> Just want to make sure we don't define GPIOs directly when there should 
> be a layer of abstraction like mdio-gpio.

Hi Rob

There was a bit of discussion about this with the RFC patches. The bit
stream protocol used here is not that used by SMI in MDIO. It is
something proprietary to Realtek. I don't expect we will be re-using
the code with other vendors. The only real reason to abstract this out
is if there happens to be other Realtek switches which uses the same
SMI bitstream, but have completely different registers, meaning a
separate DSA driver would be needed. The SMI code is well structured,
so it should not be too hard to turn it into a library which drivers
can share.

So i personally don't see a problem with this.

   Andrew

Re: [PATCH 2/4 v1] net: dsa: Add bindings for Realtek SMI DSAs

[Linus Walleij]

On Mon, Jul 16, 2018 at 10:45 PM Rob Herring <robh@kernel.org> wrote:
> On Sat, Jul 14, 2018 at 11:45:54AM +0200, Linus Walleij wrote:

> > +The SMI "Simple Management Interface" is a two-wire protocol using
>
> At least for some other Realtek chips, the documentation I find says the
> S stands for Serial. And Wikipedia says SMI is the same thing as MDIO.

The only datasheet we have mentions both:
http://realtek.info/pdf/rtl8366_8369_datasheet_1-1.pdf
calling it serial management interface when talking to an EEPROM
calling it systems management interface when talking to a
PHY.

(BTW that datsheet has no relation to this driver, that is for
ASICs 8366 and 8369 which of course have nothing to do with
RTL8366RB "revision B" that we are running here, which adds
even more to the confusion.)

Sadly it would not surprise me if Realtek doesn't even know
which one it is themselves, given the state of the code and
documentation that came out of the company.

I just have to pick something. I can rename it the
"S Management Interface" if you prefer, OpenWRT called it
Systems Management Interface.

> Just want to make sure we don't define GPIOs directly when there should
> be a layer of abstraction like mdio-gpio.

OK let's look at it we read a single register with each protocol:

1. MDIO: drivers/net/phy/mdio-bitbang.c
  send_bit is setting data and cycling clock 1-0 (falling edge)
  begins transactions by sending 32 1:s (well 33 for safe measure)
  then sends a start bit 01
  then sends the opcode 10 (read)
  then sends the PHY address (5 bits)
  then sends the register address (5 bits)
  turn around (switch MDIO to input)
  read 16 bit register value
  read stop bit

2. SMI:
   sends the sequence "101" to start transactions. (No initial ones)
   writes a whole byte which is the "command" read is 0xa9
   on RTL8366RB and apparently something else on other chips
   writes low byte of 16bit address
   writes high byte of 16bit address
   turn around (switch MDIO to input)
   reads the low byte of the 16bit data
   reads the high byte of the 16bit data
   sends the stop sequence 01
   then an extra clock pulse

As you can see those are pretty different. PHY always being
addressed in MDIO (the switch chips has PHYs but this
protocol is not defined exclusively for that) and both phy and
reg being 5 bits addressing at most 10 address bits is the most
obvious deviation then 8 bits for command instead of 2 etc.

It's just very different.

Yours,
Linus Walleij

Re: [PATCH 2/4 v1] net: dsa: Add bindings for Realtek SMI DSAs

[Rob Herring]

On Tue, Jul 17, 2018 at 08:55:51AM +0200, Linus Walleij wrote:
> On Mon, Jul 16, 2018 at 10:45 PM Rob Herring <robh@kernel.org> wrote:
> > On Sat, Jul 14, 2018 at 11:45:54AM +0200, Linus Walleij wrote:
> 
> > > +The SMI "Simple Management Interface" is a two-wire protocol using
> >
> > At least for some other Realtek chips, the documentation I find says the
> > S stands for Serial. And Wikipedia says SMI is the same thing as MDIO.
> 
> The only datasheet we have mentions both:
> http://realtek.info/pdf/rtl8366_8369_datasheet_1-1.pdf
> calling it serial management interface when talking to an EEPROM
> calling it systems management interface when talking to a
> PHY.
> 
> (BTW that datsheet has no relation to this driver, that is for
> ASICs 8366 and 8369 which of course have nothing to do with
> RTL8366RB "revision B" that we are running here, which adds
> even more to the confusion.)
> 
> Sadly it would not surprise me if Realtek doesn't even know
> which one it is themselves, given the state of the code and
> documentation that came out of the company.
> 
> I just have to pick something. I can rename it the
> "S Management Interface" if you prefer, OpenWRT called it
> Systems Management Interface.
> 
> > Just want to make sure we don't define GPIOs directly when there should
> > be a layer of abstraction like mdio-gpio.
> 
> OK let's look at it we read a single register with each protocol:
> 
> 1. MDIO: drivers/net/phy/mdio-bitbang.c
>   send_bit is setting data and cycling clock 1-0 (falling edge)
>   begins transactions by sending 32 1:s (well 33 for safe measure)
>   then sends a start bit 01
>   then sends the opcode 10 (read)
>   then sends the PHY address (5 bits)
>   then sends the register address (5 bits)
>   turn around (switch MDIO to input)
>   read 16 bit register value
>   read stop bit
> 
> 2. SMI:
>    sends the sequence "101" to start transactions. (No initial ones)
>    writes a whole byte which is the "command" read is 0xa9
>    on RTL8366RB and apparently something else on other chips
>    writes low byte of 16bit address
>    writes high byte of 16bit address
>    turn around (switch MDIO to input)
>    reads the low byte of the 16bit data
>    reads the high byte of the 16bit data
>    sends the stop sequence 01
>    then an extra clock pulse
> 
> As you can see those are pretty different. PHY always being
> addressed in MDIO (the switch chips has PHYs but this
> protocol is not defined exclusively for that) and both phy and
> reg being 5 bits addressing at most 10 address bits is the most
> obvious deviation then 8 bits for command instead of 2 etc.
> 
> It's just very different.

Okay. Just wanted to make sure.

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/4 v1] net: dsa: Add bindings for Realtek SMI DSAs

[Linus Walleij]

On Fri, Jul 20, 2018 at 6:17 PM Rob Herring <robh@kernel.org> wrote:
> On Tue, Jul 17, 2018 at 08:55:51AM +0200, Linus Walleij wrote:

> > It's just very different.
>
> Okay. Just wanted to make sure.
>
> Reviewed-by: Rob Herring <robh@kernel.org>

Thanks Rob! And thanks for asking the critical questions, more
often than not I am wrong (like that TPO screen with "custom"
wire protocol that turned out to be an SPI dialect), so it's good
to take this time to go back and check things over.

Yours,
Linus Walleij