[PATCH 17/20] dt-bindings: usb: qcom,dwc3: Validate DWC3 sub-node

[PATCH 17/20] dt-bindings: usb: qcom,dwc3: Validate DWC3 sub-node

[Serge Semin]

Qualcomm msm8996/sc7180/sdm845 DWC3 compatible DT nodes are supposed to
have a DWC USB3 compatible sub-node to describe a fully functioning USB
interface. Let's use the available DWC USB3 DT schema to validate the
Qualcomm DWC3 sub-nodes.

Note since the generic DWC USB3 DT node is supposed to be named as generic
USB HCD ("^usb(@.*)?") one we have to accordingly fix the sub-nodes name
regexp and fix the DT node example.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>

---

Changelog v2:
- Discard the "^dwc3@[0-9a-f]+$" nodes from being acceptable as sub-nodes.
---
 Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index dac10848dd7f..8f8d781e73a0 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -103,11 +103,8 @@ properties:
 # Required child node:
 
 patternProperties:
-  "^dwc3@[0-9a-f]+$":
-    type: object
-    description:
-      A child node must exist to represent the core DWC3 IP block
-      The content of the node is defined in dwc3.txt.
+  "^usb@[0-9a-f]+$":
+    $ref: snps,dwc3.yaml#
 
 required:
   - compatible
@@ -160,7 +157,7 @@ examples:
 
             resets = <&gcc GCC_USB30_PRIM_BCR>;
 
-            dwc3@a600000 {
+            usb@a600000 {
                 compatible = "snps,dwc3";
                 reg = <0 0x0a600000 0 0xcd00>;
                 interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.27.0

Re: [PATCH 17/20] dt-bindings: usb: qcom,dwc3: Validate DWC3 sub-node

[Rob Herring]

On Wed, 14 Oct 2020 13:13:59 +0300, Serge Semin wrote:
> Qualcomm msm8996/sc7180/sdm845 DWC3 compatible DT nodes are supposed to
> have a DWC USB3 compatible sub-node to describe a fully functioning USB
> interface. Let's use the available DWC USB3 DT schema to validate the
> Qualcomm DWC3 sub-nodes.
> 
> Note since the generic DWC USB3 DT node is supposed to be named as generic
> USB HCD ("^usb(@.*)?") one we have to accordingly fix the sub-nodes name
> regexp and fix the DT node example.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> ---
> 
> Changelog v2:
> - Discard the "^dwc3@[0-9a-f]+$" nodes from being acceptable as sub-nodes.
> ---
>  Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 

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

[PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name

[Serge Semin]

In accordance with the DWC USB3 bindings the corresponding node name is
suppose to comply with Generic USB HCD DT schema, which requires the USB
nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
the dtbs_check procedure failure. Let's fix the nodes naming to be
compatible with the DWC USB3 DT schema to make dtbs_check happy.

Note we don't change the DWC USB3-compatible nodes names of
arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
in-source comment says that the nodes name need to be preserved as
"^dwusb@.*" for some backward compatibility.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>

---

Please, test the patch out to make sure it doesn't brake the dependent DTS
files. I did only a manual grepping of the possible nodes dependencies.
---
 arch/arm/boot/dts/armada-375.dtsi              | 2 +-
 arch/arm/boot/dts/exynos5250.dtsi              | 2 +-
 arch/arm/boot/dts/exynos54xx.dtsi              | 4 ++--
 arch/arm/boot/dts/keystone-k2e.dtsi            | 4 ++--
 arch/arm/boot/dts/keystone.dtsi                | 2 +-
 arch/arm/boot/dts/ls1021a.dtsi                 | 2 +-
 arch/arm/boot/dts/omap5-l4.dtsi                | 2 +-
 arch/arm/boot/dts/stih407-family.dtsi          | 2 +-
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi   | 2 +-
 arch/arm64/boot/dts/exynos/exynos5433.dtsi     | 4 ++--
 arch/arm64/boot/dts/exynos/exynos7.dtsi        | 2 +-
 arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 4 ++--
 arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 +++---
 arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 ++--
 arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 ++--
 arch/arm64/boot/dts/hisilicon/hi3660.dtsi      | 2 +-
 arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi   | 4 ++--
 arch/arm64/boot/dts/qcom/ipq8074.dtsi          | 4 ++--
 arch/arm64/boot/dts/qcom/msm8996.dtsi          | 4 ++--
 arch/arm64/boot/dts/qcom/msm8998.dtsi          | 2 +-
 arch/arm64/boot/dts/qcom/qcs404-evb.dtsi       | 2 +-
 arch/arm64/boot/dts/qcom/qcs404.dtsi           | 4 ++--
 arch/arm64/boot/dts/qcom/sc7180.dtsi           | 2 +-
 arch/arm64/boot/dts/qcom/sdm845.dtsi           | 4 ++--
 arch/arm64/boot/dts/qcom/sm8150.dtsi           | 2 +-
 25 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/arch/arm/boot/dts/armada-375.dtsi b/arch/arm/boot/dts/armada-375.dtsi
index 9805e507c695..7f2f24a29e6c 100644
--- a/arch/arm/boot/dts/armada-375.dtsi
+++ b/arch/arm/boot/dts/armada-375.dtsi
@@ -426,7 +426,7 @@ usb1: usb@54000 {
 				status = "disabled";
 			};
 
-			usb2: usb3@58000 {
+			usb2: usb@58000 {
 				compatible = "marvell,armada-375-xhci";
 				reg = <0x58000 0x20000>,<0x5b880 0x80>;
 				interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index e3dbe4166836..ebcf8b40ea81 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -629,7 +629,7 @@ usb_dwc3 {
 			#size-cells = <1>;
 			ranges;
 
-			usbdrd_dwc3: dwc3@12000000 {
+			usbdrd_dwc3: usb@12000000 {
 				compatible = "synopsys,dwc3";
 				reg = <0x12000000 0x10000>;
 				interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm/boot/dts/exynos54xx.dtsi b/arch/arm/boot/dts/exynos54xx.dtsi
index 8aa5117e58ce..339243d19a80 100644
--- a/arch/arm/boot/dts/exynos54xx.dtsi
+++ b/arch/arm/boot/dts/exynos54xx.dtsi
@@ -148,7 +148,7 @@ usbdrd3_0: usb3-0 {
 			#size-cells = <1>;
 			ranges;
 
-			usbdrd_dwc3_0: dwc3@12000000 {
+			usbdrd_dwc3_0: usb@12000000 {
 				compatible = "snps,dwc3";
 				reg = <0x12000000 0x10000>;
 				interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
@@ -170,7 +170,7 @@ usbdrd3_1: usb3-1 {
 			#size-cells = <1>;
 			ranges;
 
-			usbdrd_dwc3_1: dwc3@12400000 {
+			usbdrd_dwc3_1: usb@12400000 {
 				compatible = "snps,dwc3";
 				reg = <0x12400000 0x10000>;
 				phys = <&usbdrd_phy1 0>, <&usbdrd_phy1 1>;
diff --git a/arch/arm/boot/dts/keystone-k2e.dtsi b/arch/arm/boot/dts/keystone-k2e.dtsi
index 2d94faf31fab..d625ad10cfad 100644
--- a/arch/arm/boot/dts/keystone-k2e.dtsi
+++ b/arch/arm/boot/dts/keystone-k2e.dtsi
@@ -52,7 +52,7 @@ &soc0 {
 
 		usb: usb@2680000 {
 			interrupts = <GIC_SPI 152 IRQ_TYPE_EDGE_RISING>;
-			dwc3@2690000 {
+			usb@2690000 {
 				interrupts = <GIC_SPI 152 IRQ_TYPE_EDGE_RISING>;
 			};
 		};
@@ -78,7 +78,7 @@ keystone_usb1: usb@25000000 {
 			dma-ranges;
 			status = "disabled";
 
-			usb1: dwc3@25010000 {
+			usb1: usb@25010000 {
 				compatible = "synopsys,dwc3";
 				reg = <0x25010000 0x70000>;
 				interrupts = <GIC_SPI 414 IRQ_TYPE_EDGE_RISING>;
diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index c298675a29a5..6f9748349f09 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -217,7 +217,7 @@ keystone_usb0: usb@2680000 {
 			dma-ranges;
 			status = "disabled";
 
-			usb0: dwc3@2690000 {
+			usb0: usb@2690000 {
 				compatible = "synopsys,dwc3";
 				reg = <0x2690000 0x70000>;
 				interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index 827373ef1a54..5c4104d301f1 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -874,7 +874,7 @@ usb2: usb@8600000 {
 			phy_type = "ulpi";
 		};
 
-		usb3: usb3@3100000 {
+		usb3: usb@3100000 {
 			compatible = "snps,dwc3";
 			reg = <0x0 0x3100000 0x0 0x10000>;
 			interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm/boot/dts/omap5-l4.dtsi b/arch/arm/boot/dts/omap5-l4.dtsi
index f3d3a16b7c64..887b3359dd5a 100644
--- a/arch/arm/boot/dts/omap5-l4.dtsi
+++ b/arch/arm/boot/dts/omap5-l4.dtsi
@@ -194,7 +194,7 @@ usb3: omap_dwc3@0 {
 				#size-cells = <1>;
 				utmi-mode = <2>;
 				ranges = <0 0 0x20000>;
-				dwc3: dwc3@10000 {
+				dwc3: usb@10000 {
 					compatible = "snps,dwc3";
 					reg = <0x10000 0x10000>;
 					interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>,
diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index 23a1746f3baa..2352f76b5a69 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -681,7 +681,7 @@ st_dwc3: dwc3@8f94000 {
 
 			status = "disabled";
 
-			dwc3: dwc3@9900000 {
+			dwc3: usb@9900000 {
 				compatible	= "snps,dwc3";
 				reg		= <0x09900000 0x100000>;
 				interrupts	= <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 9ce78a7b117d..7d1bbff25294 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -679,7 +679,7 @@ ohci0: usb@5101400 {
 			status = "disabled";
 		};
 
-		dwc3: dwc3@5200000 {
+		dwc3: usb@5200000 {
 			compatible = "snps,dwc3";
 			reg = <0x05200000 0x10000>;
 			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index 74ac4ac75865..3320e596cb3f 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -1651,7 +1651,7 @@ usbdrd30: usbdrd {
 			ranges;
 			status = "disabled";
 
-			usbdrd_dwc3: dwc3@15400000 {
+			usbdrd_dwc3: usb@15400000 {
 				compatible = "snps,dwc3";
 				clocks = <&cmu_fsys CLK_SCLK_USBDRD30>,
 					<&cmu_fsys CLK_ACLK_USBDRD30>,
@@ -1704,7 +1704,7 @@ usbhost30: usbhost {
 			ranges;
 			status = "disabled";
 
-			usbhost_dwc3: dwc3@15a00000 {
+			usbhost_dwc3: usb@15a00000 {
 				compatible = "snps,dwc3";
 				clocks = <&cmu_fsys CLK_SCLK_USBHOST30>,
 					<&cmu_fsys CLK_ACLK_USBHOST30>,
diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi b/arch/arm64/boot/dts/exynos/exynos7.dtsi
index b9ed6a33e290..48cd3a04fd07 100644
--- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
@@ -654,7 +654,7 @@ usbdrd3 {
 			#size-cells = <1>;
 			ranges;
 
-			dwc3@15400000 {
+			usb@15400000 {
 				compatible = "snps,dwc3";
 				reg = <0x15400000 0x10000>;
 				interrupts = <GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
index ff19ec415b60..06dac6be67e9 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
@@ -453,7 +453,7 @@ edma0: edma@2c00000 {
 				 <&clockgen 4 3>;
 		};
 
-		usb0: usb3@2f00000 {
+		usb0: usb@2f00000 {
 			compatible = "snps,dwc3";
 			reg = <0x0 0x2f00000 0x0 0x10000>;
 			interrupts = <0 60 0x4>;
@@ -474,7 +474,7 @@ sata: sata@3200000 {
 			status = "disabled";
 		};
 
-		usb1: usb2@8600000 {
+		usb1: usb@8600000 {
 			compatible = "fsl-usb2-dr-v2.5", "fsl-usb2-dr";
 			reg = <0x0 0x8600000 0x0 0x1000>;
 			interrupts = <0 139 0x4>;
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index 5c2e370f6316..1f45fa32e57b 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -750,7 +750,7 @@ edma0: edma@2c00000 {
 				 <&clockgen 4 0>;
 		};
 
-		usb0: usb3@2f00000 {
+		usb0: usb@2f00000 {
 			compatible = "snps,dwc3";
 			reg = <0x0 0x2f00000 0x0 0x10000>;
 			interrupts = <0 60 0x4>;
@@ -761,7 +761,7 @@ usb0: usb3@2f00000 {
 			status = "disabled";
 		};
 
-		usb1: usb3@3000000 {
+		usb1: usb@3000000 {
 			compatible = "snps,dwc3";
 			reg = <0x0 0x3000000 0x0 0x10000>;
 			interrupts = <0 61 0x4>;
@@ -772,7 +772,7 @@ usb1: usb3@3000000 {
 			status = "disabled";
 		};
 
-		usb2: usb3@3100000 {
+		usb2: usb@3100000 {
 			compatible = "snps,dwc3";
 			reg = <0x0 0x3100000 0x0 0x10000>;
 			interrupts = <0 63 0x4>;
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
index 169f4742ae3b..96723b53a4e9 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
@@ -402,7 +402,7 @@ esdhc: esdhc@2140000 {
 			status = "disabled";
 		};
 
-		usb0: usb3@3100000 {
+		usb0: usb@3100000 {
 			compatible = "snps,dwc3";
 			reg = <0x0 0x3100000 0x0 0x10000>;
 			interrupts = <0 80 IRQ_TYPE_LEVEL_HIGH>;
@@ -413,7 +413,7 @@ usb0: usb3@3100000 {
 			status = "disabled";
 		};
 
-		usb1: usb3@3110000 {
+		usb1: usb@3110000 {
 			compatible = "snps,dwc3";
 			reg = <0x0 0x3110000 0x0 0x10000>;
 			interrupts = <0 81 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
index 41102dacc2e1..d356ec2beee3 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
@@ -737,7 +737,7 @@ sata1: sata@3210000 {
 			dma-coherent;
 		};
 
-		usb0: usb3@3100000 {
+		usb0: usb@3100000 {
 			status = "disabled";
 			compatible = "snps,dwc3";
 			reg = <0x0 0x3100000 0x0 0x10000>;
@@ -748,7 +748,7 @@ usb0: usb3@3100000 {
 			snps,incr-burst-type-adjustment = <1>, <4>, <8>, <16>;
 		};
 
-		usb1: usb3@3110000 {
+		usb1: usb@3110000 {
 			status = "disabled";
 			compatible = "snps,dwc3";
 			reg = <0x0 0x3110000 0x0 0x10000>;
diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index d25aac5e0bf8..aea3800029b5 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -1166,7 +1166,7 @@ usb_phy: usb-phy {
 			};
 		};
 
-		dwc3: dwc3@ff100000 {
+		dwc3: usb@ff100000 {
 			compatible = "snps,dwc3";
 			reg = <0x0 0xff100000 0x0 0x100000>;
 
diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
index defcbd15edf9..34e97da98270 100644
--- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
@@ -1064,7 +1064,7 @@ &usb2 {
 	status = "okay";
 	extcon = <&usb2_id>;
 
-	dwc3@7600000 {
+	usb@7600000 {
 		extcon = <&usb2_id>;
 		dr_mode = "otg";
 		maximum-speed = "high-speed";
@@ -1075,7 +1075,7 @@ &usb3 {
 	status = "okay";
 	extcon = <&usb3_id>;
 
-	dwc3@6a00000 {
+	usb@6a00000 {
 		extcon = <&usb3_id>;
 		dr_mode = "otg";
 	};
diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index 96a5ec89b5f0..1129062a4ca1 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -427,7 +427,7 @@ usb_0: usb@8af8800 {
 			resets = <&gcc GCC_USB0_BCR>;
 			status = "disabled";
 
-			dwc_0: dwc3@8a00000 {
+			dwc_0: usb@8a00000 {
 				compatible = "snps,dwc3";
 				reg = <0x8a00000 0xcd00>;
 				interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>;
@@ -468,7 +468,7 @@ usb_1: usb@8cf8800 {
 			resets = <&gcc GCC_USB1_BCR>;
 			status = "disabled";
 
-			dwc_1: dwc3@8c00000 {
+			dwc_1: usb@8c00000 {
 				compatible = "snps,dwc3";
 				reg = <0x8c00000 0xcd00>;
 				interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 9951286db775..66b6d2f0a093 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -1767,7 +1767,7 @@ usb3: usb@6af8800 {
 			power-domains = <&gcc USB30_GDSC>;
 			status = "disabled";
 
-			dwc3@6a00000 {
+			usb@6a00000 {
 				compatible = "snps,dwc3";
 				reg = <0x06a00000 0xcc00>;
 				interrupts = <0 131 IRQ_TYPE_LEVEL_HIGH>;
@@ -1978,7 +1978,7 @@ usb2: usb@76f8800 {
 			power-domains = <&gcc USB30_GDSC>;
 			status = "disabled";
 
-			dwc3@7600000 {
+			usb@7600000 {
 				compatible = "snps,dwc3";
 				reg = <0x07600000 0xcc00>;
 				interrupts = <0 138 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index c45870600909..7cc7897e7b83 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -1678,7 +1678,7 @@ usb3: usb@a8f8800 {
 
 			resets = <&gcc GCC_USB_30_BCR>;
 
-			usb3_dwc3: dwc3@a800000 {
+			usb3_dwc3: usb@a800000 {
 				compatible = "snps,dwc3";
 				reg = <0x0a800000 0xcd00>;
 				interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi b/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi
index 6422cf9d5855..88d7b7a53743 100644
--- a/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi
@@ -337,7 +337,7 @@ &usb2_phy_sec {
 &usb3 {
 	status = "okay";
 
-	dwc3@7580000 {
+	usb@7580000 {
 		dr_mode = "host";
 	};
 };
diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index b654b802e95c..f6ef17553064 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -544,7 +544,7 @@ usb3: usb@7678800 {
 			assigned-clock-rates = <19200000>, <200000000>;
 			status = "disabled";
 
-			dwc3@7580000 {
+			usb@7580000 {
 				compatible = "snps,dwc3";
 				reg = <0x07580000 0xcd00>;
 				interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
@@ -573,7 +573,7 @@ usb2: usb@79b8800 {
 			assigned-clock-rates = <19200000>, <133333333>;
 			status = "disabled";
 
-			dwc3@78c0000 {
+			usb@78c0000 {
 				compatible = "snps,dwc3";
 				reg = <0x078c0000 0xcc00>;
 				interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index d46b3833e52f..bbc9a2b5c570 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -2673,7 +2673,7 @@ usb_1: usb@a6f8800 {
 					<&gem_noc MASTER_APPSS_PROC &config_noc SLAVE_USB3>;
 			interconnect-names = "usb-ddr", "apps-usb";
 
-			usb_1_dwc3: dwc3@a600000 {
+			usb_1_dwc3: usb@a600000 {
 				compatible = "snps,dwc3";
 				reg = <0 0x0a600000 0 0xe000>;
 				interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 2884577dcb77..ca20e4e91f61 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -3573,7 +3573,7 @@ usb_1: usb@a6f8800 {
 					<&gladiator_noc MASTER_APPSS_PROC &config_noc SLAVE_USB3_0>;
 			interconnect-names = "usb-ddr", "apps-usb";
 
-			usb_1_dwc3: dwc3@a600000 {
+			usb_1_dwc3: usb@a600000 {
 				compatible = "snps,dwc3";
 				reg = <0 0x0a600000 0 0xcd00>;
 				interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
@@ -3621,7 +3621,7 @@ usb_2: usb@a8f8800 {
 					<&gladiator_noc MASTER_APPSS_PROC &config_noc SLAVE_USB3_1>;
 			interconnect-names = "usb-ddr", "apps-usb";
 
-			usb_2_dwc3: dwc3@a800000 {
+			usb_2_dwc3: usb@a800000 {
 				compatible = "snps,dwc3";
 				reg = <0 0x0a800000 0 0xcd00>;
 				interrupts = <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
index b86a7ead3006..167d14dda974 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -846,7 +846,7 @@ usb_1: usb@a6f8800 {
 
 			resets = <&gcc GCC_USB30_PRIM_BCR>;
 
-			usb_1_dwc3: dwc3@a600000 {
+			usb_1_dwc3: usb@a600000 {
 				compatible = "snps,dwc3";
 				reg = <0 0x0a600000 0 0xcd00>;
 				interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.27.0

Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name

[Krzysztof Kozlowski]

On Wed, 14 Oct 2020 at 12:23, Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> In accordance with the DWC USB3 bindings the corresponding node name is
> suppose to comply with Generic USB HCD DT schema, which requires the USB
> nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> the dtbs_check procedure failure. Let's fix the nodes naming to be
> compatible with the DWC USB3 DT schema to make dtbs_check happy.
>
> Note we don't change the DWC USB3-compatible nodes names of
> arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> in-source comment says that the nodes name need to be preserved as
> "^dwusb@.*" for some backward compatibility.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
>
> ---
>
> Please, test the patch out to make sure it doesn't brake the dependent DTS
> files. I did only a manual grepping of the possible nodes dependencies.

1. It is you who should compare the decompiled DTS, not us. For example:
$ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done

$ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} >
dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt
dts-new/${i#dts-old/}.fdt ; done

2. Split it per arm architectures (and proper subject prefix - not
"arch") and subarchitectures so maintainers can pick it up.

3. The subject title could be more accurate - there is no fix here
because there was no errors in the first place. Requirement of DWC
node names comes recently, so it is more alignment with dtschema.
Otherwise automatic-pickup-stable-bot might want to pick up... and it
should not go to stable.

Best regards,
Krzysztof

>  arch/arm/boot/dts/armada-375.dtsi              | 2 +-
>  arch/arm/boot/dts/exynos5250.dtsi              | 2 +-
>  arch/arm/boot/dts/exynos54xx.dtsi              | 4 ++--
>  arch/arm/boot/dts/keystone-k2e.dtsi            | 4 ++--
>  arch/arm/boot/dts/keystone.dtsi                | 2 +-
>  arch/arm/boot/dts/ls1021a.dtsi                 | 2 +-
>  arch/arm/boot/dts/omap5-l4.dtsi                | 2 +-
>  arch/arm/boot/dts/stih407-family.dtsi          | 2 +-
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi   | 2 +-
>  arch/arm64/boot/dts/exynos/exynos5433.dtsi     | 4 ++--
>  arch/arm64/boot/dts/exynos/exynos7.dtsi        | 2 +-
>  arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 4 ++--
>  arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 +++---
>  arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 ++--
>  arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 ++--
>  arch/arm64/boot/dts/hisilicon/hi3660.dtsi      | 2 +-
>  arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi   | 4 ++--
>  arch/arm64/boot/dts/qcom/ipq8074.dtsi          | 4 ++--
>  arch/arm64/boot/dts/qcom/msm8996.dtsi          | 4 ++--
>  arch/arm64/boot/dts/qcom/msm8998.dtsi          | 2 +-
>  arch/arm64/boot/dts/qcom/qcs404-evb.dtsi       | 2 +-
>  arch/arm64/boot/dts/qcom/qcs404.dtsi           | 4 ++--
>  arch/arm64/boot/dts/qcom/sc7180.dtsi           | 2 +-
>  arch/arm64/boot/dts/qcom/sdm845.dtsi           | 4 ++--
>  arch/arm64/boot/dts/qcom/sm8150.dtsi           | 2 +-
>  25 files changed, 38 insertions(+), 38 deletions(-)
>

Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name

[Serge Semin]

On Wed, Oct 14, 2020 at 12:33:25PM +0200, Krzysztof Kozlowski wrote:
> On Wed, 14 Oct 2020 at 12:23, Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > In accordance with the DWC USB3 bindings the corresponding node name is
> > suppose to comply with Generic USB HCD DT schema, which requires the USB
> > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> >
> > Note we don't change the DWC USB3-compatible nodes names of
> > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > in-source comment says that the nodes name need to be preserved as
> > "^dwusb@.*" for some backward compatibility.
> >
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> >
> > ---
> >
> > Please, test the patch out to make sure it doesn't brake the dependent DTS
> > files. I did only a manual grepping of the possible nodes dependencies.
> 

> 1. It is you who should compare the decompiled DTS, not us. For example:
> $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done
> 
> $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} >
> dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt
> dts-new/${i#dts-old/}.fdt ; done

So basically you suggest first to compile the old and new dts files, then to
de-compile them, then diff old and new fdt's, and visually compare the results.
Personally it isn't that much better than what I did, since each old and new
dtbs will for sure differ due to the node names change suggested in this patch.
So it will lead to the visual debugging too, which isn't that effective. But
your approach is still more demonstrative to make sure that I didn't loose any
nodes redefinition, since in the occasion the old and new de-compiled nodes will
differ not only by the node names but with an additional old named node.

So to speak thanks for suggesting it. I'll try it to validate the proposed
changes.

Two questions:
1) Any advise of a good inliner/command to compile all dtbs at once? Of course I
can get all the updated dtsi'es, then find out all the dts'es which include
them, then directly use dtc to compile the found dts'es... On the other hand I
can just compile all dts'es, then compare old and new ones. The diff of the
non-modified dtb'es will be just empty...
2) What crosc64 is?

> 
> 2. Split it per arm architectures (and proper subject prefix - not
> "arch") and subarchitectures so maintainers can pick it up.

Why? The changes are simple and can be formatted as a single patch. I've seen
tons of patches submitted like that, accepted and then merged. What you suggest
is just much more work, which I don't see quite required.

> 
> 3. The subject title could be more accurate - there is no fix here
> because there was no errors in the first place. Requirement of DWC
> node names comes recently, so it is more alignment with dtschema.
> Otherwise automatic-pickup-stable-bot might want to pick up... and it
> should not go to stable.

Actually it is a fix, because the USB DT nodes should have been named with "usb"
prefix in the first place. Legacy DWC USB3 bindings didn't define the nodes
naming, but implied to be "usb"-prefixed by the USB HCD schema. The Qualcomm
DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which was
wrong in the first place.

Regarding automatic-pickup-stable-bot if it exists I don't think it scans all the
emails, but most likely the stable@vger.kernel.org list only or the emails
having the "Fixes:" tag. If I am wrong please give me a link to the bot sources
or refer to a doc where I can read about the way it works, to take it into
account in future commits. Just to note I submitted patches which did some fixes,
had the word "fix" in the subject but weren't selected to be backported to the
stable kernel.

Anyway I don't really care that much about the subject text using the word "fix"
or some else. So if you suggest some better alternative, I'd be glad to consider
it.

-Sergey

> 
> Best regards,
> Krzysztof
> 
> >  arch/arm/boot/dts/armada-375.dtsi              | 2 +-
> >  arch/arm/boot/dts/exynos5250.dtsi              | 2 +-
> >  arch/arm/boot/dts/exynos54xx.dtsi              | 4 ++--
> >  arch/arm/boot/dts/keystone-k2e.dtsi            | 4 ++--
> >  arch/arm/boot/dts/keystone.dtsi                | 2 +-
> >  arch/arm/boot/dts/ls1021a.dtsi                 | 2 +-
> >  arch/arm/boot/dts/omap5-l4.dtsi                | 2 +-
> >  arch/arm/boot/dts/stih407-family.dtsi          | 2 +-
> >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi   | 2 +-
> >  arch/arm64/boot/dts/exynos/exynos5433.dtsi     | 4 ++--
> >  arch/arm64/boot/dts/exynos/exynos7.dtsi        | 2 +-
> >  arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 4 ++--
> >  arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 +++---
> >  arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 ++--
> >  arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 ++--
> >  arch/arm64/boot/dts/hisilicon/hi3660.dtsi      | 2 +-
> >  arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi   | 4 ++--
> >  arch/arm64/boot/dts/qcom/ipq8074.dtsi          | 4 ++--
> >  arch/arm64/boot/dts/qcom/msm8996.dtsi          | 4 ++--
> >  arch/arm64/boot/dts/qcom/msm8998.dtsi          | 2 +-
> >  arch/arm64/boot/dts/qcom/qcs404-evb.dtsi       | 2 +-
> >  arch/arm64/boot/dts/qcom/qcs404.dtsi           | 4 ++--
> >  arch/arm64/boot/dts/qcom/sc7180.dtsi           | 2 +-
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi           | 4 ++--
> >  arch/arm64/boot/dts/qcom/sm8150.dtsi           | 2 +-
> >  25 files changed, 38 insertions(+), 38 deletions(-)
> >

Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name

[Krzysztof Kozlowski]

On Wed, 14 Oct 2020 at 19:16, Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> On Wed, Oct 14, 2020 at 12:33:25PM +0200, Krzysztof Kozlowski wrote:
> > On Wed, 14 Oct 2020 at 12:23, Serge Semin
> > <Sergey.Semin@baikalelectronics.ru> wrote:
> > >
> > > In accordance with the DWC USB3 bindings the corresponding node name is
> > > suppose to comply with Generic USB HCD DT schema, which requires the USB
> > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> > > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> > >
> > > Note we don't change the DWC USB3-compatible nodes names of
> > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > > in-source comment says that the nodes name need to be preserved as
> > > "^dwusb@.*" for some backward compatibility.
> > >
> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > >
> > > ---
> > >
> > > Please, test the patch out to make sure it doesn't brake the dependent DTS
> > > files. I did only a manual grepping of the possible nodes dependencies.
> >
>
> > 1. It is you who should compare the decompiled DTS, not us. For example:
> > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> > scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done
> >
> > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> > fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} >
> > dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt
> > dts-new/${i#dts-old/}.fdt ; done
>
> So basically you suggest first to compile the old and new dts files, then to
> de-compile them, then diff old and new fdt's, and visually compare the results.
> Personally it isn't that much better than what I did, since each old and new
> dtbs will for sure differ due to the node names change suggested in this patch.
> So it will lead to the visual debugging too, which isn't that effective. But
> your approach is still more demonstrative to make sure that I didn't loose any
> nodes redefinition, since in the occasion the old and new de-compiled nodes will
> differ not only by the node names but with an additional old named node.

My suggestion is to compare the entire, effective DTS after all
inclusions. Maybe you did it already, I don't know. The point is that
when you change node names in DTSI but you miss one in DTS, you end up
with two nodes. This is much easier to spot with dtxdiff or with
fdtdump (which behaves better for node moves).

Indeed it is still a visual comparison - if you have any ideas how to
automate it (e.g. ignore phandle changes), please share. It would
solve my testings as well. But asking others to test because you do
not want to check it is not the best way to handle such changes.

>
> So to speak thanks for suggesting it. I'll try it to validate the proposed
> changes.
>
> Two questions:
> 1) Any advise of a good inliner/command to compile all dtbs at once? Of course I
> can get all the updated dtsi'es, then find out all the dts'es which include
> them, then directly use dtc to compile the found dts'es... On the other hand I
> can just compile all dts'es, then compare old and new ones. The diff of the
> non-modified dtb'es will be just empty...

make dtbs
touch your dts or git stash pop
make dtbs
compare
diff for all unchanged will be simply empty, so easy to spot

> 2) What crosc64 is?

Ah, just an alias for cross compiling + ccache + kbuild out. I just
copied you my helpers, so you need to tweak them.

>
> >
> > 2. Split it per arm architectures (and proper subject prefix - not
> > "arch") and subarchitectures so maintainers can pick it up.
>
> Why? The changes are simple and can be formatted as a single patch. I've seen
> tons of patches submitted like that, accepted and then merged. What you suggest
> is just much more work, which I don't see quite required.

DTS changes go separate between arm64 and arm. There is nothing
unusual here - all changes are submitted like this.
Second topic is to split by subarchitectures which is necessary if you
want it to be picked up by maintainers. It also makes it easier to
review. Sure, without split ber subarchitectures this could be picked
up by SoC folks but you did not even CC them. So if you do not want to
split it per subarchitectures for maintainers and you do not CC SoC,
then how do you believe this should be picked up? Out of the regular
patch submission way? That's not how the changes are handled.

>
> >
> > 3. The subject title could be more accurate - there is no fix here
> > because there was no errors in the first place. Requirement of DWC
> > node names comes recently, so it is more alignment with dtschema.
> > Otherwise automatic-pickup-stable-bot might want to pick up... and it
> > should not go to stable.
>
> Actually it is a fix, because the USB DT nodes should have been named with "usb"
> prefix in the first place. Legacy DWC USB3 bindings didn't define the nodes
> naming, but implied to be "usb"-prefixed by the USB HCD schema. The Qualcomm
> DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which was
> wrong in the first place.

Not following the naming convention of DT spec which was loosely
enforced is not an error which should be "fixed". Simply wrong title.
This is an alignment with dtschema or correcting naming convention.
Not fixing errors.

>
> Regarding automatic-pickup-stable-bot if it exists I don't think it scans all the
> emails, but most likely the stable@vger.kernel.org list only or the emails
> having the "Fixes:" tag. If I am wrong please give me a link to the bot sources
> or refer to a doc where I can read about the way it works, to take it into
> account in future commits. Just to note I submitted patches which did some fixes,
> had the word "fix" in the subject but weren't selected to be backported to the
> stable kernel.

You mixed up bots. The regular stable bot picks commits with cc-stable
or with "Fixes". The auto-pickup bot picks all commits (not emails...
why would it look at emails?) looking like a fix. Wording could be one
of the hints used in the heuristic. Anyway, this is not a fix,
regardless of autosel, so the wording is not correct.

Just Google for AUTOSEL. You can then ask Sasha for sources...

> Anyway I don't really care that much about the subject text using the word "fix"
> or some else. So if you suggest some better alternative, I'd be glad to consider
> it.

I already did. One example is: alignment with dtschema.

Best regards,
Krzysztof

Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name

[Serge Semin]

On Wed, Oct 14, 2020 at 10:04:32PM +0200, Krzysztof Kozlowski wrote:
> On Wed, 14 Oct 2020 at 19:16, Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > On Wed, Oct 14, 2020 at 12:33:25PM +0200, Krzysztof Kozlowski wrote:
> > > On Wed, 14 Oct 2020 at 12:23, Serge Semin
> > > <Sergey.Semin@baikalelectronics.ru> wrote:
> > > >
> > > > In accordance with the DWC USB3 bindings the corresponding node name is
> > > > suppose to comply with Generic USB HCD DT schema, which requires the USB
> > > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> > > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> > > > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > > > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> > > >
> > > > Note we don't change the DWC USB3-compatible nodes names of
> > > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > > > in-source comment says that the nodes name need to be preserved as
> > > > "^dwusb@.*" for some backward compatibility.
> > > >
> > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > >
> > > > ---
> > > >
> > > > Please, test the patch out to make sure it doesn't brake the dependent DTS
> > > > files. I did only a manual grepping of the possible nodes dependencies.
> > >
> >
> > > 1. It is you who should compare the decompiled DTS, not us. For example:
> > > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> > > scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done
> > >
> > > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> > > fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} >
> > > dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt
> > > dts-new/${i#dts-old/}.fdt ; done
> >
> > So basically you suggest first to compile the old and new dts files, then to
> > de-compile them, then diff old and new fdt's, and visually compare the results.
> > Personally it isn't that much better than what I did, since each old and new
> > dtbs will for sure differ due to the node names change suggested in this patch.
> > So it will lead to the visual debugging too, which isn't that effective. But
> > your approach is still more demonstrative to make sure that I didn't loose any
> > nodes redefinition, since in the occasion the old and new de-compiled nodes will
> > differ not only by the node names but with an additional old named node.
> 

> My suggestion is to compare the entire, effective DTS after all
> inclusions. Maybe you did it already, I don't know.

Only by grepping the dts'es, which include the dtsi'es modified in this patch.
So your suggestion of compiling and de-compiling has been indeed relevant.

> The point is that
> when you change node names in DTSI but you miss one in DTS, you end up
> with two nodes.

Yep, that's exactly what I meant when I said that your approach was more
demonstrative, etc.

> This is much easier to spot with dtxdiff or with
> fdtdump (which behaves better for node moves).
> 

> Indeed it is still a visual comparison - if you have any ideas how to
> automate it (e.g. ignore phandle changes), please share. It would
> solve my testings as well.

Alas I don't. That's why to save my time of coming up with an automated solution
I did a very thorough modification making sure that each affected node isn't
updated in the corresponding dts'es and asked to test the patches out.

Anyway the approach suggested by you will indeed give us a better understanding
of my patches correctness. So I'll use it before sending v3. Thanks.

> But asking others to test because you do
> not want to check it is not the best way to handle such changes.
> 
> >
> > So to speak thanks for suggesting it. I'll try it to validate the proposed
> > changes.
> >
> > Two questions:
> > 1) Any advise of a good inliner/command to compile all dtbs at once? Of course I
> > can get all the updated dtsi'es, then find out all the dts'es which include
> > them, then directly use dtc to compile the found dts'es... On the other hand I
> > can just compile all dts'es, then compare old and new ones. The diff of the
> > non-modified dtb'es will be just empty...
> 

> make dtbs

It's not that easy.) "make dtbs" will build dtbs only for enabled boards, which
first need to be enabled in the kernel config. So I'll need to have a config
with all the affected dts. The later is the same as if I just found all the
affected dts and built them one-by-one by directly calling dtc.

> touch your dts or git stash pop
> make dtbs
> compare
> diff for all unchanged will be simply empty, so easy to spot
> 
> > 2) What crosc64 is?
> 
> Ah, just an alias for cross compiling + ccache + kbuild out. I just
> copied you my helpers, so you need to tweak them.
> 
> >
> > >
> > > 2. Split it per arm architectures (and proper subject prefix - not
> > > "arch") and subarchitectures so maintainers can pick it up.
> >
> > Why? The changes are simple and can be formatted as a single patch. I've seen
> > tons of patches submitted like that, accepted and then merged. What you suggest
> > is just much more work, which I don't see quite required.
> 

> DTS changes go separate between arm64 and arm. There is nothing
> unusual here - all changes are submitted like this.
> Second topic is to split by subarchitectures which is necessary if you
> want it to be picked up by maintainers. It also makes it easier to
> review.

The current patches are easy enough for review. The last three patches of the
series is a collection of the one-type changes concerning the same type of
nodes. So reviewing them won't cause any difficulty. But I assume that's not
the main point in this discussion.

> Sure, without split ber subarchitectures this could be picked
> up by SoC folks but you did not even CC them. So if you do not want to
> split it per subarchitectures for maintainers and you do not CC SoC,
> then how do you believe this should be picked up? Out of the regular
> patch submission way? That's not how the changes are handled.

AFAIU there are another ways of merging comprehensive patches. If they get to collect
all the Acked-by tags, they could be merged in, for instance, through Greg' or Rob'
(for dts) repos, if of course they get to agree with doing that. Am I wrong?

My hope was to ask Rob or Greg to get the patches merged in when they get
to collect all the ackes, since I thought it was an option in such cases. So if
they refuse to do so I'll have no choice but to split the series up into a
smaller patches as you say.

> 
> >
> > >
> > > 3. The subject title could be more accurate - there is no fix here
> > > because there was no errors in the first place. Requirement of DWC
> > > node names comes recently, so it is more alignment with dtschema.
> > > Otherwise automatic-pickup-stable-bot might want to pick up... and it
> > > should not go to stable.
> >
> > Actually it is a fix, because the USB DT nodes should have been named with "usb"
> > prefix in the first place. Legacy DWC USB3 bindings didn't define the nodes
> > naming, but implied to be "usb"-prefixed by the USB HCD schema. The Qualcomm
> > DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which was
> > wrong in the first place.
> 

> Not following the naming convention of DT spec which was loosely
> enforced is not an error which should be "fixed". Simply wrong title.
> This is an alignment with dtschema or correcting naming convention.
> Not fixing errors.

From your perspective it wasn't an error, from mine and most likely Rob' it
was.) Anyway as I said I don't care that much about preserving the subject
wording, so what about the next one:
<arch>: <subarch>: Harmonize DWC USB3 nodes name with DT schema
?

-Sergey

> 
> >
> > Regarding automatic-pickup-stable-bot if it exists I don't think it scans all the
> > emails, but most likely the stable@vger.kernel.org list only or the emails
> > having the "Fixes:" tag. If I am wrong please give me a link to the bot sources
> > or refer to a doc where I can read about the way it works, to take it into
> > account in future commits. Just to note I submitted patches which did some fixes,
> > had the word "fix" in the subject but weren't selected to be backported to the
> > stable kernel.
> 
> You mixed up bots. The regular stable bot picks commits with cc-stable
> or with "Fixes". The auto-pickup bot picks all commits (not emails...
> why would it look at emails?) looking like a fix. Wording could be one
> of the hints used in the heuristic. Anyway, this is not a fix,
> regardless of autosel, so the wording is not correct.
> 
> Just Google for AUTOSEL. You can then ask Sasha for sources...
> 
> > Anyway I don't really care that much about the subject text using the word "fix"
> > or some else. So if you suggest some better alternative, I'd be glad to consider
> > it.
> 
> I already did. One example is: alignment with dtschema.
> 
> Best regards,
> Krzysztof

Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name

[Krzysztof Kozlowski]

On Thu, Oct 15, 2020 at 02:51:05AM +0300, Serge Semin wrote:
 > >
> > > So to speak thanks for suggesting it. I'll try it to validate the proposed
> > > changes.
> > >
> > > Two questions:
> > > 1) Any advise of a good inliner/command to compile all dtbs at once? Of course I
> > > can get all the updated dtsi'es, then find out all the dts'es which include
> > > them, then directly use dtc to compile the found dts'es... On the other hand I
> > > can just compile all dts'es, then compare old and new ones. The diff of the
> > > non-modified dtb'es will be just empty...
> > 
> 
> > make dtbs
> 
> It's not that easy.) "make dtbs" will build dtbs only for enabled boards, which
> first need to be enabled in the kernel config. So I'll need to have a config
> with all the affected dts. The later is the same as if I just found all the
> affected dts and built them one-by-one by directly calling dtc.

True. Sometimes allyesconfig for given arch might be helpful but not
always (e.g. for ARM it does not select all of ARMv4 and ARMv5 boards).
Most likely your approach is actually faster/more reliable.

> 
> > touch your dts or git stash pop
> > make dtbs
> > compare
> > diff for all unchanged will be simply empty, so easy to spot
> > 
> > > 2) What crosc64 is?
> > 
> > Ah, just an alias for cross compiling + ccache + kbuild out. I just
> > copied you my helpers, so you need to tweak them.
> > 
> > >
> > > >
> > > > 2. Split it per arm architectures (and proper subject prefix - not
> > > > "arch") and subarchitectures so maintainers can pick it up.
> > >
> > > Why? The changes are simple and can be formatted as a single patch. I've seen
> > > tons of patches submitted like that, accepted and then merged. What you suggest
> > > is just much more work, which I don't see quite required.
> > 
> 
> > DTS changes go separate between arm64 and arm. There is nothing
> > unusual here - all changes are submitted like this.
> > Second topic is to split by subarchitectures which is necessary if you
> > want it to be picked up by maintainers. It also makes it easier to
> > review.
> 
> The current patches are easy enough for review. The last three patches of the
> series is a collection of the one-type changes concerning the same type of
> nodes. So reviewing them won't cause any difficulty. But I assume that's not
> the main point in this discussion.
> 
> > Sure, without split ber subarchitectures this could be picked
> > up by SoC folks but you did not even CC them. So if you do not want to
> > split it per subarchitectures for maintainers and you do not CC SoC,
> > then how do you believe this should be picked up? Out of the regular
> > patch submission way? That's not how the changes are handled.
> 
> AFAIU there are another ways of merging comprehensive patches. If they get to collect
> all the Acked-by tags, they could be merged in, for instance, through Greg' or Rob'
> (for dts) repos, if of course they get to agree with doing that. Am I wrong?
> 
> My hope was to ask Rob or Greg to get the patches merged in when they get
> to collect all the ackes, since I thought it was an option in such cases. So if
> they refuse to do so I'll have no choice but to split the series up into a
> smaller patches as you say.

This is neither Rob's nor Greg's patch to pick up, but ARM SoC (which was
not CCed here). And most likely they won't pick it up because judging by
contents it is obvious it should go via ARM SoC.

Sure, if there are dependencies between some patches they can go with
acks through unrelated trees, but this not the usual way. This is an
exception in the process to solve particular dependency problem.  It has
drawbacks - increases the chances of annoying conflicts.

The case here does not fall into this criteria - there is no dependency
of this patch on the others  Therefore there is no reason to use the
unusual/exceptional way of handling patches.  There is no reason why
this shouldn't go via either specific ARM subarchitecture maintainers or
via ARM SoC.

> > > > 3. The subject title could be more accurate - there is no fix here
> > > > because there was no errors in the first place. Requirement of DWC
> > > > node names comes recently, so it is more alignment with dtschema.
> > > > Otherwise automatic-pickup-stable-bot might want to pick up... and it
> > > > should not go to stable.
> > >
> > > Actually it is a fix, because the USB DT nodes should have been named with "usb"
> > > prefix in the first place. Legacy DWC USB3 bindings didn't define the nodes
> > > naming, but implied to be "usb"-prefixed by the USB HCD schema. The Qualcomm
> > > DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which was
> > > wrong in the first place.
> > 
> 
> > Not following the naming convention of DT spec which was loosely
> > enforced is not an error which should be "fixed". Simply wrong title.
> > This is an alignment with dtschema or correcting naming convention.
> > Not fixing errors.
> 
> From your perspective it wasn't an error, from mine and most likely Rob' it
> was.) Anyway as I said I don't care that much about preserving the subject
> wording, so what about the next one:
> <arch>: <subarch>: Harmonize DWC USB3 nodes name with DT schema
> ?

Looks good.

Best regards,
Krzysztof

Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name

[Serge Semin]

On Thu, Oct 15, 2020 at 08:14:39AM +0200, Krzysztof Kozlowski wrote:
> On Thu, Oct 15, 2020 at 02:51:05AM +0300, Serge Semin wrote:
>  > >
> > > > So to speak thanks for suggesting it. I'll try it to validate the proposed
> > > > changes.
> > > >
> > > > Two questions:
> > > > 1) Any advise of a good inliner/command to compile all dtbs at once? Of course I
> > > > can get all the updated dtsi'es, then find out all the dts'es which include
> > > > them, then directly use dtc to compile the found dts'es... On the other hand I
> > > > can just compile all dts'es, then compare old and new ones. The diff of the
> > > > non-modified dtb'es will be just empty...
> > > 
> > 
> > > make dtbs
> > 
> > It's not that easy.) "make dtbs" will build dtbs only for enabled boards, which
> > first need to be enabled in the kernel config. So I'll need to have a config
> > with all the affected dts. The later is the same as if I just found all the
> > affected dts and built them one-by-one by directly calling dtc.
> 
> True. Sometimes allyesconfig for given arch might be helpful but not
> always (e.g. for ARM it does not select all of ARMv4 and ARMv5 boards).
> Most likely your approach is actually faster/more reliable.
> 
> > 
> > > touch your dts or git stash pop
> > > make dtbs
> > > compare
> > > diff for all unchanged will be simply empty, so easy to spot
> > > 
> > > > 2) What crosc64 is?
> > > 
> > > Ah, just an alias for cross compiling + ccache + kbuild out. I just
> > > copied you my helpers, so you need to tweak them.
> > > 
> > > >
> > > > >
> > > > > 2. Split it per arm architectures (and proper subject prefix - not
> > > > > "arch") and subarchitectures so maintainers can pick it up.
> > > >
> > > > Why? The changes are simple and can be formatted as a single patch. I've seen
> > > > tons of patches submitted like that, accepted and then merged. What you suggest
> > > > is just much more work, which I don't see quite required.
> > > 
> > 
> > > DTS changes go separate between arm64 and arm. There is nothing
> > > unusual here - all changes are submitted like this.
> > > Second topic is to split by subarchitectures which is necessary if you
> > > want it to be picked up by maintainers. It also makes it easier to
> > > review.
> > 
> > The current patches are easy enough for review. The last three patches of the
> > series is a collection of the one-type changes concerning the same type of
> > nodes. So reviewing them won't cause any difficulty. But I assume that's not
> > the main point in this discussion.
> > 
> > > Sure, without split ber subarchitectures this could be picked
> > > up by SoC folks but you did not even CC them. So if you do not want to
> > > split it per subarchitectures for maintainers and you do not CC SoC,
> > > then how do you believe this should be picked up? Out of the regular
> > > patch submission way? That's not how the changes are handled.
> > 
> > AFAIU there are another ways of merging comprehensive patches. If they get to collect
> > all the Acked-by tags, they could be merged in, for instance, through Greg' or Rob'
> > (for dts) repos, if of course they get to agree with doing that. Am I wrong?
> > 
> > My hope was to ask Rob or Greg to get the patches merged in when they get
> > to collect all the ackes, since I thought it was an option in such cases. So if
> > they refuse to do so I'll have no choice but to split the series up into a
> > smaller patches as you say.
> 

> This is neither Rob's nor Greg's patch to pick up, but ARM SoC (which was
> not CCed here). And most likely they won't pick it up because judging by
> contents it is obvious it should go via ARM SoC.
> 
> Sure, if there are dependencies between some patches they can go with
> acks through unrelated trees, but this not the usual way. This is an
> exception in the process to solve particular dependency problem.  It has
> drawbacks - increases the chances of annoying conflicts.
> 
> The case here does not fall into this criteria - there is no dependency
> of this patch on the others  Therefore there is no reason to use the
> unusual/exceptional way of handling patches.  There is no reason why
> this shouldn't go via either specific ARM subarchitecture maintainers or
> via ARM SoC.

Ok. I see your point. To sum it up I've studied the git log arch/ commit
messages and it turns out even Rob has to split the cleanup changes like this
ones. So thanks for your patience with stating your point. I'll split the last
three patches up to be merged in via the corresponding archs/subarch'es repos.

-Sergey

> 
> > > > > 3. The subject title could be more accurate - there is no fix here
> > > > > because there was no errors in the first place. Requirement of DWC
> > > > > node names comes recently, so it is more alignment with dtschema.
> > > > > Otherwise automatic-pickup-stable-bot might want to pick up... and it
> > > > > should not go to stable.
> > > >
> > > > Actually it is a fix, because the USB DT nodes should have been named with "usb"
> > > > prefix in the first place. Legacy DWC USB3 bindings didn't define the nodes
> > > > naming, but implied to be "usb"-prefixed by the USB HCD schema. The Qualcomm
> > > > DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which was
> > > > wrong in the first place.
> > > 
> > 
> > > Not following the naming convention of DT spec which was loosely
> > > enforced is not an error which should be "fixed". Simply wrong title.
> > > This is an alignment with dtschema or correcting naming convention.
> > > Not fixing errors.
> > 
> > From your perspective it wasn't an error, from mine and most likely Rob' it
> > was.) Anyway as I said I don't care that much about preserving the subject
> > wording, so what about the next one:
> > <arch>: <subarch>: Harmonize DWC USB3 nodes name with DT schema
> > ?
> 
> Looks good.
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]


Hi Serge,

Serge Semin <Sergey.Semin@baikalelectronics.ru> writes:
> In accordance with the DWC USB3 bindings the corresponding node name is
> suppose to comply with Generic USB HCD DT schema, which requires the USB

DWC3 is not a simple HDC, though.

> nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> the dtbs_check procedure failure. Let's fix the nodes naming to be
> compatible with the DWC USB3 DT schema to make dtbs_check happy.
>
> Note we don't change the DWC USB3-compatible nodes names of
> arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> in-source comment says that the nodes name need to be preserved as
> "^dwusb@.*" for some backward compatibility.

interesting, compatibility with what? Some debugfs files, perhaps? :-)

In any case, I don't have any problems with this, so I'll let other
folks comment.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name

[Serge Semin]

On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote:
> 
> Hi Serge,
> 
> Serge Semin <Sergey.Semin@baikalelectronics.ru> writes:
> > In accordance with the DWC USB3 bindings the corresponding node name is
> > suppose to comply with Generic USB HCD DT schema, which requires the USB
> 

> DWC3 is not a simple HDC, though.

Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff,
which are tuned by the DWC USB3 driver in the kernel. But after that the
controller is registered as xhci-hcd device so it's serviced by the xHCI driver,
which then registers the HCD device so the corresponding DT node is supposed
to be compatible with the next bindings: usb/usb-hcd.yaml, usb/usb-xhci.yaml
and usb/snps,dwc3,yaml. I've created the later one so to validate the denoted
compatibility.

> 
> > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> >
> > Note we don't change the DWC USB3-compatible nodes names of
> > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > in-source comment says that the nodes name need to be preserved as
> > "^dwusb@.*" for some backward compatibility.
> 

> interesting, compatibility with what? Some debugfs files, perhaps? :-)

Don't really know.) In my experience the worst type of such compatibility is
connected with some bootloader magic, which may add/remove/modify properties
to nodes with pre-defined names.

-Sergey

> 
> In any case, I don't have any problems with this, so I'll let other
> folks comment.
> 
> -- 
> balbi

Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name

[Rob Herring]

On Wed, Oct 14, 2020 at 9:37 AM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote:
> >
> > Hi Serge,
> >
> > Serge Semin <Sergey.Semin@baikalelectronics.ru> writes:
> > > In accordance with the DWC USB3 bindings the corresponding node name is
> > > suppose to comply with Generic USB HCD DT schema, which requires the USB
> >
>
> > DWC3 is not a simple HDC, though.
>
> Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff,
> which are tuned by the DWC USB3 driver in the kernel. But after that the
> controller is registered as xhci-hcd device so it's serviced by the xHCI driver,
> which then registers the HCD device so the corresponding DT node is supposed
> to be compatible with the next bindings: usb/usb-hcd.yaml, usb/usb-xhci.yaml
> and usb/snps,dwc3,yaml. I've created the later one so to validate the denoted
> compatibility.
>
> >
> > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> > > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> > >
> > > Note we don't change the DWC USB3-compatible nodes names of
> > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > > in-source comment says that the nodes name need to be preserved as
> > > "^dwusb@.*" for some backward compatibility.
> >
>
> > interesting, compatibility with what? Some debugfs files, perhaps? :-)
>
> Don't really know.) In my experience the worst type of such compatibility is
> connected with some bootloader magic, which may add/remove/modify properties
> to nodes with pre-defined names.

I seriously doubt anyone is using the APM machines with DT (even ACPI
is somewhat doubtful). I say change them. Or remove the dts files and
see what happens. Either way it can always be reverted.

Rob

Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name

[Serge Semin]

On Wed, Oct 14, 2020 at 01:35:16PM -0500, Rob Herring wrote:
> On Wed, Oct 14, 2020 at 9:37 AM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote:
> > >
> > > Hi Serge,
> > >
> > > Serge Semin <Sergey.Semin@baikalelectronics.ru> writes:
> > > > In accordance with the DWC USB3 bindings the corresponding node name is
> > > > suppose to comply with Generic USB HCD DT schema, which requires the USB
> > >
> >
> > > DWC3 is not a simple HDC, though.
> >
> > Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff,
> > which are tuned by the DWC USB3 driver in the kernel. But after that the
> > controller is registered as xhci-hcd device so it's serviced by the xHCI driver,
> > which then registers the HCD device so the corresponding DT node is supposed
> > to be compatible with the next bindings: usb/usb-hcd.yaml, usb/usb-xhci.yaml
> > and usb/snps,dwc3,yaml. I've created the later one so to validate the denoted
> > compatibility.
> >
> > >
> > > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> > > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> > > > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > > > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> > > >
> > > > Note we don't change the DWC USB3-compatible nodes names of
> > > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > > > in-source comment says that the nodes name need to be preserved as
> > > > "^dwusb@.*" for some backward compatibility.
> > >
> >
> > > interesting, compatibility with what? Some debugfs files, perhaps? :-)
> >
> > Don't really know.) In my experience the worst type of such compatibility is
> > connected with some bootloader magic, which may add/remove/modify properties
> > to nodes with pre-defined names.
> 

> I seriously doubt anyone is using the APM machines with DT (even ACPI
> is somewhat doubtful). I say change them. Or remove the dts files and
> see what happens. Either way it can always be reverted.

Ok. I'll change them in v3.

-Sergey

> 
> Rob

Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 831 bytes --]

Serge Semin <Sergey.Semin@baikalelectronics.ru> writes:

> On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote:
>> 
>> Hi Serge,
>> 
>> Serge Semin <Sergey.Semin@baikalelectronics.ru> writes:
>> > In accordance with the DWC USB3 bindings the corresponding node name is
>> > suppose to comply with Generic USB HCD DT schema, which requires the USB
>> 
>
>> DWC3 is not a simple HDC, though.
>
> Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff,
> which are tuned by the DWC USB3 driver in the kernel. But after that the
> controller is registered as xhci-hcd device so it's serviced by the xHCI driver,

in Dual-role or host-only builds, that's correct. We can also have
peripheral-only builds (both SW or HW versions) which means xhci isn't
even in the picture.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name

[Serge Semin]

On Thu, Oct 15, 2020 at 01:15:37PM +0300, Felipe Balbi wrote:
> Serge Semin <Sergey.Semin@baikalelectronics.ru> writes:
> 
> > On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote:
> >> 
> >> Hi Serge,
> >> 
> >> Serge Semin <Sergey.Semin@baikalelectronics.ru> writes:
> >> > In accordance with the DWC USB3 bindings the corresponding node name is
> >> > suppose to comply with Generic USB HCD DT schema, which requires the USB
> >> 
> >
> >> DWC3 is not a simple HDC, though.
> >
> > Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff,
> > which are tuned by the DWC USB3 driver in the kernel. But after that the
> > controller is registered as xhci-hcd device so it's serviced by the xHCI driver,
> 

> in Dual-role or host-only builds, that's correct. We can also have
> peripheral-only builds (both SW or HW versions) which means xhci isn't
> even in the picture.

Hm, good point. In that case perhaps we'll need to apply the xHCI DT schema
conditionally. Like this:

- allOf:
-   - $ref: usb-xhci.yaml#
+ allOf:
+   - if:
+       properties:
+         dr_mode:
+           const: peripheral
+     then:
+       $ref: usb-hcd.yaml#
+     else:
+       $ref: usb-xhci.yaml#

Note, we need to have the peripheral device being compatible with the
usb-hcd.yaml schema to support the maximum-speed, dr_mode properties and to
comply with the USB node naming constraint.

-Sergey

> 
> -- 
> balbi

Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1398 bytes --]

On Thu, Oct 15, 2020 at 01:15:37PM +0300, Felipe Balbi wrote:
> Serge Semin <Sergey.Semin@baikalelectronics.ru> writes:
> 
> > On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote:
> >> 
> >> Hi Serge,
> >> 
> >> Serge Semin <Sergey.Semin@baikalelectronics.ru> writes:
> >> > In accordance with the DWC USB3 bindings the corresponding node name is
> >> > suppose to comply with Generic USB HCD DT schema, which requires the USB
> >> 
> >
> >> DWC3 is not a simple HDC, though.
> >
> > Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff,
> > which are tuned by the DWC USB3 driver in the kernel. But after that the
> > controller is registered as xhci-hcd device so it's serviced by the xHCI driver,
> 
> in Dual-role or host-only builds, that's correct. We can also have
> peripheral-only builds (both SW or HW versions) which means xhci isn't
> even in the picture.

It doesn't really matter though, or at least it does for what the new
name might be, but the old one currently used is still pretty bad.

The DT spec says that the node name is the class of the device. "usb" as
the HCD binding mandates is one, but the current nodes currently have
completely different names from one DT to another - which is already an
issue - and most of them have dwc3 or some variant of it, which doesn't
really qualify for a class name.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]