diff mbox series

dt-bindings: net: dsa: realtek-smi: convert to YAML schema

Message ID 20211228072645.32341-1-luizluca@gmail.com
State New
Headers show
Series dt-bindings: net: dsa: realtek-smi: convert to YAML schema | expand

Commit Message

Luiz Angelo Daros de Luca Dec. 28, 2021, 7:26 a.m. UTC
Schema changes:

- "interrupt-controller" was not added as a required property. It might
  still work polling the ports when missing
- "interrupt" property was mentioned but never used. According to its
  description, it was assumed it was really "interrupt-parent"

Examples changes:

- renamed "switch_intc" to make it unique between examples
- removed "dsa-mdio" from mdio compatible property
- renamed phy@0 to ethernet-phy@0 (not tested with real HW)
  phy@ requires #phy-cells

Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
 .../bindings/net/dsa/realtek-smi.txt          | 240 --------------
 .../bindings/net/dsa/realtek-smi.yaml         | 310 ++++++++++++++++++
 2 files changed, 310 insertions(+), 240 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/net/dsa/realtek-smi.txt
 create mode 100644 Documentation/devicetree/bindings/net/dsa/realtek-smi.yaml

Comments

Luiz Angelo Daros de Luca Jan. 4, 2022, 11:44 p.m. UTC | #1
Thanks Linus!

> > +    description: |
> > +      realtek,rtl8365mb: 4+1 ports
> > +      realtek,rtl8366:
> > +      realtek,rtl8366rb:

There is some confusion with the n+m port description. Some 4+1 means
4 lan + 1 wan while in other cases it means 4 user + 1 ext port, even
in Realtek documentation. The last digit in realtek product numbers is
the port number (0 means 10) and it is the sum of user ports and
external ports. From what I investigated, the last digit numbers
normally mean:

3: 2 user + 1 ext port
4: 2 user + 2 ext port
5: 4 user + 1 ext port
6: 5 user + 1 ext port
7: 5 user + 2 ext port
0: 8 user + 2 ext port.

The description in YAML was from the TXT version but it is a good time
to improve it.

BTW, I couldn't find a datasheet for rtl8366rb. The commit message
says it is from a DIR-685 but wikidevi days that device has a
RTL8366SR, which is described as "SINGLE-CHIP 5+1-PORT 10/100/1000
MBPS SWITCH CONTROLLER WITH DUAL MAC INTERFACES".

Do you have any suggestions?

Regards,

Luiz
Rob Herring Jan. 10, 2022, 6:09 p.m. UTC | #2
On Tue, Jan 04, 2022 at 08:44:37PM -0300, Luiz Angelo Daros de Luca wrote:
> Thanks Linus!
> 
> > > +    description: |
> > > +      realtek,rtl8365mb: 4+1 ports
> > > +      realtek,rtl8366:
> > > +      realtek,rtl8366rb:
> 

Why have you removed Linus' comment?

> There is some confusion with the n+m port description. Some 4+1 means
> 4 lan + 1 wan while in other cases it means 4 user + 1 ext port, even
> in Realtek documentation. The last digit in realtek product numbers is
> the port number (0 means 10) and it is the sum of user ports and
> external ports. From what I investigated, the last digit numbers
> normally mean:
> 
> 3: 2 user + 1 ext port
> 4: 2 user + 2 ext port
> 5: 4 user + 1 ext port
> 6: 5 user + 1 ext port
> 7: 5 user + 2 ext port
> 0: 8 user + 2 ext port.
> 
> The description in YAML was from the TXT version but it is a good time
> to improve it.
> 
> BTW, I couldn't find a datasheet for rtl8366rb. The commit message
> says it is from a DIR-685 but wikidevi days that device has a
> RTL8366SR, which is described as "SINGLE-CHIP 5+1-PORT 10/100/1000
> MBPS SWITCH CONTROLLER WITH DUAL MAC INTERFACES".
> 
> Do you have any suggestions?

I think Linus just meant to add spaces around the '+'.

Rob
Arınç ÜNAL Jan. 29, 2022, 7:35 p.m. UTC | #3
On 29/01/2022 19:02, Luiz Angelo Daros de Luca wrote:
> Thanks Rob, now that the code side is merged, I'm back to docs.
> 
> 
>>> +      interrupt-controller:
>>> +        description: see interrupt-controller/interrupts.txt
>>
>> Don't need generic descriptions. Just 'true' here is fine.
> 
> Do you really mean quoted true, like in "description: 'true' "?
> Without quotes it will fail
>>
>>> +
>>> +      interrupts:
>>> +        description: TODO
>>
>> You have to define how many interrupts and what they are.
> 
> I didn't write the interruption code and Linus and Alvin might help here.
> 
> The switch has a single interrupt pin that signals an interruption happened.
> The code reads a register to multiplex to these interruptions:
> 
> INT_TYPE_LINK_STATUS = 0,
> INT_TYPE_METER_EXCEED,
> INT_TYPE_LEARN_LIMIT,
> INT_TYPE_LINK_SPEED,
> INT_TYPE_CONGEST,
> INT_TYPE_GREEN_FEATURE,
> INT_TYPE_LOOP_DETECT,
> INT_TYPE_8051,
> INT_TYPE_CABLE_DIAG,
> INT_TYPE_ACL,
> INT_TYPE_RESERVED, /* Unused */
> INT_TYPE_SLIENT,
> 
> And most of them, but not all, multiplex again to each port.
> 
> However, the linux driver today does not care about any of these
> interruptions but INT_TYPE_LINK_STATUS. So it simply multiplex only
> this the interruption to each port, in a n-cell map (n being number of
> ports).
> I don't know what to describe here as device-tree should be something
> independent of a particular OS or driver.
> 
> Anyway, I doubt someone might want to plug one of these interruptions
> outside the switch driver. Could it be simple as this:
> 
>        interrupts:
>         minItems: 3
>         maxItems: 10
>         description:
>           interrupt mapping one per switch port
> 
> Once realtek-smi.yaml settles, I'll also send the realtek-mdio.yaml.

Why not turn realtek-smi.yaml into realtek.yaml which would also contain 
information for the mdio interface? The things different with using MDIO 
are that we don't use the [mdc,mdio,reset]-gpios properties and don't 
handle the PHYs to the DSA ports. Couldn't you present these differences 
on a single YAML file?

Arınç
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/dsa/realtek-smi.txt b/Documentation/devicetree/bindings/net/dsa/realtek-smi.txt
deleted file mode 100644
index 7959ec237983..000000000000
--- a/Documentation/devicetree/bindings/net/dsa/realtek-smi.txt
+++ /dev/null
@@ -1,240 +0,0 @@ 
-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,rtl8365mb" (4+1 ports)
-      "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:
-
-An example for the RTL8366RB:
-
-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>;
-		};
-	};
-};
-
-An example for the RTL8365MB-VC:
-
-switch {
-	compatible = "realtek,rtl8365mb";
-	mdc-gpios = <&gpio1 16 GPIO_ACTIVE_HIGH>;
-	mdio-gpios = <&gpio1 17 GPIO_ACTIVE_HIGH>;
-	reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
-
-	switch_intc: interrupt-controller {
-		interrupt-parent = <&gpio5>;
-		interrupts = <1 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 = "swp0";
-			phy-handle = <&ethphy0>;
-		};
-		port@1 {
-			reg = <1>;
-			label = "swp1";
-			phy-handle = <&ethphy1>;
-		};
-		port@2 {
-			reg = <2>;
-			label = "swp2";
-			phy-handle = <&ethphy2>;
-		};
-		port@3 {
-			reg = <3>;
-			label = "swp3";
-			phy-handle = <&ethphy3>;
-		};
-		port@6 {
-			reg = <6>;
-			label = "cpu";
-			ethernet = <&fec1>;
-			phy-mode = "rgmii";
-			tx-internal-delay-ps = <2000>;
-			rx-internal-delay-ps = <2000>;
-
-			fixed-link {
-				speed = <1000>;
-				full-duplex;
-				pause;
-			};
-		};
-	};
-
-	mdio {
-		compatible = "realtek,smi-mdio";
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		ethphy0: phy@0 {
-			reg = <0>;
-			interrupt-parent = <&switch_intc>;
-			interrupts = <0>;
-		};
-		ethphy1: phy@1 {
-			reg = <1>;
-			interrupt-parent = <&switch_intc>;
-			interrupts = <1>;
-		};
-		ethphy2: phy@2 {
-			reg = <2>;
-			interrupt-parent = <&switch_intc>;
-			interrupts = <2>;
-		};
-		ethphy3: phy@3 {
-			reg = <3>;
-			interrupt-parent = <&switch_intc>;
-			interrupts = <3>;
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/net/dsa/realtek-smi.yaml b/Documentation/devicetree/bindings/net/dsa/realtek-smi.yaml
new file mode 100644
index 000000000000..c4cd0038f092
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dsa/realtek-smi.yaml
@@ -0,0 +1,310 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/dsa/realtek-smi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek SMI-based Switches
+
+allOf:
+  - $ref: dsa.yaml#
+
+maintainers:
+  - Linus Walleij <linus.walleij@linaro.org>
+
+description:
+  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. The realtek-smi driver is a platform driver
+  and it must be inserted inside a platform node.
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - realtek,rtl8365mb
+          - realtek,rtl8366
+          - realtek,rtl8366rb
+          - realtek,rtl8366s
+          - realtek,rtl8367
+          - realtek,rtl8367b
+          - realtek,rtl8368s
+          - realtek,rtl8369
+          - realtek,rtl8370
+    description: |
+      realtek,rtl8365mb: 4+1 ports
+      realtek,rtl8366:
+      realtek,rtl8366rb:
+      realtek,rtl8366s: 4+1 ports
+      realtek,rtl8367:
+      realtek,rtl8367b:
+      realtek,rtl8368s: 8 ports
+      realtek,rtl8369:
+      realtek,rtl8370:  8+2 ports
+  reg:
+    maxItems: 1
+
+  mdc-gpios:
+    description: GPIO line for the MDC clock line.
+    maxItems: 1
+
+  mdio-gpios:
+    description: GPIO line for the MDIO data line.
+    maxItems: 1
+
+  reset-gpios:
+    description: GPIO to be used to reset the whole device
+    maxItems: 1
+
+  realtek,disable-leds:
+    type: boolean
+    description: |
+      if the LED drivers are not used in the
+      hardware design this will disable them so they are not turned on
+      and wasting power.
+
+  interrupt-controller:
+    type: object
+    description: |
+      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.
+    
+    properties:
+
+      interrupt-controller:
+        description: see interrupt-controller/interrupts.txt
+
+      interrupts:
+        description: TODO
+
+      '#address-cells':
+        const: 0
+
+      '#interrupt-cells':
+        const: 1
+
+    required:
+      - interrupt-parent
+      - interrupt-controller
+      - '#address-cells'
+      - '#interrupt-cells'
+
+  mdio:
+    type: object
+    description:
+      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.
+
+    properties:
+      compatible:
+        const: "realtek,smi-mdio"
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      "^(ethernet-)?phy@[0-4]$":
+        type: object
+
+        allOf:
+          - $ref: "http://devicetree.org/schemas/net/mdio.yaml#"
+
+        properties:
+          reg:
+            maxItems: 1
+
+        required:
+          - reg
+
+required:
+  - compatible
+  - mdc-gpios
+  - mdio-gpios
+  - reset-gpios
+
+additionalProperties: true
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    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_intc1: 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>;
+                    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";
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    phy0: ethernet-phy@0 {
+                            reg = <0>;
+                            interrupt-parent = <&switch_intc1>;
+                            interrupts = <0>;
+                    };
+                    phy1: ethernet-phy@1 {
+                            reg = <1>;
+                            interrupt-parent = <&switch_intc1>;
+                            interrupts = <1>;
+                    };
+                    phy2: ethernet-phy@2 {
+                            reg = <2>;
+                            interrupt-parent = <&switch_intc1>;
+                            interrupts = <2>;
+                    };
+                    phy3: ethernet-phy@3 {
+                            reg = <3>;
+                            interrupt-parent = <&switch_intc1>;
+                            interrupts = <3>;
+                    };
+                    phy4: ethernet-phy@4 {
+                            reg = <4>;
+                            interrupt-parent = <&switch_intc1>;
+                            interrupts = <12>;
+                    };
+            };
+    };
+
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    switch {
+            compatible = "realtek,rtl8365mb";
+            mdc-gpios = <&gpio1 16 GPIO_ACTIVE_HIGH>;
+            mdio-gpios = <&gpio1 17 GPIO_ACTIVE_HIGH>;
+            reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
+
+            switch_intc2: interrupt-controller {
+                    interrupt-parent = <&gpio5>;
+                    interrupts = <1 IRQ_TYPE_LEVEL_LOW>;
+                    interrupt-controller;
+                    #address-cells = <0>;
+                    #interrupt-cells = <1>;
+            };
+
+            ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+                    port@0 {
+                            reg = <0>;
+                            label = "swp0";
+                            phy-handle = <&ethphy0>;
+                    };
+                    port@1 {
+                            reg = <1>;
+                            label = "swp1";
+                            phy-handle = <&ethphy1>;
+                    };
+                    port@2 {
+                            reg = <2>;
+                            label = "swp2";
+                            phy-handle = <&ethphy2>;
+                    };
+                    port@3 {
+                            reg = <3>;
+                            label = "swp3";
+                            phy-handle = <&ethphy3>;
+                    };
+                    port@6 {
+                            reg = <6>;
+                            label = "cpu";
+                            ethernet = <&fec1>;
+                            phy-mode = "rgmii";
+                            tx-internal-delay-ps = <2000>;
+                            rx-internal-delay-ps = <2000>;
+
+                            fixed-link {
+                                    speed = <1000>;
+                                    full-duplex;
+                                    pause;
+                            };
+                    };
+            };
+
+            mdio {
+                    compatible = "realtek,smi-mdio";
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    ethphy0: ethernet-phy@0 {
+                            reg = <0>;
+                            interrupt-parent = <&switch_intc2>;
+                            interrupts = <0>;
+                    };
+                    ethphy1: ethernet-phy@1 {
+                            reg = <1>;
+                            interrupt-parent = <&switch_intc2>;
+                            interrupts = <1>;
+                    };
+                    ethphy2: ethernet-phy@2 {
+                            reg = <2>;
+                            interrupt-parent = <&switch_intc2>;
+                            interrupts = <2>;
+                    };
+                    ethphy3: ethernet-phy@3 {
+                            reg = <3>;
+                            interrupt-parent = <&switch_intc2>;
+                            interrupts = <3>;
+                    };
+            };
+    };