* [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements
@ 2023-06-09 8:29 Chen-Yu Tsai
2023-06-09 8:29 ` [PATCH 1/9] regulator: dt-bindings: mt6358: Merge ldo_vcn33_* regulators Chen-Yu Tsai
` (9 more replies)
0 siblings, 10 replies; 32+ messages in thread
From: Chen-Yu Tsai @ 2023-06-09 8:29 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel
Hi,
This series is a cleanup and improvement of the MT6358 regulator driver.
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~8 should go through the regulator tree, and patch 9 through
the soc tree. This series (patches 7 and 8) depends on "regulator: Use
bitfield values for range selectors" [1] I sent out earlier.
This series can be seen as three parts:
Part 1 - Fixing bogus regulators (patches 1~4 and 9)
There are some regulators listed in the bindings and driver that have no
corresponding pin on the actual hardware. MediaTek says these are a
hardware construct for shared control of the same regulator in the
VCN33 case and an alternative control scheme for low power suspend.
In the VCN33 case, there's only one actual regulator, so we merge the
two and rename them to match the hardware pin. No existing devices use
these AFAICT, so this should be safe to change.
In the *_SSHUB case, the two extra regulators refer to alternative
configuration registers of the same regulators. They are intended for
the SoC's low power mode companion processor to use, not the main
processor or OS. It should be left to the implementation to choose
which set of registers to actually control.
Part 2 - Code cleanup (patches 5 and 6)
Various tables in the regulator driver were not constant, even though
they are just lookup tables. With some reworking of the code, they are
made constant.
Also, some regulators that have a single linear range were using linear
range helpers. This is more complicated than just declaring the range
and step directly in the description. This is simplified to use the
latter approach.
Part 3 - Output voltage fine tuning support (patches 7 and 8)
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. After this series is done I'll send out patches for
the MT6366 PMIC, which is what started this. That will also include
updated YAML bindings for MT6366. I think we can merge MT6358 bindings
into them afterwards.
Thanks
ChenYu
[1] https://lore.kernel.org/linux-arm-kernel/20230609075032.2804554-1-wenst@chromium.org/
Chen-Yu Tsai (9):
regulator: dt-bindings: mt6358: Merge ldo_vcn33_* regulators
regulator: dt-bindings: mt6358: Drop *_sshub regulators
regulator: mt6358: Merge VCN33_* regulators
regulator: mt6358: Drop *_SSHUB regulators
regulator: mt6358: Const-ify mt6358_regulator_info data structures
regulator: mt6358: Use linear voltage helpers for single range
regulators
regulator: mt6358: Add output voltage fine tuning to fixed regulators
regulator: mt6358: Add output voltage fine tuning to variable LDOs
arm64: dts: mediatek: mt6358: Merge ldo_vcn33_* regulators
.../bindings/regulator/mt6358-regulator.txt | 34 +-
arch/arm64/boot/dts/mediatek/mt6358.dtsi | 11 +-
drivers/regulator/mt6358-regulator.c | 499 ++++++++----------
include/linux/regulator/mt6358-regulator.h | 10 +-
4 files changed, 234 insertions(+), 320 deletions(-)
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 1/9] regulator: dt-bindings: mt6358: Merge ldo_vcn33_* regulators
2023-06-09 8:29 [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
@ 2023-06-09 8:29 ` Chen-Yu Tsai
2023-06-09 15:47 ` Matthias Brugger
2023-06-09 16:02 ` Krzysztof Kozlowski
2023-06-09 8:29 ` [PATCH 2/9] regulator: dt-bindings: mt6358: Drop *_sshub regulators Chen-Yu Tsai
` (8 subsequent siblings)
9 siblings, 2 replies; 32+ messages in thread
From: Chen-Yu Tsai @ 2023-06-09 8:29 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel
The ldo_vcn33_bt and ldo_vcn33_wifi regulators are actually the same
regulator, having the same voltage setting and output pin. There are
simply two enable bits that are ORed together to enable the regulator.
Having two regulators representing the same output pin is misleading
from a design matching standpoint, and also error-prone in driver
implementations.
Merge the two as ldo_vcn33. Neither vcn33 regulators are referenced
in upstream device trees. As far as hardware designs go, none of the
Chromebooks using MT8183 w/ MT6358 use this output.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
.../bindings/regulator/mt6358-regulator.txt | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt b/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt
index 7034cdca54e0..b22b956d1cbe 100644
--- a/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt
@@ -15,8 +15,7 @@ LDO:
ldo_vcamd, ldo_vcn18, ldo_vfe28, ldo_vsram_proc11, ldo_vcn28, ldo_vsram_others,
ldo_vsram_others_sshub, ldo_vsram_gpu, ldo_vxo22, ldo_vefuse, ldo_vaux18,
ldo_vmch, ldo_vbif28, ldo_vsram_proc12, ldo_vcama1, ldo_vemc, ldo_vio28, ldo_va12,
- ldo_vrf18, ldo_vcn33_bt, ldo_vcn33_wifi, ldo_vcama2, ldo_vmc, ldo_vldo28, ldo_vaud28,
- ldo_vsim2
+ ldo_vrf18, ldo_vcn33, ldo_vcama2, ldo_vmc, ldo_vldo28, ldo_vaud28, ldo_vsim2
Example:
@@ -305,15 +304,8 @@ Example:
regulator-enable-ramp-delay = <120>;
};
- mt6358_vcn33_bt_reg: ldo_vcn33_bt {
- regulator-name = "vcn33_bt";
- regulator-min-microvolt = <3300000>;
- regulator-max-microvolt = <3500000>;
- regulator-enable-ramp-delay = <270>;
- };
-
- mt6358_vcn33_wifi_reg: ldo_vcn33_wifi {
- regulator-name = "vcn33_wifi";
+ mt6358_vcn33_reg: ldo_vcn33 {
+ regulator-name = "vcn33";
regulator-min-microvolt = <3300000>;
regulator-max-microvolt = <3500000>;
regulator-enable-ramp-delay = <270>;
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 2/9] regulator: dt-bindings: mt6358: Drop *_sshub regulators
2023-06-09 8:29 [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
2023-06-09 8:29 ` [PATCH 1/9] regulator: dt-bindings: mt6358: Merge ldo_vcn33_* regulators Chen-Yu Tsai
@ 2023-06-09 8:29 ` Chen-Yu Tsai
2023-06-09 15:47 ` Matthias Brugger
2023-06-09 16:02 ` Krzysztof Kozlowski
2023-06-09 8:30 ` [PATCH 3/9] regulator: mt6358: Merge VCN33_* regulators Chen-Yu Tsai
` (7 subsequent siblings)
9 siblings, 2 replies; 32+ messages in thread
From: Chen-Yu Tsai @ 2023-06-09 8:29 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel
The *_sshub regulators are actually alternate configuration interfaces
for their non *_sshub counterparts. They are not separate regulator
outputs. These registers are intended for the companion processor to
use to configure the power rails while the main processor is sleeping.
They are not intended for the main operating system to use.
Since they are not real outputs they shouldn't be modeled separately.
Remove them. Luckily no device tree actually uses them.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
.../bindings/regulator/mt6358-regulator.txt | 22 +++++--------------
1 file changed, 5 insertions(+), 17 deletions(-)
diff --git a/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt b/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt
index b22b956d1cbe..b6384306db5c 100644
--- a/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt
@@ -8,14 +8,14 @@ Documentation/devicetree/bindings/regulator/regulator.txt.
The valid names for regulators are::
BUCK:
- buck_vdram1, buck_vcore, buck_vcore_sshub, buck_vpa, buck_vproc11,
- buck_vproc12, buck_vgpu, buck_vs2, buck_vmodem, buck_vs1
+ buck_vdram1, buck_vcore, buck_vpa, buck_vproc11, buck_vproc12, buck_vgpu,
+ buck_vs2, buck_vmodem, buck_vs1
LDO:
ldo_vdram2, ldo_vsim1, ldo_vibr, ldo_vrf12, ldo_vio18, ldo_vusb, ldo_vcamio,
ldo_vcamd, ldo_vcn18, ldo_vfe28, ldo_vsram_proc11, ldo_vcn28, ldo_vsram_others,
- ldo_vsram_others_sshub, ldo_vsram_gpu, ldo_vxo22, ldo_vefuse, ldo_vaux18,
- ldo_vmch, ldo_vbif28, ldo_vsram_proc12, ldo_vcama1, ldo_vemc, ldo_vio28, ldo_va12,
- ldo_vrf18, ldo_vcn33, ldo_vcama2, ldo_vmc, ldo_vldo28, ldo_vaud28, ldo_vsim2
+ ldo_vsram_gpu, ldo_vxo22, ldo_vefuse, ldo_vaux18, ldo_vmch, ldo_vbif28,
+ ldo_vsram_proc12, ldo_vcama1, ldo_vemc, ldo_vio28, ldo_va12, ldo_vrf18,
+ ldo_vcn33, ldo_vcama2, ldo_vmc, ldo_vldo28, ldo_vaud28, ldo_vsim2
Example:
@@ -346,17 +346,5 @@ Example:
regulator-max-microvolt = <3100000>;
regulator-enable-ramp-delay = <540>;
};
-
- mt6358_vcore_sshub_reg: buck_vcore_sshub {
- regulator-name = "vcore_sshub";
- regulator-min-microvolt = <500000>;
- regulator-max-microvolt = <1293750>;
- };
-
- mt6358_vsram_others_sshub_reg: ldo_vsram_others_sshub {
- regulator-name = "vsram_others_sshub";
- regulator-min-microvolt = <500000>;
- regulator-max-microvolt = <1293750>;
- };
};
};
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 3/9] regulator: mt6358: Merge VCN33_* regulators
2023-06-09 8:29 [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
2023-06-09 8:29 ` [PATCH 1/9] regulator: dt-bindings: mt6358: Merge ldo_vcn33_* regulators Chen-Yu Tsai
2023-06-09 8:29 ` [PATCH 2/9] regulator: dt-bindings: mt6358: Drop *_sshub regulators Chen-Yu Tsai
@ 2023-06-09 8:30 ` Chen-Yu Tsai
2023-06-09 8:58 ` AngeloGioacchino Del Regno
` (2 more replies)
2023-06-09 8:30 ` [PATCH 4/9] regulator: mt6358: Drop *_SSHUB regulators Chen-Yu Tsai
` (6 subsequent siblings)
9 siblings, 3 replies; 32+ messages in thread
From: Chen-Yu Tsai @ 2023-06-09 8:30 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel
The VCN33_BT and VCN33_WIFI regulators are actually the same regulator,
having the same voltage setting and output pin. There are simply two
enable bits that are ORed together to enable the regulator.
Having two regulators representing the same output pin is misleading
from a design matching standpoint, and also error-prone in driver
implementations. If consumers try to set different voltages on either
regulator, the one set later would override the one set before. There
are ways around this, such as chaining them together and having the
downstream one act as a switch. But given there's only one output pin,
such a workaround doesn't match reality.
Remove the VCN33_WIFI regulator. During the probe phase, have the driver
sync the enable status of VCN33_WIFI to VCN33_BT. Also drop the suffix
so that the regulator name matches the pin name in the datasheet.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
drivers/regulator/mt6358-regulator.c | 65 +++++++++++++++++-----
include/linux/regulator/mt6358-regulator.h | 6 +-
2 files changed, 52 insertions(+), 19 deletions(-)
diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
index c9e16bd092f6..faf6b0757019 100644
--- a/drivers/regulator/mt6358-regulator.c
+++ b/drivers/regulator/mt6358-regulator.c
@@ -277,7 +277,7 @@ static const unsigned int vcama_voltages[] = {
2800000, 2900000, 3000000,
};
-static const unsigned int vcn33_bt_wifi_voltages[] = {
+static const unsigned int vcn33_voltages[] = {
3300000, 3400000, 3500000,
};
@@ -321,7 +321,7 @@ static const u32 vcama_idx[] = {
0, 7, 9, 10, 11, 12,
};
-static const u32 vcn33_bt_wifi_idx[] = {
+static const u32 vcn33_idx[] = {
1, 2, 3,
};
@@ -566,12 +566,8 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
MT6358_LDO_VCAMA1_CON0, 0, MT6358_VCAMA1_ANA_CON0, 0xf00),
MT6358_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx,
MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
- MT6358_LDO("ldo_vcn33_bt", VCN33_BT, vcn33_bt_wifi_voltages,
- vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_0,
- 0, MT6358_VCN33_ANA_CON0, 0x300),
- MT6358_LDO("ldo_vcn33_wifi", VCN33_WIFI, vcn33_bt_wifi_voltages,
- vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_1,
- 0, MT6358_VCN33_ANA_CON0, 0x300),
+ MT6358_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx,
+ MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
MT6358_LDO("ldo_vcama2", VCAMA2, vcama_voltages, vcama_idx,
MT6358_LDO_VCAMA2_CON0, 0, MT6358_VCAMA2_ANA_CON0, 0xf00),
MT6358_LDO("ldo_vmc", VMC, vmc_voltages, vmc_idx,
@@ -662,12 +658,8 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
MT6358_LDO_VMCH_CON0, 0, MT6358_VMCH_ANA_CON0, 0x700),
MT6366_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx,
MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
- MT6366_LDO("ldo_vcn33_bt", VCN33_BT, vcn33_bt_wifi_voltages,
- vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_0,
- 0, MT6358_VCN33_ANA_CON0, 0x300),
- MT6366_LDO("ldo_vcn33_wifi", VCN33_WIFI, vcn33_bt_wifi_voltages,
- vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_1,
- 0, MT6358_VCN33_ANA_CON0, 0x300),
+ MT6366_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx,
+ MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
MT6366_LDO("ldo_vmc", VMC, vmc_voltages, vmc_idx,
MT6358_LDO_VMC_CON0, 0, MT6358_VMC_ANA_CON0, 0xf00),
MT6366_LDO("ldo_vsim2", VSIM2, vsim_voltages, vsim_idx,
@@ -690,13 +682,56 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
MT6358_LDO_VSRAM_CON1, 0x7f),
};
+static int mt6358_sync_vcn33_setting(struct device *dev)
+{
+ struct mt6397_chip *mt6397 = dev_get_drvdata(dev->parent);
+ unsigned int val;
+ int ret;
+
+ /*
+ * VCN33_WIFI and VCN33_BT are two separate enable bits for the same
+ * regulator. They share the same voltage setting and output pin.
+ * Instead of having two potentially conflicting regulators, just have
+ * one VCN33 regulator. Sync the two enable bits and only use one in
+ * the regulator device.
+ */
+ ret = regmap_read(mt6397->regmap, MT6358_LDO_VCN33_CON0_1, &val);
+ if (ret) {
+ dev_err(dev, "Failed to read VCN33_WIFI setting\n");
+ return ret;
+ }
+
+ if (!(val & BIT(0)))
+ return 0;
+
+ /* Sync VCN33_WIFI enable status to VCN33_BT */
+ ret = regmap_update_bits(mt6397->regmap, MT6358_LDO_VCN33_CON0_0, BIT(0), BIT(0));
+ if (ret) {
+ dev_err(dev, "Failed to sync VCN33_WIFI setting to VCN33_BT\n");
+ return ret;
+ }
+
+ /* Disable VCN33_WIFI */
+ ret = regmap_update_bits(mt6397->regmap, MT6358_LDO_VCN33_CON0_1, BIT(0), 0);
+ if (ret) {
+ dev_err(dev, "Failed to disable VCN33_BT\n");
+ return ret;
+ }
+
+ return 0;
+}
+
static int mt6358_regulator_probe(struct platform_device *pdev)
{
struct mt6397_chip *mt6397 = dev_get_drvdata(pdev->dev.parent);
struct regulator_config config = {};
struct regulator_dev *rdev;
struct mt6358_regulator_info *mt6358_info;
- int i, max_regulator;
+ int i, max_regulator, ret;
+
+ ret = mt6358_sync_vcn33_setting(&pdev->dev);
+ if (ret)
+ return ret;
if (mt6397->chip_id == MT6366_CHIP_ID) {
max_regulator = MT6366_MAX_REGULATOR;
diff --git a/include/linux/regulator/mt6358-regulator.h b/include/linux/regulator/mt6358-regulator.h
index bdcf83cd719e..a4307cd9edd6 100644
--- a/include/linux/regulator/mt6358-regulator.h
+++ b/include/linux/regulator/mt6358-regulator.h
@@ -41,8 +41,7 @@ enum {
MT6358_ID_VIO28,
MT6358_ID_VA12,
MT6358_ID_VRF18,
- MT6358_ID_VCN33_BT,
- MT6358_ID_VCN33_WIFI,
+ MT6358_ID_VCN33,
MT6358_ID_VCAMA2,
MT6358_ID_VMC,
MT6358_ID_VLDO28,
@@ -85,8 +84,7 @@ enum {
MT6366_ID_VIO28,
MT6366_ID_VA12,
MT6366_ID_VRF18,
- MT6366_ID_VCN33_BT,
- MT6366_ID_VCN33_WIFI,
+ MT6366_ID_VCN33,
MT6366_ID_VMC,
MT6366_ID_VAUD28,
MT6366_ID_VSIM2,
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 4/9] regulator: mt6358: Drop *_SSHUB regulators
2023-06-09 8:29 [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
` (2 preceding siblings ...)
2023-06-09 8:30 ` [PATCH 3/9] regulator: mt6358: Merge VCN33_* regulators Chen-Yu Tsai
@ 2023-06-09 8:30 ` Chen-Yu Tsai
2023-06-09 9:03 ` AngeloGioacchino Del Regno
2023-06-09 15:52 ` Matthias Brugger
2023-06-09 8:30 ` [PATCH 5/9] regulator: mt6358: Const-ify mt6358_regulator_info data structures Chen-Yu Tsai
` (5 subsequent siblings)
9 siblings, 2 replies; 32+ messages in thread
From: Chen-Yu Tsai @ 2023-06-09 8:30 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel
The *_SSHUB regulators are actually alternate configuration interfaces
for their non *_SSHUB counterparts. They are not separate regulator
outputs. These registers are intended for the companion processor to
use to configure the power rails while the main processor is sleeping.
They are not intended for the main operating system to use.
Since they are not real outputs they shouldn't be modeled separately.
Remove them. Luckily no device tree actually uses them.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
drivers/regulator/mt6358-regulator.c | 14 --------------
include/linux/regulator/mt6358-regulator.h | 4 ----
2 files changed, 18 deletions(-)
diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
index faf6b0757019..946a251a8b3a 100644
--- a/drivers/regulator/mt6358-regulator.c
+++ b/drivers/regulator/mt6358-regulator.c
@@ -505,9 +505,6 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
MT6358_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
MT6358_VCORE_VGPU_ANA_CON0, 1),
- MT6358_BUCK("buck_vcore_sshub", VCORE_SSHUB, 500000, 1293750, 6250,
- buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_SSHUB_ELR0, 0x7f,
- MT6358_VCORE_VGPU_ANA_CON0, 1),
MT6358_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
buck_volt_range3, 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f,
MT6358_VPA_ANA_CON0, 3),
@@ -583,10 +580,6 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
MT6358_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
buck_volt_range1, MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
MT6358_LDO_VSRAM_CON2, 0x7f),
- MT6358_LDO1("ldo_vsram_others_sshub", VSRAM_OTHERS_SSHUB, 500000,
- 1293750, 6250, buck_volt_range1,
- MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f,
- MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f),
MT6358_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
buck_volt_range1, MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
MT6358_LDO_VSRAM_CON3, 0x7f),
@@ -603,9 +596,6 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
MT6366_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
MT6358_VCORE_VGPU_ANA_CON0, 1),
- MT6366_BUCK("buck_vcore_sshub", VCORE_SSHUB, 500000, 1293750, 6250,
- buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_SSHUB_ELR0, 0x7f,
- MT6358_VCORE_VGPU_ANA_CON0, 1),
MT6366_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
buck_volt_range3, 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f,
MT6358_VPA_ANA_CON0, 3),
@@ -670,10 +660,6 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
MT6366_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
buck_volt_range1, MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
MT6358_LDO_VSRAM_CON2, 0x7f),
- MT6366_LDO1("ldo_vsram_others_sshub", VSRAM_OTHERS_SSHUB, 500000,
- 1293750, 6250, buck_volt_range1,
- MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f,
- MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f),
MT6366_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
buck_volt_range1, MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
MT6358_LDO_VSRAM_CON3, 0x7f),
diff --git a/include/linux/regulator/mt6358-regulator.h b/include/linux/regulator/mt6358-regulator.h
index a4307cd9edd6..c71a6a9fce7a 100644
--- a/include/linux/regulator/mt6358-regulator.h
+++ b/include/linux/regulator/mt6358-regulator.h
@@ -47,8 +47,6 @@ enum {
MT6358_ID_VLDO28,
MT6358_ID_VAUD28,
MT6358_ID_VSIM2,
- MT6358_ID_VCORE_SSHUB,
- MT6358_ID_VSRAM_OTHERS_SSHUB,
MT6358_ID_RG_MAX,
};
@@ -88,8 +86,6 @@ enum {
MT6366_ID_VMC,
MT6366_ID_VAUD28,
MT6366_ID_VSIM2,
- MT6366_ID_VCORE_SSHUB,
- MT6366_ID_VSRAM_OTHERS_SSHUB,
MT6366_ID_RG_MAX,
};
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 5/9] regulator: mt6358: Const-ify mt6358_regulator_info data structures
2023-06-09 8:29 [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
` (3 preceding siblings ...)
2023-06-09 8:30 ` [PATCH 4/9] regulator: mt6358: Drop *_SSHUB regulators Chen-Yu Tsai
@ 2023-06-09 8:30 ` Chen-Yu Tsai
2023-06-09 8:30 ` [PATCH 6/9] regulator: mt6358: Use linear voltage helpers for single range regulators Chen-Yu Tsai
` (4 subsequent siblings)
9 siblings, 0 replies; 32+ messages in thread
From: Chen-Yu Tsai @ 2023-06-09 8:30 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel
In the MT6358 regulator driver, each regulator is described by a
|struct regulator_desc| wrapped by a |struct mt6358_regulator_info|.
The latter was tied to the regulator device using the config's
driver_data field, which meant that the variables could not be constant.
Since each regulator device has a pointer to its regulator_desc, and
mt6358_regulator_info wraps that, the driver could use container_of()
to retrieve it instead.
Switch to using container_of(), drop tha driver_data setting, and
const-ify all the regulator descriptions.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
drivers/regulator/mt6358-regulator.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
index 946a251a8b3a..2851ef53ac63 100644
--- a/drivers/regulator/mt6358-regulator.c
+++ b/drivers/regulator/mt6358-regulator.c
@@ -34,6 +34,8 @@ struct mt6358_regulator_info {
u32 modeset_mask;
};
+#define to_regulator_info(x) container_of((x), struct mt6358_regulator_info, desc)
+
#define MT6358_BUCK(match, vreg, min, max, step, \
volt_ranges, vosel_mask, _da_vsel_reg, _da_vsel_mask, \
_modeset_reg, _modeset_shift) \
@@ -342,9 +344,9 @@ static unsigned int mt6358_map_mode(unsigned int mode)
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;
- struct mt6358_regulator_info *info = rdev_get_drvdata(rdev);
pvol = info->index_table;
@@ -358,9 +360,9 @@ static int mt6358_set_voltage_sel(struct regulator_dev *rdev,
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;
- struct mt6358_regulator_info *info = rdev_get_drvdata(rdev);
const u32 *pvol;
ret = regmap_read(rdev->regmap, info->desc.vsel_reg, &selector);
@@ -384,8 +386,8 @@ static int mt6358_get_voltage_sel(struct regulator_dev *rdev)
static int mt6358_get_buck_voltage_sel(struct regulator_dev *rdev)
{
+ const struct mt6358_regulator_info *info = to_regulator_info(rdev->desc);
int ret, regval;
- struct mt6358_regulator_info *info = rdev_get_drvdata(rdev);
ret = regmap_read(rdev->regmap, info->da_vsel_reg, ®val);
if (ret != 0) {
@@ -402,9 +404,9 @@ static int mt6358_get_buck_voltage_sel(struct regulator_dev *rdev)
static int mt6358_get_status(struct regulator_dev *rdev)
{
+ const struct mt6358_regulator_info *info = to_regulator_info(rdev->desc);
int ret;
u32 regval;
- struct mt6358_regulator_info *info = rdev_get_drvdata(rdev);
ret = regmap_read(rdev->regmap, info->status_reg, ®val);
if (ret != 0) {
@@ -418,7 +420,7 @@ static int mt6358_get_status(struct regulator_dev *rdev)
static int mt6358_regulator_set_mode(struct regulator_dev *rdev,
unsigned int mode)
{
- struct mt6358_regulator_info *info = rdev_get_drvdata(rdev);
+ const struct mt6358_regulator_info *info = to_regulator_info(rdev->desc);
int val;
switch (mode) {
@@ -443,7 +445,7 @@ static int mt6358_regulator_set_mode(struct regulator_dev *rdev,
static unsigned int mt6358_regulator_get_mode(struct regulator_dev *rdev)
{
- struct mt6358_regulator_info *info = rdev_get_drvdata(rdev);
+ const struct mt6358_regulator_info *info = to_regulator_info(rdev->desc);
int ret, regval;
ret = regmap_read(rdev->regmap, info->modeset_reg, ®val);
@@ -498,7 +500,7 @@ static const struct regulator_ops mt6358_volt_fixed_ops = {
};
/* The array is indexed by id(MT6358_ID_XXX) */
-static struct mt6358_regulator_info mt6358_regulators[] = {
+static const struct mt6358_regulator_info mt6358_regulators[] = {
MT6358_BUCK("buck_vdram1", VDRAM1, 500000, 2087500, 12500,
buck_volt_range2, 0x7f, MT6358_BUCK_VDRAM1_DBG0, 0x7f,
MT6358_VDRAM1_ANA_CON0, 8),
@@ -589,7 +591,7 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
};
/* The array is indexed by id(MT6366_ID_XXX) */
-static struct mt6358_regulator_info mt6366_regulators[] = {
+static const struct mt6358_regulator_info mt6366_regulators[] = {
MT6366_BUCK("buck_vdram1", VDRAM1, 500000, 2087500, 12500,
buck_volt_range2, 0x7f, MT6358_BUCK_VDRAM1_DBG0, 0x7f,
MT6358_VDRAM1_ANA_CON0, 8),
@@ -712,7 +714,7 @@ static int mt6358_regulator_probe(struct platform_device *pdev)
struct mt6397_chip *mt6397 = dev_get_drvdata(pdev->dev.parent);
struct regulator_config config = {};
struct regulator_dev *rdev;
- struct mt6358_regulator_info *mt6358_info;
+ const struct mt6358_regulator_info *mt6358_info;
int i, max_regulator, ret;
ret = mt6358_sync_vcn33_setting(&pdev->dev);
@@ -729,7 +731,6 @@ static int mt6358_regulator_probe(struct platform_device *pdev)
for (i = 0; i < max_regulator; i++) {
config.dev = &pdev->dev;
- config.driver_data = &mt6358_info[i];
config.regmap = mt6397->regmap;
rdev = devm_regulator_register(&pdev->dev,
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 6/9] regulator: mt6358: Use linear voltage helpers for single range regulators
2023-06-09 8:29 [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
` (4 preceding siblings ...)
2023-06-09 8:30 ` [PATCH 5/9] regulator: mt6358: Const-ify mt6358_regulator_info data structures Chen-Yu Tsai
@ 2023-06-09 8:30 ` Chen-Yu Tsai
2023-06-09 8:30 ` [PATCH 7/9] regulator: mt6358: Add output voltage fine tuning to fixed regulators Chen-Yu Tsai
` (3 subsequent siblings)
9 siblings, 0 replies; 32+ messages in thread
From: Chen-Yu Tsai @ 2023-06-09 8:30 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel
Some of the regulators on the MT6358/MT6366 PMICs have just one linear
voltage range. These are the bulk regulators and VSRAM_* LDOs. Currently
they are modeled with one linear range, but also have their minimum,
maximum, and step voltage described.
Convert them to the linear voltage helpers. These helpers are a bit
simpler, and we can also drop the linear range definitions. Also reflow
the touched lines now that they are shorter.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
drivers/regulator/mt6358-regulator.c | 121 +++++++++------------------
1 file changed, 40 insertions(+), 81 deletions(-)
diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
index 2851ef53ac63..31a16fb28ecd 100644
--- a/drivers/regulator/mt6358-regulator.c
+++ b/drivers/regulator/mt6358-regulator.c
@@ -37,7 +37,7 @@ struct mt6358_regulator_info {
#define to_regulator_info(x) container_of((x), struct mt6358_regulator_info, desc)
#define MT6358_BUCK(match, vreg, min, max, step, \
- volt_ranges, vosel_mask, _da_vsel_reg, _da_vsel_mask, \
+ vosel_mask, _da_vsel_reg, _da_vsel_mask, \
_modeset_reg, _modeset_shift) \
[MT6358_ID_##vreg] = { \
.desc = { \
@@ -48,8 +48,8 @@ struct mt6358_regulator_info {
.id = MT6358_ID_##vreg, \
.owner = THIS_MODULE, \
.n_voltages = ((max) - (min)) / (step) + 1, \
- .linear_ranges = volt_ranges, \
- .n_linear_ranges = ARRAY_SIZE(volt_ranges), \
+ .min_uV = (min), \
+ .uV_step = (step), \
.vsel_reg = MT6358_BUCK_##vreg##_ELR0, \
.vsel_mask = vosel_mask, \
.enable_reg = MT6358_BUCK_##vreg##_CON0, \
@@ -89,7 +89,7 @@ struct mt6358_regulator_info {
}
#define MT6358_LDO1(match, vreg, min, max, step, \
- volt_ranges, _da_vsel_reg, _da_vsel_mask, \
+ _da_vsel_reg, _da_vsel_mask, \
vosel, vosel_mask) \
[MT6358_ID_##vreg] = { \
.desc = { \
@@ -100,8 +100,8 @@ struct mt6358_regulator_info {
.id = MT6358_ID_##vreg, \
.owner = THIS_MODULE, \
.n_voltages = ((max) - (min)) / (step) + 1, \
- .linear_ranges = volt_ranges, \
- .n_linear_ranges = ARRAY_SIZE(volt_ranges), \
+ .min_uV = (min), \
+ .uV_step = (step), \
.vsel_reg = vosel, \
.vsel_mask = vosel_mask, \
.enable_reg = MT6358_LDO_##vreg##_CON0, \
@@ -133,7 +133,7 @@ struct mt6358_regulator_info {
}
#define MT6366_BUCK(match, vreg, min, max, step, \
- volt_ranges, vosel_mask, _da_vsel_reg, _da_vsel_mask, \
+ vosel_mask, _da_vsel_reg, _da_vsel_mask, \
_modeset_reg, _modeset_shift) \
[MT6366_ID_##vreg] = { \
.desc = { \
@@ -144,8 +144,8 @@ struct mt6358_regulator_info {
.id = MT6366_ID_##vreg, \
.owner = THIS_MODULE, \
.n_voltages = ((max) - (min)) / (step) + 1, \
- .linear_ranges = volt_ranges, \
- .n_linear_ranges = ARRAY_SIZE(volt_ranges), \
+ .min_uV = (min), \
+ .uV_step = (step), \
.vsel_reg = MT6358_BUCK_##vreg##_ELR0, \
.vsel_mask = vosel_mask, \
.enable_reg = MT6358_BUCK_##vreg##_CON0, \
@@ -185,7 +185,7 @@ struct mt6358_regulator_info {
}
#define MT6366_LDO1(match, vreg, min, max, step, \
- volt_ranges, _da_vsel_reg, _da_vsel_mask, \
+ _da_vsel_reg, _da_vsel_mask, \
vosel, vosel_mask) \
[MT6366_ID_##vreg] = { \
.desc = { \
@@ -196,8 +196,8 @@ struct mt6358_regulator_info {
.id = MT6366_ID_##vreg, \
.owner = THIS_MODULE, \
.n_voltages = ((max) - (min)) / (step) + 1, \
- .linear_ranges = volt_ranges, \
- .n_linear_ranges = ARRAY_SIZE(volt_ranges), \
+ .min_uV = (min), \
+ .uV_step = (step), \
.vsel_reg = vosel, \
.vsel_mask = vosel_mask, \
.enable_reg = MT6358_LDO_##vreg##_CON0, \
@@ -228,21 +228,6 @@ struct mt6358_regulator_info {
.qi = BIT(15), \
}
-static const struct linear_range buck_volt_range1[] = {
- REGULATOR_LINEAR_RANGE(500000, 0, 0x7f, 6250),
-};
-
-static const struct linear_range buck_volt_range2[] = {
- REGULATOR_LINEAR_RANGE(500000, 0, 0x7f, 12500),
-};
-
-static const struct linear_range buck_volt_range3[] = {
- REGULATOR_LINEAR_RANGE(500000, 0, 0x3f, 50000),
-};
-
-static const struct linear_range buck_volt_range4[] = {
- REGULATOR_LINEAR_RANGE(1000000, 0, 0x7f, 12500),
-};
static const unsigned int vdram2_voltages[] = {
600000, 1800000,
@@ -466,8 +451,8 @@ static unsigned int mt6358_regulator_get_mode(struct regulator_dev *rdev)
}
static const struct regulator_ops mt6358_volt_range_ops = {
- .list_voltage = regulator_list_voltage_linear_range,
- .map_voltage = regulator_map_voltage_linear_range,
+ .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,
@@ -502,32 +487,23 @@ static const struct regulator_ops mt6358_volt_fixed_ops = {
/* The array is indexed by id(MT6358_ID_XXX) */
static const struct mt6358_regulator_info mt6358_regulators[] = {
MT6358_BUCK("buck_vdram1", VDRAM1, 500000, 2087500, 12500,
- buck_volt_range2, 0x7f, MT6358_BUCK_VDRAM1_DBG0, 0x7f,
- MT6358_VDRAM1_ANA_CON0, 8),
+ 0x7f, MT6358_BUCK_VDRAM1_DBG0, 0x7f, MT6358_VDRAM1_ANA_CON0, 8),
MT6358_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
- buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
- MT6358_VCORE_VGPU_ANA_CON0, 1),
+ 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f, MT6358_VCORE_VGPU_ANA_CON0, 1),
MT6358_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
- buck_volt_range3, 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f,
- MT6358_VPA_ANA_CON0, 3),
+ 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f, MT6358_VPA_ANA_CON0, 3),
MT6358_BUCK("buck_vproc11", VPROC11, 500000, 1293750, 6250,
- buck_volt_range1, 0x7f, MT6358_BUCK_VPROC11_DBG0, 0x7f,
- MT6358_VPROC_ANA_CON0, 1),
+ 0x7f, MT6358_BUCK_VPROC11_DBG0, 0x7f, MT6358_VPROC_ANA_CON0, 1),
MT6358_BUCK("buck_vproc12", VPROC12, 500000, 1293750, 6250,
- buck_volt_range1, 0x7f, MT6358_BUCK_VPROC12_DBG0, 0x7f,
- MT6358_VPROC_ANA_CON0, 2),
+ 0x7f, MT6358_BUCK_VPROC12_DBG0, 0x7f, MT6358_VPROC_ANA_CON0, 2),
MT6358_BUCK("buck_vgpu", VGPU, 500000, 1293750, 6250,
- buck_volt_range1, 0x7f, MT6358_BUCK_VGPU_ELR0, 0x7f,
- MT6358_VCORE_VGPU_ANA_CON0, 2),
+ 0x7f, MT6358_BUCK_VGPU_ELR0, 0x7f, MT6358_VCORE_VGPU_ANA_CON0, 2),
MT6358_BUCK("buck_vs2", VS2, 500000, 2087500, 12500,
- buck_volt_range2, 0x7f, MT6358_BUCK_VS2_DBG0, 0x7f,
- MT6358_VS2_ANA_CON0, 8),
+ 0x7f, MT6358_BUCK_VS2_DBG0, 0x7f, MT6358_VS2_ANA_CON0, 8),
MT6358_BUCK("buck_vmodem", VMODEM, 500000, 1293750, 6250,
- buck_volt_range1, 0x7f, MT6358_BUCK_VMODEM_DBG0, 0x7f,
- MT6358_VMODEM_ANA_CON0, 8),
+ 0x7f, MT6358_BUCK_VMODEM_DBG0, 0x7f, MT6358_VMODEM_ANA_CON0, 8),
MT6358_BUCK("buck_vs1", VS1, 1000000, 2587500, 12500,
- buck_volt_range4, 0x7f, MT6358_BUCK_VS1_DBG0, 0x7f,
- MT6358_VS1_ANA_CON0, 8),
+ 0x7f, MT6358_BUCK_VS1_DBG0, 0x7f, MT6358_VS1_ANA_CON0, 8),
MT6358_REG_FIXED("ldo_vrf12", VRF12,
MT6358_LDO_VRF12_CON0, 0, 1200000),
MT6358_REG_FIXED("ldo_vio18", VIO18,
@@ -577,48 +553,35 @@ static const struct mt6358_regulator_info mt6358_regulators[] = {
MT6358_LDO("ldo_vsim2", VSIM2, vsim_voltages, vsim_idx,
MT6358_LDO_VSIM2_CON0, 0, MT6358_VSIM2_ANA_CON0, 0xf00),
MT6358_LDO1("ldo_vsram_proc11", VSRAM_PROC11, 500000, 1293750, 6250,
- buck_volt_range1, MT6358_LDO_VSRAM_PROC11_DBG0, 0x7f00,
- MT6358_LDO_VSRAM_CON0, 0x7f),
+ MT6358_LDO_VSRAM_PROC11_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON0, 0x7f),
MT6358_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
- buck_volt_range1, MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
- MT6358_LDO_VSRAM_CON2, 0x7f),
+ MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON2, 0x7f),
MT6358_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
- buck_volt_range1, MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
- MT6358_LDO_VSRAM_CON3, 0x7f),
+ MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON3, 0x7f),
MT6358_LDO1("ldo_vsram_proc12", VSRAM_PROC12, 500000, 1293750, 6250,
- buck_volt_range1, MT6358_LDO_VSRAM_PROC12_DBG0, 0x7f00,
- MT6358_LDO_VSRAM_CON1, 0x7f),
+ MT6358_LDO_VSRAM_PROC12_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON1, 0x7f),
};
/* The array is indexed by id(MT6366_ID_XXX) */
static const struct mt6358_regulator_info mt6366_regulators[] = {
MT6366_BUCK("buck_vdram1", VDRAM1, 500000, 2087500, 12500,
- buck_volt_range2, 0x7f, MT6358_BUCK_VDRAM1_DBG0, 0x7f,
- MT6358_VDRAM1_ANA_CON0, 8),
+ 0x7f, MT6358_BUCK_VDRAM1_DBG0, 0x7f, MT6358_VDRAM1_ANA_CON0, 8),
MT6366_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
- buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
- MT6358_VCORE_VGPU_ANA_CON0, 1),
+ 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f, MT6358_VCORE_VGPU_ANA_CON0, 1),
MT6366_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
- buck_volt_range3, 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f,
- MT6358_VPA_ANA_CON0, 3),
+ 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f, MT6358_VPA_ANA_CON0, 3),
MT6366_BUCK("buck_vproc11", VPROC11, 500000, 1293750, 6250,
- buck_volt_range1, 0x7f, MT6358_BUCK_VPROC11_DBG0, 0x7f,
- MT6358_VPROC_ANA_CON0, 1),
+ 0x7f, MT6358_BUCK_VPROC11_DBG0, 0x7f, MT6358_VPROC_ANA_CON0, 1),
MT6366_BUCK("buck_vproc12", VPROC12, 500000, 1293750, 6250,
- buck_volt_range1, 0x7f, MT6358_BUCK_VPROC12_DBG0, 0x7f,
- MT6358_VPROC_ANA_CON0, 2),
+ 0x7f, MT6358_BUCK_VPROC12_DBG0, 0x7f, MT6358_VPROC_ANA_CON0, 2),
MT6366_BUCK("buck_vgpu", VGPU, 500000, 1293750, 6250,
- buck_volt_range1, 0x7f, MT6358_BUCK_VGPU_ELR0, 0x7f,
- MT6358_VCORE_VGPU_ANA_CON0, 2),
+ 0x7f, MT6358_BUCK_VGPU_ELR0, 0x7f, MT6358_VCORE_VGPU_ANA_CON0, 2),
MT6366_BUCK("buck_vs2", VS2, 500000, 2087500, 12500,
- buck_volt_range2, 0x7f, MT6358_BUCK_VS2_DBG0, 0x7f,
- MT6358_VS2_ANA_CON0, 8),
+ 0x7f, MT6358_BUCK_VS2_DBG0, 0x7f, MT6358_VS2_ANA_CON0, 8),
MT6366_BUCK("buck_vmodem", VMODEM, 500000, 1293750, 6250,
- buck_volt_range1, 0x7f, MT6358_BUCK_VMODEM_DBG0, 0x7f,
- MT6358_VMODEM_ANA_CON0, 8),
+ 0x7f, MT6358_BUCK_VMODEM_DBG0, 0x7f, MT6358_VMODEM_ANA_CON0, 8),
MT6366_BUCK("buck_vs1", VS1, 1000000, 2587500, 12500,
- buck_volt_range4, 0x7f, MT6358_BUCK_VS1_DBG0, 0x7f,
- MT6358_VS1_ANA_CON0, 8),
+ 0x7f, MT6358_BUCK_VS1_DBG0, 0x7f, MT6358_VS1_ANA_CON0, 8),
MT6366_REG_FIXED("ldo_vrf12", VRF12,
MT6358_LDO_VRF12_CON0, 0, 1200000),
MT6366_REG_FIXED("ldo_vio18", VIO18,
@@ -657,17 +620,13 @@ static const struct mt6358_regulator_info mt6366_regulators[] = {
MT6366_LDO("ldo_vsim2", VSIM2, vsim_voltages, vsim_idx,
MT6358_LDO_VSIM2_CON0, 0, MT6358_VSIM2_ANA_CON0, 0xf00),
MT6366_LDO1("ldo_vsram_proc11", VSRAM_PROC11, 500000, 1293750, 6250,
- buck_volt_range1, MT6358_LDO_VSRAM_PROC11_DBG0, 0x7f00,
- MT6358_LDO_VSRAM_CON0, 0x7f),
+ MT6358_LDO_VSRAM_PROC11_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON0, 0x7f),
MT6366_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
- buck_volt_range1, MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
- MT6358_LDO_VSRAM_CON2, 0x7f),
+ MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON2, 0x7f),
MT6366_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
- buck_volt_range1, MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
- MT6358_LDO_VSRAM_CON3, 0x7f),
+ MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON3, 0x7f),
MT6366_LDO1("ldo_vsram_proc12", VSRAM_PROC12, 500000, 1293750, 6250,
- buck_volt_range1, MT6358_LDO_VSRAM_PROC12_DBG0, 0x7f00,
- MT6358_LDO_VSRAM_CON1, 0x7f),
+ MT6358_LDO_VSRAM_PROC12_DBG0, 0x7f00, MT6358_LDO_VSRAM_CON1, 0x7f),
};
static int mt6358_sync_vcn33_setting(struct device *dev)
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 7/9] regulator: mt6358: Add output voltage fine tuning to fixed regulators
2023-06-09 8:29 [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
` (5 preceding siblings ...)
2023-06-09 8:30 ` [PATCH 6/9] regulator: mt6358: Use linear voltage helpers for single range regulators Chen-Yu Tsai
@ 2023-06-09 8:30 ` Chen-Yu Tsai
2023-06-09 10:03 ` AngeloGioacchino Del Regno
2023-06-14 16:14 ` Mark Brown
2023-06-09 8:30 ` [PATCH 8/9] regulator: mt6358: Add output voltage fine tuning to variable LDOs Chen-Yu Tsai
` (2 subsequent siblings)
9 siblings, 2 replies; 32+ messages in thread
From: Chen-Yu Tsai @ 2023-06-09 8:30 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel
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.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
drivers/regulator/mt6358-regulator.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
index 31a16fb28ecd..26060909cf90 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), \
@@ -476,8 +482,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,
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 8/9] regulator: mt6358: Add output voltage fine tuning to variable LDOs
2023-06-09 8:29 [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
` (6 preceding siblings ...)
2023-06-09 8:30 ` [PATCH 7/9] regulator: mt6358: Add output voltage fine tuning to fixed regulators Chen-Yu Tsai
@ 2023-06-09 8:30 ` Chen-Yu Tsai
2023-06-09 10:03 ` AngeloGioacchino Del Regno
2023-06-09 8:30 ` [PATCH 9/9] arm64: dts: mediatek: mt6358: Merge ldo_vcn33_* regulators Chen-Yu Tsai
2023-06-14 18:58 ` (subset) [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements Mark Brown
9 siblings, 1 reply; 32+ messages in thread
From: Chen-Yu Tsai @ 2023-06-09 8:30 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel
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>
---
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 26060909cf90..0b186b66ae29 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 = 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 = 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, 11, 21, 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, 11, 21, 10000),
+ REGULATOR_LINEAR_RANGE(2700000, 22, 32, 10000),
+ REGULATOR_LINEAR_RANGE(3000000, 33, 43, 10000),
+ REGULATOR_LINEAR_RANGE(3100000, 44, 54, 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, 11, 21, 10000),
+ REGULATOR_LINEAR_RANGE(1500000, 22, 32, 10000),
+ REGULATOR_LINEAR_RANGE(1800000, 33, 43, 10000),
+ REGULATOR_LINEAR_RANGE(2000000, 44, 54, 10000),
+ REGULATOR_LINEAR_RANGE(2800000, 55, 65, 10000),
+ REGULATOR_LINEAR_RANGE(3000000, 66, 76, 10000),
+ REGULATOR_LINEAR_RANGE(3300000, 77, 87, 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, 11, 21, 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, 11, 21, 10000),
+ REGULATOR_LINEAR_RANGE(1100000, 22, 32, 10000),
+ REGULATOR_LINEAR_RANGE(1200000, 33, 43, 10000),
+ REGULATOR_LINEAR_RANGE(1300000, 44, 54, 10000),
+ REGULATOR_LINEAR_RANGE(1500000, 55, 65, 10000),
+ REGULATOR_LINEAR_RANGE(1800000, 66, 76, 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, 11, 21, 10000),
+ REGULATOR_LINEAR_RANGE(1900000, 22, 32, 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, 11, 21, 10000),
+ REGULATOR_LINEAR_RANGE(3300000, 22, 32, 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, 11, 21, 10000),
+ REGULATOR_LINEAR_RANGE(2700000, 22, 32, 10000),
+ REGULATOR_LINEAR_RANGE(2800000, 33, 43, 10000),
+ REGULATOR_LINEAR_RANGE(2900000, 44, 54, 10000),
+ REGULATOR_LINEAR_RANGE(3000000, 55, 65, 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, 11, 21, 10000),
+ REGULATOR_LINEAR_RANGE(3500000, 22, 32, 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, 11, 21, 10000),
+ REGULATOR_LINEAR_RANGE(3000000, 22, 32, 10000),
+ REGULATOR_LINEAR_RANGE(3300000, 33, 43, 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, 11, 21, 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);
@@ -471,10 +426,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,
@@ -534,34 +489,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),
@@ -610,25 +565,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.41.0.162.gfafddb0af9-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 9/9] arm64: dts: mediatek: mt6358: Merge ldo_vcn33_* regulators
2023-06-09 8:29 [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
` (7 preceding siblings ...)
2023-06-09 8:30 ` [PATCH 8/9] regulator: mt6358: Add output voltage fine tuning to variable LDOs Chen-Yu Tsai
@ 2023-06-09 8:30 ` Chen-Yu Tsai
2023-06-14 18:58 ` (subset) [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements Mark Brown
9 siblings, 0 replies; 32+ messages in thread
From: Chen-Yu Tsai @ 2023-06-09 8:30 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel
The ldo_vcn33_bt and ldo_vcn33_wifi regulators are actually the same
regulator, having the same voltage setting and output pin. There are
simply two enable bits that are ORed together to enable the regulator.
Having two regulators representing the same output pin is misleading
from a design matching standpoint, and also error-prone in driver
implementations.
Now that the bindings have these two merged, merge them in the device
tree as well. Neither vcn33 regulators are referenced in upstream
device trees. As far as hardware designs go, none of the Chromebooks
using MT8183 w/ MT6358 use this output.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
arch/arm64/boot/dts/mediatek/mt6358.dtsi | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt6358.dtsi b/arch/arm64/boot/dts/mediatek/mt6358.dtsi
index b605313bed99..186898f9384b 100644
--- a/arch/arm64/boot/dts/mediatek/mt6358.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt6358.dtsi
@@ -304,15 +304,8 @@ mt6358_vrf18_reg: ldo_vrf18 {
regulator-enable-ramp-delay = <120>;
};
- mt6358_vcn33_bt_reg: ldo_vcn33_bt {
- regulator-name = "vcn33_bt";
- regulator-min-microvolt = <3300000>;
- regulator-max-microvolt = <3500000>;
- regulator-enable-ramp-delay = <270>;
- };
-
- mt6358_vcn33_wifi_reg: ldo_vcn33_wifi {
- regulator-name = "vcn33_wifi";
+ mt6358_vcn33_reg: ldo_vcn33 {
+ regulator-name = "vcn33";
regulator-min-microvolt = <3300000>;
regulator-max-microvolt = <3500000>;
regulator-enable-ramp-delay = <270>;
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 3/9] regulator: mt6358: Merge VCN33_* regulators
2023-06-09 8:30 ` [PATCH 3/9] regulator: mt6358: Merge VCN33_* regulators Chen-Yu Tsai
@ 2023-06-09 8:58 ` AngeloGioacchino Del Regno
2023-06-12 3:41 ` Chen-Yu Tsai
2023-06-09 15:56 ` Conor Dooley
2023-06-12 10:56 ` Fei Shao
2 siblings, 1 reply; 32+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-06-09 8:58 UTC (permalink / raw)
To: Chen-Yu Tsai, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mark Brown, Liam Girdwood, Matthias Brugger
Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
Il 09/06/23 10:30, Chen-Yu Tsai ha scritto:
> The VCN33_BT and VCN33_WIFI regulators are actually the same regulator,
> having the same voltage setting and output pin. There are simply two
> enable bits that are ORed together to enable the regulator.
>
> Having two regulators representing the same output pin is misleading
> from a design matching standpoint, and also error-prone in driver
> implementations. If consumers try to set different voltages on either
> regulator, the one set later would override the one set before. There
> are ways around this, such as chaining them together and having the
> downstream one act as a switch. But given there's only one output pin,
> such a workaround doesn't match reality.
>
> Remove the VCN33_WIFI regulator. During the probe phase, have the driver
> sync the enable status of VCN33_WIFI to VCN33_BT. Also drop the suffix
> so that the regulator name matches the pin name in the datasheet.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
> drivers/regulator/mt6358-regulator.c | 65 +++++++++++++++++-----
> include/linux/regulator/mt6358-regulator.h | 6 +-
> 2 files changed, 52 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
> index c9e16bd092f6..faf6b0757019 100644
> --- a/drivers/regulator/mt6358-regulator.c
> +++ b/drivers/regulator/mt6358-regulator.c
> @@ -277,7 +277,7 @@ static const unsigned int vcama_voltages[] = {
> 2800000, 2900000, 3000000,
> };
>
> -static const unsigned int vcn33_bt_wifi_voltages[] = {
> +static const unsigned int vcn33_voltages[] = {
> 3300000, 3400000, 3500000,
> };
>
> @@ -321,7 +321,7 @@ static const u32 vcama_idx[] = {
> 0, 7, 9, 10, 11, 12,
> };
>
> -static const u32 vcn33_bt_wifi_idx[] = {
> +static const u32 vcn33_idx[] = {
> 1, 2, 3,
> };
>
> @@ -566,12 +566,8 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
> MT6358_LDO_VCAMA1_CON0, 0, MT6358_VCAMA1_ANA_CON0, 0xf00),
> MT6358_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx,
> MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
> - MT6358_LDO("ldo_vcn33_bt", VCN33_BT, vcn33_bt_wifi_voltages,
> - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_0,
> - 0, MT6358_VCN33_ANA_CON0, 0x300),
> - MT6358_LDO("ldo_vcn33_wifi", VCN33_WIFI, vcn33_bt_wifi_voltages,
> - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_1,
> - 0, MT6358_VCN33_ANA_CON0, 0x300),
> + MT6358_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx,
> + MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
> MT6358_LDO("ldo_vcama2", VCAMA2, vcama_voltages, vcama_idx,
> MT6358_LDO_VCAMA2_CON0, 0, MT6358_VCAMA2_ANA_CON0, 0xf00),
> MT6358_LDO("ldo_vmc", VMC, vmc_voltages, vmc_idx,
> @@ -662,12 +658,8 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
> MT6358_LDO_VMCH_CON0, 0, MT6358_VMCH_ANA_CON0, 0x700),
> MT6366_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx,
> MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
> - MT6366_LDO("ldo_vcn33_bt", VCN33_BT, vcn33_bt_wifi_voltages,
> - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_0,
> - 0, MT6358_VCN33_ANA_CON0, 0x300),
> - MT6366_LDO("ldo_vcn33_wifi", VCN33_WIFI, vcn33_bt_wifi_voltages,
> - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_1,
> - 0, MT6358_VCN33_ANA_CON0, 0x300),
> + MT6366_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx,
> + MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
> MT6366_LDO("ldo_vmc", VMC, vmc_voltages, vmc_idx,
> MT6358_LDO_VMC_CON0, 0, MT6358_VMC_ANA_CON0, 0xf00),
> MT6366_LDO("ldo_vsim2", VSIM2, vsim_voltages, vsim_idx,
> @@ -690,13 +682,56 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
> MT6358_LDO_VSRAM_CON1, 0x7f),
> };
>
> +static int mt6358_sync_vcn33_setting(struct device *dev)
> +{
> + struct mt6397_chip *mt6397 = dev_get_drvdata(dev->parent);
> + unsigned int val;
> + int ret;
> +
> + /*
> + * VCN33_WIFI and VCN33_BT are two separate enable bits for the same
> + * regulator. They share the same voltage setting and output pin.
> + * Instead of having two potentially conflicting regulators, just have
> + * one VCN33 regulator. Sync the two enable bits and only use one in
> + * the regulator device.
> + */
> + ret = regmap_read(mt6397->regmap, MT6358_LDO_VCN33_CON0_1, &val);
> + if (ret) {
> + dev_err(dev, "Failed to read VCN33_WIFI setting\n");
> + return ret;
> + }
> +
> + if (!(val & BIT(0)))
> + return 0;
> +
> + /* Sync VCN33_WIFI enable status to VCN33_BT */
> + ret = regmap_update_bits(mt6397->regmap, MT6358_LDO_VCN33_CON0_0, BIT(0), BIT(0));
> + if (ret) {
> + dev_err(dev, "Failed to sync VCN33_WIFI setting to VCN33_BT\n");
> + return ret;
> + }
> +
> + /* Disable VCN33_WIFI */
> + ret = regmap_update_bits(mt6397->regmap, MT6358_LDO_VCN33_CON0_1, BIT(0), 0);
> + if (ret) {
> + dev_err(dev, "Failed to disable VCN33_BT\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static int mt6358_regulator_probe(struct platform_device *pdev)
> {
> struct mt6397_chip *mt6397 = dev_get_drvdata(pdev->dev.parent);
> struct regulator_config config = {};
> struct regulator_dev *rdev;
> struct mt6358_regulator_info *mt6358_info;
> - int i, max_regulator;
> + int i, max_regulator, ret;
> +
> + ret = mt6358_sync_vcn33_setting(&pdev->dev);
> + if (ret)
> + return ret;
I'd put this after the chip_id check, and I would also add a safety check for
that...
switch (mt6397->chip_id) {
case MT6366_CHIP_ID:
max_regulator = MT6366_MAX_REGULATOR;
mt6358_info = mt6366_regulators;
break;
case MT6358_CHIP_ID:
max_regulator = MT6358_MAX_REGULATOR;
mt6358_info = mt6358_regulators;
break;
default:
return -EINVAL;
}
ret = mt6358_sync_vcn33_setting(....)
...but I agree with your point here about this being a strange design and
also with your way of fixing the driver.
Regards,
Angelo
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 4/9] regulator: mt6358: Drop *_SSHUB regulators
2023-06-09 8:30 ` [PATCH 4/9] regulator: mt6358: Drop *_SSHUB regulators Chen-Yu Tsai
@ 2023-06-09 9:03 ` AngeloGioacchino Del Regno
2023-06-12 4:45 ` Chen-Yu Tsai
2023-06-09 15:52 ` Matthias Brugger
1 sibling, 1 reply; 32+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-06-09 9:03 UTC (permalink / raw)
To: Chen-Yu Tsai, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mark Brown, Liam Girdwood, Matthias Brugger, Gene Chen,
Johnson Wang
Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
Il 09/06/23 10:30, Chen-Yu Tsai ha scritto:
> The *_SSHUB regulators are actually alternate configuration interfaces
> for their non *_SSHUB counterparts. They are not separate regulator
> outputs. These registers are intended for the companion processor to
> use to configure the power rails while the main processor is sleeping.
> They are not intended for the main operating system to use.
>
> Since they are not real outputs they shouldn't be modeled separately.
> Remove them. Luckily no device tree actually uses them.
>
I'm not sure that MT6358/6366 are used only on Chromebook SoCs, and that this SSHUB
mechanism (companion processor) is the same across all firmwares.
I'd like someone from MediaTek to confirm that this is valid for both Chromebook
and Smartphone firmwares.
Regards,
Angelo
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
> drivers/regulator/mt6358-regulator.c | 14 --------------
> include/linux/regulator/mt6358-regulator.h | 4 ----
> 2 files changed, 18 deletions(-)
>
> diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
> index faf6b0757019..946a251a8b3a 100644
> --- a/drivers/regulator/mt6358-regulator.c
> +++ b/drivers/regulator/mt6358-regulator.c
> @@ -505,9 +505,6 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
> MT6358_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
> buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
> MT6358_VCORE_VGPU_ANA_CON0, 1),
> - MT6358_BUCK("buck_vcore_sshub", VCORE_SSHUB, 500000, 1293750, 6250,
> - buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_SSHUB_ELR0, 0x7f,
> - MT6358_VCORE_VGPU_ANA_CON0, 1),
> MT6358_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
> buck_volt_range3, 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f,
> MT6358_VPA_ANA_CON0, 3),
> @@ -583,10 +580,6 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
> MT6358_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
> buck_volt_range1, MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
> MT6358_LDO_VSRAM_CON2, 0x7f),
> - MT6358_LDO1("ldo_vsram_others_sshub", VSRAM_OTHERS_SSHUB, 500000,
> - 1293750, 6250, buck_volt_range1,
> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f,
> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f),
> MT6358_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
> buck_volt_range1, MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
> MT6358_LDO_VSRAM_CON3, 0x7f),
> @@ -603,9 +596,6 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
> MT6366_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
> buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
> MT6358_VCORE_VGPU_ANA_CON0, 1),
> - MT6366_BUCK("buck_vcore_sshub", VCORE_SSHUB, 500000, 1293750, 6250,
> - buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_SSHUB_ELR0, 0x7f,
> - MT6358_VCORE_VGPU_ANA_CON0, 1),
> MT6366_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
> buck_volt_range3, 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f,
> MT6358_VPA_ANA_CON0, 3),
> @@ -670,10 +660,6 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
> MT6366_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
> buck_volt_range1, MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
> MT6358_LDO_VSRAM_CON2, 0x7f),
> - MT6366_LDO1("ldo_vsram_others_sshub", VSRAM_OTHERS_SSHUB, 500000,
> - 1293750, 6250, buck_volt_range1,
> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f,
> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f),
> MT6366_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
> buck_volt_range1, MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
> MT6358_LDO_VSRAM_CON3, 0x7f),
> diff --git a/include/linux/regulator/mt6358-regulator.h b/include/linux/regulator/mt6358-regulator.h
> index a4307cd9edd6..c71a6a9fce7a 100644
> --- a/include/linux/regulator/mt6358-regulator.h
> +++ b/include/linux/regulator/mt6358-regulator.h
> @@ -47,8 +47,6 @@ enum {
> MT6358_ID_VLDO28,
> MT6358_ID_VAUD28,
> MT6358_ID_VSIM2,
> - MT6358_ID_VCORE_SSHUB,
> - MT6358_ID_VSRAM_OTHERS_SSHUB,
> MT6358_ID_RG_MAX,
> };
>
> @@ -88,8 +86,6 @@ enum {
> MT6366_ID_VMC,
> MT6366_ID_VAUD28,
> MT6366_ID_VSIM2,
> - MT6366_ID_VCORE_SSHUB,
> - MT6366_ID_VSRAM_OTHERS_SSHUB,
> MT6366_ID_RG_MAX,
> };
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 8/9] regulator: mt6358: Add output voltage fine tuning to variable LDOs
2023-06-09 8:30 ` [PATCH 8/9] regulator: mt6358: Add output voltage fine tuning to variable LDOs Chen-Yu Tsai
@ 2023-06-09 10:03 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 32+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-06-09 10:03 UTC (permalink / raw)
To: Chen-Yu Tsai, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mark Brown, Liam Girdwood, Matthias Brugger
Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
Il 09/06/23 10:30, 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.
>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 7/9] regulator: mt6358: Add output voltage fine tuning to fixed regulators
2023-06-09 8:30 ` [PATCH 7/9] regulator: mt6358: Add output voltage fine tuning to fixed regulators Chen-Yu Tsai
@ 2023-06-09 10:03 ` AngeloGioacchino Del Regno
2023-06-14 16:14 ` Mark Brown
1 sibling, 0 replies; 32+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-06-09 10:03 UTC (permalink / raw)
To: Chen-Yu Tsai, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mark Brown, Liam Girdwood, Matthias Brugger
Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
Il 09/06/23 10:30, Chen-Yu Tsai ha scritto:
> 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.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/9] regulator: dt-bindings: mt6358: Merge ldo_vcn33_* regulators
2023-06-09 8:29 ` [PATCH 1/9] regulator: dt-bindings: mt6358: Merge ldo_vcn33_* regulators Chen-Yu Tsai
@ 2023-06-09 15:47 ` Matthias Brugger
2023-06-09 16:02 ` Krzysztof Kozlowski
1 sibling, 0 replies; 32+ messages in thread
From: Matthias Brugger @ 2023-06-09 15:47 UTC (permalink / raw)
To: Chen-Yu Tsai, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mark Brown, Liam Girdwood, AngeloGioacchino Del Regno
Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
On 09/06/2023 10:29, Chen-Yu Tsai wrote:
> The ldo_vcn33_bt and ldo_vcn33_wifi regulators are actually the same
> regulator, having the same voltage setting and output pin. There are
> simply two enable bits that are ORed together to enable the regulator.
>
> Having two regulators representing the same output pin is misleading
> from a design matching standpoint, and also error-prone in driver
> implementations.
>
> Merge the two as ldo_vcn33. Neither vcn33 regulators are referenced
> in upstream device trees. As far as hardware designs go, none of the
> Chromebooks using MT8183 w/ MT6358 use this output.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
> ---
> .../bindings/regulator/mt6358-regulator.txt | 14 +++-----------
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt b/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt
> index 7034cdca54e0..b22b956d1cbe 100644
> --- a/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt
> +++ b/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt
> @@ -15,8 +15,7 @@ LDO:
> ldo_vcamd, ldo_vcn18, ldo_vfe28, ldo_vsram_proc11, ldo_vcn28, ldo_vsram_others,
> ldo_vsram_others_sshub, ldo_vsram_gpu, ldo_vxo22, ldo_vefuse, ldo_vaux18,
> ldo_vmch, ldo_vbif28, ldo_vsram_proc12, ldo_vcama1, ldo_vemc, ldo_vio28, ldo_va12,
> - ldo_vrf18, ldo_vcn33_bt, ldo_vcn33_wifi, ldo_vcama2, ldo_vmc, ldo_vldo28, ldo_vaud28,
> - ldo_vsim2
> + ldo_vrf18, ldo_vcn33, ldo_vcama2, ldo_vmc, ldo_vldo28, ldo_vaud28, ldo_vsim2
>
> Example:
>
> @@ -305,15 +304,8 @@ Example:
> regulator-enable-ramp-delay = <120>;
> };
>
> - mt6358_vcn33_bt_reg: ldo_vcn33_bt {
> - regulator-name = "vcn33_bt";
> - regulator-min-microvolt = <3300000>;
> - regulator-max-microvolt = <3500000>;
> - regulator-enable-ramp-delay = <270>;
> - };
> -
> - mt6358_vcn33_wifi_reg: ldo_vcn33_wifi {
> - regulator-name = "vcn33_wifi";
> + mt6358_vcn33_reg: ldo_vcn33 {
> + regulator-name = "vcn33";
> regulator-min-microvolt = <3300000>;
> regulator-max-microvolt = <3500000>;
> regulator-enable-ramp-delay = <270>;
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/9] regulator: dt-bindings: mt6358: Drop *_sshub regulators
2023-06-09 8:29 ` [PATCH 2/9] regulator: dt-bindings: mt6358: Drop *_sshub regulators Chen-Yu Tsai
@ 2023-06-09 15:47 ` Matthias Brugger
2023-06-09 16:02 ` Krzysztof Kozlowski
1 sibling, 0 replies; 32+ messages in thread
From: Matthias Brugger @ 2023-06-09 15:47 UTC (permalink / raw)
To: Chen-Yu Tsai, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mark Brown, Liam Girdwood, AngeloGioacchino Del Regno
Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
On 09/06/2023 10:29, Chen-Yu Tsai wrote:
> The *_sshub regulators are actually alternate configuration interfaces
> for their non *_sshub counterparts. They are not separate regulator
> outputs. These registers are intended for the companion processor to
> use to configure the power rails while the main processor is sleeping.
> They are not intended for the main operating system to use.
>
> Since they are not real outputs they shouldn't be modeled separately.
> Remove them. Luckily no device tree actually uses them.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
> ---
> .../bindings/regulator/mt6358-regulator.txt | 22 +++++--------------
> 1 file changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt b/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt
> index b22b956d1cbe..b6384306db5c 100644
> --- a/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt
> +++ b/Documentation/devicetree/bindings/regulator/mt6358-regulator.txt
> @@ -8,14 +8,14 @@ Documentation/devicetree/bindings/regulator/regulator.txt.
>
> The valid names for regulators are::
> BUCK:
> - buck_vdram1, buck_vcore, buck_vcore_sshub, buck_vpa, buck_vproc11,
> - buck_vproc12, buck_vgpu, buck_vs2, buck_vmodem, buck_vs1
> + buck_vdram1, buck_vcore, buck_vpa, buck_vproc11, buck_vproc12, buck_vgpu,
> + buck_vs2, buck_vmodem, buck_vs1
> LDO:
> ldo_vdram2, ldo_vsim1, ldo_vibr, ldo_vrf12, ldo_vio18, ldo_vusb, ldo_vcamio,
> ldo_vcamd, ldo_vcn18, ldo_vfe28, ldo_vsram_proc11, ldo_vcn28, ldo_vsram_others,
> - ldo_vsram_others_sshub, ldo_vsram_gpu, ldo_vxo22, ldo_vefuse, ldo_vaux18,
> - ldo_vmch, ldo_vbif28, ldo_vsram_proc12, ldo_vcama1, ldo_vemc, ldo_vio28, ldo_va12,
> - ldo_vrf18, ldo_vcn33, ldo_vcama2, ldo_vmc, ldo_vldo28, ldo_vaud28, ldo_vsim2
> + ldo_vsram_gpu, ldo_vxo22, ldo_vefuse, ldo_vaux18, ldo_vmch, ldo_vbif28,
> + ldo_vsram_proc12, ldo_vcama1, ldo_vemc, ldo_vio28, ldo_va12, ldo_vrf18,
> + ldo_vcn33, ldo_vcama2, ldo_vmc, ldo_vldo28, ldo_vaud28, ldo_vsim2
>
> Example:
>
> @@ -346,17 +346,5 @@ Example:
> regulator-max-microvolt = <3100000>;
> regulator-enable-ramp-delay = <540>;
> };
> -
> - mt6358_vcore_sshub_reg: buck_vcore_sshub {
> - regulator-name = "vcore_sshub";
> - regulator-min-microvolt = <500000>;
> - regulator-max-microvolt = <1293750>;
> - };
> -
> - mt6358_vsram_others_sshub_reg: ldo_vsram_others_sshub {
> - regulator-name = "vsram_others_sshub";
> - regulator-min-microvolt = <500000>;
> - regulator-max-microvolt = <1293750>;
> - };
> };
> };
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 4/9] regulator: mt6358: Drop *_SSHUB regulators
2023-06-09 8:30 ` [PATCH 4/9] regulator: mt6358: Drop *_SSHUB regulators Chen-Yu Tsai
2023-06-09 9:03 ` AngeloGioacchino Del Regno
@ 2023-06-09 15:52 ` Matthias Brugger
1 sibling, 0 replies; 32+ messages in thread
From: Matthias Brugger @ 2023-06-09 15:52 UTC (permalink / raw)
To: Chen-Yu Tsai, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mark Brown, Liam Girdwood, AngeloGioacchino Del Regno
Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
On 09/06/2023 10:30, Chen-Yu Tsai wrote:
> The *_SSHUB regulators are actually alternate configuration interfaces
> for their non *_SSHUB counterparts. They are not separate regulator
> outputs. These registers are intended for the companion processor to
> use to configure the power rails while the main processor is sleeping.
> They are not intended for the main operating system to use.
>
> Since they are not real outputs they shouldn't be modeled separately.
> Remove them. Luckily no device tree actually uses them.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
> ---
> drivers/regulator/mt6358-regulator.c | 14 --------------
> include/linux/regulator/mt6358-regulator.h | 4 ----
> 2 files changed, 18 deletions(-)
>
> diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
> index faf6b0757019..946a251a8b3a 100644
> --- a/drivers/regulator/mt6358-regulator.c
> +++ b/drivers/regulator/mt6358-regulator.c
> @@ -505,9 +505,6 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
> MT6358_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
> buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
> MT6358_VCORE_VGPU_ANA_CON0, 1),
> - MT6358_BUCK("buck_vcore_sshub", VCORE_SSHUB, 500000, 1293750, 6250,
> - buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_SSHUB_ELR0, 0x7f,
> - MT6358_VCORE_VGPU_ANA_CON0, 1),
> MT6358_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
> buck_volt_range3, 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f,
> MT6358_VPA_ANA_CON0, 3),
> @@ -583,10 +580,6 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
> MT6358_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
> buck_volt_range1, MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
> MT6358_LDO_VSRAM_CON2, 0x7f),
> - MT6358_LDO1("ldo_vsram_others_sshub", VSRAM_OTHERS_SSHUB, 500000,
> - 1293750, 6250, buck_volt_range1,
> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f,
> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f),
> MT6358_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
> buck_volt_range1, MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
> MT6358_LDO_VSRAM_CON3, 0x7f),
> @@ -603,9 +596,6 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
> MT6366_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
> buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
> MT6358_VCORE_VGPU_ANA_CON0, 1),
> - MT6366_BUCK("buck_vcore_sshub", VCORE_SSHUB, 500000, 1293750, 6250,
> - buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_SSHUB_ELR0, 0x7f,
> - MT6358_VCORE_VGPU_ANA_CON0, 1),
> MT6366_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
> buck_volt_range3, 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f,
> MT6358_VPA_ANA_CON0, 3),
> @@ -670,10 +660,6 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
> MT6366_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
> buck_volt_range1, MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
> MT6358_LDO_VSRAM_CON2, 0x7f),
> - MT6366_LDO1("ldo_vsram_others_sshub", VSRAM_OTHERS_SSHUB, 500000,
> - 1293750, 6250, buck_volt_range1,
> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f,
> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f),
> MT6366_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
> buck_volt_range1, MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
> MT6358_LDO_VSRAM_CON3, 0x7f),
> diff --git a/include/linux/regulator/mt6358-regulator.h b/include/linux/regulator/mt6358-regulator.h
> index a4307cd9edd6..c71a6a9fce7a 100644
> --- a/include/linux/regulator/mt6358-regulator.h
> +++ b/include/linux/regulator/mt6358-regulator.h
> @@ -47,8 +47,6 @@ enum {
> MT6358_ID_VLDO28,
> MT6358_ID_VAUD28,
> MT6358_ID_VSIM2,
> - MT6358_ID_VCORE_SSHUB,
> - MT6358_ID_VSRAM_OTHERS_SSHUB,
> MT6358_ID_RG_MAX,
> };
>
> @@ -88,8 +86,6 @@ enum {
> MT6366_ID_VMC,
> MT6366_ID_VAUD28,
> MT6366_ID_VSIM2,
> - MT6366_ID_VCORE_SSHUB,
> - MT6366_ID_VSRAM_OTHERS_SSHUB,
> MT6366_ID_RG_MAX,
> };
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 3/9] regulator: mt6358: Merge VCN33_* regulators
2023-06-09 8:30 ` [PATCH 3/9] regulator: mt6358: Merge VCN33_* regulators Chen-Yu Tsai
2023-06-09 8:58 ` AngeloGioacchino Del Regno
@ 2023-06-09 15:56 ` Conor Dooley
2023-06-10 15:28 ` Conor Dooley
2023-06-12 10:56 ` Fei Shao
2 siblings, 1 reply; 32+ messages in thread
From: Conor Dooley @ 2023-06-09 15:56 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2806 bytes --]
On Fri, Jun 09, 2023 at 04:30:00PM +0800, Chen-Yu Tsai wrote:
> The VCN33_BT and VCN33_WIFI regulators are actually the same regulator,
> having the same voltage setting and output pin. There are simply two
> enable bits that are ORed together to enable the regulator.
>
> Having two regulators representing the same output pin is misleading
> from a design matching standpoint, and also error-prone in driver
> implementations. If consumers try to set different voltages on either
> regulator, the one set later would override the one set before. There
> are ways around this, such as chaining them together and having the
> downstream one act as a switch. But given there's only one output pin,
> such a workaround doesn't match reality.
>
> Remove the VCN33_WIFI regulator. During the probe phase, have the driver
> sync the enable status of VCN33_WIFI to VCN33_BT. Also drop the suffix
> so that the regulator name matches the pin name in the datasheet.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
> drivers/regulator/mt6358-regulator.c | 65 +++++++++++++++++-----
> include/linux/regulator/mt6358-regulator.h | 6 +-
> 2 files changed, 52 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
> index c9e16bd092f6..faf6b0757019 100644
> --- a/drivers/regulator/mt6358-regulator.c
> +++ b/drivers/regulator/mt6358-regulator.c
> @@ -277,7 +277,7 @@ static const unsigned int vcama_voltages[] = {
> 2800000, 2900000, 3000000,
> };
>
> -static const unsigned int vcn33_bt_wifi_voltages[] = {
> +static const unsigned int vcn33_voltages[] = {
> 3300000, 3400000, 3500000,
> };
>
> @@ -321,7 +321,7 @@ static const u32 vcama_idx[] = {
> 0, 7, 9, 10, 11, 12,
> };
>
> -static const u32 vcn33_bt_wifi_idx[] = {
> +static const u32 vcn33_idx[] = {
> 1, 2, 3,
> };
>
> @@ -566,12 +566,8 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
> MT6358_LDO_VCAMA1_CON0, 0, MT6358_VCAMA1_ANA_CON0, 0xf00),
> MT6358_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx,
> MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
> - MT6358_LDO("ldo_vcn33_bt", VCN33_BT, vcn33_bt_wifi_voltages,
> - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_0,
> - 0, MT6358_VCN33_ANA_CON0, 0x300),
> - MT6358_LDO("ldo_vcn33_wifi", VCN33_WIFI, vcn33_bt_wifi_voltages,
> - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_1,
> - 0, MT6358_VCN33_ANA_CON0, 0x300),
> + MT6358_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx,
> + MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
Excuse me if I am being daft here, but could you explain how this change
is compatible with existing devicetrees?
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/9] regulator: dt-bindings: mt6358: Merge ldo_vcn33_* regulators
2023-06-09 8:29 ` [PATCH 1/9] regulator: dt-bindings: mt6358: Merge ldo_vcn33_* regulators Chen-Yu Tsai
2023-06-09 15:47 ` Matthias Brugger
@ 2023-06-09 16:02 ` Krzysztof Kozlowski
1 sibling, 0 replies; 32+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-09 16:02 UTC (permalink / raw)
To: Chen-Yu Tsai, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mark Brown, Liam Girdwood, Matthias Brugger,
AngeloGioacchino Del Regno
Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
On 09/06/2023 10:29, Chen-Yu Tsai wrote:
> The ldo_vcn33_bt and ldo_vcn33_wifi regulators are actually the same
> regulator, having the same voltage setting and output pin. There are
> simply two enable bits that are ORed together to enable the regulator.
>
> Having two regulators representing the same output pin is misleading
> from a design matching standpoint, and also error-prone in driver
> implementations.
>
> Merge the two as ldo_vcn33. Neither vcn33 regulators are referenced
> in upstream device trees. As far as hardware designs go, none of the
> Chromebooks using MT8183 w/ MT6358 use this output.
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/9] regulator: dt-bindings: mt6358: Drop *_sshub regulators
2023-06-09 8:29 ` [PATCH 2/9] regulator: dt-bindings: mt6358: Drop *_sshub regulators Chen-Yu Tsai
2023-06-09 15:47 ` Matthias Brugger
@ 2023-06-09 16:02 ` Krzysztof Kozlowski
1 sibling, 0 replies; 32+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-09 16:02 UTC (permalink / raw)
To: Chen-Yu Tsai, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mark Brown, Liam Girdwood, Matthias Brugger,
AngeloGioacchino Del Regno
Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
On 09/06/2023 10:29, Chen-Yu Tsai wrote:
> The *_sshub regulators are actually alternate configuration interfaces
> for their non *_sshub counterparts. They are not separate regulator
> outputs. These registers are intended for the companion processor to
> use to configure the power rails while the main processor is sleeping.
> They are not intended for the main operating system to use.
>
> Since they are not real outputs they shouldn't be modeled separately.
> Remove them. Luckily no device tree actually uses them.
>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 3/9] regulator: mt6358: Merge VCN33_* regulators
2023-06-09 15:56 ` Conor Dooley
@ 2023-06-10 15:28 ` Conor Dooley
2023-06-12 4:19 ` Chen-Yu Tsai
0 siblings, 1 reply; 32+ messages in thread
From: Conor Dooley @ 2023-06-10 15:28 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3171 bytes --]
On Fri, Jun 09, 2023 at 04:56:05PM +0100, Conor Dooley wrote:
> On Fri, Jun 09, 2023 at 04:30:00PM +0800, Chen-Yu Tsai wrote:
> > The VCN33_BT and VCN33_WIFI regulators are actually the same regulator,
> > having the same voltage setting and output pin. There are simply two
> > enable bits that are ORed together to enable the regulator.
> >
> > Having two regulators representing the same output pin is misleading
> > from a design matching standpoint, and also error-prone in driver
> > implementations. If consumers try to set different voltages on either
> > regulator, the one set later would override the one set before. There
> > are ways around this, such as chaining them together and having the
> > downstream one act as a switch. But given there's only one output pin,
> > such a workaround doesn't match reality.
> >
> > Remove the VCN33_WIFI regulator. During the probe phase, have the driver
> > sync the enable status of VCN33_WIFI to VCN33_BT. Also drop the suffix
> > so that the regulator name matches the pin name in the datasheet.
> >
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> > drivers/regulator/mt6358-regulator.c | 65 +++++++++++++++++-----
> > include/linux/regulator/mt6358-regulator.h | 6 +-
> > 2 files changed, 52 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
> > index c9e16bd092f6..faf6b0757019 100644
> > --- a/drivers/regulator/mt6358-regulator.c
> > +++ b/drivers/regulator/mt6358-regulator.c
> > @@ -277,7 +277,7 @@ static const unsigned int vcama_voltages[] = {
> > 2800000, 2900000, 3000000,
> > };
> >
> > -static const unsigned int vcn33_bt_wifi_voltages[] = {
> > +static const unsigned int vcn33_voltages[] = {
> > 3300000, 3400000, 3500000,
> > };
> >
> > @@ -321,7 +321,7 @@ static const u32 vcama_idx[] = {
> > 0, 7, 9, 10, 11, 12,
> > };
> >
> > -static const u32 vcn33_bt_wifi_idx[] = {
> > +static const u32 vcn33_idx[] = {
> > 1, 2, 3,
> > };
> >
> > @@ -566,12 +566,8 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
> > MT6358_LDO_VCAMA1_CON0, 0, MT6358_VCAMA1_ANA_CON0, 0xf00),
> > MT6358_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx,
> > MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
> > - MT6358_LDO("ldo_vcn33_bt", VCN33_BT, vcn33_bt_wifi_voltages,
> > - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_0,
> > - 0, MT6358_VCN33_ANA_CON0, 0x300),
> > - MT6358_LDO("ldo_vcn33_wifi", VCN33_WIFI, vcn33_bt_wifi_voltages,
> > - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_1,
> > - 0, MT6358_VCN33_ANA_CON0, 0x300),
> > + MT6358_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx,
> > + MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
>
> Excuse me if I am being daft here, but could you explain how this change
> is compatible with existing devicetrees?
Ah, I see in the binding commit there's a "Luckily no device tree actually
uses them." Does that just cover the kernel, or does it consider other
operating systems/bootloaders?
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 3/9] regulator: mt6358: Merge VCN33_* regulators
2023-06-09 8:58 ` AngeloGioacchino Del Regno
@ 2023-06-12 3:41 ` Chen-Yu Tsai
0 siblings, 0 replies; 32+ messages in thread
From: Chen-Yu Tsai @ 2023-06-12 3:41 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, devicetree, linux-arm-kernel,
linux-mediatek, linux-kernel
On Fri, Jun 9, 2023 at 4:58 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 09/06/23 10:30, Chen-Yu Tsai ha scritto:
> > The VCN33_BT and VCN33_WIFI regulators are actually the same regulator,
> > having the same voltage setting and output pin. There are simply two
> > enable bits that are ORed together to enable the regulator.
> >
> > Having two regulators representing the same output pin is misleading
> > from a design matching standpoint, and also error-prone in driver
> > implementations. If consumers try to set different voltages on either
> > regulator, the one set later would override the one set before. There
> > are ways around this, such as chaining them together and having the
> > downstream one act as a switch. But given there's only one output pin,
> > such a workaround doesn't match reality.
> >
> > Remove the VCN33_WIFI regulator. During the probe phase, have the driver
> > sync the enable status of VCN33_WIFI to VCN33_BT. Also drop the suffix
> > so that the regulator name matches the pin name in the datasheet.
> >
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> > drivers/regulator/mt6358-regulator.c | 65 +++++++++++++++++-----
> > include/linux/regulator/mt6358-regulator.h | 6 +-
> > 2 files changed, 52 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
> > index c9e16bd092f6..faf6b0757019 100644
> > --- a/drivers/regulator/mt6358-regulator.c
> > +++ b/drivers/regulator/mt6358-regulator.c
> > @@ -277,7 +277,7 @@ static const unsigned int vcama_voltages[] = {
> > 2800000, 2900000, 3000000,
> > };
> >
> > -static const unsigned int vcn33_bt_wifi_voltages[] = {
> > +static const unsigned int vcn33_voltages[] = {
> > 3300000, 3400000, 3500000,
> > };
> >
> > @@ -321,7 +321,7 @@ static const u32 vcama_idx[] = {
> > 0, 7, 9, 10, 11, 12,
> > };
> >
> > -static const u32 vcn33_bt_wifi_idx[] = {
> > +static const u32 vcn33_idx[] = {
> > 1, 2, 3,
> > };
> >
> > @@ -566,12 +566,8 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
> > MT6358_LDO_VCAMA1_CON0, 0, MT6358_VCAMA1_ANA_CON0, 0xf00),
> > MT6358_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx,
> > MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
> > - MT6358_LDO("ldo_vcn33_bt", VCN33_BT, vcn33_bt_wifi_voltages,
> > - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_0,
> > - 0, MT6358_VCN33_ANA_CON0, 0x300),
> > - MT6358_LDO("ldo_vcn33_wifi", VCN33_WIFI, vcn33_bt_wifi_voltages,
> > - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_1,
> > - 0, MT6358_VCN33_ANA_CON0, 0x300),
> > + MT6358_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx,
> > + MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
> > MT6358_LDO("ldo_vcama2", VCAMA2, vcama_voltages, vcama_idx,
> > MT6358_LDO_VCAMA2_CON0, 0, MT6358_VCAMA2_ANA_CON0, 0xf00),
> > MT6358_LDO("ldo_vmc", VMC, vmc_voltages, vmc_idx,
> > @@ -662,12 +658,8 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
> > MT6358_LDO_VMCH_CON0, 0, MT6358_VMCH_ANA_CON0, 0x700),
> > MT6366_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx,
> > MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
> > - MT6366_LDO("ldo_vcn33_bt", VCN33_BT, vcn33_bt_wifi_voltages,
> > - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_0,
> > - 0, MT6358_VCN33_ANA_CON0, 0x300),
> > - MT6366_LDO("ldo_vcn33_wifi", VCN33_WIFI, vcn33_bt_wifi_voltages,
> > - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_1,
> > - 0, MT6358_VCN33_ANA_CON0, 0x300),
> > + MT6366_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx,
> > + MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
> > MT6366_LDO("ldo_vmc", VMC, vmc_voltages, vmc_idx,
> > MT6358_LDO_VMC_CON0, 0, MT6358_VMC_ANA_CON0, 0xf00),
> > MT6366_LDO("ldo_vsim2", VSIM2, vsim_voltages, vsim_idx,
> > @@ -690,13 +682,56 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
> > MT6358_LDO_VSRAM_CON1, 0x7f),
> > };
> >
> > +static int mt6358_sync_vcn33_setting(struct device *dev)
> > +{
> > + struct mt6397_chip *mt6397 = dev_get_drvdata(dev->parent);
> > + unsigned int val;
> > + int ret;
> > +
> > + /*
> > + * VCN33_WIFI and VCN33_BT are two separate enable bits for the same
> > + * regulator. They share the same voltage setting and output pin.
> > + * Instead of having two potentially conflicting regulators, just have
> > + * one VCN33 regulator. Sync the two enable bits and only use one in
> > + * the regulator device.
> > + */
> > + ret = regmap_read(mt6397->regmap, MT6358_LDO_VCN33_CON0_1, &val);
> > + if (ret) {
> > + dev_err(dev, "Failed to read VCN33_WIFI setting\n");
> > + return ret;
> > + }
> > +
> > + if (!(val & BIT(0)))
> > + return 0;
> > +
> > + /* Sync VCN33_WIFI enable status to VCN33_BT */
> > + ret = regmap_update_bits(mt6397->regmap, MT6358_LDO_VCN33_CON0_0, BIT(0), BIT(0));
> > + if (ret) {
> > + dev_err(dev, "Failed to sync VCN33_WIFI setting to VCN33_BT\n");
> > + return ret;
> > + }
> > +
> > + /* Disable VCN33_WIFI */
> > + ret = regmap_update_bits(mt6397->regmap, MT6358_LDO_VCN33_CON0_1, BIT(0), 0);
> > + if (ret) {
> > + dev_err(dev, "Failed to disable VCN33_BT\n");
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int mt6358_regulator_probe(struct platform_device *pdev)
> > {
> > struct mt6397_chip *mt6397 = dev_get_drvdata(pdev->dev.parent);
> > struct regulator_config config = {};
> > struct regulator_dev *rdev;
> > struct mt6358_regulator_info *mt6358_info;
> > - int i, max_regulator;
> > + int i, max_regulator, ret;
> > +
> > + ret = mt6358_sync_vcn33_setting(&pdev->dev);
> > + if (ret)
> > + return ret;
>
> I'd put this after the chip_id check, and I would also add a safety check for
> that...
>
> switch (mt6397->chip_id) {
> case MT6366_CHIP_ID:
> max_regulator = MT6366_MAX_REGULATOR;
> mt6358_info = mt6366_regulators;
> break;
> case MT6358_CHIP_ID:
> max_regulator = MT6358_MAX_REGULATOR;
> mt6358_info = mt6358_regulators;
> break;
> default:
> return -EINVAL;
> }
>
> ret = mt6358_sync_vcn33_setting(....)
Sounds good. We wouldn't want to be poking random bits in some other PMIC.
> ...but I agree with your point here about this being a strange design and
> also with your way of fixing the driver.
What I heard was that they support separate Bluetooth and WiFi drivers that
don't have a common reference counting framework for their regulator
supplies using this scheme. Maybe they are doing the power sequencing in
some small firmware.
ChenYu
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 3/9] regulator: mt6358: Merge VCN33_* regulators
2023-06-10 15:28 ` Conor Dooley
@ 2023-06-12 4:19 ` Chen-Yu Tsai
2023-06-12 17:34 ` Conor Dooley
0 siblings, 1 reply; 32+ messages in thread
From: Chen-Yu Tsai @ 2023-06-12 4:19 UTC (permalink / raw)
To: Conor Dooley
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
On Sat, Jun 10, 2023 at 11:28 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, Jun 09, 2023 at 04:56:05PM +0100, Conor Dooley wrote:
> > On Fri, Jun 09, 2023 at 04:30:00PM +0800, Chen-Yu Tsai wrote:
> > > The VCN33_BT and VCN33_WIFI regulators are actually the same regulator,
> > > having the same voltage setting and output pin. There are simply two
> > > enable bits that are ORed together to enable the regulator.
> > >
> > > Having two regulators representing the same output pin is misleading
> > > from a design matching standpoint, and also error-prone in driver
> > > implementations. If consumers try to set different voltages on either
> > > regulator, the one set later would override the one set before. There
> > > are ways around this, such as chaining them together and having the
> > > downstream one act as a switch. But given there's only one output pin,
> > > such a workaround doesn't match reality.
> > >
> > > Remove the VCN33_WIFI regulator. During the probe phase, have the driver
> > > sync the enable status of VCN33_WIFI to VCN33_BT. Also drop the suffix
> > > so that the regulator name matches the pin name in the datasheet.
> > >
> > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > > ---
> > > drivers/regulator/mt6358-regulator.c | 65 +++++++++++++++++-----
> > > include/linux/regulator/mt6358-regulator.h | 6 +-
> > > 2 files changed, 52 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
> > > index c9e16bd092f6..faf6b0757019 100644
> > > --- a/drivers/regulator/mt6358-regulator.c
> > > +++ b/drivers/regulator/mt6358-regulator.c
> > > @@ -277,7 +277,7 @@ static const unsigned int vcama_voltages[] = {
> > > 2800000, 2900000, 3000000,
> > > };
> > >
> > > -static const unsigned int vcn33_bt_wifi_voltages[] = {
> > > +static const unsigned int vcn33_voltages[] = {
> > > 3300000, 3400000, 3500000,
> > > };
> > >
> > > @@ -321,7 +321,7 @@ static const u32 vcama_idx[] = {
> > > 0, 7, 9, 10, 11, 12,
> > > };
> > >
> > > -static const u32 vcn33_bt_wifi_idx[] = {
> > > +static const u32 vcn33_idx[] = {
> > > 1, 2, 3,
> > > };
> > >
> > > @@ -566,12 +566,8 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
> > > MT6358_LDO_VCAMA1_CON0, 0, MT6358_VCAMA1_ANA_CON0, 0xf00),
> > > MT6358_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx,
> > > MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
> > > - MT6358_LDO("ldo_vcn33_bt", VCN33_BT, vcn33_bt_wifi_voltages,
> > > - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_0,
> > > - 0, MT6358_VCN33_ANA_CON0, 0x300),
> > > - MT6358_LDO("ldo_vcn33_wifi", VCN33_WIFI, vcn33_bt_wifi_voltages,
> > > - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_1,
> > > - 0, MT6358_VCN33_ANA_CON0, 0x300),
> > > + MT6358_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx,
> > > + MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
> >
> > Excuse me if I am being daft here, but could you explain how this change
> > is compatible with existing devicetrees?
>
> Ah, I see in the binding commit there's a "Luckily no device tree actually
> uses them." Does that just cover the kernel, or does it consider other
> operating systems/bootloaders?
That comment covers the upstream kernel and the downstream ChromeOS kernel
specifically. The bootloader that ChromeOS uses (coreboot) doesn't use
device trees. I don't know what MediaTek uses for their phones though.
AFAIK MediaTek only supports the Linux kernel, be it for Android or ChromeOS.
There's not a large community around it, unlike some of the other ARM SoCs.
I did find an old v4.4 Android kernel [1] for the MediaTek Helio P60
(MT6771) that is also paired with MT6358. There are no device tree
references to the VCN33 regulator either. Only the definition exists
in the mt6358.dtsi file, much like what we have upstream.
As far as the regulator driver goes, if it can't find a matching regulator
node, it's the same as if the node doesn't exist, and therefore the given
constraints are not ingested. If no constraints are ingested that can
turn it on, and no consumer references to enable it either, we can say
that the regulator is effectively unused.
ChenYu
[1] https://github.com/nokia-dev/android_kernel_nokia_mt6771
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 4/9] regulator: mt6358: Drop *_SSHUB regulators
2023-06-09 9:03 ` AngeloGioacchino Del Regno
@ 2023-06-12 4:45 ` Chen-Yu Tsai
2023-06-12 18:13 ` Mark Brown
2023-06-14 7:39 ` AngeloGioacchino Del Regno
0 siblings, 2 replies; 32+ messages in thread
From: Chen-Yu Tsai @ 2023-06-12 4:45 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, Gene Chen, Johnson Wang,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
On Fri, Jun 9, 2023 at 5:03 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 09/06/23 10:30, Chen-Yu Tsai ha scritto:
> > The *_SSHUB regulators are actually alternate configuration interfaces
> > for their non *_SSHUB counterparts. They are not separate regulator
> > outputs. These registers are intended for the companion processor to
> > use to configure the power rails while the main processor is sleeping.
> > They are not intended for the main operating system to use.
> >
> > Since they are not real outputs they shouldn't be modeled separately.
> > Remove them. Luckily no device tree actually uses them.
> >
>
> I'm not sure that MT6358/6366 are used only on Chromebook SoCs, and that this SSHUB
> mechanism (companion processor) is the same across all firmwares.
AFAICT from Internet sources there's also the MT6771 and MT6781, which
are used on some phones.
But what part are you concerned about? The upstream regulator driver does
not actually have any code to switch to/from normal operation and SSHUB
mode.
In a downstream kernel I found that the SSHUB mode is only used if SCP is
doing DVFS [1]. In that same kernel, the regulator driver [2] doesn't even
list the *_SSHUB versions. So if SCP DVFS is used, the regulator driver
has no idea what's going on, and can't interfere either, which I think is
actually a good thing. Only one side should have complete control of one
output.
ChenYu
[1] https://github.com/nokia-dev/android_kernel_nokia_mt6771/blob/android-9.0/drivers/misc/mediatek/scp/mt6771/scp_dvfs.c
[2] https://github.com/nokia-dev/android_kernel_nokia_mt6771/blob/android-9.0/drivers/misc/mediatek/pmic/mt6358/v1/regulator_codegen.c
> I'd like someone from MediaTek to confirm that this is valid for both Chromebook
> and Smartphone firmwares.
>
> Regards,
> Angelo
>
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> > drivers/regulator/mt6358-regulator.c | 14 --------------
> > include/linux/regulator/mt6358-regulator.h | 4 ----
> > 2 files changed, 18 deletions(-)
> >
> > diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
> > index faf6b0757019..946a251a8b3a 100644
> > --- a/drivers/regulator/mt6358-regulator.c
> > +++ b/drivers/regulator/mt6358-regulator.c
> > @@ -505,9 +505,6 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
> > MT6358_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
> > buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
> > MT6358_VCORE_VGPU_ANA_CON0, 1),
> > - MT6358_BUCK("buck_vcore_sshub", VCORE_SSHUB, 500000, 1293750, 6250,
> > - buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_SSHUB_ELR0, 0x7f,
> > - MT6358_VCORE_VGPU_ANA_CON0, 1),
> > MT6358_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
> > buck_volt_range3, 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f,
> > MT6358_VPA_ANA_CON0, 3),
> > @@ -583,10 +580,6 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
> > MT6358_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
> > buck_volt_range1, MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
> > MT6358_LDO_VSRAM_CON2, 0x7f),
> > - MT6358_LDO1("ldo_vsram_others_sshub", VSRAM_OTHERS_SSHUB, 500000,
> > - 1293750, 6250, buck_volt_range1,
> > - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f,
> > - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f),
> > MT6358_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
> > buck_volt_range1, MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
> > MT6358_LDO_VSRAM_CON3, 0x7f),
> > @@ -603,9 +596,6 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
> > MT6366_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
> > buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
> > MT6358_VCORE_VGPU_ANA_CON0, 1),
> > - MT6366_BUCK("buck_vcore_sshub", VCORE_SSHUB, 500000, 1293750, 6250,
> > - buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_SSHUB_ELR0, 0x7f,
> > - MT6358_VCORE_VGPU_ANA_CON0, 1),
> > MT6366_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
> > buck_volt_range3, 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f,
> > MT6358_VPA_ANA_CON0, 3),
> > @@ -670,10 +660,6 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
> > MT6366_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
> > buck_volt_range1, MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
> > MT6358_LDO_VSRAM_CON2, 0x7f),
> > - MT6366_LDO1("ldo_vsram_others_sshub", VSRAM_OTHERS_SSHUB, 500000,
> > - 1293750, 6250, buck_volt_range1,
> > - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f,
> > - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f),
> > MT6366_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
> > buck_volt_range1, MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
> > MT6358_LDO_VSRAM_CON3, 0x7f),
> > diff --git a/include/linux/regulator/mt6358-regulator.h b/include/linux/regulator/mt6358-regulator.h
> > index a4307cd9edd6..c71a6a9fce7a 100644
> > --- a/include/linux/regulator/mt6358-regulator.h
> > +++ b/include/linux/regulator/mt6358-regulator.h
> > @@ -47,8 +47,6 @@ enum {
> > MT6358_ID_VLDO28,
> > MT6358_ID_VAUD28,
> > MT6358_ID_VSIM2,
> > - MT6358_ID_VCORE_SSHUB,
> > - MT6358_ID_VSRAM_OTHERS_SSHUB,
> > MT6358_ID_RG_MAX,
> > };
> >
> > @@ -88,8 +86,6 @@ enum {
> > MT6366_ID_VMC,
> > MT6366_ID_VAUD28,
> > MT6366_ID_VSIM2,
> > - MT6366_ID_VCORE_SSHUB,
> > - MT6366_ID_VSRAM_OTHERS_SSHUB,
> > MT6366_ID_RG_MAX,
> > };
> >
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 3/9] regulator: mt6358: Merge VCN33_* regulators
2023-06-09 8:30 ` [PATCH 3/9] regulator: mt6358: Merge VCN33_* regulators Chen-Yu Tsai
2023-06-09 8:58 ` AngeloGioacchino Del Regno
2023-06-09 15:56 ` Conor Dooley
@ 2023-06-12 10:56 ` Fei Shao
2023-06-15 7:37 ` Chen-Yu Tsai
2 siblings, 1 reply; 32+ messages in thread
From: Fei Shao @ 2023-06-12 10:56 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
On Fri, Jun 9, 2023 at 4:30 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> The VCN33_BT and VCN33_WIFI regulators are actually the same regulator,
> having the same voltage setting and output pin. There are simply two
> enable bits that are ORed together to enable the regulator.
>
> Having two regulators representing the same output pin is misleading
> from a design matching standpoint, and also error-prone in driver
> implementations. If consumers try to set different voltages on either
> regulator, the one set later would override the one set before. There
> are ways around this, such as chaining them together and having the
> downstream one act as a switch. But given there's only one output pin,
> such a workaround doesn't match reality.
>
> Remove the VCN33_WIFI regulator. During the probe phase, have the driver
> sync the enable status of VCN33_WIFI to VCN33_BT. Also drop the suffix
> so that the regulator name matches the pin name in the datasheet.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
> drivers/regulator/mt6358-regulator.c | 65 +++++++++++++++++-----
> include/linux/regulator/mt6358-regulator.h | 6 +-
> 2 files changed, 52 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
> index c9e16bd092f6..faf6b0757019 100644
> --- a/drivers/regulator/mt6358-regulator.c
> +++ b/drivers/regulator/mt6358-regulator.c
> @@ -277,7 +277,7 @@ static const unsigned int vcama_voltages[] = {
> 2800000, 2900000, 3000000,
> };
>
> -static const unsigned int vcn33_bt_wifi_voltages[] = {
> +static const unsigned int vcn33_voltages[] = {
> 3300000, 3400000, 3500000,
> };
>
> @@ -321,7 +321,7 @@ static const u32 vcama_idx[] = {
> 0, 7, 9, 10, 11, 12,
> };
>
> -static const u32 vcn33_bt_wifi_idx[] = {
> +static const u32 vcn33_idx[] = {
> 1, 2, 3,
> };
>
> @@ -566,12 +566,8 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
> MT6358_LDO_VCAMA1_CON0, 0, MT6358_VCAMA1_ANA_CON0, 0xf00),
> MT6358_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx,
> MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
> - MT6358_LDO("ldo_vcn33_bt", VCN33_BT, vcn33_bt_wifi_voltages,
> - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_0,
> - 0, MT6358_VCN33_ANA_CON0, 0x300),
> - MT6358_LDO("ldo_vcn33_wifi", VCN33_WIFI, vcn33_bt_wifi_voltages,
> - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_1,
> - 0, MT6358_VCN33_ANA_CON0, 0x300),
> + MT6358_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx,
> + MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
> MT6358_LDO("ldo_vcama2", VCAMA2, vcama_voltages, vcama_idx,
> MT6358_LDO_VCAMA2_CON0, 0, MT6358_VCAMA2_ANA_CON0, 0xf00),
> MT6358_LDO("ldo_vmc", VMC, vmc_voltages, vmc_idx,
> @@ -662,12 +658,8 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
> MT6358_LDO_VMCH_CON0, 0, MT6358_VMCH_ANA_CON0, 0x700),
> MT6366_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx,
> MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
> - MT6366_LDO("ldo_vcn33_bt", VCN33_BT, vcn33_bt_wifi_voltages,
> - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_0,
> - 0, MT6358_VCN33_ANA_CON0, 0x300),
> - MT6366_LDO("ldo_vcn33_wifi", VCN33_WIFI, vcn33_bt_wifi_voltages,
> - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_1,
> - 0, MT6358_VCN33_ANA_CON0, 0x300),
> + MT6366_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx,
> + MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
> MT6366_LDO("ldo_vmc", VMC, vmc_voltages, vmc_idx,
> MT6358_LDO_VMC_CON0, 0, MT6358_VMC_ANA_CON0, 0xf00),
> MT6366_LDO("ldo_vsim2", VSIM2, vsim_voltages, vsim_idx,
> @@ -690,13 +682,56 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
> MT6358_LDO_VSRAM_CON1, 0x7f),
> };
>
> +static int mt6358_sync_vcn33_setting(struct device *dev)
> +{
> + struct mt6397_chip *mt6397 = dev_get_drvdata(dev->parent);
> + unsigned int val;
> + int ret;
> +
> + /*
> + * VCN33_WIFI and VCN33_BT are two separate enable bits for the same
> + * regulator. They share the same voltage setting and output pin.
> + * Instead of having two potentially conflicting regulators, just have
> + * one VCN33 regulator. Sync the two enable bits and only use one in
> + * the regulator device.
> + */
> + ret = regmap_read(mt6397->regmap, MT6358_LDO_VCN33_CON0_1, &val);
> + if (ret) {
> + dev_err(dev, "Failed to read VCN33_WIFI setting\n");
> + return ret;
> + }
> +
> + if (!(val & BIT(0)))
> + return 0;
> +
> + /* Sync VCN33_WIFI enable status to VCN33_BT */
> + ret = regmap_update_bits(mt6397->regmap, MT6358_LDO_VCN33_CON0_0, BIT(0), BIT(0));
> + if (ret) {
> + dev_err(dev, "Failed to sync VCN33_WIFI setting to VCN33_BT\n");
> + return ret;
> + }
> +
> + /* Disable VCN33_WIFI */
> + ret = regmap_update_bits(mt6397->regmap, MT6358_LDO_VCN33_CON0_1, BIT(0), 0);
> + if (ret) {
> + dev_err(dev, "Failed to disable VCN33_BT\n");
I think it should be "VCN33_WIFI" in the error message?
Regards,
Fei
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 3/9] regulator: mt6358: Merge VCN33_* regulators
2023-06-12 4:19 ` Chen-Yu Tsai
@ 2023-06-12 17:34 ` Conor Dooley
0 siblings, 0 replies; 32+ messages in thread
From: Conor Dooley @ 2023-06-12 17:34 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1452 bytes --]
On Mon, Jun 12, 2023 at 12:19:01PM +0800, Chen-Yu Tsai wrote:
> On Sat, Jun 10, 2023 at 11:28 PM Conor Dooley <conor@kernel.org> wrote:
> > Ah, I see in the binding commit there's a "Luckily no device tree actually
> > uses them." Does that just cover the kernel, or does it consider other
> > operating systems/bootloaders?
>
> That comment covers the upstream kernel and the downstream ChromeOS kernel
> specifically. The bootloader that ChromeOS uses (coreboot) doesn't use
> device trees. I don't know what MediaTek uses for their phones though.
>
> AFAIK MediaTek only supports the Linux kernel, be it for Android or ChromeOS.
> There's not a large community around it, unlike some of the other ARM SoCs.
>
> I did find an old v4.4 Android kernel [1] for the MediaTek Helio P60
> (MT6771) that is also paired with MT6358. There are no device tree
> references to the VCN33 regulator either. Only the definition exists
> in the mt6358.dtsi file, much like what we have upstream.
>
> As far as the regulator driver goes, if it can't find a matching regulator
> node, it's the same as if the node doesn't exist, and therefore the given
> constraints are not ingested. If no constraints are ingested that can
> turn it on, and no consumer references to enable it either, we can say
> that the regulator is effectively unused.
Okay, that sounds reasonable. Seems like you've done your research,
so thanks for that!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 4/9] regulator: mt6358: Drop *_SSHUB regulators
2023-06-12 4:45 ` Chen-Yu Tsai
@ 2023-06-12 18:13 ` Mark Brown
2023-06-14 7:39 ` AngeloGioacchino Del Regno
1 sibling, 0 replies; 32+ messages in thread
From: Mark Brown @ 2023-06-12 18:13 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: AngeloGioacchino Del Regno, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Matthias Brugger, Gene Chen,
Johnson Wang, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 431 bytes --]
On Mon, Jun 12, 2023 at 12:45:21PM +0800, Chen-Yu Tsai wrote:
> list the *_SSHUB versions. So if SCP DVFS is used, the regulator driver
> has no idea what's going on, and can't interfere either, which I think is
> actually a good thing. Only one side should have complete control of one
> output.
Very much so - there's a SCMI regulator interface for communicating with
SCPs where that's required (eg, for things like SD cards).
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 4/9] regulator: mt6358: Drop *_SSHUB regulators
2023-06-12 4:45 ` Chen-Yu Tsai
2023-06-12 18:13 ` Mark Brown
@ 2023-06-14 7:39 ` AngeloGioacchino Del Regno
1 sibling, 0 replies; 32+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-06-14 7:39 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, Gene Chen, Johnson Wang,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
Il 12/06/23 06:45, Chen-Yu Tsai ha scritto:
> On Fri, Jun 9, 2023 at 5:03 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 09/06/23 10:30, Chen-Yu Tsai ha scritto:
>>> The *_SSHUB regulators are actually alternate configuration interfaces
>>> for their non *_SSHUB counterparts. They are not separate regulator
>>> outputs. These registers are intended for the companion processor to
>>> use to configure the power rails while the main processor is sleeping.
>>> They are not intended for the main operating system to use.
>>>
>>> Since they are not real outputs they shouldn't be modeled separately.
>>> Remove them. Luckily no device tree actually uses them.
>>>
>>
>> I'm not sure that MT6358/6366 are used only on Chromebook SoCs, and that this SSHUB
>> mechanism (companion processor) is the same across all firmwares.
>
> AFAICT from Internet sources there's also the MT6771 and MT6781, which
> are used on some phones.
>
> But what part are you concerned about? The upstream regulator driver does
> not actually have any code to switch to/from normal operation and SSHUB
> mode.
>
> In a downstream kernel I found that the SSHUB mode is only used if SCP is
> doing DVFS [1]. In that same kernel, the regulator driver [2] doesn't even
> list the *_SSHUB versions. So if SCP DVFS is used, the regulator driver
> has no idea what's going on, and can't interfere either, which I think is
> actually a good thing. Only one side should have complete control of one
> output.
>
Ok, I'm sold! :-P
Jokes apart, thanks for clarifying. At this point, I agree with you in that this
is safe to do, so:
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
P.S.: Sorry for the late reply and thank you for the links to that old
downstream kernel.
Cheers,
Angelo
> ChenYu
>
> [1] https://github.com/nokia-dev/android_kernel_nokia_mt6771/blob/android-9.0/drivers/misc/mediatek/scp/mt6771/scp_dvfs.c
> [2] https://github.com/nokia-dev/android_kernel_nokia_mt6771/blob/android-9.0/drivers/misc/mediatek/pmic/mt6358/v1/regulator_codegen.c
>
>> I'd like someone from MediaTek to confirm that this is valid for both Chromebook
>> and Smartphone firmwares.
>>
>> Regards,
>> Angelo
>>
>>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>>> ---
>>> drivers/regulator/mt6358-regulator.c | 14 --------------
>>> include/linux/regulator/mt6358-regulator.h | 4 ----
>>> 2 files changed, 18 deletions(-)
>>>
>>> diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
>>> index faf6b0757019..946a251a8b3a 100644
>>> --- a/drivers/regulator/mt6358-regulator.c
>>> +++ b/drivers/regulator/mt6358-regulator.c
>>> @@ -505,9 +505,6 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
>>> MT6358_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
>>> buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
>>> MT6358_VCORE_VGPU_ANA_CON0, 1),
>>> - MT6358_BUCK("buck_vcore_sshub", VCORE_SSHUB, 500000, 1293750, 6250,
>>> - buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_SSHUB_ELR0, 0x7f,
>>> - MT6358_VCORE_VGPU_ANA_CON0, 1),
>>> MT6358_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
>>> buck_volt_range3, 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f,
>>> MT6358_VPA_ANA_CON0, 3),
>>> @@ -583,10 +580,6 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
>>> MT6358_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
>>> buck_volt_range1, MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
>>> MT6358_LDO_VSRAM_CON2, 0x7f),
>>> - MT6358_LDO1("ldo_vsram_others_sshub", VSRAM_OTHERS_SSHUB, 500000,
>>> - 1293750, 6250, buck_volt_range1,
>>> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f,
>>> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f),
>>> MT6358_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
>>> buck_volt_range1, MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
>>> MT6358_LDO_VSRAM_CON3, 0x7f),
>>> @@ -603,9 +596,6 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
>>> MT6366_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
>>> buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
>>> MT6358_VCORE_VGPU_ANA_CON0, 1),
>>> - MT6366_BUCK("buck_vcore_sshub", VCORE_SSHUB, 500000, 1293750, 6250,
>>> - buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_SSHUB_ELR0, 0x7f,
>>> - MT6358_VCORE_VGPU_ANA_CON0, 1),
>>> MT6366_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
>>> buck_volt_range3, 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f,
>>> MT6358_VPA_ANA_CON0, 3),
>>> @@ -670,10 +660,6 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
>>> MT6366_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
>>> buck_volt_range1, MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
>>> MT6358_LDO_VSRAM_CON2, 0x7f),
>>> - MT6366_LDO1("ldo_vsram_others_sshub", VSRAM_OTHERS_SSHUB, 500000,
>>> - 1293750, 6250, buck_volt_range1,
>>> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f,
>>> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f),
>>> MT6366_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
>>> buck_volt_range1, MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
>>> MT6358_LDO_VSRAM_CON3, 0x7f),
>>> diff --git a/include/linux/regulator/mt6358-regulator.h b/include/linux/regulator/mt6358-regulator.h
>>> index a4307cd9edd6..c71a6a9fce7a 100644
>>> --- a/include/linux/regulator/mt6358-regulator.h
>>> +++ b/include/linux/regulator/mt6358-regulator.h
>>> @@ -47,8 +47,6 @@ enum {
>>> MT6358_ID_VLDO28,
>>> MT6358_ID_VAUD28,
>>> MT6358_ID_VSIM2,
>>> - MT6358_ID_VCORE_SSHUB,
>>> - MT6358_ID_VSRAM_OTHERS_SSHUB,
>>> MT6358_ID_RG_MAX,
>>> };
>>>
>>> @@ -88,8 +86,6 @@ enum {
>>> MT6366_ID_VMC,
>>> MT6366_ID_VAUD28,
>>> MT6366_ID_VSIM2,
>>> - MT6366_ID_VCORE_SSHUB,
>>> - MT6366_ID_VSRAM_OTHERS_SSHUB,
>>> MT6366_ID_RG_MAX,
>>> };
>>>
>>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 7/9] regulator: mt6358: Add output voltage fine tuning to fixed regulators
2023-06-09 8:30 ` [PATCH 7/9] regulator: mt6358: Add output voltage fine tuning to fixed regulators Chen-Yu Tsai
2023-06-09 10:03 ` AngeloGioacchino Del Regno
@ 2023-06-14 16:14 ` Mark Brown
2023-06-15 3:24 ` Chen-Yu Tsai
1 sibling, 1 reply; 32+ messages in thread
From: Mark Brown @ 2023-06-14 16:14 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
Matthias Brugger, AngeloGioacchino Del Regno, devicetree,
linux-arm-kernel, linux-mediatek, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3509 bytes --]
On Fri, Jun 09, 2023 at 04:30:04PM +0800, 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.
This and the followup patch break the build on both arm64 and x86_64:
/build/stage/linux/drivers/regulator/mt6358-regulator.c:127:29: error: ‘MT6358_VFE28_ANA_CON0’ undeclared here (not in a function); did you mean ‘MT6358_VIO28_ANA_CON0’?
127 | .vsel_reg = MT6358_##vreg##_ANA_CON0, \
| ^~~~~~~
/build/stage/linux/drivers/regulator/mt6358-regulator.c:525:9: note: in expansion of macro ‘MT6358_REG_FIXED’
525 | MT6358_REG_FIXED("ldo_vfe28", VFE28, MT6358_LDO_VFE28_CON0, 0, 2800000),
| ^~~~~~~~~~~~~~~~
/build/stage/linux/drivers/regulator/mt6358-regulator.c:127:29: error: ‘MT6358_VCN28_ANA_CON0’ undeclared here (not in a function); did you mean ‘MT6358_VCN18_ANA_CON0’?
127 | .vsel_reg = MT6358_##vreg##_ANA_CON0, \
| ^~~~~~~
/build/stage/linux/drivers/regulator/mt6358-regulator.c:526:9: note: in expansion of macro ‘MT6358_REG_FIXED’
526 | MT6358_REG_FIXED("ldo_vcn28", VCN28, MT6358_LDO_VCN28_CON0, 0, 2800000),
| ^~~~~~~~~~~~~~~~
/build/stage/linux/drivers/regulator/mt6358-regulator.c:127:29: error: ‘MT6358_VXO22_ANA_CON0’ undeclared here (not in a function); did you mean ‘MT6358_VIO28_ANA_CON0’?
127 | .vsel_reg = MT6358_##vreg##_ANA_CON0, \
| ^~~~~~~
/build/stage/linux/drivers/regulator/mt6358-regulator.c:527:9: note: in expansion of macro ‘MT6358_REG_FIXED’
527 | MT6358_REG_FIXED("ldo_vxo22", VXO22, MT6358_LDO_VXO22_CON0, 0, 2200000),
| ^~~~~~~~~~~~~~~~
/build/stage/linux/drivers/regulator/mt6358-regulator.c:127:29: error: ‘MT6358_VAUX18_ANA_CON0’ undeclared here (not in a function); did you mean ‘MT6358_VRF18_ANA_CON0’?
127 | .vsel_reg = MT6358_##vreg##_ANA_CON0, \
| ^~~~~~~
/build/stage/linux/drivers/regulator/mt6358-regulator.c:528:9: note: in expansion of macro ‘MT6358_REG_FIXED’
528 | MT6358_REG_FIXED("ldo_vaux18", VAUX18,
| ^~~~~~~~~~~~~~~~
/build/stage/linux/drivers/regulator/mt6358-regulator.c:127:29: error: ‘MT6358_VBIF28_ANA_CON0’ undeclared here (not in a function); did you mean ‘MT6358_VIO28_ANA_CON0’?
127 | .vsel_reg = MT6358_##vreg##_ANA_CON0, \
| ^~~~~~~
/build/stage/linux/drivers/regulator/mt6358-regulator.c:530:9: note: in expansion of macro ‘MT6358_REG_FIXED’
530 | MT6358_REG_FIXED("ldo_vbif28", VBIF28,
| ^~~~~~~~~~~~~~~~
/build/stage/linux/drivers/regulator/mt6358-regulator.c:127:29: error: ‘MT6358_VAUD28_ANA_CON0’ undeclared here (not in a function); did you mean ‘MT6358_VA12_ANA_CON0’?
127 | .vsel_reg = MT6358_##vreg##_ANA_CON0, \
| ^~~~~~~
/build/stage/linux/drivers/regulator/mt6358-regulator.c:535:9: note: in expansion of macro ‘MT6358_REG_FIXED’
535 | MT6358_REG_FIXED("ldo_vaud28", VAUD28,
| ^~~~~~~~~~~~~~~~
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: (subset) [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements
2023-06-09 8:29 [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
` (8 preceding siblings ...)
2023-06-09 8:30 ` [PATCH 9/9] arm64: dts: mediatek: mt6358: Merge ldo_vcn33_* regulators Chen-Yu Tsai
@ 2023-06-14 18:58 ` Mark Brown
9 siblings, 0 replies; 32+ messages in thread
From: Mark Brown @ 2023-06-14 18:58 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
Matthias Brugger, AngeloGioacchino Del Regno, Chen-Yu Tsai
Cc: devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
On Fri, 09 Jun 2023 16:29:57 +0800, Chen-Yu Tsai wrote:
> This series is a cleanup and improvement of the MT6358 regulator driver.
> 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~8 should go through the regulator tree, and patch 9 through
> the soc tree. This series (patches 7 and 8) depends on "regulator: Use
> bitfield values for range selectors" [1] I sent out earlier.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[1/9] regulator: dt-bindings: mt6358: Merge ldo_vcn33_* regulators
commit: a74d4c577c60b27fc57ea734ef8275921ae8dcb2
[2/9] regulator: dt-bindings: mt6358: Drop *_sshub regulators
commit: 82f305b18eb0505444eab8ac86bfa134b67cb38e
[3/9] regulator: mt6358: Merge VCN33_* regulators
commit: 65bae54e08c109ddbbf121bb00058cf3b3fb7b8e
[4/9] regulator: mt6358: Drop *_SSHUB regulators
commit: 04ba665248ed91576d326041108e5fc2ec2254eb
[5/9] regulator: mt6358: Const-ify mt6358_regulator_info data structures
commit: 1ff35e66cae53f7090a671afddaee45d4ccd9396
[6/9] regulator: mt6358: Use linear voltage helpers for single range regulators
commit: ea861df772fd8cca715d43f62fe13c09c975f7a2
[7/9] regulator: mt6358: Add output voltage fine tuning to fixed regulators
(no commit info)
[8/9] regulator: mt6358: Add output voltage fine tuning to variable LDOs
(no commit info)
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] 32+ messages in thread
* Re: [PATCH 7/9] regulator: mt6358: Add output voltage fine tuning to fixed regulators
2023-06-14 16:14 ` Mark Brown
@ 2023-06-15 3:24 ` Chen-Yu Tsai
0 siblings, 0 replies; 32+ messages in thread
From: Chen-Yu Tsai @ 2023-06-15 3:24 UTC (permalink / raw)
To: Mark Brown
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
Matthias Brugger, AngeloGioacchino Del Regno, devicetree,
linux-arm-kernel, linux-mediatek, linux-kernel
On Thu, Jun 15, 2023 at 12:15 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Jun 09, 2023 at 04:30:04PM +0800, 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.
>
> This and the followup patch break the build on both arm64 and x86_64:
>
> /build/stage/linux/drivers/regulator/mt6358-regulator.c:127:29: error: ‘MT6358_VFE28_ANA_CON0’ undeclared here (not in a function); did you mean ‘MT6358_VIO28_ANA_CON0’?
> 127 | .vsel_reg = MT6358_##vreg##_ANA_CON0, \
> | ^~~~~~~
Argh, I sequenced the patches in my tree incorrectly. I see you already
merged the first six patches. I'll send a new version including a header
change that this patch needs, and other fixups that reviewers suggested.
ChenYu
> /build/stage/linux/drivers/regulator/mt6358-regulator.c:525:9: note: in expansion of macro ‘MT6358_REG_FIXED’
> 525 | MT6358_REG_FIXED("ldo_vfe28", VFE28, MT6358_LDO_VFE28_CON0, 0, 2800000),
> | ^~~~~~~~~~~~~~~~
> /build/stage/linux/drivers/regulator/mt6358-regulator.c:127:29: error: ‘MT6358_VCN28_ANA_CON0’ undeclared here (not in a function); did you mean ‘MT6358_VCN18_ANA_CON0’?
> 127 | .vsel_reg = MT6358_##vreg##_ANA_CON0, \
> | ^~~~~~~
> /build/stage/linux/drivers/regulator/mt6358-regulator.c:526:9: note: in expansion of macro ‘MT6358_REG_FIXED’
> 526 | MT6358_REG_FIXED("ldo_vcn28", VCN28, MT6358_LDO_VCN28_CON0, 0, 2800000),
> | ^~~~~~~~~~~~~~~~
> /build/stage/linux/drivers/regulator/mt6358-regulator.c:127:29: error: ‘MT6358_VXO22_ANA_CON0’ undeclared here (not in a function); did you mean ‘MT6358_VIO28_ANA_CON0’?
> 127 | .vsel_reg = MT6358_##vreg##_ANA_CON0, \
> | ^~~~~~~
> /build/stage/linux/drivers/regulator/mt6358-regulator.c:527:9: note: in expansion of macro ‘MT6358_REG_FIXED’
> 527 | MT6358_REG_FIXED("ldo_vxo22", VXO22, MT6358_LDO_VXO22_CON0, 0, 2200000),
> | ^~~~~~~~~~~~~~~~
> /build/stage/linux/drivers/regulator/mt6358-regulator.c:127:29: error: ‘MT6358_VAUX18_ANA_CON0’ undeclared here (not in a function); did you mean ‘MT6358_VRF18_ANA_CON0’?
> 127 | .vsel_reg = MT6358_##vreg##_ANA_CON0, \
> | ^~~~~~~
> /build/stage/linux/drivers/regulator/mt6358-regulator.c:528:9: note: in expansion of macro ‘MT6358_REG_FIXED’
> 528 | MT6358_REG_FIXED("ldo_vaux18", VAUX18,
> | ^~~~~~~~~~~~~~~~
> /build/stage/linux/drivers/regulator/mt6358-regulator.c:127:29: error: ‘MT6358_VBIF28_ANA_CON0’ undeclared here (not in a function); did you mean ‘MT6358_VIO28_ANA_CON0’?
> 127 | .vsel_reg = MT6358_##vreg##_ANA_CON0, \
> | ^~~~~~~
> /build/stage/linux/drivers/regulator/mt6358-regulator.c:530:9: note: in expansion of macro ‘MT6358_REG_FIXED’
> 530 | MT6358_REG_FIXED("ldo_vbif28", VBIF28,
> | ^~~~~~~~~~~~~~~~
> /build/stage/linux/drivers/regulator/mt6358-regulator.c:127:29: error: ‘MT6358_VAUD28_ANA_CON0’ undeclared here (not in a function); did you mean ‘MT6358_VA12_ANA_CON0’?
> 127 | .vsel_reg = MT6358_##vreg##_ANA_CON0, \
> | ^~~~~~~
> /build/stage/linux/drivers/regulator/mt6358-regulator.c:535:9: note: in expansion of macro ‘MT6358_REG_FIXED’
> 535 | MT6358_REG_FIXED("ldo_vaud28", VAUD28,
> | ^~~~~~~~~~~~~~~~
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 3/9] regulator: mt6358: Merge VCN33_* regulators
2023-06-12 10:56 ` Fei Shao
@ 2023-06-15 7:37 ` Chen-Yu Tsai
0 siblings, 0 replies; 32+ messages in thread
From: Chen-Yu Tsai @ 2023-06-15 7:37 UTC (permalink / raw)
To: Fei Shao
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
Liam Girdwood, Matthias Brugger, AngeloGioacchino Del Regno,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel
On Mon, Jun 12, 2023 at 6:57 PM Fei Shao <fshao@chromium.org> wrote:
>
> On Fri, Jun 9, 2023 at 4:30 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > The VCN33_BT and VCN33_WIFI regulators are actually the same regulator,
> > having the same voltage setting and output pin. There are simply two
> > enable bits that are ORed together to enable the regulator.
> >
> > Having two regulators representing the same output pin is misleading
> > from a design matching standpoint, and also error-prone in driver
> > implementations. If consumers try to set different voltages on either
> > regulator, the one set later would override the one set before. There
> > are ways around this, such as chaining them together and having the
> > downstream one act as a switch. But given there's only one output pin,
> > such a workaround doesn't match reality.
> >
> > Remove the VCN33_WIFI regulator. During the probe phase, have the driver
> > sync the enable status of VCN33_WIFI to VCN33_BT. Also drop the suffix
> > so that the regulator name matches the pin name in the datasheet.
> >
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> > drivers/regulator/mt6358-regulator.c | 65 +++++++++++++++++-----
> > include/linux/regulator/mt6358-regulator.h | 6 +-
> > 2 files changed, 52 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
> > index c9e16bd092f6..faf6b0757019 100644
> > --- a/drivers/regulator/mt6358-regulator.c
> > +++ b/drivers/regulator/mt6358-regulator.c
> > @@ -277,7 +277,7 @@ static const unsigned int vcama_voltages[] = {
> > 2800000, 2900000, 3000000,
> > };
> >
> > -static const unsigned int vcn33_bt_wifi_voltages[] = {
> > +static const unsigned int vcn33_voltages[] = {
> > 3300000, 3400000, 3500000,
> > };
> >
> > @@ -321,7 +321,7 @@ static const u32 vcama_idx[] = {
> > 0, 7, 9, 10, 11, 12,
> > };
> >
> > -static const u32 vcn33_bt_wifi_idx[] = {
> > +static const u32 vcn33_idx[] = {
> > 1, 2, 3,
> > };
> >
> > @@ -566,12 +566,8 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
> > MT6358_LDO_VCAMA1_CON0, 0, MT6358_VCAMA1_ANA_CON0, 0xf00),
> > MT6358_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx,
> > MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
> > - MT6358_LDO("ldo_vcn33_bt", VCN33_BT, vcn33_bt_wifi_voltages,
> > - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_0,
> > - 0, MT6358_VCN33_ANA_CON0, 0x300),
> > - MT6358_LDO("ldo_vcn33_wifi", VCN33_WIFI, vcn33_bt_wifi_voltages,
> > - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_1,
> > - 0, MT6358_VCN33_ANA_CON0, 0x300),
> > + MT6358_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx,
> > + MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
> > MT6358_LDO("ldo_vcama2", VCAMA2, vcama_voltages, vcama_idx,
> > MT6358_LDO_VCAMA2_CON0, 0, MT6358_VCAMA2_ANA_CON0, 0xf00),
> > MT6358_LDO("ldo_vmc", VMC, vmc_voltages, vmc_idx,
> > @@ -662,12 +658,8 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
> > MT6358_LDO_VMCH_CON0, 0, MT6358_VMCH_ANA_CON0, 0x700),
> > MT6366_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx,
> > MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700),
> > - MT6366_LDO("ldo_vcn33_bt", VCN33_BT, vcn33_bt_wifi_voltages,
> > - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_0,
> > - 0, MT6358_VCN33_ANA_CON0, 0x300),
> > - MT6366_LDO("ldo_vcn33_wifi", VCN33_WIFI, vcn33_bt_wifi_voltages,
> > - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_1,
> > - 0, MT6358_VCN33_ANA_CON0, 0x300),
> > + MT6366_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx,
> > + MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300),
> > MT6366_LDO("ldo_vmc", VMC, vmc_voltages, vmc_idx,
> > MT6358_LDO_VMC_CON0, 0, MT6358_VMC_ANA_CON0, 0xf00),
> > MT6366_LDO("ldo_vsim2", VSIM2, vsim_voltages, vsim_idx,
> > @@ -690,13 +682,56 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
> > MT6358_LDO_VSRAM_CON1, 0x7f),
> > };
> >
> > +static int mt6358_sync_vcn33_setting(struct device *dev)
> > +{
> > + struct mt6397_chip *mt6397 = dev_get_drvdata(dev->parent);
> > + unsigned int val;
> > + int ret;
> > +
> > + /*
> > + * VCN33_WIFI and VCN33_BT are two separate enable bits for the same
> > + * regulator. They share the same voltage setting and output pin.
> > + * Instead of having two potentially conflicting regulators, just have
> > + * one VCN33 regulator. Sync the two enable bits and only use one in
> > + * the regulator device.
> > + */
> > + ret = regmap_read(mt6397->regmap, MT6358_LDO_VCN33_CON0_1, &val);
> > + if (ret) {
> > + dev_err(dev, "Failed to read VCN33_WIFI setting\n");
> > + return ret;
> > + }
> > +
> > + if (!(val & BIT(0)))
> > + return 0;
> > +
> > + /* Sync VCN33_WIFI enable status to VCN33_BT */
> > + ret = regmap_update_bits(mt6397->regmap, MT6358_LDO_VCN33_CON0_0, BIT(0), BIT(0));
> > + if (ret) {
> > + dev_err(dev, "Failed to sync VCN33_WIFI setting to VCN33_BT\n");
> > + return ret;
> > + }
> > +
> > + /* Disable VCN33_WIFI */
> > + ret = regmap_update_bits(mt6397->regmap, MT6358_LDO_VCN33_CON0_1, BIT(0), 0);
> > + if (ret) {
> > + dev_err(dev, "Failed to disable VCN33_BT\n");
>
> I think it should be "VCN33_WIFI" in the error message?
Mark already merged the patch. I send followup fixes for this and the
call sequence.
ChenYu
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2023-06-15 7:38 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-09 8:29 [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements Chen-Yu Tsai
2023-06-09 8:29 ` [PATCH 1/9] regulator: dt-bindings: mt6358: Merge ldo_vcn33_* regulators Chen-Yu Tsai
2023-06-09 15:47 ` Matthias Brugger
2023-06-09 16:02 ` Krzysztof Kozlowski
2023-06-09 8:29 ` [PATCH 2/9] regulator: dt-bindings: mt6358: Drop *_sshub regulators Chen-Yu Tsai
2023-06-09 15:47 ` Matthias Brugger
2023-06-09 16:02 ` Krzysztof Kozlowski
2023-06-09 8:30 ` [PATCH 3/9] regulator: mt6358: Merge VCN33_* regulators Chen-Yu Tsai
2023-06-09 8:58 ` AngeloGioacchino Del Regno
2023-06-12 3:41 ` Chen-Yu Tsai
2023-06-09 15:56 ` Conor Dooley
2023-06-10 15:28 ` Conor Dooley
2023-06-12 4:19 ` Chen-Yu Tsai
2023-06-12 17:34 ` Conor Dooley
2023-06-12 10:56 ` Fei Shao
2023-06-15 7:37 ` Chen-Yu Tsai
2023-06-09 8:30 ` [PATCH 4/9] regulator: mt6358: Drop *_SSHUB regulators Chen-Yu Tsai
2023-06-09 9:03 ` AngeloGioacchino Del Regno
2023-06-12 4:45 ` Chen-Yu Tsai
2023-06-12 18:13 ` Mark Brown
2023-06-14 7:39 ` AngeloGioacchino Del Regno
2023-06-09 15:52 ` Matthias Brugger
2023-06-09 8:30 ` [PATCH 5/9] regulator: mt6358: Const-ify mt6358_regulator_info data structures Chen-Yu Tsai
2023-06-09 8:30 ` [PATCH 6/9] regulator: mt6358: Use linear voltage helpers for single range regulators Chen-Yu Tsai
2023-06-09 8:30 ` [PATCH 7/9] regulator: mt6358: Add output voltage fine tuning to fixed regulators Chen-Yu Tsai
2023-06-09 10:03 ` AngeloGioacchino Del Regno
2023-06-14 16:14 ` Mark Brown
2023-06-15 3:24 ` Chen-Yu Tsai
2023-06-09 8:30 ` [PATCH 8/9] regulator: mt6358: Add output voltage fine tuning to variable LDOs Chen-Yu Tsai
2023-06-09 10:03 ` AngeloGioacchino Del Regno
2023-06-09 8:30 ` [PATCH 9/9] arm64: dts: mediatek: mt6358: Merge ldo_vcn33_* regulators Chen-Yu Tsai
2023-06-14 18:58 ` (subset) [PATCH 0/9] regulator: mt6358: Remove bogus regulators and improvements Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).