[PATCH v2] arm64: dts: mediatek: mt8188-geralt: Add MT6319 PMIC
Chen-Yu Tsai
wenst at chromium.org
Fri Jul 10 01:29:06 PDT 2026
On Thu, Jul 9, 2026 at 6:52 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno at collabora.com> wrote:
>
> On 7/9/26 11:27, Chen-Yu Tsai wrote:
> > On Thu, Jul 9, 2026 at 4:30 PM <sashiko-bot at kernel.org> wrote:
> >>
> >> 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 at 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 at 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?
> >
> > This discrepency is actually bigger than Sashiko mentions. The ganging
> > supported by the driver only applies to the PMICs in the Asuarada design:
> >
> > - SPMI ID 6 has 1+2 (GPU); 3 and 4 are unused
> > - SPMI ID 7 has 1+2+4 (big cores) and 3 (little cores)
> >
> > On the Cherry design we have:
> >
> > - SPMI ID 6 has 1+2 (GPU); 3 and 4 are unused
> > - SPMI ID 7 has 1+2+3+4 (big cores)
> >
> > On the Corsola designs with beefier chips:
> >
> > - SPMI ID 6 has 1+2 (big cores); 3 and 4 are unused
> >
> > On the Geralt design (this one):
> >
> > - SPMI ID 7 has 1+2 (big cores); 3 (DRAM); 4 (DRAM)
> >
> > Any ideas on what to do about this mess?
>
> wenst, I think that the only way to fix that properly is to add generic support
> for buck phases ganging in the regulator API... because AFAIK there's nothing
> right now.
>
> From what I remember from last time I looked, MediaTek PMICs wouldn't be the
> only ones, there's something else that also has ganged bucks, and again if I
> remember correctly, I remember seeing one platform data struct for each of the
> ganging combinations in that driver.
Grepping through the kernel tree, I see LP8755 and TPS6594 having multiphase
settings. Not sure which one you saw. Also, some of the X-powers PMICs used
with Allwinner Socs also support multiphase. For these we currently just
read out the multi-phase setting from the PMIC and don't register the
regulators for the secondary phases; however it seems with some recent
project, we actually need to program the ganging from the kernel [1].
> So.. especially because MTK wouldn't be the first, I think the addition of some
> kind of generic properties (and callbacks) for this kind of configuration in
> the API would be both profitable and welcome.
>
> Mark, any considerations?
Quoting Mark's response:
> > Unlike separate regulators that are ganged together, in the AXP PMICs it
> > seems that when buck outputs are ganged, only the controls for the first
> > output have any actual effect. In such cases I don't know if we should
> > just ignore / leave out the secondary outputs from both the description
> > and the runtime state.
>
> This is very common, I would expect any regulators that are ganged
> together into a single regulator like this to simply not appear in the
> DT.
So if specifying polyphase settings from the DT is needed, it would
probably be some custom property, either under the primary phase, or
at the chip level. (Because the secondary phases would be absent from
the DT, one can't just reference them using a generic property.)
ChenYu
[1] https://lore.kernel.org/linux-sunxi/20250919000020.16969-1-andre.przywara@arm.com/
> Cheers,
> Angelo
>
> >
> > Another thing is that I kind of wonder why the mode configuration register
> > setting isn't ganged in hardware like the enable and voltage settings.
> >
> >
> > Thanks
> > ChenYu
> >
> >>> + 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
>
More information about the Linux-mediatek
mailing list