[PATCH V1 0/3] dt-bindings: net: fec: convert fsl,*fec bindings to yaml

[PATCH V1 0/3] dt-bindings: net: fec: convert fsl,*fec bindings to yaml

[Joakim Zhang]

This patch set intends to convert fec binding into scheme, and fixes
when do dtbs_check on ARCH arm.

One notice is that there are below logs for some dts when do dtbs_check:
	ethernet@2188000: More than one condition true in oneOf schema:
We found that fec node in these dts all have "interrupts-extended"
property, and schema default is:
         'oneOf': [{'required': ['interrupts']},
                   {'required': ['interrupts-extended']}],
so we don't know if it is a common issue or need be fixed in specific
bindings.

Joakim Zhang (3):
  dt-bindings: net: fec: convert fsl,*fec bindings to yaml
  ARM: dts: imx35: correct node name for FEC
  ARM: dts: imx7-mba7: remove un-used "phy-reset-delay" property

 .../devicetree/bindings/net/fsl,fec.yaml      | 213 ++++++++++++++++++
 .../devicetree/bindings/net/fsl-fec.txt       |  95 --------
 arch/arm/boot/dts/imx35.dtsi                  |   2 +-
 arch/arm/boot/dts/imx7-mba7.dtsi              |   1 -
 arch/arm/boot/dts/imx7d-mba7.dts              |   1 -
 5 files changed, 214 insertions(+), 98 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/fsl,fec.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/fsl-fec.txt

-- 
2.17.1

[PATCH V1 1/3] dt-bindings: net: fec: convert fsl,*fec bindings to yaml

[Joakim Zhang]

In order to automate the verification of DT nodes convert fsl-fec.txt to
fsl,fec.yaml, and pass binding check with below command.

$ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/fsl,fec.yaml
  DTEX    Documentation/devicetree/bindings/net/fsl,fec.example.dts
  DTC     Documentation/devicetree/bindings/net/fsl,fec.example.dt.yaml
  CHECK   Documentation/devicetree/bindings/net/fsl,fec.example.dt.yaml

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 .../devicetree/bindings/net/fsl,fec.yaml      | 213 ++++++++++++++++++
 .../devicetree/bindings/net/fsl-fec.txt       |  95 --------
 2 files changed, 213 insertions(+), 95 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/fsl,fec.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/fsl-fec.txt

diff --git a/Documentation/devicetree/bindings/net/fsl,fec.yaml b/Documentation/devicetree/bindings/net/fsl,fec.yaml
new file mode 100644
index 000000000000..7fa11f6622b1
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/fsl,fec.yaml
@@ -0,0 +1,213 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/fsl,fec.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale Fast Ethernet Controller (FEC)
+
+maintainers:
+  - Joakim Zhang <qiangqing.zhang@nxp.com>
+
+allOf:
+  - $ref: ethernet-controller.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - fsl,imx25-fec
+          - fsl,imx27-fec
+          - fsl,imx28-fec
+          - fsl,imx6q-fec
+          - fsl,mvf600-fec
+      - items:
+          - enum:
+              - fsl,imx53-fec
+              - fsl,imx6sl-fec
+          - const: fsl,imx25-fec
+      - items:
+          - enum:
+              - fsl,imx35-fec
+              - fsl,imx51-fec
+          - const: fsl,imx27-fec
+      - items:
+          - enum:
+              - fsl,imx6ul-fec
+              - fsl,imx6sx-fec
+          - const: fsl,imx6q-fec
+      - items:
+          - enum:
+              - fsl,imx7d-fec
+          - const: fsl,imx6sx-fec
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    minItems: 1
+    maxItems: 4
+
+  interrupt-names:
+    description:
+      Names of the interrupts listed in interrupts property in the same order.
+      The defaults if not specified are
+      __Number of interrupts__   __Default__
+            1                       "int0"
+            2                       "int0", "pps"
+            3                       "int0", "int1", "int2"
+            4                       "int0", "int1", "int2", "pps"
+      The order may be changed as long as they correspond to the interrupts
+      property. Currently, only i.mx7 uses "int1" and "int2". They correspond to
+      tx/rx queues 1 and 2. "int0" will be used for queue 0 and ENET_MII interrupts.
+      For imx6sx, "int0" handles all 3 queues and ENET_MII. "pps" is for the pulse
+      per second interrupt associated with 1588 precision time protocol(PTP).
+
+  clocks:
+    minItems: 2
+    maxItems: 5
+    description:
+      The "ipg", for MAC ipg_clk_s, ipg_clk_mac_s that are for register accessing.
+      The "ahb", for MAC ipg_clk, ipg_clk_mac that are bus clock.
+      The "ptp"(option), for IEEE1588 timer clock that requires the clock.
+      The "enet_clk_ref"(option), for MAC transmit/receiver reference clock like
+      RGMII TXC clock or RMII reference clock. It depends on board design,
+      the clock is required if RGMII TXC and RMII reference clock source from
+      SOC internal PLL.
+      The "enet_out"(option), output clock for external device, like supply clock
+      for PHY. The clock is required if PHY clock source from SOC.
+
+  clock-names:
+    minItems: 2
+    maxItems: 5
+    contains:
+      enum:
+      - ipg
+      - ahb
+      - ptp
+      - enet_clk_ref
+      - enet_out
+
+  phy-mode: true
+
+  phy-handle: true
+
+  fixed-link: true
+
+  local-mac-address: true
+
+  mac-address: true
+
+  phy-supply:
+    description:
+      Regulator that powers the Ethernet PHY.
+
+  fsl,num-tx-queues:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      The property is valid for enet-avb IP, which supports hw multi queues.
+      Should specify the tx queue number, otherwise set tx queue number to 1.
+
+  fsl,num-rx-queues:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      The property is valid for enet-avb IP, which supports hw multi queues.
+      Should specify the rx queue number, otherwise set rx queue number to 1.
+
+  fsl,magic-packet:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      If present, indicates that the hardware supports waking up via magic packet.
+
+  fsl,err006687-workaround-present:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      If present indicates that the system has the hardware workaround for
+      ERR006687 applied and does not need a software workaround.
+
+  fsl,stop-mode:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      Register bits of stop mode control, the format is <&gpr req_gpr req_bit>.
+      gpr is the phandle to general purpose register node.
+      req_gpr is the gpr register offset for ENET stop request.
+      req_bit is the gpr bit offset for ENET stop request.
+
+  mdio:
+    type: object
+    description:
+      Specifies the mdio bus in the FEC, used as a container for phy nodes.
+
+  # Deprecated optional properties:
+  # To avoid these, create a phy node according to ethernet-phy.yaml in the same
+  # directory, and point the FEC's "phy-handle" property to it. Then use
+  # the phy's reset binding, again described by ethernet-phy.yaml.
+
+  phy-reset-gpios:
+    deprecated: true
+    description:
+      Should specify the gpio for phy reset.
+
+  phy-reset-duration:
+    deprecated: true
+    description:
+      Reset duration in milliseconds.  Should present only if property
+      "phy-reset-gpios" is available.  Missing the property will have the
+      duration be 1 millisecond.  Numbers greater than 1000 are invalid
+      and 1 millisecond will be used instead.
+
+  phy-reset-active-high:
+    deprecated: true
+    description:
+      If present then the reset sequence using the GPIO specified in the
+      "phy-reset-gpios" property is reversed (H=reset state, L=operation state).
+
+  phy-reset-post-delay:
+    deprecated: true
+    description:
+      Post reset delay in milliseconds. If present then a delay of phy-reset-post-delay
+      milliseconds will be observed after the phy-reset-gpios has been toggled.
+      Can be omitted thus no delay is observed. Delay is in range of 1ms to 1000ms.
+      Other delays are invalid.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+# FIXME: We had better set additionalProperties to false to avoid invalid or at
+# least undocumented properties. However, PHY may have a deprecated option to
+# place PHY OF properties in the MAC node, such as Micrel PHY, and we can find
+# these boards which is based on i.MX6QDL.
+additionalProperties: true
+
+examples:
+  - |
+    ethernet@83fec000 {
+      compatible = "fsl,imx51-fec", "fsl,imx27-fec";
+      reg = <0x83fec000 0x4000>;
+      interrupts = <87>;
+      phy-mode = "mii";
+      phy-reset-gpios = <&gpio2 14 0>;
+      phy-supply = <&reg_fec_supply>;
+    };
+
+    ethernet@83fed000 {
+      compatible = "fsl,imx51-fec", "fsl,imx27-fec";
+      reg = <0x83fed000 0x4000>;
+      interrupts = <87>;
+      phy-mode = "mii";
+      phy-reset-gpios = <&gpio2 14 0>;
+      phy-supply = <&reg_fec_supply>;
+      phy-handle = <&ethphy0>;
+
+      mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ethphy0: ethernet-phy@0 {
+          compatible = "ethernet-phy-ieee802.3-c22";
+          reg = <0>;
+        };
+      };
+    };
diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
deleted file mode 100644
index 9b543789cd52..000000000000
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ /dev/null
@@ -1,95 +0,0 @@
-* Freescale Fast Ethernet Controller (FEC)
-
-Required properties:
-- compatible : Should be "fsl,<soc>-fec"
-- reg : Address and length of the register set for the device
-- interrupts : Should contain fec interrupt
-- phy-mode : See ethernet.txt file in the same directory
-
-Optional properties:
-- phy-supply : regulator that powers the Ethernet PHY.
-- phy-handle : phandle to the PHY device connected to this device.
-- fixed-link : Assume a fixed link. See fixed-link.txt in the same directory.
-  Use instead of phy-handle.
-- fsl,num-tx-queues : The property is valid for enet-avb IP, which supports
-  hw multi queues. Should specify the tx queue number, otherwise set tx queue
-  number to 1.
-- fsl,num-rx-queues : The property is valid for enet-avb IP, which supports
-  hw multi queues. Should specify the rx queue number, otherwise set rx queue
-  number to 1.
-- fsl,magic-packet : If present, indicates that the hardware supports waking
-  up via magic packet.
-- fsl,err006687-workaround-present: If present indicates that the system has
-  the hardware workaround for ERR006687 applied and does not need a software
-  workaround.
-- fsl,stop-mode: register bits of stop mode control, the format is
-		 <&gpr req_gpr req_bit>.
-		 gpr is the phandle to general purpose register node.
-		 req_gpr is the gpr register offset for ENET stop request.
-		 req_bit is the gpr bit offset for ENET stop request.
- -interrupt-names:  names of the interrupts listed in interrupts property in
-  the same order. The defaults if not specified are
-  __Number of interrupts__   __Default__
-	1			"int0"
-	2			"int0", "pps"
-	3			"int0", "int1", "int2"
-	4			"int0", "int1", "int2", "pps"
-  The order may be changed as long as they correspond to the interrupts
-  property. Currently, only i.mx7 uses "int1" and "int2". They correspond to
-  tx/rx queues 1 and 2. "int0" will be used for queue 0 and ENET_MII interrupts.
-  For imx6sx, "int0" handles all 3 queues and ENET_MII. "pps" is for the pulse
-  per second interrupt associated with 1588 precision time protocol(PTP).
-
-Optional subnodes:
-- mdio : specifies the mdio bus in the FEC, used as a container for phy nodes
-  according to phy.txt in the same directory
-
-Deprecated optional properties:
-	To avoid these, create a phy node according to phy.txt in the same
-	directory, and point the fec's "phy-handle" property to it. Then use
-	the phy's reset binding, again described by phy.txt.
-- phy-reset-gpios : Should specify the gpio for phy reset
-- phy-reset-duration : Reset duration in milliseconds.  Should present
-  only if property "phy-reset-gpios" is available.  Missing the property
-  will have the duration be 1 millisecond.  Numbers greater than 1000 are
-  invalid and 1 millisecond will be used instead.
-- phy-reset-active-high : If present then the reset sequence using the GPIO
-  specified in the "phy-reset-gpios" property is reversed (H=reset state,
-  L=operation state).
-- phy-reset-post-delay : Post reset delay in milliseconds. If present then
-  a delay of phy-reset-post-delay milliseconds will be observed after the
-  phy-reset-gpios has been toggled. Can be omitted thus no delay is
-  observed. Delay is in range of 1ms to 1000ms. Other delays are invalid.
-
-Example:
-
-ethernet@83fec000 {
-	compatible = "fsl,imx51-fec", "fsl,imx27-fec";
-	reg = <0x83fec000 0x4000>;
-	interrupts = <87>;
-	phy-mode = "mii";
-	phy-reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>; /* GPIO2_14 */
-	local-mac-address = [00 04 9F 01 1B B9];
-	phy-supply = <&reg_fec_supply>;
-};
-
-Example with phy specified:
-
-ethernet@83fec000 {
-	compatible = "fsl,imx51-fec", "fsl,imx27-fec";
-	reg = <0x83fec000 0x4000>;
-	interrupts = <87>;
-	phy-mode = "mii";
-	phy-reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>; /* GPIO2_14 */
-	local-mac-address = [00 04 9F 01 1B B9];
-	phy-supply = <&reg_fec_supply>;
-	phy-handle = <&ethphy>;
-	mdio {
-	        clock-frequency = <5000000>;
-		ethphy: ethernet-phy@6 {
-			compatible = "ethernet-phy-ieee802.3-c22";
-			reg = <6>;
-			max-speed = <100>;
-		};
-	};
-};
-- 
2.17.1

Re: [PATCH V1 1/3] dt-bindings: net: fec: convert fsl,*fec bindings to yaml

[Rob Herring]

On Fri, Jul 16, 2021 at 06:29:09PM +0800, Joakim Zhang wrote:
> In order to automate the verification of DT nodes convert fsl-fec.txt to
> fsl,fec.yaml, and pass binding check with below command.
> 
> $ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/fsl,fec.yaml
>   DTEX    Documentation/devicetree/bindings/net/fsl,fec.example.dts
>   DTC     Documentation/devicetree/bindings/net/fsl,fec.example.dt.yaml
>   CHECK   Documentation/devicetree/bindings/net/fsl,fec.example.dt.yaml
> 
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
>  .../devicetree/bindings/net/fsl,fec.yaml      | 213 ++++++++++++++++++
>  .../devicetree/bindings/net/fsl-fec.txt       |  95 --------
>  2 files changed, 213 insertions(+), 95 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/fsl,fec.yaml
>  delete mode 100644 Documentation/devicetree/bindings/net/fsl-fec.txt

Since the networking maintainers can't wait for actual binding reviews, 
you get to send a follow-up patch...

> 
> diff --git a/Documentation/devicetree/bindings/net/fsl,fec.yaml b/Documentation/devicetree/bindings/net/fsl,fec.yaml
> new file mode 100644
> index 000000000000..7fa11f6622b1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/fsl,fec.yaml
> @@ -0,0 +1,213 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/fsl,fec.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale Fast Ethernet Controller (FEC)
> +
> +maintainers:
> +  - Joakim Zhang <qiangqing.zhang@nxp.com>
> +
> +allOf:
> +  - $ref: ethernet-controller.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - fsl,imx25-fec
> +          - fsl,imx27-fec
> +          - fsl,imx28-fec
> +          - fsl,imx6q-fec
> +          - fsl,mvf600-fec
> +      - items:
> +          - enum:
> +              - fsl,imx53-fec
> +              - fsl,imx6sl-fec
> +          - const: fsl,imx25-fec
> +      - items:
> +          - enum:
> +              - fsl,imx35-fec
> +              - fsl,imx51-fec
> +          - const: fsl,imx27-fec
> +      - items:
> +          - enum:
> +              - fsl,imx6ul-fec
> +              - fsl,imx6sx-fec
> +          - const: fsl,imx6q-fec
> +      - items:
> +          - enum:
> +              - fsl,imx7d-fec
> +          - const: fsl,imx6sx-fec
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 4
> +
> +  interrupt-names:
> +    description:
> +      Names of the interrupts listed in interrupts property in the same order.
> +      The defaults if not specified are
> +      __Number of interrupts__   __Default__
> +            1                       "int0"
> +            2                       "int0", "pps"
> +            3                       "int0", "int1", "int2"
> +            4                       "int0", "int1", "int2", "pps"
> +      The order may be changed as long as they correspond to the interrupts
> +      property. 

Why? None of the existing dts files do that and there is no reason to 
support random order. You can do this:

oneOf:
  - minItems: 1
    items:
      - const: int0
      - const: pps
  - minItems: 3
    items:
      - const: int0
      - const: int1
      - const: int2
      - const: pps
      


> Currently, only i.mx7 uses "int1" and "int2". They correspond to

Sounds like another constraint under an if/then schema.

> +      tx/rx queues 1 and 2. "int0" will be used for queue 0 and ENET_MII interrupts.
> +      For imx6sx, "int0" handles all 3 queues and ENET_MII. "pps" is for the pulse
> +      per second interrupt associated with 1588 precision time protocol(PTP).
> +
> +  clocks:
> +    minItems: 2
> +    maxItems: 5
> +    description:
> +      The "ipg", for MAC ipg_clk_s, ipg_clk_mac_s that are for register accessing.
> +      The "ahb", for MAC ipg_clk, ipg_clk_mac that are bus clock.
> +      The "ptp"(option), for IEEE1588 timer clock that requires the clock.
> +      The "enet_clk_ref"(option), for MAC transmit/receiver reference clock like
> +      RGMII TXC clock or RMII reference clock. It depends on board design,
> +      the clock is required if RGMII TXC and RMII reference clock source from
> +      SOC internal PLL.
> +      The "enet_out"(option), output clock for external device, like supply clock
> +      for PHY. The clock is required if PHY clock source from SOC.
> +
> +  clock-names:
> +    minItems: 2
> +    maxItems: 5
> +    contains:
> +      enum:
> +      - ipg
> +      - ahb
> +      - ptp
> +      - enet_clk_ref
> +      - enet_out

This means clock-names contains one of these strings and then anything 
else is valid.

s/contains/items/


> +
> +  phy-mode: true
> +
> +  phy-handle: true
> +
> +  fixed-link: true
> +
> +  local-mac-address: true
> +
> +  mac-address: true
> +
> +  phy-supply:
> +    description:
> +      Regulator that powers the Ethernet PHY.
> +
> +  fsl,num-tx-queues:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      The property is valid for enet-avb IP, which supports hw multi queues.
> +      Should specify the tx queue number, otherwise set tx queue number to 1.

constraints? 2^32 queues are valid?

> +
> +  fsl,num-rx-queues:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      The property is valid for enet-avb IP, which supports hw multi queues.
> +      Should specify the rx queue number, otherwise set rx queue number to 1.

constraints?

> +
> +  fsl,magic-packet:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      If present, indicates that the hardware supports waking up via magic packet.
> +
> +  fsl,err006687-workaround-present:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      If present indicates that the system has the hardware workaround for
> +      ERR006687 applied and does not need a software workaround.
> +
> +  fsl,stop-mode:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      Register bits of stop mode control, the format is <&gpr req_gpr req_bit>.

So, maxItems: 3

> +      gpr is the phandle to general purpose register node.
> +      req_gpr is the gpr register offset for ENET stop request.
> +      req_bit is the gpr bit offset for ENET stop request.
> +
> +  mdio:
> +    type: object
> +    description:
> +      Specifies the mdio bus in the FEC, used as a container for phy nodes.
> +
> +  # Deprecated optional properties:
> +  # To avoid these, create a phy node according to ethernet-phy.yaml in the same
> +  # directory, and point the FEC's "phy-handle" property to it. Then use
> +  # the phy's reset binding, again described by ethernet-phy.yaml.
> +
> +  phy-reset-gpios:
> +    deprecated: true
> +    description:
> +      Should specify the gpio for phy reset.
> +
> +  phy-reset-duration:
> +    deprecated: true
> +    description:
> +      Reset duration in milliseconds.  Should present only if property
> +      "phy-reset-gpios" is available.  Missing the property will have the
> +      duration be 1 millisecond.  Numbers greater than 1000 are invalid
> +      and 1 millisecond will be used instead.
> +
> +  phy-reset-active-high:
> +    deprecated: true
> +    description:
> +      If present then the reset sequence using the GPIO specified in the
> +      "phy-reset-gpios" property is reversed (H=reset state, L=operation state).
> +
> +  phy-reset-post-delay:
> +    deprecated: true
> +    description:
> +      Post reset delay in milliseconds. If present then a delay of phy-reset-post-delay
> +      milliseconds will be observed after the phy-reset-gpios has been toggled.
> +      Can be omitted thus no delay is observed. Delay is in range of 1ms to 1000ms.
> +      Other delays are invalid.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +# FIXME: We had better set additionalProperties to false to avoid invalid or at
> +# least undocumented properties. However, PHY may have a deprecated option to
> +# place PHY OF properties in the MAC node, such as Micrel PHY, and we can find
> +# these boards which is based on i.MX6QDL.
> +additionalProperties: true

Why can't the dts files be updated? Can you point me to an example .dts?

> +
> +examples:
> +  - |
> +    ethernet@83fec000 {
> +      compatible = "fsl,imx51-fec", "fsl,imx27-fec";
> +      reg = <0x83fec000 0x4000>;
> +      interrupts = <87>;
> +      phy-mode = "mii";
> +      phy-reset-gpios = <&gpio2 14 0>;
> +      phy-supply = <&reg_fec_supply>;
> +    };
> +
> +    ethernet@83fed000 {
> +      compatible = "fsl,imx51-fec", "fsl,imx27-fec";
> +      reg = <0x83fed000 0x4000>;
> +      interrupts = <87>;
> +      phy-mode = "mii";
> +      phy-reset-gpios = <&gpio2 14 0>;
> +      phy-supply = <&reg_fec_supply>;
> +      phy-handle = <&ethphy0>;
> +
> +      mdio {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ethphy0: ethernet-phy@0 {
> +          compatible = "ethernet-phy-ieee802.3-c22";
> +          reg = <0>;
> +        };
> +      };
> +    };
> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
> deleted file mode 100644
> index 9b543789cd52..000000000000
> --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
> +++ /dev/null
> @@ -1,95 +0,0 @@
> -* Freescale Fast Ethernet Controller (FEC)
> -
> -Required properties:
> -- compatible : Should be "fsl,<soc>-fec"
> -- reg : Address and length of the register set for the device
> -- interrupts : Should contain fec interrupt
> -- phy-mode : See ethernet.txt file in the same directory
> -
> -Optional properties:
> -- phy-supply : regulator that powers the Ethernet PHY.
> -- phy-handle : phandle to the PHY device connected to this device.
> -- fixed-link : Assume a fixed link. See fixed-link.txt in the same directory.
> -  Use instead of phy-handle.
> -- fsl,num-tx-queues : The property is valid for enet-avb IP, which supports
> -  hw multi queues. Should specify the tx queue number, otherwise set tx queue
> -  number to 1.
> -- fsl,num-rx-queues : The property is valid for enet-avb IP, which supports
> -  hw multi queues. Should specify the rx queue number, otherwise set rx queue
> -  number to 1.
> -- fsl,magic-packet : If present, indicates that the hardware supports waking
> -  up via magic packet.
> -- fsl,err006687-workaround-present: If present indicates that the system has
> -  the hardware workaround for ERR006687 applied and does not need a software
> -  workaround.
> -- fsl,stop-mode: register bits of stop mode control, the format is
> -		 <&gpr req_gpr req_bit>.
> -		 gpr is the phandle to general purpose register node.
> -		 req_gpr is the gpr register offset for ENET stop request.
> -		 req_bit is the gpr bit offset for ENET stop request.
> - -interrupt-names:  names of the interrupts listed in interrupts property in
> -  the same order. The defaults if not specified are
> -  __Number of interrupts__   __Default__
> -	1			"int0"
> -	2			"int0", "pps"
> -	3			"int0", "int1", "int2"
> -	4			"int0", "int1", "int2", "pps"
> -  The order may be changed as long as they correspond to the interrupts
> -  property. Currently, only i.mx7 uses "int1" and "int2". They correspond to
> -  tx/rx queues 1 and 2. "int0" will be used for queue 0 and ENET_MII interrupts.
> -  For imx6sx, "int0" handles all 3 queues and ENET_MII. "pps" is for the pulse
> -  per second interrupt associated with 1588 precision time protocol(PTP).
> -
> -Optional subnodes:
> -- mdio : specifies the mdio bus in the FEC, used as a container for phy nodes
> -  according to phy.txt in the same directory
> -
> -Deprecated optional properties:
> -	To avoid these, create a phy node according to phy.txt in the same
> -	directory, and point the fec's "phy-handle" property to it. Then use
> -	the phy's reset binding, again described by phy.txt.
> -- phy-reset-gpios : Should specify the gpio for phy reset
> -- phy-reset-duration : Reset duration in milliseconds.  Should present
> -  only if property "phy-reset-gpios" is available.  Missing the property
> -  will have the duration be 1 millisecond.  Numbers greater than 1000 are
> -  invalid and 1 millisecond will be used instead.
> -- phy-reset-active-high : If present then the reset sequence using the GPIO
> -  specified in the "phy-reset-gpios" property is reversed (H=reset state,
> -  L=operation state).
> -- phy-reset-post-delay : Post reset delay in milliseconds. If present then
> -  a delay of phy-reset-post-delay milliseconds will be observed after the
> -  phy-reset-gpios has been toggled. Can be omitted thus no delay is
> -  observed. Delay is in range of 1ms to 1000ms. Other delays are invalid.
> -
> -Example:
> -
> -ethernet@83fec000 {
> -	compatible = "fsl,imx51-fec", "fsl,imx27-fec";
> -	reg = <0x83fec000 0x4000>;
> -	interrupts = <87>;
> -	phy-mode = "mii";
> -	phy-reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>; /* GPIO2_14 */
> -	local-mac-address = [00 04 9F 01 1B B9];
> -	phy-supply = <&reg_fec_supply>;
> -};
> -
> -Example with phy specified:
> -
> -ethernet@83fec000 {
> -	compatible = "fsl,imx51-fec", "fsl,imx27-fec";
> -	reg = <0x83fec000 0x4000>;
> -	interrupts = <87>;
> -	phy-mode = "mii";
> -	phy-reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>; /* GPIO2_14 */
> -	local-mac-address = [00 04 9F 01 1B B9];
> -	phy-supply = <&reg_fec_supply>;
> -	phy-handle = <&ethphy>;
> -	mdio {
> -	        clock-frequency = <5000000>;
> -		ethphy: ethernet-phy@6 {
> -			compatible = "ethernet-phy-ieee802.3-c22";
> -			reg = <6>;
> -			max-speed = <100>;
> -		};
> -	};
> -};
> -- 
> 2.17.1
> 
> 

RE: [PATCH V1 1/3] dt-bindings: net: fec: convert fsl,*fec bindings to yaml

[Joakim Zhang]

Hi Rob,

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 2021年7月20日 6:45
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: davem@davemloft.net; kuba@kernel.org; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> bruno.thomsen@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> netdev@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH V1 1/3] dt-bindings: net: fec: convert fsl,*fec bindings to
> yaml
> 
> On Fri, Jul 16, 2021 at 06:29:09PM +0800, Joakim Zhang wrote:
> > In order to automate the verification of DT nodes convert fsl-fec.txt
> > to fsl,fec.yaml, and pass binding check with below command.
> >
> > $ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- dt_binding_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/fsl,fec.yaml
> >   DTEX    Documentation/devicetree/bindings/net/fsl,fec.example.dts
> >   DTC
> Documentation/devicetree/bindings/net/fsl,fec.example.dt.yaml
> >   CHECK
> Documentation/devicetree/bindings/net/fsl,fec.example.dt.yaml
> >
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> >  .../devicetree/bindings/net/fsl,fec.yaml      | 213 ++++++++++++++++++
> >  .../devicetree/bindings/net/fsl-fec.txt       |  95 --------
> >  2 files changed, 213 insertions(+), 95 deletions(-)  create mode
> > 100644 Documentation/devicetree/bindings/net/fsl,fec.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/net/fsl-fec.txt
> 
> Since the networking maintainers can't wait for actual binding reviews, you get
> to send a follow-up patch...
OK.

> >
> > diff --git a/Documentation/devicetree/bindings/net/fsl,fec.yaml
> > b/Documentation/devicetree/bindings/net/fsl,fec.yaml
> > new file mode 100644
> > index 000000000000..7fa11f6622b1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/fsl,fec.yaml
> > @@ -0,0 +1,213 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Fnet%2Ffsl%2Cfec.yaml%23&amp;data=04%7C01
> %7Cqia
> >
> +ngqing.zhang%40nxp.com%7Cf54454be267b426e60d008d94b06dfc7%7C686
> ea1d3b
> >
> +c2b4c6fa92cd99c5c301635%7C0%7C0%7C637623315180063755%7CUnknow
> n%7CTWFp
> >
> +bGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVC
> I6M
> >
> +n0%3D%7C1000&amp;sdata=MG%2F9aKVQ8quuLiYxtC%2F7zqZTY7mIHur4G
> D7tsPtfq%
> > +2Bg%3D&amp;reserved=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cqia
> ngqing
> >
> +.zhang%40nxp.com%7Cf54454be267b426e60d008d94b06dfc7%7C686ea1d3b
> c2b4c6
> >
> +fa92cd99c5c301635%7C0%7C0%7C637623315180063755%7CUnknown%7CT
> WFpbGZsb3
> >
> +d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> %3D%
> >
> +7C1000&amp;sdata=7vf8UYQ7c%2BlaJxzAzhzvkPPvbaX3UWVowCtDtPNO9p
> Q%3D&amp
> > +;reserved=0
> > +
> > +title: Freescale Fast Ethernet Controller (FEC)
> > +
> > +maintainers:
> > +  - Joakim Zhang <qiangqing.zhang@nxp.com>
> > +
> > +allOf:
> > +  - $ref: ethernet-controller.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - enum:
> > +          - fsl,imx25-fec
> > +          - fsl,imx27-fec
> > +          - fsl,imx28-fec
> > +          - fsl,imx6q-fec
> > +          - fsl,mvf600-fec
> > +      - items:
> > +          - enum:
> > +              - fsl,imx53-fec
> > +              - fsl,imx6sl-fec
> > +          - const: fsl,imx25-fec
> > +      - items:
> > +          - enum:
> > +              - fsl,imx35-fec
> > +              - fsl,imx51-fec
> > +          - const: fsl,imx27-fec
> > +      - items:
> > +          - enum:
> > +              - fsl,imx6ul-fec
> > +              - fsl,imx6sx-fec
> > +          - const: fsl,imx6q-fec
> > +      - items:
> > +          - enum:
> > +              - fsl,imx7d-fec
> > +          - const: fsl,imx6sx-fec
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    minItems: 1
> > +    maxItems: 4
> > +
> > +  interrupt-names:
> > +    description:
> > +      Names of the interrupts listed in interrupts property in the same
> order.
> > +      The defaults if not specified are
> > +      __Number of interrupts__   __Default__
> > +            1                       "int0"
> > +            2                       "int0", "pps"
> > +            3                       "int0", "int1", "int2"
> > +            4                       "int0", "int1", "int2", "pps"
> > +      The order may be changed as long as they correspond to the
> interrupts
> > +      property.
> 
> Why? None of the existing dts files do that and there is no reason to support
> random order. You can do this:
> 
> oneOf:
>   - minItems: 1
>     items:
>       - const: int0
>       - const: pps
>   - minItems: 3
>     items:
>       - const: int0
>       - const: int1
>       - const: int2
>       - const: pps

How about below?
    oneOf:
      - items:
          - const: int0
      - items:
          - const: int0
          - const: pps
      - items:
          - const: int0
          - const: int1
          - const: int2
      - items:
          - const: int0
          - const: int1
          - const: int2
          - const: pps
> 	
> 
> > Currently, only i.mx7 uses "int1" and "int2". They correspond to
> 
> Sounds like another constraint under an if/then schema.
> 
> > +      tx/rx queues 1 and 2. "int0" will be used for queue 0 and ENET_MII
> interrupts.
> > +      For imx6sx, "int0" handles all 3 queues and ENET_MII. "pps" is for
> the pulse
> > +      per second interrupt associated with 1588 precision time
> protocol(PTP).
> > +
> > +  clocks:
> > +    minItems: 2
> > +    maxItems: 5
> > +    description:
> > +      The "ipg", for MAC ipg_clk_s, ipg_clk_mac_s that are for register
> accessing.
> > +      The "ahb", for MAC ipg_clk, ipg_clk_mac that are bus clock.
> > +      The "ptp"(option), for IEEE1588 timer clock that requires the clock.
> > +      The "enet_clk_ref"(option), for MAC transmit/receiver reference
> clock like
> > +      RGMII TXC clock or RMII reference clock. It depends on board
> design,
> > +      the clock is required if RGMII TXC and RMII reference clock source
> from
> > +      SOC internal PLL.
> > +      The "enet_out"(option), output clock for external device, like supply
> clock
> > +      for PHY. The clock is required if PHY clock source from SOC.
> > +
> > +  clock-names:
> > +    minItems: 2
> > +    maxItems: 5
> > +    contains:
> > +      enum:
> > +      - ipg
> > +      - ahb
> > +      - ptp
> > +      - enet_clk_ref
> > +      - enet_out
> 
> This means clock-names contains one of these strings and then anything else is
> valid.
> 
> s/contains/items/
OK.

> 
> > +
> > +  phy-mode: true
> > +
> > +  phy-handle: true
> > +
> > +  fixed-link: true
> > +
> > +  local-mac-address: true
> > +
> > +  mac-address: true
> > +
> > +  phy-supply:
> > +    description:
> > +      Regulator that powers the Ethernet PHY.
> > +
> > +  fsl,num-tx-queues:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      The property is valid for enet-avb IP, which supports hw multi
> queues.
> > +      Should specify the tx queue number, otherwise set tx queue number
> to 1.
> 
> constraints? 2^32 queues are valid?
Yes, the value should be limited to 1, 2, 3. Will add:
default: 1
minimum: 1
maximum: 3

> 
> > +
> > +  fsl,num-rx-queues:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      The property is valid for enet-avb IP, which supports hw multi
> queues.
> > +      Should specify the rx queue number, otherwise set rx queue number
> to 1.
> 
> constraints?
Will add.

> 
> > +
> > +  fsl,magic-packet:
> > +    $ref: /schemas/types.yaml#/definitions/flag
> > +    description:
> > +      If present, indicates that the hardware supports waking up via magic
> packet.
> > +
> > +  fsl,err006687-workaround-present:
> > +    $ref: /schemas/types.yaml#/definitions/flag
> > +    description:
> > +      If present indicates that the system has the hardware workaround
> for
> > +      ERR006687 applied and does not need a software workaround.
> > +
> > +  fsl,stop-mode:
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    description:
> > +      Register bits of stop mode control, the format is <&gpr req_gpr
> req_bit>.
> 
> So, maxItems: 3
Why? I think maxItems: 1

> 
> > +      gpr is the phandle to general purpose register node.
> > +      req_gpr is the gpr register offset for ENET stop request.
> > +      req_bit is the gpr bit offset for ENET stop request.
> > +
> > +  mdio:
> > +    type: object
> > +    description:
> > +      Specifies the mdio bus in the FEC, used as a container for phy nodes.
> > +
> > +  # Deprecated optional properties:
> > +  # To avoid these, create a phy node according to ethernet-phy.yaml
> > + in the same  # directory, and point the FEC's "phy-handle" property
> > + to it. Then use  # the phy's reset binding, again described by
> ethernet-phy.yaml.
> > +
> > +  phy-reset-gpios:
> > +    deprecated: true
> > +    description:
> > +      Should specify the gpio for phy reset.
> > +
> > +  phy-reset-duration:
> > +    deprecated: true
> > +    description:
> > +      Reset duration in milliseconds.  Should present only if property
> > +      "phy-reset-gpios" is available.  Missing the property will have the
> > +      duration be 1 millisecond.  Numbers greater than 1000 are invalid
> > +      and 1 millisecond will be used instead.
> > +
> > +  phy-reset-active-high:
> > +    deprecated: true
> > +    description:
> > +      If present then the reset sequence using the GPIO specified in the
> > +      "phy-reset-gpios" property is reversed (H=reset state, L=operation
> state).
> > +
> > +  phy-reset-post-delay:
> > +    deprecated: true
> > +    description:
> > +      Post reset delay in milliseconds. If present then a delay of
> phy-reset-post-delay
> > +      milliseconds will be observed after the phy-reset-gpios has been
> toggled.
> > +      Can be omitted thus no delay is observed. Delay is in range of 1ms to
> 1000ms.
> > +      Other delays are invalid.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +
> > +# FIXME: We had better set additionalProperties to false to avoid
> > +invalid or at # least undocumented properties. However, PHY may have
> > +a deprecated option to # place PHY OF properties in the MAC node,
> > +such as Micrel PHY, and we can find # these boards which is based on
> i.MX6QDL.
> > +additionalProperties: true
> 
> Why can't the dts files be updated? Can you point me to an example .dts?

Below dtsi under fec device node:
arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi
arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi
....

Best Regards,
Joakim Zhang
> > +
> > +examples:
> > +  - |
> > +    ethernet@83fec000 {
> > +      compatible = "fsl,imx51-fec", "fsl,imx27-fec";
> > +      reg = <0x83fec000 0x4000>;
> > +      interrupts = <87>;
> > +      phy-mode = "mii";
> > +      phy-reset-gpios = <&gpio2 14 0>;
> > +      phy-supply = <&reg_fec_supply>;
> > +    };
> > +
> > +    ethernet@83fed000 {
> > +      compatible = "fsl,imx51-fec", "fsl,imx27-fec";
> > +      reg = <0x83fed000 0x4000>;
> > +      interrupts = <87>;
> > +      phy-mode = "mii";
> > +      phy-reset-gpios = <&gpio2 14 0>;
> > +      phy-supply = <&reg_fec_supply>;
> > +      phy-handle = <&ethphy0>;
> > +
> > +      mdio {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        ethphy0: ethernet-phy@0 {
> > +          compatible = "ethernet-phy-ieee802.3-c22";
> > +          reg = <0>;
> > +        };
> > +      };
> > +    };
> > diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt
> > b/Documentation/devicetree/bindings/net/fsl-fec.txt
> > deleted file mode 100644
> > index 9b543789cd52..000000000000
> > --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
> > +++ /dev/null
> > @@ -1,95 +0,0 @@
> > -* Freescale Fast Ethernet Controller (FEC)
> > -
> > -Required properties:
> > -- compatible : Should be "fsl,<soc>-fec"
> > -- reg : Address and length of the register set for the device
> > -- interrupts : Should contain fec interrupt
> > -- phy-mode : See ethernet.txt file in the same directory
> > -
> > -Optional properties:
> > -- phy-supply : regulator that powers the Ethernet PHY.
> > -- phy-handle : phandle to the PHY device connected to this device.
> > -- fixed-link : Assume a fixed link. See fixed-link.txt in the same directory.
> > -  Use instead of phy-handle.
> > -- fsl,num-tx-queues : The property is valid for enet-avb IP, which
> > supports
> > -  hw multi queues. Should specify the tx queue number, otherwise set
> > tx queue
> > -  number to 1.
> > -- fsl,num-rx-queues : The property is valid for enet-avb IP, which
> > supports
> > -  hw multi queues. Should specify the rx queue number, otherwise set
> > rx queue
> > -  number to 1.
> > -- fsl,magic-packet : If present, indicates that the hardware supports
> > waking
> > -  up via magic packet.
> > -- fsl,err006687-workaround-present: If present indicates that the
> > system has
> > -  the hardware workaround for ERR006687 applied and does not need a
> > software
> > -  workaround.
> > -- fsl,stop-mode: register bits of stop mode control, the format is
> > -		 <&gpr req_gpr req_bit>.
> > -		 gpr is the phandle to general purpose register node.
> > -		 req_gpr is the gpr register offset for ENET stop request.
> > -		 req_bit is the gpr bit offset for ENET stop request.
> > - -interrupt-names:  names of the interrupts listed in interrupts
> > property in
> > -  the same order. The defaults if not specified are
> > -  __Number of interrupts__   __Default__
> > -	1			"int0"
> > -	2			"int0", "pps"
> > -	3			"int0", "int1", "int2"
> > -	4			"int0", "int1", "int2", "pps"
> > -  The order may be changed as long as they correspond to the
> > interrupts
> > -  property. Currently, only i.mx7 uses "int1" and "int2". They
> > correspond to
> > -  tx/rx queues 1 and 2. "int0" will be used for queue 0 and ENET_MII
> interrupts.
> > -  For imx6sx, "int0" handles all 3 queues and ENET_MII. "pps" is for
> > the pulse
> > -  per second interrupt associated with 1588 precision time protocol(PTP).
> > -
> > -Optional subnodes:
> > -- mdio : specifies the mdio bus in the FEC, used as a container for
> > phy nodes
> > -  according to phy.txt in the same directory
> > -
> > -Deprecated optional properties:
> > -	To avoid these, create a phy node according to phy.txt in the same
> > -	directory, and point the fec's "phy-handle" property to it. Then use
> > -	the phy's reset binding, again described by phy.txt.
> > -- phy-reset-gpios : Should specify the gpio for phy reset
> > -- phy-reset-duration : Reset duration in milliseconds.  Should
> > present
> > -  only if property "phy-reset-gpios" is available.  Missing the
> > property
> > -  will have the duration be 1 millisecond.  Numbers greater than 1000
> > are
> > -  invalid and 1 millisecond will be used instead.
> > -- phy-reset-active-high : If present then the reset sequence using
> > the GPIO
> > -  specified in the "phy-reset-gpios" property is reversed (H=reset
> > state,
> > -  L=operation state).
> > -- phy-reset-post-delay : Post reset delay in milliseconds. If present
> > then
> > -  a delay of phy-reset-post-delay milliseconds will be observed after
> > the
> > -  phy-reset-gpios has been toggled. Can be omitted thus no delay is
> > -  observed. Delay is in range of 1ms to 1000ms. Other delays are invalid.
> > -
> > -Example:
> > -
> > -ethernet@83fec000 {
> > -	compatible = "fsl,imx51-fec", "fsl,imx27-fec";
> > -	reg = <0x83fec000 0x4000>;
> > -	interrupts = <87>;
> > -	phy-mode = "mii";
> > -	phy-reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>; /* GPIO2_14 */
> > -	local-mac-address = [00 04 9F 01 1B B9];
> > -	phy-supply = <&reg_fec_supply>;
> > -};
> > -
> > -Example with phy specified:
> > -
> > -ethernet@83fec000 {
> > -	compatible = "fsl,imx51-fec", "fsl,imx27-fec";
> > -	reg = <0x83fec000 0x4000>;
> > -	interrupts = <87>;
> > -	phy-mode = "mii";
> > -	phy-reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>; /* GPIO2_14 */
> > -	local-mac-address = [00 04 9F 01 1B B9];
> > -	phy-supply = <&reg_fec_supply>;
> > -	phy-handle = <&ethphy>;
> > -	mdio {
> > -	        clock-frequency = <5000000>;
> > -		ethphy: ethernet-phy@6 {
> > -			compatible = "ethernet-phy-ieee802.3-c22";
> > -			reg = <6>;
> > -			max-speed = <100>;
> > -		};
> > -	};
> > -};
> > --
> > 2.17.1
> >
> >

Re: [PATCH V1 1/3] dt-bindings: net: fec: convert fsl,*fec bindings to yaml

[Rob Herring]

On Tue, Jul 20, 2021 at 3:58 AM Joakim Zhang <qiangqing.zhang@nxp.com> wrote:
>
>
> Hi Rob,
>
> > -----Original Message-----
> > From: Rob Herring <robh@kernel.org>
> > Sent: 2021年7月20日 6:45
> > To: Joakim Zhang <qiangqing.zhang@nxp.com>
> > Cc: davem@davemloft.net; kuba@kernel.org; shawnguo@kernel.org;
> > s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> > bruno.thomsen@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> > netdev@vger.kernel.org; devicetree@vger.kernel.org;
> > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> > Subject: Re: [PATCH V1 1/3] dt-bindings: net: fec: convert fsl,*fec bindings to
> > yaml
> >
> > On Fri, Jul 16, 2021 at 06:29:09PM +0800, Joakim Zhang wrote:
> > > In order to automate the verification of DT nodes convert fsl-fec.txt
> > > to fsl,fec.yaml, and pass binding check with below command.
> > >
> > > $ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- dt_binding_check
> > DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/fsl,fec.yaml
> > >   DTEX    Documentation/devicetree/bindings/net/fsl,fec.example.dts
> > >   DTC
> > Documentation/devicetree/bindings/net/fsl,fec.example.dt.yaml
> > >   CHECK
> > Documentation/devicetree/bindings/net/fsl,fec.example.dt.yaml
> > >
> > > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > > ---
> > >  .../devicetree/bindings/net/fsl,fec.yaml      | 213 ++++++++++++++++++
> > >  .../devicetree/bindings/net/fsl-fec.txt       |  95 --------
> > >  2 files changed, 213 insertions(+), 95 deletions(-)  create mode
> > > 100644 Documentation/devicetree/bindings/net/fsl,fec.yaml
> > >  delete mode 100644 Documentation/devicetree/bindings/net/fsl-fec.txt
> >
> > Since the networking maintainers can't wait for actual binding reviews, you get
> > to send a follow-up patch...
> OK.
>
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/fsl,fec.yaml
> > > b/Documentation/devicetree/bindings/net/fsl,fec.yaml
> > > new file mode 100644
> > > index 000000000000..7fa11f6622b1
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/fsl,fec.yaml
> > > @@ -0,0 +1,213 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id:
> > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > >
> > +cetree.org%2Fschemas%2Fnet%2Ffsl%2Cfec.yaml%23&amp;data=04%7C01
> > %7Cqia
> > >
> > +ngqing.zhang%40nxp.com%7Cf54454be267b426e60d008d94b06dfc7%7C686
> > ea1d3b
> > >
> > +c2b4c6fa92cd99c5c301635%7C0%7C0%7C637623315180063755%7CUnknow
> > n%7CTWFp
> > >
> > +bGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVC
> > I6M
> > >
> > +n0%3D%7C1000&amp;sdata=MG%2F9aKVQ8quuLiYxtC%2F7zqZTY7mIHur4G
> > D7tsPtfq%
> > > +2Bg%3D&amp;reserved=0
> > > +$schema:
> > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > >
> > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cqia
> > ngqing
> > >
> > +.zhang%40nxp.com%7Cf54454be267b426e60d008d94b06dfc7%7C686ea1d3b
> > c2b4c6
> > >
> > +fa92cd99c5c301635%7C0%7C0%7C637623315180063755%7CUnknown%7CT
> > WFpbGZsb3
> > >
> > +d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> > %3D%
> > >
> > +7C1000&amp;sdata=7vf8UYQ7c%2BlaJxzAzhzvkPPvbaX3UWVowCtDtPNO9p
> > Q%3D&amp
> > > +;reserved=0
> > > +
> > > +title: Freescale Fast Ethernet Controller (FEC)
> > > +
> > > +maintainers:
> > > +  - Joakim Zhang <qiangqing.zhang@nxp.com>
> > > +
> > > +allOf:
> > > +  - $ref: ethernet-controller.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - enum:
> > > +          - fsl,imx25-fec
> > > +          - fsl,imx27-fec
> > > +          - fsl,imx28-fec
> > > +          - fsl,imx6q-fec
> > > +          - fsl,mvf600-fec
> > > +      - items:
> > > +          - enum:
> > > +              - fsl,imx53-fec
> > > +              - fsl,imx6sl-fec
> > > +          - const: fsl,imx25-fec
> > > +      - items:
> > > +          - enum:
> > > +              - fsl,imx35-fec
> > > +              - fsl,imx51-fec
> > > +          - const: fsl,imx27-fec
> > > +      - items:
> > > +          - enum:
> > > +              - fsl,imx6ul-fec
> > > +              - fsl,imx6sx-fec
> > > +          - const: fsl,imx6q-fec
> > > +      - items:
> > > +          - enum:
> > > +              - fsl,imx7d-fec
> > > +          - const: fsl,imx6sx-fec
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    minItems: 1
> > > +    maxItems: 4
> > > +
> > > +  interrupt-names:
> > > +    description:
> > > +      Names of the interrupts listed in interrupts property in the same
> > order.
> > > +      The defaults if not specified are
> > > +      __Number of interrupts__   __Default__
> > > +            1                       "int0"
> > > +            2                       "int0", "pps"
> > > +            3                       "int0", "int1", "int2"
> > > +            4                       "int0", "int1", "int2", "pps"
> > > +      The order may be changed as long as they correspond to the
> > interrupts
> > > +      property.
> >
> > Why? None of the existing dts files do that and there is no reason to support
> > random order. You can do this:
> >
> > oneOf:
> >   - minItems: 1
> >     items:
> >       - const: int0
> >       - const: pps
> >   - minItems: 3
> >     items:
> >       - const: int0
> >       - const: int1
> >       - const: int2
> >       - const: pps
>
> How about below?
>     oneOf:
>       - items:
>           - const: int0
>       - items:
>           - const: int0
>           - const: pps
>       - items:
>           - const: int0
>           - const: int1
>           - const: int2
>       - items:
>           - const: int0
>           - const: int1
>           - const: int2
>           - const: pps

What I wrote for you is the same thing, just more concise.

> >
> >
> > > Currently, only i.mx7 uses "int1" and "int2". They correspond to
> >
> > Sounds like another constraint under an if/then schema.
> >
> > > +      tx/rx queues 1 and 2. "int0" will be used for queue 0 and ENET_MII
> > interrupts.
> > > +      For imx6sx, "int0" handles all 3 queues and ENET_MII. "pps" is for
> > the pulse
> > > +      per second interrupt associated with 1588 precision time
> > protocol(PTP).
> > > +
> > > +  clocks:
> > > +    minItems: 2
> > > +    maxItems: 5
> > > +    description:
> > > +      The "ipg", for MAC ipg_clk_s, ipg_clk_mac_s that are for register
> > accessing.
> > > +      The "ahb", for MAC ipg_clk, ipg_clk_mac that are bus clock.
> > > +      The "ptp"(option), for IEEE1588 timer clock that requires the clock.
> > > +      The "enet_clk_ref"(option), for MAC transmit/receiver reference
> > clock like
> > > +      RGMII TXC clock or RMII reference clock. It depends on board
> > design,
> > > +      the clock is required if RGMII TXC and RMII reference clock source
> > from
> > > +      SOC internal PLL.
> > > +      The "enet_out"(option), output clock for external device, like supply
> > clock
> > > +      for PHY. The clock is required if PHY clock source from SOC.
> > > +
> > > +  clock-names:
> > > +    minItems: 2
> > > +    maxItems: 5
> > > +    contains:
> > > +      enum:
> > > +      - ipg
> > > +      - ahb
> > > +      - ptp
> > > +      - enet_clk_ref
> > > +      - enet_out
> >
> > This means clock-names contains one of these strings and then anything else is
> > valid.
> >
> > s/contains/items/
> OK.
>
> >
> > > +
> > > +  phy-mode: true
> > > +
> > > +  phy-handle: true
> > > +
> > > +  fixed-link: true
> > > +
> > > +  local-mac-address: true
> > > +
> > > +  mac-address: true
> > > +
> > > +  phy-supply:
> > > +    description:
> > > +      Regulator that powers the Ethernet PHY.
> > > +
> > > +  fsl,num-tx-queues:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description:
> > > +      The property is valid for enet-avb IP, which supports hw multi
> > queues.
> > > +      Should specify the tx queue number, otherwise set tx queue number
> > to 1.
> >
> > constraints? 2^32 queues are valid?
> Yes, the value should be limited to 1, 2, 3. Will add:
> default: 1
> minimum: 1
> maximum: 3

Personally, I'd do 'enum: [ 1, 2, 3 ]' instead.

>
> >
> > > +
> > > +  fsl,num-rx-queues:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description:
> > > +      The property is valid for enet-avb IP, which supports hw multi
> > queues.
> > > +      Should specify the rx queue number, otherwise set rx queue number
> > to 1.
> >
> > constraints?
> Will add.
>
> >
> > > +
> > > +  fsl,magic-packet:
> > > +    $ref: /schemas/types.yaml#/definitions/flag
> > > +    description:
> > > +      If present, indicates that the hardware supports waking up via magic
> > packet.
> > > +
> > > +  fsl,err006687-workaround-present:
> > > +    $ref: /schemas/types.yaml#/definitions/flag
> > > +    description:
> > > +      If present indicates that the system has the hardware workaround
> > for
> > > +      ERR006687 applied and does not need a software workaround.
> > > +
> > > +  fsl,stop-mode:
> > > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > > +    description:
> > > +      Register bits of stop mode control, the format is <&gpr req_gpr
> > req_bit>.
> >
> > So, maxItems: 3
> Why? I think maxItems: 1
>
> >
> > > +      gpr is the phandle to general purpose register node.
> > > +      req_gpr is the gpr register offset for ENET stop request.
> > > +      req_bit is the gpr bit offset for ENET stop request.
> > > +
> > > +  mdio:
> > > +    type: object
> > > +    description:
> > > +      Specifies the mdio bus in the FEC, used as a container for phy nodes.
> > > +
> > > +  # Deprecated optional properties:
> > > +  # To avoid these, create a phy node according to ethernet-phy.yaml
> > > + in the same  # directory, and point the FEC's "phy-handle" property
> > > + to it. Then use  # the phy's reset binding, again described by
> > ethernet-phy.yaml.
> > > +
> > > +  phy-reset-gpios:
> > > +    deprecated: true
> > > +    description:
> > > +      Should specify the gpio for phy reset.
> > > +
> > > +  phy-reset-duration:
> > > +    deprecated: true
> > > +    description:
> > > +      Reset duration in milliseconds.  Should present only if property
> > > +      "phy-reset-gpios" is available.  Missing the property will have the
> > > +      duration be 1 millisecond.  Numbers greater than 1000 are invalid
> > > +      and 1 millisecond will be used instead.
> > > +
> > > +  phy-reset-active-high:
> > > +    deprecated: true
> > > +    description:
> > > +      If present then the reset sequence using the GPIO specified in the
> > > +      "phy-reset-gpios" property is reversed (H=reset state, L=operation
> > state).
> > > +
> > > +  phy-reset-post-delay:
> > > +    deprecated: true
> > > +    description:
> > > +      Post reset delay in milliseconds. If present then a delay of
> > phy-reset-post-delay
> > > +      milliseconds will be observed after the phy-reset-gpios has been
> > toggled.
> > > +      Can be omitted thus no delay is observed. Delay is in range of 1ms to
> > 1000ms.
> > > +      Other delays are invalid.
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - interrupts
> > > +
> > > +# FIXME: We had better set additionalProperties to false to avoid
> > > +invalid or at # least undocumented properties. However, PHY may have
> > > +a deprecated option to # place PHY OF properties in the MAC node,
> > > +such as Micrel PHY, and we can find # these boards which is based on
> > i.MX6QDL.
> > > +additionalProperties: true
> >
> > Why can't the dts files be updated? Can you point me to an example .dts?
>
> Below dtsi under fec device node:
> arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi
> arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi

You either need to document all those deprecated properties or update
the dts files. I'd prefer the latter.

Rob

[PATCH V1 2/3] ARM: dts: imx35: correct node name for FEC

[Joakim Zhang]

Correct node name for FEC which found when do dtbs_check.

$ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/fsl,fec.yaml
arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard.dt.yaml: fec@50038000: $nodename:0: 'fec@50038000' does not match '^ethernet(@.*)?$'
arch/arm/boot/dts/imx35-pdk.dt.yaml: fec@50038000: $nodename:0: 'fec@50038000' does not match '^ethernet(@.*)?$'

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 arch/arm/boot/dts/imx35.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx35.dtsi b/arch/arm/boot/dts/imx35.dtsi
index 98ccc81ca6d9..8e41c8b7bd70 100644
--- a/arch/arm/boot/dts/imx35.dtsi
+++ b/arch/arm/boot/dts/imx35.dtsi
@@ -189,7 +189,7 @@
 				status = "disabled";
 			};
 
-			fec: fec@50038000 {
+			fec: ethernet@50038000 {
 				compatible = "fsl,imx35-fec", "fsl,imx27-fec";
 				reg = <0x50038000 0x4000>;
 				clocks = <&clks 46>, <&clks 8>;
-- 
2.17.1

[PATCH V1 3/3] ARM: dts: imx7-mba7: remove un-used "phy-reset-delay" property

[Joakim Zhang]

Remove un-used "phy-reset-delay" property which found when do dtbs_check
(set additionalProperties: false in fsl,fec.yaml).
Double check current driver and commit history, "phy-reset-delay" never comes
up, so it should be safe to remove it.

$ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/fsl,fec.yaml
arch/arm/boot/dts/imx7d-mba7.dt.yaml: ethernet@30be0000: 'phy-reset-delay' does not match any of the regexes: 'pinctrl-[0-9]+'
arch/arm/boot/dts/imx7d-mba7.dt.yaml: ethernet@30bf0000: 'phy-reset-delay' does not match any of the regexes: 'pinctrl-[0-9]+'
/arch/arm/boot/dts/imx7s-mba7.dt.yaml: ethernet@30be0000: 'phy-reset-delay' does not match any of the regexes: 'pinctrl-[0-9]+'

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 arch/arm/boot/dts/imx7-mba7.dtsi | 1 -
 arch/arm/boot/dts/imx7d-mba7.dts | 1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx7-mba7.dtsi b/arch/arm/boot/dts/imx7-mba7.dtsi
index c6d1c63f7905..5e6bef230dc7 100644
--- a/arch/arm/boot/dts/imx7-mba7.dtsi
+++ b/arch/arm/boot/dts/imx7-mba7.dtsi
@@ -216,7 +216,6 @@
 	phy-mode = "rgmii-id";
 	phy-reset-gpios = <&gpio7 15 GPIO_ACTIVE_LOW>;
 	phy-reset-duration = <1>;
-	phy-reset-delay = <1>;
 	phy-supply = <&reg_fec1_pwdn>;
 	phy-handle = <&ethphy1_0>;
 	fsl,magic-packet;
diff --git a/arch/arm/boot/dts/imx7d-mba7.dts b/arch/arm/boot/dts/imx7d-mba7.dts
index 23856a8d4b8c..36ef6a3cdb0b 100644
--- a/arch/arm/boot/dts/imx7d-mba7.dts
+++ b/arch/arm/boot/dts/imx7d-mba7.dts
@@ -23,7 +23,6 @@
 	phy-mode = "rgmii-id";
 	phy-reset-gpios = <&gpio2 28 GPIO_ACTIVE_LOW>;
 	phy-reset-duration = <1>;
-	phy-reset-delay = <1>;
 	phy-supply = <&reg_fec2_pwdn>;
 	phy-handle = <&ethphy2_0>;
 	fsl,magic-packet;
-- 
2.17.1

Re: [PATCH V1 0/3] dt-bindings: net: fec: convert fsl,*fec bindings to yaml

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Fri, 16 Jul 2021 18:29:08 +0800 you wrote:
> This patch set intends to convert fec binding into scheme, and fixes
> when do dtbs_check on ARCH arm.
> 
> One notice is that there are below logs for some dts when do dtbs_check:
> 	ethernet@2188000: More than one condition true in oneOf schema:
> We found that fec node in these dts all have "interrupts-extended"
> property, and schema default is:
>          'oneOf': [{'required': ['interrupts']},
>                    {'required': ['interrupts-extended']}],
> so we don't know if it is a common issue or need be fixed in specific
> bindings.
> 
> [...]

Here is the summary with links:
  - [V1,1/3] dt-bindings: net: fec: convert fsl,*fec bindings to yaml
    https://git.kernel.org/netdev/net-next/c/96e4781b3d93
  - [V1,2/3] ARM: dts: imx35: correct node name for FEC
    https://git.kernel.org/netdev/net-next/c/95740a9a3ad9
  - [V1,3/3] ARM: dts: imx7-mba7: remove un-used "phy-reset-delay" property
    https://git.kernel.org/netdev/net-next/c/86a176f485b5

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html