| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGXv+5G-s6+sF-PS0iywC5uxMkrfH1FskWG_HcHAi_tfuHWtZA@mail.gmail.com>
Date: Wed, 25 Jun 2025 13:06:20 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: broonie@...nel.org, lgirdwood@...il.com, robh@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org, matthias.bgg@...il.com,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
kernel@...labora.com
Subject: Re: [PATCH v2 2/6] regulator: Add support for MediaTek MT6316 SPMI
PMIC Regulators
Hi,
On Tue, Jun 24, 2025 at 3:46 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com> wrote:
>
> Add a driver for the regulators found on all types of the MediaTek
> MT6316 SPMI PMIC, fully controlled by SPMI interface and featuring
> four step down DCDC (buck) converters.
>
> In particular, this includes support for:
> - MT6316(BP/VP): 2+2 Phase (Phase 1: buck1+2, Phase 2: buck3+4)
> - MT6316(CP/HP/KP): 3+1 Phase (Phase 1: buck1+2+4, Phase 2: buck3)
> - MT6316(DP/TP): 4+0 Phase (Single phase, buck1+2+3+4)
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
> ---
> drivers/regulator/Kconfig | 9 +
> drivers/regulator/Makefile | 1 +
> drivers/regulator/mt6316-regulator.c | 345 +++++++++++++++++++++++++++
> 3 files changed, 355 insertions(+)
> create mode 100644 drivers/regulator/mt6316-regulator.c
>
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index 6d8988387da4..bca2ebbd89b7 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -864,6 +864,15 @@ config REGULATOR_MT6315
> This driver supports the control of different power rails of device
> through regulator interface.
>
> +config REGULATOR_MT6316
> + tristate "MT6316 SPMI PMIC regulator driver"
> + depends on SPMI || COMPILE_TEST
> + help
> + Say Y here to enable support for 2+2, 3+1 and 4 phase regulators
> + found in the MediaTek MT6316 BP, CP, DP, HP, VP and TP SPMI PMICs.
> + This driver supports the control of different power rails of device
> + through regulator interface.
> +
> config REGULATOR_MT6323
> tristate "MediaTek MT6323 PMIC"
> depends on MFD_MT6397
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index c0bc7a0f4e67..58643e35d2cc 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -103,6 +103,7 @@ obj-$(CONFIG_REGULATOR_MP886X) += mp886x.o
> obj-$(CONFIG_REGULATOR_MPQ7920) += mpq7920.o
> obj-$(CONFIG_REGULATOR_MT6311) += mt6311-regulator.o
> obj-$(CONFIG_REGULATOR_MT6315) += mt6315-regulator.o
> +obj-$(CONFIG_REGULATOR_MT6315) += mt6316-regulator.o
> obj-$(CONFIG_REGULATOR_MT6323) += mt6323-regulator.o
> obj-$(CONFIG_REGULATOR_MT6331) += mt6331-regulator.o
> obj-$(CONFIG_REGULATOR_MT6332) += mt6332-regulator.o
> diff --git a/drivers/regulator/mt6316-regulator.c b/drivers/regulator/mt6316-regulator.c
> new file mode 100644
> index 000000000000..952852bbe923
> --- /dev/null
> +++ b/drivers/regulator/mt6316-regulator.c
> @@ -0,0 +1,345 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (c) 2024 MediaTek Inc.
> +// Copyright (c) 2025 Collabora Ltd
> +// AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
> +
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +#include <linux/spmi.h>
> +
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/regulator/of_regulator.h>
> +
> +#define MT6316_BUCK_MODE_AUTO 0
> +#define MT6316_BUCK_MODE_FORCE_PWM 1
> +#define MT6316_BUCK_MODE_LP 2
> +
> +#define MT6316_CHIP_ID 0x20b
> +#define MT6316_BUCK_TOP_CON0 0x1440
> +#define EN_SET_OFFSET 0x1
> +#define EN_CLR_OFFSET 0x2
> +
> +#define MT6316_BUCK_TOP_CON1 0x1443
> +
> +#define MT6316_BUCK_TOP_ELR0 0x1448
> +#define MT6316_BUCK_TOP_ELR2 0x144a
> +#define MT6316_BUCK_TOP_ELR4 0x144c
> +#define MT6316_BUCK_TOP_ELR6 0x144e
> +#define MT6316_VSEL_MASK GENMASK(8, 0)
> +
> +#define MT6316_VBUCK1_DBG 0x14a8
> +#define MT6316_VBUCK2_DBG 0x1528
> +#define MT6316_VBUCK3_DBG 0x15a8
> +#define MT6316_VBUCK4_DBG 0x1628
> +#define MT6316_BUCK_QI BIT(0)
> +
> +#define MT6316_BUCK_TOP_4PHASE_TOP_ANA_CON0 0x1688
> +#define MT6316_BUCK_TOP_4PHASE_TOP_ELR_0 0x1690
> +
> +enum mt6316_type {
> + MT6316_TYPE_2PHASE,
> + MT6316_TYPE_3PHASE,
> + MT6316_TYPE_4PHASE
> +};
> +
> +/**
> + * struct mt6316_regulator_info - MT6316 regulators information
> + * @desc: Regulator description structure
> + * @debug_reg: Debug register for regulator status
> + * @lp_mode_reg: Low Power mode register (normal/idle)
> + * @lp_mode_mask: Low Power mode regulator mask
> + * @modeset_reg: AUTO/PWM mode register
> + * @modeset_mask: AUTO/PWM regulator mask
> + */
> +struct mt6316_regulator_info {
> + struct regulator_desc desc;
> + u16 debug_reg;
> + u16 lp_mode_reg;
> + u16 lp_mode_mask;
> + u16 modeset_reg;
> + u16 modeset_mask;
> +};
> +
> +#define MT6316_BUCK(match, vreg_id, min, max, step, vs_reg) \
> +{ \
> + .desc = { \
> + .name = match, \
> + .of_match = of_match_ptr(match), \
> + .ops = &mt6316_vreg_setclr_ops, \
> + .type = REGULATOR_VOLTAGE, \
> + .owner = THIS_MODULE, \
> + .n_voltages = (max - min) / step + 1, \
> + .min_uV = min, \
> + .uV_step = step, \
> + .enable_reg = MT6316_BUCK_TOP_CON0, \
> + .enable_mask = BIT(vreg_id - 1), \
> + .vsel_reg = vs_reg, \
> + .vsel_mask = MT6316_VSEL_MASK, \
> + .of_map_mode = mt6316_map_mode, \
> + }, \
> + .lp_mode_reg = MT6316_BUCK_TOP_CON1, \
> + .lp_mode_mask = BIT(vreg_id - 1), \
> + .modeset_reg = MT6316_BUCK_TOP_4PHASE_TOP_ANA_CON0, \
> + .modeset_mask = BIT(vreg_id - 1), \
> + .debug_reg = MT6316_VBUCK##vreg_id##_DBG, \
> +}
[...]
> +
> +/* MT6316BP/VP - 2+2 phase buck */
> +static struct mt6316_regulator_info mt6316bv_regulators[] = {
> + MT6316_BUCK("vbuck12", 1, 0, 1277500, 2500, MT6316_BUCK_TOP_ELR0),
> + MT6316_BUCK("vbuck34", 3, 0, 1277500, 2500, MT6316_BUCK_TOP_ELR4),
^
While this is technically correct as the selector does start from 0,
I don't think the regulator can actually go down to 0V. In the past
for the MT6311 / MT6315, MediaTek always added a standard minimum
voltage in the device tree.
I believe a combination of setting .linear_min_sel and adjusting min_uV
and n_voltages is the correct solution.
> +};
> +
> +/* MT6316CP/HP/KP - 3+1 phase buck */
> +static struct mt6316_regulator_info mt6316chk_regulators[] = {
> + MT6316_BUCK("vbuck124", 1, 0, 1277500, 2500, MT6316_BUCK_TOP_ELR0),
> + MT6316_BUCK("vbuck3", 3, 0, 1277500, 2500, MT6316_BUCK_TOP_ELR4),
> +};
> +
> +/* MT6316DP/TP - 4 phase buck */
> +static struct mt6316_regulator_info mt6316dt_regulators[] = {
> + MT6316_BUCK("vbuck1234", 1, 0, 1277500, 2500, MT6316_BUCK_TOP_ELR0),
> +};
> +
> +static const struct regmap_config mt6316_spmi_regmap_config = {
> + .reg_bits = 16,
> + .val_bits = 8,
> + .max_register = 0x1700,
> + .fast_io = true,
> +};
> +
> +static int mt6316_regulator_probe(struct spmi_device *sdev)
> +{
> + struct regulator_config config = {};
> + struct mt6316_regulator_info *info;
> + struct regulator_dev *rdev;
> + enum mt6316_type type;
> + int num_vregs, ret;
> + unsigned int i;
> + u32 chip_id;
> +
> + config.regmap = devm_regmap_init_spmi_ext(sdev, &mt6316_spmi_regmap_config);
> + if (IS_ERR(config.regmap))
> + return PTR_ERR(config.regmap);
> +
> + /*
> + * The first read is expected to fail: this PMIC needs to be woken up
> + * and that can be done with any activity over the SPMI bus.
> + */
> + regmap_read(config.regmap, MT6316_CHIP_ID, &chip_id);
> +
> + /* The second read, instead, shall not fail! */
> + ret = regmap_read(config.regmap, MT6316_CHIP_ID, &chip_id);
> + if (ret) {
> + dev_err(&sdev->dev, "Cannot read Chip ID!\n");
> + return ret;
> + }
> + dev_dbg(&sdev->dev, "Chip ID: 0x%x\n", chip_id);
> +
> + config.dev = &sdev->dev;
> +
> + type = (uintptr_t)device_get_match_data(&sdev->dev);
> + switch (type) {
> + case MT6316_TYPE_2PHASE:
Instead of being tied to the compatible string / PMIC variant,
I wonder if this is something that can be read back from the
PMIC. You wouldn't need so many variant compatible strings if
that's the case.
FWIW we do that for some of the X-Powers AXP PMICs.
ChenYu
Powered by blists - more mailing lists