linux-kernel - Re: [PATCH v1 2/2] arm64: dts: rockchip: rk3588s: Add USB3 host controller

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] [day] [month] [year] [list]

Message-ID: <0a503489-ae6f-3299-6d4e-f14350f1ecc0@wolfvision.net>
Date:   Thu, 20 Jul 2023 09:50:49 +0200
From:   Michael Riesch <michael.riesch@...fvision.net>
To:     Sebastian Reichel <sebastian.reichel@...labora.com>,
        Heiko Stuebner <heiko@...ech.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        linux-rockchip@...ts.infradead.org, linux-usb@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, kernel@...labora.com
Subject: Re: [PATCH v1 2/2] arm64: dts: rockchip: rk3588s: Add USB3 host
 controller

Hi Sebastian,

On 7/19/23 19:40, Sebastian Reichel wrote:
> RK3588 has three USB3 controllers. One of them is host-only and uses
> the naneng-combphy. The other two are dual-role and using a different
> PHY that is not yet supported upstream.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@...labora.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 29 +++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> index b9b509257aaa..416581dd3bb5 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> @@ -443,6 +443,35 @@ usb_host1_ohci: usb@...c0000 {
>  		status = "disabled";
>  	};
>  
> +	usbhost3_0: usb@...00000 {

I'd recommend aligning the handle names with rk356x.dtsi:

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

I saw that your USB2 series follows this convention already, so let's
stick to it here as well.

> +		compatible = "rockchip,rk3588-dwc3-host", "rockchip,rk3399-dwc3";
> +		clocks = <&cru REF_CLK_USB3OTG2>, <&cru SUSPEND_CLK_USB3OTG2>,
> +			 <&cru ACLK_USB3OTG2>, <&cru CLK_UTMI_OTG2>,
> +			 <&cru PCLK_PHP_ROOT>, <&cru CLK_PIPEPHY2_PIPE_U3_G>;
> +		clock-names = "ref", "suspend", "bus", "utmi", "php", "pipe";
> +		ranges;
> +		resets = <&cru SRST_A_USB3OTG2>;
> +		reset-names = "usb3-host";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		status = "disabled";
> +
> +		usbhost_dwc3_0: usb@...00000 {

I had the impression that the embedded nodes were out of fashion and the
preferred way was to have one node with the

compatible = "rockchip,rk3568-dwc3", "snps,dwc3";

Again, try to align it with rk356x.dtsi.

Thanks and best regards,
Michael

> +			compatible = "snps,dwc3";
> +			reg = <0x0 0xfcd00000 0x0 0x400000>;
> +			interrupts = <GIC_SPI 222 IRQ_TYPE_LEVEL_HIGH 0>;
> +			dr_mode = "host";
> +			phys = <&combphy2_psu PHY_TYPE_USB3>;
> +			phy-names = "usb3-phy";
> +			phy_type = "utmi_wide";
> +			snps,dis_enblslpm_quirk;
> +			snps,dis-u2-freeclk-exists-quirk;
> +			snps,dis-del-phy-power-chg-quirk;
> +			snps,dis-tx-ipgap-linecheck-quirk;
> +			snps,dis_rxdet_inp3_quirk;
> +		};
> +	};
> +
>  	sys_grf: syscon@...8c000 {
>  		compatible = "rockchip,rk3588-sys-grf", "syscon";
>  		reg = <0x0 0xfd58c000 0x0 0x1000>;