linux-kernel - Re: [PATCH v2 1/2] arm64: dts: qcom: sc8280xp-wdk2023: dt definition for WDK2023

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: <ZvWTAEwEcmh76Xhe@hu-bjorande-lv.qualcomm.com>
Date: Thu, 26 Sep 2024 09:59:44 -0700
From: Bjorn Andersson <quic_bjorande@...cinc.com>
To: <jens.glathe@...schoolsolutions.biz>
CC: Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio
	<konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski
	<krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, <linux-arm-msm@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Merck Hung <merckhung@...il.com>
Subject: Re: [PATCH v2 1/2] arm64: dts: qcom: sc8280xp-wdk2023: dt definition
 for WDK2023

On Fri, Sep 20, 2024 at 07:47:24PM +0200, Jens Glathe via B4 Relay wrote:
[..]
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-microsoft-blackrock.dts b/arch/arm64/boot/dts/qcom/sc8280xp-microsoft-blackrock.dts
[..]
> +/ {
> +	model = "Windows Dev Kit 2023";
> +	compatible = "microsoft,blackrock", "qcom,sc8280xp";
> +
> +	aliases {
> +		i2c4 = &i2c4;
> +		i2c21 = &i2c21;
> +		serial1 = &uart2;
> +	};
> +
> +	wcd938x: audio-codec {
> +		compatible = "qcom,wcd9380-codec";
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&wcd_default>;
> +
> +		reset-gpios = <&tlmm 106 GPIO_ACTIVE_LOW>;
> +
> +		vdd-buck-supply = <&vreg_s10b>;
> +		vdd-rxtx-supply = <&vreg_s10b>;
> +		vdd-io-supply = <&vreg_s10b>;
> +		vdd-mic-bias-supply = <&vreg_bob>;
> +
> +		qcom,micbias1-microvolt = <1800000>;
> +		qcom,micbias2-microvolt = <1800000>;
> +		qcom,micbias3-microvolt = <1800000>;
> +		qcom,micbias4-microvolt = <1800000>;
> +		qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>;
> +		qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
> +		qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
> +		qcom,rx-device = <&wcd_rx>;
> +		qcom,tx-device = <&wcd_tx>;
> +
> +		#sound-dai-cells = <1>;
> +	};
> +
> +	firmware {
> +		qseecom {
> +			compatible = "qcom,qseecom-sc8280xp", "qcom,qseecom";
> +			qcom,scm = <&scm>;
> +		};
> +	};
> +
> +	pmic-glink {
[..]
> +	};
> +
> +	dp1-connector@0 {

Please keep nodes sorted by {address, node name, label}, i.e. this
should move 2 steps up.

Also the unit address (i.e. the "@0" part) should be dropped, right?

> +		compatible = "dp-connector";
> +		label = "DP1";
> +		type = "mini";
> +
> +		dp-pwr-supply = <&vreg_misc_3p3>;
> +
> +		//hpd-gpios = <&tlmm 2 GPIO_ACTIVE_HIGH>;
> +
> +		port {
> +			dp1_connector_in: endpoint {
> +				remote-endpoint = <&mdss0_dp2_phy_out>;
> +			};
> +		};
> +	};
> +
[..]
> +&mdss0_dp2 {
> +  compatible = "qcom,sc8280xp-dp";
> +
> +  data-lanes = <0 1 2 3>;
> +
> +	status = "okay";

Indentation looks off here.

> +
> +	ports {
> +		port@1 {
> +			reg = <1>;
> +			mdss0_dp2_phy_out: endpoint {
> +				remote-endpoint = <&dp1_connector_in>;
> +			};
> +		};
> +	};
> +};
> +
> +&mdss0_dp2_phy {
> +	compatible = "qcom,sc8280xp-dp-phy";
> +
> +	vdda-phy-supply = <&vreg_l3b>;
> +	phy-supply = <&vreg_l3b>;
> +	vdda-pll-supply = <&vreg_l6b>;
> +	pll-supply = <&vreg_l6b>;
> +
> +	status = "okay";
> +};
> +
> +&pcie2a {
> +	aspm-no-l0s;
> +	max-link-speed = <16>;
> +
> +	perst-gpios = <&tlmm 143 GPIO_ACTIVE_LOW>;
> +	wake-gpios = <&tlmm 145 GPIO_ACTIVE_LOW>;
> +
> +	vddpe-3v3-supply = <&vreg_nvme>;
> +	pcie-vdda-supply = <&vreg_l7d>;
> +	vdda-supply = <&vreg_l7d>;
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie2a_default>;
> +
> +	status = "okay";
> +};
> +
> +&pcie2a_phy {
> +	vdda-phy-supply = <&vreg_l4d>;
> +	phy-supply = <&vreg_l4d>;
> +	vdda-pll-supply = <&vreg_l6d>;
> +	pll-supply = <&vreg_l6d>;
> +
> +	status = "okay";
> +};
> +
> +&pcie4 {
> +	max-link-speed = <2>;
> +	aspm-no-l0s;

It would be nice if these two followed the same order here and in
pcie2a. (Don't know if we have a preferred order...)

> +
> +	perst-gpios = <&tlmm 141 GPIO_ACTIVE_LOW>;
> +	wake-gpios = <&tlmm 139 GPIO_ACTIVE_LOW>;
> +
> +	vddpe-3v3-supply = <&vreg_wlan>;
> +	pcie-vdda-supply = <&vreg_l7d>;
> +	vdda-supply = <&vreg_l7d>;
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie4_default>;
> +
> +	status = "okay";
> +};
> +
> +&pcie4_port0 {
> +	wifi@0 {
> +		compatible = "pci17cb,1103";
> +		reg = <0x10000 0x0 0x0 0x0 0x0>;
> +
> +		qcom,ath11k-calibration-variant = "volterra";

Can you please confirm that this matches what has landed in
linux-firmware?

> +	};
> +};
> +
[..]
> +&remoteproc_adsp {
> +	firmware-name = "qcom/sc8280xp/MICROSOFT/DEVKIT23/qcadsp8280.mbn";

Perhaps use blackrock here instead?

> +
> +	status = "okay";
> +};

Regards,
Bjorn