2022-02-25 16:53:29

by Peter Geis

[permalink] [raw]
Subject: [PATCH v1 7/8] arm64: dts: rockchip: add rk356x dwc3 usb3 nodes

Add the dwc3 device nodes to the rk356x device trees.
The rk3566 has one usb2 capable dwc3 otg controller and one usb3 capable
dwc3 host controller.
The rk3568 has one usb3 capable dwc3 otg controller and one usb3 capable
dwc3 host controller.

Signed-off-by: Peter Geis <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3566.dtsi | 12 +++++++
arch/arm64/boot/dts/rockchip/rk3568.dtsi | 9 +++++
arch/arm64/boot/dts/rockchip/rk356x.dtsi | 45 +++++++++++++++++++++++-
3 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi b/arch/arm64/boot/dts/rockchip/rk3566.dtsi
index 3839eef5e4f7..8e8b52f58f44 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3566.dtsi
@@ -6,6 +6,10 @@ / {
compatible = "rockchip,rk3566";
};

+&pipegrf {
+ compatible = "rockchip,rk3566-pipe-grf", "syscon";
+};
+
&power {
power-domain@RK3568_PD_PIPE {
reg = <RK3568_PD_PIPE>;
@@ -18,3 +22,11 @@ power-domain@RK3568_PD_PIPE {
#power-domain-cells = <0>;
};
};
+
+&usbdrd30 {
+ phys = <&usb2phy0_otg>;
+ phy-names = "usb2-phy";
+ extcon = <&usb2phy0>;
+ maximum-speed = "high-speed";
+ snps,dis_u2_susphy_quirk;
+};
diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
index 5b0f528d6818..77c044cbaaad 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
@@ -99,6 +99,10 @@ opp-1992000000 {
};
};

+&pipegrf {
+ compatible = "rockchip,rk3568-pipe-grf", "syscon";
+};
+
&power {
power-domain@RK3568_PD_PIPE {
reg = <RK3568_PD_PIPE>;
@@ -114,3 +118,8 @@ power-domain@RK3568_PD_PIPE {
#power-domain-cells = <0>;
};
};
+
+&usbdrd30 {
+ phys = <&usb2phy0_otg>, <&combphy0 PHY_TYPE_USB3>;
+ phy-names = "usb2-phy", "usb3-phy";
+};
diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 84d5d607e693..4fae5b3b326e 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -230,6 +230,50 @@ scmi_shmem: sram@0 {
};
};

+ usbdrd30: usbdrd@fcc00000 {
+ compatible = "rockchip,rk3568-dwc3", "snps,dwc3";
+ reg = <0x0 0xfcc00000 0x0 0x400000>;
+ interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cru CLK_USB3OTG0_REF>, <&cru CLK_USB3OTG0_SUSPEND>,
+ <&cru ACLK_USB3OTG0>, <&cru PCLK_PIPE>;
+ clock-names = "ref_clk", "suspend_clk",
+ "bus_clk", "grf_clk";
+ dr_mode = "host";
+ phy_type = "utmi_wide";
+ power-domains = <&power RK3568_PD_PIPE>;
+ resets = <&cru SRST_USB3OTG0>;
+ reset-names = "usb3-otg";
+ snps,dis_enblslpm_quirk;
+ snps,dis-u2-freeclk-exists-quirk;
+ snps,dis-del-phy-power-chg-quirk;
+ snps,dis-tx-ipgap-linecheck-quirk;
+ snps,xhci-trb-ent-quirk;
+ status = "disabled";
+ };
+
+ usbhost30: usbhost@fd000000 {
+ compatible = "rockchip,rk3568-dwc3", "snps,dwc3";
+ reg = <0x0 0xfd000000 0x0 0x400000>;
+ interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cru CLK_USB3OTG1_REF>, <&cru CLK_USB3OTG1_SUSPEND>,
+ <&cru ACLK_USB3OTG1>, <&cru PCLK_PIPE>;
+ clock-names = "ref_clk", "suspend_clk",
+ "bus_clk", "grf_clk";
+ dr_mode = "host";
+ phys = <&usb2phy0_host>, <&combphy1 PHY_TYPE_USB3>;
+ phy-names = "usb2-phy", "usb3-phy";
+ phy_type = "utmi_wide";
+ power-domains = <&power RK3568_PD_PIPE>;
+ resets = <&cru SRST_USB3OTG1>;
+ reset-names = "usb3-host";
+ snps,dis_enblslpm_quirk;
+ snps,dis-u2-freeclk-exists-quirk;
+ snps,dis_u2_susphy_quirk;
+ snps,dis-del-phy-power-chg-quirk;
+ snps,dis-tx-ipgap-linecheck-quirk;
+ status = "disabled";
+ };
+
gic: interrupt-controller@fd400000 {
compatible = "arm,gic-v3";
reg = <0x0 0xfd400000 0 0x10000>, /* GICD */
@@ -297,7 +341,6 @@ pmu_io_domains: io-domains {
};

pipegrf: syscon@fdc50000 {
- compatible = "rockchip,rk3568-pipe-grf", "syscon";
reg = <0x0 0xfdc50000 0x0 0x1000>;
};

--
2.25.1


2022-02-25 20:15:16

by Peter Geis

[permalink] [raw]
Subject: Re: [PATCH v1 7/8] arm64: dts: rockchip: add rk356x dwc3 usb3 nodes

On Fri, Feb 25, 2022 at 12:01 PM Michael Riesch
<[email protected]> wrote:
>
> Hi Peter,
>
> (It should be noted that there was a slight mishap in communications
> between the two of us resulting in two series with the same goal. Now
> let's clean up the mess :-)
>
> Thanks for your series. Seeing that it contains more patches than mine
> it probably makes sense to use your series as basis. Please Cc: me in
> future iterations of this patch series and consider my comments below.

Will do.
If you'd like I can also pull your enablement patch.

>
> On 2/25/22 15:54, Peter Geis wrote:
> > Add the dwc3 device nodes to the rk356x device trees.
> > The rk3566 has one usb2 capable dwc3 otg controller and one usb3 capable
> > dwc3 host controller.
> > The rk3568 has one usb3 capable dwc3 otg controller and one usb3 capable
> > dwc3 host controller.
> >
> > Signed-off-by: Peter Geis <[email protected]>
> > ---
> > arch/arm64/boot/dts/rockchip/rk3566.dtsi | 12 +++++++
> > arch/arm64/boot/dts/rockchip/rk3568.dtsi | 9 +++++
> > arch/arm64/boot/dts/rockchip/rk356x.dtsi | 45 +++++++++++++++++++++++-
> > 3 files changed, 65 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi b/arch/arm64/boot/dts/rockchip/rk3566.dtsi
> > index 3839eef5e4f7..8e8b52f58f44 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3566.dtsi
> > @@ -6,6 +6,10 @@ / {
> > compatible = "rockchip,rk3566";
> > };
> >
> > +&pipegrf {
> > + compatible = "rockchip,rk3566-pipe-grf", "syscon";
> > +};
> > +
> > &power {
> > power-domain@RK3568_PD_PIPE {
> > reg = <RK3568_PD_PIPE>;
> > @@ -18,3 +22,11 @@ power-domain@RK3568_PD_PIPE {
> > #power-domain-cells = <0>;
> > };
> > };
> > +
> > +&usbdrd30 {
>
> I would really love to have some alignment with the other USB controllers
>
> usb_host{0,1}_{e,o}hci
>
> here. I am aware that older SoCs and the SDK are using these names and
> it might be painful to have different versions to maintain at the
> moment, but can we please agree on
>
> usb_host0_xhci
> usb_host1_xhci
>
> or something like that?

I agree, I like your naming better.

>
> > + phys = <&usb2phy0_otg>;
> > + phy-names = "usb2-phy";
> > + extcon = <&usb2phy0>;
> > + maximum-speed = "high-speed";
> > + snps,dis_u2_susphy_quirk;
> > +};
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> > index 5b0f528d6818..77c044cbaaad 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> > @@ -99,6 +99,10 @@ opp-1992000000 {
> > };
> > };
> >
> > +&pipegrf {
> > + compatible = "rockchip,rk3568-pipe-grf", "syscon";
> > +};
> > +
> > &power {
> > power-domain@RK3568_PD_PIPE {
> > reg = <RK3568_PD_PIPE>;
> > @@ -114,3 +118,8 @@ power-domain@RK3568_PD_PIPE {
> > #power-domain-cells = <0>;
> > };
> > };
> > +
> > +&usbdrd30 {
> > + phys = <&usb2phy0_otg>, <&combphy0 PHY_TYPE_USB3>;
> > + phy-names = "usb2-phy", "usb3-phy";
> > +};
> > diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > index 84d5d607e693..4fae5b3b326e 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > @@ -230,6 +230,50 @@ scmi_shmem: sram@0 {
> > };
> > };
> >
> > + usbdrd30: usbdrd@fcc00000 {
> > + compatible = "rockchip,rk3568-dwc3", "snps,dwc3";
> > + reg = <0x0 0xfcc00000 0x0 0x400000>;
> > + interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&cru CLK_USB3OTG0_REF>, <&cru CLK_USB3OTG0_SUSPEND>,
> > + <&cru ACLK_USB3OTG0>, <&cru PCLK_PIPE>;
> > + clock-names = "ref_clk", "suspend_clk",
> > + "bus_clk", "grf_clk";
>
> Please consider Johan's comments on my first series. In my tests
> removing the PCLK_PIPE clock did not make any difference.

I'd like to test that this isn't being used, or isn't just working
because it's enabled elsewhere.
If both of those are false, then I'll be happy to drop this.

>
> > + dr_mode = "host";
>
> Based on the description in the commit message above it should be "otg",
> right? Boards are free to overrule this, of course.

Currently the usb2phy does not support OTG mode correctly.
There are patches in the works for this, but at the moment it's safer
to default to host.

>
> > + phy_type = "utmi_wide";
> > + power-domains = <&power RK3568_PD_PIPE>;
> > + resets = <&cru SRST_USB3OTG0>;
> > + reset-names = "usb3-otg";
> > + snps,dis_enblslpm_quirk;
> > + snps,dis-u2-freeclk-exists-quirk;
> > + snps,dis-del-phy-power-chg-quirk;
> > + snps,dis-tx-ipgap-linecheck-quirk;
> > + snps,xhci-trb-ent-quirk;
>
> In my first version I had all those quirks as well, but are they
> actually necessary? I decided to remove them all to have a fresh start
> (also activating them did not seem to affect my test setup).

I'm now curious about this, can someone weigh in on valid ways of
testing each one of these in a way that is definite?

>
> > + status = "disabled";
> > + };
> > +
> > + usbhost30: usbhost@fd000000 {
>
> Please reconsider the this name as well.
>
> > + compatible = "rockchip,rk3568-dwc3", "snps,dwc3";
> > + reg = <0x0 0xfd000000 0x0 0x400000>;
> > + interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&cru CLK_USB3OTG1_REF>, <&cru CLK_USB3OTG1_SUSPEND>,
> > + <&cru ACLK_USB3OTG1>, <&cru PCLK_PIPE>;
> > + clock-names = "ref_clk", "suspend_clk",
> > + "bus_clk", "grf_clk";
> > + dr_mode = "host";
>
> Here "host" clearly makes sense, as this controller is not capable of otg.
>
> > + phys = <&usb2phy0_host>, <&combphy1 PHY_TYPE_USB3>;
> > + phy-names = "usb2-phy", "usb3-phy";
> > + phy_type = "utmi_wide";
> > + power-domains = <&power RK3568_PD_PIPE>;
> > + resets = <&cru SRST_USB3OTG1>;
> > + reset-names = "usb3-host";
> > + snps,dis_enblslpm_quirk;
> > + snps,dis-u2-freeclk-exists-quirk;
> > + snps,dis_u2_susphy_quirk;
> > + snps,dis-del-phy-power-chg-quirk;
> > + snps,dis-tx-ipgap-linecheck-quirk;
>
> What was said about quirks above holds here as well (although one quirk
> not documented in the bindings is missing here).

Same thing here, I'd like absolute testing to determine that these are
not necessary, since downstream (the oem) felt they were.

>
> Best regards,
> Michael
>
> > + status = "disabled";
> > + };
> > +
> > gic: interrupt-controller@fd400000 {
> > compatible = "arm,gic-v3";
> > reg = <0x0 0xfd400000 0 0x10000>, /* GICD */
> > @@ -297,7 +341,6 @@ pmu_io_domains: io-domains {
> > };
> >
> > pipegrf: syscon@fdc50000 {
> > - compatible = "rockchip,rk3568-pipe-grf", "syscon";
> > reg = <0x0 0xfdc50000 0x0 0x1000>;
> > };
> >

2022-02-26 01:34:27

by Michael Riesch

[permalink] [raw]
Subject: Re: [PATCH v1 7/8] arm64: dts: rockchip: add rk356x dwc3 usb3 nodes

Hi Peter,

On 2/25/22 18:59, Peter Geis wrote:
> On Fri, Feb 25, 2022 at 12:01 PM Michael Riesch
> <[email protected]> wrote:
>>
>> Hi Peter,
>>
>> (It should be noted that there was a slight mishap in communications
>> between the two of us resulting in two series with the same goal. Now
>> let's clean up the mess :-)
>>
>> Thanks for your series. Seeing that it contains more patches than mine
>> it probably makes sense to use your series as basis. Please Cc: me in
>> future iterations of this patch series and consider my comments below.
>
> Will do.
> If you'd like I can also pull your enablement patch.

That'd be great, thanks! I'll help testing of course :-)

>> On 2/25/22 15:54, Peter Geis wrote:
> [...]
>>> + dr_mode = "host";
>>
>> Based on the description in the commit message above it should be "otg",
>> right? Boards are free to overrule this, of course.
>
> Currently the usb2phy does not support OTG mode correctly.
> There are patches in the works for this, but at the moment it's safer
> to default to host.

Ah OK, makes sense!

Best regards,
Michael

>
>>
>>> + phy_type = "utmi_wide";
>>> + power-domains = <&power RK3568_PD_PIPE>;
>>> + resets = <&cru SRST_USB3OTG0>;
>>> + reset-names = "usb3-otg";
>>> + snps,dis_enblslpm_quirk;
>>> + snps,dis-u2-freeclk-exists-quirk;
>>> + snps,dis-del-phy-power-chg-quirk;
>>> + snps,dis-tx-ipgap-linecheck-quirk;
>>> + snps,xhci-trb-ent-quirk;
>>
>> In my first version I had all those quirks as well, but are they
>> actually necessary? I decided to remove them all to have a fresh start
>> (also activating them did not seem to affect my test setup).
>
> I'm now curious about this, can someone weigh in on valid ways of
> testing each one of these in a way that is definite?
>
>>
>>> + status = "disabled";
>>> + };
>>> +
>>> + usbhost30: usbhost@fd000000 {
>>
>> Please reconsider the this name as well.
>>
>>> + compatible = "rockchip,rk3568-dwc3", "snps,dwc3";
>>> + reg = <0x0 0xfd000000 0x0 0x400000>;
>>> + interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
>>> + clocks = <&cru CLK_USB3OTG1_REF>, <&cru CLK_USB3OTG1_SUSPEND>,
>>> + <&cru ACLK_USB3OTG1>, <&cru PCLK_PIPE>;
>>> + clock-names = "ref_clk", "suspend_clk",
>>> + "bus_clk", "grf_clk";
>>> + dr_mode = "host";
>>
>> Here "host" clearly makes sense, as this controller is not capable of otg.
>>
>>> + phys = <&usb2phy0_host>, <&combphy1 PHY_TYPE_USB3>;
>>> + phy-names = "usb2-phy", "usb3-phy";
>>> + phy_type = "utmi_wide";
>>> + power-domains = <&power RK3568_PD_PIPE>;
>>> + resets = <&cru SRST_USB3OTG1>;
>>> + reset-names = "usb3-host";
>>> + snps,dis_enblslpm_quirk;
>>> + snps,dis-u2-freeclk-exists-quirk;
>>> + snps,dis_u2_susphy_quirk;
>>> + snps,dis-del-phy-power-chg-quirk;
>>> + snps,dis-tx-ipgap-linecheck-quirk;
>>
>> What was said about quirks above holds here as well (although one quirk
>> not documented in the bindings is missing here).
>
> Same thing here, I'd like absolute testing to determine that these are
> not necessary, since downstream (the oem) felt they were.
>
>>
>> Best regards,
>> Michael
>>
>>> + status = "disabled";
>>> + };
>>> +
>>> gic: interrupt-controller@fd400000 {
>>> compatible = "arm,gic-v3";
>>> reg = <0x0 0xfd400000 0 0x10000>, /* GICD */
>>> @@ -297,7 +341,6 @@ pmu_io_domains: io-domains {
>>> };
>>>
>>> pipegrf: syscon@fdc50000 {
>>> - compatible = "rockchip,rk3568-pipe-grf", "syscon";
>>> reg = <0x0 0xfdc50000 0x0 0x1000>;
>>> };
>>>

2022-02-26 02:40:47

by Michael Riesch

[permalink] [raw]
Subject: Re: [PATCH v1 7/8] arm64: dts: rockchip: add rk356x dwc3 usb3 nodes

Hi Peter,

(It should be noted that there was a slight mishap in communications
between the two of us resulting in two series with the same goal. Now
let's clean up the mess :-)

Thanks for your series. Seeing that it contains more patches than mine
it probably makes sense to use your series as basis. Please Cc: me in
future iterations of this patch series and consider my comments below.

On 2/25/22 15:54, Peter Geis wrote:
> Add the dwc3 device nodes to the rk356x device trees.
> The rk3566 has one usb2 capable dwc3 otg controller and one usb3 capable
> dwc3 host controller.
> The rk3568 has one usb3 capable dwc3 otg controller and one usb3 capable
> dwc3 host controller.
>
> Signed-off-by: Peter Geis <[email protected]>
> ---
> arch/arm64/boot/dts/rockchip/rk3566.dtsi | 12 +++++++
> arch/arm64/boot/dts/rockchip/rk3568.dtsi | 9 +++++
> arch/arm64/boot/dts/rockchip/rk356x.dtsi | 45 +++++++++++++++++++++++-
> 3 files changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi b/arch/arm64/boot/dts/rockchip/rk3566.dtsi
> index 3839eef5e4f7..8e8b52f58f44 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3566.dtsi
> @@ -6,6 +6,10 @@ / {
> compatible = "rockchip,rk3566";
> };
>
> +&pipegrf {
> + compatible = "rockchip,rk3566-pipe-grf", "syscon";
> +};
> +
> &power {
> power-domain@RK3568_PD_PIPE {
> reg = <RK3568_PD_PIPE>;
> @@ -18,3 +22,11 @@ power-domain@RK3568_PD_PIPE {
> #power-domain-cells = <0>;
> };
> };
> +
> +&usbdrd30 {

I would really love to have some alignment with the other USB controllers

usb_host{0,1}_{e,o}hci

here. I am aware that older SoCs and the SDK are using these names and
it might be painful to have different versions to maintain at the
moment, but can we please agree on

usb_host0_xhci
usb_host1_xhci

or something like that?

> + phys = <&usb2phy0_otg>;
> + phy-names = "usb2-phy";
> + extcon = <&usb2phy0>;
> + maximum-speed = "high-speed";
> + snps,dis_u2_susphy_quirk;
> +};
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> index 5b0f528d6818..77c044cbaaad 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> @@ -99,6 +99,10 @@ opp-1992000000 {
> };
> };
>
> +&pipegrf {
> + compatible = "rockchip,rk3568-pipe-grf", "syscon";
> +};
> +
> &power {
> power-domain@RK3568_PD_PIPE {
> reg = <RK3568_PD_PIPE>;
> @@ -114,3 +118,8 @@ power-domain@RK3568_PD_PIPE {
> #power-domain-cells = <0>;
> };
> };
> +
> +&usbdrd30 {
> + phys = <&usb2phy0_otg>, <&combphy0 PHY_TYPE_USB3>;
> + phy-names = "usb2-phy", "usb3-phy";
> +};
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index 84d5d607e693..4fae5b3b326e 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -230,6 +230,50 @@ scmi_shmem: sram@0 {
> };
> };
>
> + usbdrd30: usbdrd@fcc00000 {
> + compatible = "rockchip,rk3568-dwc3", "snps,dwc3";
> + reg = <0x0 0xfcc00000 0x0 0x400000>;
> + interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cru CLK_USB3OTG0_REF>, <&cru CLK_USB3OTG0_SUSPEND>,
> + <&cru ACLK_USB3OTG0>, <&cru PCLK_PIPE>;
> + clock-names = "ref_clk", "suspend_clk",
> + "bus_clk", "grf_clk";

Please consider Johan's comments on my first series. In my tests
removing the PCLK_PIPE clock did not make any difference.

> + dr_mode = "host";

Based on the description in the commit message above it should be "otg",
right? Boards are free to overrule this, of course.

> + phy_type = "utmi_wide";
> + power-domains = <&power RK3568_PD_PIPE>;
> + resets = <&cru SRST_USB3OTG0>;
> + reset-names = "usb3-otg";
> + snps,dis_enblslpm_quirk;
> + snps,dis-u2-freeclk-exists-quirk;
> + snps,dis-del-phy-power-chg-quirk;
> + snps,dis-tx-ipgap-linecheck-quirk;
> + snps,xhci-trb-ent-quirk;

In my first version I had all those quirks as well, but are they
actually necessary? I decided to remove them all to have a fresh start
(also activating them did not seem to affect my test setup).

> + status = "disabled";
> + };
> +
> + usbhost30: usbhost@fd000000 {

Please reconsider the this name as well.

> + compatible = "rockchip,rk3568-dwc3", "snps,dwc3";
> + reg = <0x0 0xfd000000 0x0 0x400000>;
> + interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cru CLK_USB3OTG1_REF>, <&cru CLK_USB3OTG1_SUSPEND>,
> + <&cru ACLK_USB3OTG1>, <&cru PCLK_PIPE>;
> + clock-names = "ref_clk", "suspend_clk",
> + "bus_clk", "grf_clk";
> + dr_mode = "host";

Here "host" clearly makes sense, as this controller is not capable of otg.

> + phys = <&usb2phy0_host>, <&combphy1 PHY_TYPE_USB3>;
> + phy-names = "usb2-phy", "usb3-phy";
> + phy_type = "utmi_wide";
> + power-domains = <&power RK3568_PD_PIPE>;
> + resets = <&cru SRST_USB3OTG1>;
> + reset-names = "usb3-host";
> + snps,dis_enblslpm_quirk;
> + snps,dis-u2-freeclk-exists-quirk;
> + snps,dis_u2_susphy_quirk;
> + snps,dis-del-phy-power-chg-quirk;
> + snps,dis-tx-ipgap-linecheck-quirk;

What was said about quirks above holds here as well (although one quirk
not documented in the bindings is missing here).

Best regards,
Michael

> + status = "disabled";
> + };
> +
> gic: interrupt-controller@fd400000 {
> compatible = "arm,gic-v3";
> reg = <0x0 0xfd400000 0 0x10000>, /* GICD */
> @@ -297,7 +341,6 @@ pmu_io_domains: io-domains {
> };
>
> pipegrf: syscon@fdc50000 {
> - compatible = "rockchip,rk3568-pipe-grf", "syscon";
> reg = <0x0 0xfdc50000 0x0 0x1000>;
> };
>