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

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

[Sandeep Maheswaram]

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

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         | 154 +++++++++++++++++++++
 2 files changed, 154 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 v3 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         | 153 +++++++++++++++++++++
 2 files changed, 153 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..c8eda58
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -0,0 +1,153 @@
+# 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.
+    minItems: 3
+    items:
+      - description: System Config NOC clock. Not present on "qcom,msm8996-dwc3" compatible.
+      - description: Master/Core clock, have to be >= 125 MHz for SS operation and >= 60MHz for HS operation
+      - description: System bus AXI clock. Not present on "qcom,msm8996-dwc3" compatible.
+      - 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:
+    minItems: 3
+    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:
+    description:
+      Should be 19.2MHz (19200000) for MOCK_UTMI_CLK
+      >=125MHz (125000000) for MASTER_CLK in SS mode
+      >=60MHz (60000000) for MASTER_CLK in HS mode
+    maxItems: 2
+
+  resets:
+    maxItems: 1
+
+  interrupts:
+    description:
+      Specifies interrupts from controller wrapper used
+      to wakeup from low power/suspend state. Must contain
+      one or more entry for interrupt-names property.
+    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:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    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.
+
+# 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
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+  - power-domains
+  - clocks
+  - clock-names
+
+examples:
+  - |
+        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>;
+
+            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";
+            };
+        };
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v3 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>
---
 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 c8eda58..6bf057a 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 v3 1/2] dt-bindings: usb: qcom,dwc3: Convert USB DWC3 bindings

[Stephen Boyd]

Quoting Sandeep Maheswaram (2019-12-15 21:39:01)
> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> new file mode 100644
> index 0000000..c8eda58
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> @@ -0,0 +1,153 @@
> +# 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 ]

Hm... ok. Interesting.

> +
> +  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.
> +    minItems: 3
> +    items:
> +      - description: System Config NOC clock. Not present on "qcom,msm8996-dwc3" compatible.
> +      - description: Master/Core clock, have to be >= 125 MHz for SS operation and >= 60MHz for HS operation
> +      - description: System bus AXI clock. Not present on "qcom,msm8996-dwc3" compatible.
> +      - 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:
> +    minItems: 3
> +    items:
> +      - const: cfg_noc
> +      - const: core
> +      - const: iface
> +      - const: mock_utmi
> +      - const: sleep

Order matters. Can 'core' and 'iface' come first and then the others
after that? Same comment applies to clocks property.

> +
> +  assigned-clocks:
> +    items:
> +      - description: Phandle to MOCK_UTMI_CLK.
> +      - description: Phandle to MASTER_CLK.
> +
> +  assigned-clock-rates:
> +    description:
> +      Should be 19.2MHz (19200000) for MOCK_UTMI_CLK
> +      >=125MHz (125000000) for MASTER_CLK in SS mode
> +      >=60MHz (60000000) for MASTER_CLK in HS mode
> +    maxItems: 2
> +
> +  resets:
> +    maxItems: 1
> +
> +  interrupts:
> +    description:
> +      Specifies interrupts from controller wrapper used
> +      to wakeup from low power/suspend state. Must contain
> +      one or more entry for interrupt-names property.
> +    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:
> +    $ref: /schemas/types.yaml#/definitions/string-array
> +    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:

Don't these multi-line descriptions need a pipe, | ?

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

Are 'interrupts' required?

> +
> +examples:
> +  - |
> +        usb3_0: usb30@a6f8800 {

Should node name be something like 'usb3'? Or is this usb 3.0 so it's
'usb30'?

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

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

[Stephen Boyd]

Quoting Sandeep Maheswaram (2019-12-15 21:39:02)
> 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>

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

[Doug Anderson]

Hi,

On Sun, Dec 15, 2019 at 9:40 PM Sandeep Maheswaram <sanm@codeaurora.org> wrote:
>
> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> new file mode 100644
> index 0000000..c8eda58
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> @@ -0,0 +1,153 @@
> +# 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.
> +    minItems: 3

Actually, maybe the best way to express the min/max is to match what
'gpu/arm,mali-midgard.yaml' does.  Specifically later down in the
bindings you can amend this, like this I think:

allOf:
- if:
    properties:
      compatible:
        contains:
          const: "qcom,msm8996-dwc3"
  then:
    properties:
      clocks:
        minItems: 3
        maxItems: 3
      clock-names:
        minItems: 3
        maxItems: 3

...then remove minItems from here.  Now you'll have the default
min/max of 5 for most devices but for the special case of
"qcom,msm8996-dwc3" you'll have min/max of 3.


> +    items:
> +      - description: System Config NOC clock. Not present on "qcom,msm8996-dwc3" compatible.
> +      - description: Master/Core clock, have to be >= 125 MHz for SS operation and >= 60MHz for HS operation

To make the grammer gooder, s/have/has/


> +      - description: System bus AXI clock. Not present on "qcom,msm8996-dwc3" compatible.
> +      - 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).

* Please word wrap to ~80 chracters.
* As Stephen says, order matters.  Please match order of old bindings
(and in clock-names)
* Please end each with a period.


> +  clock-names:
> +    minItems: 3
> +    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:
> +    description:
> +      Should be 19.2MHz (19200000) for MOCK_UTMI_CLK
> +      >=125MHz (125000000) for MASTER_CLK in SS mode
> +      >=60MHz (60000000) for MASTER_CLK in HS mode
> +    maxItems: 2

You can still express some limits here even if we don't go all out
with the "oneOf".  AKA I think this is better:

assigned-clock-rates:
  items:
    - const: 19200000
    - minimum: 60000000
      description: >= 60 MHz in HS mode, >= 125 MHz in SS mode


> +  resets:
> +    maxItems: 1
> +
> +  interrupts:
> +    description:
> +      Specifies interrupts from controller wrapper used
> +      to wakeup from low power/suspend state. Must contain
> +      one or more entry for interrupt-names property.

Now that sub-items have a description this top level description
doesn't add anything.  Delete.


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

Word wrap please.


> +      - description: Wakeup event on DM line.
> +      - description: Wakeup event on DP line.
> +
> +  interrupt-names:
> +    $ref: /schemas/types.yaml#/definitions/string-array

As Rob said in response to your previous version: "Already has a type,
don't need."


> +    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.
> +
> +# 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
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +  - power-domains
> +  - clocks
> +  - clock-names
> +
> +examples:
> +  - |
> +        usb3_0: usb30@a6f8800 {
> +            compatible = "qcom,dwc3";

Your example is missing the SoC-specific compatible string, so it
fails your own schema.


> +            reg = <0xa6f8800 0x400>;
> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +            ranges;
> +            interrupts = <0 131 0>, <0 486 0>, <0 488 0>, <0 489 0>;

In general I believe "0" is frowned upon for IRQ flags.  Your example
should probably be this from sdm845:

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

...and you'll likely need a #include in the example.  See below.


> +            interrupt-names = "hs_phy_irq", "ss_phy_irq",
> +                    "dm_hs_phy_irq", "dp_hs_phy_irq";
> +
> +            clocks = <&gcc GCC_USB30_PRIM_MASTER_CLK>,

As Rob requested in the previous version, please run:

make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/usb/qcom,dwc3.yaml

If the next version doesn't pass "dt_binding_check" I suspect that
people will stop reviewing your change, so please make sure you've
figured out how to do this.

When you do that, you'll see a syntax error here because
"GCC_USB30_PRIM_MASTER_CLK" isn't defined anywhere in your example.
You need to include it.  AKA this should be at the top of your
example:

#include <dt-bindings/clock/qcom,gcc-sdm845.h>

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

[Doug Anderson]

Hi,

On Mon, Dec 16, 2019 at 10:09 AM Stephen Boyd <swboyd@chromium.org> wrote:
>
> > +  "#address-cells":
> > +    enum: [ 1, 2 ]
> > +
> > +  "#size-cells":
> > +    enum: [ 1, 2 ]
>
> Hm... ok. Interesting.

Use of enum seems to match 'timer/arm,arch_timer_mmio.yaml'.  ...and
sub-device probably uses DMA so IIUC it's important to pass
#size-cells of 2 down to it if the parent had it.


> > +  qcom,select-utmi-as-pipe-clk:
> > +    description:
>
> Don't these multi-line descriptions need a pipe, | ?

The pipe just means that carriage returns are important.  They aren't
here, so I think it's OK/better w/out it.  The example-schema.yaml has
many without it.


> > +        usb3_0: usb30@a6f8800 {
>
> Should node name be something like 'usb3'? Or is this usb 3.0 so it's
> 'usb30'?

Probably should be just 'usb@' as per 'usb/usb-hcd.yaml'.

-Doug

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

[Doug Anderson]

Hi,

On Wed, Dec 18, 2019 at 4:41 AM Sandeep Maheswaram (Temp)
<sanm@codeaurora.org> wrote:
> +  assigned-clock-rates:
> +    description:
> +      Should be 19.2MHz (19200000) for MOCK_UTMI_CLK
> +      >=125MHz (125000000) for MASTER_CLK in SS mode
> +      >=60MHz (60000000) for MASTER_CLK in HS mode
> +    maxItems: 2
>
> You can still express some limits here even if we don't go all out
> with the "oneOf".  AKA I think this is better:
>
> assigned-clock-rates:
>   items:
>     - const: 19200000
>     - minimum: 60000000
>       description: >= 60 MHz in HS mode, >= 125 MHz in SS mode
>
> Facing error when i add as above.
> properties:assigned-clock-rates: {'items': [{'const': 19200000}, {'minimum': 60000000}]} is not valid under any of the given schemas
> Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/usb/qcom,dwc3.example.dts' failed

I couldn't figure it out either.  ...but at least this seemed to work
and is slightly better than what you have:

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

So I'd say go with that.


-Doug

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

[Doug Anderson]

Hi,

On Wed, Dec 18, 2019 at 4:48 AM Sandeep Maheswaram (Temp)
<sanm@codeaurora.org> wrote:
>
> +  "#address-cells":
> +    enum: [ 1, 2 ]
> +
> +  "#size-cells":
> +    enum: [ 1, 2 ]
>
> Hm... ok. Interesting.
>
> Use of enum seems to match 'timer/arm,arch_timer_mmio.yaml'.  ...and
> sub-device probably uses DMA so IIUC it's important to pass
> #size-cells of 2 down to it if the parent had it.
>
> Should i mention this as below?
>
>  "#address-cells":
>     const: 2
>
>   "#size-cells":
>     const: 2

No, keep it like you have unless Rob disagrees.  If the parent is only
32-bits it should be fine to keep it.

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

[Rob Herring]

On Wed, Dec 18, 2019 at 10:37:53AM -0800, Doug Anderson wrote:
> Hi,
> 
> On Wed, Dec 18, 2019 at 4:41 AM Sandeep Maheswaram (Temp)
> <sanm@codeaurora.org> wrote:
> > +  assigned-clock-rates:
> > +    description:
> > +      Should be 19.2MHz (19200000) for MOCK_UTMI_CLK
> > +      >=125MHz (125000000) for MASTER_CLK in SS mode
> > +      >=60MHz (60000000) for MASTER_CLK in HS mode
> > +    maxItems: 2
> >
> > You can still express some limits here even if we don't go all out
> > with the "oneOf".  AKA I think this is better:
> >
> > assigned-clock-rates:
> >   items:
> >     - const: 19200000
> >     - minimum: 60000000
> >       description: >= 60 MHz in HS mode, >= 125 MHz in SS mode
> >
> > Facing error when i add as above.
> > properties:assigned-clock-rates: {'items': [{'const': 19200000}, {'minimum': 60000000}]} is not valid under any of the given schemas
> > Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/usb/qcom,dwc3.example.dts' failed

Update dtschema and you should get a better error message now.

I believe the problem is we require both minimum and maximum to be 
specified or none. Maybe we should relax that, but OTOH I doubt 4GHz is 
valid.

Rob

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

[Sandeep Maheswaram (Temp)]

Hi

On 12/18/2019 12:44 AM, Doug Anderson wrote:
> Hi,
>
> On Sun, Dec 15, 2019 at 9:40 PM Sandeep Maheswaram <sanm@codeaurora.org> wrote:
>>
>>
>> +    items:
>> +      - description: System Config NOC clock. Not present on "qcom,msm8996-dwc3" compatible.
>> +      - description: Master/Core clock, have to be >= 125 MHz for SS operation and >= 60MHz for HS operation
> To make the grammer gooder, s/have/has/
>
>
>> +      - description: System bus AXI clock. Not present on "qcom,msm8996-dwc3" compatible.
>> +      - 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).
> * Please word wrap to ~80 chracters.
> * As Stephen says, order matters.  Please match order of old bindings
> (and in clock-names)
> * Please end each with a period.

Regarding the order of clocks, If I change the order  I am facing error 
in make dtbs_check

linux-next/arch/arm64/boot/dts/qcom/sc7180-idp.dt.yaml: usb@a6f8800: 
clock-names:0: 'core' was expected
linux-next/arch/arm64/boot/dts/qcom/sc7180-idp.dt.yaml: usb@a6f8800: 
clock-names:1: 'mock_utmi' was expected
linux-next/arch/arm64/boot/dts/qcom/sc7180-idp.dt.yaml: usb@a6f8800: 
clock-names:2: 'sleep' was expected
linux-next/arch/arm64/boot/dts/qcom/sc7180-idp.dt.yaml: usb@a6f8800: 
clock-names:3: 'cfg_noc' was expected
linux-next/arch/arm64/boot/dts/qcom/sc7180-idp.dt.yaml: usb@a6f8800: 
clock-names:4: 'iface' was expected

Need to modify in the sc7180.dtsi and sdm845.dtsi  to avoid the error  . 
Please let me know how to proceed.

>
>
>> +  clock-names:
>> +    minItems: 3
>> +    items:
>> +      - const: cfg_noc
>> +      - const: core
>> +      - const: iface
>> +      - const: mock_utmi
>> +      - const: sleep
>> +
>>
>
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

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

[Doug Anderson]

Hi,

On Wed, Dec 25, 2019 at 9:56 PM Sandeep Maheswaram (Temp)
<sanm@codeaurora.org> wrote:
>
> Hi
>
> On 12/18/2019 12:44 AM, Doug Anderson wrote:
> > Hi,
> >
> > On Sun, Dec 15, 2019 at 9:40 PM Sandeep Maheswaram <sanm@codeaurora.org> wrote:
> >>
> >>
> >> +    items:
> >> +      - description: System Config NOC clock. Not present on "qcom,msm8996-dwc3" compatible.
> >> +      - description: Master/Core clock, have to be >= 125 MHz for SS operation and >= 60MHz for HS operation
> > To make the grammer gooder, s/have/has/
> >
> >
> >> +      - description: System bus AXI clock. Not present on "qcom,msm8996-dwc3" compatible.
> >> +      - 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).
> > * Please word wrap to ~80 chracters.
> > * As Stephen says, order matters.  Please match order of old bindings
> > (and in clock-names)
> > * Please end each with a period.
>
> Regarding the order of clocks, If I change the order  I am facing error
> in make dtbs_check
>
> linux-next/arch/arm64/boot/dts/qcom/sc7180-idp.dt.yaml: usb@a6f8800:
> clock-names:0: 'core' was expected
> linux-next/arch/arm64/boot/dts/qcom/sc7180-idp.dt.yaml: usb@a6f8800:
> clock-names:1: 'mock_utmi' was expected
> linux-next/arch/arm64/boot/dts/qcom/sc7180-idp.dt.yaml: usb@a6f8800:
> clock-names:2: 'sleep' was expected
> linux-next/arch/arm64/boot/dts/qcom/sc7180-idp.dt.yaml: usb@a6f8800:
> clock-names:3: 'cfg_noc' was expected
> linux-next/arch/arm64/boot/dts/qcom/sc7180-idp.dt.yaml: usb@a6f8800:
> clock-names:4: 'iface' was expected
>
> Need to modify in the sc7180.dtsi and sdm845.dtsi  to avoid the error  .
> Please let me know how to proceed.

Ick.  OK, so I guess technically the old bindings didn't specify
order.  It seems like it was implied that the required clocks should
come before the optional, but I guess it wasn't explicit...

Ugh, it gets even worse, at least for msm8996.  It looks like things
are nicely consistent for all the new ones.  They _all_ do:

  "cfg_noc", "core", "iface", "mock_utmi", "sleep";

...but msm8996 is a mess.  It looks to have two USB ports (usb2 and
usb3).  Both ports have exactly the same compatible string listed, AKA
"qcom,msm8996-dwc3", "qcom,dwc3".  ...but we don't seem to use
"clock-names" for either port and things seem jumbled.  Specifically:

* USB3 port: actually has 6 clocks listed instead of 5 (but, on the
plus side, the first 5 match everyone else in ordering).  The 6th
clock seems to be an extra PHY clock listed which seems wrong.

* USB2 port: has 5 clocks.  Missing "iface" but has the extra "PHY"
clock.  AKA listed order is: "cfg_noc", "core", "mock_utmi", "sleep",
extra PHY clock.


If I had to make a decision, I'd do this:

* For 8996, document exactly what's in the device tree.

* For everything except 8996, use the order that your current patch
has (AKA also document what's in device trees).

* Consider seeing if we can remove the "PHY" clock from the 8996
device tree.  If you can, make it optional (but deprecated) to supply
it.


Probably something like this (UNTESTED) would work.  Optionally you
could figure out how to be smarter and list 6 clocks for the PHY with
the node name "usb@6af8800" and 5 clocks for the PHY with the node
name "usb@76f8800", but I'm not sure it's worth it.


allOf:
- if:
  properties:
    compatible:
      contains:
        const: "qcom,msm8996-dwc3"
  then:
    anyOf:
    - properties:
        # For USB 3 port with node name usb@6af8800
        clocks:
          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).
           - description: PHY clock.  ((Maybe optional?  If so, set
minItems to 5))
   - properties:
        # For USB 2 port with node name usb@76f8800
       clocks:
          items:
           - description: System Config NOC clock.
           - description: Master/Core clock, has to be >= 60MHz.
           - 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).
           - description: PHY clock.  ((Maybe optional?  If so, set
minItems to 4))
 else:
    properties:
      clocks:
        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

NOTE: looking at the current Linux driver, it is very simplistic and
just enables all listed clocks in bulk.  It never refers to clocks by
name.

-Doug