2023-10-09 17:21:56

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCH v3 1/3] dt-bindings: usb: add rk3588 compatible to rockchip,dwc3

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.

While adding the extra clocks required by RK3588, I noticed grf_clk
is not available on RK3568, so I disallowed it for that platform.

Signed-off-by: Sebastian Reichel <[email protected]>
---
.../bindings/usb/rockchip,dwc3.yaml | 66 +++++++++++++++++--
1 file changed, 61 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
index 291844c8f3e1..517879290099 100644
--- a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
@@ -20,9 +20,6 @@ description:
Type-C PHY
Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt

-allOf:
- - $ref: snps,dwc3.yaml#
-
select:
properties:
compatible:
@@ -30,6 +27,7 @@ select:
enum:
- rockchip,rk3328-dwc3
- rockchip,rk3568-dwc3
+ - rockchip,rk3588-dwc3
required:
- compatible

@@ -39,6 +37,7 @@ properties:
- enum:
- rockchip,rk3328-dwc3
- rockchip,rk3568-dwc3
+ - rockchip,rk3588-dwc3
- const: snps,dwc3

reg:
@@ -58,7 +57,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 +67,10 @@ properties:
- const: ref_clk
- const: suspend_clk
- const: bus_clk
- - const: grf_clk
+ - enum:
+ - grf_clk
+ - utmi
+ - const: pipe

power-domains:
maxItems: 1
@@ -86,6 +90,58 @@ required:
- clocks
- clock-names

+allOf:
+ - $ref: snps,dwc3.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: rockchip,rk3328-dwc3
+ then:
+ properties:
+ clocks:
+ minItems: 3
+ maxItems: 4
+ clock-names:
+ minItems: 3
+ items:
+ - const: ref_clk
+ - const: suspend_clk
+ - const: bus_clk
+ - const: grf_clk
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: rockchip,rk3568-dwc3
+ then:
+ properties:
+ clocks:
+ maxItems: 3
+ clock-names:
+ items:
+ - const: ref_clk
+ - const: suspend_clk
+ - const: bus_clk
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: rockchip,rk3588-dwc3
+ then:
+ properties:
+ clocks:
+ minItems: 3
+ maxItems: 5
+ clock-names:
+ minItems: 3
+ items:
+ - const: ref_clk
+ - const: suspend_clk
+ - const: bus_clk
+ - const: utmi
+ - const: pipe
+
examples:
- |
#include <dt-bindings/clock/rk3328-cru.h>
--
2.42.0


2023-10-10 16:27:38

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] dt-bindings: usb: add rk3588 compatible to rockchip,dwc3

On Mon, Oct 09, 2023 at 07:20:09PM +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.
>
> While adding the extra clocks required by RK3588, I noticed grf_clk
> is not available on RK3568, so I disallowed it for that platform.
>
> Signed-off-by: Sebastian Reichel <[email protected]>
> ---
> .../bindings/usb/rockchip,dwc3.yaml | 66 +++++++++++++++++--
> 1 file changed, 61 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> index 291844c8f3e1..517879290099 100644
> --- a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml
> @@ -20,9 +20,6 @@ description:
> Type-C PHY
> Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
>
> -allOf:
> - - $ref: snps,dwc3.yaml#
> -
> select:
> properties:
> compatible:
> @@ -30,6 +27,7 @@ select:
> enum:
> - rockchip,rk3328-dwc3
> - rockchip,rk3568-dwc3
> + - rockchip,rk3588-dwc3
> required:
> - compatible
>
> @@ -39,6 +37,7 @@ properties:
> - enum:
> - rockchip,rk3328-dwc3
> - rockchip,rk3568-dwc3
> + - rockchip,rk3588-dwc3
> - const: snps,dwc3
>
> reg:
> @@ -58,7 +57,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 +67,10 @@ properties:
> - const: ref_clk
> - const: suspend_clk
> - const: bus_clk
> - - const: grf_clk
> + - enum:
> + - grf_clk
> + - utmi
> + - const: pipe
>
> power-domains:
> maxItems: 1
> @@ -86,6 +90,58 @@ required:
> - clocks
> - clock-names
>
> +allOf:
> + - $ref: snps,dwc3.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: rockchip,rk3328-dwc3
> + then:
> + properties:
> + clocks:
> + minItems: 3
> + maxItems: 4
> + clock-names:
> + minItems: 3
> + items:
> + - const: ref_clk
> + - const: suspend_clk
> + - const: bus_clk
> + - const: grf_clk

No need to list everything again. Just:

contains:
const: grf_clk

> + - if:
> + properties:
> + compatible:
> + contains:
> + const: rockchip,rk3568-dwc3
> + then:
> + properties:
> + clocks:
> + maxItems: 3
> + clock-names:
> + items:
> + - const: ref_clk
> + - const: suspend_clk
> + - const: bus_clk

Just 'maxItems: 3' is sufficient here.

> + - if:
> + properties:
> + compatible:
> + contains:
> + const: rockchip,rk3588-dwc3
> + then:
> + properties:
> + clocks:
> + minItems: 3

3 is already the min.

> + maxItems: 5

And 5 is already the max.

> + clock-names:
> + minItems: 3
> + items:
> + - const: ref_clk
> + - const: suspend_clk
> + - const: bus_clk
> + - const: utmi
> + - const: pipe

Again, can use 'contains' here. Where 'utmi' is in the list is already
defined by the top-level schema.

Rob

2023-10-12 12:31:20

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] dt-bindings: usb: add rk3588 compatible to rockchip,dwc3

Hi,

On Tue, Oct 10, 2023 at 11:27:22AM -0500, Rob Herring wrote:
> [...]
> > +allOf:
> > + - $ref: snps,dwc3.yaml#
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: rockchip,rk3328-dwc3
> > + then:
> > + properties:
> > + clocks:
> > + minItems: 3
> > + maxItems: 4
> > + clock-names:
> > + minItems: 3
> > + items:
> > + - const: ref_clk
> > + - const: suspend_clk
> > + - const: bus_clk
> > + - const: grf_clk
>
> No need to list everything again. Just:
>
> contains:
> const: grf_clk

No, that does not work because 'grf_clk' is optional and by using
'contains: grf_clk' the check will complain if the list does not
contain 'grf_clk'.

> [...] more improvements suggested by Rob [...]

These look all fine to me and I fixed them up for v4.

> > + clock-names:
> > + minItems: 3
> > + items:
> > + - const: ref_clk
> > + - const: suspend_clk
> > + - const: bus_clk
> > + - const: utmi
> > + - const: pipe
>
> Again, can use 'contains' here. Where 'utmi' is in the list is already
> defined by the top-level schema.

Same issue as above. On RK3588 there is one USB3 controller, which
needs all 5 clocks and two controllers with just the first 3 clocks.
I initially had two different compatible strings to have fixed lists,
but Krzysztof asked to use only a single one.

-- Sebastian


Attachments:
(No filename) (1.55 kB)
signature.asc (849.00 B)
Download all attachments