[PATCH v2 0/3] Add USB DWC3 support for SC7180

[PATCH v2 0/3] Add USB DWC3 support for SC7180

[Sandeep Maheswaram]

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

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

Sandeep Maheswaram (3):
  usb: dwc3: Add support for SC7180 SOC
  dt-bindings: usb: qcom,dwc3: Convert USB DWC3 bindings to yaml
  dt-bindings: usb: qcom,dwc3: Add compatible for SC7180

 .../devicetree/bindings/usb/qcom,dwc3.txt          | 104 --------------
 .../devicetree/bindings/usb/qcom,dwc3.yaml         | 156 +++++++++++++++++++++
 drivers/usb/dwc3/dwc3-qcom.c                       |   3 +-
 3 files changed, 158 insertions(+), 105 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 v2 1/3] usb: dwc3: Add support for SC7180 SOC

[Sandeep Maheswaram]

Add compatible for SC7180 SOC in USB DWC3 driver

Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
---
 drivers/usb/dwc3/dwc3-qcom.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 261af9e..1df2372 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
-/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
+/* Copyright (c) 2018-2019, The Linux Foundation. All rights reserved.
  *
  * Inspired by dwc3-of-simple.c
  */
@@ -753,6 +753,7 @@ static const struct of_device_id dwc3_qcom_of_match[] = {
 	{ .compatible = "qcom,dwc3" },
 	{ .compatible = "qcom,msm8996-dwc3" },
 	{ .compatible = "qcom,msm8998-dwc3" },
+	{ .compatible = "qcom,sc7180-dwc3" },
 	{ .compatible = "qcom,sdm845-dwc3" },
 	{ }
 };
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v2 1/3] usb: dwc3: Add support for SC7180 SOC

[Doug Anderson]

Hi,

On Thu, Nov 28, 2019 at 3:35 AM Sandeep Maheswaram <sanm@codeaurora.org> wrote:
>
> Add compatible for SC7180 SOC in USB DWC3 driver
>
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 261af9e..1df2372 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -1,5 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0
> -/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
> +/* Copyright (c) 2018-2019, The Linux Foundation. All rights reserved.
>   *
>   * Inspired by dwc3-of-simple.c
>   */
> @@ -753,6 +753,7 @@ static const struct of_device_id dwc3_qcom_of_match[] = {
>         { .compatible = "qcom,dwc3" },
>         { .compatible = "qcom,msm8996-dwc3" },
>         { .compatible = "qcom,msm8998-dwc3" },
> +       { .compatible = "qcom,sc7180-dwc3" },
>         { .compatible = "qcom,sdm845-dwc3" },

It is, of course, up to Felipe.  ...but in my opinion this is the
wrong change and instead we should be deleting the SoC-specific
strings (msm8996, msm8998, sdm845) from this file because they don't
buy us anything.  To explain how it works:

1. Device tree should have both the "SoC-specific" and generic
"qcom,dwc3" strings.  Only the "qcom,dwc3" will actually be used but
the SoC-specific string is there so if we find a case later where we
need to handle a SoC-specific quirk then it'll already be there.

2. Bindings should have both the "SoC-specific" and generic
"qcom,dwc3" strings.  The binding is describing what's in the device
tree.

3. Until we have a SoC-specific quirk to handle, we _don't_ need to
add the SoC-specific string to the driver itself.


-Doug

Re: [PATCH v2 1/3] usb: dwc3: Add support for SC7180 SOC

[Sandeep Maheswaram (Temp)]

Hi,

On 12/12/2019 1:13 AM, Doug Anderson wrote:
> Hi,
>
> On Thu, Nov 28, 2019 at 3:35 AM Sandeep Maheswaram <sanm@codeaurora.org> wrote:
>> Add compatible for SC7180 SOC in USB DWC3 driver
>>
>> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
>> ---
>>   drivers/usb/dwc3/dwc3-qcom.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>> index 261af9e..1df2372 100644
>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>> @@ -1,5 +1,5 @@
>>   // SPDX-License-Identifier: GPL-2.0
>> -/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
>> +/* Copyright (c) 2018-2019, The Linux Foundation. All rights reserved.
>>    *
>>    * Inspired by dwc3-of-simple.c
>>    */
>> @@ -753,6 +753,7 @@ static const struct of_device_id dwc3_qcom_of_match[] = {
>>          { .compatible = "qcom,dwc3" },
>>          { .compatible = "qcom,msm8996-dwc3" },
>>          { .compatible = "qcom,msm8998-dwc3" },
>> +       { .compatible = "qcom,sc7180-dwc3" },
>>          { .compatible = "qcom,sdm845-dwc3" },
> It is, of course, up to Felipe.  ...but in my opinion this is the
> wrong change and instead we should be deleting the SoC-specific
> strings (msm8996, msm8998, sdm845) from this file because they don't
> buy us anything.  To explain how it works:
>
> 1. Device tree should have both the "SoC-specific" and generic
> "qcom,dwc3" strings.  Only the "qcom,dwc3" will actually be used but
> the SoC-specific string is there so if we find a case later where we
> need to handle a SoC-specific quirk then it'll already be there.
>
> 2. Bindings should have both the "SoC-specific" and generic
> "qcom,dwc3" strings.  The binding is describing what's in the device
> tree.
>
> 3. Until we have a SoC-specific quirk to handle, we _don't_ need to
> add the SoC-specific string to the driver itself.
>
>
> -Doug
>
Can i remove this patch { .compatible = "qcom,sc7180-dwc3" }, in the 
next version of this series ?

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

Re: [PATCH v2 1/3] usb: dwc3: Add support for SC7180 SOC

[Doug Anderson]

Hi,

On Thu, Dec 12, 2019 at 4:00 AM Sandeep Maheswaram (Temp)
<sanm@codeaurora.org> wrote:
>
> Hi,
>
> On 12/12/2019 1:13 AM, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Nov 28, 2019 at 3:35 AM Sandeep Maheswaram <sanm@codeaurora.org> wrote:
> >> Add compatible for SC7180 SOC in USB DWC3 driver
> >>
> >> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> >> ---
> >>   drivers/usb/dwc3/dwc3-qcom.c | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> >> index 261af9e..1df2372 100644
> >> --- a/drivers/usb/dwc3/dwc3-qcom.c
> >> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> >> @@ -1,5 +1,5 @@
> >>   // SPDX-License-Identifier: GPL-2.0
> >> -/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
> >> +/* Copyright (c) 2018-2019, The Linux Foundation. All rights reserved.
> >>    *
> >>    * Inspired by dwc3-of-simple.c
> >>    */
> >> @@ -753,6 +753,7 @@ static const struct of_device_id dwc3_qcom_of_match[] = {
> >>          { .compatible = "qcom,dwc3" },
> >>          { .compatible = "qcom,msm8996-dwc3" },
> >>          { .compatible = "qcom,msm8998-dwc3" },
> >> +       { .compatible = "qcom,sc7180-dwc3" },
> >>          { .compatible = "qcom,sdm845-dwc3" },
> > It is, of course, up to Felipe.  ...but in my opinion this is the
> > wrong change and instead we should be deleting the SoC-specific
> > strings (msm8996, msm8998, sdm845) from this file because they don't
> > buy us anything.  To explain how it works:
> >
> > 1. Device tree should have both the "SoC-specific" and generic
> > "qcom,dwc3" strings.  Only the "qcom,dwc3" will actually be used but
> > the SoC-specific string is there so if we find a case later where we
> > need to handle a SoC-specific quirk then it'll already be there.
> >
> > 2. Bindings should have both the "SoC-specific" and generic
> > "qcom,dwc3" strings.  The binding is describing what's in the device
> > tree.
> >
> > 3. Until we have a SoC-specific quirk to handle, we _don't_ need to
> > add the SoC-specific string to the driver itself.
> >
> >
> > -Doug
> >
> Can i remove this patch { .compatible = "qcom,sc7180-dwc3" }, in the
> next version of this series ?

Yeah, drop patch #1 from your series.  I've helped you out and posted:

https://lore.kernel.org/r/20191212132122.1.I85a23bdcff04dbce48cc46ddb8f1ffe7a51015eb@changeid

-Doug

[PATCH v2 2/3] 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..48ff9c5
--- /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
+
+  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.
+    maxItems: 5
+
+  clock-names:
+    maxItems: 5
+    items:
+    #Master/Core clock, have to be >= 125 MHz for SS operation
+    # and >= 60MHz for HS operation
+      - const: core
+    #Mock utmi clock needed for ITP/SOF generation in host mode.
+    #Its frequency should be 19.2MHz.
+      - const: mock_utmi
+    #Sleep clock, used for wakeup when USB3 core goes into low power mode (U3).
+      - const: sleep
+    #Optional:System bus AXI clock.Not present on "qcom,msm8996-dwc3" compatible.
+      - const: iface
+    #Optional:System Config NOC clock.Not present on "qcom,msm8996-dwc3" compatible.
+      - const: cfg_noc
+
+  assigned-clocks:
+    description:
+      Should be MOCK_UTMI_CLK and MASTER_CLK
+    maxItems: 2
+
+  assigned-clock-rates:
+    description:
+      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
+    maxItems: 2
+
+  resets:
+    maxItems: 1
+    description: Phandle to reset control that resets core and wrapper.
+
+  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
+
+  interrupt-names:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    items:
+      #The interrupt that is asserted when a wakeup event
+      #is received on USB2 bus
+      - const: hs_phy_irq
+      #The interrupt that is asserted when a wakeup event
+      #is received on USB3 bus
+      - const: ss_phy_irq
+      #Separate interrupts for any wakeup event on DM and DP lines
+      - 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:
+#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
+
+required:
+  - compatible
+  - reg
+  - power-domains
+  - clocks
+  - clock-names
+  - assigned-clocks
+  - assigned-clock-rates
+
+examples:
+  - |
+    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";
+            };
+        };
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

[Doug Anderson]

Hi,

This is my first time reviewing a yaml conversion, so if I say
something that seems wrong it might very well be.  That being said...

On Thu, Nov 28, 2019 at 3:33 AM 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..48ff9c5
> --- /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>

Really weird that you list him as the maintainer but don't CC him on
this email.  Is he still the maintainer?


> +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
> +
> +  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.
> +    maxItems: 5

Add: "minItems: 3"


> +  clock-names:
> +    maxItems: 5

Remove "maxItems: 5".  Having maxItems exactly equal to the number of
items listed is the default.

Add "minItems: 3"


> +    items:
> +    #Master/Core clock, have to be >= 125 MHz for SS operation
> +    # and >= 60MHz for HS operation

Rather than add these as comments, I think you can add them as
descriptions up under the "clocks" section.  Look at
"Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml" as an
example.  Then you can also get rid of "maxItems: 5" in the "clocks"
section".


> +      - const: core
> +    #Mock utmi clock needed for ITP/SOF generation in host mode.
> +    #Its frequency should be 19.2MHz.
> +      - const: mock_utmi
> +    #Sleep clock, used for wakeup when USB3 core goes into low power mode (U3).
> +      - const: sleep
> +    #Optional:System bus AXI clock.Not present on "qcom,msm8996-dwc3" compatible.

spaces after a period.  Also: once you move this up to be in the
"description" of the clock node you can get rid of "Optional:" since
it will be implied by "minItems: 3".


> +      - const: iface
> +    #Optional:System Config NOC clock.Not present on "qcom,msm8996-dwc3" compatible.
> +      - const: cfg_noc
> +
> +  assigned-clocks:
> +    description:
> +      Should be MOCK_UTMI_CLK and MASTER_CLK
> +    maxItems: 2

Instead, maybe:

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


> +  assigned-clock-rates:
> +    description:
> +      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
> +    maxItems: 2

not new for your patch, but nit that "Mhz" should be "MHz".  Also,
maybe the above can be expressed by:

assigned-clock-rates:
  items:
    - const: 192000000
      $ref: "/schemas/types.yaml#/definitions/uint32"
    - oneOf:
        # If SS mode
        - minimum: 125000000
          $ref: "/schemas/types.yaml#/definitions/uint32"
        # If HS mode
       - minimum: 60000000
         $ref: "/schemas/types.yaml#/definitions/uint32"

...it's a little silly to specify that a number has to be either >= 60
MHz or >= 125 MHz but I'm not sure how to express it otherwise...


> +  resets:
> +    maxItems: 1
> +    description: Phandle to reset control that resets core and wrapper.
> +
> +  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

Make the description proper sentences: start with a capital letter,
include a space after your period, and end with a period.


> +  interrupt-names:
> +    $ref: /schemas/types.yaml#/definitions/string-array
> +    items:
> +      #The interrupt that is asserted when a wakeup event
> +      #is received on USB2 bus

Like "clocks", I think these comments can be moved as "description" in
the "interrupts" section.


> +      - const: hs_phy_irq
> +      #The interrupt that is asserted when a wakeup event
> +      #is received on USB3 bus
> +      - const: ss_phy_irq
> +      #Separate interrupts for any wakeup event on DM and DP lines
> +      - 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.

Start description with a capital letter?


> +    type: boolean
> +
> +#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.

Probably we should pick a name and list it as a real thing, not just a
comment.  I think you have "type: object".  So maybe:

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.

Eventually maybe we can include the "dwc3.yaml" file once it's created?


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

Comments usually have a space after the "#".  See example-schema.yaml.
I wonder if this even needs to be here, though.


> +required:
> +  - compatible
> +  - reg
> +  - power-domains
> +  - clocks
> +  - clock-names
> +  - assigned-clocks
> +  - assigned-clock-rates

If I'm reading the old bindings properly "assigned-clocks" and
"assigned-clock-rates" were previously not "required".



> +
> +examples:
> +  - |
> +    hs_phy: phy@100f8800 {
> +            compatible = "qcom,qusb2-v2-phy";
> +            ...
> +        };

Your indentation for this whole example is off.  The braces don't line
up properly.

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

#address-cells, #size-cells, and ranges are not documented in the
schema.  Do they need to be?

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

The 'reset-names' property is not in the schema and it doesn't appear
to be removed.  Remove from the example.


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

-Doug

On Thu, Nov 28, 2019 at 3:33 AM Sandeep Maheswaram <sanm@codeaurora.org> wrote:
>
> 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..48ff9c5
> --- /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
> +
> +  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.
> +    maxItems: 5
> +
> +  clock-names:
> +    maxItems: 5
> +    items:
> +    #Master/Core clock, have to be >= 125 MHz for SS operation
> +    # and >= 60MHz for HS operation
> +      - const: core
> +    #Mock utmi clock needed for ITP/SOF generation in host mode.
> +    #Its frequency should be 19.2MHz.
> +      - const: mock_utmi
> +    #Sleep clock, used for wakeup when USB3 core goes into low power mode (U3).
> +      - const: sleep
> +    #Optional:System bus AXI clock.Not present on "qcom,msm8996-dwc3" compatible.
> +      - const: iface
> +    #Optional:System Config NOC clock.Not present on "qcom,msm8996-dwc3" compatible.
> +      - const: cfg_noc
> +
> +  assigned-clocks:
> +    description:
> +      Should be MOCK_UTMI_CLK and MASTER_CLK
> +    maxItems: 2
> +
> +  assigned-clock-rates:
> +    description:
> +      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
> +    maxItems: 2
> +
> +  resets:
> +    maxItems: 1
> +    description: Phandle to reset control that resets core and wrapper.
> +
> +  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
> +
> +  interrupt-names:
> +    $ref: /schemas/types.yaml#/definitions/string-array
> +    items:
> +      #The interrupt that is asserted when a wakeup event
> +      #is received on USB2 bus
> +      - const: hs_phy_irq
> +      #The interrupt that is asserted when a wakeup event
> +      #is received on USB3 bus
> +      - const: ss_phy_irq
> +      #Separate interrupts for any wakeup event on DM and DP lines
> +      - 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:
> +#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
> +
> +required:
> +  - compatible
> +  - reg
> +  - power-domains
> +  - clocks
> +  - clock-names
> +  - assigned-clocks
> +  - assigned-clock-rates
> +
> +examples:
> +  - |
> +    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";
> +            };
> +        };
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>

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

[Rob Herring]

On Wed, Dec 11, 2019 at 02:10:02PM -0800, Doug Anderson wrote:
> Hi,
> 
> This is my first time reviewing a yaml conversion, so if I say
> something that seems wrong it might very well be.  That being said...
> 
> On Thu, Nov 28, 2019 at 3:33 AM 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..48ff9c5
> > --- /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>
> 
> Really weird that you list him as the maintainer but don't CC him on
> this email.  Is he still the maintainer?
> 
> 
> > +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
> > +
> > +  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.
> > +    maxItems: 5
> 
> Add: "minItems: 3"
> 
> 
> > +  clock-names:
> > +    maxItems: 5
> 
> Remove "maxItems: 5".  Having maxItems exactly equal to the number of
> items listed is the default.
> 
> Add "minItems: 3"
> 
> 
> > +    items:
> > +    #Master/Core clock, have to be >= 125 MHz for SS operation
> > +    # and >= 60MHz for HS operation
> 
> Rather than add these as comments, I think you can add them as
> descriptions up under the "clocks" section.  Look at
> "Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml" as an
> example.  Then you can also get rid of "maxItems: 5" in the "clocks"
> section".
> 
> 
> > +      - const: core
> > +    #Mock utmi clock needed for ITP/SOF generation in host mode.
> > +    #Its frequency should be 19.2MHz.
> > +      - const: mock_utmi
> > +    #Sleep clock, used for wakeup when USB3 core goes into low power mode (U3).
> > +      - const: sleep
> > +    #Optional:System bus AXI clock.Not present on "qcom,msm8996-dwc3" compatible.
> 
> spaces after a period.  Also: once you move this up to be in the
> "description" of the clock node you can get rid of "Optional:" since
> it will be implied by "minItems: 3".
> 
> 
> > +      - const: iface
> > +    #Optional:System Config NOC clock.Not present on "qcom,msm8996-dwc3" compatible.
> > +      - const: cfg_noc
> > +
> > +  assigned-clocks:
> > +    description:
> > +      Should be MOCK_UTMI_CLK and MASTER_CLK
> > +    maxItems: 2
> 
> Instead, maybe:
> 
> assigned-clocks:
>   items:
>     - description: Phandle to MOCK_UTMI_CLK.
>     - description: Phandle to MASTER_CLK
> 
> 
> > +  assigned-clock-rates:
> > +    description:
> > +      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
> > +    maxItems: 2
> 
> not new for your patch, but nit that "Mhz" should be "MHz".  Also,
> maybe the above can be expressed by:
> 
> assigned-clock-rates:
>   items:
>     - const: 192000000
>       $ref: "/schemas/types.yaml#/definitions/uint32"

Don't need a type here. You can assume common properties have a type 
defined already. Note that the $ref would have to be under an 'allOf' or 
else const is ignored. But this sillyness in json-schema changes in 
draft8, so soon you can forget what I just said.

>     - oneOf:
>         # If SS mode
>         - minimum: 125000000
>           $ref: "/schemas/types.yaml#/definitions/uint32"
>         # If HS mode
>        - minimum: 60000000
>          $ref: "/schemas/types.yaml#/definitions/uint32"
> 
> ...it's a little silly to specify that a number has to be either >= 60
> MHz or >= 125 MHz but I'm not sure how to express it otherwise...

Indeed. Not really any way to handle this, so it's just for 
documentation. I would put a description for each entry.

> 
> > +  resets:
> > +    maxItems: 1
> > +    description: Phandle to reset control that resets core and wrapper.

Pretty generic. Just drop the description.

> > +
> > +  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
> 
> Make the description proper sentences: start with a capital letter,
> include a space after your period, and end with a period.

Needs to define how many and what each one is:

items:
  - description: ...
  - description: ...
> 
> 
> > +  interrupt-names:
> > +    $ref: /schemas/types.yaml#/definitions/string-array

Already has a type, don't need.

> > +    items:
> > +      #The interrupt that is asserted when a wakeup event
> > +      #is received on USB2 bus
> 
> Like "clocks", I think these comments can be moved as "description" in
> the "interrupts" section.

Yes, a per item description.

> > +      - const: hs_phy_irq
> > +      #The interrupt that is asserted when a wakeup event
> > +      #is received on USB3 bus
> > +      - const: ss_phy_irq
> > +      #Separate interrupts for any wakeup event on DM and DP lines
> > +      - 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.
> 
> Start description with a capital letter?
> 
> 
> > +    type: boolean
> > +
> > +#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.
> 
> Probably we should pick a name and list it as a real thing, not just a
> comment.  I think you have "type: object".  So maybe:

Yes.
> 
> 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.
> 
> Eventually maybe we can include the "dwc3.yaml" file once it's created?

Yes.

> 
> 
> > +#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
> 
> Comments usually have a space after the "#".  See example-schema.yaml.
> I wonder if this even needs to be here, though.
> 
> 
> > +required:
> > +  - compatible
> > +  - reg
> > +  - power-domains
> > +  - clocks
> > +  - clock-names
> > +  - assigned-clocks
> > +  - assigned-clock-rates
> 
> If I'm reading the old bindings properly "assigned-clocks" and
> "assigned-clock-rates" were previously not "required".
> 
> 
> 
> > +
> > +examples:
> > +  - |
> > +    hs_phy: phy@100f8800 {
> > +            compatible = "qcom,qusb2-v2-phy";
> > +            ...

This won't compile. Use 'make dt_binding_check'.

> > +        };
> 
> Your indentation for this whole example is off.  The braces don't line
> up properly.
> 
> > +
> > +        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;
> 
> #address-cells, #size-cells, and ranges are not documented in the
> schema.  Do they need to be?

Yes.

> 
> > +
> > +            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";
> 
> The 'reset-names' property is not in the schema and it doesn't appear
> to be removed.  Remove from the example.
> 
> 
> > +            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";
> > +            };
> > +        };

[PATCH v2 3/3] 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>
---
 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 48ff9c5..3eab91a 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 v2 3/3] dt-bindings: usb: qcom,dwc3: Add compatible for SC7180

[Doug Anderson]

Hi,

On Thu, Nov 28, 2019 at 3:33 AM Sandeep Maheswaram <sanm@codeaurora.org> wrote:
>
> Add compatible for SC7180 in usb dwc3 bindings.
>
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

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

[Rob Herring]

On Thu, 28 Nov 2019 17:03:07 +0530, Sandeep Maheswaram wrote:
> Add compatible for SC7180 in usb dwc3 bindings.
> 
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

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