* [PATCH] arm64: dts: mediatek: mt8188-geralt: Add MT6319 PMIC @ 2026-07-07 10:44 Chen-Yu Tsai 2026-07-07 11:05 ` AngeloGioacchino Del Regno 0 siblings, 1 reply; 7+ messages in thread From: Chen-Yu Tsai @ 2026-07-07 10:44 UTC (permalink / raw) To: Matthias Brugger, AngeloGioacchino Del Regno Cc: Chen-Yu Tsai, linux-mediatek, devicetree, linux-arm-kernel, linux-kernel The Geralt design uses a MT6319 PMIC to power the big cores and LPDDR4X DRAM. Add a device node for it and hook up all the supplies. This change requires a firmware fix for the SPMI bus to read back correctly. The required firmware version is 15842.175.0. This is included in ChromeOS releases R150-16700.22.0 (available in Beta channel as of writing or stable channel in mid-July) or R151-16721.0.0 and later. Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> --- .../boot/dts/mediatek/mt8188-geralt.dtsi | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi index f382f90c48f5..fea52c377d88 100644 --- a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi @@ -4,6 +4,8 @@ */ /dts-v1/; #include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/spmi/spmi.h> + #include "mt8188.dtsi" #include "mt6359.dtsi" @@ -241,6 +243,14 @@ &cpu5 { cpu-supply = <&mt6359_vcore_buck_reg>; }; +&cpu6 { + cpu-supply = <&mt6319_buck1>; +}; + +&cpu7 { + cpu-supply = <&mt6319_buck1>; +}; + /* * Geralt is the reference design and doesn't have target TDP. * Ciri is (currently) the only device following Geralt, and its @@ -1156,6 +1166,14 @@ pins-bus { }; }; + spmi_pins: spmi-pins { + pins-bus { + pinmux = <PINMUX_GPIO175__FUNC_B0_SPMI_M_SCL>, + <PINMUX_GPIO176__FUNC_B0_SPMI_M_SDA>; + bias-disable; + }; + }; + uart0_pins: uart0-pins { pins-bus { pinmux = <PINMUX_GPIO31__FUNC_O_UTXD0>, @@ -1267,6 +1285,54 @@ &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>; + pvdd1-supply = <&pp4200_s5>; + pvdd2-supply = <&pp4200_s5>; + pvdd3-supply = <&pp4200_s5>; + pvdd4-supply = <&pp4200_s5>; + + regulators { + mt6319_buck1: vbuck1 { + regulator-name = "ppvar_dvdd_proc_bc"; + regulator-min-microvolt = <520000>; + regulator-max-microvolt = <1155000>; + regulator-enable-ramp-delay = <256>; + regulator-allowed-modes = <0 1 2>; + regulator-always-on; + }; + + /* vbuck2 is ganged with vbuck1 */ + + mt6319_buck3: vbuck3 { + regulator-name = "pp1125_emi_vdd2"; + regulator-min-microvolt = <1060000>; + regulator-max-microvolt = <1170000>; + regulator-enable-ramp-delay = <256>; + regulator-allowed-modes = <0 1 2>; + regulator-always-on; + }; + + mt6319_buck4: vbuck4 { + regulator-name = "pp0600_emi_vddq"; + regulator-min-microvolt = <570000>; + regulator-max-microvolt = <650000>; + regulator-enable-ramp-delay = <256>; + regulator-allowed-modes = <0 1 2>; + regulator-always-on; + }; + }; + }; +}; + &uart0 { pinctrl-names = "default"; pinctrl-0 = <&uart0_pins>; -- 2.55.0.rc2.803.g1fd1e6609c-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: dts: mediatek: mt8188-geralt: Add MT6319 PMIC 2026-07-07 10:44 [PATCH] arm64: dts: mediatek: mt8188-geralt: Add MT6319 PMIC Chen-Yu Tsai @ 2026-07-07 11:05 ` AngeloGioacchino Del Regno 2026-07-07 11:08 ` Chen-Yu Tsai 0 siblings, 1 reply; 7+ messages in thread From: AngeloGioacchino Del Regno @ 2026-07-07 11:05 UTC (permalink / raw) To: Chen-Yu Tsai, Matthias Brugger Cc: linux-mediatek, devicetree, linux-arm-kernel, linux-kernel On 7/7/26 12:44, Chen-Yu Tsai wrote: > The Geralt design uses a MT6319 PMIC to power the big cores and LPDDR4X > DRAM. > > Add a device node for it and hook up all the supplies. > > This change requires a firmware fix for the SPMI bus to read back > correctly. The required firmware version is 15842.175.0. This is > included in ChromeOS releases R150-16700.22.0 (available in Beta > channel as of writing or stable channel in mid-July) or > R151-16721.0.0 and later. > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> This is a big problem then. I take it as if the firmware fix is not in place, probing the CPU power supplies will fail, with all the consequences. This means that with this, we're breaking all Geralt machines with older firmware, which is not acceptable... ...so this needs a different solution, or strong reasons to make me understand that I'm wrong, if I'm wrong. Cheers, Angelo > --- > .../boot/dts/mediatek/mt8188-geralt.dtsi | 66 +++++++++++++++++++ > 1 file changed, 66 insertions(+) > > diff --git a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi > index f382f90c48f5..fea52c377d88 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi > +++ b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi > @@ -4,6 +4,8 @@ > */ > /dts-v1/; > #include <dt-bindings/gpio/gpio.h> > +#include <dt-bindings/spmi/spmi.h> > + > #include "mt8188.dtsi" > #include "mt6359.dtsi" > > @@ -241,6 +243,14 @@ &cpu5 { > cpu-supply = <&mt6359_vcore_buck_reg>; > }; > > +&cpu6 { > + cpu-supply = <&mt6319_buck1>; > +}; > + > +&cpu7 { > + cpu-supply = <&mt6319_buck1>; > +}; > + > /* > * Geralt is the reference design and doesn't have target TDP. > * Ciri is (currently) the only device following Geralt, and its > @@ -1156,6 +1166,14 @@ pins-bus { > }; > }; > > + spmi_pins: spmi-pins { > + pins-bus { > + pinmux = <PINMUX_GPIO175__FUNC_B0_SPMI_M_SCL>, > + <PINMUX_GPIO176__FUNC_B0_SPMI_M_SDA>; > + bias-disable; > + }; > + }; > + > uart0_pins: uart0-pins { > pins-bus { > pinmux = <PINMUX_GPIO31__FUNC_O_UTXD0>, > @@ -1267,6 +1285,54 @@ &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>; > + pvdd1-supply = <&pp4200_s5>; > + pvdd2-supply = <&pp4200_s5>; > + pvdd3-supply = <&pp4200_s5>; > + pvdd4-supply = <&pp4200_s5>; > + > + regulators { > + mt6319_buck1: vbuck1 { > + regulator-name = "ppvar_dvdd_proc_bc"; > + regulator-min-microvolt = <520000>; > + regulator-max-microvolt = <1155000>; > + regulator-enable-ramp-delay = <256>; > + regulator-allowed-modes = <0 1 2>; > + regulator-always-on; > + }; > + > + /* vbuck2 is ganged with vbuck1 */ > + > + mt6319_buck3: vbuck3 { > + regulator-name = "pp1125_emi_vdd2"; > + regulator-min-microvolt = <1060000>; > + regulator-max-microvolt = <1170000>; > + regulator-enable-ramp-delay = <256>; > + regulator-allowed-modes = <0 1 2>; > + regulator-always-on; > + }; > + > + mt6319_buck4: vbuck4 { > + regulator-name = "pp0600_emi_vddq"; > + regulator-min-microvolt = <570000>; > + regulator-max-microvolt = <650000>; > + regulator-enable-ramp-delay = <256>; > + regulator-allowed-modes = <0 1 2>; > + regulator-always-on; > + }; > + }; > + }; > +}; > + > &uart0 { > pinctrl-names = "default"; > pinctrl-0 = <&uart0_pins>; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: dts: mediatek: mt8188-geralt: Add MT6319 PMIC 2026-07-07 11:05 ` AngeloGioacchino Del Regno @ 2026-07-07 11:08 ` Chen-Yu Tsai 2026-07-07 11:24 ` AngeloGioacchino Del Regno 0 siblings, 1 reply; 7+ messages in thread From: Chen-Yu Tsai @ 2026-07-07 11:08 UTC (permalink / raw) To: AngeloGioacchino Del Regno Cc: Matthias Brugger, linux-mediatek, devicetree, linux-arm-kernel, linux-kernel On Tue, Jul 7, 2026 at 7:05 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > On 7/7/26 12:44, Chen-Yu Tsai wrote: > > The Geralt design uses a MT6319 PMIC to power the big cores and LPDDR4X > > DRAM. > > > > Add a device node for it and hook up all the supplies. > > > > This change requires a firmware fix for the SPMI bus to read back > > correctly. The required firmware version is 15842.175.0. This is > > included in ChromeOS releases R150-16700.22.0 (available in Beta > > channel as of writing or stable channel in mid-July) or > > R151-16721.0.0 and later. > > > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > > This is a big problem then. > > I take it as if the firmware fix is not in place, probing the CPU power supplies > will fail, with all the consequences. That's right. > This means that with this, we're breaking all Geralt machines with older firmware, > which is not acceptable... > > ...so this needs a different solution, or strong reasons to make me understand that > I'm wrong, if I'm wrong. We can drop the CPU supplies (they don't matter since cpufreq is hardware driven) and just add the regulators. How does that sound? If the firmware isn't updated, the PMIC will fail to probe, but since nothing is using it, the system will continue to work (with some annoying error messages). ChenYu > Cheers, > Angelo > > > --- > > .../boot/dts/mediatek/mt8188-geralt.dtsi | 66 +++++++++++++++++++ > > 1 file changed, 66 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi > > index f382f90c48f5..fea52c377d88 100644 > > --- a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi > > +++ b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi > > @@ -4,6 +4,8 @@ > > */ > > /dts-v1/; > > #include <dt-bindings/gpio/gpio.h> > > +#include <dt-bindings/spmi/spmi.h> > > + > > #include "mt8188.dtsi" > > #include "mt6359.dtsi" > > > > @@ -241,6 +243,14 @@ &cpu5 { > > cpu-supply = <&mt6359_vcore_buck_reg>; > > }; > > > > +&cpu6 { > > + cpu-supply = <&mt6319_buck1>; > > +}; > > + > > +&cpu7 { > > + cpu-supply = <&mt6319_buck1>; > > +}; > > + > > /* > > * Geralt is the reference design and doesn't have target TDP. > > * Ciri is (currently) the only device following Geralt, and its > > @@ -1156,6 +1166,14 @@ pins-bus { > > }; > > }; > > > > + spmi_pins: spmi-pins { > > + pins-bus { > > + pinmux = <PINMUX_GPIO175__FUNC_B0_SPMI_M_SCL>, > > + <PINMUX_GPIO176__FUNC_B0_SPMI_M_SDA>; > > + bias-disable; > > + }; > > + }; > > + > > uart0_pins: uart0-pins { > > pins-bus { > > pinmux = <PINMUX_GPIO31__FUNC_O_UTXD0>, > > @@ -1267,6 +1285,54 @@ &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>; > > + pvdd1-supply = <&pp4200_s5>; > > + pvdd2-supply = <&pp4200_s5>; > > + pvdd3-supply = <&pp4200_s5>; > > + pvdd4-supply = <&pp4200_s5>; > > + > > + regulators { > > + mt6319_buck1: vbuck1 { > > + regulator-name = "ppvar_dvdd_proc_bc"; > > + regulator-min-microvolt = <520000>; > > + regulator-max-microvolt = <1155000>; > > + regulator-enable-ramp-delay = <256>; > > + regulator-allowed-modes = <0 1 2>; > > + regulator-always-on; > > + }; > > + > > + /* vbuck2 is ganged with vbuck1 */ > > + > > + mt6319_buck3: vbuck3 { > > + regulator-name = "pp1125_emi_vdd2"; > > + regulator-min-microvolt = <1060000>; > > + regulator-max-microvolt = <1170000>; > > + regulator-enable-ramp-delay = <256>; > > + regulator-allowed-modes = <0 1 2>; > > + regulator-always-on; > > + }; > > + > > + mt6319_buck4: vbuck4 { > > + regulator-name = "pp0600_emi_vddq"; > > + regulator-min-microvolt = <570000>; > > + regulator-max-microvolt = <650000>; > > + regulator-enable-ramp-delay = <256>; > > + regulator-allowed-modes = <0 1 2>; > > + regulator-always-on; > > + }; > > + }; > > + }; > > +}; > > + > > &uart0 { > > pinctrl-names = "default"; > > pinctrl-0 = <&uart0_pins>; > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: dts: mediatek: mt8188-geralt: Add MT6319 PMIC 2026-07-07 11:08 ` Chen-Yu Tsai @ 2026-07-07 11:24 ` AngeloGioacchino Del Regno 2026-07-08 4:25 ` Chen-Yu Tsai 0 siblings, 1 reply; 7+ messages in thread From: AngeloGioacchino Del Regno @ 2026-07-07 11:24 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Matthias Brugger, linux-mediatek, devicetree, linux-arm-kernel, linux-kernel On 7/7/26 13:08, Chen-Yu Tsai wrote: > On Tue, Jul 7, 2026 at 7:05 PM AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: >> >> On 7/7/26 12:44, Chen-Yu Tsai wrote: >>> The Geralt design uses a MT6319 PMIC to power the big cores and LPDDR4X >>> DRAM. >>> >>> Add a device node for it and hook up all the supplies. >>> >>> This change requires a firmware fix for the SPMI bus to read back >>> correctly. The required firmware version is 15842.175.0. This is >>> included in ChromeOS releases R150-16700.22.0 (available in Beta >>> channel as of writing or stable channel in mid-July) or >>> R151-16721.0.0 and later. >>> >>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> >> >> This is a big problem then. >> >> I take it as if the firmware fix is not in place, probing the CPU power supplies >> will fail, with all the consequences. > > That's right. > >> This means that with this, we're breaking all Geralt machines with older firmware, >> which is not acceptable... >> >> ...so this needs a different solution, or strong reasons to make me understand that >> I'm wrong, if I'm wrong. > > We can drop the CPU supplies (they don't matter since cpufreq is hardware > driven) and just add the regulators. How does that sound? If the firmware > isn't updated, the PMIC will fail to probe, but since nothing is using it, > the system will continue to work (with some annoying error messages). > That'd be wrong, but less wrong than not having anything described... ...I wonder if, at this point, you could set the SPMI node to status = "fail" and have the *new* firmware override that to "ok". That's the only reasonable way to go forward, IMO. Of course, avoid having the firmware adding the CPU supplies, because that would be rather sketchy then. Just "if spmi status fail found, change to ok". Cheers, Angelo > > ChenYu > >> Cheers, >> Angelo >> >>> --- >>> .../boot/dts/mediatek/mt8188-geralt.dtsi | 66 +++++++++++++++++++ >>> 1 file changed, 66 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi >>> index f382f90c48f5..fea52c377d88 100644 >>> --- a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi >>> +++ b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi >>> @@ -4,6 +4,8 @@ >>> */ >>> /dts-v1/; >>> #include <dt-bindings/gpio/gpio.h> >>> +#include <dt-bindings/spmi/spmi.h> >>> + >>> #include "mt8188.dtsi" >>> #include "mt6359.dtsi" >>> >>> @@ -241,6 +243,14 @@ &cpu5 { >>> cpu-supply = <&mt6359_vcore_buck_reg>; >>> }; >>> >>> +&cpu6 { >>> + cpu-supply = <&mt6319_buck1>; >>> +}; >>> + >>> +&cpu7 { >>> + cpu-supply = <&mt6319_buck1>; >>> +}; >>> + >>> /* >>> * Geralt is the reference design and doesn't have target TDP. >>> * Ciri is (currently) the only device following Geralt, and its >>> @@ -1156,6 +1166,14 @@ pins-bus { >>> }; >>> }; >>> >>> + spmi_pins: spmi-pins { >>> + pins-bus { >>> + pinmux = <PINMUX_GPIO175__FUNC_B0_SPMI_M_SCL>, >>> + <PINMUX_GPIO176__FUNC_B0_SPMI_M_SDA>; >>> + bias-disable; >>> + }; >>> + }; >>> + >>> uart0_pins: uart0-pins { >>> pins-bus { >>> pinmux = <PINMUX_GPIO31__FUNC_O_UTXD0>, >>> @@ -1267,6 +1285,54 @@ &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>; >>> + pvdd1-supply = <&pp4200_s5>; >>> + pvdd2-supply = <&pp4200_s5>; >>> + pvdd3-supply = <&pp4200_s5>; >>> + pvdd4-supply = <&pp4200_s5>; >>> + >>> + regulators { >>> + mt6319_buck1: vbuck1 { >>> + regulator-name = "ppvar_dvdd_proc_bc"; >>> + regulator-min-microvolt = <520000>; >>> + regulator-max-microvolt = <1155000>; >>> + regulator-enable-ramp-delay = <256>; >>> + regulator-allowed-modes = <0 1 2>; >>> + regulator-always-on; >>> + }; >>> + >>> + /* vbuck2 is ganged with vbuck1 */ >>> + >>> + mt6319_buck3: vbuck3 { >>> + regulator-name = "pp1125_emi_vdd2"; >>> + regulator-min-microvolt = <1060000>; >>> + regulator-max-microvolt = <1170000>; >>> + regulator-enable-ramp-delay = <256>; >>> + regulator-allowed-modes = <0 1 2>; >>> + regulator-always-on; >>> + }; >>> + >>> + mt6319_buck4: vbuck4 { >>> + regulator-name = "pp0600_emi_vddq"; >>> + regulator-min-microvolt = <570000>; >>> + regulator-max-microvolt = <650000>; >>> + regulator-enable-ramp-delay = <256>; >>> + regulator-allowed-modes = <0 1 2>; >>> + regulator-always-on; >>> + }; >>> + }; >>> + }; >>> +}; >>> + >>> &uart0 { >>> pinctrl-names = "default"; >>> pinctrl-0 = <&uart0_pins>; >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: dts: mediatek: mt8188-geralt: Add MT6319 PMIC 2026-07-07 11:24 ` AngeloGioacchino Del Regno @ 2026-07-08 4:25 ` Chen-Yu Tsai 2026-07-08 8:02 ` AngeloGioacchino Del Regno 0 siblings, 1 reply; 7+ messages in thread From: Chen-Yu Tsai @ 2026-07-08 4:25 UTC (permalink / raw) To: AngeloGioacchino Del Regno Cc: Matthias Brugger, linux-mediatek, devicetree, linux-arm-kernel, linux-kernel On Tue, Jul 7, 2026 at 7:24 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > On 7/7/26 13:08, Chen-Yu Tsai wrote: > > On Tue, Jul 7, 2026 at 7:05 PM AngeloGioacchino Del Regno > > <angelogioacchino.delregno@collabora.com> wrote: > >> > >> On 7/7/26 12:44, Chen-Yu Tsai wrote: > >>> The Geralt design uses a MT6319 PMIC to power the big cores and LPDDR4X > >>> DRAM. > >>> > >>> Add a device node for it and hook up all the supplies. > >>> > >>> This change requires a firmware fix for the SPMI bus to read back > >>> correctly. The required firmware version is 15842.175.0. This is > >>> included in ChromeOS releases R150-16700.22.0 (available in Beta > >>> channel as of writing or stable channel in mid-July) or > >>> R151-16721.0.0 and later. > >>> > >>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > >> > >> This is a big problem then. > >> > >> I take it as if the firmware fix is not in place, probing the CPU power supplies > >> will fail, with all the consequences. > > > > That's right. > > > >> This means that with this, we're breaking all Geralt machines with older firmware, > >> which is not acceptable... > >> > >> ...so this needs a different solution, or strong reasons to make me understand that > >> I'm wrong, if I'm wrong. > > > > We can drop the CPU supplies (they don't matter since cpufreq is hardware > > driven) and just add the regulators. How does that sound? If the firmware > > isn't updated, the PMIC will fail to probe, but since nothing is using it, > > the system will continue to work (with some annoying error messages). > > > > That'd be wrong, but less wrong than not having anything described... Yeah. As I said, it doesn't affect usability. > ...I wonder if, at this point, you could set the SPMI node to status = "fail" and > have the *new* firmware override that to "ok". > > That's the only reasonable way to go forward, IMO. I'm afraid it is unlikely to get a firmware release to fix a non-critical issue. We were fortunate that there was an actual critical issue being fixed that allowed me to merge the small fix for the SPMI controller. I think it would be great if everyone could update their OS and firmware, but I understand that some devices never get updates, such as those in board farms that never boot into ChromeOS. ChenYu > Of course, avoid having the firmware adding the CPU supplies, because that would > be rather sketchy then. Just "if spmi status fail found, change to ok". > > Cheers, > Angelo > > > > > ChenYu > > > >> Cheers, > >> Angelo > >> > >>> --- > >>> .../boot/dts/mediatek/mt8188-geralt.dtsi | 66 +++++++++++++++++++ > >>> 1 file changed, 66 insertions(+) > >>> > >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi > >>> index f382f90c48f5..fea52c377d88 100644 > >>> --- a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi > >>> +++ b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi > >>> @@ -4,6 +4,8 @@ > >>> */ > >>> /dts-v1/; > >>> #include <dt-bindings/gpio/gpio.h> > >>> +#include <dt-bindings/spmi/spmi.h> > >>> + > >>> #include "mt8188.dtsi" > >>> #include "mt6359.dtsi" > >>> > >>> @@ -241,6 +243,14 @@ &cpu5 { > >>> cpu-supply = <&mt6359_vcore_buck_reg>; > >>> }; > >>> > >>> +&cpu6 { > >>> + cpu-supply = <&mt6319_buck1>; > >>> +}; > >>> + > >>> +&cpu7 { > >>> + cpu-supply = <&mt6319_buck1>; > >>> +}; > >>> + > >>> /* > >>> * Geralt is the reference design and doesn't have target TDP. > >>> * Ciri is (currently) the only device following Geralt, and its > >>> @@ -1156,6 +1166,14 @@ pins-bus { > >>> }; > >>> }; > >>> > >>> + spmi_pins: spmi-pins { > >>> + pins-bus { > >>> + pinmux = <PINMUX_GPIO175__FUNC_B0_SPMI_M_SCL>, > >>> + <PINMUX_GPIO176__FUNC_B0_SPMI_M_SDA>; > >>> + bias-disable; > >>> + }; > >>> + }; > >>> + > >>> uart0_pins: uart0-pins { > >>> pins-bus { > >>> pinmux = <PINMUX_GPIO31__FUNC_O_UTXD0>, > >>> @@ -1267,6 +1285,54 @@ &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>; > >>> + pvdd1-supply = <&pp4200_s5>; > >>> + pvdd2-supply = <&pp4200_s5>; > >>> + pvdd3-supply = <&pp4200_s5>; > >>> + pvdd4-supply = <&pp4200_s5>; > >>> + > >>> + regulators { > >>> + mt6319_buck1: vbuck1 { > >>> + regulator-name = "ppvar_dvdd_proc_bc"; > >>> + regulator-min-microvolt = <520000>; > >>> + regulator-max-microvolt = <1155000>; > >>> + regulator-enable-ramp-delay = <256>; > >>> + regulator-allowed-modes = <0 1 2>; > >>> + regulator-always-on; > >>> + }; > >>> + > >>> + /* vbuck2 is ganged with vbuck1 */ > >>> + > >>> + mt6319_buck3: vbuck3 { > >>> + regulator-name = "pp1125_emi_vdd2"; > >>> + regulator-min-microvolt = <1060000>; > >>> + regulator-max-microvolt = <1170000>; > >>> + regulator-enable-ramp-delay = <256>; > >>> + regulator-allowed-modes = <0 1 2>; > >>> + regulator-always-on; > >>> + }; > >>> + > >>> + mt6319_buck4: vbuck4 { > >>> + regulator-name = "pp0600_emi_vddq"; > >>> + regulator-min-microvolt = <570000>; > >>> + regulator-max-microvolt = <650000>; > >>> + regulator-enable-ramp-delay = <256>; > >>> + regulator-allowed-modes = <0 1 2>; > >>> + regulator-always-on; > >>> + }; > >>> + }; > >>> + }; > >>> +}; > >>> + > >>> &uart0 { > >>> pinctrl-names = "default"; > >>> pinctrl-0 = <&uart0_pins>; > >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: dts: mediatek: mt8188-geralt: Add MT6319 PMIC 2026-07-08 4:25 ` Chen-Yu Tsai @ 2026-07-08 8:02 ` AngeloGioacchino Del Regno 2026-07-08 8:15 ` Chen-Yu Tsai 0 siblings, 1 reply; 7+ messages in thread From: AngeloGioacchino Del Regno @ 2026-07-08 8:02 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Matthias Brugger, linux-mediatek, devicetree, linux-arm-kernel, linux-kernel On 7/8/26 06:25, Chen-Yu Tsai wrote: > On Tue, Jul 7, 2026 at 7:24 PM AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: >> >> On 7/7/26 13:08, Chen-Yu Tsai wrote: >>> On Tue, Jul 7, 2026 at 7:05 PM AngeloGioacchino Del Regno >>> <angelogioacchino.delregno@collabora.com> wrote: >>>> >>>> On 7/7/26 12:44, Chen-Yu Tsai wrote: >>>>> The Geralt design uses a MT6319 PMIC to power the big cores and LPDDR4X >>>>> DRAM. >>>>> >>>>> Add a device node for it and hook up all the supplies. >>>>> >>>>> This change requires a firmware fix for the SPMI bus to read back >>>>> correctly. The required firmware version is 15842.175.0. This is >>>>> included in ChromeOS releases R150-16700.22.0 (available in Beta >>>>> channel as of writing or stable channel in mid-July) or >>>>> R151-16721.0.0 and later. >>>>> >>>>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> >>>> >>>> This is a big problem then. >>>> >>>> I take it as if the firmware fix is not in place, probing the CPU power supplies >>>> will fail, with all the consequences. >>> >>> That's right. >>> >>>> This means that with this, we're breaking all Geralt machines with older firmware, >>>> which is not acceptable... >>>> >>>> ...so this needs a different solution, or strong reasons to make me understand that >>>> I'm wrong, if I'm wrong. >>> >>> We can drop the CPU supplies (they don't matter since cpufreq is hardware >>> driven) and just add the regulators. How does that sound? If the firmware >>> isn't updated, the PMIC will fail to probe, but since nothing is using it, >>> the system will continue to work (with some annoying error messages). >>> >> >> That'd be wrong, but less wrong than not having anything described... > > Yeah. As I said, it doesn't affect usability. > >> ...I wonder if, at this point, you could set the SPMI node to status = "fail" and >> have the *new* firmware override that to "ok". >> >> That's the only reasonable way to go forward, IMO. > > I'm afraid it is unlikely to get a firmware release to fix a non-critical > issue. We were fortunate that there was an actual critical issue being > fixed that allowed me to merge the small fix for the SPMI controller. > Yeah, I know how it is regarding firmware updates... but you know, I *must* try to get OEMs/ODMs to do the right thing when hacky things get in front of me, and I am fully aware that many of them largely underestimate such issues, and even close the possibility to fix those, which is even worst. On the other hand, I know you, and I know that you always try to do the same whenever you can, so please, be aware that I'm not complaining about you. > I think it would be great if everyone could update their OS and firmware, > but I understand that some devices never get updates, such as those in > board farms that never boot into ChromeOS. Unfortunately, this is not just a ChromeOS problem but way larger... Anyway, I'll stop here to prevent myself from going OT too much and being grumpier than I already am :-P I guess then just add the SPMI PMIC and add a big comment that clearly states something like /* * The mt6319_buck1 is the cpu-supply for CPU6 and CPU7. * * However, this device ships with a broken firmware which needs to be updated to * at version XXXXXXX or newer in order to workaround a bug that (describe bug). * * [blahblah text so that's why the cpu supply was not assigned to cpu6/7]. */ ...so that everyone reading the DT is fully aware of what's going on and can act accordingly if they wish. How does that sound? Cheers, Angelo > > > ChenYu > >> Of course, avoid having the firmware adding the CPU supplies, because that would >> be rather sketchy then. Just "if spmi status fail found, change to ok". >> >> Cheers, >> Angelo >> >>> >>> ChenYu >>> >>>> Cheers, >>>> Angelo >>>> >>>>> --- >>>>> .../boot/dts/mediatek/mt8188-geralt.dtsi | 66 +++++++++++++++++++ >>>>> 1 file changed, 66 insertions(+) >>>>> >>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi >>>>> index f382f90c48f5..fea52c377d88 100644 >>>>> --- a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi >>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi >>>>> @@ -4,6 +4,8 @@ >>>>> */ >>>>> /dts-v1/; >>>>> #include <dt-bindings/gpio/gpio.h> >>>>> +#include <dt-bindings/spmi/spmi.h> >>>>> + >>>>> #include "mt8188.dtsi" >>>>> #include "mt6359.dtsi" >>>>> >>>>> @@ -241,6 +243,14 @@ &cpu5 { >>>>> cpu-supply = <&mt6359_vcore_buck_reg>; >>>>> }; >>>>> >>>>> +&cpu6 { >>>>> + cpu-supply = <&mt6319_buck1>; >>>>> +}; >>>>> + >>>>> +&cpu7 { >>>>> + cpu-supply = <&mt6319_buck1>; >>>>> +}; >>>>> + >>>>> /* >>>>> * Geralt is the reference design and doesn't have target TDP. >>>>> * Ciri is (currently) the only device following Geralt, and its >>>>> @@ -1156,6 +1166,14 @@ pins-bus { >>>>> }; >>>>> }; >>>>> >>>>> + spmi_pins: spmi-pins { >>>>> + pins-bus { >>>>> + pinmux = <PINMUX_GPIO175__FUNC_B0_SPMI_M_SCL>, >>>>> + <PINMUX_GPIO176__FUNC_B0_SPMI_M_SDA>; >>>>> + bias-disable; >>>>> + }; >>>>> + }; >>>>> + >>>>> uart0_pins: uart0-pins { >>>>> pins-bus { >>>>> pinmux = <PINMUX_GPIO31__FUNC_O_UTXD0>, >>>>> @@ -1267,6 +1285,54 @@ &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>; >>>>> + pvdd1-supply = <&pp4200_s5>; >>>>> + pvdd2-supply = <&pp4200_s5>; >>>>> + pvdd3-supply = <&pp4200_s5>; >>>>> + pvdd4-supply = <&pp4200_s5>; >>>>> + >>>>> + regulators { >>>>> + mt6319_buck1: vbuck1 { >>>>> + regulator-name = "ppvar_dvdd_proc_bc"; >>>>> + regulator-min-microvolt = <520000>; >>>>> + regulator-max-microvolt = <1155000>; >>>>> + regulator-enable-ramp-delay = <256>; >>>>> + regulator-allowed-modes = <0 1 2>; >>>>> + regulator-always-on; >>>>> + }; >>>>> + >>>>> + /* vbuck2 is ganged with vbuck1 */ >>>>> + >>>>> + mt6319_buck3: vbuck3 { >>>>> + regulator-name = "pp1125_emi_vdd2"; >>>>> + regulator-min-microvolt = <1060000>; >>>>> + regulator-max-microvolt = <1170000>; >>>>> + regulator-enable-ramp-delay = <256>; >>>>> + regulator-allowed-modes = <0 1 2>; >>>>> + regulator-always-on; >>>>> + }; >>>>> + >>>>> + mt6319_buck4: vbuck4 { >>>>> + regulator-name = "pp0600_emi_vddq"; >>>>> + regulator-min-microvolt = <570000>; >>>>> + regulator-max-microvolt = <650000>; >>>>> + regulator-enable-ramp-delay = <256>; >>>>> + regulator-allowed-modes = <0 1 2>; >>>>> + regulator-always-on; >>>>> + }; >>>>> + }; >>>>> + }; >>>>> +}; >>>>> + >>>>> &uart0 { >>>>> pinctrl-names = "default"; >>>>> pinctrl-0 = <&uart0_pins>; >>>> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: dts: mediatek: mt8188-geralt: Add MT6319 PMIC 2026-07-08 8:02 ` AngeloGioacchino Del Regno @ 2026-07-08 8:15 ` Chen-Yu Tsai 0 siblings, 0 replies; 7+ messages in thread From: Chen-Yu Tsai @ 2026-07-08 8:15 UTC (permalink / raw) To: AngeloGioacchino Del Regno Cc: Matthias Brugger, linux-mediatek, devicetree, linux-arm-kernel, linux-kernel On Wed, Jul 8, 2026 at 4:02 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > On 7/8/26 06:25, Chen-Yu Tsai wrote: > > On Tue, Jul 7, 2026 at 7:24 PM AngeloGioacchino Del Regno > > <angelogioacchino.delregno@collabora.com> wrote: > >> > >> On 7/7/26 13:08, Chen-Yu Tsai wrote: > >>> On Tue, Jul 7, 2026 at 7:05 PM AngeloGioacchino Del Regno > >>> <angelogioacchino.delregno@collabora.com> wrote: > >>>> > >>>> On 7/7/26 12:44, Chen-Yu Tsai wrote: > >>>>> The Geralt design uses a MT6319 PMIC to power the big cores and LPDDR4X > >>>>> DRAM. > >>>>> > >>>>> Add a device node for it and hook up all the supplies. > >>>>> > >>>>> This change requires a firmware fix for the SPMI bus to read back > >>>>> correctly. The required firmware version is 15842.175.0. This is > >>>>> included in ChromeOS releases R150-16700.22.0 (available in Beta > >>>>> channel as of writing or stable channel in mid-July) or > >>>>> R151-16721.0.0 and later. > >>>>> > >>>>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > >>>> > >>>> This is a big problem then. > >>>> > >>>> I take it as if the firmware fix is not in place, probing the CPU power supplies > >>>> will fail, with all the consequences. > >>> > >>> That's right. > >>> > >>>> This means that with this, we're breaking all Geralt machines with older firmware, > >>>> which is not acceptable... > >>>> > >>>> ...so this needs a different solution, or strong reasons to make me understand that > >>>> I'm wrong, if I'm wrong. > >>> > >>> We can drop the CPU supplies (they don't matter since cpufreq is hardware > >>> driven) and just add the regulators. How does that sound? If the firmware > >>> isn't updated, the PMIC will fail to probe, but since nothing is using it, > >>> the system will continue to work (with some annoying error messages). > >>> > >> > >> That'd be wrong, but less wrong than not having anything described... > > > > Yeah. As I said, it doesn't affect usability. > > > >> ...I wonder if, at this point, you could set the SPMI node to status = "fail" and > >> have the *new* firmware override that to "ok". > >> > >> That's the only reasonable way to go forward, IMO. > > > > I'm afraid it is unlikely to get a firmware release to fix a non-critical > > issue. We were fortunate that there was an actual critical issue being > > fixed that allowed me to merge the small fix for the SPMI controller. > > > > Yeah, I know how it is regarding firmware updates... but you know, I *must* try to > get OEMs/ODMs to do the right thing when hacky things get in front of me, and I am > fully aware that many of them largely underestimate such issues, and even close the > possibility to fix those, which is even worst. Well, it is at least possible to land a fix into our tree, but it's not going to go through the qualification process for a firmware release because of resource (cost) reasons. I think (?) the firmware builds are public, but that doesn't really help the average user. > On the other hand, I know you, and I know that you always try to do the same > whenever you can, so please, be aware that I'm not complaining about you. I understand what you are asking and why, and I would ask the same if possible. > > I think it would be great if everyone could update their OS and firmware, > > but I understand that some devices never get updates, such as those in > > board farms that never boot into ChromeOS. > > Unfortunately, this is not just a ChromeOS problem but way larger... > > Anyway, I'll stop here to prevent myself from going OT too much and being grumpier > than I already am :-P > > I guess then just add the SPMI PMIC and add a big comment that clearly states > something like > > /* > * The mt6319_buck1 is the cpu-supply for CPU6 and CPU7. > * > * However, this device ships with a broken firmware which needs to be updated to > * at version XXXXXXX or newer in order to workaround a bug that (describe bug). > * > * [blahblah text so that's why the cpu supply was not assigned to cpu6/7]. > */ > > ...so that everyone reading the DT is fully aware of what's going on and can act > accordingly if they wish. > > How does that sound? Works for me. I'll send a revised patch. Thanks ChenYu > Cheers, > Angelo > > > > > > > ChenYu > > > >> Of course, avoid having the firmware adding the CPU supplies, because that would > >> be rather sketchy then. Just "if spmi status fail found, change to ok". > >> > >> Cheers, > >> Angelo > >> > >>> > >>> ChenYu > >>> > >>>> Cheers, > >>>> Angelo > >>>> > >>>>> --- > >>>>> .../boot/dts/mediatek/mt8188-geralt.dtsi | 66 +++++++++++++++++++ > >>>>> 1 file changed, 66 insertions(+) > >>>>> > >>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi > >>>>> index f382f90c48f5..fea52c377d88 100644 > >>>>> --- a/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi > >>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8188-geralt.dtsi > >>>>> @@ -4,6 +4,8 @@ > >>>>> */ > >>>>> /dts-v1/; > >>>>> #include <dt-bindings/gpio/gpio.h> > >>>>> +#include <dt-bindings/spmi/spmi.h> > >>>>> + > >>>>> #include "mt8188.dtsi" > >>>>> #include "mt6359.dtsi" > >>>>> > >>>>> @@ -241,6 +243,14 @@ &cpu5 { > >>>>> cpu-supply = <&mt6359_vcore_buck_reg>; > >>>>> }; > >>>>> > >>>>> +&cpu6 { > >>>>> + cpu-supply = <&mt6319_buck1>; > >>>>> +}; > >>>>> + > >>>>> +&cpu7 { > >>>>> + cpu-supply = <&mt6319_buck1>; > >>>>> +}; > >>>>> + > >>>>> /* > >>>>> * Geralt is the reference design and doesn't have target TDP. > >>>>> * Ciri is (currently) the only device following Geralt, and its > >>>>> @@ -1156,6 +1166,14 @@ pins-bus { > >>>>> }; > >>>>> }; > >>>>> > >>>>> + spmi_pins: spmi-pins { > >>>>> + pins-bus { > >>>>> + pinmux = <PINMUX_GPIO175__FUNC_B0_SPMI_M_SCL>, > >>>>> + <PINMUX_GPIO176__FUNC_B0_SPMI_M_SDA>; > >>>>> + bias-disable; > >>>>> + }; > >>>>> + }; > >>>>> + > >>>>> uart0_pins: uart0-pins { > >>>>> pins-bus { > >>>>> pinmux = <PINMUX_GPIO31__FUNC_O_UTXD0>, > >>>>> @@ -1267,6 +1285,54 @@ &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>; > >>>>> + pvdd1-supply = <&pp4200_s5>; > >>>>> + pvdd2-supply = <&pp4200_s5>; > >>>>> + pvdd3-supply = <&pp4200_s5>; > >>>>> + pvdd4-supply = <&pp4200_s5>; > >>>>> + > >>>>> + regulators { > >>>>> + mt6319_buck1: vbuck1 { > >>>>> + regulator-name = "ppvar_dvdd_proc_bc"; > >>>>> + regulator-min-microvolt = <520000>; > >>>>> + regulator-max-microvolt = <1155000>; > >>>>> + regulator-enable-ramp-delay = <256>; > >>>>> + regulator-allowed-modes = <0 1 2>; > >>>>> + regulator-always-on; > >>>>> + }; > >>>>> + > >>>>> + /* vbuck2 is ganged with vbuck1 */ > >>>>> + > >>>>> + mt6319_buck3: vbuck3 { > >>>>> + regulator-name = "pp1125_emi_vdd2"; > >>>>> + regulator-min-microvolt = <1060000>; > >>>>> + regulator-max-microvolt = <1170000>; > >>>>> + regulator-enable-ramp-delay = <256>; > >>>>> + regulator-allowed-modes = <0 1 2>; > >>>>> + regulator-always-on; > >>>>> + }; > >>>>> + > >>>>> + mt6319_buck4: vbuck4 { > >>>>> + regulator-name = "pp0600_emi_vddq"; > >>>>> + regulator-min-microvolt = <570000>; > >>>>> + regulator-max-microvolt = <650000>; > >>>>> + regulator-enable-ramp-delay = <256>; > >>>>> + regulator-allowed-modes = <0 1 2>; > >>>>> + regulator-always-on; > >>>>> + }; > >>>>> + }; > >>>>> + }; > >>>>> +}; > >>>>> + > >>>>> &uart0 { > >>>>> pinctrl-names = "default"; > >>>>> pinctrl-0 = <&uart0_pins>; > >>>> > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-07-08 8:15 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-07 10:44 [PATCH] arm64: dts: mediatek: mt8188-geralt: Add MT6319 PMIC Chen-Yu Tsai 2026-07-07 11:05 ` AngeloGioacchino Del Regno 2026-07-07 11:08 ` Chen-Yu Tsai 2026-07-07 11:24 ` AngeloGioacchino Del Regno 2026-07-08 4:25 ` Chen-Yu Tsai 2026-07-08 8:02 ` AngeloGioacchino Del Regno 2026-07-08 8:15 ` Chen-Yu Tsai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox