Skip to contentSkip to conversation

Add USB DWC3 support for SC7180

PATCH Superseded Sandeep Maheswaram 4 participants 8 messages Feb 10, 2020 – Mar 24, 2020

Subsystems (3)

v4 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v4 cover Add USB DWC3 support for SC7180

Converting dt binding to yaml.
Adding compatible for SC7180 in dt bindings.

Changes in v4:
Addressed the comments from Doug in yaml file.

Changes in v3:
Dropped the patch for adding the compatible in dwc3 driver from the series. Addressed the comments from Doug in yaml file.

Changes in v2:
Sorted the compatible in dwc3 driver.
Converted dt binding to yaml.
Added compatible in yaml.

Sandeep Maheswaram (2):
dt-bindings: usb: qcom,dwc3: Convert USB DWC3 bindings
dt-bindings: usb: qcom,dwc3: Add compatible for SC7180

 .../devicetree/bindings/usb/qcom,dwc3.txt          | 104 --------------
 .../devicetree/bindings/usb/qcom,dwc3.yaml         | 156 +++++++++++++++++++++
 2 files changed, 156 insertions(+), 104 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.txt
 create mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#1 lore raw

PATCH v4 1/2 dt-bindings: usb: qcom,dwc3: Convert USB DWC3 bindings

Convert USB DWC3 bindings to DT schema format using json-schema.

---
 .../devicetree/bindings/usb/qcom,dwc3.txt          | 104 --------------
 .../devicetree/bindings/usb/qcom,dwc3.yaml         | 155 +++++++++++++++++++++
 2 files changed, 155 insertions(+), 104 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.txt
 create mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
Documentation/devicetree/bindings/usb/qcom,dwc3.txt deleted +0 -104
deleted file mode 100644
index cb695aa..0000000
@@ -1,104 +0,0 @@
-Qualcomm SuperSpeed DWC3 USB SoC controller
-
-Required properties:
-- compatible:		Compatible list, contains
-			"qcom,dwc3"
-			"qcom,msm8996-dwc3" for msm8996 SOC.
-			"qcom,msm8998-dwc3" for msm8998 SOC.
-			"qcom,sdm845-dwc3" for sdm845 SOC.
-- reg:			Offset and length of register set for QSCRATCH wrapper
-- power-domains:	specifies a phandle to PM domain provider node
-- clocks:		A list of phandle + clock-specifier pairs for the
-				clocks listed in clock-names
-- clock-names:		Should contain the following:
-  "core"		Master/Core clock, have to be >= 125 MHz for SS
-				operation and >= 60MHz for HS operation
-  "mock_utmi"		Mock utmi clock needed for ITP/SOF generation in
-				host mode. Its frequency should be 19.2MHz.
-  "sleep"		Sleep clock, used for wakeup when USB3 core goes
-				into low power mode (U3).
-
-Optional clocks:
-  "iface"		System bus AXI clock.
-			Not present on "qcom,msm8996-dwc3" compatible.
-  "cfg_noc"		System Config NOC clock.
-			Not present on "qcom,msm8996-dwc3" compatible.
-- assigned-clocks:	Should be:
-				MOCK_UTMI_CLK
-				MASTER_CLK
-- assigned-clock-rates: Should be:
-                                19.2Mhz (192000000) for MOCK_UTMI_CLK
-                                >=125Mhz (125000000) for MASTER_CLK in SS mode
-                                >=60Mhz (60000000) for MASTER_CLK in HS mode
-
-Optional properties:
-- resets:		Phandle to reset control that resets core and wrapper.
-- interrupts:		specifies interrupts from controller wrapper used
-			to wakeup from low power/susepnd state.	Must contain
-			one or more entry for interrupt-names property
-- interrupt-names:	Must include the following entries:
-			- "hs_phy_irq": The interrupt that is asserted when a
-			   wakeup event is received on USB2 bus
-			- "ss_phy_irq": The interrupt that is asserted when a
-			   wakeup event is received on USB3 bus
-			- "dm_hs_phy_irq" and "dp_hs_phy_irq": Separate
-			   interrupts for any wakeup event on DM and DP lines
-- qcom,select-utmi-as-pipe-clk: if present, disable USB3 pipe_clk requirement.
-				Used when dwc3 operates without SSPHY and only
-				HS/FS/LS modes are supported.
-
-Required child node:
-A child node must exist to represent the core DWC3 IP block. The name of
-the node is not important. The content of the node is defined in dwc3.txt.
-
-Phy documentation is provided in the following places:
-Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt   - USB3 QMP PHY
-Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt - USB2 QUSB2 PHY
-
-Example device nodes:
-
-		hs_phy: phy@100f8800 {
-			compatible = "qcom,qusb2-v2-phy";
-			...
-		};
-
-		ss_phy: phy@100f8830 {
-			compatible = "qcom,qmp-v3-usb3-phy";
-			...
-		};
-
-		usb3_0: usb30@a6f8800 {
-			compatible = "qcom,dwc3";
-			reg = <0xa6f8800 0x400>;
-			#address-cells = <1>;
-			#size-cells = <1>;
-			ranges;
-
-			interrupts = <0 131 0>, <0 486 0>, <0 488 0>, <0 489 0>;
-			interrupt-names = "hs_phy_irq", "ss_phy_irq",
-				  "dm_hs_phy_irq", "dp_hs_phy_irq";
-
-			clocks = <&gcc GCC_USB30_PRIM_MASTER_CLK>,
-				<&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
-				<&gcc GCC_USB30_PRIM_SLEEP_CLK>;
-			clock-names = "core", "mock_utmi", "sleep";
-
-			assigned-clocks = <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
-					  <&gcc GCC_USB30_PRIM_MASTER_CLK>;
-			assigned-clock-rates = <19200000>, <133000000>;
-
-			resets = <&gcc GCC_USB30_PRIM_BCR>;
-			reset-names = "core_reset";
-			power-domains = <&gcc USB30_PRIM_GDSC>;
-			qcom,select-utmi-as-pipe-clk;
-
-			dwc3@10000000 {
-				compatible = "snps,dwc3";
-				reg = <0x10000000 0xcd00>;
-				interrupts = <0 205 0x4>;
-				phys = <&hs_phy>, <&ss_phy>;
-				phy-names = "usb2-phy", "usb3-phy";
-				dr_mode = "host";
-			};
-		};
-
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +155 -0

155 lines not on this page · raw message

Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#2 lore raw

Re: [PATCH v4 1/2] dt-bindings: usb: qcom,dwc3: Convert USB DWC3 bindings

Quoting Sandeep Maheswaram (2020-02-09 22:36:44)

Quoted 41 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +35 -0
new file mode 100644
index 0000000..0353401
@@ -0,0 +1,155 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/qcom,dwc3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SuperSpeed DWC3 USB SoC controller
+
+maintainers:
+  - Manu Gautam <[email protected]>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qcom,msm8996-dwc3
+          - qcom,msm8998-dwc3
+          - qcom,sdm845-dwc3
+      - const: qcom,dwc3
+
+  reg:
+    description: Offset and length of register set for QSCRATCH wrapper
+    maxItems: 1
+
+  "#address-cells":
+    enum: [ 1, 2 ]
+
+  "#size-cells":
+    enum: [ 1, 2 ]
+
+  power-domains:
+    description: specifies a phandle to PM domain provider node
+    maxItems: 1
+
+  clocks:

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 45

Quoted 10 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +10 -0
+    description:
+      A list of phandle and clock-specifier pairs for the clocks
+      listed in clock-names.
+    items:
+      - description: System Config NOC clock.
+      - description: Master/Core clock, has to be >= 125 MHz
+          for SS operation and >= 60MHz for HS operation.
+      - description: System bus AXI clock.
+      - description: Mock utmi clock needed for ITP/SOF generation
+          in host mode.Its frequency should be 19.2MHz.

Please add a space between the end of sentence and next one.

Quoted 5 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +5 -0
+      - description: Sleep clock, used for wakeup when
+          USB3 core goes into low power mode (U3).
+
+  clock-names:
+    items:

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 60

Quoted 10 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +10 -0
+      - const: cfg_noc
+      - const: core
+      - const: iface
+      - const: mock_utmi
+      - const: sleep
+
+  assigned-clocks:
+    items:
+      - description: Phandle to MOCK_UTMI_CLK.
+      - description: Phandle to MASTER_CLK.

It's a phandle and clock specifier pair, not always just a phandle. Maybe the base schema can enforce that somehow, but the description isn't accurate.

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 65

Quoted 5 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +5 -0
+
+  assigned-clock-rates:
+    items:
+      - description: Must be 19.2MHz (19200000).
+      - description: Must be >= 60 MHz in HS mode, >= 125 MHz in SS mode.

Can this be more strict? I see in [1] that it was suggested to update the schema checker. Did you try that?

Quoted 34 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +34 -0
+
+  resets:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: The interrupt that is asserted
+          when a wakeup event is received on USB2 bus.
+      - description: The interrupt that is asserted
+          when a wakeup event is received on USB3 bus.
+      - description: Wakeup event on DM line.
+      - description: Wakeup event on DP line.
+
+  interrupt-names:
+    items:
+      - const: hs_phy_irq
+      - const: ss_phy_irq
+      - const: dm_hs_phy_irq
+      - const: dp_hs_phy_irq
+
+  qcom,select-utmi-as-pipe-clk:
+    description:
+      If present, disable USB3 pipe_clk requirement.
+      Used when dwc3 operates without SSPHY and only
+      HS/FS/LS modes are supported.
+    type: boolean
+
+# Required child node:
+
+patternProperties:
+  "^dwc3@[0-9a-f]+$":
+    type: object
+    description:
+      A child node must exist to represent the core DWC3 IP block

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 109

Quoted 10 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +10 -0
+      The content of the node is defined in dwc3.txt.
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+  - power-domains
+  - clocks
+  - clock-names

Why aren't interrupts required? They're always present, aren't they?

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 114

Quoted 5 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +5 -0
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-sdm845.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>

It would be good to include <dt-bindings/interrupt-controller/irq.h> here too, just in case someone wants to move that include out of arm-gic.h, which is possible.

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 115

Quoted 1 line
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +1 -0
+    usb_1: usb@a6f8800 {

Can we drop the phandle? It's not used.

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 128

Quoted 13 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +13 -0
+        compatible = "qcom,sdm845-dwc3", "qcom,dwc3";
+        reg = <0 0x0a6f8800 0 0x400>;
+
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        clocks = <&gcc GCC_CFG_NOC_USB3_PRIM_AXI_CLK>,
+                 <&gcc GCC_USB30_PRIM_MASTER_CLK>,
+                 <&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>,
+                 <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
+                 <&gcc GCC_USB30_PRIM_SLEEP_CLK>;
+        clock-names = "cfg_noc", "core", "iface", "mock_utmi",
+                        "sleep";

Spacing looks off. Are there tabs?

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 139

Quoted 11 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +11 -0
+
+        assigned-clocks = <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
+                          <&gcc GCC_USB30_PRIM_MASTER_CLK>;
+        assigned-clock-rates = <19200000>, <150000000>;
+
+        interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 486 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 488 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 489 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "hs_phy_irq", "ss_phy_irq",
+                              "dm_hs_phy_irq", "dp_hs_phy_irq";

Same spacing nit

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 145

Quoted 6 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +6 -0
+
+            power-domains = <&gcc USB30_PRIM_GDSC>;
+
+            resets = <&gcc GCC_USB30_PRIM_BCR>;
+
+            usb_1_dwc3: dwc3@a600000 {

Drop this phandle too? It isn't used.

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 154

Quoted 9 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +9 -0
+                compatible = "snps,dwc3";
+                reg = <0 0x0a600000 0 0xcd00>;
+                interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
+                iommus = <&apps_smmu 0x740 0>;
+                snps,dis_u2_susphy_quirk;
+                snps,dis_enblslpm_quirk;
+                phys = <&usb_1_hsphy>, <&usb_1_ssphy>;
+                phy-names = "usb2-phy", "usb3-phy";
+            };

[1] https://lkml.kernel.org/r/20191218221310.GA4624@bogus

#3 lore raw

Re: [PATCH v4 1/2] dt-bindings: usb: qcom,dwc3: Convert USB DWC3 bindings

Sandeep, are you going to spin this series?
-Evan

#4 lore raw

Re: [PATCH v4 1/2] dt-bindings: usb: qcom,dwc3: Convert USB DWC3 bindings

Quoted 1 line · Evan Green

Sandeep, are you going to spin this series?

ping

#5 lore raw

Re: [PATCH v4 1/2] dt-bindings: usb: qcom,dwc3: Convert USB DWC3 bindings

In reply to Stephen Boyd

Hi Stephen,

Quoted 45 lines · Stephen Boyd

Quoting Sandeep Maheswaram (2020-02-09 22:36:44)

Quoted 44 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +38 -0
new file mode 100644
index 0000000..0353401
@@ -0,0 +1,155 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/qcom,dwc3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SuperSpeed DWC3 USB SoC controller
+
+maintainers:
+  - Manu Gautam <[email protected]>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qcom,msm8996-dwc3
+          - qcom,msm8998-dwc3
+          - qcom,sdm845-dwc3
+      - const: qcom,dwc3
+
+  reg:
+    description: Offset and length of register set for QSCRATCH wrapper
+    maxItems: 1
+
+  "#address-cells":
+    enum: [ 1, 2 ]
+
+  "#size-cells":
+    enum: [ 1, 2 ]
+
+  power-domains:
+    description: specifies a phandle to PM domain provider node
+    maxItems: 1
+
+  clocks:
+    description:
+      A list of phandle and clock-specifier pairs for the clocks
+      listed in clock-names.
Quoted 8 lines

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 45

Quoted 7 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +7 -0
+    items:
+      - description: System Config NOC clock.
+      - description: Master/Core clock, has to be >= 125 MHz
+          for SS operation and >= 60MHz for HS operation.
+      - description: System bus AXI clock.
+      - description: Mock utmi clock needed for ITP/SOF generation
+          in host mode.Its frequency should be 19.2MHz.

Please add a space between the end of sentence and next one.

will do in next version

Quoted 11 lines
Quoted 11 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +10 -0
+      - description: Sleep clock, used for wakeup when
+          USB3 core goes into low power mode (U3).
+
+  clock-names:
+    items:
+      - const: cfg_noc
+      - const: core
+      - const: iface
+      - const: mock_utmi
+      - const: sleep
Quoted 8 lines

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 60

Quoted 5 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +5 -0
+
+  assigned-clocks:
+    items:
+      - description: Phandle to MOCK_UTMI_CLK.
+      - description: Phandle to MASTER_CLK.

It's a phandle and clock specifier pair, not always just a phandle. Maybe the base schema can enforce that somehow, but the description isn't accurate.

will do in next version

Quoted 8 lines

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 65

Quoted 6 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +5 -0
+
+  assigned-clock-rates:
+    items:
+      - description: Must be 19.2MHz (19200000).
+      - description: Must be >= 60 MHz in HS mode, >= 125 MHz in SS mode.

Can this be more strict? I see in [1] that it was suggested to update the schema checker. Did you try that?

Tried that but need to add maximum value also and even after that getting some errors as below.

/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dt.yaml: usb@a6f8800: assigned-clock-rates: Additional items are not allowed ([150000000] was unexpected)

/local/mnt/workspace/sandeep/bu_build/src/third_party/kernel/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dt.yaml: usb@a6f8800: assigned-clock-rates:0: [19200000] is too short

Quoted 38 lines
Quoted 38 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +37 -0
+
+  resets:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: The interrupt that is asserted
+          when a wakeup event is received on USB2 bus.
+      - description: The interrupt that is asserted
+          when a wakeup event is received on USB3 bus.
+      - description: Wakeup event on DM line.
+      - description: Wakeup event on DP line.
+
+  interrupt-names:
+    items:
+      - const: hs_phy_irq
+      - const: ss_phy_irq
+      - const: dm_hs_phy_irq
+      - const: dp_hs_phy_irq
+
+  qcom,select-utmi-as-pipe-clk:
+    description:
+      If present, disable USB3 pipe_clk requirement.
+      Used when dwc3 operates without SSPHY and only
+      HS/FS/LS modes are supported.
+    type: boolean
+
+# 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.
+
+required:
Quoted 8 lines

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 109

Quoted 7 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +7 -0
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+  - power-domains
+  - clocks
+  - clock-names

Why aren't interrupts required? They're always present, aren't they?

In qcom,dwc3.txt file interrupts are mentioned in Optional properties and I also didnt find any interrupts in 8996.dtsi

Quoted 10 lines

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 114

Quoted 6 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +5 -0
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-sdm845.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>

It would be good to include <dt-bindings/interrupt-controller/irq.h> here too, just in case someone wants to move that include out of arm-gic.h, which is possible.

Quoted 2 lines

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 115

Quoted 1 line
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +1 -0
+    usb_1: usb@a6f8800 {

Can we drop the phandle? It's not used.

will do in next version

Quoted 15 lines
Quoted 4 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +3 -0
+        compatible = "qcom,sdm845-dwc3", "qcom,dwc3";
+        reg = <0 0x0a6f8800 0 0x400>;
+

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 128

Quoted 10 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +10 -0
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        clocks = <&gcc GCC_CFG_NOC_USB3_PRIM_AXI_CLK>,
+                 <&gcc GCC_USB30_PRIM_MASTER_CLK>,
+                 <&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>,
+                 <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
+                 <&gcc GCC_USB30_PRIM_SLEEP_CLK>;
+        clock-names = "cfg_noc", "core", "iface", "mock_utmi",
+                        "sleep";

Spacing looks off. Are there tabs?

will correct in next version

Quoted 13 lines

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 139

Quoted 12 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +11 -0
+
+        assigned-clocks = <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
+                          <&gcc GCC_USB30_PRIM_MASTER_CLK>;
+        assigned-clock-rates = <19200000>, <150000000>;
+
+        interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 486 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 488 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 489 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "hs_phy_irq", "ss_phy_irq",
+                              "dm_hs_phy_irq", "dp_hs_phy_irq";

Same spacing nit

will correct in next version

Quoted 8 lines

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 145

Quoted 7 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +6 -0
+
+            power-domains = <&gcc USB30_PRIM_GDSC>;
+
+            resets = <&gcc GCC_USB30_PRIM_BCR>;
+
+            usb_1_dwc3: dwc3@a600000 {

Drop this phandle too? It isn't used.

will correct in next version

Quoted 11 lines

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 154

Quoted 10 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +9 -0
+                compatible = "snps,dwc3";
+                reg = <0 0x0a600000 0 0xcd00>;
+                interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
+                iommus = <&apps_smmu 0x740 0>;
+                snps,dis_u2_susphy_quirk;
+                snps,dis_enblslpm_quirk;
+                phys = <&usb_1_hsphy>, <&usb_1_ssphy>;
+                phy-names = "usb2-phy", "usb3-phy";
+            };

[1] https://lkml.kernel.org/r/20191218221310.GA4624@bogus

Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
#6 lore raw

Re: [PATCH v4 1/2] dt-bindings: usb: qcom,dwc3: Convert USB DWC3 bindings

Hi Stephen,

Can you check my reply to the review comments and let me know how to proceeed.

Thanks

Sandeep

Quoted 204 lines · Sandeep Maheswaram (Temp)

Hi Stephen,

Quoted 46 lines · Stephen Boyd

Quoting Sandeep Maheswaram (2020-02-09 22:36:44)

Quoted 45 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +22 -0
new file mode 100644
index 0000000..0353401
@@ -0,0 +1,155 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/qcom,dwc3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SuperSpeed DWC3 USB SoC controller
+
+maintainers:
+  - Manu Gautam <[email protected]>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qcom,msm8996-dwc3
+          - qcom,msm8998-dwc3
+          - qcom,sdm845-dwc3
+      - const: qcom,dwc3
+
+  reg:
+    description: Offset and length of register set for QSCRATCH 

wrapper
+    maxItems: 1

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +15 -0
+
+  "#address-cells":
+    enum: [ 1, 2 ]
+
+  "#size-cells":
+    enum: [ 1, 2 ]
+
+  power-domains:
+    description: specifies a phandle to PM domain provider node
+    maxItems: 1
+
+  clocks:
+    description:
+      A list of phandle and clock-specifier pairs for the clocks
+      listed in clock-names.
Quoted 8 lines
Quoted 7 lines

+    items:
+      - description: System Config NOC clock.
+      - description: Master/Core clock, has to be >= 125 MHz +          for SS operation and >= 60MHz for HS operation. +      - description: System bus AXI clock.
+      - description: Mock utmi clock needed for ITP/SOF generation +          in host mode.Its frequency should be 19.2MHz.

Please add a space between the end of sentence and next one.

will do in next version

Quoted 11 lines
Quoted 11 lines

+      - description: Sleep clock, used for wakeup when +          USB3 core goes into low power mode (U3). +
+  clock-names:
+    items:
+      - const: cfg_noc
+      - const: core
+      - const: iface
+      - const: mock_utmi
+      - const: sleep

Quoted 8 lines
Quoted 5 lines

+
+  assigned-clocks:
+    items:
+      - description: Phandle to MOCK_UTMI_CLK.
+      - description: Phandle to MASTER_CLK.

It's a phandle and clock specifier pair, not always just a phandle. Maybe the base schema can enforce that somehow, but the description isn't accurate.

will do in next version

Quoted 9 lines
Quoted 7 lines

+
+  assigned-clock-rates:
+    items:
+      - description: Must be 19.2MHz (19200000).
+      - description: Must be >= 60 MHz in HS mode, >= 125 MHz in SS mode.

Can this be more strict? I see in [1] that it was suggested to update the schema checker. Did you try that?

Tried that but need to add maximum value also and even after that getting some errors as below.

/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dt.yaml: usb@a6f8800: assigned-clock-rates: Additional items are not allowed ([150000000] was unexpected)

/local/mnt/workspace/sandeep/bu_build/src/third_party/kernel/linux-next/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dt.yaml: usb@a6f8800: assigned-clock-rates:0: [19200000] is too short

Quoted 38 lines
Quoted 38 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +37 -0
+
+  resets:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: The interrupt that is asserted
+          when a wakeup event is received on USB2 bus.
+      - description: The interrupt that is asserted
+          when a wakeup event is received on USB3 bus.
+      - description: Wakeup event on DM line.
+      - description: Wakeup event on DP line.
+
+  interrupt-names:
+    items:
+      - const: hs_phy_irq
+      - const: ss_phy_irq
+      - const: dm_hs_phy_irq
+      - const: dp_hs_phy_irq
+
+  qcom,select-utmi-as-pipe-clk:
+    description:
+      If present, disable USB3 pipe_clk requirement.
+      Used when dwc3 operates without SSPHY and only
+      HS/FS/LS modes are supported.
+    type: boolean
+
+# 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.
+
+required:
Quoted 8 lines
Quoted 7 lines

+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+  - power-domains
+  - clocks
+  - clock-names

Why aren't interrupts required? They're always present, aren't they?

In qcom,dwc3.txt file interrupts are mentioned in Optional properties and I also didnt find any interrupts in 8996.dtsi

Quoted 10 lines

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 114

Quoted 6 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +5 -0
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-sdm845.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>

It would be good to include <dt-bindings/interrupt-controller/irq.h> here too, just in case someone wants to move that include out of arm-gic.h, which is possible.

Quoted 2 lines
Quoted 1 line

+    usb_1: usb@a6f8800 {

Can we drop the phandle? It's not used.

will do in next version

Quoted 15 lines
Quoted 4 lines

+        compatible = "qcom,sdm845-dwc3", "qcom,dwc3"; +        reg = <0 0x0a6f8800 0 0x400>;

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +1 -0
+

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 128

Quoted 10 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +10 -0
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        clocks = <&gcc GCC_CFG_NOC_USB3_PRIM_AXI_CLK>,
+                 <&gcc GCC_USB30_PRIM_MASTER_CLK>,
+                 <&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>,
+                 <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
+                 <&gcc GCC_USB30_PRIM_SLEEP_CLK>;
+        clock-names = "cfg_noc", "core", "iface", "mock_utmi",
+                        "sleep";

Spacing looks off. Are there tabs?

will correct in next version

Quoted 13 lines

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 139

Quoted 12 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +11 -0
+
+        assigned-clocks = <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
+                          <&gcc GCC_USB30_PRIM_MASTER_CLK>;
+        assigned-clock-rates = <19200000>, <150000000>;
+
+        interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 486 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 488 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 489 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "hs_phy_irq", "ss_phy_irq",
+                              "dm_hs_phy_irq", "dp_hs_phy_irq";

Same spacing nit

will correct in next version

Quoted 8 lines

Documentation/devicetree/bindings/usb/qcom,dwc3.yaml line 145

Quoted 7 lines
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml new file +6 -0
+
+            power-domains = <&gcc USB30_PRIM_GDSC>;
+
+            resets = <&gcc GCC_USB30_PRIM_BCR>;
+
+            usb_1_dwc3: dwc3@a600000 {

Drop this phandle too? It isn't used.

will correct in next version

Quoted 11 lines
Quoted 10 lines

+                compatible = "snps,dwc3";
+                reg = <0 0x0a600000 0 0xcd00>;
+                interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>; +                iommus = <&apps_smmu 0x740 0>;
+                snps,dis_u2_susphy_quirk;
+                snps,dis_enblslpm_quirk;
+                phys = <&usb_1_hsphy>, <&usb_1_ssphy>; +                phy-names = "usb2-phy", "usb3-phy";
+            };

[1] https://lkml.kernel.org/r/20191218221310.GA4624@bogus

Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
#7 lore raw

PATCH v4 2/2 dt-bindings: usb: qcom,dwc3: Add compatible for SC7180

In reply to Sandeep Maheswaram

Add compatible for SC7180 in usb dwc3 bindings.

---
 Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml +1 -0
index 0353401..4a36ab5 100644
@@ -15,6 +15,7 @@ properties:
       - enum:
           - qcom,msm8996-dwc3
           - qcom,msm8998-dwc3
+          - qcom,sc7180-dwc3
           - qcom,sdm845-dwc3
       - const: qcom,dwc3
 
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation