Re: [PATCH 2/3] arm64: dts: qcom: Add Acer Swift SFA14-11

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dts: qcom: missing power-supply for backlight causes display blackout
- [High] dts: qcom: swapped QMP PHY regulators risk hardware damage
--

commit 11d3bb0749b2f4ee1528ce4d8e70f906912678bb
Author: Jianfeng Liu <liujianfeng1994@gmail.com>

arm64: dts: qcom: Add Acer Swift SFA14-11

This commit adds device tree support for the Acer Swift SFA14-11 laptop,
based on the Qualcomm X1E78100 SoC.

> diff --git a/arch/arm64/boot/dts/qcom/x1e78100-acer-sfa14-11.dts b/arch/arm64/boot/dts/qcom/x1e78100-acer-sfa14-11.dts
> new file mode 100644
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/x1e78100-acer-sfa14-11.dts
[ ... ]
> +	backlight: backlight {
> +		compatible = "pwm-backlight";
> +		pwms = <&pm8550_pwm 3 500000>;
> +		enable-gpios = <&pmc8380_3_gpios 4 GPIO_ACTIVE_HIGH>;
> +
> +		brightness-levels = <30 255>;
> +		num-interpolated-steps = <225>;
> +		default-brightness-level = <170>;
> +
> +		pinctrl-0 = <&edp_bl_en>, <&edp_bl_pwm>;
> +		pinctrl-names = "default";
> +	};

[Severity: High]
Should power-supply = <&vreg_edp_bl>; be added here to ensure the backlight
regulator is claimed and remains enabled?

[ ... ]
> +	vreg_edp_bl: regulator-edp-bl {
> +		compatible = "regulator-fixed";
> +
> +		regulator-name = "VBL9";
> +		regulator-min-microvolt = <3600000>;
> +		regulator-max-microvolt = <3600000>;
> +
> +		gpio = <&pmc8380_3_gpios 10 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +
> +		pinctrl-0 = <&edp_bl_reg_en>;
> +		pinctrl-names = "default";
> +
> +		regulator-boot-on;
> +	};

[Severity: High]
Does this regulator need an active consumer or regulator-always-on? It is
defined with regulator-boot-on, but without any consumers in the device tree
(such as the backlight node), the kernel regulator framework will
automatically disable it during late init, causing the display to black out.

[ ... ]
> +&usb_1_ss0_qmpphy {
> +	vdda-phy-supply = <&vreg_l2j_1p2>;
> +	vdda-pll-supply = <&vreg_l1j_0p8>;
> +
> +	status = "okay";
> +};

[Severity: High]
Are these supplies swapped? It looks like the 1.25V regulator (vreg_l2j_1p2)
is mapped to the 0.88V core vdda-phy-supply, and the 0.88V regulator
(vreg_l1j_0p8) is mapped to the 1.2V vdda-pll-supply. Over-voltaging the
core PHY rail can cause hardware damage, and under-voltaging the PLL rail
will cause the PLL to fail to lock.

[ ... ]
> +&usb_1_ss2_qmpphy {
> +	vdda-phy-supply = <&vreg_l2j_1p2>;
> +	vdda-pll-supply = <&vreg_l2d_0p9>;
> +
> +	/delete-property/ mode-switch;

[Severity: High]
Could the vdda-phy-supply and vdda-pll-supply be backwards here as well? The
1.25V regulator (vreg_l2j_1p2) is mapped to the 0.88V core vdda-phy-supply,
and the 0.9V regulator (vreg_l2d_0p9) is mapped to the 1.2V vdda-pll-supply.

[ ... ]
> +&usb_mp_qmpphy0 {
> +	vdda-phy-supply = <&vreg_l3e_1p2>;
> +	vdda-pll-supply = <&vreg_l3c_0p8>;
> +
> +	status = "okay";
> +};

[Severity: High]
Is this another instance where the supplies are swapped? It looks like the
1.2V regulator (vreg_l3e_1p2) is supplied to the 0.9V core vdda-phy-supply,
and the 0.9V regulator (vreg_l3c_0p8) is supplied to the 1.2V vdda-pll-supply.

[ ... ]
> +&usb_mp_qmpphy1 {
> +	vdda-phy-supply = <&vreg_l3e_1p2>;
> +	vdda-pll-supply = <&vreg_l3c_0p8>;
> +
> +	status = "okay";
> +};

[Severity: High]
Does this have the same swap issue as usb_mp_qmpphy0? The 1.2V regulator
(vreg_l3e_1p2) is mapped to the 0.9V vdda-phy-supply, and the 0.9V regulator
(vreg_l3c_0p8) is mapped to the 1.2V vdda-pll-supply.

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