linux-kernel - Re: [PATCH v2 1/2] arm64: dts: qcom: Add support for usb nodes on QCS8300

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: <297dbc48-4c34-4bac-822c-be3ae2d00d32@oss.qualcomm.com>
Date: Fri, 25 Oct 2024 20:28:02 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Krishna Kurapati <quic_kriskura@...cinc.com>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Rob Herring <robh@...nel.org>, Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        devicetree@...r.kernel.org, quic_ppratap@...cinc.com,
        quic_jackp@...cinc.com
Subject: Re: [PATCH v2 1/2] arm64: dts: qcom: Add support for usb nodes on
 QCS8300

On 11.10.2024 9:46 AM, Krishna Kurapati wrote:

The commit title should include a `qcs8300: ` part, like others in
the directory (see git log --oneline arch/arm64/boot/dts/qcom).

> Add support for USB controllers on QCS8300. The second
> controller is only High Speed capable.
> 
> Signed-off-by: Krishna Kurapati <quic_kriskura@...cinc.com>
> ---
>  arch/arm64/boot/dts/qcom/qcs8300.dtsi | 168 ++++++++++++++++++++++++++
>  1 file changed, 168 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcs8300.dtsi b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
> index 2c35f96c3f28..4e6ba9f49b95 100644
> --- a/arch/arm64/boot/dts/qcom/qcs8300.dtsi
> +++ b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
> @@ -1363,6 +1363,174 @@ IPCC_MPROC_SIGNAL_GLINK_QMP
>  				qcom,remote-pid = <5>;
>  			};
>  		};
> +
> +		usb_1_hsphy: phy@...4000 {
> +			compatible = "qcom,qcs8300-usb-hs-phy",
> +				     "qcom,usb-snps-hs-7nm-phy";
> +			reg = <0x0 0x8904000 0x0 0x400>;

Please pad the address parts to 8 hex digits with leading zeroes.

> +
> +			clocks = <&rpmhcc RPMH_CXO_CLK>;
> +			clock-names = "ref";
> +
> +			resets = <&gcc GCC_USB2_PHY_PRIM_BCR>;
> +
> +			#phy-cells = <0>;
> +
> +			status = "disabled";
> +		};
> +
> +		usb_2_hsphy: phy@...6000 {
> +			compatible = "qcom,qcs8300-usb-hs-phy",
> +				     "qcom,usb-snps-hs-7nm-phy";
> +			reg = <0x0 0x08906000 0x0 0x400>;
> +
> +			clocks = <&rpmhcc RPMH_CXO_CLK>;
> +			clock-names = "ref";
> +
> +			resets = <&gcc GCC_USB2_PHY_SEC_BCR>;
> +
> +			#phy-cells = <0>;
> +
> +			status = "disabled";
> +		};
> +
> +		usb_qmpphy: phy@...7000 {
> +			compatible = "qcom,qcs8300-qmp-usb3-uni-phy";
> +			reg = <0x0 0x8907000 0x0 0x2000>;
> +
> +			clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
> +				 <&gcc GCC_USB_CLKREF_EN>,
> +				 <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> +				 <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> +			clock-names = "aux", "ref", "com_aux", "pipe";

Please make this a vertical list like in the node below.

[...]

> +			interconnects = <&aggre1_noc MASTER_USB3_0 0 &mc_virt SLAVE_EBI1 0>,

QCOM_ICC_TAG_ALWAYS, see x1e80100.dtsi

> +					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_0 0>;
> +			interconnect-names = "usb-ddr", "apps-usb";
> +
> +			wakeup-source;
> +
> +			status = "disabled";
> +
> +			usb_1_dwc3: usb@...0000 {
> +				compatible = "snps,dwc3";
> +				reg = <0x0 0x0a600000 0x0 0xe000>;
> +				interrupts = <GIC_SPI 292 IRQ_TYPE_LEVEL_HIGH>;
> +				iommus = <&apps_smmu 0x80 0x0>;
> +				phys = <&usb_1_hsphy>, <&usb_qmpphy>;
> +				phy-names = "usb2-phy", "usb3-phy";
> +				snps,dis_u2_susphy_quirk;
> +				snps,dis_enblslpm_quirk;

That's a very low number of quirks.. Should we have some more?

Should it also be marked dma-coherent?

Identical comments for the second instance.

Konrad