[PATCH] arm64: dts: rockchip: add usb3 controller node for RK3328 SoCs

[PATCH] arm64: dts: rockchip: add usb3 controller node for RK3328 SoCs

[Leonidas P. Papadakos]

From: William Wu <william.wu@rock-chips.com>

RK3328 has one USB 3.0 OTG controller which uses DWC_USB3
core's general architecture. It can act as static xHCI host
controller, static device controller, USB 3.0/2.0 OTG basing
on ID of USB3.0 PHY.

Signed-off-by: William Wu <william.wu@rock-chips.com>
Signed-off-by: Leonidas P. Papadakos <papadakospan@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3328.dtsi | 27 ++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
index 84f14b132..f5c1ab596 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
@@ -917,6 +917,33 @@
 		status = "disabled";
 	};
 
+	usbdrd3: usb@ff600000 {
+		compatible = "rockchip,rk3328-dwc3", "rockchip,rk3399-dwc3";
+		clocks = <&cru SCLK_USB3OTG_REF>, <&cru SCLK_USB3OTG_SUSPEND>,
+			 <&cru ACLK_USB3OTG>;
+		clock-names = "ref_clk", "suspend_clk",
+			      "bus_clk";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+		status = "disabled";
+
+		usbdrd_dwc3: dwc3@ff600000 {
+			compatible = "snps,dwc3";
+			reg = <0x0 0xff600000 0x0 0x100000>;
+			interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
+			dr_mode = "otg";
+			phy_type = "utmi_wide";
+			snps,dis_enblslpm_quirk;
+			snps,dis-u2-freeclk-exists-quirk;
+			snps,dis_u2_susphy_quirk;
+			snps,dis_u3_susphy_quirk;
+			snps,dis-del-phy-power-chg-quirk;
+			snps,dis-tx-ipgap-linecheck-quirk;
+			status = "disabled";
+		};
+	};
+
 	gic: interrupt-controller@ff811000 {
 		compatible = "arm,gic-400";
 		#interrupt-cells = <3>;
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: dts: rockchip: add usb3 controller node for RK3328 SoCs

[Heiko Stübner]

Hi,

Am Donnerstag, 14. März 2019, 19:20:35 CET schrieb Leonidas P. Papadakos:
> From: William Wu <william.wu@rock-chips.com>
> 
> RK3328 has one USB 3.0 OTG controller which uses DWC_USB3
> core's general architecture. It can act as static xHCI host
> controller, static device controller, USB 3.0/2.0 OTG basing
> on ID of USB3.0 PHY.
> 
> Signed-off-by: William Wu <william.wu@rock-chips.com>
> Signed-off-by: Leonidas P. Papadakos <papadakospan@gmail.com>

the rk3328 usb3-phy has an issue with detecting any plugin events
after a previous device got removed - see the inno-usb3-phy driver
in the vendor kernel.

So once you unplug an usb device from the port it won't detect
a newly connected device. This is also the reason why the original
usb3 patches are still sitting around in my tree, as I don't want to
codify a binding that may not last once somebody actually manages
to go around that issue with kernel code - especially as the node
below claims to be compatible with the rk3399 dwc3.


Heiko

> ---
>  arch/arm64/boot/dts/rockchip/rk3328.dtsi | 27 ++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3328.dtsi index 84f14b132..f5c1ab596
> 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> @@ -917,6 +917,33 @@
>  		status = "disabled";
>  	};
> 
> +	usbdrd3: usb@ff600000 {
> +		compatible = "rockchip,rk3328-dwc3", "rockchip,rk3399-dwc3";
> +		clocks = <&cru SCLK_USB3OTG_REF>, <&cru SCLK_USB3OTG_SUSPEND>,
> +			 <&cru ACLK_USB3OTG>;
> +		clock-names = "ref_clk", "suspend_clk",
> +			      "bus_clk";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +		status = "disabled";
> +
> +		usbdrd_dwc3: dwc3@ff600000 {
> +			compatible = "snps,dwc3";
> +			reg = <0x0 0xff600000 0x0 0x100000>;
> +			interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
> +			dr_mode = "otg";
> +			phy_type = "utmi_wide";
> +			snps,dis_enblslpm_quirk;
> +			snps,dis-u2-freeclk-exists-quirk;
> +			snps,dis_u2_susphy_quirk;
> +			snps,dis_u3_susphy_quirk;
> +			snps,dis-del-phy-power-chg-quirk;
> +			snps,dis-tx-ipgap-linecheck-quirk;
> +			status = "disabled";
> +		};
> +	};
> +
>  	gic: interrupt-controller@ff811000 {
>  		compatible = "arm,gic-400";
>  		#interrupt-cells = <3>;





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: dts: rockchip: add usb3 controller node for RK3328 SoCs

[Leonidas P. Papadakos]

Στις Πεμ, 14 Μαρ, 2019 at 9:02 ΜΜ, ο/η Heiko 
=?iso-8859-1?q?St=FCbner?= <heiko@sntech.de> έγραψε:
> Hi,
> 
> Am Donnerstag, 14. März 2019, 19:20:35 CET schrieb Leonidas P. 
> Papadakos:
>>  From: William Wu <william.wu@rock-chips.com>
>> 
>>  RK3328 has one USB 3.0 OTG controller which uses DWC_USB3
>>  core's general architecture. It can act as static xHCI host
>>  controller, static device controller, USB 3.0/2.0 OTG basing
>>  on ID of USB3.0 PHY.
>> 
>>  Signed-off-by: William Wu <william.wu@rock-chips.com>
>>  Signed-off-by: Leonidas P. Papadakos <papadakospan@gmail.com>
> 
> the rk3328 usb3-phy has an issue with detecting any plugin events
> after a previous device got removed - see the inno-usb3-phy driver
> in the vendor kernel.
> 
> So once you unplug an usb device from the port it won't detect
> a newly connected device. This is also the reason why the original
> usb3 patches are still sitting around in my tree, as I don't want to
> codify a binding that may not last once somebody actually manages
> to go around that issue with kernel code - especially as the node
> below claims to be compatible with the rk3399 dwc3.
> 
> 
> Heiko
> 
>>  ---
>>   arch/arm64/boot/dts/rockchip/rk3328.dtsi | 27 
>> ++++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>> 
>>  diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
>>  b/arch/arm64/boot/dts/rockchip/rk3328.dtsi index 
>> 84f14b132..f5c1ab596
>>  100644
>>  --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
>>  +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
>>  @@ -917,6 +917,33 @@
>>   		status = "disabled";
>>   	};
>> 
>>  +	usbdrd3: usb@ff600000 {
>>  +		compatible = "rockchip,rk3328-dwc3", "rockchip,rk3399-dwc3";
>>  +		clocks = <&cru SCLK_USB3OTG_REF>, <&cru SCLK_USB3OTG_SUSPEND>,
>>  +			 <&cru ACLK_USB3OTG>;
>>  +		clock-names = "ref_clk", "suspend_clk",
>>  +			      "bus_clk";
>>  +		#address-cells = <2>;
>>  +		#size-cells = <2>;
>>  +		ranges;
>>  +		status = "disabled";
>>  +
>>  +		usbdrd_dwc3: dwc3@ff600000 {
>>  +			compatible = "snps,dwc3";
>>  +			reg = <0x0 0xff600000 0x0 0x100000>;
>>  +			interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
>>  +			dr_mode = "otg";
>>  +			phy_type = "utmi_wide";
>>  +			snps,dis_enblslpm_quirk;
>>  +			snps,dis-u2-freeclk-exists-quirk;
>>  +			snps,dis_u2_susphy_quirk;
>>  +			snps,dis_u3_susphy_quirk;
>>  +			snps,dis-del-phy-power-chg-quirk;
>>  +			snps,dis-tx-ipgap-linecheck-quirk;
>>  +			status = "disabled";
>>  +		};
>>  +	};
>>  +
>>   	gic: interrupt-controller@ff811000 {
>>   		compatible = "arm,gic-400";
>>   		#interrupt-cells = <3>;
> 
> 
> 
> 
Now it makes sense. I thought it was board specific. It's a general 
rk3328 issue, then.
Thanks for clearing it up. (yet again, oops)



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: dts: rockchip: add usb3 controller node for RK3328 SoCs

[Leonidas P. Papadakos]

I must say with my testing, I never really have any disconnects. My 
drive generally works
Surely it's better that it works somewhat than not at all.
Maybe we could include the patches and revert them later if a solution 
arrises?

Or is that against the Linux way of doing things?



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: dts: rockchip: add usb3 controller node for RK3328 SoCs

[Heiko Stübner]

Hi again :-)

Am Donnerstag, 14. März 2019, 20:08:30 CET schrieb Leonidas P. Papadakos:
> I must say with my testing, I never really have any disconnects. My
> drive generally works

It's not any disconnects, it is really when you unplug the device from
the root port that the 

> Surely it's better that it works somewhat than not at all.
> Maybe we could include the patches and revert them later if a solution
> arrises?
> 
> Or is that against the Linux way of doing things?

The main issue is
               compatible = "rockchip,rk3328-dwc3", "rockchip,rk3399-dwc3";

because:
(1) On the devicetree-side you declare that they are compatible which may
    or may not conflict with needed later changes
(2) On the kernel-driver-side with the current status the rk3328-dwc3
    will get ignored and rk3399-dwc3 used to bind to the generic dt-dwc3
    driver.
    From looking at the Rockchip code in the vendor kernel we may very well
    need a separate driver to handle the big issue which creates the issue
    which driver will bind to the dt-node. It will be either the generic one
    binding to the rk3399-dwc3 or the special one binding to rk3328-dwc3.
    And sadly it will probably depend on which module gets loaded first.

And devicetrees are supposed to be backwards compatible so a newer
kernel should keep working with an old devicetree.

A possible way forward would be to introduce a separate compatible for
rk3328 (both in the binding doc and the generic dwc3 driver), so that
we get only a
               compatible = "rockchip,rk3328-dwc3";

Which then even can move easily into a new driver if necessary without
causing issues for existing devicetrees.

Heiko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: dts: rockchip: add usb3 controller node for RK3328 SoCs

[Peter Geis]

Has anyone tested this since Jonas Karlman's clk patch landed?
I noticed a lot of my USB hotplug issues disappeared when I pulled
that patch in.
Especially since one of the fixes was aclk_usb3otg, since it was
incorrectly defined to usb2's aclk.

On Thu, Mar 14, 2019 at 3:21 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Hi again :-)
>
> Am Donnerstag, 14. März 2019, 20:08:30 CET schrieb Leonidas P. Papadakos:
> > I must say with my testing, I never really have any disconnects. My
> > drive generally works
>
> It's not any disconnects, it is really when you unplug the device from
> the root port that the
>
> > Surely it's better that it works somewhat than not at all.
> > Maybe we could include the patches and revert them later if a solution
> > arrises?
> >
> > Or is that against the Linux way of doing things?
>
> The main issue is
>                compatible = "rockchip,rk3328-dwc3", "rockchip,rk3399-dwc3";
>
> because:
> (1) On the devicetree-side you declare that they are compatible which may
>     or may not conflict with needed later changes
> (2) On the kernel-driver-side with the current status the rk3328-dwc3
>     will get ignored and rk3399-dwc3 used to bind to the generic dt-dwc3
>     driver.
>     From looking at the Rockchip code in the vendor kernel we may very well
>     need a separate driver to handle the big issue which creates the issue
>     which driver will bind to the dt-node. It will be either the generic one
>     binding to the rk3399-dwc3 or the special one binding to rk3328-dwc3.
>     And sadly it will probably depend on which module gets loaded first.
>
> And devicetrees are supposed to be backwards compatible so a newer
> kernel should keep working with an old devicetree.
>
> A possible way forward would be to introduce a separate compatible for
> rk3328 (both in the binding doc and the generic dwc3 driver), so that
> we get only a
>                compatible = "rockchip,rk3328-dwc3";
>
> Which then even can move easily into a new driver if necessary without
> causing issues for existing devicetrees.
>
> Heiko
>
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: dts: rockchip: add usb3 controller node for RK3328 SoCs

[Jonas Karlman]

I have been using that clk patch since I started initial testing of rk3328 on mainline 4.16 [1]
and I cannot remember seeing any real issue with usb2/usb3 (beside usb regulator related issue).
The clk patch may have helped with some usb issues that I never experienced :-)

[1] https://github.com/Kwiboo/linux-rockchip/commits/rockchip-4.16/drivers/clk/rockchip

Regards,
Jonas

On 2019-03-14 20:24, Peter Geis wrote:
> Has anyone tested this since Jonas Karlman's clk patch landed?
> I noticed a lot of my USB hotplug issues disappeared when I pulled
> that patch in.
> Especially since one of the fixes was aclk_usb3otg, since it was
> incorrectly defined to usb2's aclk.
>
> On Thu, Mar 14, 2019 at 3:21 PM Heiko Stübner <heiko@sntech.de> wrote:
>> Hi again :-)
>>
>> Am Donnerstag, 14. März 2019, 20:08:30 CET schrieb Leonidas P. Papadakos:
>>> I must say with my testing, I never really have any disconnects. My
>>> drive generally works
>> It's not any disconnects, it is really when you unplug the device from
>> the root port that the
>>
>>> Surely it's better that it works somewhat than not at all.
>>> Maybe we could include the patches and revert them later if a solution
>>> arrises?
>>>
>>> Or is that against the Linux way of doing things?
>> The main issue is
>>                compatible = "rockchip,rk3328-dwc3", "rockchip,rk3399-dwc3";
>>
>> because:
>> (1) On the devicetree-side you declare that they are compatible which may
>>     or may not conflict with needed later changes
>> (2) On the kernel-driver-side with the current status the rk3328-dwc3
>>     will get ignored and rk3399-dwc3 used to bind to the generic dt-dwc3
>>     driver.
>>     From looking at the Rockchip code in the vendor kernel we may very well
>>     need a separate driver to handle the big issue which creates the issue
>>     which driver will bind to the dt-node. It will be either the generic one
>>     binding to the rk3399-dwc3 or the special one binding to rk3328-dwc3.
>>     And sadly it will probably depend on which module gets loaded first.
>>
>> And devicetrees are supposed to be backwards compatible so a newer
>> kernel should keep working with an old devicetree.
>>
>> A possible way forward would be to introduce a separate compatible for
>> rk3328 (both in the binding doc and the generic dwc3 driver), so that
>> we get only a
>>                compatible = "rockchip,rk3328-dwc3";
>>
>> Which then even can move easily into a new driver if necessary without
>> causing issues for existing devicetrees.
>>
>> Heiko
>>
>>
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@lists.infradead.org
>> https://nam01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infradead.org%2Fmailman%2Flistinfo%2Flinux-rockchip&amp;data=02%7C01%7C%7C9a4c35bda0bf40b48ce508d6a8b2bb01%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636881882922347560&amp;sdata=GJpD0AQPTcCT0BLjb%2BKKdjSMlPKvniqOxigx5qnOEmA%3D&amp;reserved=0
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> https://nam01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infradead.org%2Fmailman%2Flistinfo%2Flinux-rockchip&amp;data=02%7C01%7C%7C9a4c35bda0bf40b48ce508d6a8b2bb01%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636881882922347560&amp;sdata=GJpD0AQPTcCT0BLjb%2BKKdjSMlPKvniqOxigx5qnOEmA%3D&amp;reserved=0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: dts: rockchip: add usb3 controller node for RK3328 SoCs

[Jonas Karlman]

The usb3 issue Heiko describe is still there, my usb3 device is only discovered once on my Rock 64.
I must have only tested inserting my usb3 devices once when I testing before.

Regards,
Jonas

On 2019-03-14 20:52, Jonas Karlman wrote:
> I have been using that clk patch since I started initial testing of rk3328 on mainline 4.16 [1]
> and I cannot remember seeing any real issue with usb2/usb3 (beside usb regulator related issue).
> The clk patch may have helped with some usb issues that I never experienced :-)
>
> [1] https://github.com/Kwiboo/linux-rockchip/commits/rockchip-4.16/drivers/clk/rockchip
>
> Regards,
> Jonas
>
> On 2019-03-14 20:24, Peter Geis wrote:
>> Has anyone tested this since Jonas Karlman's clk patch landed?
>> I noticed a lot of my USB hotplug issues disappeared when I pulled
>> that patch in.
>> Especially since one of the fixes was aclk_usb3otg, since it was
>> incorrectly defined to usb2's aclk.
>>
>> On Thu, Mar 14, 2019 at 3:21 PM Heiko Stübner <heiko@sntech.de> wrote:
>>> Hi again :-)
>>>
>>> Am Donnerstag, 14. März 2019, 20:08:30 CET schrieb Leonidas P. Papadakos:
>>>> I must say with my testing, I never really have any disconnects. My
>>>> drive generally works
>>> It's not any disconnects, it is really when you unplug the device from
>>> the root port that the
>>>
>>>> Surely it's better that it works somewhat than not at all.
>>>> Maybe we could include the patches and revert them later if a solution
>>>> arrises?
>>>>
>>>> Or is that against the Linux way of doing things?
>>> The main issue is
>>>                compatible = "rockchip,rk3328-dwc3", "rockchip,rk3399-dwc3";
>>>
>>> because:
>>> (1) On the devicetree-side you declare that they are compatible which may
>>>     or may not conflict with needed later changes
>>> (2) On the kernel-driver-side with the current status the rk3328-dwc3
>>>     will get ignored and rk3399-dwc3 used to bind to the generic dt-dwc3
>>>     driver.
>>>     From looking at the Rockchip code in the vendor kernel we may very well
>>>     need a separate driver to handle the big issue which creates the issue
>>>     which driver will bind to the dt-node. It will be either the generic one
>>>     binding to the rk3399-dwc3 or the special one binding to rk3328-dwc3.
>>>     And sadly it will probably depend on which module gets loaded first.
>>>
>>> And devicetrees are supposed to be backwards compatible so a newer
>>> kernel should keep working with an old devicetree.
>>>
>>> A possible way forward would be to introduce a separate compatible for
>>> rk3328 (both in the binding doc and the generic dwc3 driver), so that
>>> we get only a
>>>                compatible = "rockchip,rk3328-dwc3";
>>>
>>> Which then even can move easily into a new driver if necessary without
>>> causing issues for existing devicetrees.
>>>
>>> Heiko
>>>
>>>
>>>
>>> _______________________________________________
>>> Linux-rockchip mailing list
>>> Linux-rockchip@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel