[PATCH v4 0/2] Add USB DWC3 support for SC7180

[PATCH v4 0/2] Add USB DWC3 support for SC7180

[Sandeep Maheswaram]

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

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

[Sandeep Maheswaram]

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

Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
---
 .../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

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
deleted file mode 100644
index cb695aa..0000000
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
+++ /dev/null
@@ -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";
-			};
-		};
-
diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
new file mode 100644
index 0000000..0353401
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -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 <mgautam@codeaurora.org>
+
+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.
+    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.
+      - 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
+
+  assigned-clocks:
+    items:
+      - description: Phandle to MOCK_UTMI_CLK.
+      - description: Phandle to MASTER_CLK.
+
+  assigned-clock-rates:
+    items:
+      - description: Must be 19.2MHz (19200000).
+      - description: Must be >= 60 MHz in HS mode, >= 125 MHz in SS mode.
+
+  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:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+  - power-domains
+  - clocks
+  - clock-names
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-sdm845.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    usb_1: usb@a6f8800 {
+        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";
+
+        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";
+
+            power-domains = <&gcc USB30_PRIM_GDSC>;
+
+            resets = <&gcc GCC_USB30_PRIM_BCR>;
+
+            usb_1_dwc3: dwc3@a600000 {
+                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";
+            };
+        };
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

[Sandeep Maheswaram]

Add compatible for SC7180 in usb dwc3 bindings.

Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---
 Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index 0353401..4a36ab5 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -15,6 +15,7 @@ properties:
       - enum:
           - qcom,msm8996-dwc3
           - qcom,msm8998-dwc3
+          - qcom,sc7180-dwc3
           - qcom,sdm845-dwc3
       - const: qcom,dwc3
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

[Stephen Boyd]

Quoting Sandeep Maheswaram (2020-02-09 22:36:44)
> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> new file mode 100644
> index 0000000..0353401
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> @@ -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 <mgautam@codeaurora.org>
> +
> +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.
> +    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.

> +      - 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
> +
> +  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.

> +
> +  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?

> +
> +  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:
> +  - compatible
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +  - power-domains
> +  - clocks
> +  - clock-names

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

> +
> +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.

> +    usb_1: usb@a6f8800 {

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

> +        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?

> +
> +        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

> +
> +            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.

> +                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

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

[Evan Green]

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

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

[Matthias Kaehlcke]

On Fri, Feb 28, 2020 at 03:41:47PM -0800, Evan Green wrote:
> Sandeep, are you going to spin this series?

ping

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

[Sandeep Maheswaram (Temp)]

Hi Stephen,

On 2/11/2020 3:01 AM, Stephen Boyd wrote:
> Quoting Sandeep Maheswaram (2020-02-09 22:36:44)
>> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>> new file mode 100644
>> index 0000000..0353401
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>> @@ -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 <mgautam@codeaurora.org>
>> +
>> +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.
>> +    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
>
>> +      - 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
>> +
>> +  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
>
>> +
>> +  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
>
>> +
>> +  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:
>> +  - 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
>
>> +
>> +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.
>
>> +    usb_1: usb@a6f8800 {
> Can we drop the phandle? It's not used.
will do in next version
>
>> +        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?
will correct in next version
>
>> +
>> +        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
>
>> +
>> +            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
>
>> +                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

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

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

[Sandeep Maheswaram (Temp)]

Hi Stephen,

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

Thanks

Sandeep

On 3/13/2020 5:41 PM, Sandeep Maheswaram (Temp) wrote:
> Hi Stephen,
>
> On 2/11/2020 3:01 AM, Stephen Boyd wrote:
>> Quoting Sandeep Maheswaram (2020-02-09 22:36:44)
>>> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml 
>>> b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>>> new file mode 100644
>>> index 0000000..0353401
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>>> @@ -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 <mgautam@codeaurora.org>
>>> +
>>> +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.
>>> +    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
>>
>>> +      - 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
>>> +
>>> +  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
>>
>>> +
>>> +  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
>>
>>> +
>>> +  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:
>>> +  - 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
>>
>>> +
>>> +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.
>>
>>> +    usb_1: usb@a6f8800 {
>> Can we drop the phandle? It's not used.
> will do in next version
>>
>>> +        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?
> will correct in next version
>>
>>> +
>>> +        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
>>
>>> +
>>> +            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
>>
>>> +                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
>
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation