2023-07-20 17:45:52

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCH v2 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.

Signed-off-by: Sebastian Reichel <[email protected]>
---
.../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



2023-07-22 12:10:42

by Conor Dooley

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

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 <[email protected]>

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
>


Attachments:
(No filename) (2.22 kB)
signature.asc (235.00 B)
Download all attachments

2023-07-31 15:42:35

by Sebastian Reichel

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

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 <[email protected]>
>
> 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
> >



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

2023-07-31 16:19:49

by Conor Dooley

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

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 <[email protected]>
> >
> > 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
> > >
>
>



Attachments:
(No filename) (3.26 kB)
signature.asc (235.00 B)
Download all attachments