[PATCH 0/2] regulator: mt6363: Fix the regmap value width and the VSRAM LDO registers

[PATCH 0/2] regulator: mt6363: Fix the regmap value width and the VSRAM LDO registers

[GIBEOM LEE (이기범)]

These patches fix two problems in the MT6363 regulator driver that can be
seen from the code. They are on top of regulator for-next, where the driver
is the same as in master and v7.2.12.

Patch 1: the registers have 8 bits but the regmap has 16-bit values, so
every write also writes the next register. For example, disabling one of
VS2 and VBUCK1-7 also disables VS1, VS3 and VSRAM_{DIGRF,MDFE,MODEM}. The
MT6373 driver on the list has 16-bit values too, so its next version will
want the same change.

Patch 2: VSRAM_{CPUB,CPUM,CPUL,APU} use the enable and low power registers
of the BUCK1 group instead of their own, so disabling one of them disables
VS1.

Testing: none of this has run on hardware, because the bootloader of my
board (Lenovo TB375FC, MT6897) is still locked. Each patch builds with W=1.
The driver file also ran unchanged in user space, against a model of the
registers made from MediaTek's header and driver, before and after the
patches:

 - 16 of the 33 checks of the model fail before and pass after. The 9 that
   still fail are about the VEMC voltage callbacks and the overcurrent
   handling, for which I have fixes that I would like to test first.
 - ASan reports a stack buffer overflow in the BUCK_TOP key write before
   patch 1, and nothing after it.
 - The resolved table of the 35 rails differs from MediaTek's driver in 8
   entries before patch 2, and in none after it.

The model comes from MediaTek's sources, so it does not show how the chip
really behaves. A Tested-by from anyone with an MT8196 or MT6991 board would
be very welcome.

How this was made: Claude (claude-opus-5-5, an AI coding assistant) found
both problems while comparing the driver with MediaTek's driver in the
OnePlus MT6897 kernel, during a longer session of porting mainline Linux to
this tablet. It wrote the patches, their changelogs and this letter, and it
ran the checks above. I reviewed the result before signing off.

GIBEOM LEE (이기범) (2):
  regulator: mt6363: Use 8-bit regmap values
  regulator: mt6363: Fix the registers of the VSRAM CPU and APU LDOs

 drivers/regulator/mt6363-regulator.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

[PATCH 1/2] regulator: mt6363: Use 8-bit regmap values

[GIBEOM LEE (이기범)]

The registers of the MT6363 are 8 bits wide, but the regmap has 16-bit
values. On SPMI that makes every access two bytes long: a read also returns
the next register in the upper byte, and a write also writes the next
register, with 0 or with what a read-modify-write found there.

The next register is rarely related to the one the driver means, so this
goes wrong in several places:

 - mt6363_vreg_disable_setclr() writes the mask to the CLR register and 0
   to the one after it. For VS2 and VBUCK1-7 that is the enable register of
   VS1, VS3 and VSRAM_{DIGRF,MDFE,MODEM}, so disabling any of them disables
   those five as well. For those five it is the low power register of VS2
   and VBUCK1-7.
 - mt6363_regulator_set_load() zeroes the register after OP_CFG and the one
   after the third OP_EN register.
 - A read-modify-write of the low power register of a buck writes back to
   its SET register what it read from it. If SET reads like the register
   itself, that sets again the bit that was just cleared.
 - TOP_TRAP is read together with MISC_TOP_INT_CON0, so the VEMC voltage
   callbacks fail with -EINVAL once a MISC interrupt is enabled.
 - regmap_bulk_write() takes a number of registers. The sizeof() of the u16
   keys of TMA and BUCK_TOP asks for two 16-bit registers, so it reads past
   the key on the stack and writes that to the two registers after the key.

MediaTek's regulator driver uses the regmap of their MFD driver, which has
8-bit values (mt6363-regulator.c and mtk-spmi-pmic.c in the OnePlus MT6897
kernel, OnePlusOSS android_kernel_modules_and_devicetree_oneplus_mt6897,
branch oneplus/mt6897_b_16.0.0_oneplus_pad). With 8-bit values one access
is one register, and the sizeof() of the keys is the right number of
registers.

Fixes: 3c36965df808 ("regulator: Add support for MediaTek MT6363 SPMI PMIC Regulators")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: GIBEOM LEE (이기범) <rtx4060@gmail.com>
---
 drivers/regulator/mt6363-regulator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/mt6363-regulator.c b/drivers/regulator/mt6363-regulator.c
index aa6a8eb..cc2e841 100644
--- a/drivers/regulator/mt6363-regulator.c
+++ b/drivers/regulator/mt6363-regulator.c
@@ -811,7 +811,7 @@ static struct regmap *mt6363_spmi_register_regmap(struct device *dev)
 {
 	struct regmap_config mt6363_regmap_config = {
 		.reg_bits = 16,
-		.val_bits = 16,
+		.val_bits = 8,
 		.max_register = 0x1f90,
 		.fast_io = true,
 	};

[PATCH 2/2] regulator: mt6363: Fix the registers of the VSRAM CPU and APU LDOs

[GIBEOM LEE (이기범)]

VSRAM_CPUB, VSRAM_CPUM, VSRAM_CPUL and VSRAM_APU have a register of their
own for the enable (bit 0) and low power (bit 1) bits, which the header
defines as MT6363_RG_LDO_VSRAM_*_ADDR. The driver does not use it:
MT6363_LDO_L() takes the registers of the BUCK1 group, which is right only
for the VSRAM_{DIGRF,MDFE,MODEM} LDOs of MT6363_LDO_L_SC().

Bit 0 of the BUCK1 enable register is VS1 and bit 1 of the BUCK1 low power
register is VS3. So disabling one of these LDOs disables VS1, their state
is the state of VS1, and asking for their low power mode puts VS3 in it.
The LDOs themselves are never switched.

Use the register of the LDO, as MediaTek's driver does.

Fixes: 3c36965df808 ("regulator: Add support for MediaTek MT6363 SPMI PMIC Regulators")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: GIBEOM LEE (이기범) <rtx4060@gmail.com>
---
 drivers/regulator/mt6363-regulator.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/regulator/mt6363-regulator.c b/drivers/regulator/mt6363-regulator.c
index cc2e841..c5de049 100644
--- a/drivers/regulator/mt6363-regulator.c
+++ b/drivers/regulator/mt6363-regulator.c
@@ -147,7 +147,7 @@ struct mt6363_regulator_info {
 }
 
 #define MT6363_LDO_LINEAR_OPS(match, vreg, in_sup, vops, min, max,	\
-			      step, buck_reg, ocp_intn)			\
+			      step, en_reg, lp_reg, ocp_intn)		\
 [MT6363_ID_##vreg] = {							\
 	.desc = {							\
 		.name = match,						\
@@ -160,13 +160,13 @@ struct mt6363_regulator_info {
 		.n_voltages = (max - min) / step + 1,			\
 		.min_uV = min,						\
 		.uV_step = step,					\
-		.enable_reg = MT6363_RG_##buck_reg##_EN_ADDR,		\
+		.enable_reg = en_reg,					\
 		.enable_mask = BIT(MT6363_RG_LDO_##vreg##_EN_BIT),	\
 		.vsel_reg = MT6363_RG_LDO_##vreg##_VOSEL_ADDR,		\
 		.vsel_mask = MT6363_RG_LDO_##vreg##_VOSEL_MASK,		\
 		.of_map_mode = mt6363_map_mode,				\
 	},								\
-	.lp_mode_reg = MT6363_RG_##buck_reg##_LP_ADDR,			\
+	.lp_mode_reg = lp_reg,						\
 	.lp_mode_mask = BIT(MT6363_RG_LDO_##vreg##_LP_BIT),		\
 	.hw_lp_mode_reg = MT6363_LDO_##vreg##_HW_LP_MODE,		\
 	.hw_lp_mode_mask = 0x4,						\
@@ -176,12 +176,15 @@ struct mt6363_regulator_info {
 #define MT6363_LDO_L_SC(match, vreg, inp, min, max, step, buck_reg,	\
 			ocp_intn)					\
 	MT6363_LDO_LINEAR_OPS(match, vreg, inp, mt6363_vreg_setclr_ops,	\
-			      min, max, step, buck_reg, ocp_intn)
+			      min, max, step,				\
+			      MT6363_RG_##buck_reg##_EN_ADDR,		\
+			      MT6363_RG_##buck_reg##_LP_ADDR, ocp_intn)
 
-#define MT6363_LDO_L(match, vreg, inp, min, max, step, buck_reg,	\
-		     ocp_intn)						\
+#define MT6363_LDO_L(match, vreg, inp, min, max, step, ocp_intn)	\
 	MT6363_LDO_LINEAR_OPS(match, vreg, inp, mt6363_ldo_linear_ops,	\
-			      min, max, step, buck_reg, ocp_intn)
+			      min, max, step,				\
+			      MT6363_RG_LDO_##vreg##_ADDR,		\
+			      MT6363_RG_LDO_##vreg##_ADDR, ocp_intn)
 
 #define MT6363_LDO_LINEAR_CAL_OPS(match, vreg, in_sup, vops, vrnum,	\
 				  ocp_intn)				\
@@ -754,10 +757,10 @@ static struct mt6363_regulator_info mt6363_regulators[] = {
 	MT6363_LDO_VT("vio0p75", VIO075, "vs1-ldo1", 5, 36),
 	MT6363_LDO_VT("vio18", VIO18, "vs1-ldo2", 3, 35),
 	MT6363_LDO_VT("vm18", VM18, "vs1-ldo1", 4, 40),
-	MT6363_LDO_L("vsram-apu", VSRAM_APU, "vs3-ldo1", 400000, 1193750, 6250, BUCK1, 30),
-	MT6363_LDO_L("vsram-cpub", VSRAM_CPUB, "vs2-ldo1", 400000, 1193750, 6250, BUCK1, 27),
-	MT6363_LDO_L("vsram-cpum", VSRAM_CPUM, "vs2-ldo1", 400000, 1193750, 6250, BUCK1, 28),
-	MT6363_LDO_L("vsram-cpul", VSRAM_CPUL, "vs2-ldo2", 400000, 1193750, 6250, BUCK1, 29),
+	MT6363_LDO_L("vsram-apu", VSRAM_APU, "vs3-ldo1", 400000, 1193750, 6250, 30),
+	MT6363_LDO_L("vsram-cpub", VSRAM_CPUB, "vs2-ldo1", 400000, 1193750, 6250, 27),
+	MT6363_LDO_L("vsram-cpum", VSRAM_CPUM, "vs2-ldo1", 400000, 1193750, 6250, 28),
+	MT6363_LDO_L("vsram-cpul", VSRAM_CPUL, "vs2-ldo2", 400000, 1193750, 6250, 29),
 	MT6363_LDO_L_SC("vsram-digrf", VSRAM_DIGRF, "vs3-ldo1", 400000, 1193750, 6250, BUCK1, 23),
 	MT6363_LDO_L_SC("vsram-mdfe", VSRAM_MDFE, "vs3-ldo1", 400000, 1193750, 6250, BUCK1, 24),
 	MT6363_LDO_L_SC("vsram-modem", VSRAM_MODEM, "vs3-ldo2", 400000, 1193750, 6250, BUCK1, 25),

Re: [PATCH 1/2] regulator: mt6363: Use 8-bit regmap values

[amergnat]

On Mon, Nov 02, 2026 at 01:44:03AM +0900, GIBEOM LEE (이기범) wrote:
> The registers of the MT6363 are 8 bits wide, but the regmap has 16-bit
> values.

I had the same fix ready before this series and have now posted it,
together with a VEMC voltage fix:
https://lore.kernel.org/all/20261102-rgp-67-mt6363-regulator-fixes-v1-1-420789a3113e@baylibre.com/

The only difference in behavior is the BUCK_TOP key, which mine stores
as __le16: with 8-bit values, regmap_bulk_write() sends the u16 key byte
by byte in memory order, so a big-endian kernel writes its high byte to
the first key register.

Re: [PATCH 0/2] regulator: mt6363: Fix the regmap value width and the VSRAM LDO registers

[Alexandre Mergnat]

Hi Gibeom,

Thanks for looking into this driver!

On Mon, Nov 02, 2026 at 01:44:02AM +0900, GIBEOM LEE (이기범) wrote:
>  - 16 of the 33 checks of the model fail before and pass after. The 9 that
>    still fail are about the VEMC voltage callbacks and the overcurrent
>    handling, for which I have fixes that I would like to test first.

I happened to have a fix for the VEMC voltage callbacks as well, and I
just posted it on top of my regmap patch:
https://lore.kernel.org/all/20261102-rgp-67-mt6363-regulator-fixes-v1-2-420789a3113e@baylibre.com/

It fixes the range lookup of the getter and selects the trap 0 or trap 1
registers at probe. Could you check it against the VEMC failures of your
model? If it covers them, it may save you some work on your own fix. I
have nothing for the overcurrent handling, so that part is all yours.

Re: [PATCH 2/2] regulator: mt6363: Fix the registers of the VSRAM CPU and APU LDOs

[Alexandre Mergnat]

On Mon, Nov 02, 2026 at 01:44:04AM +0900, GIBEOM LEE (이기범) wrote:
> VSRAM_CPUB, VSRAM_CPUM, VSRAM_CPUL and VSRAM_APU have a register of their
> own for the enable (bit 0) and low power (bit 1) bits, which the header
> defines as MT6363_RG_LDO_VSRAM_*_ADDR.

I tested this patch on a board with an MT6363. Without it, vsram-cpum
reads as enabled while its own enable bit is clear, because the driver
reads the VS1 bit. With it, the four LDOs report their own enable bit,
and vsram-cpum, which the board DT keeps always on, is now enabled at
boot. With always-on dropped for a test, enabling and disabling
vsram-cpum from a test consumer only changes its own enable bit, and VS1
stays on. I did not test the low power mode, nor disabling the other
three LDOs: on my board they supply SoC I/O and analog rails.

Tested-by: Alexandre Mergnat <amergnat@baylibre.com>

Re: [PATCH 0/2] regulator: mt6363: Fix the regmap value width and the VSRAM LDO registers

[GIBEOM LEE (이기범)]

Hi Alexandre,

On Mon, Nov 02, 2026 at 04:43:45PM +0000, Alexandre Mergnat wrote:
> It fixes the range lookup of the getter and selects the trap 0 or trap 1
> registers at probe. Could you check it against the VEMC failures of your
> model? If it covers them, it may save you some work on your own fix. I
> have nothing for the overcurrent handling, so that part is all yours.

It covers them, thanks. Six of the nine failures in my cover letter
were about VEMC. As the model stood, three VEMC checks still failed on
your series, only because they change TOP_TRAP after probe. I changed
the model to probe once per trap value and to take the trap 1 trim from
0x1b1c, and then:

 - With VOSEL 6 and VOCAL 5, the unpatched driver reads 1.25 V, as you
   saw on hardware, and your series reads 2.55 V.
 - With trap 0 and with trap 1, all 176 VEMC selectors read back as
   they were set. Only the fields of that trap and the TMA key change,
   and the key is closed afterwards.
 - Trap 2 fails the probe, as your changelog says.
 - ASan reports nothing. With GCC 14 on arm64, each patch builds with
   W=1 on v7.2.12, where this file is the same as in for-next.

This is a model, so it is not a Tested-by. It takes 0x1b1c from your
patch, so it says nothing about that address. The MediaTek header and
driver I have don't define it. My own VEMC fix guessed bits [7:4] of
0x1b1b for that field, so I will drop it.

I will post the overcurrent fixes once I have tested them on my board.

Thanks also for testing 2/2 on hardware. That shows more than my model
could.

Mark, please take Alexandre's regmap patch instead of my 1/2. It is the
same change, it gets the BUCK_TOP key order right, and Alexandre can
run this driver and has the datasheet. My board is still locked and I
have no datasheet.

My 2/2 is not covered by Alexandre's series and is still needed, and
it now has Alexandre's Tested-by. It does not depend on either regmap
patch. It applies to regulator for-next on its own or on top of that
series, and both orders give the same file. I can resend it alone if
that is easier.

Claude, the AI assistant from my cover letter, ran these checks and
drafted this mail. I read both before sending.