mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/3] regulator: mt6358: Remove bogus regulators and improvements
@ 2023-09-13  8:29 Chen-Yu Tsai
  2023-09-13  8:29 ` [PATCH v3 1/3] regulator: mt6358: Fail probe on unknown chip ID Chen-Yu Tsai
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Chen-Yu Tsai @ 2023-09-13  8:29 UTC (permalink / raw)
  To: Lee Jones, Matthias Brugger, AngeloGioacchino Del Regno, Mark Brown
  Cc: Chen-Yu Tsai, Zhiyong Tao, linux-kernel, linux-arm-kernel,
	linux-mediatek

Hi,

This is v3 of the remainder of the MT6358 regulator driver cleanup
and improvement series. v1 can be found here [1]; v2 is here [2].

Changes since v2:
- Merged patches dropped
- Fixed up pickable linear ranges' selector values
- Collected tags
- Patch adding missing regulator definitions squashed into patch using
  the definitions; recommended by Krzysztof on my MT6366 series.
- Remaining dts patch split out to be sent separately

Changes since v1:
- Merged patches dropped
- Added patch to move VCN33 regulator status sync after ID check
- Added patch to fix VCN33 sync fail error message
- Added patch to add missing register definitions

Various discrepancies were found while preparing to upstream MT8186
device trees, which utilize the MT6366 PMIC, that is also covered by
this driver.

Patches 1~3 should go through the regulator tree, and patch 4 through
the soc/mediatek tree.

** Note: patch 2 needs an ack from Lee for the mfd header change.

This v3 series can be seen as two parts. v1 had three parts, but one
part was fully merged, and then v2 gained another cleanup. v3 drops
the "fixing bogus regulators" part: driver changes are fully merged
and device tree change will be sent separately.

Part 1 - Robust chip ID checking (patch 1)

Angelo suggested making the driver fail to probe if an unexpected chip
ID was found. Patch 1 implements this.

Part 2 - Output voltage fine tuning support (patches 2, 3)

Many of the LDOs on these PMIC support an extra level of output voltage
fine tuning. Most default to no offset, but a couple have a non-zero
offset by default. Previously this was unaccounted for in the driver and
device tree constraints. On the outputs with non-zero offset, this ends
up becoming a discrepancy between the device tree and actual hardware.
These two patches adds support for this second level of tuning, modeled
as bunch of linear ranges. While it's unlikely we need this level of
control, it's nice to be able to read back the accurate hardware
settings.

Please have a look.

Thanks
ChenYu

[1] https://lore.kernel.org/linux-arm-kernel/20230609083009.2822259-1-wenst@chromium.org/
[2] https://lore.kernel.org/linux-mediatek/20230721082903.2038975-1-wenst@chromium.org/

Chen-Yu Tsai (3):
  regulator: mt6358: Fail probe on unknown chip ID
  regulator: mt6358: Add output voltage fine tuning to fixed regulators
  regulator: mt6358: Add output voltage fine tuning to variable LDOs

 drivers/regulator/mt6358-regulator.c | 304 ++++++++++++---------------
 include/linux/mfd/mt6358/registers.h |   6 +
 2 files changed, 144 insertions(+), 166 deletions(-)

-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 1/3] regulator: mt6358: Fail probe on unknown chip ID
  2023-09-13  8:29 [PATCH v3 0/3] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
@ 2023-09-13  8:29 ` Chen-Yu Tsai
  2023-09-13  8:29 ` [PATCH v3 2/3] regulator: mt6358: Add output voltage fine tuning to fixed regulators Chen-Yu Tsai
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Chen-Yu Tsai @ 2023-09-13  8:29 UTC (permalink / raw)
  To: Lee Jones, Matthias Brugger, AngeloGioacchino Del Regno, Mark Brown
  Cc: Chen-Yu Tsai, Zhiyong Tao, linux-kernel, linux-arm-kernel,
	linux-mediatek

The MT6358 and MT6366 PMICs, and likely many others from MediaTek, have
a chip ID register, making the chip semi-discoverable.

The driver currently supports two PMICs and expects to be probed on one
or the other. It does not account for incorrect mfd driver entries or
device trees. While these should not happen, if they do, it could be
catastrophic for the device. The driver should be sure the hardware is
what it expects.

Make the driver fail to probe if the chip ID presented is not a known
one.

Suggested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Fixes: f0e3c6261af1 ("regulator: mt6366: Add support for MT6366 regulator")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/regulator/mt6358-regulator.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
index 120c60d40ed4..935c7ad56c7e 100644
--- a/drivers/regulator/mt6358-regulator.c
+++ b/drivers/regulator/mt6358-regulator.c
@@ -682,12 +682,18 @@ static int mt6358_regulator_probe(struct platform_device *pdev)
 	const struct mt6358_regulator_info *mt6358_info;
 	int i, max_regulator, ret;
 
-	if (mt6397->chip_id == MT6366_CHIP_ID) {
-		max_regulator = MT6366_MAX_REGULATOR;
-		mt6358_info = mt6366_regulators;
-	} else {
+	switch (mt6397->chip_id) {
+	case MT6358_CHIP_ID:
 		max_regulator = MT6358_MAX_REGULATOR;
 		mt6358_info = mt6358_regulators;
+		break;
+	case MT6366_CHIP_ID:
+		max_regulator = MT6366_MAX_REGULATOR;
+		mt6358_info = mt6366_regulators;
+		break;
+	default:
+		dev_err(&pdev->dev, "unsupported chip ID: %d\n", mt6397->chip_id);
+		return -EINVAL;
 	}
 
 	ret = mt6358_sync_vcn33_setting(&pdev->dev);
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 2/3] regulator: mt6358: Add output voltage fine tuning to fixed regulators
  2023-09-13  8:29 [PATCH v3 0/3] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
  2023-09-13  8:29 ` [PATCH v3 1/3] regulator: mt6358: Fail probe on unknown chip ID Chen-Yu Tsai
@ 2023-09-13  8:29 ` Chen-Yu Tsai
  2023-09-21 10:30   ` Lee Jones
  2023-09-13  8:29 ` [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs Chen-Yu Tsai
  2023-09-26 12:59 ` [PATCH v3 0/3] regulator: mt6358: Remove bogus regulators and improvements Mark Brown
  3 siblings, 1 reply; 13+ messages in thread
From: Chen-Yu Tsai @ 2023-09-13  8:29 UTC (permalink / raw)
  To: Lee Jones, Matthias Brugger, AngeloGioacchino Del Regno, Mark Brown
  Cc: Chen-Yu Tsai, Zhiyong Tao, linux-kernel, linux-arm-kernel,
	linux-mediatek

The "fixed" LDO regulators found on the MT6358 and MT6366 PMICs have
either no voltage selection register, or only one valid setting.
However these do have a fine voltage calibration setting that can
slightly boost the output voltage from 0 mV to 100 mV, in 10 mV
increments.

Add support for this by changing these into linear range regulators.
Some register definitions that are missing are also added.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
This patch needs an ack from Lee.

 drivers/regulator/mt6358-regulator.c | 15 +++++++++++++--
 include/linux/mfd/mt6358/registers.h |  6 ++++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
index 935c7ad56c7e..ecb2cd1eb34f 100644
--- a/drivers/regulator/mt6358-regulator.c
+++ b/drivers/regulator/mt6358-regulator.c
@@ -123,10 +123,13 @@ struct mt6358_regulator_info {
 		.type = REGULATOR_VOLTAGE,	\
 		.id = MT6358_ID_##vreg,	\
 		.owner = THIS_MODULE,	\
-		.n_voltages = 1,	\
+		.n_voltages = 11,	\
+		.vsel_reg = MT6358_##vreg##_ANA_CON0,	\
+		.vsel_mask = GENMASK(3, 0),	\
 		.enable_reg = enreg,	\
 		.enable_mask = BIT(enbit),	\
 		.min_uV = volt,	\
+		.uV_step = 10000, \
 	},	\
 	.status_reg = MT6358_LDO_##vreg##_CON1,	\
 	.qi = BIT(15),							\
@@ -219,10 +222,13 @@ struct mt6358_regulator_info {
 		.type = REGULATOR_VOLTAGE,	\
 		.id = MT6366_ID_##vreg,	\
 		.owner = THIS_MODULE,	\
-		.n_voltages = 1,	\
+		.n_voltages = 11,	\
+		.vsel_reg = MT6358_##vreg##_ANA_CON0,	\
+		.vsel_mask = GENMASK(3, 0),	\
 		.enable_reg = enreg,	\
 		.enable_mask = BIT(enbit),	\
 		.min_uV = volt,	\
+		.uV_step = 10000, \
 	},	\
 	.status_reg = MT6358_LDO_##vreg##_CON1,	\
 	.qi = BIT(15),							\
@@ -482,8 +488,13 @@ static const struct regulator_ops mt6358_volt_table_ops = {
 	.get_status = mt6358_get_status,
 };
 
+/* "Fixed" LDOs with output voltage calibration +0 ~ +10 mV */
 static const struct regulator_ops mt6358_volt_fixed_ops = {
 	.list_voltage = regulator_list_voltage_linear,
+	.map_voltage = regulator_map_voltage_linear,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = mt6358_get_buck_voltage_sel,
+	.set_voltage_time_sel = regulator_set_voltage_time_sel,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
 	.is_enabled = regulator_is_enabled_regmap,
diff --git a/include/linux/mfd/mt6358/registers.h b/include/linux/mfd/mt6358/registers.h
index 3d33517f178c..5ea2590be710 100644
--- a/include/linux/mfd/mt6358/registers.h
+++ b/include/linux/mfd/mt6358/registers.h
@@ -262,6 +262,12 @@
 #define MT6358_LDO_VBIF28_CON3                0x1db0
 #define MT6358_VCAMA1_ANA_CON0                0x1e08
 #define MT6358_VCAMA2_ANA_CON0                0x1e0c
+#define MT6358_VFE28_ANA_CON0                 0x1e10
+#define MT6358_VCN28_ANA_CON0                 0x1e14
+#define MT6358_VBIF28_ANA_CON0                0x1e18
+#define MT6358_VAUD28_ANA_CON0                0x1e1c
+#define MT6358_VAUX18_ANA_CON0                0x1e20
+#define MT6358_VXO22_ANA_CON0                 0x1e24
 #define MT6358_VCN33_ANA_CON0                 0x1e28
 #define MT6358_VSIM1_ANA_CON0                 0x1e2c
 #define MT6358_VSIM2_ANA_CON0                 0x1e30
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs
  2023-09-13  8:29 [PATCH v3 0/3] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
  2023-09-13  8:29 ` [PATCH v3 1/3] regulator: mt6358: Fail probe on unknown chip ID Chen-Yu Tsai
  2023-09-13  8:29 ` [PATCH v3 2/3] regulator: mt6358: Add output voltage fine tuning to fixed regulators Chen-Yu Tsai
@ 2023-09-13  8:29 ` Chen-Yu Tsai
  2023-09-21  9:13   ` AngeloGioacchino Del Regno
  2023-09-26 12:59 ` [PATCH v3 0/3] regulator: mt6358: Remove bogus regulators and improvements Mark Brown
  3 siblings, 1 reply; 13+ messages in thread
From: Chen-Yu Tsai @ 2023-09-13  8:29 UTC (permalink / raw)
  To: Lee Jones, Matthias Brugger, AngeloGioacchino Del Regno, Mark Brown
  Cc: Chen-Yu Tsai, Zhiyong Tao, linux-kernel, linux-arm-kernel,
	linux-mediatek

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 <wenst@chromium.org>
---
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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs
  2023-09-13  8:29 ` [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs Chen-Yu Tsai
@ 2023-09-21  9:13   ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 13+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-09-21  9:13 UTC (permalink / raw)
  To: Chen-Yu Tsai, Lee Jones, Matthias Brugger, Mark Brown
  Cc: Zhiyong Tao, linux-kernel, linux-arm-kernel, linux-mediatek

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 <wenst@chromium.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/3] regulator: mt6358: Add output voltage fine tuning to fixed regulators
  2023-09-13  8:29 ` [PATCH v3 2/3] regulator: mt6358: Add output voltage fine tuning to fixed regulators Chen-Yu Tsai
@ 2023-09-21 10:30   ` Lee Jones
  0 siblings, 0 replies; 13+ messages in thread
From: Lee Jones @ 2023-09-21 10:30 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Mark Brown,
	Zhiyong Tao, linux-kernel, linux-arm-kernel, linux-mediatek

On Wed, 13 Sep 2023, Chen-Yu Tsai wrote:

> The "fixed" LDO regulators found on the MT6358 and MT6366 PMICs have
> either no voltage selection register, or only one valid setting.
> However these do have a fine voltage calibration setting that can
> slightly boost the output voltage from 0 mV to 100 mV, in 10 mV
> increments.
> 
> Add support for this by changing these into linear range regulators.
> Some register definitions that are missing are also added.
> 
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
> This patch needs an ack from Lee.
> 
>  drivers/regulator/mt6358-regulator.c | 15 +++++++++++++--

>  include/linux/mfd/mt6358/registers.h |  6 ++++++

Acked-by: Lee Jones <lee@kernel.org>

>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
> index 935c7ad56c7e..ecb2cd1eb34f 100644
> --- a/drivers/regulator/mt6358-regulator.c
> +++ b/drivers/regulator/mt6358-regulator.c
> @@ -123,10 +123,13 @@ struct mt6358_regulator_info {
>  		.type = REGULATOR_VOLTAGE,	\
>  		.id = MT6358_ID_##vreg,	\
>  		.owner = THIS_MODULE,	\
> -		.n_voltages = 1,	\
> +		.n_voltages = 11,	\
> +		.vsel_reg = MT6358_##vreg##_ANA_CON0,	\
> +		.vsel_mask = GENMASK(3, 0),	\
>  		.enable_reg = enreg,	\
>  		.enable_mask = BIT(enbit),	\
>  		.min_uV = volt,	\
> +		.uV_step = 10000, \
>  	},	\
>  	.status_reg = MT6358_LDO_##vreg##_CON1,	\
>  	.qi = BIT(15),							\
> @@ -219,10 +222,13 @@ struct mt6358_regulator_info {
>  		.type = REGULATOR_VOLTAGE,	\
>  		.id = MT6366_ID_##vreg,	\
>  		.owner = THIS_MODULE,	\
> -		.n_voltages = 1,	\
> +		.n_voltages = 11,	\
> +		.vsel_reg = MT6358_##vreg##_ANA_CON0,	\
> +		.vsel_mask = GENMASK(3, 0),	\
>  		.enable_reg = enreg,	\
>  		.enable_mask = BIT(enbit),	\
>  		.min_uV = volt,	\
> +		.uV_step = 10000, \
>  	},	\
>  	.status_reg = MT6358_LDO_##vreg##_CON1,	\
>  	.qi = BIT(15),							\
> @@ -482,8 +488,13 @@ static const struct regulator_ops mt6358_volt_table_ops = {
>  	.get_status = mt6358_get_status,
>  };
>  
> +/* "Fixed" LDOs with output voltage calibration +0 ~ +10 mV */
>  static const struct regulator_ops mt6358_volt_fixed_ops = {
>  	.list_voltage = regulator_list_voltage_linear,
> +	.map_voltage = regulator_map_voltage_linear,
> +	.set_voltage_sel = regulator_set_voltage_sel_regmap,
> +	.get_voltage_sel = mt6358_get_buck_voltage_sel,
> +	.set_voltage_time_sel = regulator_set_voltage_time_sel,
>  	.enable = regulator_enable_regmap,
>  	.disable = regulator_disable_regmap,
>  	.is_enabled = regulator_is_enabled_regmap,
> diff --git a/include/linux/mfd/mt6358/registers.h b/include/linux/mfd/mt6358/registers.h
> index 3d33517f178c..5ea2590be710 100644
> --- a/include/linux/mfd/mt6358/registers.h
> +++ b/include/linux/mfd/mt6358/registers.h
> @@ -262,6 +262,12 @@
>  #define MT6358_LDO_VBIF28_CON3                0x1db0
>  #define MT6358_VCAMA1_ANA_CON0                0x1e08
>  #define MT6358_VCAMA2_ANA_CON0                0x1e0c
> +#define MT6358_VFE28_ANA_CON0                 0x1e10
> +#define MT6358_VCN28_ANA_CON0                 0x1e14
> +#define MT6358_VBIF28_ANA_CON0                0x1e18
> +#define MT6358_VAUD28_ANA_CON0                0x1e1c
> +#define MT6358_VAUX18_ANA_CON0                0x1e20
> +#define MT6358_VXO22_ANA_CON0                 0x1e24
>  #define MT6358_VCN33_ANA_CON0                 0x1e28
>  #define MT6358_VSIM1_ANA_CON0                 0x1e2c
>  #define MT6358_VSIM2_ANA_CON0                 0x1e30
> -- 
> 2.42.0.283.g2d96d420d3-goog
> 

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/3] regulator: mt6358: Remove bogus regulators and improvements
  2023-09-13  8:29 [PATCH v3 0/3] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
                   ` (2 preceding siblings ...)
  2023-09-13  8:29 ` [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs Chen-Yu Tsai
@ 2023-09-26 12:59 ` Mark Brown
  3 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2023-09-26 12:59 UTC (permalink / raw)
  To: Lee Jones, Matthias Brugger, AngeloGioacchino Del Regno, Chen-Yu Tsai
  Cc: Zhiyong Tao, linux-kernel, linux-arm-kernel, linux-mediatek

On Wed, 13 Sep 2023 16:29:15 +0800, Chen-Yu Tsai wrote:
> This is v3 of the remainder of the MT6358 regulator driver cleanup
> and improvement series. v1 can be found here [1]; v2 is here [2].
> 
> Changes since v2:
> - Merged patches dropped
> - Fixed up pickable linear ranges' selector values
> - Collected tags
> - Patch adding missing regulator definitions squashed into patch using
>   the definitions; recommended by Krzysztof on my MT6366 series.
> - Remaining dts patch split out to be sent separately
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/3] regulator: mt6358: Fail probe on unknown chip ID
      commit: 7442edec72bc657e6ce38ae01de9f10e55decfaa
[2/3] regulator: mt6358: Add output voltage fine tuning to fixed regulators
      commit: cf08fa74c716cf20e5038d1e7dbbd7dba1b76062
[3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs
      commit: 017c6658fd59740f9845ca0d3369ddd778e3e0c0

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs
  2023-11-24  4:58       ` Bret Joseph
@ 2023-12-12  5:04         ` Bret Joseph
  0 siblings, 0 replies; 13+ messages in thread
From: Bret Joseph @ 2023-12-12  5:04 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: AngeloGioacchino Del Regno, Mark Brown, Matthias Brugger, LKML,
	Zhiyong Tao,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	moderated list:ARM/Mediatek SoC support

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,

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs
  2023-11-24  4:29     ` Chen-Yu Tsai
@ 2023-11-24  4:58       ` Bret Joseph
  2023-12-12  5:04         ` Bret Joseph
  0 siblings, 1 reply; 13+ messages in thread
From: Bret Joseph @ 2023-11-24  4:58 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: AngeloGioacchino Del Regno, Mark Brown, Matthias Brugger, LKML,
	Zhiyong Tao,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	moderated list:ARM/Mediatek SoC support

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 <wenst@chromium.org> wrote:
>
> Please keep the whole CC list. Please don't top post.
>
> On Thu, Nov 23, 2023 at 7:27 PM Bret Joseph <bretjoseph@gmail.com> 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 <wenst@chromium.org> wrote:
> > >
> > > Hi Bret,
> > >
> > > On Thu, Nov 16, 2023 at 12:52 AM Bret Joseph <bretjoseph@gmail.com> 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs
       [not found]   ` <CAKpemQJDf9RqeOStB4Rm5hzuQdzYF6JEbuz00Cr-a80cWZKR6w@mail.gmail.com>
@ 2023-11-24  4:29     ` Chen-Yu Tsai
  2023-11-24  4:58       ` Bret Joseph
  0 siblings, 1 reply; 13+ messages in thread
From: Chen-Yu Tsai @ 2023-11-24  4:29 UTC (permalink / raw)
  To: Bret Joseph
  Cc: AngeloGioacchino Del Regno, Mark Brown, Matthias Brugger, LKML,
	Zhiyong Tao,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	moderated list:ARM/Mediatek SoC support

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

On Thu, Nov 23, 2023 at 7:27 PM Bret Joseph <bretjoseph@gmail.com> 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 <wenst@chromium.org> wrote:
> >
> > Hi Bret,
> >
> > On Thu, Nov 16, 2023 at 12:52 AM Bret Joseph <bretjoseph@gmail.com> 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs
       [not found] <CAKpemQKo2gigO44amj=eFUXgUnUeSmiOoTbS8GdXMx3O07N1vg@mail.gmail.com>
  2023-11-15 20:15 ` [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs Mark Brown
@ 2023-11-23  6:25 ` Chen-Yu Tsai
       [not found]   ` <CAKpemQJDf9RqeOStB4Rm5hzuQdzYF6JEbuz00Cr-a80cWZKR6w@mail.gmail.com>
  1 sibling, 1 reply; 13+ messages in thread
From: Chen-Yu Tsai @ 2023-11-23  6:25 UTC (permalink / raw)
  To: Bret Joseph
  Cc: angelogioacchino.delregno, broonie, matthias.bgg, linux-kernel,
	zhiyong.tao, linux-arm-kernel, linux-mediatek

Hi Bret,

On Thu, Nov 16, 2023 at 12:52 AM Bret Joseph <bretjoseph@gmail.com> 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs
  2023-11-15 20:15 ` [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs Mark Brown
@ 2023-11-16  3:50   ` Chen-Yu Tsai
  0 siblings, 0 replies; 13+ messages in thread
From: Chen-Yu Tsai @ 2023-11-16  3:50 UTC (permalink / raw)
  To: Mark Brown
  Cc: Bret Joseph, angelogioacchino.delregno, matthias.bgg,
	linux-kernel, zhiyong.tao, linux-arm-kernel, linux-mediatek

On Thu, Nov 16, 2023 at 4:15 AM Mark Brown <broonie@kernel.org> 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs
       [not found] <CAKpemQKo2gigO44amj=eFUXgUnUeSmiOoTbS8GdXMx3O07N1vg@mail.gmail.com>
@ 2023-11-15 20:15 ` Mark Brown
  2023-11-16  3:50   ` Chen-Yu Tsai
  2023-11-23  6:25 ` Chen-Yu Tsai
  1 sibling, 1 reply; 13+ messages in thread
From: Mark Brown @ 2023-11-15 20:15 UTC (permalink / raw)
  To: Bret Joseph
  Cc: wenst, angelogioacchino.delregno, matthias.bgg, linux-kernel,
	zhiyong.tao, linux-arm-kernel, linux-mediatek

[-- Attachment #1: Type: text/plain, Size: 173 bytes --]

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?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-12-12  5:05 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-13  8:29 [PATCH v3 0/3] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
2023-09-13  8:29 ` [PATCH v3 1/3] regulator: mt6358: Fail probe on unknown chip ID Chen-Yu Tsai
2023-09-13  8:29 ` [PATCH v3 2/3] regulator: mt6358: Add output voltage fine tuning to fixed regulators Chen-Yu Tsai
2023-09-21 10:30   ` Lee Jones
2023-09-13  8:29 ` [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs Chen-Yu Tsai
2023-09-21  9:13   ` AngeloGioacchino Del Regno
2023-09-26 12:59 ` [PATCH v3 0/3] regulator: mt6358: Remove bogus regulators and improvements Mark Brown
     [not found] <CAKpemQKo2gigO44amj=eFUXgUnUeSmiOoTbS8GdXMx3O07N1vg@mail.gmail.com>
2023-11-15 20:15 ` [PATCH v3 3/3] regulator: mt6358: Add output voltage fine tuning to variable LDOs Mark Brown
2023-11-16  3:50   ` Chen-Yu Tsai
2023-11-23  6:25 ` Chen-Yu Tsai
     [not found]   ` <CAKpemQJDf9RqeOStB4Rm5hzuQdzYF6JEbuz00Cr-a80cWZKR6w@mail.gmail.com>
2023-11-24  4:29     ` Chen-Yu Tsai
2023-11-24  4:58       ` Bret Joseph
2023-12-12  5:04         ` Bret Joseph

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®