diff mbox series

[net-next,v2,1/5] dt-bindings: net: Convert Altera TSE bindings to yaml

Message ID 20220830095549.120625-2-maxime.chevallier@bootlin.com
State New
Headers show
Series net: altera: tse: phylink conversion | expand

Commit Message

Maxime Chevallier Aug. 30, 2022, 9:55 a.m. UTC
This converts the bindings for the Altera Triple-Speed Ethernet to yaml.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V1->V2:
 - Removed unnedded maxItems
 - Added missing minItems
 - Fixed typos in some properties names
 - Fixed the mdio subnode definition

 .../devicetree/bindings/net/altera_tse.txt    | 113 -------------
 .../devicetree/bindings/net/altr,tse.yaml     | 156 ++++++++++++++++++
 2 files changed, 156 insertions(+), 113 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/net/altera_tse.txt
 create mode 100644 Documentation/devicetree/bindings/net/altr,tse.yaml

Comments

Maxime Chevallier Aug. 30, 2022, 7:16 p.m. UTC | #1
On Tue, 30 Aug 2022 20:13:56 +0300
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 30/08/2022 12:55, Maxime Chevallier wrote:
> > This converts the bindings for the Altera Triple-Speed Ethernet to
> > yaml.  
> 
> Do not use "This commit/patch".
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

ack

> > 
> > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>  
> 
> Rebase your changes on some decent kernel and use
> get_maintainers.pl...

I'm rebased against net-next, so I don't understand how I'm supposed to
do for this series, should I sent binding patches separately and based
on another tree ?

I'll cc you next time, sorry about that.

> > ---
> > V1->V2:
> >  - Removed unnedded maxItems
> >  - Added missing minItems
> >  - Fixed typos in some properties names
> >  - Fixed the mdio subnode definition
> > 
> >  .../devicetree/bindings/net/altera_tse.txt    | 113 -------------
> >  .../devicetree/bindings/net/altr,tse.yaml     | 156
> > ++++++++++++++++++ 2 files changed, 156 insertions(+), 113
> > deletions(-) delete mode 100644
> > Documentation/devicetree/bindings/net/altera_tse.txt create mode
> > 100644 Documentation/devicetree/bindings/net/altr,tse.yaml 
> 
> (...)
> 
> > diff --git a/Documentation/devicetree/bindings/net/altr,tse.yaml
> > b/Documentation/devicetree/bindings/net/altr,tse.yaml new file mode
> > 100644 index 000000000000..1676e13b8c64
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/altr,tse.yaml
> > @@ -0,0 +1,156 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/altr,tse.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Altera Triple Speed Ethernet MAC driver (TSE)
> > +
> > +maintainers:
> > +  - Maxime Chevallier <maxime.chevallier@bootlin.com>
> > +
> > +allOf:  
> 
> Put allOf below "required".

Ack

> > +  - $ref: "ethernet-controller.yaml#"
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - altr,tse-1.0
> > +              - ALTR,tse-1.0
> > +    then:
> > +      properties:
> > +        reg:
> > +          minItems: 4
> > +        reg-names:
> > +          items:
> > +            - const: control_port
> > +            - const: rx_csr
> > +            - const: tx_csr
> > +            - const: s1
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - altr,tse-msgdma-1.0
> > +    then:
> > +      properties:
> > +        reg:
> > +          minItems: 6
> > +        reg-names:
> > +          minItems: 6  
> 
> No need for minItems.

Ok I'll remove it

> > +          items:
> > +            - const: control_port
> > +            - const: rx_csr
> > +            - const: rx_desc
> > +            - const: rx_resp
> > +            - const: tx_csr
> > +            - const: tx_desc
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - altr,tse-1.0
> > +      - ALTR,tse-1.0  
> 
> This is deprecated compatible. You need oneOf and then deprecated:
> true.

Ok thanks for the tip

> > +      - altr,tse-msgdma-1.0
> > +
> > +  reg:
> > +    minItems: 4
> > +    maxItems: 6
> > +
> > +  reg-names:
> > +    minItems: 4
> > +    items:
> > +      - const: control_port
> > +      - const: rx_csr
> > +      - const: rx_desc
> > +      - const: rx_resp
> > +      - const: tx_csr
> > +      - const: tx_desc
> > +      - const: s1  
> 
> This is messed up. You allow only 6 items maximum, but list 7. It
> contradicts your other variants in allOf:if:then.

I'll remove that part then, apparently it's not needed at all if the
allOf:if:then cover all cases.

Thanks for the review,

Maxime
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski Aug. 30, 2022, 7:19 p.m. UTC | #2
On 30/08/2022 22:16, Maxime Chevallier wrote:
>>> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>  
>>
>> Rebase your changes on some decent kernel and use
>> get_maintainers.pl...
> 
> I'm rebased against net-next, so I don't understand how I'm supposed to
> do for this series, should I sent binding patches separately and based
> on another tree ?
> 
> I'll cc you next time, sorry about that.

net-next is correct, I just assumed it is some older tree since you did
not Cc me.

> 
>>> ---
>>> V1->V2:
>>>  - Removed unnedded maxItems
>>>  - Added missing minItems
>>>  - Fixed typos in some properties names
>>>  - Fixed the mdio subnode definition
>>>
>>>  .../devicetree/bindings/net/altera_tse.txt    | 113 -------------
>>>  .../devicetree/bindings/net/altr,tse.yaml     | 156
>>> ++++++++++++++++++ 2 files changed, 156 insertions(+), 113
>>> deletions(-) delete mode 100644
>>> Documentation/devicetree/bindings/net/altera_tse.txt create mode
>>> 100644 Documentation/devicetree/bindings/net/altr,tse.yaml 
>>
>> (...)
>>
>>> diff --git a/Documentation/devicetree/bindings/net/altr,tse.yaml
>>> b/Documentation/devicetree/bindings/net/altr,tse.yaml new file mode
>>> 100644 index 000000000000..1676e13b8c64
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/altr,tse.yaml
>>> @@ -0,0 +1,156 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/net/altr,tse.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Altera Triple Speed Ethernet MAC driver (TSE)
>>> +
>>> +maintainers:
>>> +  - Maxime Chevallier <maxime.chevallier@bootlin.com>
>>> +
>>> +allOf:  
>>
>> Put allOf below "required".
> 
> Ack
> 
>>> +  - $ref: "ethernet-controller.yaml#"
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - altr,tse-1.0
>>> +              - ALTR,tse-1.0
>>> +    then:
>>> +      properties:
>>> +        reg:
>>> +          minItems: 4
>>> +        reg-names:
>>> +          items:
>>> +            - const: control_port
>>> +            - const: rx_csr
>>> +            - const: tx_csr
>>> +            - const: s1
>>> +
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - altr,tse-msgdma-1.0
>>> +    then:
>>> +      properties:
>>> +        reg:
>>> +          minItems: 6
>>> +        reg-names:
>>> +          minItems: 6  
>>
>> No need for minItems.
> 
> Ok I'll remove it
> 
>>> +          items:
>>> +            - const: control_port
>>> +            - const: rx_csr
>>> +            - const: rx_desc
>>> +            - const: rx_resp
>>> +            - const: tx_csr
>>> +            - const: tx_desc
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - altr,tse-1.0
>>> +      - ALTR,tse-1.0  
>>
>> This is deprecated compatible. You need oneOf and then deprecated:
>> true.
> 
> Ok thanks for the tip
> 
>>> +      - altr,tse-msgdma-1.0
>>> +
>>> +  reg:
>>> +    minItems: 4
>>> +    maxItems: 6
>>> +
>>> +  reg-names:
>>> +    minItems: 4
>>> +    items:
>>> +      - const: control_port
>>> +      - const: rx_csr
>>> +      - const: rx_desc
>>> +      - const: rx_resp
>>> +      - const: tx_csr
>>> +      - const: tx_desc
>>> +      - const: s1  
>>
>> This is messed up. You allow only 6 items maximum, but list 7. It
>> contradicts your other variants in allOf:if:then.
> 
> I'll remove that part then, apparently it's not needed at all if the
> allOf:if:then cover all cases.

Right. The typical pattern is like clocks/clock-names here:

https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L57

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/altera_tse.txt b/Documentation/devicetree/bindings/net/altera_tse.txt
deleted file mode 100644
index 1d9148ff5130..000000000000
--- a/Documentation/devicetree/bindings/net/altera_tse.txt
+++ /dev/null
@@ -1,113 +0,0 @@ 
-* Altera Triple-Speed Ethernet MAC driver (TSE)
-
-Required properties:
-- compatible: Should be "altr,tse-1.0" for legacy SGDMA based TSE, and should
-		be "altr,tse-msgdma-1.0" for the preferred MSGDMA based TSE.
-		ALTR is supported for legacy device trees, but is deprecated.
-		altr should be used for all new designs.
-- reg: Address and length of the register set for the device. It contains
-  the information of registers in the same order as described by reg-names
-- reg-names: Should contain the reg names
-  "control_port": MAC configuration space region
-  "tx_csr":       xDMA Tx dispatcher control and status space region
-  "tx_desc":      MSGDMA Tx dispatcher descriptor space region
-  "rx_csr" :      xDMA Rx dispatcher control and status space region
-  "rx_desc":      MSGDMA Rx dispatcher descriptor space region
-  "rx_resp":      MSGDMA Rx dispatcher response space region
-  "s1":		  SGDMA descriptor memory
-- interrupts: Should contain the TSE interrupts and its mode.
-- interrupt-names: Should contain the interrupt names
-  "rx_irq":       xDMA Rx dispatcher interrupt
-  "tx_irq":       xDMA Tx dispatcher interrupt
-- rx-fifo-depth: MAC receive FIFO buffer depth in bytes
-- tx-fifo-depth: MAC transmit FIFO buffer depth in bytes
-- phy-mode: See ethernet.txt in the same directory.
-- phy-handle: See ethernet.txt in the same directory.
-- phy-addr: See ethernet.txt in the same directory. A configuration should
-		include phy-handle or phy-addr.
-- altr,has-supplementary-unicast:
-		If present, TSE supports additional unicast addresses.
-		Otherwise additional unicast addresses are not supported.
-- altr,has-hash-multicast-filter:
-		If present, TSE supports a hash based multicast filter.
-		Otherwise, hash-based multicast filtering is not supported.
-
-- mdio device tree subnode: When the TSE has a phy connected to its local
-		mdio, there must be device tree subnode with the following
-		required properties:
-
-	- compatible: Must be "altr,tse-mdio".
-	- #address-cells: Must be <1>.
-	- #size-cells: Must be <0>.
-
-	For each phy on the mdio bus, there must be a node with the following
-	fields:
-
-	- reg: phy id used to communicate to phy.
-	- device_type: Must be "ethernet-phy".
-
-The MAC address will be determined using the optional properties defined in
-ethernet.txt.
-
-Example:
-
-	tse_sub_0_eth_tse_0: ethernet@1,00000000 {
-		compatible = "altr,tse-msgdma-1.0";
-		reg =	<0x00000001 0x00000000 0x00000400>,
-			<0x00000001 0x00000460 0x00000020>,
-			<0x00000001 0x00000480 0x00000020>,
-			<0x00000001 0x000004A0 0x00000008>,
-			<0x00000001 0x00000400 0x00000020>,
-			<0x00000001 0x00000420 0x00000020>;
-		reg-names = "control_port", "rx_csr", "rx_desc", "rx_resp", "tx_csr", "tx_desc";
-		interrupt-parent = <&hps_0_arm_gic_0>;
-		interrupts = <0 41 4>, <0 40 4>;
-		interrupt-names = "rx_irq", "tx_irq";
-		rx-fifo-depth = <2048>;
-		tx-fifo-depth = <2048>;
-		address-bits = <48>;
-		max-frame-size = <1500>;
-		local-mac-address = [ 00 00 00 00 00 00 ];
-		phy-mode = "gmii";
-		altr,has-supplementary-unicast;
-		altr,has-hash-multicast-filter;
-		phy-handle = <&phy0>;
-		mdio {
-			compatible = "altr,tse-mdio";
-			#address-cells = <1>;
-			#size-cells = <0>;
-			phy0: ethernet-phy@0 {
-				reg = <0x0>;
-				device_type = "ethernet-phy";
-			};
-
-			phy1: ethernet-phy@1 {
-				reg = <0x1>;
-				device_type = "ethernet-phy";
-			};
-
-		};
-	};
-
-	tse_sub_1_eth_tse_0: ethernet@1,00001000 {
-		compatible = "altr,tse-msgdma-1.0";
-		reg = 	<0x00000001 0x00001000 0x00000400>,
-			<0x00000001 0x00001460 0x00000020>,
-			<0x00000001 0x00001480 0x00000020>,
-			<0x00000001 0x000014A0 0x00000008>,
-			<0x00000001 0x00001400 0x00000020>,
-			<0x00000001 0x00001420 0x00000020>;
-		reg-names = "control_port", "rx_csr", "rx_desc", "rx_resp", "tx_csr", "tx_desc";
-		interrupt-parent = <&hps_0_arm_gic_0>;
-		interrupts = <0 43 4>, <0 42 4>;
-		interrupt-names = "rx_irq", "tx_irq";
-		rx-fifo-depth = <2048>;
-		tx-fifo-depth = <2048>;
-		address-bits = <48>;
-		max-frame-size = <1500>;
-		local-mac-address = [ 00 00 00 00 00 00 ];
-		phy-mode = "gmii";
-		altr,has-supplementary-unicast;
-		altr,has-hash-multicast-filter;
-		phy-handle = <&phy1>;
-	};
diff --git a/Documentation/devicetree/bindings/net/altr,tse.yaml b/Documentation/devicetree/bindings/net/altr,tse.yaml
new file mode 100644
index 000000000000..1676e13b8c64
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/altr,tse.yaml
@@ -0,0 +1,156 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/altr,tse.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Altera Triple Speed Ethernet MAC driver (TSE)
+
+maintainers:
+  - Maxime Chevallier <maxime.chevallier@bootlin.com>
+
+allOf:
+  - $ref: "ethernet-controller.yaml#"
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - altr,tse-1.0
+              - ALTR,tse-1.0
+    then:
+      properties:
+        reg:
+          minItems: 4
+        reg-names:
+          items:
+            - const: control_port
+            - const: rx_csr
+            - const: tx_csr
+            - const: s1
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - altr,tse-msgdma-1.0
+    then:
+      properties:
+        reg:
+          minItems: 6
+        reg-names:
+          minItems: 6
+          items:
+            - const: control_port
+            - const: rx_csr
+            - const: rx_desc
+            - const: rx_resp
+            - const: tx_csr
+            - const: tx_desc
+
+properties:
+  compatible:
+    enum:
+      - altr,tse-1.0
+      - ALTR,tse-1.0
+      - altr,tse-msgdma-1.0
+
+  reg:
+    minItems: 4
+    maxItems: 6
+
+  reg-names:
+    minItems: 4
+    items:
+      - const: control_port
+      - const: rx_csr
+      - const: rx_desc
+      - const: rx_resp
+      - const: tx_csr
+      - const: tx_desc
+      - const: s1
+
+  interrupts:
+    minItems: 2
+
+  interrupt-names:
+    items:
+      - const: rx_irq
+      - const: tx_irq
+
+  rx-fifo-depth:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Depth in bytes of the RX FIFO
+
+  tx-fifo-depth:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Depth in bytes of the TX FIFO
+
+  altr,has-supplementary-unicast:
+    type: boolean
+    description:
+      If present, TSE supports additional unicast addresses.
+
+  altr,has-hash-multicast-filter:
+    type: boolean
+    description:
+      If present, TSE supports hash based multicast filter.
+
+  mdio:
+    $ref: mdio.yaml#
+    unevaluatedProperties: false
+    description:
+      Creates and registers an MDIO bus.
+
+    properties:
+      compatible:
+        const: altr,tse-mdio
+
+    required:
+      - compatible
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - rx-fifo-depth
+  - tx-fifo-depth
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    tse_sub_1_eth_tse_0: ethernet@1,00001000 {
+        compatible = "altr,tse-msgdma-1.0";
+        reg = <0x00001000 0x00000400>,
+              <0x00001460 0x00000020>,
+              <0x00001480 0x00000020>,
+              <0x000014A0 0x00000008>,
+              <0x00001400 0x00000020>,
+              <0x00001420 0x00000020>;
+        reg-names = "control_port", "rx_csr", "rx_desc", "rx_resp", "tx_csr", "tx_desc";
+        interrupt-parent = <&hps_0_arm_gic_0>;
+        interrupts = <0 43 4>, <0 42 4>;
+        interrupt-names = "rx_irq", "tx_irq";
+        rx-fifo-depth = <2048>;
+        tx-fifo-depth = <2048>;
+        max-frame-size = <1500>;
+        local-mac-address = [ 00 00 00 00 00 00 ];
+        phy-mode = "gmii";
+        altr,has-supplementary-unicast;
+        altr,has-hash-multicast-filter;
+        phy-handle = <&phy1>;
+        mdio {
+            compatible = "altr,tse-mdio";
+            #address-cells = <1>;
+            #size-cells = <0>;
+            phy1: ethernet-phy@1 {
+                reg = <0x1>;
+            };
+        };
+    };
+
+...