Re: [PATCH v2] arm64: dts: mediatek: mt8188-geralt: Add MT6319 PMIC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The MT6319 PMIC fallback compatible triggers incorrect MT6315_PP mode ganging in the regulator driver, causing silent cross-talk between the CPU and EMI VDDQ regulators.
- [Low] Grammatical error in the comment explaining the firmware workaround.
--

--- Patch [1]: [PATCH v2] arm64: dts: mediatek: mt8188-geralt: Add MT6319 PMIC ---
commit 0ff94c6622559a57199ed1f2d1f4063c6c911df1
Author: Chen-Yu Tsai <wenst@chromium.org>

arm64: dts: mediatek: mt8188-geralt: Add MT6319 PMIC

The Geralt design uses a MT6319 PMIC to power the big cores and LPDDR4X
DRAM.

> diff --git a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi
> index 8e423504ec052..af05315092f53 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi
[ ... ]
> @@ -1225,6 +1235,70 @@ &spi2 {
>  	status = "okay";
>  };
>  
> +&spmi {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&spmi_pins>;
> +	#address-cells = <2>;
> +	#size-cells = <0>;
> +	status = "okay";
> +
> +	pmic@6 {
> +		compatible = "mediatek,mt6319-regulator", "mediatek,mt6315-regulator";
> +		reg = <0x6 SPMI_USID>;

[Severity: High]
Does this fallback compatible trigger incorrect mode ganging?

The mt6315-regulator driver maps USID 6 to the MT6315_PP variant, which
hardcodes mode ganging for vbuck1, vbuck2, and vbuck4. On the MT8188
Geralt's MT6319, vbuck4 is wired as an independent regulator for
pp0600_emi_vddq.

If a mode change happens on vbuck1 (which is allowed by the
regulator-allowed-modes property), the driver will apply the ganged
mask and silently overwrite vbuck4's mode bits in the PMIC, potentially
forcing the RAM supply into an unexpected mode.

Could this silent cross-talk between the CPU and EMI VDDQ regulators
lead to memory voltage instability?

> +		pvdd1-supply = <&pp4200_s5>;
> +		pvdd2-supply = <&pp4200_s5>;
> +		pvdd3-supply = <&pp4200_s5>;
> +		pvdd4-supply = <&pp4200_s5>;
> +
> +		regulators {
> +			/*
> +			 * vbuck1 is the cpu-supply for CPU6 and CPU7.
> +			 *
> +			 * However, this device originally shipped with a broken
> +			 * firmware that causes register reads over SPMI fail.
> +			 * This is fixed in firmware version 15842.175.0, which
> +			 * is included in ChromeOS releases R150-16700.22.0 or
> +			 * R151-16721.0.0 and later.
> +			 *
> +			 * Assigning the cpu-supply properties for CPU6 and CPU7
> +			 * without the fix will likely cause CPU DVFS to stop
> +			 * working. Hence the assignment is left out to avoid a
> +			 * regression of the function. If the user is confident
> +			 * that their system has the fix, they can added the

[Severity: Low]
This isn't a bug, but there is a typo in this comment. It should say
"they can add the property themselves".

> +			 * property themselves.
> +			 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709081928.612294-1-wenst@chromium.org?part=1