| Message ID | 20231009172129.43568-2-sebastian.reichel@collabora.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | RK3588 USB3 host controller support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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 <sebastian.reichel@collabora.com> > --- > .../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
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
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>
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 <sebastian.reichel@collabora.com> --- .../bindings/usb/rockchip,dwc3.yaml | 66 +++++++++++++++++-- 1 file changed, 61 insertions(+), 5 deletions(-)