[PATCH v2 1/2] dt-bindings: usb: snps,dwc3: Allow power-domains property

[PATCH v2 1/2] dt-bindings: usb: snps,dwc3: Allow power-domains property

[Rob Herring]

The Rockchip RK3399 DWC3 node has 'power-domains' property which isn't
allowed by the schema:

usb@fe900000: Unevaluated properties are not allowed ('power-domains' was unexpected)

Allow DWC3 nodes to have a power-domains entry. We could instead move
the power-domains property to the parent wrapper node, but the could be
an ABI break (Linux shouldn't care). Also, we don't want to encourage
the pattern of wrapper nodes just to define resources such as clocks,
resets, power-domains, etc. when not necessary.

Signed-off-by: Rob Herring <robh@kernel.org>
---
v2:
 - Allow 2 power domains and explain what they are
---
 Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index 6d78048c4613..be36956af53b 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -91,6 +91,16 @@ properties:
         - usb2-phy
         - usb3-phy
 
+  power-domains:
+    description:
+      The DWC3 has 2 power-domains. The power management unit (PMU) and
+      everything else. The PMU is typically always powered and may not have an
+      entry.
+    minItems: 1
+    items:
+      - description: Core
+      - description: Power management unit
+
   resets:
     minItems: 1
 
-- 
2.39.0

[PATCH v2 2/2] dt-bindings: usb: rockchip,dwc3: Move RK3399 to its own schema

[Rob Herring]

The rockchip,dwc3.yaml schema defines a single DWC3 node, but the RK3399
uses the discouraged parent wrapper node and child 'generic' DWC3 node.
The intent was to modify the RK3399 DTs to use a single node, but the DT
changes were rejected for ABI reasons. However, the schema was accepted
as-is.

To fix this, we need to move the RK3399 binding to its own schema file.
The RK3328 and RK3568 bindings are correct and use a single node.

Cc: Johan Jonker <jbx6244@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../bindings/usb/rockchip,dwc3.yaml           |  10 +-
 .../bindings/usb/rockchip,rk3399-dwc3.yaml    | 115 ++++++++++++++++++
 2 files changed, 119 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml

diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
index b3798d94d2fd..edb130c780e4 100644
--- a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
@@ -29,7 +29,6 @@ select:
       contains:
         enum:
           - rockchip,rk3328-dwc3
-          - rockchip,rk3399-dwc3
           - rockchip,rk3568-dwc3
   required:
     - compatible
@@ -39,7 +38,6 @@ properties:
     items:
       - enum:
           - rockchip,rk3328-dwc3
-          - rockchip,rk3399-dwc3
           - rockchip,rk3568-dwc3
       - const: snps,dwc3
 
@@ -90,7 +88,7 @@ required:
 
 examples:
   - |
-    #include <dt-bindings/clock/rk3399-cru.h>
+    #include <dt-bindings/clock/rk3328-cru.h>
     #include <dt-bindings/interrupt-controller/arm-gic.h>
 
     bus {
@@ -98,11 +96,11 @@ examples:
       #size-cells = <2>;
 
       usbdrd3_0: usb@fe800000 {
-        compatible = "rockchip,rk3399-dwc3", "snps,dwc3";
+        compatible = "rockchip,rk3328-dwc3", "snps,dwc3";
         reg = <0x0 0xfe800000 0x0 0x100000>;
         interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
-        clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
-                 <&cru ACLK_USB3OTG0>, <&cru ACLK_USB3_GRF>;
+        clocks = <&cru SCLK_USB3OTG_REF>, <&cru SCLK_USB3OTG_SUSPEND>,
+                <&cru ACLK_USB3OTG>;
         clock-names = "ref_clk", "suspend_clk",
                       "bus_clk", "grf_clk";
         dr_mode = "otg";
diff --git a/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
new file mode 100644
index 000000000000..e39a8a3a7ab3
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
@@ -0,0 +1,115 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/rockchip,rk3399-dwc3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip RK3399 SuperSpeed DWC3 USB SoC controller
+
+maintainers:
+  - Heiko Stuebner <heiko@sntech.de>
+
+properties:
+  compatible:
+    const: rockchip,rk3399-dwc3
+
+  '#address-cells':
+    const: 2
+
+  '#size-cells':
+    const: 2
+
+  ranges: true
+
+  clocks:
+    items:
+      - description:
+          Controller reference clock, must to be 24 MHz
+      - description:
+          Controller suspend clock, must to be 24 MHz or 32 KHz
+      - description:
+          Master/Core clock, must to be >= 62.5 MHz for SS
+          operation and >= 30MHz for HS operation
+      - description:
+          USB3 aclk peri
+      - description:
+          USB3 aclk
+      - description:
+          Controller grf clock
+
+  clock-names:
+    items:
+      - const: ref_clk
+      - const: suspend_clk
+      - const: bus_clk
+      - const: aclk_usb3_rksoc_axi_perf
+      - const: aclk_usb3
+      - const: grf_clk
+
+  resets:
+    maxItems: 1
+
+  reset-names:
+    const: usb3-otg
+
+patternProperties:
+  '^usb@':
+    $ref: snps,dwc3.yaml#
+
+additionalProperties: false
+
+required:
+  - compatible
+  - '#address-cells'
+  - '#size-cells'
+  - ranges
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+
+examples:
+  - |
+    #include <dt-bindings/clock/rk3399-cru.h>
+    #include <dt-bindings/power/rk3399-power.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    bus {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        usb {
+            compatible = "rockchip,rk3399-dwc3";
+            #address-cells = <2>;
+            #size-cells = <2>;
+            ranges;
+            clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
+              <&cru ACLK_USB3OTG0>, <&cru ACLK_USB3_RKSOC_AXI_PERF>,
+              <&cru ACLK_USB3>, <&cru ACLK_USB3_GRF>;
+            clock-names = "ref_clk", "suspend_clk",
+                    "bus_clk", "aclk_usb3_rksoc_axi_perf",
+                    "aclk_usb3", "grf_clk";
+            resets = <&cru SRST_A_USB3_OTG0>;
+            reset-names = "usb3-otg";
+
+            usb@fe800000 {
+                compatible = "snps,dwc3";
+                reg = <0x0 0xfe800000 0x0 0x100000>;
+                interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
+                clocks = <&cru SCLK_USB3OTG0_REF>, <&cru ACLK_USB3OTG0>,
+                  <&cru SCLK_USB3OTG0_SUSPEND>;
+                clock-names = "ref", "bus_early", "suspend";
+                dr_mode = "otg";
+                phys = <&u2phy0_otg>, <&tcphy0_usb3>;
+                phy-names = "usb2-phy", "usb3-phy";
+                phy_type = "utmi_wide";
+                snps,dis_enblslpm_quirk;
+                snps,dis-u2-freeclk-exists-quirk;
+                snps,dis_u2_susphy_quirk;
+                snps,dis-del-phy-power-chg-quirk;
+                snps,dis-tx-ipgap-linecheck-quirk;
+                power-domains = <&power RK3399_PD_USB3>;
+            };
+        };
+    };
+...
-- 
2.39.0

Re: [PATCH v2 2/2] dt-bindings: usb: rockchip,dwc3: Move RK3399 to its own schema

[Johan Jonker]

Hi,

Some alignment at the examples and the unknown extcon property.

usb@fe800000: 'extcon' does not match any of the regexes

Johan

On 1/18/23 20:30, Rob Herring wrote:
> The rockchip,dwc3.yaml schema defines a single DWC3 node, but the RK3399
> uses the discouraged parent wrapper node and child 'generic' DWC3 node.
> The intent was to modify the RK3399 DTs to use a single node, but the DT
> changes were rejected for ABI reasons. However, the schema was accepted
> as-is.
> 
> To fix this, we need to move the RK3399 binding to its own schema file.
> The RK3328 and RK3568 bindings are correct and use a single node.
> 
> Cc: Johan Jonker <jbx6244@gmail.com>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/usb/rockchip,dwc3.yaml           |  10 +-
>  .../bindings/usb/rockchip,rk3399-dwc3.yaml    | 115 ++++++++++++++++++
>  2 files changed, 119 insertions(+), 6 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> index b3798d94d2fd..edb130c780e4 100644
> --- a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> @@ -29,7 +29,6 @@ select:
>        contains:
>          enum:
>            - rockchip,rk3328-dwc3
> -          - rockchip,rk3399-dwc3
>            - rockchip,rk3568-dwc3
>    required:
>      - compatible
> @@ -39,7 +38,6 @@ properties:
>      items:
>        - enum:
>            - rockchip,rk3328-dwc3
> -          - rockchip,rk3399-dwc3
>            - rockchip,rk3568-dwc3
>        - const: snps,dwc3
>  
> @@ -90,7 +88,7 @@ required:
>  
>  examples:
>    - |
> -    #include <dt-bindings/clock/rk3399-cru.h>
> +    #include <dt-bindings/clock/rk3328-cru.h>
>      #include <dt-bindings/interrupt-controller/arm-gic.h>
>  
>      bus {
> @@ -98,11 +96,11 @@ examples:
>        #size-cells = <2>;
>  
>        usbdrd3_0: usb@fe800000 {
> -        compatible = "rockchip,rk3399-dwc3", "snps,dwc3";
> +        compatible = "rockchip,rk3328-dwc3", "snps,dwc3";
>          reg = <0x0 0xfe800000 0x0 0x100000>;
>          interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
> -        clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
> -                 <&cru ACLK_USB3OTG0>, <&cru ACLK_USB3_GRF>;
> +        clocks = <&cru SCLK_USB3OTG_REF>, <&cru SCLK_USB3OTG_SUSPEND>,

> +                <&cru ACLK_USB3OTG>;

align

>          clock-names = "ref_clk", "suspend_clk",
>                        "bus_clk", "grf_clk";
>          dr_mode = "otg";
> diff --git a/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
> new file mode 100644
> index 000000000000..e39a8a3a7ab3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
> @@ -0,0 +1,115 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/rockchip,rk3399-dwc3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip RK3399 SuperSpeed DWC3 USB SoC controller
> +
> +maintainers:
> +  - Heiko Stuebner <heiko@sntech.de>
> +
> +properties:
> +  compatible:
> +    const: rockchip,rk3399-dwc3
> +
> +  '#address-cells':
> +    const: 2
> +
> +  '#size-cells':
> +    const: 2
> +
> +  ranges: true
> +
> +  clocks:
> +    items:
> +      - description:
> +          Controller reference clock, must to be 24 MHz
> +      - description:
> +          Controller suspend clock, must to be 24 MHz or 32 KHz
> +      - description:
> +          Master/Core clock, must to be >= 62.5 MHz for SS
> +          operation and >= 30MHz for HS operation
> +      - description:
> +          USB3 aclk peri
> +      - description:
> +          USB3 aclk
> +      - description:
> +          Controller grf clock
> +
> +  clock-names:
> +    items:
> +      - const: ref_clk
> +      - const: suspend_clk
> +      - const: bus_clk
> +      - const: aclk_usb3_rksoc_axi_perf
> +      - const: aclk_usb3
> +      - const: grf_clk
> +
> +  resets:
> +    maxItems: 1
> +
> +  reset-names:
> +    const: usb3-otg
> +
> +patternProperties:
> +  '^usb@':
> +    $ref: snps,dwc3.yaml#
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - '#address-cells'
> +  - '#size-cells'
> +  - ranges
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/rk3399-cru.h>
> +    #include <dt-bindings/power/rk3399-power.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    bus {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        usb {
> +            compatible = "rockchip,rk3399-dwc3";
> +            #address-cells = <2>;
> +            #size-cells = <2>;
> +            ranges;
> +            clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,

> +              <&cru ACLK_USB3OTG0>, <&cru ACLK_USB3_RKSOC_AXI_PERF>,
> +              <&cru ACLK_USB3>, <&cru ACLK_USB3_GRF>;

align

> +            clock-names = "ref_clk", "suspend_clk",

> +                    "bus_clk", "aclk_usb3_rksoc_axi_perf",
> +                    "aclk_usb3", "grf_clk";

align

> +            resets = <&cru SRST_A_USB3_OTG0>;
> +            reset-names = "usb3-otg";
> +
> +            usb@fe800000 {
> +                compatible = "snps,dwc3";
> +                reg = <0x0 0xfe800000 0x0 0x100000>;
> +                interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
> +                clocks = <&cru SCLK_USB3OTG0_REF>, <&cru ACLK_USB3OTG0>,

> +                  <&cru SCLK_USB3OTG0_SUSPEND>;

align

> +                clock-names = "ref", "bus_early", "suspend";
> +                dr_mode = "otg";
> +                phys = <&u2phy0_otg>, <&tcphy0_usb3>;
> +                phy-names = "usb2-phy", "usb3-phy";
> +                phy_type = "utmi_wide";
> +                snps,dis_enblslpm_quirk;
> +                snps,dis-u2-freeclk-exists-quirk;
> +                snps,dis_u2_susphy_quirk;
> +                snps,dis-del-phy-power-chg-quirk;
> +                snps,dis-tx-ipgap-linecheck-quirk;
> +                power-domains = <&power RK3399_PD_USB3>;
> +            };
> +        };
> +    };
> +...

Re: [PATCH v2 2/2] dt-bindings: usb: rockchip,dwc3: Move RK3399 to its own schema

[Rob Herring]

On Wed, Jan 18, 2023 at 3:05 PM Johan Jonker <jbx6244@gmail.com> wrote:
>
> Hi,
>
> Some alignment at the examples and the unknown extcon property.
>
> usb@fe800000: 'extcon' does not match any of the regexes

Does that go in the wrapper or dwc3 node?:

arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dtb: usb@fe800000:
usb@fe800000: Unevaluated properties are not allowed ('extcon' was
unexpected)
        From schema:
/home/rob/proj/linux-dt/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml

That's the dwc3 node, but the majority are in the wrapper node, so I'm
going with the majority and leaving this one.

Rob

Re: [PATCH v2 2/2] dt-bindings: usb: rockchip,dwc3: Move RK3399 to its own schema

[Johan Jonker]

On 1/20/23 21:30, Rob Herring wrote:
> On Wed, Jan 18, 2023 at 3:05 PM Johan Jonker <jbx6244@gmail.com> wrote:
>>
>> Hi,
>>
>> Some alignment at the examples and the unknown extcon property.
>>
>> usb@fe800000: 'extcon' does not match any of the regexes
> 
> Does that go in the wrapper or dwc3 node?:
> 
> arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dtb: usb@fe800000:
> usb@fe800000: Unevaluated properties are not allowed ('extcon' was
> unexpected)
>         From schema:
> /home/rob/proj/linux-dt/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
> 


> That's the dwc3 node, but the majority are in the wrapper node, so I'm
> going with the majority and leaving this one.

In wrapper code for rk33899 in dwc3-of-simple.c I don't see no extcon activity I think.

In core there's recently made some changes:
https://github.com/torvalds/linux/blame/master/drivers/usb/dwc3/core.c#L1710

usb: dwc3: Don't switch OTG -> peripheral if extcon is present 
https://lore.kernel.org/all/20221017233510.53336-1-andriy.shevchenko@linux.intel.com/

Binding status update for that is unknown for me.
Do whatever suites you best.

Johan
> Rob

Re: [PATCH v2 2/2] dt-bindings: usb: rockchip,dwc3: Move RK3399 to its own schema

[Rob Herring]

On Sat, Jan 21, 2023 at 12:40:12AM +0100, Johan Jonker wrote:
> 
> 
> On 1/20/23 21:30, Rob Herring wrote:
> > On Wed, Jan 18, 2023 at 3:05 PM Johan Jonker <jbx6244@gmail.com> wrote:
> >>
> >> Hi,
> >>
> >> Some alignment at the examples and the unknown extcon property.
> >>
> >> usb@fe800000: 'extcon' does not match any of the regexes
> > 
> > Does that go in the wrapper or dwc3 node?:
> > 
> > arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dtb: usb@fe800000:
> > usb@fe800000: Unevaluated properties are not allowed ('extcon' was
> > unexpected)
> >         From schema:
> > /home/rob/proj/linux-dt/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
> > 
> 
> 
> > That's the dwc3 node, but the majority are in the wrapper node, so I'm
> > going with the majority and leaving this one.
> 
> In wrapper code for rk33899 in dwc3-of-simple.c I don't see no extcon activity I think.

So all the other cases are just a stray property and the above one is 
correct.

'extcon' should be replaced in favor of a connector node, so if not 
used, it should be able to be removed. Though maybe BSD or something 
else cares.

> In core there's recently made some changes:
> https://github.com/torvalds/linux/blame/master/drivers/usb/dwc3/core.c#L1710
> 
> usb: dwc3: Don't switch OTG -> peripheral if extcon is present 
> https://lore.kernel.org/all/20221017233510.53336-1-andriy.shevchenko@linux.intel.com/
> 
> Binding status update for that is unknown for me.
> Do whatever suites you best.

I'm going to leave it as-is for now.

Rob

Re: [PATCH v2 1/2] dt-bindings: usb: snps,dwc3: Allow power-domains property

[Thinh Nguyen]

On Wed, Jan 18, 2023, Rob Herring wrote:
> The Rockchip RK3399 DWC3 node has 'power-domains' property which isn't
> allowed by the schema:
> 
> usb@fe900000: Unevaluated properties are not allowed ('power-domains' was unexpected)
> 
> Allow DWC3 nodes to have a power-domains entry. We could instead move
> the power-domains property to the parent wrapper node, but the could be
> an ABI break (Linux shouldn't care). Also, we don't want to encourage
> the pattern of wrapper nodes just to define resources such as clocks,
> resets, power-domains, etc. when not necessary.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> v2:
>  - Allow 2 power domains and explain what they are
> ---
>  Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> index 6d78048c4613..be36956af53b 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> @@ -91,6 +91,16 @@ properties:
>          - usb2-phy
>          - usb3-phy
>  
> +  power-domains:
> +    description:
> +      The DWC3 has 2 power-domains. The power management unit (PMU) and
> +      everything else. The PMU is typically always powered and may not have an
> +      entry.
> +    minItems: 1
> +    items:
> +      - description: Core
> +      - description: Power management unit
> +
>    resets:
>      minItems: 1
>  
> -- 
> 2.39.0
> 

Reviewed-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh