2023-08-03 08:14:33

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH 0/9] regulator: mt6366: Split out of MT6358 and cleanup

Hi everyone,

This series splits out the MT6366 PMIC from the MT6358. The two PMICs
are mostly identical, except for the regulator bits. The MT6366 is
missing the VCAM* (camera related) LDOs, but in their place has a few
other ones. This thus requires a separate DT binding and compatible.

This depends on my previous "regulator: mt6358: Remove bogus regulators
and improvements" series [1]. The series is still in flight, but I think
posting this earlier would help get reviews underway.

Patch 1 splits out the compatible string for the MT6366 PMIC, so that
it no longer falls back to the MT6358 one.

Patch 2 adds new register definitions for the MT6366-specific
regulators.

Patch 3 adds a new entry for the MT6366 PMIC to the MT6397 mfd driver.

These three should go through the MFD tree and put on an immutable
branch for the regulator tree to consume patch 2.

Patch 4 adds new binding for MT6366 regulators. This was previously done
by Zhiyong Tao [2] from MediaTek. I cleaned up the patch based on previous
review comments, simplified the regulator names, and added regulator
supplies. Bogus regulators were also dropped, like what was done for the
MT6358 [1].

Patch 5 simplifies the MT6366 regulator names to match the new names
specified in the binding.

Patch 6 makes the MT6366 VCN18 LDO regulator configurable. This is one
of the differences between the MT6358 and MT6366.

Patch 7 adds regulators that were missing from the originally proposed
binding and driver.

Patch 8 adds regulator supply names to the MT6366 regulators

Patch 9 adds an entry for the MT6366 PMIC to the MedaiTek pwrap driver.

As mentioned, patches 1 through 3 should go through the mfd tree on an
immutable branch. patches 4 through 8 should go through the regulator
tree, on top of the aforementioned immutable branch. Patch 9 should go
through the MediaTek tree.

[1] https://lore.kernel.org/linux-arm-kernel/[email protected]/
[2] https://lore.kernel.org/linux-arm-kernel/[email protected]/

Chen-Yu Tsai (8):
dt-bindings: mfd: mt6397: Split out compatible for MediaTek MT6366
PMIC
mfd: mt6358: Add registers for MT6366 specific regulators
mfd: mt6397: Split MediaTek MT6366 PMIC out of MT6358
regulator: mt6358: fix and drop type prefix in MT6366 regulator node
names
regulator: mt6358: Make MT6366 vcn18 LDO configurable
regulator: mt6358: Add missing regulators for MT6366
regulator: mt6358: Add supply names for MT6366 regulators
soc: mediatek: pwrap: add support for MT6366 PMIC

Zhiyong Tao (1):
regulator: dt-bindings: mediatek: Add MT6366 PMIC

.../devicetree/bindings/mfd/mt6397.txt | 5 +-
.../regulator/mediatek,mt6366-regulator.yaml | 154 ++++++++++++++++++
drivers/mfd/mt6397-core.c | 31 ++++
drivers/regulator/mt6358-regulator.c | 127 ++++++++++-----
drivers/soc/mediatek/mtk-pmic-wrap.c | 2 +
include/linux/mfd/mt6358/registers.h | 18 ++
include/linux/regulator/mt6358-regulator.h | 3 +
7 files changed, 296 insertions(+), 44 deletions(-)
create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml

--
2.41.0.585.gd2178a4bd4-goog



2023-08-03 08:16:27

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH 9/9] soc: mediatek: pwrap: add support for MT6366 PMIC

The MT6366 PMIC is mostly, but not fully, compatible with MT6358. It has
a different set of regulators. Specifically, it lacks the camera related
VCAM* LDOs, but has additional VM18, VMDDR, and VSRAM_CORE LDOs.

Add a separate compatible for the MT6366 PMIC.

Signed-off-by: Chen-Yu Tsai <[email protected]>
---
drivers/soc/mediatek/mtk-pmic-wrap.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 11095b8de71a..e3c0e767033b 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -2257,6 +2257,8 @@ static const struct of_device_id of_slave_match_tbl[] = {
{ .compatible = "mediatek,mt6357", .data = &pmic_mt6357 },
{ .compatible = "mediatek,mt6358", .data = &pmic_mt6358 },
{ .compatible = "mediatek,mt6359", .data = &pmic_mt6359 },
+ /* MT6366 is mostly compatible with MT6358, except for slightly different regulators. */
+ { .compatible = "mediatek,mt6366", .data = &pmic_mt6358 },

/* The MT6380 PMIC only implements a regulator, so we bind it
* directly instead of using a MFD.
--
2.41.0.585.gd2178a4bd4-goog


2023-08-03 08:20:47

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH 8/9] regulator: mt6358: Add supply names for MT6366 regulators

The DT bindings for MT6366 regulator defines the supply names for the
PMIC.

Add support for them by adding .supply_name field settings for each
regulator. The buck regulators each have their own supply whose name
can be derived from the regulator name. The LDOs have shared supplies.

Signed-off-by: Chen-Yu Tsai <[email protected]>
---
drivers/regulator/mt6358-regulator.c | 73 ++++++++++++++--------------
1 file changed, 36 insertions(+), 37 deletions(-)

diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
index c9fd5904d13f..b3de933c58b3 100644
--- a/drivers/regulator/mt6358-regulator.c
+++ b/drivers/regulator/mt6358-regulator.c
@@ -139,6 +139,7 @@ struct mt6358_regulator_info {
[MT6366_ID_##vreg] = { \
.desc = { \
.name = #vreg, \
+ .supply_name = "vsys_" match, \
.of_match = of_match_ptr(match), \
.ops = &mt6358_volt_range_ops, \
.type = REGULATOR_VOLTAGE, \
@@ -161,10 +162,11 @@ struct mt6358_regulator_info {
.modeset_mask = BIT(_modeset_shift), \
}

-#define MT6366_LDO(match, vreg, volt_ranges, enreg, enbit, vosel, vosel_mask) \
+#define MT6366_LDO(match, vreg, volt_ranges, supply, enreg, enbit, vosel, vosel_mask) \
[MT6366_ID_##vreg] = { \
.desc = { \
.name = #vreg, \
+ .supply_name = supply, \
.of_match = of_match_ptr(match), \
.ops = &mt6358_volt_table_ops, \
.type = REGULATOR_VOLTAGE, \
@@ -185,12 +187,13 @@ struct mt6358_regulator_info {
.qi = BIT(15), \
}

-#define MT6366_LDO1(match, vreg, min, max, step, \
+#define MT6366_LDO1(match, vreg, supply, min, max, step, \
_da_vsel_reg, _da_vsel_mask, \
vosel, vosel_mask) \
[MT6366_ID_##vreg] = { \
.desc = { \
.name = #vreg, \
+ .supply_name = supply, \
.of_match = of_match_ptr(match), \
.ops = &mt6358_volt_range_ops, \
.type = REGULATOR_VOLTAGE, \
@@ -210,11 +213,12 @@ struct mt6358_regulator_info {
.qi = BIT(0), \
}

-#define MT6366_REG_FIXED(match, vreg, \
+#define MT6366_REG_FIXED(match, vreg, supply, \
enreg, enbit, volt) \
[MT6366_ID_##vreg] = { \
.desc = { \
.name = #vreg, \
+ .supply_name = supply, \
.of_match = of_match_ptr(match), \
.ops = &mt6358_volt_fixed_ops, \
.type = REGULATOR_VOLTAGE, \
@@ -589,57 +593,52 @@ static const struct mt6358_regulator_info mt6366_regulators[] = {
0x7f, MT6358_BUCK_VMODEM_DBG0, 0x7f, MT6358_VMODEM_ANA_CON0, 8),
MT6366_BUCK("vs1", VS1, 1000000, 2587500, 12500,
0x7f, MT6358_BUCK_VS1_DBG0, 0x7f, MT6358_VS1_ANA_CON0, 8),
- MT6366_REG_FIXED("vrf12", VRF12,
- MT6358_LDO_VRF12_CON0, 0, 1200000),
- MT6366_REG_FIXED("vio18", VIO18,
- MT6358_LDO_VIO18_CON0, 0, 1800000),
- MT6366_REG_FIXED("vfe28", VFE28, MT6358_LDO_VFE28_CON0, 0, 2800000),
- MT6366_REG_FIXED("vcn28", VCN28, MT6358_LDO_VCN28_CON0, 0, 2800000),
- MT6366_REG_FIXED("vxo22", VXO22, MT6358_LDO_VXO22_CON0, 0, 2200000),
- MT6366_REG_FIXED("vaux18", VAUX18,
- MT6358_LDO_VAUX18_CON0, 0, 1800000),
- MT6366_REG_FIXED("vbif28", VBIF28,
- MT6358_LDO_VBIF28_CON0, 0, 2800000),
- MT6366_REG_FIXED("vio28", VIO28, MT6358_LDO_VIO28_CON0, 0, 2800000),
- MT6366_REG_FIXED("va12", VA12, MT6358_LDO_VA12_CON0, 0, 1200000),
- MT6366_REG_FIXED("vrf18", VRF18, MT6358_LDO_VRF18_CON0, 0, 1800000),
- MT6366_REG_FIXED("vaud28", VAUD28,
- MT6358_LDO_VAUD28_CON0, 0, 2800000),
- MT6366_LDO("vdram2", VDRAM2, vdram2,
+ MT6366_REG_FIXED("vrf12", VRF12, "vs2_ldo2", MT6358_LDO_VRF12_CON0, 0, 1200000),
+ MT6366_REG_FIXED("vio18", VIO18, "vs1_ldo1", MT6358_LDO_VIO18_CON0, 0, 1800000),
+ MT6366_REG_FIXED("vfe28", VFE28, "vsys_ldo1", MT6358_LDO_VFE28_CON0, 0, 2800000),
+ MT6366_REG_FIXED("vcn28", VCN28, "vsys_ldo1", MT6358_LDO_VCN28_CON0, 0, 2800000),
+ MT6366_REG_FIXED("vxo22", VXO22, "vsys_ldo1", MT6358_LDO_VXO22_CON0, 0, 2200000),
+ MT6366_REG_FIXED("vaux18", VAUX18, "vsys_ldo1", MT6358_LDO_VAUX18_CON0, 0, 1800000),
+ MT6366_REG_FIXED("vbif28", VBIF28, "vsys_ldo1", MT6358_LDO_VBIF28_CON0, 0, 2800000),
+ MT6366_REG_FIXED("vio28", VIO28, "vsys_ldo2", MT6358_LDO_VIO28_CON0, 0, 2800000),
+ MT6366_REG_FIXED("va12", VA12, "vs2_ldo2", MT6358_LDO_VA12_CON0, 0, 1200000),
+ MT6366_REG_FIXED("vrf18", VRF18, "vs1_ldo1", MT6358_LDO_VRF18_CON0, 0, 1800000),
+ MT6366_REG_FIXED("vaud28", VAUD28, "vsys_ldo1", MT6358_LDO_VAUD28_CON0, 0, 2800000),
+ MT6366_LDO("vdram2", VDRAM2, vdram2, "vs2_ldo1",
MT6358_LDO_VDRAM2_CON0, 0, MT6358_LDO_VDRAM2_ELR0, 0x10),
- MT6366_LDO("vsim1", VSIM1, vsim,
+ MT6366_LDO("vsim1", VSIM1, vsim, "vsys_ldo1",
MT6358_LDO_VSIM1_CON0, 0, MT6358_VSIM1_ANA_CON0, 0xf00),
- MT6366_LDO("vibr", VIBR, vibr,
+ MT6366_LDO("vibr", VIBR, vibr, "vsys_ldo3",
MT6358_LDO_VIBR_CON0, 0, MT6358_VIBR_ANA_CON0, 0xf00),
- MT6366_LDO("vusb", VUSB, vusb,
+ MT6366_LDO("vusb", VUSB, vusb, "vsys_ldo1",
MT6358_LDO_VUSB_CON0_0, 0, MT6358_VUSB_ANA_CON0, 0x700),
- MT6366_LDO("vefuse", VEFUSE, vefuse,
+ MT6366_LDO("vefuse", VEFUSE, vefuse, "vs1_ldo1",
MT6358_LDO_VEFUSE_CON0, 0, MT6358_VEFUSE_ANA_CON0, 0xf00),
- MT6366_LDO("vmch", VMCH, vmch_vemc,
+ MT6366_LDO("vmch", VMCH, vmch_vemc, "vsys_ldo2",
MT6358_LDO_VMCH_CON0, 0, MT6358_VMCH_ANA_CON0, 0x700),
- MT6366_LDO("vemc", VEMC, vmch_vemc,
+ MT6366_LDO("vemc", VEMC, vmch_vemc, "vsys_ldo3",
MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
- MT6366_LDO("vcn33", VCN33, vcn33,
+ MT6366_LDO("vcn33", VCN33, vcn33, "vsys_ldo3",
MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
- MT6366_LDO("vmc", VMC, vmc,
+ MT6366_LDO("vmc", VMC, vmc, "vsys_ldo2",
MT6358_LDO_VMC_CON0, 0, MT6358_VMC_ANA_CON0, 0xf00),
- MT6366_LDO("vsim2", VSIM2, vsim,
+ MT6366_LDO("vsim2", VSIM2, vsim, "vsys_ldo2",
MT6358_LDO_VSIM2_CON0, 0, MT6358_VSIM2_ANA_CON0, 0xf00),
- MT6366_LDO("vcn18", VCN18, mt6366_vcn18_vm18,
+ MT6366_LDO("vcn18", VCN18, mt6366_vcn18_vm18, "vs1_ldo1",
MT6358_LDO_VCN18_CON0, 0, MT6358_VCN18_ANA_CON0, 0xf00),
- MT6366_LDO("vm18", VM18, mt6366_vcn18_vm18,
+ MT6366_LDO("vm18", VM18, mt6366_vcn18_vm18, "vs1_ldo1",
MT6358_LDO_VM18_CON0, 0, MT6358_VM18_ANA_CON0, 0xf00),
- MT6366_LDO("vmddr", VMDDR, mt6366_vmddr,
+ MT6366_LDO("vmddr", VMDDR, mt6366_vmddr, "vs2_ldo1",
MT6358_LDO_VMDDR_CON0, 0, MT6358_VMDDR_ANA_CON0, 0xf00),
- MT6366_LDO1("vsram-proc11", VSRAM_PROC11, 500000, 1293750, 6250,
+ MT6366_LDO1("vsram-proc11", VSRAM_PROC11, "vs2_ldo3", 500000, 1293750, 6250,
MT6358_LDO_VSRAM_PROC11_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON0, 0x7f),
- MT6366_LDO1("vsram-others", VSRAM_OTHERS, 500000, 1293750, 6250,
+ MT6366_LDO1("vsram-others", VSRAM_OTHERS, "vs2_ldo3", 500000, 1293750, 6250,
MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON2, 0x7f),
- MT6366_LDO1("vsram-gpu", VSRAM_GPU, 500000, 1293750, 6250,
+ MT6366_LDO1("vsram-gpu", VSRAM_GPU, "vs2_ldo3", 500000, 1293750, 6250,
MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON3, 0x7f),
- MT6366_LDO1("vsram-proc12", VSRAM_PROC12, 500000, 1293750, 6250,
+ MT6366_LDO1("vsram-proc12", VSRAM_PROC12, "vs2_ldo3", 500000, 1293750, 6250,
MT6358_LDO_VSRAM_PROC12_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON1, 0x7f),
- MT6366_LDO1("vsram-core", VSRAM_CORE, 500000, 1293750, 6250,
+ MT6366_LDO1("vsram-core", VSRAM_CORE, "vs2_ldo3", 500000, 1293750, 6250,
MT6358_LDO_VSRAM_CORE_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON5, 0x7f),
};

--
2.41.0.585.gd2178a4bd4-goog


2023-08-03 08:25:36

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH 5/9] regulator: mt6358: fix and drop type prefix in MT6366 regulator node names

The new MT6366 binding does away with the type prefix ("buck_", "ldo_")
in the regulator node names. This better matches the PMIC pin names.
Remaining underscores in names are also replaced with hyphens.

Drop the type prefixes and replace remaining underscores to match the
MT6366 binding.

Signed-off-by: Chen-Yu Tsai <[email protected]>
---
drivers/regulator/mt6358-regulator.c | 70 ++++++++++++++--------------
1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
index 3cb2e6768e25..5a43a84c7a3e 100644
--- a/drivers/regulator/mt6358-regulator.c
+++ b/drivers/regulator/mt6358-regulator.c
@@ -536,68 +536,68 @@ static const struct mt6358_regulator_info mt6358_regulators[] = {

/* The array is indexed by id(MT6366_ID_XXX) */
static const struct mt6358_regulator_info mt6366_regulators[] = {
- MT6366_BUCK("buck_vdram1", VDRAM1, 500000, 2087500, 12500,
+ MT6366_BUCK("vdram1", VDRAM1, 500000, 2087500, 12500,
0x7f, MT6358_BUCK_VDRAM1_DBG0, 0x7f, MT6358_VDRAM1_ANA_CON0, 8),
- MT6366_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
+ MT6366_BUCK("vcore", VCORE, 500000, 1293750, 6250,
0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f, MT6358_VCORE_VGPU_ANA_CON0, 1),
- MT6366_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
+ MT6366_BUCK("vpa", VPA, 500000, 3650000, 50000,
0x3f, MT6358_BUCK_VPA_DBG0, 0x3f, MT6358_VPA_ANA_CON0, 3),
- MT6366_BUCK("buck_vproc11", VPROC11, 500000, 1293750, 6250,
+ MT6366_BUCK("vproc11", VPROC11, 500000, 1293750, 6250,
0x7f, MT6358_BUCK_VPROC11_DBG0, 0x7f, MT6358_VPROC_ANA_CON0, 1),
- MT6366_BUCK("buck_vproc12", VPROC12, 500000, 1293750, 6250,
+ MT6366_BUCK("vproc12", VPROC12, 500000, 1293750, 6250,
0x7f, MT6358_BUCK_VPROC12_DBG0, 0x7f, MT6358_VPROC_ANA_CON0, 2),
- MT6366_BUCK("buck_vgpu", VGPU, 500000, 1293750, 6250,
+ MT6366_BUCK("vgpu", VGPU, 500000, 1293750, 6250,
0x7f, MT6358_BUCK_VGPU_ELR0, 0x7f, MT6358_VCORE_VGPU_ANA_CON0, 2),
- MT6366_BUCK("buck_vs2", VS2, 500000, 2087500, 12500,
+ MT6366_BUCK("vs2", VS2, 500000, 2087500, 12500,
0x7f, MT6358_BUCK_VS2_DBG0, 0x7f, MT6358_VS2_ANA_CON0, 8),
- MT6366_BUCK("buck_vmodem", VMODEM, 500000, 1293750, 6250,
+ MT6366_BUCK("vmodem", VMODEM, 500000, 1293750, 6250,
0x7f, MT6358_BUCK_VMODEM_DBG0, 0x7f, MT6358_VMODEM_ANA_CON0, 8),
- MT6366_BUCK("buck_vs1", VS1, 1000000, 2587500, 12500,
+ MT6366_BUCK("vs1", VS1, 1000000, 2587500, 12500,
0x7f, MT6358_BUCK_VS1_DBG0, 0x7f, MT6358_VS1_ANA_CON0, 8),
- MT6366_REG_FIXED("ldo_vrf12", VRF12,
+ MT6366_REG_FIXED("vrf12", VRF12,
MT6358_LDO_VRF12_CON0, 0, 1200000),
- MT6366_REG_FIXED("ldo_vio18", VIO18,
+ MT6366_REG_FIXED("vio18", VIO18,
MT6358_LDO_VIO18_CON0, 0, 1800000),
- MT6366_REG_FIXED("ldo_vcn18", VCN18, MT6358_LDO_VCN18_CON0, 0, 1800000),
- MT6366_REG_FIXED("ldo_vfe28", VFE28, MT6358_LDO_VFE28_CON0, 0, 2800000),
- MT6366_REG_FIXED("ldo_vcn28", VCN28, MT6358_LDO_VCN28_CON0, 0, 2800000),
- MT6366_REG_FIXED("ldo_vxo22", VXO22, MT6358_LDO_VXO22_CON0, 0, 2200000),
- MT6366_REG_FIXED("ldo_vaux18", VAUX18,
+ MT6366_REG_FIXED("vcn18", VCN18, MT6358_LDO_VCN18_CON0, 0, 1800000),
+ MT6366_REG_FIXED("vfe28", VFE28, MT6358_LDO_VFE28_CON0, 0, 2800000),
+ MT6366_REG_FIXED("vcn28", VCN28, MT6358_LDO_VCN28_CON0, 0, 2800000),
+ MT6366_REG_FIXED("vxo22", VXO22, MT6358_LDO_VXO22_CON0, 0, 2200000),
+ MT6366_REG_FIXED("vaux18", VAUX18,
MT6358_LDO_VAUX18_CON0, 0, 1800000),
- MT6366_REG_FIXED("ldo_vbif28", VBIF28,
+ MT6366_REG_FIXED("vbif28", VBIF28,
MT6358_LDO_VBIF28_CON0, 0, 2800000),
- MT6366_REG_FIXED("ldo_vio28", VIO28, MT6358_LDO_VIO28_CON0, 0, 2800000),
- MT6366_REG_FIXED("ldo_va12", VA12, MT6358_LDO_VA12_CON0, 0, 1200000),
- MT6366_REG_FIXED("ldo_vrf18", VRF18, MT6358_LDO_VRF18_CON0, 0, 1800000),
- MT6366_REG_FIXED("ldo_vaud28", VAUD28,
+ MT6366_REG_FIXED("vio28", VIO28, MT6358_LDO_VIO28_CON0, 0, 2800000),
+ MT6366_REG_FIXED("va12", VA12, MT6358_LDO_VA12_CON0, 0, 1200000),
+ MT6366_REG_FIXED("vrf18", VRF18, MT6358_LDO_VRF18_CON0, 0, 1800000),
+ MT6366_REG_FIXED("vaud28", VAUD28,
MT6358_LDO_VAUD28_CON0, 0, 2800000),
- MT6366_LDO("ldo_vdram2", VDRAM2, vdram2,
+ MT6366_LDO("vdram2", VDRAM2, vdram2,
MT6358_LDO_VDRAM2_CON0, 0, MT6358_LDO_VDRAM2_ELR0, 0x10),
- MT6366_LDO("ldo_vsim1", VSIM1, vsim,
+ MT6366_LDO("vsim1", VSIM1, vsim,
MT6358_LDO_VSIM1_CON0, 0, MT6358_VSIM1_ANA_CON0, 0xf00),
- MT6366_LDO("ldo_vibr", VIBR, vibr,
+ MT6366_LDO("vibr", VIBR, vibr,
MT6358_LDO_VIBR_CON0, 0, MT6358_VIBR_ANA_CON0, 0xf00),
- MT6366_LDO("ldo_vusb", VUSB, vusb,
+ MT6366_LDO("vusb", VUSB, vusb,
MT6358_LDO_VUSB_CON0_0, 0, MT6358_VUSB_ANA_CON0, 0x700),
- MT6366_LDO("ldo_vefuse", VEFUSE, vefuse,
+ MT6366_LDO("vefuse", VEFUSE, vefuse,
MT6358_LDO_VEFUSE_CON0, 0, MT6358_VEFUSE_ANA_CON0, 0xf00),
- MT6366_LDO("ldo_vmch", VMCH, vmch_vemc,
+ MT6366_LDO("vmch", VMCH, vmch_vemc,
MT6358_LDO_VMCH_CON0, 0, MT6358_VMCH_ANA_CON0, 0x700),
- MT6366_LDO("ldo_vemc", VEMC, vmch_vemc,
+ MT6366_LDO("vemc", VEMC, vmch_vemc,
MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
- MT6366_LDO("ldo_vcn33", VCN33, vcn33,
+ MT6366_LDO("vcn33", VCN33, vcn33,
MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
- MT6366_LDO("ldo_vmc", VMC, vmc,
+ MT6366_LDO("vmc", VMC, vmc,
MT6358_LDO_VMC_CON0, 0, MT6358_VMC_ANA_CON0, 0xf00),
- MT6366_LDO("ldo_vsim2", VSIM2, vsim,
+ MT6366_LDO("vsim2", VSIM2, vsim,
MT6358_LDO_VSIM2_CON0, 0, MT6358_VSIM2_ANA_CON0, 0xf00),
- MT6366_LDO1("ldo_vsram_proc11", VSRAM_PROC11, 500000, 1293750, 6250,
+ MT6366_LDO1("vsram-proc11", VSRAM_PROC11, 500000, 1293750, 6250,
MT6358_LDO_VSRAM_PROC11_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON0, 0x7f),
- MT6366_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
+ MT6366_LDO1("vsram-others", VSRAM_OTHERS, 500000, 1293750, 6250,
MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON2, 0x7f),
- MT6366_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
+ MT6366_LDO1("vsram-gpu", VSRAM_GPU, 500000, 1293750, 6250,
MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON3, 0x7f),
- MT6366_LDO1("ldo_vsram_proc12", VSRAM_PROC12, 500000, 1293750, 6250,
+ MT6366_LDO1("vsram-proc12", VSRAM_PROC12, 500000, 1293750, 6250,
MT6358_LDO_VSRAM_PROC12_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON1, 0x7f),
};

--
2.41.0.585.gd2178a4bd4-goog


2023-08-03 08:31:51

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH 4/9] regulator: dt-bindings: mediatek: Add MT6366 PMIC

From: Zhiyong Tao <[email protected]>

The MediaTek MT6366 PMIC is similar to the MT6358 PMIC. It is designed
to be paired with the MediaTek MT8186 SoC. It has 9 buck regulators and
29 LDO regulators, not counting ones that feed internally and basically
have no controls. The regulators are named after their intended usage
for the SoC and system design, thus not named generically as ldoX or
dcdcX, but as vcn33 or vgpu.

Add a binding document describing all the regulators and their supplies.

Signed-off-by: Zhiyong Tao <[email protected]>
[[email protected]: major rework and added commit message]
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
Changes since Zhiyong's last version (v4) [1]:
- simplified regulator names
- added descriptions to regulators
- removed bogus regulators (*_sshub)
- merged vcn33-wifi and vcn33-bt as vcn33
- added missing regulators (vm18, vmddr, vsram-core)
- cut down examples to a handful of cases and made them complete
- expanded commit message a lot

[1] https://lore.kernel.org/linux-arm-kernel/[email protected]/

.../regulator/mediatek,mt6366-regulator.yaml | 154 ++++++++++++++++++
1 file changed, 154 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml

diff --git a/Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml b/Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml
new file mode 100644
index 000000000000..715c6ffcb3ab
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml
@@ -0,0 +1,154 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/mediatek,mt6366-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MT6366 Regulator from MediaTek Integrated
+
+maintainers:
+ - Zhiyong Tao <[email protected]>
+
+description: |
+ Regulator part of the MT6366 PMIC. This node should be under the PMIC's
+ device node.
+
+properties:
+ compatible:
+ const: mediatek,mt6366-regulator
+
+ vsys_ldo1-supply:
+ description: Supply for LDOs vfe28, vxo22, vcn28, vaux18, vaud28, vsim1, vusb, vbif28
+ vsys_ldo2-supply:
+ description: Supply for LDOs vio28, vmc, vmch, vsim2
+ vsys_ldo3-supply:
+ description: Supply for LDOs vcn33, vemc, vibr
+ vsys_vcore-supply:
+ description: Supply for buck regulator vcore
+ vsys_vdram1-supply:
+ description: Supply for buck regulator vdram1
+ vsys_vgpu-supply:
+ description: Supply for buck regulator vgpu
+ vsys_vmodem-supply:
+ description: Supply for buck regulator vmodem
+ vsys_vpa-supply:
+ description: Supply for buck regulator vpa
+ vsys_vproc11-supply:
+ description: Supply for buck regulator vproc11
+ vsys_vproc12-supply:
+ description: Supply for buck regulator vproc12
+ vsys_vs1-supply:
+ description: Supply for buck regulator vs1
+ vsys_vs2-supply:
+ description: Supply for buck regulator vs2
+ vs1_ldo1-supply:
+ description: Supply for LDOs vrf18, vefuse, vcn18, vio18, vm18
+ vs2_ldo1-supply:
+ description: Supply for LDOs vmddr, vdram2
+ vs2_ldo2-supply:
+ description: Supply for LDOs vrf12, va12
+ vs2_ldo3-supply:
+ description: Supply for LDOs vsram-core, vsram-gpu, vsram-others, vsram-proc11, vsram-proc12
+
+patternProperties:
+ "^v(core|dram1|gpu|modem|pa|proc1[12]|s[12])$":
+ description: Buck regulators
+ type: object
+ $ref: regulator.yaml#
+ unevaluatedProperties: false
+
+ "^v(a|rf)12":
+ description: LDOs with fixed 1.2V output and 0~100/10mV tuning
+ type: object
+ $ref: regulator.yaml#
+ unevaluatedProperties: false
+
+ "^v(aux|io|rf)18":
+ description: LDOs with fixed 1.8V output and 0~100/10mV tuning
+ type: object
+ $ref: regulator.yaml#
+ unevaluatedProperties: false
+
+ "^vxo22":
+ description: LDOs with fixed 2.2V output and 0~100/10mV tuning
+ type: object
+ $ref: regulator.yaml#
+ unevaluatedProperties: false
+
+ "^v(aud|bif|cn|fe|io)28":
+ description: LDOs with fixed 2.8V output and 0~100/10mV tuning
+ type: object
+ $ref: regulator.yaml#
+ unevaluatedProperties: false
+
+ "^vusb":
+ description: LDOs with fixed 3.0V output and 0~100/10mV tuning
+ type: object
+ $ref: regulator.yaml#
+ unevaluatedProperties: false
+
+ "^vsram-(core|gpu|others|proc1[12])$":
+ description: LDOs with variable output
+ type: object
+ $ref: regulator.yaml#
+ unevaluatedProperties: false
+
+ "^v(cn18|cn33|dram2|efuse|emc|ibr|m18|mc|mch|mddr|sim[12])$":
+ description: LDOs with variable output and 0~100/10mV tuning
+ type: object
+ $ref: regulator.yaml#
+ unevaluatedProperties: false
+
+required:
+ - compatible
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/regulator/mediatek,mt6397-regulator.h>
+
+ regulator {
+ compatible = "mediatek,mt6366-regulator";
+
+ vdram1 {
+ regulator-name = "pp1125_emi_vdd2";
+ regulator-min-microvolt = <1125000>;
+ regulator-max-microvolt = <1125000>;
+ regulator-ramp-delay = <12500>;
+ regulator-enable-ramp-delay = <0>;
+ regulator-allowed-modes = <MT6397_BUCK_MODE_AUTO
+ MT6397_BUCK_MODE_FORCE_PWM>;
+ regulator-always-on;
+ };
+
+ vproc11 {
+ regulator-name = "ppvar_dvdd_proc_bc_mt6366";
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-ramp-delay = <6250>;
+ regulator-enable-ramp-delay = <200>;
+ regulator-allowed-modes = <MT6397_BUCK_MODE_AUTO
+ MT6397_BUCK_MODE_FORCE_PWM>;
+ regulator-always-on;
+ };
+
+ vmddr {
+ regulator-name = "pm0750_emi_vmddr";
+ regulator-min-microvolt = <700000>;
+ regulator-max-microvolt = <750000>;
+ regulator-enable-ramp-delay = <325>;
+ regulator-always-on;
+ };
+
+ vsram-proc11 {
+ regulator-name = "pp0900_dvdd_sram_bc";
+ regulator-min-microvolt = <850000>;
+ regulator-max-microvolt = <1120000>;
+ regulator-ramp-delay = <6250>;
+ regulator-enable-ramp-delay = <240>;
+ regulator-always-on;
+ };
+ };
+
+...
--
2.41.0.585.gd2178a4bd4-goog


2023-08-03 08:43:24

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH 1/9] dt-bindings: mfd: mt6397: Split out compatible for MediaTek MT6366 PMIC

The MT6366 PMIC is mostly, but not fully, compatible with MT6358. It has
a different set of regulators. Specifically, it lacks the camera related
VCAM* LDOs, but has additional VM18, VMDDR, and VSRAM_CORE LDOs.

Add a separate compatible for the MT6366 PMIC.

Fixes: 49be16305587 ("dt-bindings: mfd: Add compatible for the MediaTek MT6366 PMIC")
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
Documentation/devicetree/bindings/mfd/mt6397.txt | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/mt6397.txt b/Documentation/devicetree/bindings/mfd/mt6397.txt
index 294693a8906c..d4778b0d5f5c 100644
--- a/Documentation/devicetree/bindings/mfd/mt6397.txt
+++ b/Documentation/devicetree/bindings/mfd/mt6397.txt
@@ -22,8 +22,9 @@ compatible:
"mediatek,mt6323" for PMIC MT6323
"mediatek,mt6331" for PMIC MT6331 and MT6332
"mediatek,mt6357" for PMIC MT6357
- "mediatek,mt6358" for PMIC MT6358 and MT6366
+ "mediatek,mt6358" for PMIC MT6358
"mediatek,mt6359" for PMIC MT6359
+ "mediatek,mt6366" for PMIC MT6366
"mediatek,mt6397" for PMIC MT6397

Optional subnodes:
@@ -41,6 +42,8 @@ Optional subnodes:
see ../regulator/mt6323-regulator.txt
- compatible: "mediatek,mt6358-regulator"
see ../regulator/mt6358-regulator.txt
+ - compatible: "mediatek,mt6366-regulator"
+ see ../regulator/mediatek,mt6366-regulator.yaml
- compatible: "mediatek,mt6397-regulator"
see ../regulator/mt6397-regulator.txt
- codec
--
2.41.0.585.gd2178a4bd4-goog


Subject: Re: [PATCH 9/9] soc: mediatek: pwrap: add support for MT6366 PMIC

Il 03/08/23 09:42, Chen-Yu Tsai ha scritto:
> The MT6366 PMIC is mostly, but not fully, compatible with MT6358. It has
> a different set of regulators. Specifically, it lacks the camera related
> VCAM* LDOs, but has additional VM18, VMDDR, and VSRAM_CORE LDOs.
>
> Add a separate compatible for the MT6366 PMIC.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>

We can express the same partial-compatibility state with bindings, adding a
new compatible here with the same platform data doesn't really add any value...

...also because in DT we'll have something like
compatible = "mediatek,mt6366", "mediatek,mt6358";

so if any variation of platform data in mtk-pmic-wrap will ever be needed, we
would still be able to do that in the future without any headache.

In my opinion, this commit can be dropped.

P.S.: We could add a comment in the mt6366.dtsi pmic devicetree for that, too?

Cheers,
Angelo

> ---
> drivers/soc/mediatek/mtk-pmic-wrap.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 11095b8de71a..e3c0e767033b 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -2257,6 +2257,8 @@ static const struct of_device_id of_slave_match_tbl[] = {
> { .compatible = "mediatek,mt6357", .data = &pmic_mt6357 },
> { .compatible = "mediatek,mt6358", .data = &pmic_mt6358 },
> { .compatible = "mediatek,mt6359", .data = &pmic_mt6359 },
> + /* MT6366 is mostly compatible with MT6358, except for slightly different regulators. */
> + { .compatible = "mediatek,mt6366", .data = &pmic_mt6358 },
>
> /* The MT6380 PMIC only implements a regulator, so we bind it
> * directly instead of using a MFD.



2023-08-03 11:01:52

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH 3/9] mfd: mt6397: Split MediaTek MT6366 PMIC out of MT6358

The MT6366 PMIC is mostly, but not fully, compatible with MT6358. It has
a different set of regulators. Specifically, it lacks the camera related
VCAM* LDOs, but has additional VM18, VMDDR, and VSRAM_CORE LDOs.

Add a separate compatible for the MT6366 PMIC. The regulator cell for
this new entry uses a new compatible string matching MT6366.

Fixes: c47383f84909 ("mfd: Add support for the MediaTek MT6366 PMIC")
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
drivers/mfd/mt6397-core.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)

diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
index f6c1f80f94a4..3f8dfe60a59b 100644
--- a/drivers/mfd/mt6397-core.c
+++ b/drivers/mfd/mt6397-core.c
@@ -206,6 +206,26 @@ static const struct mfd_cell mt6359_devs[] = {
},
};

+static const struct mfd_cell mt6366_devs[] = {
+ {
+ .name = "mt6358-regulator",
+ .of_compatible = "mediatek,mt6366-regulator"
+ }, {
+ .name = "mt6358-rtc",
+ .num_resources = ARRAY_SIZE(mt6358_rtc_resources),
+ .resources = mt6358_rtc_resources,
+ .of_compatible = "mediatek,mt6358-rtc",
+ }, {
+ .name = "mt6358-sound",
+ .of_compatible = "mediatek,mt6358-sound"
+ }, {
+ .name = "mt6358-keys",
+ .num_resources = ARRAY_SIZE(mt6358_keys_resources),
+ .resources = mt6358_keys_resources,
+ .of_compatible = "mediatek,mt6358-keys"
+ },
+};
+
static const struct mfd_cell mt6397_devs[] = {
{
.name = "mt6397-rtc",
@@ -280,6 +300,14 @@ static const struct chip_data mt6359_core = {
.irq_init = mt6358_irq_init,
};

+static const struct chip_data mt6366_core = {
+ .cid_addr = MT6358_SWCID,
+ .cid_shift = 8,
+ .cells = mt6366_devs,
+ .cell_size = ARRAY_SIZE(mt6366_devs),
+ .irq_init = mt6358_irq_init,
+};
+
static const struct chip_data mt6397_core = {
.cid_addr = MT6397_CID,
.cid_shift = 0,
@@ -358,6 +386,9 @@ static const struct of_device_id mt6397_of_match[] = {
}, {
.compatible = "mediatek,mt6359",
.data = &mt6359_core,
+ }, {
+ .compatible = "mediatek,mt6366",
+ .data = &mt6366_core,
}, {
.compatible = "mediatek,mt6397",
.data = &mt6397_core,
--
2.41.0.585.gd2178a4bd4-goog


Subject: Re: [PATCH 3/9] mfd: mt6397: Split MediaTek MT6366 PMIC out of MT6358

Il 03/08/23 09:42, Chen-Yu Tsai ha scritto:
> The MT6366 PMIC is mostly, but not fully, compatible with MT6358. It has
> a different set of regulators. Specifically, it lacks the camera related
> VCAM* LDOs, but has additional VM18, VMDDR, and VSRAM_CORE LDOs.
>
> Add a separate compatible for the MT6366 PMIC. The regulator cell for
> this new entry uses a new compatible string matching MT6366.
>
> Fixes: c47383f84909 ("mfd: Add support for the MediaTek MT6366 PMIC")
> Signed-off-by: Chen-Yu Tsai <[email protected]>

I agree in that the LDOs are a bit different, but that's handled by the
mt6358-regulator driver regardless of the actual devicetree compatible,
as that's selected through a chip_id check.

Finally, looking at the driver implementation itself, the addition of a
specific mt6366 compatible here seems redundant, because the actual HW is
- Handled by drivers, but
- Described by bindings

Any other opinions on this?

Regards,
Angelo

> ---
> drivers/mfd/mt6397-core.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
> index f6c1f80f94a4..3f8dfe60a59b 100644
> --- a/drivers/mfd/mt6397-core.c
> +++ b/drivers/mfd/mt6397-core.c
> @@ -206,6 +206,26 @@ static const struct mfd_cell mt6359_devs[] = {
> },
> };
>
> +static const struct mfd_cell mt6366_devs[] = {
> + {
> + .name = "mt6358-regulator",
> + .of_compatible = "mediatek,mt6366-regulator"
> + }, {
> + .name = "mt6358-rtc",
> + .num_resources = ARRAY_SIZE(mt6358_rtc_resources),
> + .resources = mt6358_rtc_resources,
> + .of_compatible = "mediatek,mt6358-rtc",
> + }, {
> + .name = "mt6358-sound",
> + .of_compatible = "mediatek,mt6358-sound"
> + }, {
> + .name = "mt6358-keys",
> + .num_resources = ARRAY_SIZE(mt6358_keys_resources),
> + .resources = mt6358_keys_resources,
> + .of_compatible = "mediatek,mt6358-keys"
> + },
> +};
> +
> static const struct mfd_cell mt6397_devs[] = {
> {
> .name = "mt6397-rtc",
> @@ -280,6 +300,14 @@ static const struct chip_data mt6359_core = {
> .irq_init = mt6358_irq_init,
> };
>
> +static const struct chip_data mt6366_core = {
> + .cid_addr = MT6358_SWCID,
> + .cid_shift = 8,
> + .cells = mt6366_devs,
> + .cell_size = ARRAY_SIZE(mt6366_devs),
> + .irq_init = mt6358_irq_init,
> +};
> +
> static const struct chip_data mt6397_core = {
> .cid_addr = MT6397_CID,
> .cid_shift = 0,
> @@ -358,6 +386,9 @@ static const struct of_device_id mt6397_of_match[] = {
> }, {
> .compatible = "mediatek,mt6359",
> .data = &mt6359_core,
> + }, {
> + .compatible = "mediatek,mt6366",
> + .data = &mt6366_core,
> }, {
> .compatible = "mediatek,mt6397",
> .data = &mt6397_core,



Subject: Re: [PATCH 8/9] regulator: mt6358: Add supply names for MT6366 regulators

Il 03/08/23 09:42, Chen-Yu Tsai ha scritto:
> The DT bindings for MT6366 regulator defines the supply names for the
> PMIC.
>
> Add support for them by adding .supply_name field settings for each
> regulator. The buck regulators each have their own supply whose name
> can be derived from the regulator name. The LDOs have shared supplies.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>



Subject: Re: [PATCH 5/9] regulator: mt6358: fix and drop type prefix in MT6366 regulator node names

Il 03/08/23 09:42, Chen-Yu Tsai ha scritto:
> The new MT6366 binding does away with the type prefix ("buck_", "ldo_")
> in the regulator node names. This better matches the PMIC pin names.
> Remaining underscores in names are also replaced with hyphens.
>
> Drop the type prefixes and replace remaining underscores to match the
> MT6366 binding.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>

There is *no upstream devicetree* using any of those names, as the first devices
that will use the MT6366 will be MT8186 Corsola, which are not upstreamed yet.

This means that this commit does not break ABI, so, I agree - which implies....

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>

Cheers,
Angelo

> ---
> drivers/regulator/mt6358-regulator.c | 70 ++++++++++++++--------------
> 1 file changed, 35 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
> index 3cb2e6768e25..5a43a84c7a3e 100644
> --- a/drivers/regulator/mt6358-regulator.c
> +++ b/drivers/regulator/mt6358-regulator.c
> @@ -536,68 +536,68 @@ static const struct mt6358_regulator_info mt6358_regulators[] = {
>
> /* The array is indexed by id(MT6366_ID_XXX) */
> static const struct mt6358_regulator_info mt6366_regulators[] = {
> - MT6366_BUCK("buck_vdram1", VDRAM1, 500000, 2087500, 12500,
> + MT6366_BUCK("vdram1", VDRAM1, 500000, 2087500, 12500,
> 0x7f, MT6358_BUCK_VDRAM1_DBG0, 0x7f, MT6358_VDRAM1_ANA_CON0, 8),
> - MT6366_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
> + MT6366_BUCK("vcore", VCORE, 500000, 1293750, 6250,
> 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f, MT6358_VCORE_VGPU_ANA_CON0, 1),
> - MT6366_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
> + MT6366_BUCK("vpa", VPA, 500000, 3650000, 50000,
> 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f, MT6358_VPA_ANA_CON0, 3),
> - MT6366_BUCK("buck_vproc11", VPROC11, 500000, 1293750, 6250,
> + MT6366_BUCK("vproc11", VPROC11, 500000, 1293750, 6250,
> 0x7f, MT6358_BUCK_VPROC11_DBG0, 0x7f, MT6358_VPROC_ANA_CON0, 1),
> - MT6366_BUCK("buck_vproc12", VPROC12, 500000, 1293750, 6250,
> + MT6366_BUCK("vproc12", VPROC12, 500000, 1293750, 6250,
> 0x7f, MT6358_BUCK_VPROC12_DBG0, 0x7f, MT6358_VPROC_ANA_CON0, 2),
> - MT6366_BUCK("buck_vgpu", VGPU, 500000, 1293750, 6250,
> + MT6366_BUCK("vgpu", VGPU, 500000, 1293750, 6250,
> 0x7f, MT6358_BUCK_VGPU_ELR0, 0x7f, MT6358_VCORE_VGPU_ANA_CON0, 2),
> - MT6366_BUCK("buck_vs2", VS2, 500000, 2087500, 12500,
> + MT6366_BUCK("vs2", VS2, 500000, 2087500, 12500,
> 0x7f, MT6358_BUCK_VS2_DBG0, 0x7f, MT6358_VS2_ANA_CON0, 8),
> - MT6366_BUCK("buck_vmodem", VMODEM, 500000, 1293750, 6250,
> + MT6366_BUCK("vmodem", VMODEM, 500000, 1293750, 6250,
> 0x7f, MT6358_BUCK_VMODEM_DBG0, 0x7f, MT6358_VMODEM_ANA_CON0, 8),
> - MT6366_BUCK("buck_vs1", VS1, 1000000, 2587500, 12500,
> + MT6366_BUCK("vs1", VS1, 1000000, 2587500, 12500,
> 0x7f, MT6358_BUCK_VS1_DBG0, 0x7f, MT6358_VS1_ANA_CON0, 8),
> - MT6366_REG_FIXED("ldo_vrf12", VRF12,
> + MT6366_REG_FIXED("vrf12", VRF12,
> MT6358_LDO_VRF12_CON0, 0, 1200000),
> - MT6366_REG_FIXED("ldo_vio18", VIO18,
> + MT6366_REG_FIXED("vio18", VIO18,
> MT6358_LDO_VIO18_CON0, 0, 1800000),
> - MT6366_REG_FIXED("ldo_vcn18", VCN18, MT6358_LDO_VCN18_CON0, 0, 1800000),
> - MT6366_REG_FIXED("ldo_vfe28", VFE28, MT6358_LDO_VFE28_CON0, 0, 2800000),
> - MT6366_REG_FIXED("ldo_vcn28", VCN28, MT6358_LDO_VCN28_CON0, 0, 2800000),
> - MT6366_REG_FIXED("ldo_vxo22", VXO22, MT6358_LDO_VXO22_CON0, 0, 2200000),
> - MT6366_REG_FIXED("ldo_vaux18", VAUX18,
> + MT6366_REG_FIXED("vcn18", VCN18, MT6358_LDO_VCN18_CON0, 0, 1800000),
> + MT6366_REG_FIXED("vfe28", VFE28, MT6358_LDO_VFE28_CON0, 0, 2800000),
> + MT6366_REG_FIXED("vcn28", VCN28, MT6358_LDO_VCN28_CON0, 0, 2800000),
> + MT6366_REG_FIXED("vxo22", VXO22, MT6358_LDO_VXO22_CON0, 0, 2200000),
> + MT6366_REG_FIXED("vaux18", VAUX18,
> MT6358_LDO_VAUX18_CON0, 0, 1800000),
> - MT6366_REG_FIXED("ldo_vbif28", VBIF28,
> + MT6366_REG_FIXED("vbif28", VBIF28,
> MT6358_LDO_VBIF28_CON0, 0, 2800000),
> - MT6366_REG_FIXED("ldo_vio28", VIO28, MT6358_LDO_VIO28_CON0, 0, 2800000),
> - MT6366_REG_FIXED("ldo_va12", VA12, MT6358_LDO_VA12_CON0, 0, 1200000),
> - MT6366_REG_FIXED("ldo_vrf18", VRF18, MT6358_LDO_VRF18_CON0, 0, 1800000),
> - MT6366_REG_FIXED("ldo_vaud28", VAUD28,
> + MT6366_REG_FIXED("vio28", VIO28, MT6358_LDO_VIO28_CON0, 0, 2800000),
> + MT6366_REG_FIXED("va12", VA12, MT6358_LDO_VA12_CON0, 0, 1200000),
> + MT6366_REG_FIXED("vrf18", VRF18, MT6358_LDO_VRF18_CON0, 0, 1800000),
> + MT6366_REG_FIXED("vaud28", VAUD28,
> MT6358_LDO_VAUD28_CON0, 0, 2800000),
> - MT6366_LDO("ldo_vdram2", VDRAM2, vdram2,
> + MT6366_LDO("vdram2", VDRAM2, vdram2,
> MT6358_LDO_VDRAM2_CON0, 0, MT6358_LDO_VDRAM2_ELR0, 0x10),
> - MT6366_LDO("ldo_vsim1", VSIM1, vsim,
> + MT6366_LDO("vsim1", VSIM1, vsim,
> MT6358_LDO_VSIM1_CON0, 0, MT6358_VSIM1_ANA_CON0, 0xf00),
> - MT6366_LDO("ldo_vibr", VIBR, vibr,
> + MT6366_LDO("vibr", VIBR, vibr,
> MT6358_LDO_VIBR_CON0, 0, MT6358_VIBR_ANA_CON0, 0xf00),
> - MT6366_LDO("ldo_vusb", VUSB, vusb,
> + MT6366_LDO("vusb", VUSB, vusb,
> MT6358_LDO_VUSB_CON0_0, 0, MT6358_VUSB_ANA_CON0, 0x700),
> - MT6366_LDO("ldo_vefuse", VEFUSE, vefuse,
> + MT6366_LDO("vefuse", VEFUSE, vefuse,
> MT6358_LDO_VEFUSE_CON0, 0, MT6358_VEFUSE_ANA_CON0, 0xf00),
> - MT6366_LDO("ldo_vmch", VMCH, vmch_vemc,
> + MT6366_LDO("vmch", VMCH, vmch_vemc,
> MT6358_LDO_VMCH_CON0, 0, MT6358_VMCH_ANA_CON0, 0x700),
> - MT6366_LDO("ldo_vemc", VEMC, vmch_vemc,
> + MT6366_LDO("vemc", VEMC, vmch_vemc,
> MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
> - MT6366_LDO("ldo_vcn33", VCN33, vcn33,
> + MT6366_LDO("vcn33", VCN33, vcn33,
> MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
> - MT6366_LDO("ldo_vmc", VMC, vmc,
> + MT6366_LDO("vmc", VMC, vmc,
> MT6358_LDO_VMC_CON0, 0, MT6358_VMC_ANA_CON0, 0xf00),
> - MT6366_LDO("ldo_vsim2", VSIM2, vsim,
> + MT6366_LDO("vsim2", VSIM2, vsim,
> MT6358_LDO_VSIM2_CON0, 0, MT6358_VSIM2_ANA_CON0, 0xf00),
> - MT6366_LDO1("ldo_vsram_proc11", VSRAM_PROC11, 500000, 1293750, 6250,
> + MT6366_LDO1("vsram-proc11", VSRAM_PROC11, 500000, 1293750, 6250,
> MT6358_LDO_VSRAM_PROC11_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON0, 0x7f),
> - MT6366_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
> + MT6366_LDO1("vsram-others", VSRAM_OTHERS, 500000, 1293750, 6250,
> MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON2, 0x7f),
> - MT6366_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
> + MT6366_LDO1("vsram-gpu", VSRAM_GPU, 500000, 1293750, 6250,
> MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON3, 0x7f),
> - MT6366_LDO1("ldo_vsram_proc12", VSRAM_PROC12, 500000, 1293750, 6250,
> + MT6366_LDO1("vsram-proc12", VSRAM_PROC12, 500000, 1293750, 6250,
> MT6358_LDO_VSRAM_PROC12_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON1, 0x7f),
> };
>



2023-08-04 04:07:10

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH 3/9] mfd: mt6397: Split MediaTek MT6366 PMIC out of MT6358

On Thu, Aug 3, 2023 at 5:01 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Il 03/08/23 09:42, Chen-Yu Tsai ha scritto:
> > The MT6366 PMIC is mostly, but not fully, compatible with MT6358. It has
> > a different set of regulators. Specifically, it lacks the camera related
> > VCAM* LDOs, but has additional VM18, VMDDR, and VSRAM_CORE LDOs.
> >
> > Add a separate compatible for the MT6366 PMIC. The regulator cell for
> > this new entry uses a new compatible string matching MT6366.
> >
> > Fixes: c47383f84909 ("mfd: Add support for the MediaTek MT6366 PMIC")
> > Signed-off-by: Chen-Yu Tsai <[email protected]>
>
> I agree in that the LDOs are a bit different, but that's handled by the
> mt6358-regulator driver regardless of the actual devicetree compatible,
> as that's selected through a chip_id check.
>
> Finally, looking at the driver implementation itself, the addition of a
> specific mt6366 compatible here seems redundant, because the actual HW is
> - Handled by drivers, but
> - Described by bindings
>
> Any other opinions on this?

Well, on the bindings side, we can't have MT6366 fall back to MT6358,
neither for the whole PMIC nor just for the regulators. For the latter
it's because neither is a subset of the other, which a) makes them not
fallback compatible as required by the spirit of fallback compatibles,
and b) cannot be described with a fallback compatible, as the fallback
one will have properties/nodes that are not valid for the other, and
vice versa.

Without a fallback compatible to lean in for the regulator driver, we
will need to split out the compatible at the mfd level as well. AFAIU
the mfd core matches mfd-cells based on the compatible strings it is
given in the driver.

ChenYu

> Regards,
> Angelo
>
> > ---
> > drivers/mfd/mt6397-core.c | 31 +++++++++++++++++++++++++++++++
> > 1 file changed, 31 insertions(+)
> >
> > diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
> > index f6c1f80f94a4..3f8dfe60a59b 100644
> > --- a/drivers/mfd/mt6397-core.c
> > +++ b/drivers/mfd/mt6397-core.c
> > @@ -206,6 +206,26 @@ static const struct mfd_cell mt6359_devs[] = {
> > },
> > };
> >
> > +static const struct mfd_cell mt6366_devs[] = {
> > + {
> > + .name = "mt6358-regulator",
> > + .of_compatible = "mediatek,mt6366-regulator"
> > + }, {
> > + .name = "mt6358-rtc",
> > + .num_resources = ARRAY_SIZE(mt6358_rtc_resources),
> > + .resources = mt6358_rtc_resources,
> > + .of_compatible = "mediatek,mt6358-rtc",
> > + }, {
> > + .name = "mt6358-sound",
> > + .of_compatible = "mediatek,mt6358-sound"
> > + }, {
> > + .name = "mt6358-keys",
> > + .num_resources = ARRAY_SIZE(mt6358_keys_resources),
> > + .resources = mt6358_keys_resources,
> > + .of_compatible = "mediatek,mt6358-keys"
> > + },
> > +};
> > +
> > static const struct mfd_cell mt6397_devs[] = {
> > {
> > .name = "mt6397-rtc",
> > @@ -280,6 +300,14 @@ static const struct chip_data mt6359_core = {
> > .irq_init = mt6358_irq_init,
> > };
> >
> > +static const struct chip_data mt6366_core = {
> > + .cid_addr = MT6358_SWCID,
> > + .cid_shift = 8,
> > + .cells = mt6366_devs,
> > + .cell_size = ARRAY_SIZE(mt6366_devs),
> > + .irq_init = mt6358_irq_init,
> > +};
> > +
> > static const struct chip_data mt6397_core = {
> > .cid_addr = MT6397_CID,
> > .cid_shift = 0,
> > @@ -358,6 +386,9 @@ static const struct of_device_id mt6397_of_match[] = {
> > }, {
> > .compatible = "mediatek,mt6359",
> > .data = &mt6359_core,
> > + }, {
> > + .compatible = "mediatek,mt6366",
> > + .data = &mt6366_core,
> > }, {
> > .compatible = "mediatek,mt6397",
> > .data = &mt6397_core,
>
>

2023-08-04 04:08:20

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH 9/9] soc: mediatek: pwrap: add support for MT6366 PMIC

On Thu, Aug 3, 2023 at 5:01 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Il 03/08/23 09:42, Chen-Yu Tsai ha scritto:
> > The MT6366 PMIC is mostly, but not fully, compatible with MT6358. It has
> > a different set of regulators. Specifically, it lacks the camera related
> > VCAM* LDOs, but has additional VM18, VMDDR, and VSRAM_CORE LDOs.
> >
> > Add a separate compatible for the MT6366 PMIC.
> >
> > Signed-off-by: Chen-Yu Tsai <[email protected]>
>
> We can express the same partial-compatibility state with bindings, adding a
> new compatible here with the same platform data doesn't really add any value...
>
> ...also because in DT we'll have something like
> compatible = "mediatek,mt6366", "mediatek,mt6358";
>
> so if any variation of platform data in mtk-pmic-wrap will ever be needed, we
> would still be able to do that in the future without any headache.
>
> In my opinion, this commit can be dropped.

As mentioned in my reply to the mfd patch, this might not be doable. There
are limitations on both the DT bindings and mfd driver sides.

ChenYu

> P.S.: We could add a comment in the mt6366.dtsi pmic devicetree for that, too?
>
> Cheers,
> Angelo
>
> > ---
> > drivers/soc/mediatek/mtk-pmic-wrap.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> > index 11095b8de71a..e3c0e767033b 100644
> > --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> > +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> > @@ -2257,6 +2257,8 @@ static const struct of_device_id of_slave_match_tbl[] = {
> > { .compatible = "mediatek,mt6357", .data = &pmic_mt6357 },
> > { .compatible = "mediatek,mt6358", .data = &pmic_mt6358 },
> > { .compatible = "mediatek,mt6359", .data = &pmic_mt6359 },
> > + /* MT6366 is mostly compatible with MT6358, except for slightly different regulators. */
> > + { .compatible = "mediatek,mt6366", .data = &pmic_mt6358 },
> >
> > /* The MT6380 PMIC only implements a regulator, so we bind it
> > * directly instead of using a MFD.
>
>

Subject: Re: [PATCH 3/9] mfd: mt6397: Split MediaTek MT6366 PMIC out of MT6358

Il 04/08/23 05:47, Chen-Yu Tsai ha scritto:
> On Thu, Aug 3, 2023 at 5:01 PM AngeloGioacchino Del Regno
> <[email protected]> wrote:
>>
>> Il 03/08/23 09:42, Chen-Yu Tsai ha scritto:
>>> The MT6366 PMIC is mostly, but not fully, compatible with MT6358. It has
>>> a different set of regulators. Specifically, it lacks the camera related
>>> VCAM* LDOs, but has additional VM18, VMDDR, and VSRAM_CORE LDOs.
>>>
>>> Add a separate compatible for the MT6366 PMIC. The regulator cell for
>>> this new entry uses a new compatible string matching MT6366.
>>>
>>> Fixes: c47383f84909 ("mfd: Add support for the MediaTek MT6366 PMIC")
>>> Signed-off-by: Chen-Yu Tsai <[email protected]>
>>
>> I agree in that the LDOs are a bit different, but that's handled by the
>> mt6358-regulator driver regardless of the actual devicetree compatible,
>> as that's selected through a chip_id check.
>>
>> Finally, looking at the driver implementation itself, the addition of a
>> specific mt6366 compatible here seems redundant, because the actual HW is
>> - Handled by drivers, but
>> - Described by bindings
>>
>> Any other opinions on this?
>
> Well, on the bindings side, we can't have MT6366 fall back to MT6358,
> neither for the whole PMIC nor just for the regulators. For the latter
> it's because neither is a subset of the other, which a) makes them not
> fallback compatible as required by the spirit of fallback compatibles,
> and b) cannot be described with a fallback compatible, as the fallback
> one will have properties/nodes that are not valid for the other, and
> vice versa.
>
> Without a fallback compatible to lean in for the regulator driver, we
> will need to split out the compatible at the mfd level as well. AFAIU
> the mfd core matches mfd-cells based on the compatible strings it is
> given in the driver.
>

Hmm... you might actually be right on this.
But! I just want to be sure that we're doing things the right way.. and
I'd like to get an opinion from a bindings person, as I think that's the
most appropriate thing that can be done.

Krzysztof, please, can you check this one?

Thanks!
Angelo

> ChenYu
>
>> Regards,
>> Angelo
>>
>>> ---
>>> drivers/mfd/mt6397-core.c | 31 +++++++++++++++++++++++++++++++
>>> 1 file changed, 31 insertions(+)
>>>
>>> diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
>>> index f6c1f80f94a4..3f8dfe60a59b 100644
>>> --- a/drivers/mfd/mt6397-core.c
>>> +++ b/drivers/mfd/mt6397-core.c
>>> @@ -206,6 +206,26 @@ static const struct mfd_cell mt6359_devs[] = {
>>> },
>>> };
>>>
>>> +static const struct mfd_cell mt6366_devs[] = {
>>> + {
>>> + .name = "mt6358-regulator",
>>> + .of_compatible = "mediatek,mt6366-regulator"
>>> + }, {
>>> + .name = "mt6358-rtc",
>>> + .num_resources = ARRAY_SIZE(mt6358_rtc_resources),
>>> + .resources = mt6358_rtc_resources,
>>> + .of_compatible = "mediatek,mt6358-rtc",
>>> + }, {
>>> + .name = "mt6358-sound",
>>> + .of_compatible = "mediatek,mt6358-sound"
>>> + }, {
>>> + .name = "mt6358-keys",
>>> + .num_resources = ARRAY_SIZE(mt6358_keys_resources),
>>> + .resources = mt6358_keys_resources,
>>> + .of_compatible = "mediatek,mt6358-keys"
>>> + },
>>> +};
>>> +
>>> static const struct mfd_cell mt6397_devs[] = {
>>> {
>>> .name = "mt6397-rtc",
>>> @@ -280,6 +300,14 @@ static const struct chip_data mt6359_core = {
>>> .irq_init = mt6358_irq_init,
>>> };
>>>
>>> +static const struct chip_data mt6366_core = {
>>> + .cid_addr = MT6358_SWCID,
>>> + .cid_shift = 8,
>>> + .cells = mt6366_devs,
>>> + .cell_size = ARRAY_SIZE(mt6366_devs),
>>> + .irq_init = mt6358_irq_init,
>>> +};
>>> +
>>> static const struct chip_data mt6397_core = {
>>> .cid_addr = MT6397_CID,
>>> .cid_shift = 0,
>>> @@ -358,6 +386,9 @@ static const struct of_device_id mt6397_of_match[] = {
>>> }, {
>>> .compatible = "mediatek,mt6359",
>>> .data = &mt6359_core,
>>> + }, {
>>> + .compatible = "mediatek,mt6366",
>>> + .data = &mt6366_core,
>>> }, {
>>> .compatible = "mediatek,mt6397",
>>> .data = &mt6397_core,
>>
>>


2023-08-04 11:09:59

by Eugen Hristev

[permalink] [raw]
Subject: Re: [PATCH 5/9] regulator: mt6358: fix and drop type prefix in MT6366 regulator node names

Hi Chen-Yu,

On 8/3/23 12:01, AngeloGioacchino Del Regno wrote:
> Il 03/08/23 09:42, Chen-Yu Tsai ha scritto:
>> The new MT6366 binding does away with the type prefix ("buck_", "ldo_")
>> in the regulator node names. This better matches the PMIC pin names.
>> Remaining underscores in names are also replaced with hyphens.
>>
>> Drop the type prefixes and replace remaining underscores to match the
>> MT6366 binding.
>>
>> Signed-off-by: Chen-Yu Tsai <[email protected]>
>
> There is *no upstream devicetree* using any of those names, as the first
> devices
> that will use the MT6366 will be MT8186 Corsola, which are not
> upstreamed yet.
>
> This means that this commit does not break ABI, so, I agree - which
> implies....
>
> Reviewed-by: AngeloGioacchino Del Regno
> <[email protected]>
>
> Cheers,
> Angelo
>
>> ---
>>   drivers/regulator/mt6358-regulator.c | 70 ++++++++++++++--------------
>>   1 file changed, 35 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/regulator/mt6358-regulator.c
>> b/drivers/regulator/mt6358-regulator.c
>> index 3cb2e6768e25..5a43a84c7a3e 100644
>> --- a/drivers/regulator/mt6358-regulator.c
>> +++ b/drivers/regulator/mt6358-regulator.c
>> @@ -536,68 +536,68 @@ static const struct mt6358_regulator_info
>> mt6358_regulators[] = {
>>   /* The array is indexed by id(MT6366_ID_XXX) */
>>   static const struct mt6358_regulator_info mt6366_regulators[] = {
>> -    MT6366_BUCK("buck_vdram1", VDRAM1, 500000, 2087500, 12500,
>> +    MT6366_BUCK("vdram1", VDRAM1, 500000, 2087500, 12500,
>>               0x7f, MT6358_BUCK_VDRAM1_DBG0, 0x7f,
>> MT6358_VDRAM1_ANA_CON0, 8),
>> -    MT6366_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
>> +    MT6366_BUCK("vcore", VCORE, 500000, 1293750, 6250,
>>               0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
>> MT6358_VCORE_VGPU_ANA_CON0, 1),
>> -    MT6366_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
>> +    MT6366_BUCK("vpa", VPA, 500000, 3650000, 50000,
>>               0x3f, MT6358_BUCK_VPA_DBG0, 0x3f, MT6358_VPA_ANA_CON0, 3),
>> -    MT6366_BUCK("buck_vproc11", VPROC11, 500000, 1293750, 6250,
>> +    MT6366_BUCK("vproc11", VPROC11, 500000, 1293750, 6250,
>>               0x7f, MT6358_BUCK_VPROC11_DBG0, 0x7f,
>> MT6358_VPROC_ANA_CON0, 1),
>> -    MT6366_BUCK("buck_vproc12", VPROC12, 500000, 1293750, 6250,
>> +    MT6366_BUCK("vproc12", VPROC12, 500000, 1293750, 6250,
>>               0x7f, MT6358_BUCK_VPROC12_DBG0, 0x7f,
>> MT6358_VPROC_ANA_CON0, 2),
>> -    MT6366_BUCK("buck_vgpu", VGPU, 500000, 1293750, 6250,
>> +    MT6366_BUCK("vgpu", VGPU, 500000, 1293750, 6250,
>>               0x7f, MT6358_BUCK_VGPU_ELR0, 0x7f,
>> MT6358_VCORE_VGPU_ANA_CON0, 2),
>> -    MT6366_BUCK("buck_vs2", VS2, 500000, 2087500, 12500,
>> +    MT6366_BUCK("vs2", VS2, 500000, 2087500, 12500,
>>               0x7f, MT6358_BUCK_VS2_DBG0, 0x7f, MT6358_VS2_ANA_CON0, 8),
>> -    MT6366_BUCK("buck_vmodem", VMODEM, 500000, 1293750, 6250,
>> +    MT6366_BUCK("vmodem", VMODEM, 500000, 1293750, 6250,
>>               0x7f, MT6358_BUCK_VMODEM_DBG0, 0x7f,
>> MT6358_VMODEM_ANA_CON0, 8),
>> -    MT6366_BUCK("buck_vs1", VS1, 1000000, 2587500, 12500,
>> +    MT6366_BUCK("vs1", VS1, 1000000, 2587500, 12500,
>>               0x7f, MT6358_BUCK_VS1_DBG0, 0x7f, MT6358_VS1_ANA_CON0, 8),
>> -    MT6366_REG_FIXED("ldo_vrf12", VRF12,
>> +    MT6366_REG_FIXED("vrf12", VRF12,
>>                MT6358_LDO_VRF12_CON0, 0, 1200000),
>> -    MT6366_REG_FIXED("ldo_vio18", VIO18,
>> +    MT6366_REG_FIXED("vio18", VIO18,
>>                MT6358_LDO_VIO18_CON0, 0, 1800000),
>> -    MT6366_REG_FIXED("ldo_vcn18", VCN18, MT6358_LDO_VCN18_CON0, 0,
>> 1800000),
>> -    MT6366_REG_FIXED("ldo_vfe28", VFE28, MT6358_LDO_VFE28_CON0, 0,
>> 2800000),
>> -    MT6366_REG_FIXED("ldo_vcn28", VCN28, MT6358_LDO_VCN28_CON0, 0,
>> 2800000),
>> -    MT6366_REG_FIXED("ldo_vxo22", VXO22, MT6358_LDO_VXO22_CON0, 0,
>> 2200000),
>> -    MT6366_REG_FIXED("ldo_vaux18", VAUX18,
>> +    MT6366_REG_FIXED("vcn18", VCN18, MT6358_LDO_VCN18_CON0, 0, 1800000),
>> +    MT6366_REG_FIXED("vfe28", VFE28, MT6358_LDO_VFE28_CON0, 0, 2800000),
>> +    MT6366_REG_FIXED("vcn28", VCN28, MT6358_LDO_VCN28_CON0, 0, 2800000),
>> +    MT6366_REG_FIXED("vxo22", VXO22, MT6358_LDO_VXO22_CON0, 0, 2200000),
>> +    MT6366_REG_FIXED("vaux18", VAUX18,
>>                MT6358_LDO_VAUX18_CON0, 0, 1800000),

The line being shorter, can't you compact them on a single line now ?
Here, and on other occasions in the changes you made.

Regards,

>> -    MT6366_REG_FIXED("ldo_vbif28", VBIF28,
>> +    MT6366_REG_FIXED("vbif28", VBIF28,
>>                MT6358_LDO_VBIF28_CON0, 0, 2800000),
>> -    MT6366_REG_FIXED("ldo_vio28", VIO28, MT6358_LDO_VIO28_CON0, 0,
>> 2800000),
>> -    MT6366_REG_FIXED("ldo_va12", VA12, MT6358_LDO_VA12_CON0, 0,
>> 1200000),
>> -    MT6366_REG_FIXED("ldo_vrf18", VRF18, MT6358_LDO_VRF18_CON0, 0,
>> 1800000),
>> -    MT6366_REG_FIXED("ldo_vaud28", VAUD28,
>> +    MT6366_REG_FIXED("vio28", VIO28, MT6358_LDO_VIO28_CON0, 0, 2800000),
>> +    MT6366_REG_FIXED("va12", VA12, MT6358_LDO_VA12_CON0, 0, 1200000),
>> +    MT6366_REG_FIXED("vrf18", VRF18, MT6358_LDO_VRF18_CON0, 0, 1800000),
>> +    MT6366_REG_FIXED("vaud28", VAUD28,
>>                MT6358_LDO_VAUD28_CON0, 0, 2800000),
>> -    MT6366_LDO("ldo_vdram2", VDRAM2, vdram2,
>> +    MT6366_LDO("vdram2", VDRAM2, vdram2,
>>              MT6358_LDO_VDRAM2_CON0, 0, MT6358_LDO_VDRAM2_ELR0, 0x10),
>> -    MT6366_LDO("ldo_vsim1", VSIM1, vsim,
>> +    MT6366_LDO("vsim1", VSIM1, vsim,
>>              MT6358_LDO_VSIM1_CON0, 0, MT6358_VSIM1_ANA_CON0, 0xf00),
>> -    MT6366_LDO("ldo_vibr", VIBR, vibr,
>> +    MT6366_LDO("vibr", VIBR, vibr,
>>              MT6358_LDO_VIBR_CON0, 0, MT6358_VIBR_ANA_CON0, 0xf00),
>> -    MT6366_LDO("ldo_vusb", VUSB, vusb,
>> +    MT6366_LDO("vusb", VUSB, vusb,
>>              MT6358_LDO_VUSB_CON0_0, 0, MT6358_VUSB_ANA_CON0, 0x700),
>> -    MT6366_LDO("ldo_vefuse", VEFUSE, vefuse,
>> +    MT6366_LDO("vefuse", VEFUSE, vefuse,
>>              MT6358_LDO_VEFUSE_CON0, 0, MT6358_VEFUSE_ANA_CON0, 0xf00),
>> -    MT6366_LDO("ldo_vmch", VMCH, vmch_vemc,
>> +    MT6366_LDO("vmch", VMCH, vmch_vemc,
>>              MT6358_LDO_VMCH_CON0, 0, MT6358_VMCH_ANA_CON0, 0x700),
>> -    MT6366_LDO("ldo_vemc", VEMC, vmch_vemc,
>> +    MT6366_LDO("vemc", VEMC, vmch_vemc,
>>              MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
>> -    MT6366_LDO("ldo_vcn33", VCN33, vcn33,
>> +    MT6366_LDO("vcn33", VCN33, vcn33,
>>              MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
>> -    MT6366_LDO("ldo_vmc", VMC, vmc,
>> +    MT6366_LDO("vmc", VMC, vmc,
>>              MT6358_LDO_VMC_CON0, 0, MT6358_VMC_ANA_CON0, 0xf00),
>> -    MT6366_LDO("ldo_vsim2", VSIM2, vsim,
>> +    MT6366_LDO("vsim2", VSIM2, vsim,
>>              MT6358_LDO_VSIM2_CON0, 0, MT6358_VSIM2_ANA_CON0, 0xf00),
>> -    MT6366_LDO1("ldo_vsram_proc11", VSRAM_PROC11, 500000, 1293750, 6250,
>> +    MT6366_LDO1("vsram-proc11", VSRAM_PROC11, 500000, 1293750, 6250,
>>               MT6358_LDO_VSRAM_PROC11_DBG0, 0x7f00,
>> MT6358_LDO_VSRAM_CON0, 0x7f),
>> -    MT6366_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
>> +    MT6366_LDO1("vsram-others", VSRAM_OTHERS, 500000, 1293750, 6250,
>>               MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
>> MT6358_LDO_VSRAM_CON2, 0x7f),
>> -    MT6366_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
>> +    MT6366_LDO1("vsram-gpu", VSRAM_GPU, 500000, 1293750, 6250,
>>               MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
>> MT6358_LDO_VSRAM_CON3, 0x7f),
>> -    MT6366_LDO1("ldo_vsram_proc12", VSRAM_PROC12, 500000, 1293750, 6250,
>> +    MT6366_LDO1("vsram-proc12", VSRAM_PROC12, 500000, 1293750, 6250,
>>               MT6358_LDO_VSRAM_PROC12_DBG0, 0x7f00,
>> MT6358_LDO_VSRAM_CON1, 0x7f),
>>   };
>
>
>


2023-08-04 16:50:35

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 1/9] dt-bindings: mfd: mt6397: Split out compatible for MediaTek MT6366 PMIC

On Thu, Aug 03, 2023 at 03:42:39PM +0800, Chen-Yu Tsai wrote:
> The MT6366 PMIC is mostly, but not fully, compatible with MT6358. It has
> a different set of regulators. Specifically, it lacks the camera related
> VCAM* LDOs, but has additional VM18, VMDDR, and VSRAM_CORE LDOs.
>
> Add a separate compatible for the MT6366 PMIC.
>
> Fixes: 49be16305587 ("dt-bindings: mfd: Add compatible for the MediaTek MT6366 PMIC")
> Signed-off-by: Chen-Yu Tsai <[email protected]>

Reviewed-by: Conor Dooley <[email protected]>

Thanks,
Conor.


Attachments:
(No filename) (559.00 B)
signature.asc (235.00 B)
Download all attachments

2023-08-07 05:36:16

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH 5/9] regulator: mt6358: fix and drop type prefix in MT6366 regulator node names

On Fri, Aug 4, 2023 at 6:13 PM Eugen Hristev
<[email protected]> wrote:
>
> Hi Chen-Yu,
>
> On 8/3/23 12:01, AngeloGioacchino Del Regno wrote:
> > Il 03/08/23 09:42, Chen-Yu Tsai ha scritto:
> >> The new MT6366 binding does away with the type prefix ("buck_", "ldo_")
> >> in the regulator node names. This better matches the PMIC pin names.
> >> Remaining underscores in names are also replaced with hyphens.
> >>
> >> Drop the type prefixes and replace remaining underscores to match the
> >> MT6366 binding.
> >>
> >> Signed-off-by: Chen-Yu Tsai <[email protected]>
> >
> > There is *no upstream devicetree* using any of those names, as the first
> > devices
> > that will use the MT6366 will be MT8186 Corsola, which are not
> > upstreamed yet.
> >
> > This means that this commit does not break ABI, so, I agree - which
> > implies....
> >
> > Reviewed-by: AngeloGioacchino Del Regno
> > <[email protected]>
> >
> > Cheers,
> > Angelo
> >
> >> ---
> >> drivers/regulator/mt6358-regulator.c | 70 ++++++++++++++--------------
> >> 1 file changed, 35 insertions(+), 35 deletions(-)
> >>
> >> diff --git a/drivers/regulator/mt6358-regulator.c
> >> b/drivers/regulator/mt6358-regulator.c
> >> index 3cb2e6768e25..5a43a84c7a3e 100644
> >> --- a/drivers/regulator/mt6358-regulator.c
> >> +++ b/drivers/regulator/mt6358-regulator.c
> >> @@ -536,68 +536,68 @@ static const struct mt6358_regulator_info
> >> mt6358_regulators[] = {
> >> /* The array is indexed by id(MT6366_ID_XXX) */
> >> static const struct mt6358_regulator_info mt6366_regulators[] = {
> >> - MT6366_BUCK("buck_vdram1", VDRAM1, 500000, 2087500, 12500,
> >> + MT6366_BUCK("vdram1", VDRAM1, 500000, 2087500, 12500,
> >> 0x7f, MT6358_BUCK_VDRAM1_DBG0, 0x7f,
> >> MT6358_VDRAM1_ANA_CON0, 8),
> >> - MT6366_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
> >> + MT6366_BUCK("vcore", VCORE, 500000, 1293750, 6250,
> >> 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
> >> MT6358_VCORE_VGPU_ANA_CON0, 1),
> >> - MT6366_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
> >> + MT6366_BUCK("vpa", VPA, 500000, 3650000, 50000,
> >> 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f, MT6358_VPA_ANA_CON0, 3),
> >> - MT6366_BUCK("buck_vproc11", VPROC11, 500000, 1293750, 6250,
> >> + MT6366_BUCK("vproc11", VPROC11, 500000, 1293750, 6250,
> >> 0x7f, MT6358_BUCK_VPROC11_DBG0, 0x7f,
> >> MT6358_VPROC_ANA_CON0, 1),
> >> - MT6366_BUCK("buck_vproc12", VPROC12, 500000, 1293750, 6250,
> >> + MT6366_BUCK("vproc12", VPROC12, 500000, 1293750, 6250,
> >> 0x7f, MT6358_BUCK_VPROC12_DBG0, 0x7f,
> >> MT6358_VPROC_ANA_CON0, 2),
> >> - MT6366_BUCK("buck_vgpu", VGPU, 500000, 1293750, 6250,
> >> + MT6366_BUCK("vgpu", VGPU, 500000, 1293750, 6250,
> >> 0x7f, MT6358_BUCK_VGPU_ELR0, 0x7f,
> >> MT6358_VCORE_VGPU_ANA_CON0, 2),
> >> - MT6366_BUCK("buck_vs2", VS2, 500000, 2087500, 12500,
> >> + MT6366_BUCK("vs2", VS2, 500000, 2087500, 12500,
> >> 0x7f, MT6358_BUCK_VS2_DBG0, 0x7f, MT6358_VS2_ANA_CON0, 8),
> >> - MT6366_BUCK("buck_vmodem", VMODEM, 500000, 1293750, 6250,
> >> + MT6366_BUCK("vmodem", VMODEM, 500000, 1293750, 6250,
> >> 0x7f, MT6358_BUCK_VMODEM_DBG0, 0x7f,
> >> MT6358_VMODEM_ANA_CON0, 8),
> >> - MT6366_BUCK("buck_vs1", VS1, 1000000, 2587500, 12500,
> >> + MT6366_BUCK("vs1", VS1, 1000000, 2587500, 12500,
> >> 0x7f, MT6358_BUCK_VS1_DBG0, 0x7f, MT6358_VS1_ANA_CON0, 8),
> >> - MT6366_REG_FIXED("ldo_vrf12", VRF12,
> >> + MT6366_REG_FIXED("vrf12", VRF12,
> >> MT6358_LDO_VRF12_CON0, 0, 1200000),
> >> - MT6366_REG_FIXED("ldo_vio18", VIO18,
> >> + MT6366_REG_FIXED("vio18", VIO18,
> >> MT6358_LDO_VIO18_CON0, 0, 1800000),
> >> - MT6366_REG_FIXED("ldo_vcn18", VCN18, MT6358_LDO_VCN18_CON0, 0,
> >> 1800000),
> >> - MT6366_REG_FIXED("ldo_vfe28", VFE28, MT6358_LDO_VFE28_CON0, 0,
> >> 2800000),
> >> - MT6366_REG_FIXED("ldo_vcn28", VCN28, MT6358_LDO_VCN28_CON0, 0,
> >> 2800000),
> >> - MT6366_REG_FIXED("ldo_vxo22", VXO22, MT6358_LDO_VXO22_CON0, 0,
> >> 2200000),
> >> - MT6366_REG_FIXED("ldo_vaux18", VAUX18,
> >> + MT6366_REG_FIXED("vcn18", VCN18, MT6358_LDO_VCN18_CON0, 0, 1800000),
> >> + MT6366_REG_FIXED("vfe28", VFE28, MT6358_LDO_VFE28_CON0, 0, 2800000),
> >> + MT6366_REG_FIXED("vcn28", VCN28, MT6358_LDO_VCN28_CON0, 0, 2800000),
> >> + MT6366_REG_FIXED("vxo22", VXO22, MT6358_LDO_VXO22_CON0, 0, 2200000),
> >> + MT6366_REG_FIXED("vaux18", VAUX18,
> >> MT6358_LDO_VAUX18_CON0, 0, 1800000),
>
> The line being shorter, can't you compact them on a single line now ?
> Here, and on other occasions in the changes you made.

I would prefer to do a final cleanup after all this has landed.
That makes the refactoring easier to read, i.e. identify the changes.

ChenYu

> Regards,
>
> >> - MT6366_REG_FIXED("ldo_vbif28", VBIF28,
> >> + MT6366_REG_FIXED("vbif28", VBIF28,
> >> MT6358_LDO_VBIF28_CON0, 0, 2800000),
> >> - MT6366_REG_FIXED("ldo_vio28", VIO28, MT6358_LDO_VIO28_CON0, 0,
> >> 2800000),
> >> - MT6366_REG_FIXED("ldo_va12", VA12, MT6358_LDO_VA12_CON0, 0,
> >> 1200000),
> >> - MT6366_REG_FIXED("ldo_vrf18", VRF18, MT6358_LDO_VRF18_CON0, 0,
> >> 1800000),
> >> - MT6366_REG_FIXED("ldo_vaud28", VAUD28,
> >> + MT6366_REG_FIXED("vio28", VIO28, MT6358_LDO_VIO28_CON0, 0, 2800000),
> >> + MT6366_REG_FIXED("va12", VA12, MT6358_LDO_VA12_CON0, 0, 1200000),
> >> + MT6366_REG_FIXED("vrf18", VRF18, MT6358_LDO_VRF18_CON0, 0, 1800000),
> >> + MT6366_REG_FIXED("vaud28", VAUD28,
> >> MT6358_LDO_VAUD28_CON0, 0, 2800000),
> >> - MT6366_LDO("ldo_vdram2", VDRAM2, vdram2,
> >> + MT6366_LDO("vdram2", VDRAM2, vdram2,
> >> MT6358_LDO_VDRAM2_CON0, 0, MT6358_LDO_VDRAM2_ELR0, 0x10),
> >> - MT6366_LDO("ldo_vsim1", VSIM1, vsim,
> >> + MT6366_LDO("vsim1", VSIM1, vsim,
> >> MT6358_LDO_VSIM1_CON0, 0, MT6358_VSIM1_ANA_CON0, 0xf00),
> >> - MT6366_LDO("ldo_vibr", VIBR, vibr,
> >> + MT6366_LDO("vibr", VIBR, vibr,
> >> MT6358_LDO_VIBR_CON0, 0, MT6358_VIBR_ANA_CON0, 0xf00),
> >> - MT6366_LDO("ldo_vusb", VUSB, vusb,
> >> + MT6366_LDO("vusb", VUSB, vusb,
> >> MT6358_LDO_VUSB_CON0_0, 0, MT6358_VUSB_ANA_CON0, 0x700),
> >> - MT6366_LDO("ldo_vefuse", VEFUSE, vefuse,
> >> + MT6366_LDO("vefuse", VEFUSE, vefuse,
> >> MT6358_LDO_VEFUSE_CON0, 0, MT6358_VEFUSE_ANA_CON0, 0xf00),
> >> - MT6366_LDO("ldo_vmch", VMCH, vmch_vemc,
> >> + MT6366_LDO("vmch", VMCH, vmch_vemc,
> >> MT6358_LDO_VMCH_CON0, 0, MT6358_VMCH_ANA_CON0, 0x700),
> >> - MT6366_LDO("ldo_vemc", VEMC, vmch_vemc,
> >> + MT6366_LDO("vemc", VEMC, vmch_vemc,
> >> MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
> >> - MT6366_LDO("ldo_vcn33", VCN33, vcn33,
> >> + MT6366_LDO("vcn33", VCN33, vcn33,
> >> MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
> >> - MT6366_LDO("ldo_vmc", VMC, vmc,
> >> + MT6366_LDO("vmc", VMC, vmc,
> >> MT6358_LDO_VMC_CON0, 0, MT6358_VMC_ANA_CON0, 0xf00),
> >> - MT6366_LDO("ldo_vsim2", VSIM2, vsim,
> >> + MT6366_LDO("vsim2", VSIM2, vsim,
> >> MT6358_LDO_VSIM2_CON0, 0, MT6358_VSIM2_ANA_CON0, 0xf00),
> >> - MT6366_LDO1("ldo_vsram_proc11", VSRAM_PROC11, 500000, 1293750, 6250,
> >> + MT6366_LDO1("vsram-proc11", VSRAM_PROC11, 500000, 1293750, 6250,
> >> MT6358_LDO_VSRAM_PROC11_DBG0, 0x7f00,
> >> MT6358_LDO_VSRAM_CON0, 0x7f),
> >> - MT6366_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
> >> + MT6366_LDO1("vsram-others", VSRAM_OTHERS, 500000, 1293750, 6250,
> >> MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
> >> MT6358_LDO_VSRAM_CON2, 0x7f),
> >> - MT6366_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
> >> + MT6366_LDO1("vsram-gpu", VSRAM_GPU, 500000, 1293750, 6250,
> >> MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
> >> MT6358_LDO_VSRAM_CON3, 0x7f),
> >> - MT6366_LDO1("ldo_vsram_proc12", VSRAM_PROC12, 500000, 1293750, 6250,
> >> + MT6366_LDO1("vsram-proc12", VSRAM_PROC12, 500000, 1293750, 6250,
> >> MT6358_LDO_VSRAM_PROC12_DBG0, 0x7f00,
> >> MT6358_LDO_VSRAM_CON1, 0x7f),
> >> };
> >
> >
> >
>

2023-08-07 06:57:36

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH 4/9] regulator: dt-bindings: mediatek: Add MT6366 PMIC

On Mon, Aug 7, 2023 at 2:23 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 03/08/2023 09:42, Chen-Yu Tsai wrote:
> > From: Zhiyong Tao <[email protected]>
> >
> > The MediaTek MT6366 PMIC is similar to the MT6358 PMIC. It is designed
> > to be paired with the MediaTek MT8186 SoC. It has 9 buck regulators and
> > 29 LDO regulators, not counting ones that feed internally and basically
> > have no controls. The regulators are named after their intended usage
> > for the SoC and system design, thus not named generically as ldoX or
> > dcdcX, but as vcn33 or vgpu.
> >
> > Add a binding document describing all the regulators and their supplies.
> >
> > Signed-off-by: Zhiyong Tao <[email protected]>
> > [[email protected]: major rework and added commit message]
> > Signed-off-by: Chen-Yu Tsai <[email protected]>
> > ---
> > Changes since Zhiyong's last version (v4) [1]:
> > - simplified regulator names
> > - added descriptions to regulators
> > - removed bogus regulators (*_sshub)
> > - merged vcn33-wifi and vcn33-bt as vcn33
> > - added missing regulators (vm18, vmddr, vsram-core)
> > - cut down examples to a handful of cases and made them complete
> > - expanded commit message a lot
> >
> > [1] https://lore.kernel.org/linux-arm-kernel/[email protected]/
> >
> > .../regulator/mediatek,mt6366-regulator.yaml | 154 ++++++++++++++++++
> > 1 file changed, 154 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml b/Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml
> > new file mode 100644
> > index 000000000000..715c6ffcb3ab
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml
> > @@ -0,0 +1,154 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/regulator/mediatek,mt6366-regulator.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MT6366 Regulator from MediaTek Integrated
> > +
> > +maintainers:
> > + - Zhiyong Tao <[email protected]>
> > +
> > +description: |
> > + Regulator part of the MT6366 PMIC. This node should be under the PMIC's
> > + device node.
> > +
> > +properties:
> > + compatible:
> > + const: mediatek,mt6366-regulator
> > +
> > + vsys_ldo1-supply:
>
> Don't use underscores in property names. Only hyphens, if needed.

I assume that applies even if the actual name in the datasheet has
underscores?

Will change, but will wait on discussion on patch 3 before sending a new
revision.

ChenYu


> > + description: Supply for LDOs vfe28, vxo22, vcn28, vaux18, vaud28, vsim1, vusb, vbif28
> > + vsys_ldo2-supply:
>
>
>
> Best regards,
> Krzysztof
>

2023-08-07 07:16:43

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/9] regulator: dt-bindings: mediatek: Add MT6366 PMIC

On 03/08/2023 09:42, Chen-Yu Tsai wrote:
> From: Zhiyong Tao <[email protected]>
>
> The MediaTek MT6366 PMIC is similar to the MT6358 PMIC. It is designed
> to be paired with the MediaTek MT8186 SoC. It has 9 buck regulators and
> 29 LDO regulators, not counting ones that feed internally and basically
> have no controls. The regulators are named after their intended usage
> for the SoC and system design, thus not named generically as ldoX or
> dcdcX, but as vcn33 or vgpu.
>
> Add a binding document describing all the regulators and their supplies.
>
> Signed-off-by: Zhiyong Tao <[email protected]>
> [[email protected]: major rework and added commit message]
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> ---
> Changes since Zhiyong's last version (v4) [1]:
> - simplified regulator names
> - added descriptions to regulators
> - removed bogus regulators (*_sshub)
> - merged vcn33-wifi and vcn33-bt as vcn33
> - added missing regulators (vm18, vmddr, vsram-core)
> - cut down examples to a handful of cases and made them complete
> - expanded commit message a lot
>
> [1] https://lore.kernel.org/linux-arm-kernel/[email protected]/
>
> .../regulator/mediatek,mt6366-regulator.yaml | 154 ++++++++++++++++++
> 1 file changed, 154 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml
>
> diff --git a/Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml b/Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml
> new file mode 100644
> index 000000000000..715c6ffcb3ab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml
> @@ -0,0 +1,154 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/mediatek,mt6366-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MT6366 Regulator from MediaTek Integrated
> +
> +maintainers:
> + - Zhiyong Tao <[email protected]>
> +
> +description: |
> + Regulator part of the MT6366 PMIC. This node should be under the PMIC's
> + device node.
> +
> +properties:
> + compatible:
> + const: mediatek,mt6366-regulator
> +
> + vsys_ldo1-supply:

Don't use underscores in property names. Only hyphens, if needed.

> + description: Supply for LDOs vfe28, vxo22, vcn28, vaux18, vaud28, vsim1, vusb, vbif28
> + vsys_ldo2-supply:



Best regards,
Krzysztof


2023-08-07 07:53:39

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/9] regulator: dt-bindings: mediatek: Add MT6366 PMIC

On 07/08/2023 08:30, Chen-Yu Tsai wrote:
> On Mon, Aug 7, 2023 at 2:23 PM Krzysztof Kozlowski
> <[email protected]> wrote:
>>
>> On 03/08/2023 09:42, Chen-Yu Tsai wrote:
>>> From: Zhiyong Tao <[email protected]>
>>>
>>> The MediaTek MT6366 PMIC is similar to the MT6358 PMIC. It is designed
>>> to be paired with the MediaTek MT8186 SoC. It has 9 buck regulators and
>>> 29 LDO regulators, not counting ones that feed internally and basically
>>> have no controls. The regulators are named after their intended usage
>>> for the SoC and system design, thus not named generically as ldoX or
>>> dcdcX, but as vcn33 or vgpu.
>>>
>>> Add a binding document describing all the regulators and their supplies.
>>>
>>> Signed-off-by: Zhiyong Tao <[email protected]>
>>> [[email protected]: major rework and added commit message]
>>> Signed-off-by: Chen-Yu Tsai <[email protected]>
>>> ---
>>> Changes since Zhiyong's last version (v4) [1]:
>>> - simplified regulator names
>>> - added descriptions to regulators
>>> - removed bogus regulators (*_sshub)
>>> - merged vcn33-wifi and vcn33-bt as vcn33
>>> - added missing regulators (vm18, vmddr, vsram-core)
>>> - cut down examples to a handful of cases and made them complete
>>> - expanded commit message a lot
>>>
>>> [1] https://lore.kernel.org/linux-arm-kernel/[email protected]/
>>>
>>> .../regulator/mediatek,mt6366-regulator.yaml | 154 ++++++++++++++++++
>>> 1 file changed, 154 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml b/Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml
>>> new file mode 100644
>>> index 000000000000..715c6ffcb3ab
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/regulator/mediatek,mt6366-regulator.yaml
>>> @@ -0,0 +1,154 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/regulator/mediatek,mt6366-regulator.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: MT6366 Regulator from MediaTek Integrated
>>> +
>>> +maintainers:
>>> + - Zhiyong Tao <[email protected]>
>>> +
>>> +description: |
>>> + Regulator part of the MT6366 PMIC. This node should be under the PMIC's
>>> + device node.
>>> +
>>> +properties:
>>> + compatible:
>>> + const: mediatek,mt6366-regulator
>>> +
>>> + vsys_ldo1-supply:
>>
>> Don't use underscores in property names. Only hyphens, if needed.
>
> I assume that applies even if the actual name in the datasheet has
> underscores?

Yes. We follow this already in other bindings. Hardware people can use
any names or characters in the datasheet, e.g. "*".


Best regards,
Krzysztof