Re: [PATCH v4 1/1] arm64: dts: qcom: Add Microsoft Surface Pro 12in

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Numerous LDO regulators are missing their required parent supply properties (vdd-lX-supply) in the PMIC regulator nodes, breaking the Linux regulator supply chain.
--

commit 6be7c2c68f42bc5f87da842354ed30a95efb06f4
Author: Harrison Vanderbyl <harrison.vanderbyl@gmail.com>

arm64: dts: qcom: Add Microsoft Surface Pro 12in

Initial device tree for Microsoft Surface Pro 12in

> diff --git a/arch/arm64/boot/dts/qcom/x1p42100-microsoft-sp12in.dts b/arch/arm64/boot/dts/qcom/x1p42100-microsoft-sp12in.dts
> new file mode 100644
> index 0000000000000..0314b2246cdb7
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/x1p42100-microsoft-sp12in.dts

[ ... ]

> +	regulators-2 {
> +		compatible = "qcom,pmc8380-rpmh-regulators";
> +		qcom,pmic-id = "d";
> +
> +		vdd-l3-supply = <&vreg_s4c_1p8>;
> +		vdd-s1-supply = <&vph_pwr>;
> +

[Severity: High]
Does this node miss vdd-l1-supply and vdd-l2-supply?

It defines vreg_l1d_0p8 and vreg_l2d_0p9 below, but without their parent
supply properties, the Linux regulator framework cannot enable the parent
when these child LDOs are enabled.

If the parent SMPS is turned off, could it cause power loss to the devices
relying on these regulators?

> +		vreg_l1d_0p8: ldo1 {
> +			regulator-name = "vreg_l1d_0p8";
> +			regulator-min-microvolt = <880000>;
> +			regulator-max-microvolt = <920000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +		};
> +
> +		vreg_l2d_0p9: ldo2 {
> +			regulator-name = "vreg_l2d_0p9";
> +			regulator-min-microvolt = <912000>;
> +			regulator-max-microvolt = <920000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +		};

[ ... ]

> +	regulators-6 {
> +		compatible = "qcom,pm8550ve-rpmh-regulators";
> +		qcom,pmic-id = "i";
> +
> +		vdd-l1-supply = <&vreg_s4c_1p8>;
> +		vdd-l2-supply = <&vreg_s5j_1p2>;
> +		vdd-s1-supply = <&vph_pwr>;
> +		vdd-s2-supply = <&vph_pwr>;
> +

[Severity: High]
Does this also miss vdd-l3-supply for vreg_l3i_0p8 defined below?

This LDO is used by UFS and PCIe PHYs. If the parent isn't enabled, this
could result in system crashes or peripheral failures.

> +		vreg_s1i_0p9: smps1 {
> +			regulator-name = "vreg_s1i_0p9";

[ ... ]

> +		vreg_l3i_0p8: ldo3 {
> +			regulator-name = "vreg_l3i_0p8";
> +			regulator-min-microvolt = <880000>;
> +			regulator-max-microvolt = <880000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +			regulator-allow-set-load;
> +			regulator-allowed-modes = <RPMH_REGULATOR_MODE_LPM
> +						   RPMH_REGULATOR_MODE_HPM>;
> +		};
> +	};
> +
> +	regulators-7 {
> +		compatible = "qcom,pm8550ve-rpmh-regulators";
> +		qcom,pmic-id = "j";
> +
> +		vdd-l2-supply = <&vreg_s5j_1p2>;
> +		vdd-s5-supply = <&vph_pwr>;
> +

[Severity: High]
Similarly, do we need vdd-l1-supply and vdd-l3-supply here?

This node defines vreg_l1j_0p8 and vreg_l3j_0p8 (used by UFS vdd-hba-supply
and USB PHYs) below.

Since the pmc8380_6 node was deleted at the top of the file, did these
supplies need to be re-routed instead of just being omitted?

> +		vreg_s5j_1p2: smps5 {
> +			regulator-name = "vreg_s5j_1p2";

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609145906.40854-2-harrison.vanderbyl@gmail.com?part=1