[PATCH v1 0/2] RK3588 USB3 host controller support

[PATCH v1 0/2] RK3588 USB3 host controller support

[Sebastian Reichel]

Hi,

This adds RK3588 USB3 host controller support. The DT binding is
already prepared for the dual-role controllers, which are also DWC3
based, but using a different PHY and a different set of clocks.

The series has been tested with Radxa Rock 5B, which uses the controller
for the upper USB3 port. The patch enabling &combphy2_psu and &usbhost3_0
for this board will be send separately once this has been merged.

-- Sebastian

Sebastian Reichel (2):
  dt-bindings: usb: rockchip,dwc3: Add RK3588 binding
  arm64: dts: rockchip: rk3588s: Add USB3 host controller

 .../bindings/usb/rockchip,rk3399-dwc3.yaml    | 107 ++++++++++++++----
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi     |  29 +++++
 2 files changed, 114 insertions(+), 22 deletions(-)

-- 
2.40.1

[PATCH v1 1/2] dt-bindings: usb: rockchip,dwc3: Add RK3588 binding

[Sebastian Reichel]

RK3588 contains three DWC3 cores. Two of them are connected to
dedicated USBDP PHY and can be used in dual-role. The third is
connected to one of the shared combo PHYs used for PCIe/SATA/USB3
and can only be used in host mode. Since the binding is all about
the PHY glueing and involved clocks, separate compatible values
have been created for these two types.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 .../bindings/usb/rockchip,rk3399-dwc3.yaml    | 107 ++++++++++++++----
 1 file changed, 85 insertions(+), 22 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
index 3159f9a6a0f7..0db4dc86e506 100644
--- a/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
@@ -11,7 +11,13 @@ maintainers:
 
 properties:
   compatible:
-    const: rockchip,rk3399-dwc3
+    oneOf:
+      - items:
+          - enum:
+              - rockchip,rk3588-dwc3-otg
+              - rockchip,rk3588-dwc3-host
+          - const: rockchip,rk3399-dwc3
+      - const: rockchip,rk3399-dwc3
 
   '#address-cells':
     const: 2
@@ -22,35 +28,37 @@ properties:
   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
+    minItems: 3
+    maxItems: 6
 
   clock-names:
     items:
-      - const: ref_clk
-      - const: suspend_clk
-      - const: bus_clk
-      - const: aclk_usb3_rksoc_axi_perf
-      - const: aclk_usb3
-      - const: grf_clk
+      oneOf:
+        - enum:
+            - ref
+            - ref_clk
+        - enum:
+            - suspend
+            - suspend_clk
+        - enum:
+            - bus
+            - bus_clk
+        - const: aclk_usb3_rksoc_axi_perf
+        - const: aclk_usb3
+        - const: grf_clk
+        - const: utmi
+        - const: php
+        - const: pipe
+    minItems: 3
+    maxItems: 6
 
   resets:
     maxItems: 1
 
   reset-names:
-    const: usb3-otg
+    enum:
+      - usb3-host
+      - usb3-otg
 
 patternProperties:
   '^usb@':
@@ -68,6 +76,61 @@ required:
   - resets
   - reset-names
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          const: rockchip,rk3399-dwc3
+    then:
+      properties:
+        clocks:
+          minItems: 6
+        clock-names:
+          items:
+            - const: ref_clk
+            - const: suspend_clk
+            - const: bus_clk
+            - const: aclk_usb3_rksoc_axi_perf
+            - const: aclk_usb3
+            - const: grf_clk
+        reset-names:
+          const: usb3-otg
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: rockchip,rk3588-dwc3-otg
+    then:
+      properties:
+        clocks:
+          maxItems: 3
+        clock-names:
+          items:
+            - const: ref
+            - const: suspend
+            - const: bus
+        reset-names:
+          const: usb3-otg
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: rockchip,rk3588-dwc3-host
+    then:
+      properties:
+        clocks:
+          minItems: 6
+        clock-names:
+          items:
+            - const: ref
+            - const: suspend
+            - const: bus
+            - const: utmi
+            - const: php
+            - const: pipe
+        reset-names:
+          const: usb3-host
+
 examples:
   - |
     #include <dt-bindings/clock/rk3399-cru.h>
-- 
2.40.1

[PATCH v1 2/2] arm64: dts: rockchip: rk3588s: Add USB3 host controller

[Sebastian Reichel]

RK3588 has three USB3 controllers. One of them is host-only and uses
the naneng-combphy. The other two are dual-role and using a different
PHY that is not yet supported upstream.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 29 +++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index b9b509257aaa..416581dd3bb5 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -443,6 +443,35 @@ usb_host1_ohci: usb@fc8c0000 {
 		status = "disabled";
 	};
 
+	usbhost3_0: usb@fcd00000 {
+		compatible = "rockchip,rk3588-dwc3-host", "rockchip,rk3399-dwc3";
+		clocks = <&cru REF_CLK_USB3OTG2>, <&cru SUSPEND_CLK_USB3OTG2>,
+			 <&cru ACLK_USB3OTG2>, <&cru CLK_UTMI_OTG2>,
+			 <&cru PCLK_PHP_ROOT>, <&cru CLK_PIPEPHY2_PIPE_U3_G>;
+		clock-names = "ref", "suspend", "bus", "utmi", "php", "pipe";
+		ranges;
+		resets = <&cru SRST_A_USB3OTG2>;
+		reset-names = "usb3-host";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		status = "disabled";
+
+		usbhost_dwc3_0: usb@fcd00000 {
+			compatible = "snps,dwc3";
+			reg = <0x0 0xfcd00000 0x0 0x400000>;
+			interrupts = <GIC_SPI 222 IRQ_TYPE_LEVEL_HIGH 0>;
+			dr_mode = "host";
+			phys = <&combphy2_psu PHY_TYPE_USB3>;
+			phy-names = "usb3-phy";
+			phy_type = "utmi_wide";
+			snps,dis_enblslpm_quirk;
+			snps,dis-u2-freeclk-exists-quirk;
+			snps,dis-del-phy-power-chg-quirk;
+			snps,dis-tx-ipgap-linecheck-quirk;
+			snps,dis_rxdet_inp3_quirk;
+		};
+	};
+
 	sys_grf: syscon@fd58c000 {
 		compatible = "rockchip,rk3588-sys-grf", "syscon";
 		reg = <0x0 0xfd58c000 0x0 0x1000>;
-- 
2.40.1

Re: [PATCH v1 1/2] dt-bindings: usb: rockchip,dwc3: Add RK3588 binding

[Krzysztof Kozlowski]

On 19/07/2023 19:40, Sebastian Reichel wrote:
> RK3588 contains three DWC3 cores. Two of them are connected to
> dedicated USBDP PHY and can be used in dual-role. The third is
> connected to one of the shared combo PHYs used for PCIe/SATA/USB3
> and can only be used in host mode. Since the binding is all about
> the PHY glueing and involved clocks, separate compatible values
> have been created for these two types.

The cores are the same. To which phy they are connected to, does not
justify difference in compatibles. It's still the same devoce.

> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  .../bindings/usb/rockchip,rk3399-dwc3.yaml    | 107 ++++++++++++++----
>  1 file changed, 85 insertions(+), 22 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
> index 3159f9a6a0f7..0db4dc86e506 100644
> --- a/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
> @@ -11,7 +11,13 @@ maintainers:
>  
>  properties:
>    compatible:
> -    const: rockchip,rk3399-dwc3
> +    oneOf:
> +      - items:
> +          - enum:
> +              - rockchip,rk3588-dwc3-otg
> +              - rockchip,rk3588-dwc3-host
> +          - const: rockchip,rk3399-dwc3
> +      - const: rockchip,rk3399-dwc3
>  
>    '#address-cells':
>      const: 2
> @@ -22,35 +28,37 @@ properties:
>    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
> +    minItems: 3
> +    maxItems: 6
>  
>    clock-names:
>      items:
> -      - const: ref_clk
> -      - const: suspend_clk
> -      - const: bus_clk
> -      - const: aclk_usb3_rksoc_axi_perf
> -      - const: aclk_usb3
> -      - const: grf_clk
> +      oneOf:
> +        - enum:
> +            - ref
> +            - ref_clk

This is not oneOf. You cannot have here one clock. There is also no
point in making all these changes here,.

> +        - enum:
> +            - suspend
> +            - suspend_clk
> +        - enum:
> +            - bus
> +            - bus_clk
> +        - const: aclk_usb3_rksoc_axi_perf
> +        - const: aclk_usb3
> +        - const: grf_clk
> +        - const: utmi
> +        - const: php
> +        - const: pipe
> +    minItems: 3
> +    maxItems: 6
>  
>    resets:
>      maxItems: 1
>  
>    reset-names:
> -    const: usb3-otg
> +    enum:
> +      - usb3-host
> +      - usb3-otg

I don't see a point in changing this.

>  
>  patternProperties:
>    '^usb@':
> @@ -68,6 +76,61 @@ required:
>    - resets
>    - reset-names
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          const: rockchip,rk3399-dwc3
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 6
> +        clock-names:
> +          items:
> +            - const: ref_clk
> +            - const: suspend_clk
> +            - const: bus_clk
> +            - const: aclk_usb3_rksoc_axi_perf
> +            - const: aclk_usb3
> +            - const: grf_clk
> +        reset-names:
> +          const: usb3-otg
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: rockchip,rk3588-dwc3-otg
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 3
> +        clock-names:
> +          items:
> +            - const: ref
> +            - const: suspend
> +            - const: bus

Use the same clock names.

> +        reset-names:
> +          const: usb3-otg
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: rockchip,rk3588-dwc3-host
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 6
> +        clock-names:
> +          items:
> +            - const: ref
> +            - const: suspend
> +            - const: bus
> +            - const: utmi
> +            - const: php
> +            - const: pipe

Same clock names

> +        reset-names:
> +          const: usb3-host

Same reset name.


Best regards,
Krzysztof

Re: [PATCH v1 2/2] arm64: dts: rockchip: rk3588s: Add USB3 host controller

[Michael Riesch]

Hi Sebastian,

On 7/19/23 19:40, Sebastian Reichel wrote:
> RK3588 has three USB3 controllers. One of them is host-only and uses
> the naneng-combphy. The other two are dual-role and using a different
> PHY that is not yet supported upstream.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 29 +++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> index b9b509257aaa..416581dd3bb5 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> @@ -443,6 +443,35 @@ usb_host1_ohci: usb@fc8c0000 {
>  		status = "disabled";
>  	};
>  
> +	usbhost3_0: usb@fcd00000 {

I'd recommend aligning the handle names with rk356x.dtsi:

usb_host{0,1,..}_{e,o,x}hci

I saw that your USB2 series follows this convention already, so let's
stick to it here as well.

> +		compatible = "rockchip,rk3588-dwc3-host", "rockchip,rk3399-dwc3";
> +		clocks = <&cru REF_CLK_USB3OTG2>, <&cru SUSPEND_CLK_USB3OTG2>,
> +			 <&cru ACLK_USB3OTG2>, <&cru CLK_UTMI_OTG2>,
> +			 <&cru PCLK_PHP_ROOT>, <&cru CLK_PIPEPHY2_PIPE_U3_G>;
> +		clock-names = "ref", "suspend", "bus", "utmi", "php", "pipe";
> +		ranges;
> +		resets = <&cru SRST_A_USB3OTG2>;
> +		reset-names = "usb3-host";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		status = "disabled";
> +
> +		usbhost_dwc3_0: usb@fcd00000 {

I had the impression that the embedded nodes were out of fashion and the
preferred way was to have one node with the

compatible = "rockchip,rk3568-dwc3", "snps,dwc3";

Again, try to align it with rk356x.dtsi.

Thanks and best regards,
Michael

> +			compatible = "snps,dwc3";
> +			reg = <0x0 0xfcd00000 0x0 0x400000>;
> +			interrupts = <GIC_SPI 222 IRQ_TYPE_LEVEL_HIGH 0>;
> +			dr_mode = "host";
> +			phys = <&combphy2_psu PHY_TYPE_USB3>;
> +			phy-names = "usb3-phy";
> +			phy_type = "utmi_wide";
> +			snps,dis_enblslpm_quirk;
> +			snps,dis-u2-freeclk-exists-quirk;
> +			snps,dis-del-phy-power-chg-quirk;
> +			snps,dis-tx-ipgap-linecheck-quirk;
> +			snps,dis_rxdet_inp3_quirk;
> +		};
> +	};
> +
>  	sys_grf: syscon@fd58c000 {
>  		compatible = "rockchip,rk3588-sys-grf", "syscon";
>  		reg = <0x0 0xfd58c000 0x0 0x1000>;