| Message ID | 20231020150022.48725-2-sebastian.reichel@collabora.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 98bad5bc447ec962988a48c92f7d0f8c4dc473d2 |
| Headers | show |
| Series | RK3588 USB3 host controller support | expand |
On Fri, Oct 20, 2023 at 04:11:40PM +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 | 60 +++++++++++++++++-- > 1 file changed, 55 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml > index 291844c8f3e1..264c2178d61d 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,52 @@ required: > - clocks > - clock-names > > +allOf: > + - $ref: snps,dwc3.yaml# > + - if: > + properties: > + compatible: > + contains: > + const: rockchip,rk3328-dwc3 > + then: > + properties: > + clocks: > + minItems: 3 minItems for clocks and clock-names is already 3, is it not? Otherwise, Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Thanks, Conor. > + 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: > + maxItems: 3 > + - if: > + properties: > + compatible: > + contains: > + const: rockchip,rk3588-dwc3 > + then: > + properties: > + 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 >
Hi Conor, On Fri, Oct 20, 2023 at 04:36:19PM +0100, Conor Dooley wrote: > On Fri, Oct 20, 2023 at 04:11:40PM +0200, Sebastian Reichel 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 > > minItems for clocks and clock-names is already 3, is it not? Yes, but the following 'maxItems: 4' implicitly sets it to 4, so I had to set it again. The same is true for clock-names - providings new 'items:' effectively drops the "minItems: 3" from the generic section. > Otherwise, > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > Thanks, > Conor. Thanks, -- Sebastian
On Fri, Oct 20, 2023 at 06:03:29PM +0200, Sebastian Reichel wrote: > Hi Conor, > > On Fri, Oct 20, 2023 at 04:36:19PM +0100, Conor Dooley wrote: > > On Fri, Oct 20, 2023 at 04:11:40PM +0200, Sebastian Reichel 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 > > > > minItems for clocks and clock-names is already 3, is it not? > > Yes, but the following 'maxItems: 4' implicitly sets it to 4, > so I had to set it again. The same is true for clock-names - > providings new 'items:' effectively drops the "minItems: 3" > from the generic section. Are you sure? We don't add anything implicit in the if/then schemas. Could be a tool issue though. Rob
Hi Rob, On Sun, Oct 22, 2023 at 04:42:19PM -0500, Rob Herring wrote: > On Fri, Oct 20, 2023 at 06:03:29PM +0200, Sebastian Reichel wrote: > > On Fri, Oct 20, 2023 at 04:36:19PM +0100, Conor Dooley wrote: > > > On Fri, Oct 20, 2023 at 04:11:40PM +0200, Sebastian Reichel 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 > > > > > > minItems for clocks and clock-names is already 3, is it not? > > > > Yes, but the following 'maxItems: 4' implicitly sets it to 4, > > so I had to set it again. The same is true for clock-names - > > providings new 'items:' effectively drops the "minItems: 3" > > from the generic section. > > Are you sure? We don't add anything implicit in the if/then schemas. > Could be a tool issue though. I had this issue in the past. But just in case I also did a re-test before sending my last mail and I did get a warning. So yes, I'm quite sure :) -- Sebastian
On Mon, Oct 23, 2023 at 02:18:03AM +0200, Sebastian Reichel wrote: > Hi Rob, > > On Sun, Oct 22, 2023 at 04:42:19PM -0500, Rob Herring wrote: > > On Fri, Oct 20, 2023 at 06:03:29PM +0200, Sebastian Reichel wrote: > > > On Fri, Oct 20, 2023 at 04:36:19PM +0100, Conor Dooley wrote: > > > > On Fri, Oct 20, 2023 at 04:11:40PM +0200, Sebastian Reichel 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 > > > > > > > > minItems for clocks and clock-names is already 3, is it not? > > > > > > Yes, but the following 'maxItems: 4' implicitly sets it to 4, > > > so I had to set it again. The same is true for clock-names - > > > providings new 'items:' effectively drops the "minItems: 3" > > > from the generic section. > > > > Are you sure? We don't add anything implicit in the if/then schemas. > > Could be a tool issue though. > > I had this issue in the past. But just in case I also did a re-test > before sending my last mail and I did get a warning. So yes, I'm > quite sure :) Well, I'm quite surprised no one else noticed. Anyways, I'm working on a fix for it. In the meantime, just leave it as you have it. Note that there's an existing error in this binding I noticed in the example. The clocks and clock-names lengths don't match. Rob
diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml b/Documentation/devicetree/bindings/usb/rockchip,dwc3.yaml index 291844c8f3e1..264c2178d61d 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,52 @@ 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: + maxItems: 3 + - if: + properties: + compatible: + contains: + const: rockchip,rk3588-dwc3 + then: + properties: + 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 | 60 +++++++++++++++++-- 1 file changed, 55 insertions(+), 5 deletions(-)