Re: [PATCH v7 07/12] arm64: dts: qcom: x1e80100-t14s: Add pm8010 camera PMIC with voltage levels for IR and RGB camera

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dts: qcom: missing vdd-l5-supply for PM8010 ldo5
- [Medium] dts: qcom: undocumented and unused regulator vreg_l7b_2p8
--

commit fc35d30463b7ad642cf725c431af83b35de74414
Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

arm64: dts: qcom: x1e80100-t14s: Add pm8010 camera PMIC with voltage levels for IR and RGB camera

This commit adds the PM8010 PMIC and configures voltage rails for the IR and
RGB sensors on the Lenovo Thinkpad T14s.

> diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
> index 0bee2a3d6e7a9..d2eaabf917f11 100644
> --- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
> @@ -562,6 +562,13 @@ vreg_l6b_1p8: ldo6 {
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  
> +		vreg_l7b_2p8: ldo7 {
> +			regulator-name = "vreg_l7b_2p8";
> +			regulator-min-microvolt = <2800000>;
> +			regulator-max-microvolt = <2800000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +		};
> +

[Severity: Medium]
Is this regulator intentionally added here?

The commit message enumerates rails only for the new PM8010 PMIC, but this
silently adds vreg_l7b_2p8 to the regulators node.

Furthermore, is vreg_l7b_2p8 used anywhere in the T14s device tree? It
appears this board uses vreg_l7m_2p8 for its camera instead, making this
addition potentially unused dead code for this specific board.

>  		vreg_l8b_3p0: ldo8 {

[ ... ]

> @@ -805,6 +812,58 @@ vreg_l3j_0p8: ldo3 {
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>  		};
>  	};
> +
> +	regulators-8 {
> +		compatible = "qcom,pm8010-rpmh-regulators";
> +		qcom,pmic-id = "m";
> +
> +		vdd-l1-l2-supply = <&vreg_s5j_1p2>;
> +		vdd-l3-l4-supply = <&vreg_s4c_1p8>;
> +		vdd-l7-supply = <&vreg_bob1>;

[Severity: Medium]
Should vdd-l5-supply be included in this list to provide the parent supply
for ldo5?

Without this, the regulator framework cannot resolve the parent supply when
vreg_l5m_2p8 is enabled. This means it will not properly enable the parent
regulator or vote for its voltage, which could lead to silent power failures
for the IR sensor if the parent is not kept on by other clients.

> +
> +		vreg_l1m_1p2: ldo1 {

[ ... ]

> +		vreg_l4m_1p8: ldo4 {
> +			regulator-name = "vreg_l4m_1p8";
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1900000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +		};
> +
> +		vreg_l5m_2p8: ldo5 {
> +			regulator-name = "vreg_l5m_2p8";
> +			regulator-min-microvolt = <2800000>;
> +			regulator-max-microvolt = <3072000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917-x1e-camss-csi2-phy-dtsi-v7-0-1a63eb35838b@linaro.org?part=7