2023-09-13 09:54:33

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs

Some of the LDO regulators in the MT6358/MT6366 have sparsely populated
voltage tables, supported by custom get/set operators. While it works,
it requires more code and an extra field to store the lookup table.
These LDOs also have fine voltage calibration settings that can slightly
boost the output voltage from 0 mV to 100 mV, in 10 mV increments.

These combined could be modeled as a pickable set of linear ranges. The
coarse voltage setting is modeled as the range selector, while each
range has 11 selectors, starting from the range's base voltage, up to
+100 mV, in 10mV increments.

Signed-off-by: Chen-Yu Tsai <[email protected]>
---
Changes since v2:
- selector values for all the linear ranges have been fixed.
I misinterpreted how the selector values for the pickable linear ranges
should be filled in.

Angelo's reviewed-by has been dropped as a result of the change.

drivers/regulator/mt6358-regulator.c | 275 +++++++++++----------------
1 file changed, 115 insertions(+), 160 deletions(-)

diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
index ecb2cd1eb34f..4a6ad0ccf03b 100644
--- a/drivers/regulator/mt6358-regulator.c
+++ b/drivers/regulator/mt6358-regulator.c
@@ -26,8 +26,6 @@ struct mt6358_regulator_info {
struct regulator_desc desc;
u32 status_reg;
u32 qi;
- const u32 *index_table;
- unsigned int n_table;
u32 da_vsel_reg;
u32 da_vsel_mask;
u32 modeset_reg;
@@ -64,9 +62,7 @@ struct mt6358_regulator_info {
.modeset_mask = BIT(_modeset_shift), \
}

-#define MT6358_LDO(match, vreg, ldo_volt_table, \
- ldo_index_table, enreg, enbit, vosel, \
- vosel_mask) \
+#define MT6358_LDO(match, vreg, volt_ranges, enreg, enbit, vosel, vosel_mask) \
[MT6358_ID_##vreg] = { \
.desc = { \
.name = #vreg, \
@@ -75,17 +71,19 @@ struct mt6358_regulator_info {
.type = REGULATOR_VOLTAGE, \
.id = MT6358_ID_##vreg, \
.owner = THIS_MODULE, \
- .n_voltages = ARRAY_SIZE(ldo_volt_table), \
- .volt_table = ldo_volt_table, \
- .vsel_reg = vosel, \
- .vsel_mask = vosel_mask, \
+ .n_voltages = ARRAY_SIZE(volt_ranges##_ranges) * 11, \
+ .linear_ranges = volt_ranges##_ranges, \
+ .linear_range_selectors_bitfield = volt_ranges##_selectors, \
+ .n_linear_ranges = ARRAY_SIZE(volt_ranges##_ranges), \
+ .vsel_range_reg = vosel, \
+ .vsel_range_mask = vosel_mask, \
+ .vsel_reg = MT6358_##vreg##_ANA_CON0, \
+ .vsel_mask = GENMASK(3, 0), \
.enable_reg = enreg, \
.enable_mask = BIT(enbit), \
}, \
.status_reg = MT6358_LDO_##vreg##_CON1, \
.qi = BIT(15), \
- .index_table = ldo_index_table, \
- .n_table = ARRAY_SIZE(ldo_index_table), \
}

#define MT6358_LDO1(match, vreg, min, max, step, \
@@ -163,9 +161,7 @@ struct mt6358_regulator_info {
.modeset_mask = BIT(_modeset_shift), \
}

-#define MT6366_LDO(match, vreg, ldo_volt_table, \
- ldo_index_table, enreg, enbit, vosel, \
- vosel_mask) \
+#define MT6366_LDO(match, vreg, volt_ranges, enreg, enbit, vosel, vosel_mask) \
[MT6366_ID_##vreg] = { \
.desc = { \
.name = #vreg, \
@@ -174,17 +170,19 @@ struct mt6358_regulator_info {
.type = REGULATOR_VOLTAGE, \
.id = MT6366_ID_##vreg, \
.owner = THIS_MODULE, \
- .n_voltages = ARRAY_SIZE(ldo_volt_table), \
- .volt_table = ldo_volt_table, \
- .vsel_reg = vosel, \
- .vsel_mask = vosel_mask, \
+ .n_voltages = ARRAY_SIZE(volt_ranges##_ranges) * 11, \
+ .linear_ranges = volt_ranges##_ranges, \
+ .linear_range_selectors_bitfield = volt_ranges##_selectors, \
+ .n_linear_ranges = ARRAY_SIZE(volt_ranges##_ranges), \
+ .vsel_range_reg = vosel, \
+ .vsel_range_mask = vosel_mask, \
+ .vsel_reg = MT6358_##vreg##_ANA_CON0, \
+ .vsel_mask = GENMASK(3, 0), \
.enable_reg = enreg, \
.enable_mask = BIT(enbit), \
}, \
.status_reg = MT6358_LDO_##vreg##_CON1, \
.qi = BIT(15), \
- .index_table = ldo_index_table, \
- .n_table = ARRAY_SIZE(ldo_index_table), \
}

#define MT6366_LDO1(match, vreg, min, max, step, \
@@ -235,95 +233,95 @@ struct mt6358_regulator_info {
}


-static const unsigned int vdram2_voltages[] = {
- 600000, 1800000,
-};
-
-static const unsigned int vsim_voltages[] = {
- 1700000, 1800000, 2700000, 3000000, 3100000,
-};
-
-static const unsigned int vibr_voltages[] = {
- 1200000, 1300000, 1500000, 1800000,
- 2000000, 2800000, 3000000, 3300000,
+/* VDRAM2 voltage selector not shown in datasheet */
+static const unsigned int vdram2_selectors[] = { 0, 12 };
+static const struct linear_range vdram2_ranges[] = {
+ REGULATOR_LINEAR_RANGE(600000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(1800000, 0, 10, 10000),
};

-static const unsigned int vusb_voltages[] = {
- 3000000, 3100000,
+static const unsigned int vsim_selectors[] = { 3, 4, 8, 11, 12 };
+static const struct linear_range vsim_ranges[] = {
+ REGULATOR_LINEAR_RANGE(1700000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(1800000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(2700000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(3000000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(3100000, 0, 10, 10000),
};

-static const unsigned int vcamd_voltages[] = {
- 900000, 1000000, 1100000, 1200000,
- 1300000, 1500000, 1800000,
+static const unsigned int vibr_selectors[] = { 0, 1, 2, 4, 5, 9, 11, 13 };
+static const struct linear_range vibr_ranges[] = {
+ REGULATOR_LINEAR_RANGE(1200000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(1300000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(1500000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(1800000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(2000000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(2800000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(3000000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(3300000, 0, 10, 10000),
};

-static const unsigned int vefuse_voltages[] = {
- 1700000, 1800000, 1900000,
+/* VUSB voltage selector not shown in datasheet */
+static const unsigned int vusb_selectors[] = { 3, 4 };
+static const struct linear_range vusb_ranges[] = {
+ REGULATOR_LINEAR_RANGE(3000000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(3100000, 0, 10, 10000),
};

-static const unsigned int vmch_vemc_voltages[] = {
- 2900000, 3000000, 3300000,
+static const unsigned int vcamd_selectors[] = { 3, 4, 5, 6, 7, 9, 12 };
+static const struct linear_range vcamd_ranges[] = {
+ REGULATOR_LINEAR_RANGE(900000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(1000000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(1100000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(1200000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(1300000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(1500000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(1800000, 0, 10, 10000),
};

-static const unsigned int vcama_voltages[] = {
- 1800000, 2500000, 2700000,
- 2800000, 2900000, 3000000,
+static const unsigned int vefuse_selectors[] = { 11, 12, 13 };
+static const struct linear_range vefuse_ranges[] = {
+ REGULATOR_LINEAR_RANGE(1700000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(1800000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(1900000, 0, 10, 10000),
};

-static const unsigned int vcn33_voltages[] = {
- 3300000, 3400000, 3500000,
+static const unsigned int vmch_vemc_selectors[] = { 2, 3, 5 };
+static const struct linear_range vmch_vemc_ranges[] = {
+ REGULATOR_LINEAR_RANGE(2900000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(3000000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(3300000, 0, 10, 10000),
};

-static const unsigned int vmc_voltages[] = {
- 1800000, 2900000, 3000000, 3300000,
+static const unsigned int vcama_selectors[] = { 0, 7, 9, 10, 11, 12 };
+static const struct linear_range vcama_ranges[] = {
+ REGULATOR_LINEAR_RANGE(1800000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(2500000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(2700000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(2800000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(2900000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(3000000, 0, 10, 10000),
};

-static const unsigned int vldo28_voltages[] = {
- 2800000, 3000000,
+static const unsigned int vcn33_selectors[] = { 1, 2, 3 };
+static const struct linear_range vcn33_ranges[] = {
+ REGULATOR_LINEAR_RANGE(3300000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(3400000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(3500000, 0, 10, 10000),
};

-static const u32 vdram2_idx[] = {
- 0, 12,
+static const unsigned int vmc_selectors[] = { 4, 10, 11, 13 };
+static const struct linear_range vmc_ranges[] = {
+ REGULATOR_LINEAR_RANGE(1800000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(2900000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(3000000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(3300000, 0, 10, 10000),
};

-static const u32 vsim_idx[] = {
- 3, 4, 8, 11, 12,
-};
-
-static const u32 vibr_idx[] = {
- 0, 1, 2, 4, 5, 9, 11, 13,
-};
-
-static const u32 vusb_idx[] = {
- 3, 4,
-};
-
-static const u32 vcamd_idx[] = {
- 3, 4, 5, 6, 7, 9, 12,
-};
-
-static const u32 vefuse_idx[] = {
- 11, 12, 13,
-};
-
-static const u32 vmch_vemc_idx[] = {
- 2, 3, 5,
-};
-
-static const u32 vcama_idx[] = {
- 0, 7, 9, 10, 11, 12,
-};
-
-static const u32 vcn33_idx[] = {
- 1, 2, 3,
-};
-
-static const u32 vmc_idx[] = {
- 4, 10, 11, 13,
-};
-
-static const u32 vldo28_idx[] = {
- 1, 3,
+static const unsigned int vldo28_selectors[] = { 1, 3 };
+static const struct linear_range vldo28_ranges[] = {
+ REGULATOR_LINEAR_RANGE(2800000, 0, 10, 10000),
+ REGULATOR_LINEAR_RANGE(3000000, 0, 10, 10000),
};

static unsigned int mt6358_map_mode(unsigned int mode)
@@ -332,49 +330,6 @@ static unsigned int mt6358_map_mode(unsigned int mode)
REGULATOR_MODE_NORMAL : REGULATOR_MODE_FAST;
}

-static int mt6358_set_voltage_sel(struct regulator_dev *rdev,
- unsigned int selector)
-{
- const struct mt6358_regulator_info *info = to_regulator_info(rdev->desc);
- int idx, ret;
- const u32 *pvol;
-
- pvol = info->index_table;
-
- idx = pvol[selector];
- idx <<= ffs(info->desc.vsel_mask) - 1;
- ret = regmap_update_bits(rdev->regmap, info->desc.vsel_reg,
- info->desc.vsel_mask, idx);
-
- return ret;
-}
-
-static int mt6358_get_voltage_sel(struct regulator_dev *rdev)
-{
- const struct mt6358_regulator_info *info = to_regulator_info(rdev->desc);
- int idx, ret;
- u32 selector;
- const u32 *pvol;
-
- ret = regmap_read(rdev->regmap, info->desc.vsel_reg, &selector);
- if (ret != 0) {
- dev_info(&rdev->dev,
- "Failed to get mt6358 %s vsel reg: %d\n",
- info->desc.name, ret);
- return ret;
- }
-
- selector = (selector & info->desc.vsel_mask) >>
- (ffs(info->desc.vsel_mask) - 1);
- pvol = info->index_table;
- for (idx = 0; idx < info->desc.n_voltages; idx++) {
- if (pvol[idx] == selector)
- return idx;
- }
-
- return -EINVAL;
-}
-
static int mt6358_get_buck_voltage_sel(struct regulator_dev *rdev)
{
const struct mt6358_regulator_info *info = to_regulator_info(rdev->desc);
@@ -477,10 +432,10 @@ static const struct regulator_ops mt6358_volt_range_ops = {
};

static const struct regulator_ops mt6358_volt_table_ops = {
- .list_voltage = regulator_list_voltage_table,
- .map_voltage = regulator_map_voltage_iterate,
- .set_voltage_sel = mt6358_set_voltage_sel,
- .get_voltage_sel = mt6358_get_voltage_sel,
+ .list_voltage = regulator_list_voltage_pickable_linear_range,
+ .map_voltage = regulator_map_voltage_pickable_linear_range,
+ .set_voltage_sel = regulator_set_voltage_sel_pickable_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_pickable_regmap,
.set_voltage_time_sel = regulator_set_voltage_time_sel,
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,
@@ -540,34 +495,34 @@ static const struct mt6358_regulator_info mt6358_regulators[] = {
MT6358_REG_FIXED("ldo_vrf18", VRF18, MT6358_LDO_VRF18_CON0, 0, 1800000),
MT6358_REG_FIXED("ldo_vaud28", VAUD28,
MT6358_LDO_VAUD28_CON0, 0, 2800000),
- MT6358_LDO("ldo_vdram2", VDRAM2, vdram2_voltages, vdram2_idx,
+ MT6358_LDO("ldo_vdram2", VDRAM2, vdram2,
MT6358_LDO_VDRAM2_CON0, 0, MT6358_LDO_VDRAM2_ELR0, 0xf),
- MT6358_LDO("ldo_vsim1", VSIM1, vsim_voltages, vsim_idx,
+ MT6358_LDO("ldo_vsim1", VSIM1, vsim,
MT6358_LDO_VSIM1_CON0, 0, MT6358_VSIM1_ANA_CON0, 0xf00),
- MT6358_LDO("ldo_vibr", VIBR, vibr_voltages, vibr_idx,
+ MT6358_LDO("ldo_vibr", VIBR, vibr,
MT6358_LDO_VIBR_CON0, 0, MT6358_VIBR_ANA_CON0, 0xf00),
- MT6358_LDO("ldo_vusb", VUSB, vusb_voltages, vusb_idx,
+ MT6358_LDO("ldo_vusb", VUSB, vusb,
MT6358_LDO_VUSB_CON0_0, 0, MT6358_VUSB_ANA_CON0, 0x700),
- MT6358_LDO("ldo_vcamd", VCAMD, vcamd_voltages, vcamd_idx,
+ MT6358_LDO("ldo_vcamd", VCAMD, vcamd,
MT6358_LDO_VCAMD_CON0, 0, MT6358_VCAMD_ANA_CON0, 0xf00),
- MT6358_LDO("ldo_vefuse", VEFUSE, vefuse_voltages, vefuse_idx,
+ MT6358_LDO("ldo_vefuse", VEFUSE, vefuse,
MT6358_LDO_VEFUSE_CON0, 0, MT6358_VEFUSE_ANA_CON0, 0xf00),
- MT6358_LDO("ldo_vmch", VMCH, vmch_vemc_voltages, vmch_vemc_idx,
+ MT6358_LDO("ldo_vmch", VMCH, vmch_vemc,
MT6358_LDO_VMCH_CON0, 0, MT6358_VMCH_ANA_CON0, 0x700),
- MT6358_LDO("ldo_vcama1", VCAMA1, vcama_voltages, vcama_idx,
+ MT6358_LDO("ldo_vcama1", VCAMA1, vcama,
MT6358_LDO_VCAMA1_CON0, 0, MT6358_VCAMA1_ANA_CON0, 0xf00),
- MT6358_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx,
+ MT6358_LDO("ldo_vemc", VEMC, vmch_vemc,
MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
- MT6358_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx,
+ MT6358_LDO("ldo_vcn33", VCN33, vcn33,
MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
- MT6358_LDO("ldo_vcama2", VCAMA2, vcama_voltages, vcama_idx,
+ MT6358_LDO("ldo_vcama2", VCAMA2, vcama,
MT6358_LDO_VCAMA2_CON0, 0, MT6358_VCAMA2_ANA_CON0, 0xf00),
- MT6358_LDO("ldo_vmc", VMC, vmc_voltages, vmc_idx,
+ MT6358_LDO("ldo_vmc", VMC, vmc,
MT6358_LDO_VMC_CON0, 0, MT6358_VMC_ANA_CON0, 0xf00),
- MT6358_LDO("ldo_vldo28", VLDO28, vldo28_voltages, vldo28_idx,
+ MT6358_LDO("ldo_vldo28", VLDO28, vldo28,
MT6358_LDO_VLDO28_CON0_0, 0,
MT6358_VLDO28_ANA_CON0, 0x300),
- MT6358_LDO("ldo_vsim2", VSIM2, vsim_voltages, vsim_idx,
+ MT6358_LDO("ldo_vsim2", VSIM2, vsim,
MT6358_LDO_VSIM2_CON0, 0, MT6358_VSIM2_ANA_CON0, 0xf00),
MT6358_LDO1("ldo_vsram_proc11", VSRAM_PROC11, 500000, 1293750, 6250,
MT6358_LDO_VSRAM_PROC11_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON0, 0x7f),
@@ -616,25 +571,25 @@ static const struct mt6358_regulator_info mt6366_regulators[] = {
MT6366_REG_FIXED("ldo_vrf18", VRF18, MT6358_LDO_VRF18_CON0, 0, 1800000),
MT6366_REG_FIXED("ldo_vaud28", VAUD28,
MT6358_LDO_VAUD28_CON0, 0, 2800000),
- MT6366_LDO("ldo_vdram2", VDRAM2, vdram2_voltages, vdram2_idx,
+ MT6366_LDO("ldo_vdram2", VDRAM2, vdram2,
MT6358_LDO_VDRAM2_CON0, 0, MT6358_LDO_VDRAM2_ELR0, 0x10),
- MT6366_LDO("ldo_vsim1", VSIM1, vsim_voltages, vsim_idx,
+ MT6366_LDO("ldo_vsim1", VSIM1, vsim,
MT6358_LDO_VSIM1_CON0, 0, MT6358_VSIM1_ANA_CON0, 0xf00),
- MT6366_LDO("ldo_vibr", VIBR, vibr_voltages, vibr_idx,
+ MT6366_LDO("ldo_vibr", VIBR, vibr,
MT6358_LDO_VIBR_CON0, 0, MT6358_VIBR_ANA_CON0, 0xf00),
- MT6366_LDO("ldo_vusb", VUSB, vusb_voltages, vusb_idx,
+ MT6366_LDO("ldo_vusb", VUSB, vusb,
MT6358_LDO_VUSB_CON0_0, 0, MT6358_VUSB_ANA_CON0, 0x700),
- MT6366_LDO("ldo_vefuse", VEFUSE, vefuse_voltages, vefuse_idx,
+ MT6366_LDO("ldo_vefuse", VEFUSE, vefuse,
MT6358_LDO_VEFUSE_CON0, 0, MT6358_VEFUSE_ANA_CON0, 0xf00),
- MT6366_LDO("ldo_vmch", VMCH, vmch_vemc_voltages, vmch_vemc_idx,
+ MT6366_LDO("ldo_vmch", VMCH, vmch_vemc,
MT6358_LDO_VMCH_CON0, 0, MT6358_VMCH_ANA_CON0, 0x700),
- MT6366_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx,
+ MT6366_LDO("ldo_vemc", VEMC, vmch_vemc,
MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
- MT6366_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx,
+ MT6366_LDO("ldo_vcn33", VCN33, vcn33,
MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
- MT6366_LDO("ldo_vmc", VMC, vmc_voltages, vmc_idx,
+ MT6366_LDO("ldo_vmc", VMC, vmc,
MT6358_LDO_VMC_CON0, 0, MT6358_VMC_ANA_CON0, 0xf00),
- MT6366_LDO("ldo_vsim2", VSIM2, vsim_voltages, vsim_idx,
+ MT6366_LDO("ldo_vsim2", VSIM2, vsim,
MT6358_LDO_VSIM2_CON0, 0, MT6358_VSIM2_ANA_CON0, 0xf00),
MT6366_LDO1("ldo_vsram_proc11", VSRAM_PROC11, 500000, 1293750, 6250,
MT6358_LDO_VSRAM_PROC11_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON0, 0x7f),
--
2.42.0.283.g2d96d420d3-goog


Subject: Re: [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs

Il 13/09/23 10:29, Chen-Yu Tsai ha scritto:
> Some of the LDO regulators in the MT6358/MT6366 have sparsely populated
> voltage tables, supported by custom get/set operators. While it works,
> it requires more code and an extra field to store the lookup table.
> These LDOs also have fine voltage calibration settings that can slightly
> boost the output voltage from 0 mV to 100 mV, in 10 mV increments.
>
> These combined could be modeled as a pickable set of linear ranges. The
> coarse voltage setting is modeled as the range selector, while each
> range has 11 selectors, starting from the range's base voltage, up to
> +100 mV, in 10mV increments.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>

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


2023-11-15 20:16:03

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs

On Wed, Nov 15, 2023 at 06:52:12PM +0200, Bret Joseph wrote:
> *In mt6358-regulator.c*

I only have patch 3/3 from this series, what's going on with
dependencies and so on?


Attachments:
(No filename) (178.00 B)
signature.asc (499.00 B)
Download all attachments

2023-11-16 03:51:01

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs

On Thu, Nov 16, 2023 at 4:15 AM Mark Brown <[email protected]> wrote:
>
> On Wed, Nov 15, 2023 at 06:52:12PM +0200, Bret Joseph wrote:
> > *In mt6358-regulator.c*
>
> I only have patch 3/3 from this series, what's going on with
> dependencies and so on?

You already applied the series in September and the changes are already in
v6.7-rc1.

I'll take a look at the report by Bret.

ChenYu

2023-11-23 06:25:29

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs

Hi Bret,

On Thu, Nov 16, 2023 at 12:52 AM Bret Joseph <[email protected]> wrote:
>
> In mt6358-regulator.c
>
> static const struct regulator_ops mt6358_volt_table_ops = {
> .list_voltage = regulator_list_voltage_pickable_linear_range,
> .map_voltage = regulator_map_voltage_pickable_linear_range,
> .set_voltage_sel = regulator_set_voltage_sel_pickable_regmap,
> .get_voltage_sel = mt6358_get_buck_voltage_sel /*regulator_get_voltage_sel_pickable_regmap*/,
> .set_voltage_time_sel = regulator_set_voltage_time_sel,
> .enable = regulator_enable_regmap,
> .disable = regulator_disable_regmap,
> .is_enabled = regulator_is_enabled_regmap,
> .get_status = mt6358_get_status,
> };
>
> the function [regulator_get_voltage_sel_pickable_regmap] causes a -EINVAL‬ when registering
>
> vsim1 vusb vcamd vefuse vmch vcama1 vemc vmc vldo28 vsim2
>
> using [mt6358_get_buck_voltage_sel] results in a successful probe

Sure it will probe, but any values you read back will be bogus.

Can you provide a dump of /sys/kernel/debug/regulator/regulator_summary
and `grep ^1e..: /sys/kernel/debug/regmap/1000d000.pwrap/registers`,
and also any relevant logs?

ChenYu

2023-11-24 04:30:03

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs

Please keep the whole CC list. Please don't top post.

On Thu, Nov 23, 2023 at 7:27 PM Bret Joseph <[email protected]> wrote:
>
> /sys/kernel/debug/regmap/1000d000.pwrap/registers
> 1e00: 2808
> 1e02: 2808
> 1e04: 2000
> 1e06: 2000
> 1e08: 2808

According to the datasheet I have, x8xx is not a valid voltage setting.
It wasn't valid before my patch either.

This is vcama1.

> 1e0a: 2808
> 1e0c: 2000
> 1e0e: 2000
> 1e10: 0000 # I have deleted everything with 0000
> 1e20: 2808
> 1e22: 2808
> 1e24: 2000
> 1e26: 2000
> 1e28: 2808

x8xx is not valid here either. This is vcn33.

> 1e2a: 2808
> 1e2c: 2000

x0xx is not valid here either. This is vsim1.

> 1e2e: 2000

x0xx for register 0x1e30 is not valid. This is vsim2.
x0xx for register 0x1e38 is not valid. This is vemc.
x0xx for register 0x1e3c is not valid. This is vldo28.
x0xx for register 0x1e48 is not valid. This is vmch.
x0xx for register 0x1e4c is not valid. This is vmc.

> 1e80: 2808
> 1e82: 2808
> 1e84: 2000
> 1e86: 2000
> 1e88: 2808
> 1e8a: 2808
> 1e8c: 2000
> 1e8e: 2000

x0xx for register 0x1e98 is not valid. This is vefuse.

> 1ea0: 2808
> 1ea2: 2808
> 1ea4: 2000
> 1ea6: 2000
> 1ea8: 2808
> 1eaa: 2808
> 1eac: 2000
> 1eae: 2000

x0xx for register 0x1eae is not valid. This is vcamd.

So basically all the failures you see are due to (as far as the datasheet
says) invalid values. The driver prior to my patch would error out as well.

Either the register readback is faulty on your platform, or your bootloader
has configured the system into a weird state.


ChenYu

> /sys/kernel/debug/regulator/regulator_summary
> regulator use open bypass opmode voltage
> current min max
> ---------------------------------------------------------------------------------------
> regulator-dummy 3 35 0 unknown 0mV 0mA 0mV 0mV
> vdram1 0 0 0 normal 500mV 0mA
> 500mV 2087mV
> vcore 0 0 0 normal 500mV 0mA
> 500mV 1293mV
> vpa 0 0 0 normal 500mV
> 0mA 500mV 3650mV
> vproc11 0 0 0 normal 500mV 0mA
> 500mV 1293mV
> vproc12 0 0 0 normal 500mV 0mA
> 500mV 1293mV
> vgpu 0 0 0 normal 500mV 0mA
> 500mV 1293mV
> vs2 0 0 0 normal 500mV
> 0mA 500mV 2087mV
> vmodem 0 0 0 normal 500mV 0mA
> 500mV 1293mV
> vs1 2 5 0 normal 1000mV
> 0mA 1000mV 2587mV
> vio18 1 0 0 unknown 1800mV 0mA
> 1800mV 1800mV
> vcamio 0 0 0 unknown 1800mV 0mA
> 1800mV 1800mV
> vcn18 0 0 0 unknown 1800mV 0mA
> 1800mV 1800mV
> vefuse 0 0 0 unknown 1700mV 0mA
> 1700mV 1900mV
> vrf18 0 0 0 unknown 1800mV
> 0mA 1800mV 1800mV
> vdram2 0 0 0 unknown 600mV 0mA
> 600mV 1800mV
> vsim1 0 0 0 unknown 1700mV 0mA
> 1700mV 3100mV
> vibr 0 0 0 unknown 1200mV
> 0mA 1200mV 3300mV
> vrf12 0 0 0 unknown 1200mV
> 0mA 1200mV 1200mV
> vusb 0 0 0 unknown 3000mV
> 0mA 3000mV 3100mV
> vcamd 0 0 0 unknown 900mV 0mA
> 900mV 1800mV
> vfe28 0 0 0 unknown 2800mV
> 0mA 2800mV 2800mV
> vsram_proc11 0 0 0 unknown 500mV 0mA 500mV 1293mV
> vcn28 0 0 0 unknown 2800mV 0mA
> 2800mV 2800mV
> vsram_others 0 0 0 unknown 500mV 0mA
> 500mV 1293mV
> vsram_gpu 0 0 0 unknown 500mV 0mA
> 500mV 1293mV
> vxo22 0 0 0 unknown 2200mV 0mA
> 2200mV 2200mV
> vaux18 0 0 0 unknown 1800mV 0mA
> 1800mV 1800mV
> vmch 0 0 0 unknown 2900mV 0mA
> 2900mV 3300mV
> vbif28 0 0 0 unknown 2800mV
> 0mA 2800mV 2800mV
> vsram_proc12 0 0 0 unknown 550mV 0mA 500mV 1293mV
> vcama1 0 0 0 unknown 1800mV 0mA
> 1800mV 3000mV
> vemc 0 0 0 unknown 2900mV 0mA
> 2900mV 3300mV
> vio28 1 0 0 unknown 2800mV
> 0mA 2800mV 2800mV
> va12 0 0 0 unknown 1200mV
> 0mA 1200mV 1200mV
> VCN33 0 0 0 unknown 3300mV 0mA
> 0mV 0mV
> vcama2 0 0 0 unknown 1800mV 0mA
> 1800mV 3000mV
> vmc 0 0 0 unknown 1800mV
> 0mA 1800mV 3300mV
> vldo28 0 0 0 unknown 2800mV 0mA
> 2800mV 3000mV
> vaud28 0 0 0 unknown 2800mV 0mA
> 2800mV 2800mV
> vsim2 0 0 0 unknown 1700mV 0mA
> 1700mV 3100mV
>
> I have also seen errors when consumers use regulators which I think
> could be related to setting values on regulators
> Here is an example using mtk-ccifreq driver
> [ 0.668869] mtk-ccifreq cci: no pinctrl handle
> [ 0.669128] device: 'regulator:regulator.5--platform:cci': device_add
> [ 0.669278] devices_kset: Moving cci to end of list
> [ 0.669311] PM: Moving platform:cci to end of list
> [ 0.669340] mtk-ccifreq cci: Linked as a consumer to regulator.5
> [ 0.669713] of: _opp_add_static_v2: turbo:0 rate:500000000 uv:600000
> uvmin:600000 uvmax:600000 latency:0 level:0
> [ 0.669969] of: _opp_add_static_v2: turbo:0 rate:560000000 uv:675000
> uvmin:675000 uvmax:675000 latency:0 level:0
> [ 0.670245] of: _opp_add_static_v2: turbo:0 rate:612000000 uv:693750
> uvmin:693750 uvmax:693750 latency:0 level:0
> [ 0.670512] of: _opp_add_static_v2: turbo:0 rate:682000000 uv:718750
> uvmin:718750 uvmax:718750 latency:0 level:0
> [ 0.670763] of: _opp_add_static_v2: turbo:0 rate:752000000 uv:743750
> uvmin:743750 uvmax:743750 latency:0 level:0
> [ 0.671031] of: _opp_add_static_v2: turbo:0 rate:822000000 uv:768750
> uvmin:768750 uvmax:768750 latency:0 level:0
> [ 0.671598] of: _opp_add_static_v2: turbo:0 rate:875000000 uv:781250
> uvmin:781250 uvmax:781250 latency:0 level:0
> [ 0.671921] of: _opp_add_static_v2: turbo:0 rate:927000000 uv:800000
> uvmin:800000 uvmax:800000 latency:0 level:0
> [ 0.672178] of: _opp_add_static_v2: turbo:0 rate:980000000 uv:818750
> uvmin:818750 uvmax:818750 latency:0 level:0
> [ 0.672434] of: _opp_add_static_v2: turbo:0 rate:1050000000 uv:843750
> uvmin:843750 uvmax:843750 latency:0 level:0
> [ 0.672681] of: _opp_add_static_v2: turbo:0 rate:1120000000 uv:862500
> uvmin:862500 uvmax:862500 latency:0 level:0
> [ 0.672945] of: _opp_add_static_v2: turbo:0 rate:1155000000 uv:887500
> uvmin:887500 uvmax:887500 latency:0 level:0
> [ 0.673200] of: _opp_add_static_v2: turbo:0 rate:1190000000 uv:906250
> uvmin:906250 uvmax:906250 latency:0 level:0
> [ 0.673465] of: _opp_add_static_v2: turbo:0 rate:1260000000 uv:950000
> uvmin:950000 uvmax:950000 latency:0 level:0
> [ 0.673720] of: _opp_add_static_v2: turbo:0 rate:1330000000 uv:993750
> uvmin:993750 uvmax:993750 latency:0 level:0
> [ 0.673988] of: _opp_add_static_v2: turbo:0 rate:1400000000 uv:1031250
> uvmin:1031250 uvmax:1031250 latency:0 level:0
> [ 0.674327] device: 'cci': device_add
> [ 0.674499] PM: Adding info for No Bus:cci
> [ 0.674643] mtk-ccifreq cci: error -EPROBE_DEFER: devfreq_add_device:
> Unable to start governor for the device
> [ 0.675571] device: 'cci': device_unregister
> [ 0.675730] PM: Removing info for No Bus:cci
> [ 0.675862] mtk-ccifreq cci: failed to add devfreq device: -517
> [ 0.680084] mtk-ccifreq cci: Driver mtk-ccifreq requests probe deferral
> [ 0.680148] ------------[ cut here ]------------
> [ 0.680163] WARNING: CPU: 6 PID: 56 at _regulator_put+0x340/0x354
> [ 0.680190] Modules linked in:
> [ 0.680208] CPU: 6 PID: 56 Comm: kworker/u16:1 Tainted: G W
> 6.7.0-rc1-next-20231115-g2b600507077d-dirty #34
> [ 0.680232] Hardware name: MT6769H (DT)
> [ 0.680246] Workqueue: events_unbound deferred_probe_work_func
> [ 0.680268] pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 0.680285] pc : _regulator_put+0x340/0x354
> [ 0.680302] lr : regulator_put+0x58/0x8c
> [ 0.680317] sp : ffffffc080483950
> [ 0.680328] x29: ffffffc080483950 x28: ffffff80c0030000 x27: ffffffdd002037f0
> [ 0.680353] x26: ffffffdd01747650 x25: ffffff80c0f5d0c0 x24: ffffffdd00180d80
> [ 0.680376] x23: ffffffc080483a98 x22: ffffffdd01f46d50 x21: ffffff80c0f5d780
> [ 0.680400] x20: ffffffdd01ee2bc0 x19: ffffff80c0f5d780 x18: ffffff80c1f8c660
> [ 0.680423] x17: 000000040044ffff x16: 005000f4b5593519 x15: ffffffdd0221fdb0
> [ 0.680446] x14: 0000000000000891 x13: 0000000000000891 x12: 0000000000000001
> [ 0.680468] x11: 0000000000000891 x10: 000000000001cde2 x9 : 0000000000001464
> [ 0.680491] x8 : 000000008020001f x7 : ffffff80c249e600 x6 : 000000000000018d
> [ 0.680513] x5 : 00000000000091ec x4 : 00000000000091d3 x3 : ffffffdd01a5e070
> [ 0.680535] x2 : 0000000000000001 x1 : ffffffdd01ee2b60 x0 : 0000000000000002
> [ 0.680558] Call trace:
> [ 0.680568] _regulator_put+0x340/0x354
> [ 0.680583] regulator_put+0x58/0x8c
> [ 0.680597] devm_regulator_release+0x30/0x4c
> [ 0.680613] release_nodes+0x8c/0xdc
> [ 0.680629] devres_release_all+0xc0/0x164
> [ 0.680646] device_unbind_cleanup+0x34/0xe4
> [ 0.680660] really_probe+0x200/0x75c
> [ 0.680674] __driver_probe_device+0x100/0x2e4
> [ 0.680688] driver_probe_device+0x6c/0x1d4
> [ 0.680703] __device_attach_driver+0x170/0x238
> [ 0.680717] bus_for_each_drv+0xbc/0x18c
> [ 0.680734] __device_attach+0x124/0x2ec
> [ 0.680748] device_initial_probe+0x30/0x4c
> [ 0.680762] bus_probe_device+0x138/0x168
> [ 0.680775] deferred_probe_work_func+0x134/0x1fc
> [ 0.680790] process_scheduled_works+0x26c/0x73c
> [ 0.680808] worker_thread+0x294/0x690
> [ 0.680823] kthread+0x1fc/0x22c
> [ 0.680837] ret_from_fork+0x10/0x20
> [ 0.680853] ---[ end trace 0000000000000000 ]---
> [ 0.680923] mtk-ccifreq cci: Dropping the link to regulator.5
> [ 0.680941] device: 'regulator:regulator.5--platform:cci': device_unregister
> [ 0.681041] platform cci: Added to deferred list
>
> On Thu, 23 Nov 2023 at 08:25, Chen-Yu Tsai <[email protected]> wrote:
> >
> > Hi Bret,
> >
> > On Thu, Nov 16, 2023 at 12:52 AM Bret Joseph <[email protected]> wrote:
> > >
> > > In mt6358-regulator.c
> > >
> > > static const struct regulator_ops mt6358_volt_table_ops = {
> > > .list_voltage = regulator_list_voltage_pickable_linear_range,
> > > .map_voltage = regulator_map_voltage_pickable_linear_range,
> > > .set_voltage_sel = regulator_set_voltage_sel_pickable_regmap,
> > > .get_voltage_sel = mt6358_get_buck_voltage_sel /*regulator_get_voltage_sel_pickable_regmap*/,
> > > .set_voltage_time_sel = regulator_set_voltage_time_sel,
> > > .enable = regulator_enable_regmap,
> > > .disable = regulator_disable_regmap,
> > > .is_enabled = regulator_is_enabled_regmap,
> > > .get_status = mt6358_get_status,
> > > };
> > >
> > > the function [regulator_get_voltage_sel_pickable_regmap] causes a -EINVAL‬ when registering
> > >
> > > vsim1 vusb vcamd vefuse vmch vcama1 vemc vmc vldo28 vsim2
> > >
> > > using [mt6358_get_buck_voltage_sel] results in a successful probe
> >
> > Sure it will probe, but any values you read back will be bogus.
> >
> > Can you provide a dump of /sys/kernel/debug/regulator/regulator_summary
> > and `grep ^1e..: /sys/kernel/debug/regmap/1000d000.pwrap/registers`,
> > and also any relevant logs?
> >
> > ChenYu

2023-11-24 04:59:29

by Bret Joseph

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs

ok if thats the case it means I will have to customize things on my end.


On Fri, 24 Nov 2023 at 06:29, Chen-Yu Tsai <[email protected]> wrote:
>
> Please keep the whole CC list. Please don't top post.
>
> On Thu, Nov 23, 2023 at 7:27 PM Bret Joseph <[email protected]> wrote:
> >
> > /sys/kernel/debug/regmap/1000d000.pwrap/registers
> > 1e00: 2808
> > 1e02: 2808
> > 1e04: 2000
> > 1e06: 2000
> > 1e08: 2808
>
> According to the datasheet I have, x8xx is not a valid voltage setting.
> It wasn't valid before my patch either.
>
> This is vcama1.
>
> > 1e0a: 2808
> > 1e0c: 2000
> > 1e0e: 2000
> > 1e10: 0000 # I have deleted everything with 0000
> > 1e20: 2808
> > 1e22: 2808
> > 1e24: 2000
> > 1e26: 2000
> > 1e28: 2808
>
> x8xx is not valid here either. This is vcn33.
>
> > 1e2a: 2808
> > 1e2c: 2000
>
> x0xx is not valid here either. This is vsim1.
>
> > 1e2e: 2000
>
> x0xx for register 0x1e30 is not valid. This is vsim2.
> x0xx for register 0x1e38 is not valid. This is vemc.
> x0xx for register 0x1e3c is not valid. This is vldo28.
> x0xx for register 0x1e48 is not valid. This is vmch.
> x0xx for register 0x1e4c is not valid. This is vmc.
>
> > 1e80: 2808
> > 1e82: 2808
> > 1e84: 2000
> > 1e86: 2000
> > 1e88: 2808
> > 1e8a: 2808
> > 1e8c: 2000
> > 1e8e: 2000
>
> x0xx for register 0x1e98 is not valid. This is vefuse.
>
> > 1ea0: 2808
> > 1ea2: 2808
> > 1ea4: 2000
> > 1ea6: 2000
> > 1ea8: 2808
> > 1eaa: 2808
> > 1eac: 2000
> > 1eae: 2000
>
> x0xx for register 0x1eae is not valid. This is vcamd.
>
> So basically all the failures you see are due to (as far as the datasheet
> says) invalid values. The driver prior to my patch would error out as well.
>
> Either the register readback is faulty on your platform, or your bootloader
> has configured the system into a weird state.
>
>
> ChenYu
>
> > /sys/kernel/debug/regulator/regulator_summary
> > regulator use open bypass opmode voltage
> > current min max
> > ---------------------------------------------------------------------------------------
> > regulator-dummy 3 35 0 unknown 0mV 0mA 0mV 0mV
> > vdram1 0 0 0 normal 500mV 0mA
> > 500mV 2087mV
> > vcore 0 0 0 normal 500mV 0mA
> > 500mV 1293mV
> > vpa 0 0 0 normal 500mV
> > 0mA 500mV 3650mV
> > vproc11 0 0 0 normal 500mV 0mA
> > 500mV 1293mV
> > vproc12 0 0 0 normal 500mV 0mA
> > 500mV 1293mV
> > vgpu 0 0 0 normal 500mV 0mA
> > 500mV 1293mV
> > vs2 0 0 0 normal 500mV
> > 0mA 500mV 2087mV
> > vmodem 0 0 0 normal 500mV 0mA
> > 500mV 1293mV
> > vs1 2 5 0 normal 1000mV
> > 0mA 1000mV 2587mV
> > vio18 1 0 0 unknown 1800mV 0mA
> > 1800mV 1800mV
> > vcamio 0 0 0 unknown 1800mV 0mA
> > 1800mV 1800mV
> > vcn18 0 0 0 unknown 1800mV 0mA
> > 1800mV 1800mV
> > vefuse 0 0 0 unknown 1700mV 0mA
> > 1700mV 1900mV
> > vrf18 0 0 0 unknown 1800mV
> > 0mA 1800mV 1800mV
> > vdram2 0 0 0 unknown 600mV 0mA
> > 600mV 1800mV
> > vsim1 0 0 0 unknown 1700mV 0mA
> > 1700mV 3100mV
> > vibr 0 0 0 unknown 1200mV
> > 0mA 1200mV 3300mV
> > vrf12 0 0 0 unknown 1200mV
> > 0mA 1200mV 1200mV
> > vusb 0 0 0 unknown 3000mV
> > 0mA 3000mV 3100mV
> > vcamd 0 0 0 unknown 900mV 0mA
> > 900mV 1800mV
> > vfe28 0 0 0 unknown 2800mV
> > 0mA 2800mV 2800mV
> > vsram_proc11 0 0 0 unknown 500mV 0mA 500mV 1293mV
> > vcn28 0 0 0 unknown 2800mV 0mA
> > 2800mV 2800mV
> > vsram_others 0 0 0 unknown 500mV 0mA
> > 500mV 1293mV
> > vsram_gpu 0 0 0 unknown 500mV 0mA
> > 500mV 1293mV
> > vxo22 0 0 0 unknown 2200mV 0mA
> > 2200mV 2200mV
> > vaux18 0 0 0 unknown 1800mV 0mA
> > 1800mV 1800mV
> > vmch 0 0 0 unknown 2900mV 0mA
> > 2900mV 3300mV
> > vbif28 0 0 0 unknown 2800mV
> > 0mA 2800mV 2800mV
> > vsram_proc12 0 0 0 unknown 550mV 0mA 500mV 1293mV
> > vcama1 0 0 0 unknown 1800mV 0mA
> > 1800mV 3000mV
> > vemc 0 0 0 unknown 2900mV 0mA
> > 2900mV 3300mV
> > vio28 1 0 0 unknown 2800mV
> > 0mA 2800mV 2800mV
> > va12 0 0 0 unknown 1200mV
> > 0mA 1200mV 1200mV
> > VCN33 0 0 0 unknown 3300mV 0mA
> > 0mV 0mV
> > vcama2 0 0 0 unknown 1800mV 0mA
> > 1800mV 3000mV
> > vmc 0 0 0 unknown 1800mV
> > 0mA 1800mV 3300mV
> > vldo28 0 0 0 unknown 2800mV 0mA
> > 2800mV 3000mV
> > vaud28 0 0 0 unknown 2800mV 0mA
> > 2800mV 2800mV
> > vsim2 0 0 0 unknown 1700mV 0mA
> > 1700mV 3100mV
> >
> > I have also seen errors when consumers use regulators which I think
> > could be related to setting values on regulators
> > Here is an example using mtk-ccifreq driver
> > [ 0.668869] mtk-ccifreq cci: no pinctrl handle
> > [ 0.669128] device: 'regulator:regulator.5--platform:cci': device_add
> > [ 0.669278] devices_kset: Moving cci to end of list
> > [ 0.669311] PM: Moving platform:cci to end of list
> > [ 0.669340] mtk-ccifreq cci: Linked as a consumer to regulator.5
> > [ 0.669713] of: _opp_add_static_v2: turbo:0 rate:500000000 uv:600000
> > uvmin:600000 uvmax:600000 latency:0 level:0
> > [ 0.669969] of: _opp_add_static_v2: turbo:0 rate:560000000 uv:675000
> > uvmin:675000 uvmax:675000 latency:0 level:0
> > [ 0.670245] of: _opp_add_static_v2: turbo:0 rate:612000000 uv:693750
> > uvmin:693750 uvmax:693750 latency:0 level:0
> > [ 0.670512] of: _opp_add_static_v2: turbo:0 rate:682000000 uv:718750
> > uvmin:718750 uvmax:718750 latency:0 level:0
> > [ 0.670763] of: _opp_add_static_v2: turbo:0 rate:752000000 uv:743750
> > uvmin:743750 uvmax:743750 latency:0 level:0
> > [ 0.671031] of: _opp_add_static_v2: turbo:0 rate:822000000 uv:768750
> > uvmin:768750 uvmax:768750 latency:0 level:0
> > [ 0.671598] of: _opp_add_static_v2: turbo:0 rate:875000000 uv:781250
> > uvmin:781250 uvmax:781250 latency:0 level:0
> > [ 0.671921] of: _opp_add_static_v2: turbo:0 rate:927000000 uv:800000
> > uvmin:800000 uvmax:800000 latency:0 level:0
> > [ 0.672178] of: _opp_add_static_v2: turbo:0 rate:980000000 uv:818750
> > uvmin:818750 uvmax:818750 latency:0 level:0
> > [ 0.672434] of: _opp_add_static_v2: turbo:0 rate:1050000000 uv:843750
> > uvmin:843750 uvmax:843750 latency:0 level:0
> > [ 0.672681] of: _opp_add_static_v2: turbo:0 rate:1120000000 uv:862500
> > uvmin:862500 uvmax:862500 latency:0 level:0
> > [ 0.672945] of: _opp_add_static_v2: turbo:0 rate:1155000000 uv:887500
> > uvmin:887500 uvmax:887500 latency:0 level:0
> > [ 0.673200] of: _opp_add_static_v2: turbo:0 rate:1190000000 uv:906250
> > uvmin:906250 uvmax:906250 latency:0 level:0
> > [ 0.673465] of: _opp_add_static_v2: turbo:0 rate:1260000000 uv:950000
> > uvmin:950000 uvmax:950000 latency:0 level:0
> > [ 0.673720] of: _opp_add_static_v2: turbo:0 rate:1330000000 uv:993750
> > uvmin:993750 uvmax:993750 latency:0 level:0
> > [ 0.673988] of: _opp_add_static_v2: turbo:0 rate:1400000000 uv:1031250
> > uvmin:1031250 uvmax:1031250 latency:0 level:0
> > [ 0.674327] device: 'cci': device_add
> > [ 0.674499] PM: Adding info for No Bus:cci
> > [ 0.674643] mtk-ccifreq cci: error -EPROBE_DEFER: devfreq_add_device:
> > Unable to start governor for the device
> > [ 0.675571] device: 'cci': device_unregister
> > [ 0.675730] PM: Removing info for No Bus:cci
> > [ 0.675862] mtk-ccifreq cci: failed to add devfreq device: -517
> > [ 0.680084] mtk-ccifreq cci: Driver mtk-ccifreq requests probe deferral
> > [ 0.680148] ------------[ cut here ]------------
> > [ 0.680163] WARNING: CPU: 6 PID: 56 at _regulator_put+0x340/0x354
> > [ 0.680190] Modules linked in:
> > [ 0.680208] CPU: 6 PID: 56 Comm: kworker/u16:1 Tainted: G W
> > 6.7.0-rc1-next-20231115-g2b600507077d-dirty #34
> > [ 0.680232] Hardware name: MT6769H (DT)
> > [ 0.680246] Workqueue: events_unbound deferred_probe_work_func
> > [ 0.680268] pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > [ 0.680285] pc : _regulator_put+0x340/0x354
> > [ 0.680302] lr : regulator_put+0x58/0x8c
> > [ 0.680317] sp : ffffffc080483950
> > [ 0.680328] x29: ffffffc080483950 x28: ffffff80c0030000 x27: ffffffdd002037f0
> > [ 0.680353] x26: ffffffdd01747650 x25: ffffff80c0f5d0c0 x24: ffffffdd00180d80
> > [ 0.680376] x23: ffffffc080483a98 x22: ffffffdd01f46d50 x21: ffffff80c0f5d780
> > [ 0.680400] x20: ffffffdd01ee2bc0 x19: ffffff80c0f5d780 x18: ffffff80c1f8c660
> > [ 0.680423] x17: 000000040044ffff x16: 005000f4b5593519 x15: ffffffdd0221fdb0
> > [ 0.680446] x14: 0000000000000891 x13: 0000000000000891 x12: 0000000000000001
> > [ 0.680468] x11: 0000000000000891 x10: 000000000001cde2 x9 : 0000000000001464
> > [ 0.680491] x8 : 000000008020001f x7 : ffffff80c249e600 x6 : 000000000000018d
> > [ 0.680513] x5 : 00000000000091ec x4 : 00000000000091d3 x3 : ffffffdd01a5e070
> > [ 0.680535] x2 : 0000000000000001 x1 : ffffffdd01ee2b60 x0 : 0000000000000002
> > [ 0.680558] Call trace:
> > [ 0.680568] _regulator_put+0x340/0x354
> > [ 0.680583] regulator_put+0x58/0x8c
> > [ 0.680597] devm_regulator_release+0x30/0x4c
> > [ 0.680613] release_nodes+0x8c/0xdc
> > [ 0.680629] devres_release_all+0xc0/0x164
> > [ 0.680646] device_unbind_cleanup+0x34/0xe4
> > [ 0.680660] really_probe+0x200/0x75c
> > [ 0.680674] __driver_probe_device+0x100/0x2e4
> > [ 0.680688] driver_probe_device+0x6c/0x1d4
> > [ 0.680703] __device_attach_driver+0x170/0x238
> > [ 0.680717] bus_for_each_drv+0xbc/0x18c
> > [ 0.680734] __device_attach+0x124/0x2ec
> > [ 0.680748] device_initial_probe+0x30/0x4c
> > [ 0.680762] bus_probe_device+0x138/0x168
> > [ 0.680775] deferred_probe_work_func+0x134/0x1fc
> > [ 0.680790] process_scheduled_works+0x26c/0x73c
> > [ 0.680808] worker_thread+0x294/0x690
> > [ 0.680823] kthread+0x1fc/0x22c
> > [ 0.680837] ret_from_fork+0x10/0x20
> > [ 0.680853] ---[ end trace 0000000000000000 ]---
> > [ 0.680923] mtk-ccifreq cci: Dropping the link to regulator.5
> > [ 0.680941] device: 'regulator:regulator.5--platform:cci': device_unregister
> > [ 0.681041] platform cci: Added to deferred list
> >
> > On Thu, 23 Nov 2023 at 08:25, Chen-Yu Tsai <[email protected]> wrote:
> > >
> > > Hi Bret,
> > >
> > > On Thu, Nov 16, 2023 at 12:52 AM Bret Joseph <[email protected]> wrote:
> > > >
> > > > In mt6358-regulator.c
> > > >
> > > > static const struct regulator_ops mt6358_volt_table_ops = {
> > > > .list_voltage = regulator_list_voltage_pickable_linear_range,
> > > > .map_voltage = regulator_map_voltage_pickable_linear_range,
> > > > .set_voltage_sel = regulator_set_voltage_sel_pickable_regmap,
> > > > .get_voltage_sel = mt6358_get_buck_voltage_sel /*regulator_get_voltage_sel_pickable_regmap*/,
> > > > .set_voltage_time_sel = regulator_set_voltage_time_sel,
> > > > .enable = regulator_enable_regmap,
> > > > .disable = regulator_disable_regmap,
> > > > .is_enabled = regulator_is_enabled_regmap,
> > > > .get_status = mt6358_get_status,
> > > > };
> > > >
> > > > the function [regulator_get_voltage_sel_pickable_regmap] causes a -EINVAL‬ when registering
> > > >
> > > > vsim1 vusb vcamd vefuse vmch vcama1 vemc vmc vldo28 vsim2
> > > >
> > > > using [mt6358_get_buck_voltage_sel] results in a successful probe
> > >
> > > Sure it will probe, but any values you read back will be bogus.
> > >
> > > Can you provide a dump of /sys/kernel/debug/regulator/regulator_summary
> > > and `grep ^1e..: /sys/kernel/debug/regmap/1000d000.pwrap/registers`,
> > > and also any relevant logs?
> > >
> > > ChenYu

2023-12-12 05:05:35

by Bret Joseph

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs

For the LDO regulators that were not probing, I changed the pickable
regmap to just a regmap function.
In static int mt6358_get_buck_voltage_sel(struct regulator_dev *rdev){}

For LDO1
ret = (regval & info->da_vsel_mask) >> (ffs(info->da_vsel_mask) - 1);
returns
[ 3.470672] shift exponent -1 is negative
[ 3.470910] CPU: 7 PID: 63 Comm: kworker/u16:2 Not tainted 6.7.0-rc5 #3
[ 3.471278] Hardware name: MT6769H (DT)
[ 3.471521] Workqueue: events_unbound async_run_entry_fn

fixed by adding
.da_vsel_shift = 8

--- linux-6.7.0-rc5/drivers/regulator/mt6358-regulator.c 2023-12-12
05:32:19.703310500 +0200
+++ linux/drivers/regulator/mt6358-regulator.c 2023-12-12
06:25:06.855132318 +0200
@@ -27,6 +27,7 @@
u32 qi;
u32 da_vsel_reg;
u32 da_vsel_mask;
+ u32 da_vsel_shift;
u32 modeset_reg;
u32 modeset_mask;
};
@@ -108,6 +109,7 @@
}, \
.da_vsel_reg = _da_vsel_reg, \
.da_vsel_mask = _da_vsel_mask, \
+ .da_vsel_shift = 8, \
.status_reg = MT6358_LDO_##vreg##_DBG1, \
.qi = BIT(0), \
}
@@ -209,6 +211,7 @@
}, \
.da_vsel_reg = _da_vsel_reg, \
.da_vsel_mask = _da_vsel_mask, \
+ .da_vsel_shift = 8, \
.status_reg = MT6358_LDO_##vreg##_DBG1, \
.qi = BIT(0), \
}
@@ -381,7 +384,7 @@
return ret;
}

- ret = (regval & info->da_vsel_mask) >> (ffs(info->da_vsel_mask) - 1);
+ ret = (regval >> info->da_vsel_shift) & info->da_vsel_mask;

return ret;
}
@@ -478,8 +481,8 @@
static const struct regulator_ops mt6358_volt_table_ops = {
.list_voltage = regulator_list_voltage_pickable_linear_range,
.map_voltage = regulator_map_voltage_pickable_linear_range,
- .set_voltage_sel = regulator_set_voltage_sel_pickable_regmap,
- .get_voltage_sel = regulator_get_voltage_sel_pickable_regmap,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
.set_voltage_time_sel = regulator_set_voltage_time_sel,
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,