diff mbox series

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

Message ID 20230118193056.673514-2-robh@kernel.org (mailing list archive)
State Superseded
Headers show
Series [v2,1/2] dt-bindings: usb: snps,dwc3: Allow power-domains property | expand

Commit Message

Rob Herring (Arm) Jan. 18, 2023, 7:30 p.m. UTC 
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

Comments

Johan Jonker Jan. 18, 2023, 9:05 p.m. UTC | #1 
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>;
> +            };
> +        };
> +    };
> +...
Rob Herring (Arm) Jan. 20, 2023, 8:30 p.m. UTC | #2 
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
Johan Jonker Jan. 20, 2023, 11:40 p.m. UTC | #3 
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
Rob Herring (Arm) Jan. 23, 2023, 9:07 p.m. UTC | #4 
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
diff mbox series

Patch

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