Re: [PATCH v6 5/5] arm64: dts: qcom: sm8750-lenovo-elden: describe the WL2868C regulators

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] arm64: dts: qcom: sm8750-lenovo-elden: incomplete WL2868C supplies on stub DT
--

commit 0758c044c3db5c70cf435a167f553582f960cdb2
Author: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>

arm64: dts: qcom: sm8750-lenovo-elden: describe the WL2868C regulators

This commit enables gpi_dma2, qupv3_2, and i2c10 on the Lenovo Legion Y700
Gen4 devicetree and adds the WL2868C camera PMIC node along with its LDO
regulators and a fixed VSYS supply.

> diff --git a/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts b/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
> index 821950715c8e4..73f4452f46120 100644
> --- a/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
> @@ -5,6 +5,7 @@
>  
>  /dts-v1/;
>  
> +#include <dt-bindings/gpio/gpio.h>
>  #include "sm8750.dtsi"
>  
>  / {
> @@ -43,8 +44,86 @@ bi_tcxo_ao_div2: bi-tcxo-ao-div2-clk {
>  			clock-div = <2>;
>  		};
>  	};
> +
> +	/*
> +	 * VSYS of the camera PMIC is the always-on system rail of the tablet.
> +	 * The SPMI PMICs that generate it are not described yet, so it is
> +	 * modelled as a fixed regulator without a voltage constraint.
> +	 */
> +	wl2868c_vsys: regulator-vsys {
> +		compatible = "regulator-fixed";
> +		regulator-name = "wl2868c_vsys";
> +		regulator-always-on;
> +	};
> +};
> +
> +&gpi_dma2 {
> +	status = "okay";
> +};
> +
> +&qupv3_2 {
> +	status = "okay";
> +};
> +
> +&i2c10 {
> +	status = "okay";
> +
> +	pmic@2f {
> +		compatible = "willsemi,wl2868c";
> +		reg = <0x2f>;
> +		reset-gpios = <&tlmm 126 GPIO_ACTIVE_LOW>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&camera_pmic_default>;
> +		vsys-supply = <&wl2868c_vsys>;
> +
> +		regulators {
> +			/* Voltages include the offsets programmed by stock firmware. */
> +			camera_ldo1: ldo1 {
> +				regulator-name = "camera_ldo1";
> +				regulator-min-microvolt = <1128000>;
> +				regulator-max-microvolt = <1128000>;
> +			};

[Severity: Medium]
As reported by Konrad Dybcio on v5 [1], does adding the pmic@2f node to
sm8750-lenovo-elden.dts before the board SPMI PMICs, RPMh regulators, and
camera consumers are present provide a complete or usable hardware
description beyond what the DT binding example in
Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml already
covers?

In drivers/regulator/wl2868c-regulator.c, wl2868c_regulators[] defines
supply_name as "vin12" for ldo1 and ldo2, "vin34" for ldo3 and ldo4, "vin5"
for ldo5, and "vin7" for ldo7. When wl2868c_probe() registers each regulator
via devm_regulator_register(), the regulator core resolves those parent input
supplies on pmic@2f.

Because sm8750-lenovo-elden.dts does not yet describe the SPMI PMICs or RPMh
regulators (nor the basic board infrastructure to boot), pmic@2f omits
vin12-supply, vin34-supply, vin5-supply, and vin7-supply and only provides a
placeholder regulator-fixed wl2868c_vsys node without voltage constraints,
causing all six registered LDOs to fall back to dummy parent supplies with no
consumers attached.

Would it be better to introduce this node together with the board PMICs and
camera consumers once the parent supplies can be properly wired up?

[1] https://lore.kernel.org/r/dddc808a-6407-4db1-aa72-0ebe5652f97c@oss.qualcomm.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104032648.209747-1-xiaqinghuan@mail.ustc.edu.cn?part=5