| Message ID | 20230720173643.69553-2-sebastian.reichel@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RK3588 USB3 host controller support | expand |
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 >
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 > >
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 > > > > >
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
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(-)