linux-kernel - Re: [PATCH 4/5] arm64: dts: rockchip: Enable all 3 USBs on Turing RK1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ed4b6913-f19b-4280-b3b2-f5bb1e7f47eb@kwiboo.se>
Date: Thu, 12 Sep 2024 21:53:16 +0200
From: Jonas Karlman <jonas@...boo.se>
To: Sam Edwards <cfsworks@...il.com>, Heiko Stuebner <heiko@...ech.de>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Ondrej Jirman <megi@....cz>,
 Chris Morgan <macromorgan@...mail.com>, Alex Zhao <zzc@...k-chips.com>,
 Dragan Simic <dsimic@...jaro.org>, FUKAUMI Naoki <naoki@...xa.com>,
 Sebastian Reichel <sebastian.reichel@...labora.com>,
 Jing Luo <jing@...g.rocks>, Kever Yang <kever.yang@...k-chips.com>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 "linux-arm-kernel@...ts.infradead.org"
 <linux-arm-kernel@...ts.infradead.org>,
 "linux-rockchip@...ts.infradead.org" <linux-rockchip@...ts.infradead.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 Daniel Kukieła <daniel@...iela.pl>,
 Joshua Riek <jjriek@...izon.net>
Subject: Re: [PATCH 4/5] arm64: dts: rockchip: Enable all 3 USBs on Turing RK1

Hi Sam,

On 2024-09-12 04:50, Sam Edwards wrote:
> The Turing RK1 contains 3 different USBs:
> - USB0: USB 2.0, OTG
> - USB1: USB 3.0, host
> - USB2: USB 2.0, host
> 
> This patch activates the necessary DT nodes to enable all 3 buses.
> 
> Future work will be needed on USB0: it is not USB 3.0, but Linux creates
> an unused USB 3.0 root hub and the controller also requires that USBDP0
> be powered up. However, it is possible to remove this dependency. By
> either patching the xHCI driver to ignore the enumerated USB 3.0 port or
> setting usb3otg0_host_num_u3_port=0 in the GRF to stop the controller
> from enumerating a USB 3.0 port in the first place, neither Linux nor
> the controller will expect USB 3.0 capability, and USBDP0 can then
> safely be removed from the `phys` property.
> 
> Signed-off-by: Sam Edwards <CFSworks@...il.com>
> ---
>  .../boot/dts/rockchip/rk3588-turing-rk1.dtsi  | 64 +++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi
> index f6a12fe12d45..6036c4fe6727 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi
> @@ -666,3 +666,67 @@ &uart9 {
>  	pinctrl-0 = <&uart9m0_xfer>;
>  	status = "okay";
>  };
> +
> +/* USB 0: USB 2.0 only, OTG-capable */
> +&u2phy0 {
> +	status = "okay";
> +};
> +
> +&u2phy0_otg {
> +	status = "okay";
> +};
> +
> +&usbdp_phy0 {
> +	/*
> +	 * TODO: On the RK1, USBDP0 drives the DisplayPort pins, and is not
> +	 * involved in this USB2-only bus.  However, if the USB3 port is
> +	 * enabled in the xHCI below, the controller will expect this PHY to be
> +	 * powered up and holding RX_STATUS idle, or else it will generate an
> +	 * endless stream of CSC events whenever a device is plugged in.  Until
> +	 * there is a way to communicate to usb_host0_xhci that it doesn't have
> +	 * a USB3 port, we have no choice but to power up USBDP0.
> +	 */

Sounds like this may be missing

	rockchip,dp-lane-mux = <0 1 2 3>;

or

	rockchip,dp-lane-mux = <3 2 1 0>;

if all lanes are used for DP and none are used for USB.

It should help describe the hw and also help the driver set mode to
UDPHY_MODE_DP and that should disable the u3 port, or there may be an
issue to fix in the phy driver.

> +	status = "okay";
> +};
> +
> +&usb_host0_xhci {
> +	extcon = <&u2phy0>;
> +	maximum-speed = "high-speed";

If this only use the USB2 phy, this should probably also override the
default phys and phy-names props with:

	phys = <&u2phy0_otg>;
	phy-names = "usb2-phy";

Regards,
Jonas

> +	status = "okay";
> +};
> +
> +/* USB 1: USB 3.0, host only */
> +&u2phy1 {
> +	status = "okay";
> +};
> +
> +&u2phy1_otg {
> +	status = "okay";
> +};
> +
> +&usbdp_phy1 {
> +	status = "okay";
> +};
> +
> +&usb_host1_xhci {
> +	extcon = <&u2phy1>;
> +	dr_mode = "host";
> +	status = "okay";
> +};
> +
> +/* USB 2: USB 2.0, host only */
> +&u2phy2 {
> +	status = "okay";
> +};
> +
> +&u2phy2_host {
> +	status = "okay";
> +};
> +
> +&usb_host0_ehci {
> +	status = "okay";
> +};
> +
> +&usb_host0_ohci {
> +	status = "okay";
> +};