Re: [PATCH v8 3/3] arm64: dts: qcom: Add hamoa Samsung Galaxy Book4 Edge devicetrees

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dts: swapped voltage regulators for USB QMP PHY nodes
- [Medium] dts: invalid power topology supplying 3.3V switch from 1.8V source
--

commit e084aaae679956c0579caf519602293f9f79b553
Author: Maxim Storetvedt <mstoretv@cern.ch>
Subject: arm64: dts: qcom: Add hamoa Samsung Galaxy Book4 Edge devicetrees

Adds devicetrees for the 14-inch and 16-inch hamoa SKUs of the Samsung
Galaxy Book4 Edge based on a common dtsi.

Link: https://lore.kernel.org/all/p3mhtj2rp6y2ezuwpd2gu7dwx5cbckfu4s4pazcudi4j2wogtr@4yecb2bkeyms/
Link: https://lore.kernel.org/all/20260629154812.9066-1-mail@etehtsea.me/

> diff --git a/arch/arm64/boot/dts/qcom/x1-samsung-galaxy-book4-edge.dtsi b/arch/arm64/boot/dts/qcom/x1-samsung-galaxy-book4-edge.dtsi
> new file mode 100644
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/x1-samsung-galaxy-book4-edge.dtsi
[ ... ]
> +	vreg_display_vdd: regulator-display-vdd {
> +		compatible = "regulator-fixed";
> +
> +		regulator-name = "VREG_DISPLAY_VDD";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +
> +		gpio = <&tlmm 120 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +
> +		pinctrl-0 = <&display_vdd_en>;
> +		pinctrl-names = "default";
> +
> +		startup-delay-us = <400000>;
> +		regulator-enable-ramp-delay = <400000>;
> +
> +		vin-supply = <&vreg_octa_vdd>;

[Severity: Medium]
Is it expected that this 3.3V regulator uses a 1.8V input supply?

Looking at the definition of vreg_octa_vdd in this file, it is fixed at
1.8V. A 3.3V switch or LDO fed by a 1.8V source will cause the regulator
core to flag a 'supply voltage too low' or unsupportable constraints error
during probe when attempting to resolve the display voltage.

> +
> +		regulator-always-on;
> +		regulator-boot-on;
> +	};
[ ... ]
> +&usb_1_ss0_qmpphy {
> +	vdda-phy-supply = <&vreg_l2j_1p2>;
> +	vdda-pll-supply = <&vreg_l1j_0p8>;

[Severity: High]
Could these voltage supplies be swapped?

The 1.2V supply (&vreg_l2j_1p2) is assigned to the 0.8V PHY core rail
(vdda-phy-supply), while the 0.8V supply (&vreg_l1j_0p8) is assigned to the
1.2V PLL rail (vdda-pll-supply).

During device probe, this swapped regulator assignment will activate,
causing a significant overvoltage on the PHY core and undervoltage on the
PLL, which could lead to hardware damage or initialization failures.

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

[Severity: High]
Does this node also swap the voltage supplies?

Similar to &usb_1_ss0_qmpphy above, a 1.2V supply is incorrectly assigned
to the 0.9V PHY core rail.

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

[Severity: High]
Does this node also swap the voltage supplies?

Similar to the earlier QMP PHY nodes, a 1.2V supply is incorrectly assigned
to the 0.9V PHY core rail here as well.

> +
> +	/delete-property/ mode-switch;
[ ... ]
> +&usb_mp_qmpphy0 {
> +	vdda-phy-supply = <&vreg_l3e_1p2>;
> +	vdda-pll-supply = <&vreg_l3c_0p8>;

[Severity: High]
Does this node also swap the voltage supplies?

A 1.2V supply is incorrectly assigned to the 0.8V PHY core rail here as
well.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930212421.4810-1-mstoretv@cern.ch?part=3