[PATCH v2 0/3] RK3588 USB3 host controller support

[PATCH v2 0/3] RK3588 USB3 host controller support

[Sebastian Reichel]

Hi,

This adds RK3588 USB3 host controller support. The same DT binding
will be used for the two dual-role controllers, which are also DWC3
based, but using a different PHY and don't need the extra UTMI/PIPE
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 series has been merged.

Changes since PATCHv1:
 * https://lore.kernel.org/all/20230719174015.68153-1-sebastian.reichel@collabora.com/
 * use same compatible for USB3 host and drd controllers (Krzysztof Kozlowski)
 * do not update reset-names (Krzysztof Kozlowski)
   - note: I dropped reset-names property, since there is only one reset line
     anyways. Binding could stay the same, since the reset-names property is
     optional
 * use "ref_clk", "suspend_clk" and "bus_clk" instead of "ref", "suspend" and "bus",
   so that they are the same as in RK3568 (Krzysztof Kozlowski)
 * rename handle name to "usb_host2_xhci" (Michael Riesch)
 * use RK356x style DWC3 binding instead of DWC3399 style
   - required adding an extra patch, so that the DWC3 core supports enabling
     the UTMI/PIPE clocks

-- Sebastian

Sebastian Reichel (3):
  dt-bindings: usb: add rk3588 compatible to rockchip,dwc3
  usb: dwc3: add optional PHY interface clocks
  arm64: dts: rockchip: rk3588s: Add USB3 host controller

 .../bindings/usb/rockchip,dwc3.yaml           | 11 ++++++--
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi     | 21 +++++++++++++++
 drivers/usb/dwc3/core.c                       | 26 +++++++++++++++++++
 drivers/usb/dwc3/core.h                       |  4 +++
 4 files changed, 60 insertions(+), 2 deletions(-)

-- 
2.40.1

[PATCH v2 1/3] dt-bindings: usb: add rk3588 compatible to rockchip,dwc3

[Sebastian Reichel]

RK3588 has three DWC3 controllers. Two of them are fully functional in
host, device and OTG mode including USB2 support. They are connected to
dedicated PHYs, that also support USB-C's DisplayPort alternate mode.

The third controller is connected to one of the combphy's shared
with PCIe and SATA. It can only be used in host mode and does not
support USB2. Compared to the other controllers this one needs
some extra clocks.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 .../devicetree/bindings/usb/rockchip,dwc3.yaml        | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
index 291844c8f3e1..cbc3e55e05e1 100644
--- a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
@@ -30,6 +30,7 @@ select:
         enum:
           - rockchip,rk3328-dwc3
           - rockchip,rk3568-dwc3
+          - rockchip,rk3588-dwc3
   required:
     - compatible
 
@@ -39,6 +40,7 @@ properties:
       - enum:
           - rockchip,rk3328-dwc3
           - rockchip,rk3568-dwc3
+          - rockchip,rk3588-dwc3
       - const: snps,dwc3
 
   reg:
@@ -58,7 +60,9 @@ properties:
           Master/Core clock, must to be >= 62.5 MHz for SS
           operation and >= 30MHz for HS operation
       - description:
-          Controller grf clock
+          Controller grf clock OR UTMI clock
+      - description:
+          PIPE clock
 
   clock-names:
     minItems: 3
@@ -66,7 +70,10 @@ properties:
       - const: ref_clk
       - const: suspend_clk
       - const: bus_clk
-      - const: grf_clk
+      - enum:
+          - grf_clk
+          - utmi
+      - const: pipe
 
   power-domains:
     maxItems: 1
-- 
2.40.1

[PATCH v2 2/3] usb: dwc3: add optional PHY interface clocks

[Sebastian Reichel]

On Rockchip RK3588 one of the DWC3 cores is integrated weirdly and
requires two extra clocks to be enabled. Without these extra clocks
hot-plugging USB devices is broken.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/usb/dwc3/core.c | 26 ++++++++++++++++++++++++++
 drivers/usb/dwc3/core.h |  4 ++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index f6689b731718..0618fcc4d0af 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -817,8 +817,20 @@ static int dwc3_clk_enable(struct dwc3 *dwc)
 	if (ret)
 		goto disable_ref_clk;
 
+	ret = clk_prepare_enable(dwc->utmi_clk);
+	if (ret)
+		goto disable_susp_clk;
+
+	ret = clk_prepare_enable(dwc->pipe_clk);
+	if (ret)
+		goto disable_utmi_clk;
+
 	return 0;
 
+disable_utmi_clk:
+	clk_disable_unprepare(dwc->utmi_clk);
+disable_susp_clk:
+	clk_disable_unprepare(dwc->susp_clk);
 disable_ref_clk:
 	clk_disable_unprepare(dwc->ref_clk);
 disable_bus_clk:
@@ -828,6 +840,8 @@ static int dwc3_clk_enable(struct dwc3 *dwc)
 
 static void dwc3_clk_disable(struct dwc3 *dwc)
 {
+	clk_disable_unprepare(dwc->pipe_clk);
+	clk_disable_unprepare(dwc->utmi_clk);
 	clk_disable_unprepare(dwc->susp_clk);
 	clk_disable_unprepare(dwc->ref_clk);
 	clk_disable_unprepare(dwc->bus_clk);
@@ -1764,6 +1778,18 @@ static int dwc3_get_clocks(struct dwc3 *dwc)
 		}
 	}
 
+	dwc->utmi_clk = devm_clk_get_optional(dev, "utmi");
+	if (IS_ERR(dwc->utmi_clk)) {
+		return dev_err_probe(dev, PTR_ERR(dwc->utmi_clk),
+				"could not get utmi clock\n");
+	}
+
+	dwc->pipe_clk = devm_clk_get_optional(dev, "pipe");
+	if (IS_ERR(dwc->pipe_clk)) {
+		return dev_err_probe(dev, PTR_ERR(dwc->pipe_clk),
+				"could not get pipe clock\n");
+	}
+
 	return 0;
 }
 
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 8b1295e4dcdd..fa49a87025cf 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -994,6 +994,8 @@ struct dwc3_scratchpad_array {
  * @bus_clk: clock for accessing the registers
  * @ref_clk: reference clock
  * @susp_clk: clock used when the SS phy is in low power (S3) state
+ * @utmi_clk: clock used for USB2 PHY communication
+ * @pipe_clk: clock used for USB3 PHY communication
  * @reset: reset control
  * @regs: base address for our registers
  * @regs_size: address space size
@@ -1159,6 +1161,8 @@ struct dwc3 {
 	struct clk		*bus_clk;
 	struct clk		*ref_clk;
 	struct clk		*susp_clk;
+	struct clk		*utmi_clk;
+	struct clk		*pipe_clk;
 
 	struct reset_control	*reset;
 
-- 
2.40.1

[PATCH v2 3/3] arm64: dts: rockchip: rk3588s: Add USB3 host controller

[Sebastian Reichel]

RK3588 has three USB3 controllers. This adds the host-only controller,
which is using the naneng-combphy shared with PCIe and SATA.

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 | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index b9b509257aaa..5cda58ea52e2 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -443,6 +443,27 @@ usb_host1_ohci: usb@fc8c0000 {
 		status = "disabled";
 	};
 
+	usb_host2_xhci: usb@fcd00000 {
+		compatible = "rockchip,rk3588-dwc3", "snps,dwc3";
+		reg = <0x0 0xfcd00000 0x0 0x400000>;
+		interrupts = <GIC_SPI 222 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru REF_CLK_USB3OTG2>, <&cru SUSPEND_CLK_USB3OTG2>,
+			 <&cru ACLK_USB3OTG2>, <&cru CLK_UTMI_OTG2>,
+			 <&cru CLK_PIPEPHY2_PIPE_U3_G>;
+		clock-names = "ref_clk", "suspend_clk", "bus_clk", "utmi", "pipe";
+		dr_mode = "host";
+		phys = <&combphy2_psu PHY_TYPE_USB3>;
+		phy-names = "usb3-phy";
+		phy_type = "utmi_wide";
+		resets = <&cru SRST_A_USB3OTG2>;
+		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;
+		status = "disabled";
+	};
+
 	sys_grf: syscon@fd58c000 {
 		compatible = "rockchip,rk3588-sys-grf", "syscon";
 		reg = <0x0 0xfd58c000 0x0 0x1000>;
-- 
2.40.1

Re: [PATCH v2 1/3] dt-bindings: usb: add rk3588 compatible to rockchip,dwc3

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2278 bytes --]

On Thu, Jul 20, 2023 at 07:36:41PM +0200, Sebastian Reichel wrote:
> RK3588 has three DWC3 controllers. Two of them are fully functional in
> host, device and OTG mode including USB2 support. They are connected to
> dedicated PHYs, that also support USB-C's DisplayPort alternate mode.
> 
> The third controller is connected to one of the combphy's shared
> with PCIe and SATA. It can only be used in host mode and does not
> support USB2. Compared to the other controllers this one needs
> some extra clocks.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

I feel like I say it a bunch for some of these Rockchip bindings
patches, but if you're adding more clocks for some SoCs, should some
per-SoC constraints not also be added?

> ---
>  .../devicetree/bindings/usb/rockchip,dwc3.yaml        | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> index 291844c8f3e1..cbc3e55e05e1 100644
> --- a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> @@ -30,6 +30,7 @@ select:
>          enum:
>            - rockchip,rk3328-dwc3
>            - rockchip,rk3568-dwc3
> +          - rockchip,rk3588-dwc3
>    required:
>      - compatible
>  
> @@ -39,6 +40,7 @@ properties:
>        - enum:
>            - rockchip,rk3328-dwc3
>            - rockchip,rk3568-dwc3
> +          - rockchip,rk3588-dwc3
>        - const: snps,dwc3
>  
>    reg:
> @@ -58,7 +60,9 @@ properties:
>            Master/Core clock, must to be >= 62.5 MHz for SS
>            operation and >= 30MHz for HS operation
>        - description:
> -          Controller grf clock
> +          Controller grf clock OR UTMI clock
> +      - description:
> +          PIPE clock
>  
>    clock-names:
>      minItems: 3
> @@ -66,7 +70,10 @@ properties:
>        - const: ref_clk
>        - const: suspend_clk
>        - const: bus_clk
> -      - const: grf_clk
> +      - enum:
> +          - grf_clk
> +          - utmi
> +      - const: pipe
>  
>    power-domains:
>      maxItems: 1
> -- 
> 2.40.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/3] dt-bindings: usb: add rk3588 compatible to rockchip,dwc3

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2801 bytes --]

Hi,

On Sat, Jul 22, 2023 at 12:42:09PM +0100, Conor Dooley wrote:
> On Thu, Jul 20, 2023 at 07:36:41PM +0200, Sebastian Reichel wrote:
> > RK3588 has three DWC3 controllers. Two of them are fully functional in
> > host, device and OTG mode including USB2 support. They are connected to
> > dedicated PHYs, that also support USB-C's DisplayPort alternate mode.
> > 
> > The third controller is connected to one of the combphy's shared
> > with PCIe and SATA. It can only be used in host mode and does not
> > support USB2. Compared to the other controllers this one needs
> > some extra clocks.
> > 
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
> I feel like I say it a bunch for some of these Rockchip bindings
> patches, but if you're adding more clocks for some SoCs, should some
> per-SoC constraints not also be added?

The extra clocks are not actually needed by all the USB3 controllers
in the SoC. Only one of three USB3 controllers needs them. In v1 I
used different compatible values to narrow the clock binding down
and Krzysztof asked to remove that. So please tell me what it should
look like.

Greetings,

-- Sebastian

> > ---
> >  .../devicetree/bindings/usb/rockchip,dwc3.yaml        | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> > index 291844c8f3e1..cbc3e55e05e1 100644
> > --- a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> > +++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> > @@ -30,6 +30,7 @@ select:
> >          enum:
> >            - rockchip,rk3328-dwc3
> >            - rockchip,rk3568-dwc3
> > +          - rockchip,rk3588-dwc3
> >    required:
> >      - compatible
> >  
> > @@ -39,6 +40,7 @@ properties:
> >        - enum:
> >            - rockchip,rk3328-dwc3
> >            - rockchip,rk3568-dwc3
> > +          - rockchip,rk3588-dwc3
> >        - const: snps,dwc3
> >  
> >    reg:
> > @@ -58,7 +60,9 @@ properties:
> >            Master/Core clock, must to be >= 62.5 MHz for SS
> >            operation and >= 30MHz for HS operation
> >        - description:
> > -          Controller grf clock
> > +          Controller grf clock OR UTMI clock
> > +      - description:
> > +          PIPE clock
> >  
> >    clock-names:
> >      minItems: 3
> > @@ -66,7 +70,10 @@ properties:
> >        - const: ref_clk
> >        - const: suspend_clk
> >        - const: bus_clk
> > -      - const: grf_clk
> > +      - enum:
> > +          - grf_clk
> > +          - utmi
> > +      - const: pipe
> >  
> >    power-domains:
> >      maxItems: 1
> > -- 
> > 2.40.1
> > 



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 1/3] dt-bindings: usb: add rk3588 compatible to rockchip,dwc3

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3337 bytes --]

On Mon, Jul 31, 2023 at 05:12:24PM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Sat, Jul 22, 2023 at 12:42:09PM +0100, Conor Dooley wrote:
> > On Thu, Jul 20, 2023 at 07:36:41PM +0200, Sebastian Reichel wrote:
> > > RK3588 has three DWC3 controllers. Two of them are fully functional in
> > > host, device and OTG mode including USB2 support. They are connected to
> > > dedicated PHYs, that also support USB-C's DisplayPort alternate mode.
> > > 
> > > The third controller is connected to one of the combphy's shared
> > > with PCIe and SATA. It can only be used in host mode and does not
> > > support USB2. Compared to the other controllers this one needs
> > > some extra clocks.
> > > 
> > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > 
> > I feel like I say it a bunch for some of these Rockchip bindings
> > patches, but if you're adding more clocks for some SoCs, should some
> > per-SoC constraints not also be added?
> 
> The extra clocks are not actually needed by all the USB3 controllers
> in the SoC. Only one of three USB3 controllers needs them. In v1 I
> used different compatible values to narrow the clock binding down
> and Krzysztof asked to remove that. So please tell me what it should
> look like.

Maybe Krzysztof and I were talking about different things. I was talking
about constraining !3588 SoCs so that nobody tries to use a utmi and
pipe clock on those, and only allowing them on a 3588. AFAICT, what
Krzysztof objected to was having more than one compatible for
controllers on the 3588. Maybe he weigh in.

Thanks,
Conor.

> > > ---
> > >  .../devicetree/bindings/usb/rockchip,dwc3.yaml        | 11 +++++++++--
> > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> > > index 291844c8f3e1..cbc3e55e05e1 100644
> > > --- a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> > > @@ -30,6 +30,7 @@ select:
> > >          enum:
> > >            - rockchip,rk3328-dwc3
> > >            - rockchip,rk3568-dwc3
> > > +          - rockchip,rk3588-dwc3
> > >    required:
> > >      - compatible
> > >  
> > > @@ -39,6 +40,7 @@ properties:
> > >        - enum:
> > >            - rockchip,rk3328-dwc3
> > >            - rockchip,rk3568-dwc3
> > > +          - rockchip,rk3588-dwc3
> > >        - const: snps,dwc3
> > >  
> > >    reg:
> > > @@ -58,7 +60,9 @@ properties:
> > >            Master/Core clock, must to be >= 62.5 MHz for SS
> > >            operation and >= 30MHz for HS operation
> > >        - description:
> > > -          Controller grf clock
> > > +          Controller grf clock OR UTMI clock
> > > +      - description:
> > > +          PIPE clock
> > >  
> > >    clock-names:
> > >      minItems: 3
> > > @@ -66,7 +70,10 @@ properties:
> > >        - const: ref_clk
> > >        - const: suspend_clk
> > >        - const: bus_clk
> > > -      - const: grf_clk
> > > +      - enum:
> > > +          - grf_clk
> > > +          - utmi
> > > +      - const: pipe
> > >  
> > >    power-domains:
> > >      maxItems: 1
> > > -- 
> > > 2.40.1
> > > 
> 
> 



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]