[PATCH 0/4] dt-bindings: net: Convert qcom,ethqos bindings to YAML (and related fixes)

[PATCH 0/4] dt-bindings: net: Convert qcom,ethqos bindings to YAML (and related fixes)

[Bhupesh Sharma]

This patchset converts the qcom,ethqos bindings to YAML. It also
contains a few related fixes in the snps,dwmac bindings to support
Qualcomm ethqos ethernet controller for qcs404 (based) and sa8155p-adp
boards.

Note that this patchset depends on the following dts fix to avoid
any 'make dtbs_check' errors:
https://lore.kernel.org/linux-arm-msm/20220907204153.2039776-1-bhupesh.sharma@linaro.org/T/#u

Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>

Bhupesh Sharma (4):
  dt-bindings: net: qcom,ethqos: Convert bindings to yaml
  dt-bindings: net: snps,dwmac: Add Qualcomm Ethernet ETHQOS compatibles
  dt-bindings: net: snps,dwmac: Update reg maxitems
  dt-bindings: net: snps,dwmac: Update interrupt-names

 .../devicetree/bindings/net/qcom,ethqos.txt   |  66 ---------
 .../devicetree/bindings/net/qcom,ethqos.yaml  | 139 ++++++++++++++++++
 .../devicetree/bindings/net/snps,dwmac.yaml   |  16 +-
 3 files changed, 150 insertions(+), 71 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.txt
 create mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.yaml

-- 
2.37.1

[PATCH 1/4] dt-bindings: net: qcom,ethqos: Convert bindings to yaml

[Bhupesh Sharma]

Convert Qualcomm ETHQOS Ethernet devicetree binding to YAML.

Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 .../devicetree/bindings/net/qcom,ethqos.txt   |  66 ---------
 .../devicetree/bindings/net/qcom,ethqos.yaml  | 139 ++++++++++++++++++
 2 files changed, 139 insertions(+), 66 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.txt
 create mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.yaml

diff --git a/Documentation/devicetree/bindings/net/qcom,ethqos.txt b/Documentation/devicetree/bindings/net/qcom,ethqos.txt
deleted file mode 100644
index 1f5746849a71..000000000000
--- a/Documentation/devicetree/bindings/net/qcom,ethqos.txt
+++ /dev/null
@@ -1,66 +0,0 @@
-Qualcomm Ethernet ETHQOS device
-
-This documents dwmmac based ethernet device which supports Gigabit
-ethernet for version v2.3.0 onwards.
-
-This device has following properties:
-
-Required properties:
-
-- compatible: Should be one of:
-		"qcom,qcs404-ethqos"
-		"qcom,sm8150-ethqos"
-
-- reg: Address and length of the register set for the device
-
-- reg-names: Should contain register names "stmmaceth", "rgmii"
-
-- clocks: Should contain phandle to clocks
-
-- clock-names: Should contain clock names "stmmaceth", "pclk",
-		"ptp_ref", "rgmii"
-
-- interrupts: Should contain phandle to interrupts
-
-- interrupt-names: Should contain interrupt names "macirq", "eth_lpi"
-
-Rest of the properties are defined in stmmac.txt file in same directory
-
-
-Example:
-
-ethernet: ethernet@7a80000 {
-	compatible = "qcom,qcs404-ethqos";
-	reg = <0x07a80000 0x10000>,
-		<0x07a96000 0x100>;
-	reg-names = "stmmaceth", "rgmii";
-	clock-names = "stmmaceth", "pclk", "ptp_ref", "rgmii";
-	clocks = <&gcc GCC_ETH_AXI_CLK>,
-		<&gcc GCC_ETH_SLAVE_AHB_CLK>,
-		<&gcc GCC_ETH_PTP_CLK>,
-		<&gcc GCC_ETH_RGMII_CLK>;
-	interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
-			<GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
-	interrupt-names = "macirq", "eth_lpi";
-	snps,reset-gpio = <&tlmm 60 GPIO_ACTIVE_LOW>;
-	snps,reset-active-low;
-
-	snps,txpbl = <8>;
-	snps,rxpbl = <2>;
-	snps,aal;
-	snps,tso;
-
-	phy-handle = <&phy1>;
-	phy-mode = "rgmii";
-
-	mdio {
-		#address-cells = <0x1>;
-		#size-cells = <0x0>;
-		compatible = "snps,dwmac-mdio";
-		phy1: phy@4 {
-			device_type = "ethernet-phy";
-			reg = <0x4>;
-		};
-	};
-
-};
diff --git a/Documentation/devicetree/bindings/net/qcom,ethqos.yaml b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
new file mode 100644
index 000000000000..f05df9b0d106
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
@@ -0,0 +1,139 @@
+# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/qcom,ethqos.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Ethernet ETHQOS device
+
+maintainers:
+  - Bhupesh Sharma <bhupesh.sharma@linaro.org>
+
+description:
+  This binding describes the dwmmac based Qualcomm ethernet devices which
+  support Gigabit ethernet (version v2.3.0 onwards).
+
+  So, this file documents platform glue layer for dwmmac stmmac based Qualcomm
+  ethernet devices.
+
+allOf:
+  - $ref: "snps,dwmac.yaml#"
+
+properties:
+  compatible:
+    enum:
+      - qcom,qcs404-ethqos
+      - qcom,sm8150-ethqos
+
+  reg: true
+
+  reg-names:
+    minItems: 1
+    items:
+      - const: stmmaceth
+      - const: rgmii
+
+  interrupts: true
+
+  interrupt-names: true
+
+  clocks:
+    minItems: 1
+    maxItems: 4
+
+  clock-names:
+    minItems: 1
+    items:
+      - const: stmmaceth
+      - const: pclk
+      - const: ptp_ref
+      - const: rgmii
+
+  iommus:
+    minItems: 1
+    maxItems: 2
+
+  mdio: true
+
+  phy-handle: true
+
+  phy-mode: true
+
+  snps,reset-gpio: true
+
+  snps,tso:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Enables the TSO feature otherwise it will be managed by MAC HW capability register.
+
+  power-domains: true
+
+  resets: true
+
+  rx-fifo-depth: true
+
+  tx-fifo-depth: true
+
+required:
+  - compatible
+  - clocks
+  - clock-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/qcom,gcc-sm8150.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    ethernet1: ethernet@20000 {
+      compatible = "qcom,sm8150-ethqos";
+      reg = <0x0 0x00020000 0x0 0x10000>,
+            <0x0 0x00036000 0x0 0x100>;
+      reg-names = "stmmaceth", "rgmii";
+      clock-names = "stmmaceth", "pclk", "ptp_ref", "rgmii";
+      clocks = <&gcc GCC_EMAC_AXI_CLK>,
+               <&gcc GCC_EMAC_SLV_AHB_CLK>,
+               <&gcc GCC_EMAC_PTP_CLK>,
+               <&gcc GCC_EMAC_RGMII_CLK>;
+      interrupts = <GIC_SPI 689 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 699 IRQ_TYPE_LEVEL_HIGH>;
+      interrupt-names = "macirq", "eth_lpi";
+
+      power-domains = <&gcc EMAC_GDSC>;
+      resets = <&gcc GCC_EMAC_BCR>;
+      iommus = <&apps_smmu 0x3C0 0x0>;
+
+      snps,tso;
+      rx-fifo-depth = <4096>;
+      tx-fifo-depth = <4096>;
+
+      snps,reset-gpio = <&tlmm 79 GPIO_ACTIVE_LOW>;
+      snps,reset-active-low;
+      snps,reset-delays-us = <0 11000 70000>;
+
+      snps,mtl-rx-config = <&mtl_rx_setup>;
+      snps,mtl-tx-config = <&mtl_tx_setup>;
+
+      pinctrl-names = "default";
+      pinctrl-0 = <&ethernet_defaults>;
+
+      phy-handle = <&rgmii_phy>;
+      phy-mode = "rgmii";
+      max-speed = <1000>;
+
+      mdio {
+        #address-cells = <0x1>;
+        #size-cells = <0x0>;
+
+        compatible = "snps,dwmac-mdio";
+        rgmii_phy: phy@7 {
+          reg = <0x7>;
+          interrupt-parent = <&tlmm>;
+          interrupts-extended = <&tlmm 124 IRQ_TYPE_EDGE_FALLING>;
+          device_type = "ethernet-phy";
+          compatible = "ethernet-phy-ieee802.3-c22";
+        };
+      };
+    };
-- 
2.37.1

[PATCH 2/4] dt-bindings: net: snps,dwmac: Add Qualcomm Ethernet ETHQOS compatibles

[Bhupesh Sharma]

Add Qualcomm Ethernet ETHQOS compatible checks
in snps,dwmac YAML binding document.

Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 2d4e7c7c230a..2b6023ce3ac1 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -65,6 +65,8 @@ properties:
         - ingenic,x2000-mac
         - loongson,ls2k-dwmac
         - loongson,ls7a-dwmac
+        - qcom,qcs404-ethqos
+        - qcom,sm8150-ethqos
         - renesas,r9a06g032-gmac
         - renesas,rzn1-gmac
         - rockchip,px30-gmac
@@ -384,6 +386,8 @@ allOf:
               - ingenic,x1600-mac
               - ingenic,x1830-mac
               - ingenic,x2000-mac
+              - qcom,qcs404-ethqos
+              - qcom,sm8150-ethqos
               - snps,dwmac-4.00
               - snps,dwmac-4.10a
               - snps,dwmac-4.20a
-- 
2.37.1

[PATCH 3/4] dt-bindings: net: snps,dwmac: Update reg maxitems

[Bhupesh Sharma]

Since the Qualcomm dwmac based ETHQOS ethernet block
supports 64-bit register addresses, update the
reg maxitems inside snps,dwmac YAML bindings.

Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 2b6023ce3ac1..f89ca308d55f 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -94,7 +94,7 @@ properties:
 
   reg:
     minItems: 1
-    maxItems: 2
+    maxItems: 4
 
   interrupts:
     minItems: 1
-- 
2.37.1

[PATCH 4/4] dt-bindings: net: snps,dwmac: Update interrupt-names

[Bhupesh Sharma]

As commit fc191af1bb0d ("net: stmmac: platform: Fix misleading
interrupt error msg") noted, not every stmmac based platform
makes use of the 'eth_wake_irq' or 'eth_lpi' interrupts.

So, update the 'interrupt-names' inside 'snps,dwmac' YAML
bindings to reflect the same.

Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index f89ca308d55f..4d7fe4ee3d87 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -105,10 +105,12 @@ properties:
 
   interrupt-names:
     minItems: 1
-    items:
-      - const: macirq
-      - const: eth_wake_irq
-      - const: eth_lpi
+    maxItems: 3
+    contains:
+      enum:
+        - macirq
+        - eth_wake_irq
+        - eth_lpi
 
   clocks:
     minItems: 1
-- 
2.37.1

Re: [PATCH 1/4] dt-bindings: net: qcom,ethqos: Convert bindings to yaml

[Rob Herring]

On Thu, 08 Sep 2022 02:19:21 +0530, Bhupesh Sharma wrote:
> Convert Qualcomm ETHQOS Ethernet devicetree binding to YAML.
> 
> Cc: Bjorn Andersson <andersson@kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
>  .../devicetree/bindings/net/qcom,ethqos.txt   |  66 ---------
>  .../devicetree/bindings/net/qcom,ethqos.yaml  | 139 ++++++++++++++++++
>  2 files changed, 139 insertions(+), 66 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.txt
>  create mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ethqos.example.dtb: ethernet@20000: compatible: ['qcom,sm8150-ethqos'] does not contain items matching the given schema
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ethqos.example.dtb: ethernet@20000: reg: [[0, 131072], [0, 65536], [0, 221184], [0, 256]] is too long
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ethqos.example.dtb: ethernet@20000: interrupt-names:1: 'eth_wake_irq' was expected
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ethqos.example.dtb: ethernet@20000: Unevaluated properties are not allowed ('max-speed', 'snps,mtl-rx-config', 'snps,mtl-tx-config', 'snps,reset-active-low', 'snps,reset-delays-us' were unexpected)
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ethqos.example.dtb: phy@7: '#phy-cells' is a required property
	From schema: /usr/local/lib/python3.10/dist-packages/dtschema/schemas/phy/phy-provider.yaml

doc reference errors (make refcheckdocs):
MAINTAINERS: Documentation/devicetree/bindings/net/qcom,ethqos.txt

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 1/4] dt-bindings: net: qcom,ethqos: Convert bindings to yaml

[Krzysztof Kozlowski]

On 07/09/2022 22:49, Bhupesh Sharma wrote:
> Convert Qualcomm ETHQOS Ethernet devicetree binding to YAML.
> 
> Cc: Bjorn Andersson <andersson@kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>

Thank you for your patch. There is something to discuss/improve.

> ---
>  .../devicetree/bindings/net/qcom,ethqos.txt   |  66 ---------
>  .../devicetree/bindings/net/qcom,ethqos.yaml  | 139 ++++++++++++++++++

You need to update maintainers - old path.

>  2 files changed, 139 insertions(+), 66 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.txt
>  create mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/qcom,ethqos.txt b/Documentation/devicetree/bindings/net/qcom,ethqos.txt
> deleted file mode 100644
> index 1f5746849a71..000000000000
> --- a/Documentation/devicetree/bindings/net/qcom,ethqos.txt
> +++ /dev/null
> @@ -1,66 +0,0 @@
> -Qualcomm Ethernet ETHQOS device
> -
> -This documents dwmmac based ethernet device which supports Gigabit
> -ethernet for version v2.3.0 onwards.
> -
> -This device has following properties:
> -
> -Required properties:
> -
> -- compatible: Should be one of:
> -		"qcom,qcs404-ethqos"
> -		"qcom,sm8150-ethqos"
> -
> -- reg: Address and length of the register set for the device
> -
> -- reg-names: Should contain register names "stmmaceth", "rgmii"
> -
> -- clocks: Should contain phandle to clocks
> -
> -- clock-names: Should contain clock names "stmmaceth", "pclk",
> -		"ptp_ref", "rgmii"
> -
> -- interrupts: Should contain phandle to interrupts
> -
> -- interrupt-names: Should contain interrupt names "macirq", "eth_lpi"
> -
> -Rest of the properties are defined in stmmac.txt file in same directory
> -
> -
> -Example:
> -
> -ethernet: ethernet@7a80000 {
> -	compatible = "qcom,qcs404-ethqos";
> -	reg = <0x07a80000 0x10000>,
> -		<0x07a96000 0x100>;
> -	reg-names = "stmmaceth", "rgmii";
> -	clock-names = "stmmaceth", "pclk", "ptp_ref", "rgmii";
> -	clocks = <&gcc GCC_ETH_AXI_CLK>,
> -		<&gcc GCC_ETH_SLAVE_AHB_CLK>,
> -		<&gcc GCC_ETH_PTP_CLK>,
> -		<&gcc GCC_ETH_RGMII_CLK>;
> -	interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
> -			<GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
> -	interrupt-names = "macirq", "eth_lpi";
> -	snps,reset-gpio = <&tlmm 60 GPIO_ACTIVE_LOW>;
> -	snps,reset-active-low;
> -
> -	snps,txpbl = <8>;
> -	snps,rxpbl = <2>;
> -	snps,aal;
> -	snps,tso;
> -
> -	phy-handle = <&phy1>;
> -	phy-mode = "rgmii";
> -
> -	mdio {
> -		#address-cells = <0x1>;
> -		#size-cells = <0x0>;
> -		compatible = "snps,dwmac-mdio";
> -		phy1: phy@4 {
> -			device_type = "ethernet-phy";
> -			reg = <0x4>;
> -		};
> -	};
> -
> -};
> diff --git a/Documentation/devicetree/bindings/net/qcom,ethqos.yaml b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
> new file mode 100644
> index 000000000000..f05df9b0d106
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
> @@ -0,0 +1,139 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/qcom,ethqos.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Ethernet ETHQOS device
> +
> +maintainers:
> +  - Bhupesh Sharma <bhupesh.sharma@linaro.org>
> +
> +description:
> +  This binding describes the dwmmac based Qualcomm ethernet devices which
> +  support Gigabit ethernet (version v2.3.0 onwards).
> +
> +  So, this file documents platform glue layer for dwmmac stmmac based Qualcomm
> +  ethernet devices.
> +
> +allOf:
> +  - $ref: "snps,dwmac.yaml#"

No need for quotes.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,qcs404-ethqos
> +      - qcom,sm8150-ethqos
> +
> +  reg: true

I think both devices use two reg spaces.

> +
> +  reg-names:
> +    minItems: 1

Why allowing only one item?

> +    items:
> +      - const: stmmaceth
> +      - const: rgmii
> +
> +  interrupts: true

This should be specific/fixed.

> +
> +  interrupt-names: true

This should be specific/fixed.


> +
> +  clocks:
> +    minItems: 1
> +    maxItems: 4

Why such flexibility?

> +
> +  clock-names:
> +    minItems: 1
> +    items:
> +      - const: stmmaceth
> +      - const: pclk
> +      - const: ptp_ref
> +      - const: rgmii
> +
> +  iommus:
> +    minItems: 1
> +    maxItems: 2

Aren't we using only one MMU?

> +
> +  mdio: true
> +
> +  phy-handle: true
> +
> +  phy-mode: true
> +
> +  snps,reset-gpio: true
> +
> +  snps,tso:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      Enables the TSO feature otherwise it will be managed by MAC HW capability register.
> +
> +  power-domains: true
> +
> +  resets: true
> +
> +  rx-fifo-depth: true
> +
> +  tx-fifo-depth: true

You do not list all these properties, because you use
unevaluatedProperties. Drop all of these "xxx :true".
> +
> +required:
> +  - compatible
> +  - clocks
> +  - clock-names


Best regards,
Krzysztof

Re: [PATCH 2/4] dt-bindings: net: snps,dwmac: Add Qualcomm Ethernet ETHQOS compatibles

[Krzysztof Kozlowski]

On 07/09/2022 22:49, Bhupesh Sharma wrote:
> Add Qualcomm Ethernet ETHQOS compatible checks
> in snps,dwmac YAML binding document.
> 
> Cc: Bjorn Andersson <andersson@kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
>  Documentation/devicetree/bindings/net/snps,dwmac.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 2d4e7c7c230a..2b6023ce3ac1 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -65,6 +65,8 @@ properties:
>          - ingenic,x2000-mac
>          - loongson,ls2k-dwmac
>          - loongson,ls7a-dwmac
> +        - qcom,qcs404-ethqos
> +        - qcom,sm8150-ethqos

This must be squashed with previous one, otherwise patchset is not
bisectable.

Best regards,
Krzysztof

Re: [PATCH 3/4] dt-bindings: net: snps,dwmac: Update reg maxitems

[Krzysztof Kozlowski]

On 07/09/2022 22:49, Bhupesh Sharma wrote:
> Since the Qualcomm dwmac based ETHQOS ethernet block
> supports 64-bit register addresses, update the
> reg maxitems inside snps,dwmac YAML bindings.

Please wrap commit message according to Linux coding style / submission
process:
https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586

> 
> Cc: Bjorn Andersson <andersson@kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
>  Documentation/devicetree/bindings/net/snps,dwmac.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 2b6023ce3ac1..f89ca308d55f 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -94,7 +94,7 @@ properties:
>  
>    reg:
>      minItems: 1
> -    maxItems: 2
> +    maxItems: 4

Qualcomm ETHQOS schema allows only 2 in reg-names, so this does not make
sense for Qualcomm and there are no users of 4 items.

Best regards,
Krzysztof

Re: [PATCH 4/4] dt-bindings: net: snps,dwmac: Update interrupt-names

[Krzysztof Kozlowski]

On 07/09/2022 22:49, Bhupesh Sharma wrote:
> As commit fc191af1bb0d ("net: stmmac: platform: Fix misleading
> interrupt error msg") noted, not every stmmac based platform
> makes use of the 'eth_wake_irq' or 'eth_lpi' interrupts.
> 
> So, update the 'interrupt-names' inside 'snps,dwmac' YAML
> bindings to reflect the same.
> 
> Cc: Bjorn Andersson <andersson@kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
>  Documentation/devicetree/bindings/net/snps,dwmac.yaml | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index f89ca308d55f..4d7fe4ee3d87 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -105,10 +105,12 @@ properties:
>  
>    interrupt-names:
>      minItems: 1
> -    items:
> -      - const: macirq
> -      - const: eth_wake_irq
> -      - const: eth_lpi
> +    maxItems: 3
> +    contains:
> +      enum:
> +        - macirq
> +        - eth_wake_irq
> +        - eth_lpi
>  

This gives quite a flexibility, e.g. missing macirq. Instead should be
probably a list with enums:
items:
  - const: macirq
  - enum: [eth_wake_irq, eth_lpi]
  - enum: [eth_wake_irq, eth_lpi]


Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: net: qcom,ethqos: Convert bindings to yaml

[Bhupesh Sharma]

Hi Krzysztof,

Thanks for your comments.

On 9/8/22 8:08 PM, Krzysztof Kozlowski wrote:
> On 07/09/2022 22:49, Bhupesh Sharma wrote:
>> Convert Qualcomm ETHQOS Ethernet devicetree binding to YAML.
>>
>> Cc: Bjorn Andersson <andersson@kernel.org>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Vinod Koul <vkoul@kernel.org>
>> Cc: David Miller <davem@davemloft.net>
>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> 
> Thank you for your patch. There is something to discuss/improve.
> 
>> ---
>>   .../devicetree/bindings/net/qcom,ethqos.txt   |  66 ---------
>>   .../devicetree/bindings/net/qcom,ethqos.yaml  | 139 ++++++++++++++++++
> 
> You need to update maintainers - old path.

Sure, my bad. Will do in v2.

>>   2 files changed, 139 insertions(+), 66 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.txt
>>   create mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/net/qcom,ethqos.txt b/Documentation/devicetree/bindings/net/qcom,ethqos.txt
>> deleted file mode 100644
>> index 1f5746849a71..000000000000
>> --- a/Documentation/devicetree/bindings/net/qcom,ethqos.txt
>> +++ /dev/null
>> @@ -1,66 +0,0 @@
>> -Qualcomm Ethernet ETHQOS device
>> -
>> -This documents dwmmac based ethernet device which supports Gigabit
>> -ethernet for version v2.3.0 onwards.
>> -
>> -This device has following properties:
>> -
>> -Required properties:
>> -
>> -- compatible: Should be one of:
>> -		"qcom,qcs404-ethqos"
>> -		"qcom,sm8150-ethqos"
>> -
>> -- reg: Address and length of the register set for the device
>> -
>> -- reg-names: Should contain register names "stmmaceth", "rgmii"
>> -
>> -- clocks: Should contain phandle to clocks
>> -
>> -- clock-names: Should contain clock names "stmmaceth", "pclk",
>> -		"ptp_ref", "rgmii"
>> -
>> -- interrupts: Should contain phandle to interrupts
>> -
>> -- interrupt-names: Should contain interrupt names "macirq", "eth_lpi"
>> -
>> -Rest of the properties are defined in stmmac.txt file in same directory
>> -
>> -
>> -Example:
>> -
>> -ethernet: ethernet@7a80000 {
>> -	compatible = "qcom,qcs404-ethqos";
>> -	reg = <0x07a80000 0x10000>,
>> -		<0x07a96000 0x100>;
>> -	reg-names = "stmmaceth", "rgmii";
>> -	clock-names = "stmmaceth", "pclk", "ptp_ref", "rgmii";
>> -	clocks = <&gcc GCC_ETH_AXI_CLK>,
>> -		<&gcc GCC_ETH_SLAVE_AHB_CLK>,
>> -		<&gcc GCC_ETH_PTP_CLK>,
>> -		<&gcc GCC_ETH_RGMII_CLK>;
>> -	interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
>> -			<GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
>> -	interrupt-names = "macirq", "eth_lpi";
>> -	snps,reset-gpio = <&tlmm 60 GPIO_ACTIVE_LOW>;
>> -	snps,reset-active-low;
>> -
>> -	snps,txpbl = <8>;
>> -	snps,rxpbl = <2>;
>> -	snps,aal;
>> -	snps,tso;
>> -
>> -	phy-handle = <&phy1>;
>> -	phy-mode = "rgmii";
>> -
>> -	mdio {
>> -		#address-cells = <0x1>;
>> -		#size-cells = <0x0>;
>> -		compatible = "snps,dwmac-mdio";
>> -		phy1: phy@4 {
>> -			device_type = "ethernet-phy";
>> -			reg = <0x4>;
>> -		};
>> -	};
>> -
>> -};
>> diff --git a/Documentation/devicetree/bindings/net/qcom,ethqos.yaml b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
>> new file mode 100644
>> index 000000000000..f05df9b0d106
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
>> @@ -0,0 +1,139 @@
>> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/qcom,ethqos.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Ethernet ETHQOS device
>> +
>> +maintainers:
>> +  - Bhupesh Sharma <bhupesh.sharma@linaro.org>
>> +
>> +description:
>> +  This binding describes the dwmmac based Qualcomm ethernet devices which
>> +  support Gigabit ethernet (version v2.3.0 onwards).
>> +
>> +  So, this file documents platform glue layer for dwmmac stmmac based Qualcomm
>> +  ethernet devices.
>> +
>> +allOf:
>> +  - $ref: "snps,dwmac.yaml#"
> 
> No need for quotes.

Ok.

>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,qcs404-ethqos
>> +      - qcom,sm8150-ethqos
>> +
>> +  reg: true
> 
> I think both devices use two reg spaces.

On this platform the two reg spaces are 64-bit, whereas for other
platforms based on dwmmac, for e.g. stm32 have 32-bit address space.

>> +
>> +  reg-names:
>> +    minItems: 1
> 
> Why allowing only one item?

Ok, let me remove this in v2.

>> +    items:
>> +      - const: stmmaceth
>> +      - const: rgmii
>> +
>> +  interrupts: true
> 
> This should be specific/fixed.
> 
>> +
>> +  interrupt-names: true
> 
> This should be specific/fixed.

These are same as in $ref: "snps,dwmac.yaml#", so
do we really need to specify them here? I remember on the sdhci-msm
YAML patch review, Rob mentioned that we should just set the property to 
true, in such cases.

Am I missing something here?

>> +
>> +  clocks:
>> +    minItems: 1
>> +    maxItems: 4
> 
> Why such flexibility?

Ok, let me just keep 'maxItems: 4' here for now.

>> +
>> +  clock-names:
>> +    minItems: 1
>> +    items:
>> +      - const: stmmaceth
>> +      - const: pclk
>> +      - const: ptp_ref
>> +      - const: rgmii
>> +
>> +  iommus:
>> +    minItems: 1
>> +    maxItems: 2
> 
> Aren't we using only one MMU?

It was just for future compatibility, but I get your point.
Let me keep the 'maxItems: 1' here for now.

>> +
>> +  mdio: true
>> +
>> +  phy-handle: true
>> +
>> +  phy-mode: true
>> +
>> +  snps,reset-gpio: true
>> +
>> +  snps,tso:
>> +    $ref: /schemas/types.yaml#/definitions/flag
>> +    description:
>> +      Enables the TSO feature otherwise it will be managed by MAC HW capability register.
>> +
>> +  power-domains: true
>> +
>> +  resets: true
>> +
>> +  rx-fifo-depth: true
>> +
>> +  tx-fifo-depth: true
> 
> You do not list all these properties, because you use
> unevaluatedProperties. Drop all of these "xxx :true".

Same query as above. May be I am missing something here.

>> +
>> +required:
>> +  - compatible
>> +  - clocks
>> +  - clock-names

Thanks,
Bhupesh

Re: [PATCH 2/4] dt-bindings: net: snps,dwmac: Add Qualcomm Ethernet ETHQOS compatibles

[Bhupesh Sharma]

On 9/8/22 8:09 PM, Krzysztof Kozlowski wrote:
> On 07/09/2022 22:49, Bhupesh Sharma wrote:
>> Add Qualcomm Ethernet ETHQOS compatible checks
>> in snps,dwmac YAML binding document.
>>
>> Cc: Bjorn Andersson <andersson@kernel.org>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Vinod Koul <vkoul@kernel.org>
>> Cc: David Miller <davem@davemloft.net>
>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>> ---
>>   Documentation/devicetree/bindings/net/snps,dwmac.yaml | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> index 2d4e7c7c230a..2b6023ce3ac1 100644
>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> @@ -65,6 +65,8 @@ properties:
>>           - ingenic,x2000-mac
>>           - loongson,ls2k-dwmac
>>           - loongson,ls7a-dwmac
>> +        - qcom,qcs404-ethqos
>> +        - qcom,sm8150-ethqos
> 
> This must be squashed with previous one, otherwise patchset is not
> bisectable.

Ok, will fix in v2.

Thanks.

Re: [PATCH 4/4] dt-bindings: net: snps,dwmac: Update interrupt-names

[Bhupesh Sharma]

On 9/8/22 8:13 PM, Krzysztof Kozlowski wrote:
> On 07/09/2022 22:49, Bhupesh Sharma wrote:
>> As commit fc191af1bb0d ("net: stmmac: platform: Fix misleading
>> interrupt error msg") noted, not every stmmac based platform
>> makes use of the 'eth_wake_irq' or 'eth_lpi' interrupts.
>>
>> So, update the 'interrupt-names' inside 'snps,dwmac' YAML
>> bindings to reflect the same.
>>
>> Cc: Bjorn Andersson <andersson@kernel.org>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Vinod Koul <vkoul@kernel.org>
>> Cc: David Miller <davem@davemloft.net>
>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>> ---
>>   Documentation/devicetree/bindings/net/snps,dwmac.yaml | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> index f89ca308d55f..4d7fe4ee3d87 100644
>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> @@ -105,10 +105,12 @@ properties:
>>   
>>     interrupt-names:
>>       minItems: 1
>> -    items:
>> -      - const: macirq
>> -      - const: eth_wake_irq
>> -      - const: eth_lpi
>> +    maxItems: 3
>> +    contains:
>> +      enum:
>> +        - macirq
>> +        - eth_wake_irq
>> +        - eth_lpi
>>   
> 
> This gives quite a flexibility, e.g. missing macirq. Instead should be
> probably a list with enums:
> items:
>    - const: macirq
>    - enum: [eth_wake_irq, eth_lpi]
>    - enum: [eth_wake_irq, eth_lpi]

Ok, will fix in v2.

Thanks.

Re: [PATCH 3/4] dt-bindings: net: snps,dwmac: Update reg maxitems

[Bhupesh Sharma]

On 9/8/22 8:11 PM, Krzysztof Kozlowski wrote:
> On 07/09/2022 22:49, Bhupesh Sharma wrote:
>> Since the Qualcomm dwmac based ETHQOS ethernet block
>> supports 64-bit register addresses, update the
>> reg maxitems inside snps,dwmac YAML bindings.
> 
> Please wrap commit message according to Linux coding style / submission
> process:
> https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586
> 
>>
>> Cc: Bjorn Andersson <andersson@kernel.org>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Vinod Koul <vkoul@kernel.org>
>> Cc: David Miller <davem@davemloft.net>
>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>> ---
>>   Documentation/devicetree/bindings/net/snps,dwmac.yaml | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> index 2b6023ce3ac1..f89ca308d55f 100644
>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> @@ -94,7 +94,7 @@ properties:
>>   
>>     reg:
>>       minItems: 1
>> -    maxItems: 2
>> +    maxItems: 4
> 
> Qualcomm ETHQOS schema allows only 2 in reg-names, so this does not make
> sense for Qualcomm and there are no users of 4 items.

On this platform the two reg spaces are 64-bit, whereas for other
platforms based on dwmmac, for e.g. stm32 have 32-bit address space.

Without this fix I was getting the following error with 'make dtbs_check':

Documentation/devicetree/bindings/net/qcom,ethqos.example.dtb: 
ethernet@20000: reg: [[0, 131072], [0, 65536], [0, 221184], [0, 256]] is 
too long
	From schema: 
/home/bhsharma/code/upstream/linux-bckup/linux/Documentation/devicetree/bindings/net/snps,dwmac.yaml

Thanks.

Re: [PATCH 3/4] dt-bindings: net: snps,dwmac: Update reg maxitems

[Rob Herring]

On Tue, Sep 13, 2022 at 12:23:42AM +0530, Bhupesh Sharma wrote:
> On 9/8/22 8:11 PM, Krzysztof Kozlowski wrote:
> > On 07/09/2022 22:49, Bhupesh Sharma wrote:
> > > Since the Qualcomm dwmac based ETHQOS ethernet block
> > > supports 64-bit register addresses, update the
> > > reg maxitems inside snps,dwmac YAML bindings.
> > 
> > Please wrap commit message according to Linux coding style / submission
> > process:
> > https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586
> > 
> > > 
> > > Cc: Bjorn Andersson <andersson@kernel.org>
> > > Cc: Rob Herring <robh@kernel.org>
> > > Cc: Vinod Koul <vkoul@kernel.org>
> > > Cc: David Miller <davem@davemloft.net>
> > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> > > ---
> > >   Documentation/devicetree/bindings/net/snps,dwmac.yaml | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > index 2b6023ce3ac1..f89ca308d55f 100644
> > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > @@ -94,7 +94,7 @@ properties:
> > >     reg:
> > >       minItems: 1
> > > -    maxItems: 2
> > > +    maxItems: 4
> > 
> > Qualcomm ETHQOS schema allows only 2 in reg-names, so this does not make
> > sense for Qualcomm and there are no users of 4 items.
> 
> On this platform the two reg spaces are 64-bit, whereas for other
> platforms based on dwmmac, for e.g. stm32 have 32-bit address space.

The schema for reg is how many addr/size entries regardless of cell 
sizes.

> Without this fix I was getting the following error with 'make dtbs_check':
> 
> Documentation/devicetree/bindings/net/qcom,ethqos.example.dtb:
> ethernet@20000: reg: [[0, 131072], [0, 65536], [0, 221184], [0, 256]] is too
> long
> 	From schema: /home/bhsharma/code/upstream/linux-bckup/linux/Documentation/devicetree/bindings/net/snps,dwmac.yaml

The default cell sizes for examples is 1 for addr/size. If you want it 
to be 2, you have to write your own parent node. But why? It's just an 
example. Use 1 cell like the example originally had.

Rob

Re: [PATCH 1/4] dt-bindings: net: qcom,ethqos: Convert bindings to yaml

[Krzysztof Kozlowski]

On 12/09/2022 19:28, Bhupesh Sharma wrote:
> Hi Krzysztof,
> 
> Thanks for your comments.
> 
> On 9/8/22 8:08 PM, Krzysztof Kozlowski wrote:
>> On 07/09/2022 22:49, Bhupesh Sharma wrote:
>>> Convert Qualcomm ETHQOS Ethernet devicetree binding to YAML.
>>>
>>> Cc: Bjorn Andersson <andersson@kernel.org>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: Vinod Koul <vkoul@kernel.org>
>>> Cc: David Miller <davem@davemloft.net>
>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>>
>> Thank you for your patch. There is something to discuss/improve.
>>
>>> ---
>>>   .../devicetree/bindings/net/qcom,ethqos.txt   |  66 ---------
>>>   .../devicetree/bindings/net/qcom,ethqos.yaml  | 139 ++++++++++++++++++
>>
>> You need to update maintainers - old path.
> 
> Sure, my bad. Will do in v2.
> 
>>>   2 files changed, 139 insertions(+), 66 deletions(-)
>>>   delete mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.txt
>>>   create mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/qcom,ethqos.txt b/Documentation/devicetree/bindings/net/qcom,ethqos.txt
>>> deleted file mode 100644
>>> index 1f5746849a71..000000000000
>>> --- a/Documentation/devicetree/bindings/net/qcom,ethqos.txt
>>> +++ /dev/null
>>> @@ -1,66 +0,0 @@
>>> -Qualcomm Ethernet ETHQOS device
>>> -
>>> -This documents dwmmac based ethernet device which supports Gigabit
>>> -ethernet for version v2.3.0 onwards.
>>> -
>>> -This device has following properties:
>>> -
>>> -Required properties:
>>> -
>>> -- compatible: Should be one of:
>>> -		"qcom,qcs404-ethqos"
>>> -		"qcom,sm8150-ethqos"
>>> -
>>> -- reg: Address and length of the register set for the device
>>> -
>>> -- reg-names: Should contain register names "stmmaceth", "rgmii"
>>> -
>>> -- clocks: Should contain phandle to clocks
>>> -
>>> -- clock-names: Should contain clock names "stmmaceth", "pclk",
>>> -		"ptp_ref", "rgmii"
>>> -
>>> -- interrupts: Should contain phandle to interrupts
>>> -
>>> -- interrupt-names: Should contain interrupt names "macirq", "eth_lpi"
>>> -
>>> -Rest of the properties are defined in stmmac.txt file in same directory
>>> -
>>> -
>>> -Example:
>>> -
>>> -ethernet: ethernet@7a80000 {
>>> -	compatible = "qcom,qcs404-ethqos";
>>> -	reg = <0x07a80000 0x10000>,
>>> -		<0x07a96000 0x100>;
>>> -	reg-names = "stmmaceth", "rgmii";
>>> -	clock-names = "stmmaceth", "pclk", "ptp_ref", "rgmii";
>>> -	clocks = <&gcc GCC_ETH_AXI_CLK>,
>>> -		<&gcc GCC_ETH_SLAVE_AHB_CLK>,
>>> -		<&gcc GCC_ETH_PTP_CLK>,
>>> -		<&gcc GCC_ETH_RGMII_CLK>;
>>> -	interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
>>> -			<GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
>>> -	interrupt-names = "macirq", "eth_lpi";
>>> -	snps,reset-gpio = <&tlmm 60 GPIO_ACTIVE_LOW>;
>>> -	snps,reset-active-low;
>>> -
>>> -	snps,txpbl = <8>;
>>> -	snps,rxpbl = <2>;
>>> -	snps,aal;
>>> -	snps,tso;
>>> -
>>> -	phy-handle = <&phy1>;
>>> -	phy-mode = "rgmii";
>>> -
>>> -	mdio {
>>> -		#address-cells = <0x1>;
>>> -		#size-cells = <0x0>;
>>> -		compatible = "snps,dwmac-mdio";
>>> -		phy1: phy@4 {
>>> -			device_type = "ethernet-phy";
>>> -			reg = <0x4>;
>>> -		};
>>> -	};
>>> -
>>> -};
>>> diff --git a/Documentation/devicetree/bindings/net/qcom,ethqos.yaml b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
>>> new file mode 100644
>>> index 000000000000..f05df9b0d106
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
>>> @@ -0,0 +1,139 @@
>>> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/net/qcom,ethqos.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Qualcomm Ethernet ETHQOS device
>>> +
>>> +maintainers:
>>> +  - Bhupesh Sharma <bhupesh.sharma@linaro.org>
>>> +
>>> +description:
>>> +  This binding describes the dwmmac based Qualcomm ethernet devices which
>>> +  support Gigabit ethernet (version v2.3.0 onwards).
>>> +
>>> +  So, this file documents platform glue layer for dwmmac stmmac based Qualcomm
>>> +  ethernet devices.
>>> +
>>> +allOf:
>>> +  - $ref: "snps,dwmac.yaml#"
>>
>> No need for quotes.
> 
> Ok.
> 
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - qcom,qcs404-ethqos
>>> +      - qcom,sm8150-ethqos
>>> +
>>> +  reg: true
>>
>> I think both devices use two reg spaces.
> 
> On this platform the two reg spaces are 64-bit, whereas for other
> platforms based on dwmmac, for e.g. stm32 have 32-bit address space.

Then for this platform this should be made specific/constrained, so it
must be two items.

> 
>>> +
>>> +  reg-names:
>>> +    minItems: 1
>>
>> Why allowing only one item?
> 
> Ok, let me remove this in v2.

And then as well you allow only one item... This should be specific. If
not - why?

> 
>>> +    items:
>>> +      - const: stmmaceth
>>> +      - const: rgmii
>>> +
>>> +  interrupts: true
>>
>> This should be specific/fixed.
>>
>>> +
>>> +  interrupt-names: true
>>
>> This should be specific/fixed.
> 
> These are same as in $ref: "snps,dwmac.yaml#", so
> do we really need to specify them here? I remember on the sdhci-msm
> YAML patch review, Rob mentioned that we should just set the property to 
> true, in such cases.

But it is not specific in dwmac.yaml. You use "xxx: true" when you want
to accept property from other schema, assuming it is defined there
properly. However the snps,dwmac does not define it in specific way
because it expects specific implementation to narrow the details.

> 
> Am I missing something here?
> 
>>> +
>>> +  clocks:
>>> +    minItems: 1
>>> +    maxItems: 4
>>
>> Why such flexibility?
> 
> Ok, let me just keep 'maxItems: 4' here for now.
> 
>>> +
>>> +  clock-names:
>>> +    minItems: 1
>>> +    items:
>>> +      - const: stmmaceth
>>> +      - const: pclk
>>> +      - const: ptp_ref
>>> +      - const: rgmii
>>> +
>>> +  iommus:
>>> +    minItems: 1
>>> +    maxItems: 2
>>
>> Aren't we using only one MMU?
> 
> It was just for future compatibility, but I get your point.
> Let me keep the 'maxItems: 1' here for now.
> 
>>> +
>>> +  mdio: true
>>> +
>>> +  phy-handle: true
>>> +
>>> +  phy-mode: true
>>> +
>>> +  snps,reset-gpio: true
>>> +
>>> +  snps,tso:
>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>> +    description:
>>> +      Enables the TSO feature otherwise it will be managed by MAC HW capability register.
>>> +
>>> +  power-domains: true
>>> +
>>> +  resets: true
>>> +
>>> +  rx-fifo-depth: true
>>> +
>>> +  tx-fifo-depth: true
>>
>> You do not list all these properties, because you use
>> unevaluatedProperties. Drop all of these "xxx :true".
> 
> Same query as above. May be I am missing something here.

You do not list any properties:true from other schema, if you use
unevaluatedProperties:false.


Best regards,
Krzysztof

Re: [PATCH 3/4] dt-bindings: net: snps,dwmac: Update reg maxitems

[Bhupesh Sharma]

Hi Rob,

On 9/13/22 2:44 AM, Rob Herring wrote:
> On Tue, Sep 13, 2022 at 12:23:42AM +0530, Bhupesh Sharma wrote:
>> On 9/8/22 8:11 PM, Krzysztof Kozlowski wrote:
>>> On 07/09/2022 22:49, Bhupesh Sharma wrote:
>>>> Since the Qualcomm dwmac based ETHQOS ethernet block
>>>> supports 64-bit register addresses, update the
>>>> reg maxitems inside snps,dwmac YAML bindings.
>>>
>>> Please wrap commit message according to Linux coding style / submission
>>> process:
>>> https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586
>>>
>>>>
>>>> Cc: Bjorn Andersson <andersson@kernel.org>
>>>> Cc: Rob Herring <robh@kernel.org>
>>>> Cc: Vinod Koul <vkoul@kernel.org>
>>>> Cc: David Miller <davem@davemloft.net>
>>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>>>> ---
>>>>    Documentation/devicetree/bindings/net/snps,dwmac.yaml | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>> index 2b6023ce3ac1..f89ca308d55f 100644
>>>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>> @@ -94,7 +94,7 @@ properties:
>>>>      reg:
>>>>        minItems: 1
>>>> -    maxItems: 2
>>>> +    maxItems: 4
>>>
>>> Qualcomm ETHQOS schema allows only 2 in reg-names, so this does not make
>>> sense for Qualcomm and there are no users of 4 items.
>>
>> On this platform the two reg spaces are 64-bit, whereas for other
>> platforms based on dwmmac, for e.g. stm32 have 32-bit address space.
> 
> The schema for reg is how many addr/size entries regardless of cell
> sizes.
> 
>> Without this fix I was getting the following error with 'make dtbs_check':
>>
>> Documentation/devicetree/bindings/net/qcom,ethqos.example.dtb:
>> ethernet@20000: reg: [[0, 131072], [0, 65536], [0, 221184], [0, 256]] is too
>> long
>> 	From schema: /home/bhsharma/code/upstream/linux-bckup/linux/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> 
> The default cell sizes for examples is 1 for addr/size. If you want it
> to be 2, you have to write your own parent node. But why? It's just an
> example. Use 1 cell like the example originally had.

Got your point. Let me revert to the original example in v2.

Thanks,
Bhupesh

Re: [PATCH 1/4] dt-bindings: net: qcom,ethqos: Convert bindings to yaml

[Bhupesh Sharma]

On 9/13/22 2:20 PM, Krzysztof Kozlowski wrote:
> On 12/09/2022 19:28, Bhupesh Sharma wrote:
>> Hi Krzysztof,
>>
>> Thanks for your comments.
>>
>> On 9/8/22 8:08 PM, Krzysztof Kozlowski wrote:
>>> On 07/09/2022 22:49, Bhupesh Sharma wrote:
>>>> Convert Qualcomm ETHQOS Ethernet devicetree binding to YAML.
>>>>
>>>> Cc: Bjorn Andersson <andersson@kernel.org>
>>>> Cc: Rob Herring <robh@kernel.org>
>>>> Cc: Vinod Koul <vkoul@kernel.org>
>>>> Cc: David Miller <davem@davemloft.net>
>>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>>>
>>> Thank you for your patch. There is something to discuss/improve.
>>>
>>>> ---
>>>>    .../devicetree/bindings/net/qcom,ethqos.txt   |  66 ---------
>>>>    .../devicetree/bindings/net/qcom,ethqos.yaml  | 139 ++++++++++++++++++
>>>
>>> You need to update maintainers - old path.
>>
>> Sure, my bad. Will do in v2.
>>
>>>>    2 files changed, 139 insertions(+), 66 deletions(-)
>>>>    delete mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.txt
>>>>    create mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/qcom,ethqos.txt b/Documentation/devicetree/bindings/net/qcom,ethqos.txt
>>>> deleted file mode 100644
>>>> index 1f5746849a71..000000000000
>>>> --- a/Documentation/devicetree/bindings/net/qcom,ethqos.txt
>>>> +++ /dev/null
>>>> @@ -1,66 +0,0 @@
>>>> -Qualcomm Ethernet ETHQOS device
>>>> -
>>>> -This documents dwmmac based ethernet device which supports Gigabit
>>>> -ethernet for version v2.3.0 onwards.
>>>> -
>>>> -This device has following properties:
>>>> -
>>>> -Required properties:
>>>> -
>>>> -- compatible: Should be one of:
>>>> -		"qcom,qcs404-ethqos"
>>>> -		"qcom,sm8150-ethqos"
>>>> -
>>>> -- reg: Address and length of the register set for the device
>>>> -
>>>> -- reg-names: Should contain register names "stmmaceth", "rgmii"
>>>> -
>>>> -- clocks: Should contain phandle to clocks
>>>> -
>>>> -- clock-names: Should contain clock names "stmmaceth", "pclk",
>>>> -		"ptp_ref", "rgmii"
>>>> -
>>>> -- interrupts: Should contain phandle to interrupts
>>>> -
>>>> -- interrupt-names: Should contain interrupt names "macirq", "eth_lpi"
>>>> -
>>>> -Rest of the properties are defined in stmmac.txt file in same directory
>>>> -
>>>> -
>>>> -Example:
>>>> -
>>>> -ethernet: ethernet@7a80000 {
>>>> -	compatible = "qcom,qcs404-ethqos";
>>>> -	reg = <0x07a80000 0x10000>,
>>>> -		<0x07a96000 0x100>;
>>>> -	reg-names = "stmmaceth", "rgmii";
>>>> -	clock-names = "stmmaceth", "pclk", "ptp_ref", "rgmii";
>>>> -	clocks = <&gcc GCC_ETH_AXI_CLK>,
>>>> -		<&gcc GCC_ETH_SLAVE_AHB_CLK>,
>>>> -		<&gcc GCC_ETH_PTP_CLK>,
>>>> -		<&gcc GCC_ETH_RGMII_CLK>;
>>>> -	interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
>>>> -			<GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
>>>> -	interrupt-names = "macirq", "eth_lpi";
>>>> -	snps,reset-gpio = <&tlmm 60 GPIO_ACTIVE_LOW>;
>>>> -	snps,reset-active-low;
>>>> -
>>>> -	snps,txpbl = <8>;
>>>> -	snps,rxpbl = <2>;
>>>> -	snps,aal;
>>>> -	snps,tso;
>>>> -
>>>> -	phy-handle = <&phy1>;
>>>> -	phy-mode = "rgmii";
>>>> -
>>>> -	mdio {
>>>> -		#address-cells = <0x1>;
>>>> -		#size-cells = <0x0>;
>>>> -		compatible = "snps,dwmac-mdio";
>>>> -		phy1: phy@4 {
>>>> -			device_type = "ethernet-phy";
>>>> -			reg = <0x4>;
>>>> -		};
>>>> -	};
>>>> -
>>>> -};
>>>> diff --git a/Documentation/devicetree/bindings/net/qcom,ethqos.yaml b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
>>>> new file mode 100644
>>>> index 000000000000..f05df9b0d106
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
>>>> @@ -0,0 +1,139 @@
>>>> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/net/qcom,ethqos.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Qualcomm Ethernet ETHQOS device
>>>> +
>>>> +maintainers:
>>>> +  - Bhupesh Sharma <bhupesh.sharma@linaro.org>
>>>> +
>>>> +description:
>>>> +  This binding describes the dwmmac based Qualcomm ethernet devices which
>>>> +  support Gigabit ethernet (version v2.3.0 onwards).
>>>> +
>>>> +  So, this file documents platform glue layer for dwmmac stmmac based Qualcomm
>>>> +  ethernet devices.
>>>> +
>>>> +allOf:
>>>> +  - $ref: "snps,dwmac.yaml#"
>>>
>>> No need for quotes.
>>
>> Ok.
>>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - qcom,qcs404-ethqos
>>>> +      - qcom,sm8150-ethqos
>>>> +
>>>> +  reg: true
>>>
>>> I think both devices use two reg spaces.
>>
>> On this platform the two reg spaces are 64-bit, whereas for other
>> platforms based on dwmmac, for e.g. stm32 have 32-bit address space.
> 
> Then for this platform this should be made specific/constrained, so it
> must be two items.
> 
>>
>>>> +
>>>> +  reg-names:
>>>> +    minItems: 1
>>>
>>> Why allowing only one item?
>>
>> Ok, let me remove this in v2.
> 
> And then as well you allow only one item... This should be specific. If
> not - why?
> 
>>
>>>> +    items:
>>>> +      - const: stmmaceth
>>>> +      - const: rgmii
>>>> +
>>>> +  interrupts: true
>>>
>>> This should be specific/fixed.
>>>
>>>> +
>>>> +  interrupt-names: true
>>>
>>> This should be specific/fixed.
>>
>> These are same as in $ref: "snps,dwmac.yaml#", so
>> do we really need to specify them here? I remember on the sdhci-msm
>> YAML patch review, Rob mentioned that we should just set the property to
>> true, in such cases.
> 
> But it is not specific in dwmac.yaml. You use "xxx: true" when you want
> to accept property from other schema, assuming it is defined there
> properly. However the snps,dwmac does not define it in specific way
> because it expects specific implementation to narrow the details.
> 
>>
>> Am I missing something here?
>>
>>>> +
>>>> +  clocks:
>>>> +    minItems: 1
>>>> +    maxItems: 4
>>>
>>> Why such flexibility?
>>
>> Ok, let me just keep 'maxItems: 4' here for now.
>>
>>>> +
>>>> +  clock-names:
>>>> +    minItems: 1
>>>> +    items:
>>>> +      - const: stmmaceth
>>>> +      - const: pclk
>>>> +      - const: ptp_ref
>>>> +      - const: rgmii
>>>> +
>>>> +  iommus:
>>>> +    minItems: 1
>>>> +    maxItems: 2
>>>
>>> Aren't we using only one MMU?
>>
>> It was just for future compatibility, but I get your point.
>> Let me keep the 'maxItems: 1' here for now.
>>
>>>> +
>>>> +  mdio: true
>>>> +
>>>> +  phy-handle: true
>>>> +
>>>> +  phy-mode: true
>>>> +
>>>> +  snps,reset-gpio: true
>>>> +
>>>> +  snps,tso:
>>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>>> +    description:
>>>> +      Enables the TSO feature otherwise it will be managed by MAC HW capability register.
>>>> +
>>>> +  power-domains: true
>>>> +
>>>> +  resets: true
>>>> +
>>>> +  rx-fifo-depth: true
>>>> +
>>>> +  tx-fifo-depth: true
>>>
>>> You do not list all these properties, because you use
>>> unevaluatedProperties. Drop all of these "xxx :true".
>>
>> Same query as above. May be I am missing something here.
> 
> You do not list any properties:true from other schema, if you use
> unevaluatedProperties:false.

Ok, let me try and fix these in the v2. I will share the same soon.

Thanks,
Bhupesh