[RFC PATCH 0/5] mfd: axp20x: program poly-phased regulators

[RFC PATCH 0/5] mfd: axp20x: program poly-phased regulators

[Andre Przywara]

Hi,

this series introduces support for X-Power AXP PMICs to actively control
poly-phase configurations. Some AXP PMICs allow to join two or more voltage
rails together, under the PMIC's control, to achieve higher currents and
reduced ripple, by offsetting each rail's phase.

So far we were just querying the active poly-phase configuration, as all
boards were using the reset default setup, which matched the board's
requirements. This information was just used to leave the coupled regulator
alone, as its configuration is synchronised internally to the "leading"
regulator.

Now boards with the A523 family of SoCs often come with an AXP323
dual-phase regulator, but use a single-phase reset default. However this
regulator pair supplies the "big" CPU cluster, so definitely needs a
dual-phase setup. Ignoring that leads to an immediate hang as soon as the
cluster leaves the safe 792 MHz/900mv OPP.

Looking at various datasheets of AXP PMICs reveals that there exist
dual-phase, but also triple-phase setups, although the latter is only used
by one PMIC at the moment. To be future proof, the following DT binding
design is proposed:
- The "x-powers,polyphased" property is introduced to list phandles of
  regulators that are phased together. Only the lead regulator needs
  to use that property:
	regulators {
		reg_dcdc1: dcdc1 {
			regulator-always-on;
			regulator-min-microvolt = <900000>;
			regulator-max-microvolt = <1220000>;
			regulator-name = "vdd-cpub";
			x-powers,polyphased = <&reg_dcdc2>;
		};

		reg_dcdc2: dcdc2 {
			/* dual-phased to DCDC1 */
		};
	};
- If the "x-powers,polyphased" property exists, but the list is empty
  (like a boolean property), the regulator will be removed from the
  poly-phase setup, so the affected regulators become independent.
- If the "x-powers,polyphased" property is absent, the existing poly-phase
  configuration will not be touched. This preserves backwards compatibility
  with the current situation.

Some off-tree downstream patches use a temptingly simpler solution: the
secondary rails just list a boolean property, to opt in to a poly-phase
setup. Besides becoming slightly ambiguous in a true multiphase
configuration, with three or more regulators involved, this has the big
disadvantage of not being backwards compatible: a missing property might
let the driver turn the poly-phase setup off, potentially breaking
existing setups.
The design proposed above avoids this problem by explicitly pointing out
all participating regulator rails, at the cost of some more code
complexity.

This series introduces the DT binding changes in patch 1/5.
Patch 2/5 refactors the current poly-phase *detection* code, to introduce
a data structure describing the known pairs. This should be considered
regardless of the rest of the patches, as a clean up.
Patch 3/5 adds support for connecting dual-phase regulators, patch 4/5
extends this to cover the one used tri-phase special case.
The final patch adds the property to the A527/T527 board DTs that need it.

This is a prerequisite for the upcoming DVFS support for the A523 family
of SoCs, as all those boards so far require to actively enable the
dual-phase bit.

I am marking this as RFC, to get opinions on those questions:
- Is the proposed DT binding a good solution? Shall the property name
  be more generic? Is there an existing solution to this problem?
- Is there a better, more generic way to describe poly-phased regulators
  in a data structure? The one proposed in patch 2/5 just supports
  dual-phased regulators, for simplicity.
- Do we need support for the special-cased tri-phase setup, as seen in
  patch 4/5? This is coded as some special case, but technically the
  boards using this tri-phase setup (Allwinner A80 based boards) are
  happy with the PMIC reset default.

I would be grateful to get some feedback on this, so that we can push
for a solution to eventually enable DVFS for the A523 family of SoCs.

Based on v6.17-rc1.

Cheers,
Andre

Andre Przywara (5):
  dt-bindings: mfd: x-powers,axp152: Add polyphased property
  mfd: axp20x: Refactor axp20x_is_polyphase_slave()
  mfd: axp20x: Allow programming dual-phase regulator pairs
  mfd: axp20x: Support tri-phase setup
  arm64: dts: allwinner: a523: Mark dual-phased regulators

 .../bindings/mfd/x-powers,axp152.yaml         |  14 ++
 .../dts/allwinner/sun55i-a527-cubie-a5e.dts   |   5 +-
 .../dts/allwinner/sun55i-h728-x96qpro+.dts    |   5 +-
 .../dts/allwinner/sun55i-t527-avaota-a1.dts   |   5 +-
 .../dts/allwinner/sun55i-t527-orangepi-4a.dts |   5 +-
 drivers/regulator/axp20x-regulator.c          | 190 ++++++++++++++----
 6 files changed, 181 insertions(+), 43 deletions(-)

-- 
2.46.4

[RFC PATCH 1/5] dt-bindings: mfd: x-powers,axp152: Add polyphased property

[Andre Przywara]

Some X-Powers AXP PMICs can combine some of their DC/DC buck converter
outputs in a multi-phase fashion, to achieve higher currents and
decrease the output ripple. The datasheets call this poly-phase. This is
programmable in the PMIC, although often set up as the PMIC's reset
default.

Add the "x-powers,polyphased" property to the binding, to describe those
pairs or tuples of regulators that should work together. In the lead
regulator node, the property lists the phandles of the connected
regulators. Just an empty property means no poly-phasing.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 .../devicetree/bindings/mfd/x-powers,axp152.yaml   | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
index 45f015d63df16..260c4c0afc475 100644
--- a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
+++ b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
@@ -304,6 +304,15 @@ properties:
               noise. This probably makes sense for HiFi audio related
               applications that aren't battery constrained.
 
+          x-powers,polyphased:
+            $ref: /schemas/types.yaml#/definitions/phandle-array
+            description:
+              A list of phandles pointing to other regulators that should be
+              polyphased with this regulator. The linked regulators will be
+              synchronised with this regulator, within the PMIC, but only if
+              supported by the PMIC. An empty list means this regulator
+              should be configured in a single-phase setup.
+
     additionalProperties: false
 
 required:
@@ -377,6 +386,7 @@ examples:
                     regulator-min-microvolt = <1000000>;
                     regulator-max-microvolt = <1450000>;
                     regulator-name = "vdd-cpu";
+                    x-powers,polyphased = <&reg_dcdc4>;
                 };
 
                 reg_dcdc3: dcdc3 {
@@ -386,6 +396,10 @@ examples:
                     regulator-name = "vdd-int-dll";
                 };
 
+                reg_dcdc4: dcdc4 {
+                    /* dual-phased with DCDC2 */
+                };
+
                 reg_ldo1: ldo1 {
                     /* LDO1 is a fixed output regulator */
                     regulator-always-on;
-- 
2.46.4

[RFC PATCH 2/5] mfd: axp20x: Refactor axp20x_is_polyphase_slave()

[Andre Przywara]

Some X-Powers AXP PMICs allow to combine certain DC/DC rails together in
a multi-phase fashion. So far we don't actively program those
connections, but we detect the existing setup, and prevent the connected
regulators from being re-programmed or turned off. At the moment this is
done in a switch/case construct, listing the known regulator pairs for
those PMICs supported.

To get rid of this ever growing code section, create a data structure
that describes the relationship, and have generic code that iterates
over the entries and checks for matches.

This not only cleans that function up and makes extensions much simpler,
but also allows to reuse this information for the upcoming programming
of those poly-phase setups.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/regulator/axp20x-regulator.c | 91 ++++++++++++++--------------
 1 file changed, 45 insertions(+), 46 deletions(-)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index da891415efc0b..19c9a98d1835a 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -1481,70 +1481,69 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
 	return regmap_update_bits(rdev->regmap, reg, mask, workmode);
 }
 
+struct dualphase_regulator {
+	int axp_id;
+	int reg1, reg2;
+	unsigned int polyphase_reg;
+	unsigned int bitmask;
+} dualphase_regulators[] = {
+	{ AXP323_ID, AXP313A_DCDC1, AXP313A_DCDC2,
+		AXP323_DCDC_MODE_CTRL2, BIT(1), },
+	{ AXP803_ID, AXP803_DCDC2, AXP803_DCDC3, AXP803_POLYPHASE_CTRL,
+		AXP803_DCDC23_POLYPHASE_DUAL, },
+	{ AXP803_ID, AXP803_DCDC5, AXP803_DCDC6, AXP803_POLYPHASE_CTRL,
+		AXP803_DCDC56_POLYPHASE_DUAL, },
+	/* AXP806's DCDC-A/B/C is a tri-phase regulator */
+	{ AXP806_ID, AXP806_DCDCD, AXP806_DCDCE, AXP806_DCDC_MODE_CTRL2,
+		AXP806_DCDCDE_POLYPHASE_DUAL, },
+	{ AXP813_ID, AXP803_DCDC2, AXP803_DCDC3, AXP803_POLYPHASE_CTRL,
+		AXP803_DCDC23_POLYPHASE_DUAL, },
+	{ AXP813_ID, AXP803_DCDC5, AXP803_DCDC6, AXP803_POLYPHASE_CTRL,
+		AXP803_DCDC56_POLYPHASE_DUAL, },
+	{ AXP15060_ID, AXP15060_DCDC2, AXP15060_DCDC3, AXP15060_DCDC_MODE_CTRL1,
+		AXP15060_DCDC23_POLYPHASE_DUAL_MASK, },
+	{ AXP15060_ID, AXP15060_DCDC4, AXP15060_DCDC6, AXP15060_DCDC_MODE_CTRL1,
+		AXP15060_DCDC46_POLYPHASE_DUAL_MASK, },
+};
+
 /*
  * This function checks whether a regulator is part of a poly-phase
  * output setup based on the registers settings. Returns true if it is.
  */
 static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
 {
+	struct dualphase_regulator *dpreg;
 	u32 reg = 0;
+	int i;
 
-	/*
-	 * Currently in our supported AXP variants, only AXP803, AXP806,
-	 * AXP813 and AXP15060 have polyphase regulators.
-	 */
-	switch (axp20x->variant) {
-	case AXP803_ID:
-	case AXP813_ID:
-		regmap_read(axp20x->regmap, AXP803_POLYPHASE_CTRL, &reg);
+	for (i = 0; i < ARRAY_SIZE(dualphase_regulators); i++) {
+		dpreg = &dualphase_regulators[i];
 
-		switch (id) {
-		case AXP803_DCDC3:
-			return !!(reg & AXP803_DCDC23_POLYPHASE_DUAL);
-		case AXP803_DCDC6:
-			return !!(reg & AXP803_DCDC56_POLYPHASE_DUAL);
+		if (axp20x->variant != dpreg->axp_id)
+			continue;
+		/* Is this the second regulator from a dual-phase pair? */
+		if (id == dpreg->reg2) {
+			regmap_read(axp20x->regmap, dpreg->polyphase_reg, &reg);
+
+			return !!(reg & dpreg->bitmask);
 		}
-		break;
+	}
 
-	case AXP806_ID:
+	/*
+	 * DCDC-A/B/C can be configured either as a dual-phase (A+B) or
+	 * as a triple-phase regulator (A+B+C), but not in any other
+	 * combination. Treat this as a special case here.
+	 */
+	if (axp20x->variant == AXP806_ID) {
 		regmap_read(axp20x->regmap, AXP806_DCDC_MODE_CTRL2, &reg);
-
-		switch (id) {
-		case AXP806_DCDCB:
+		if (id == AXP806_DCDCB)
 			return (((reg & AXP806_DCDCABC_POLYPHASE_MASK) ==
 				AXP806_DCDCAB_POLYPHASE_DUAL) ||
 				((reg & AXP806_DCDCABC_POLYPHASE_MASK) ==
 				AXP806_DCDCABC_POLYPHASE_TRI));
-		case AXP806_DCDCC:
+		if (id == AXP806_DCDCC)
 			return ((reg & AXP806_DCDCABC_POLYPHASE_MASK) ==
 				AXP806_DCDCABC_POLYPHASE_TRI);
-		case AXP806_DCDCE:
-			return !!(reg & AXP806_DCDCDE_POLYPHASE_DUAL);
-		}
-		break;
-
-	case AXP15060_ID:
-		regmap_read(axp20x->regmap, AXP15060_DCDC_MODE_CTRL1, &reg);
-
-		switch (id) {
-		case AXP15060_DCDC3:
-			return !!(reg & AXP15060_DCDC23_POLYPHASE_DUAL_MASK);
-		case AXP15060_DCDC6:
-			return !!(reg & AXP15060_DCDC46_POLYPHASE_DUAL_MASK);
-		}
-		break;
-
-	case AXP323_ID:
-		regmap_read(axp20x->regmap, AXP323_DCDC_MODE_CTRL2, &reg);
-
-		switch (id) {
-		case AXP313A_DCDC2:
-			return !!(reg & BIT(1));
-		}
-		break;
-
-	default:
-		return false;
 	}
 
 	return false;
-- 
2.46.4

[RFC PATCH 3/5] mfd: axp20x: Allow programming dual-phase regulator pairs

[Andre Przywara]

Some X-Powers AXP PMICs allow to combine certain DC/DC rails together in
a multi-phase fashion. So far we don't actively program those connections,
since the PMIC reset default for the multi-phasing setup was always
correct for the existing boards.
Now a new set of boards appeared where the reset default is not correct,
so we need to actively program the multi-phase setup.

Use the new data structure describing the dual-phased regulators, and
the new "x-powers,polyphased" DT property to enable or disable the
dual-phase setup on the PMICs that support it.

This works by checking how many regulators this DT property list:
- If it's none, this means any existing poly-phase setup should be broken
up.
- If the property references at least one other regulator, we can use our
dual-phase regulator table to find the register and bitmask required to
establish the dual-phase connection.

This supports only dual-phased regulator pairs so far, but we will
somewhat paper ov^W^W fix this in the next patch.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/regulator/axp20x-regulator.c | 68 ++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 19c9a98d1835a..e3acc4635a0ed 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -1549,6 +1549,70 @@ static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
 	return false;
 }
 
+static int axp20x_find_polyphased_reg(const struct regulator_desc *regs,
+				      int nregulators,
+				      const struct device_node *np, int index)
+{
+	struct of_phandle_args args;
+	int ret, i;
+
+	ret = of_parse_phandle_with_fixed_args(np, "x-powers,polyphased",
+						   0, index, &args);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < nregulators; i++) {
+		if (!strcmp(regs[i].name, args.np->name))
+			return i;
+	}
+
+	return -ENODEV;
+}
+
+static int axp20x_parse_polyphase(struct axp20x_dev *axp20x, int primary_reg_id,
+				  const struct regulator_desc *regs,
+				  int nregulators, const struct device_node *np)
+{
+	struct dualphase_regulator *dpreg;
+	int reg_id, i;
+
+	if (!of_property_present(np, "x-powers,polyphased"))
+		return 0;
+
+	reg_id = axp20x_find_polyphased_reg(regs, nregulators, np, 0);
+	if (reg_id < 0 && reg_id != -ENOENT)	/* not just empty property */
+		return reg_id;
+
+	for (i = 0; i < ARRAY_SIZE(dualphase_regulators); i++) {
+		dpreg = &dualphase_regulators[i];
+
+		if (axp20x->variant != dpreg->axp_id)
+			continue;
+
+		if (dpreg->reg1 != primary_reg_id &&
+		    dpreg->reg2 != primary_reg_id)
+			continue;
+
+		/* Empty property means breaking any polyphase setup. */
+		if (reg_id == -ENOENT) {
+			regmap_update_bits(axp20x->regmap, dpreg->polyphase_reg,
+					   dpreg->bitmask, 0);
+
+			return 0;
+		}
+
+		if ((dpreg->reg1 == primary_reg_id && dpreg->reg2 == reg_id) ||
+		    (dpreg->reg2 == primary_reg_id && dpreg->reg1 == reg_id)) {
+			regmap_update_bits(axp20x->regmap, dpreg->polyphase_reg,
+					   dpreg->bitmask, dpreg->bitmask);
+
+			return 0;
+		}
+	}
+
+	return 0;
+}
+
 static int axp20x_regulator_probe(struct platform_device *pdev)
 {
 	struct regulator_dev *rdev;
@@ -1703,6 +1767,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 					rdev->desc->name);
 		}
 
+		if (rdev->dev.of_node)
+			axp20x_parse_polyphase(axp20x, i, regulators,
+					       nregulators, rdev->dev.of_node);
+
 		/*
 		 * Save AXP22X DCDC1 / DCDC5 / AXP15060 ALDO1 regulator names for later.
 		 */
-- 
2.46.4

[RFC PATCH 4/5] mfd: axp20x: Support tri-phase setup

[Andre Przywara]

Of the PMICs that support multi-phased regulators, all but one just
support a dual-phase setup, with exactly two regulators tied together.
This allows for a simple data model, since just two is somewhat of a
special case.

However there is the AXP806, which supports a triple-phase setup, that is
also used on at least one board: the Cubieboard 4, where DCDC-A+B+C
together supply the Cortex-A15 CPU cluster.
Since this is just one case, and a fairly old one now, let's not boil
the ocean by coming up with a complex data structure that allows
describing arbitrary combinations, but instead handle this as a special
case. This is supported by the fact, that the AXP806 only supports two
specific setups: DCDC-A+B or DCDC-A+B+C, but nothing else.

Add a function that checks for the regulators on this PMIC, and handle
the two cases, plus the one without any poly-phasing.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/regulator/axp20x-regulator.c | 45 ++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index e3acc4635a0ed..9dd666f228b1e 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -1569,6 +1569,39 @@ static int axp20x_find_polyphased_reg(const struct regulator_desc *regs,
 	return -ENODEV;
 }
 
+static int axp20x_handle_triphase(struct axp20x_dev *axp20x,
+				  int reg1, int reg2, int reg3)
+{
+	if (axp20x->variant == AXP806_ID && reg1 == AXP806_DCDCA) {
+		/* no other regulator listed: single phase setup */
+		if (reg2 == -ENOENT && reg3 == -ENOENT) {
+			regmap_update_bits(axp20x->regmap,
+					   AXP806_DCDC_MODE_CTRL2,
+					   AXP806_DCDCABC_POLYPHASE_MASK, 0);
+			return 0;
+		}
+		/* only regulator listed is DCDC-B: dual phase setup */
+		if (reg2 == AXP806_DCDCB && reg3 == -ENOENT) {
+			regmap_update_bits(axp20x->regmap,
+					   AXP806_DCDC_MODE_CTRL2,
+					   AXP806_DCDCABC_POLYPHASE_MASK,
+					   AXP806_DCDCAB_POLYPHASE_DUAL);
+			return 0;
+		}
+		/* both DCDC-B+C regulators listed: tri phase setup */
+		if ((reg2 == AXP806_DCDCB && reg3 == AXP806_DCDCC) ||
+		    (reg2 == AXP806_DCDCC && reg3 == AXP806_DCDCB)) {
+			regmap_update_bits(axp20x->regmap,
+					   AXP806_DCDC_MODE_CTRL2,
+					   AXP806_DCDCABC_POLYPHASE_MASK,
+					   AXP806_DCDCABC_POLYPHASE_TRI);
+			return 0;
+		}
+	}
+
+	return 0;
+}
+
 static int axp20x_parse_polyphase(struct axp20x_dev *axp20x, int primary_reg_id,
 				  const struct regulator_desc *regs,
 				  int nregulators, const struct device_node *np)
@@ -1610,6 +1643,18 @@ static int axp20x_parse_polyphase(struct axp20x_dev *axp20x, int primary_reg_id,
 		}
 	}
 
+	/* Special handling for the AXP806 DCDC-A/B/C tri-phase regulator. */
+	if (axp20x->variant == AXP806_ID && primary_reg_id == AXP806_DCDCA) {
+		int reg3_id;
+
+		reg3_id = axp20x_find_polyphased_reg(regs, nregulators, np, 1);
+		if (reg3_id < 0 && reg3_id != -ENOENT)
+			return reg_id;
+
+		return axp20x_handle_triphase(axp20x, primary_reg_id,
+					      reg_id, reg3_id);
+	}
+
 	return 0;
 }
 
-- 
2.46.4

[RFC PATCH 5/5] arm64: dts: allwinner: a523: Mark dual-phased regulators

[Andre Przywara]

The X-Powers AXP323 PMIC on the boards with a SoC from the Allwinner
A523 family typically uses DCDC1 and DCDC2 in a dual-phase setup to
supply the "big" CPU cluster. For some reason this dual-phase
configuration is not the PMIC's reset default, but needs to be actively
programmed at runtime.

Add the newly introduced x-powers,polyphased property in the board DTs,
to mark this connection and let drivers program the dual-phase setup.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts   | 5 ++++-
 arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dts    | 5 ++++-
 arch/arm64/boot/dts/allwinner/sun55i-t527-avaota-a1.dts   | 5 ++++-
 arch/arm64/boot/dts/allwinner/sun55i-t527-orangepi-4a.dts | 5 ++++-
 4 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts b/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
index 4ad91b6f01d34..a51446482927c 100644
--- a/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
+++ b/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
@@ -269,9 +269,12 @@ reg_dcdc1_323: dcdc1 {
 				regulator-min-microvolt = <900000>;
 				regulator-max-microvolt = <1160000>;
 				regulator-name = "vdd-cpub";
+				x-powers,polyphased = <&reg_dcdc2_323>;
 			};
 
-			/* DCDC2 is polyphased with DCDC1 */
+			reg_dcdc2_323: dcdc2 {
+				/* dual-phased with DCDC1 */
+			};
 
 			/* RISC-V management core supply */
 			reg_dcdc3_323: dcdc3 {
diff --git a/arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dts b/arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dts
index 68c5765c2e919..848b5abb4203f 100644
--- a/arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dts
+++ b/arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dts
@@ -285,9 +285,12 @@ reg_dcdc1_323: dcdc1 {
 				regulator-min-microvolt = <900000>;
 				regulator-max-microvolt = <1160000>;
 				regulator-name = "vdd-cpub";
+				x-powers,polyphased = <&reg_dcdc2_323>;
 			};
 
-			/* DCDC2 is polyphased with DCDC1 */
+			reg_dcdc2_323: dcdc2 {
+				/* dual-phased with DCDC1 */
+			};
 
 			reg_dcdc3_323: dcdc3 {
 				regulator-always-on;
diff --git a/arch/arm64/boot/dts/allwinner/sun55i-t527-avaota-a1.dts b/arch/arm64/boot/dts/allwinner/sun55i-t527-avaota-a1.dts
index 7b7ef54ec7684..ec69b409ac47f 100644
--- a/arch/arm64/boot/dts/allwinner/sun55i-t527-avaota-a1.dts
+++ b/arch/arm64/boot/dts/allwinner/sun55i-t527-avaota-a1.dts
@@ -291,9 +291,12 @@ reg_dcdc1_323: dcdc1 {
 				regulator-min-microvolt = <900000>;
 				regulator-max-microvolt = <1160000>;
 				regulator-name = "vdd-cpub";
+				x-powers,polyphased = <&reg_dcdc2_323>;
 			};
 
-			/* DCDC2 is polyphased with DCDC1 */
+			reg_dcdc2_323: dcdc2 {
+				/* dual-phased with DCDC1 */
+			};
 
 			/* Some RISC-V management core related voltage */
 			reg_dcdc3_323: dcdc3 {
diff --git a/arch/arm64/boot/dts/allwinner/sun55i-t527-orangepi-4a.dts b/arch/arm64/boot/dts/allwinner/sun55i-t527-orangepi-4a.dts
index d07bb9193b438..e9e6d85fb84f7 100644
--- a/arch/arm64/boot/dts/allwinner/sun55i-t527-orangepi-4a.dts
+++ b/arch/arm64/boot/dts/allwinner/sun55i-t527-orangepi-4a.dts
@@ -322,9 +322,12 @@ reg_dcdc1_323: dcdc1 {
 				regulator-min-microvolt = <900000>;
 				regulator-max-microvolt = <1150000>;
 				regulator-name = "vdd-cpub";
+				x-powers,polyphased = <&reg_dcdc2_323>;
 			};
 
-			/* DCDC2 is polyphased with DCDC1 */
+			reg_dcdc2_323: dcdc2 {
+				/* dual-phased with DCDC1 */
+			};
 
 			/* Some RISC-V management core related voltage */
 			reg_dcdc3_323: dcdc3 {
-- 
2.46.4

Re: [RFC PATCH 4/5] mfd: axp20x: Support tri-phase setup

[Mark Brown]

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

On Fri, Sep 19, 2025 at 01:00:19AM +0100, Andre Przywara wrote:
> Of the PMICs that support multi-phased regulators, all but one just
> support a dual-phase setup, with exactly two regulators tied together.
> This allows for a simple data model, since just two is somewhat of a
> special case.

Acked-by: Mark Brown <broonie@kernel.org>

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

Re: [RFC PATCH 4/5] mfd: axp20x: Support tri-phase setup

[Mark Brown]

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

On Fri, Sep 19, 2025 at 01:00:19AM +0100, Andre Przywara wrote:
> Of the PMICs that support multi-phased regulators, all but one just
> support a dual-phase setup, with exactly two regulators tied together.
> This allows for a simple data model, since just two is somewhat of a
> special case.

Oh, and:

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

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

Re: [RFC PATCH 1/5] dt-bindings: mfd: x-powers,axp152: Add polyphased property

[Rob Herring]

On Fri, Sep 19, 2025 at 01:00:16AM +0100, Andre Przywara wrote:
> Some X-Powers AXP PMICs can combine some of their DC/DC buck converter
> outputs in a multi-phase fashion, to achieve higher currents and
> decrease the output ripple. The datasheets call this poly-phase. This is
> programmable in the PMIC, although often set up as the PMIC's reset
> default.
> 
> Add the "x-powers,polyphased" property to the binding, to describe those
> pairs or tuples of regulators that should work together. In the lead
> regulator node, the property lists the phandles of the connected
> regulators. Just an empty property means no poly-phasing.

Don't we have a coupled regulator binding already?

> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../devicetree/bindings/mfd/x-powers,axp152.yaml   | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> index 45f015d63df16..260c4c0afc475 100644
> --- a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> +++ b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> @@ -304,6 +304,15 @@ properties:
>                noise. This probably makes sense for HiFi audio related
>                applications that aren't battery constrained.
>  
> +          x-powers,polyphased:
> +            $ref: /schemas/types.yaml#/definitions/phandle-array
> +            description:
> +              A list of phandles pointing to other regulators that should be
> +              polyphased with this regulator. The linked regulators will be
> +              synchronised with this regulator, within the PMIC, but only if
> +              supported by the PMIC. An empty list means this regulator
> +              should be configured in a single-phase setup.

phandle-array is poorly named and is really a matrix because you can 
have arg cells. So you need:

items:
  maxItems: 1

And is there an outer max for linked regulators?

Rob

Re: [RFC PATCH 1/5] dt-bindings: mfd: x-powers,axp152: Add polyphased property

[Chen-Yu Tsai]

On Tue, Sep 23, 2025 at 2:16 AM Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Sep 19, 2025 at 01:00:16AM +0100, Andre Przywara wrote:
> > Some X-Powers AXP PMICs can combine some of their DC/DC buck converter
> > outputs in a multi-phase fashion, to achieve higher currents and
> > decrease the output ripple. The datasheets call this poly-phase. This is
> > programmable in the PMIC, although often set up as the PMIC's reset
> > default.
> >
> > Add the "x-powers,polyphased" property to the binding, to describe those
> > pairs or tuples of regulators that should work together. In the lead
> > regulator node, the property lists the phandles of the connected
> > regulators. Just an empty property means no poly-phasing.
>
> Don't we have a coupled regulator binding already?

That was my first thought as well.

Though on the driver side, I guess we would need to add a coupler to the
axp regulator driver to handle the polyphase case directly, or somehow
tell the regulator core to ignore this property.

Unlike separate regulators that are ganged together, in the AXP PMICs it
seems that when buck outputs are ganged, only the controls for the first
output have any actual effect. In such cases I don't know if we should
just ignore / leave out the secondary outputs from both the description
and the runtime state.


ChenYu


> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  .../devicetree/bindings/mfd/x-powers,axp152.yaml   | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> > index 45f015d63df16..260c4c0afc475 100644
> > --- a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> > @@ -304,6 +304,15 @@ properties:
> >                noise. This probably makes sense for HiFi audio related
> >                applications that aren't battery constrained.
> >
> > +          x-powers,polyphased:
> > +            $ref: /schemas/types.yaml#/definitions/phandle-array
> > +            description:
> > +              A list of phandles pointing to other regulators that should be
> > +              polyphased with this regulator. The linked regulators will be
> > +              synchronised with this regulator, within the PMIC, but only if
> > +              supported by the PMIC. An empty list means this regulator
> > +              should be configured in a single-phase setup.
>
> phandle-array is poorly named and is really a matrix because you can
> have arg cells. So you need:
>
> items:
>   maxItems: 1
>
> And is there an outer max for linked regulators?
>
> Rob
>

Re: [RFC PATCH 1/5] dt-bindings: mfd: x-powers,axp152: Add polyphased property

[Mark Brown]

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

On Thu, Sep 25, 2025 at 10:52:45PM +0800, Chen-Yu Tsai wrote:
> On Tue, Sep 23, 2025 at 2:16 AM Rob Herring <robh@kernel.org> wrote:
> > On Fri, Sep 19, 2025 at 01:00:16AM +0100, Andre Przywara wrote:

> > > Add the "x-powers,polyphased" property to the binding, to describe those
> > > pairs or tuples of regulators that should work together. In the lead
> > > regulator node, the property lists the phandles of the connected
> > > regulators. Just an empty property means no poly-phasing.

> > Don't we have a coupled regulator binding already?

> That was my first thought as well.

That's something separate, that's two separate regulators that need to
be kept within a range of each other manually.

> Unlike separate regulators that are ganged together, in the AXP PMICs it
> seems that when buck outputs are ganged, only the controls for the first
> output have any actual effect. In such cases I don't know if we should
> just ignore / leave out the secondary outputs from both the description
> and the runtime state.

This is very common, I would expect any regulators that are ganged
together into a single regulator like this to simply not appear in the
DT.

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

Re: [RFC PATCH 2/5] mfd: axp20x: Refactor axp20x_is_polyphase_slave()

[Chen-Yu Tsai]

On Fri, Sep 19, 2025 at 8:01 AM Andre Przywara <andre.przywara@arm.com> wrote:
>
> Some X-Powers AXP PMICs allow to combine certain DC/DC rails together in
> a multi-phase fashion. So far we don't actively program those
> connections, but we detect the existing setup, and prevent the connected
> regulators from being re-programmed or turned off.

This last part is a bit incorrect. We simply don't register the secondary
phases as separate regulators anymore. The hardware ignores any changes
to their independent configuration (I assume, since otherwise the hardware
would be really hard to program...).


> At the moment this is
> done in a switch/case construct, listing the known regulator pairs for
> those PMICs supported.
>
> To get rid of this ever growing code section, create a data structure
> that describes the relationship, and have generic code that iterates
> over the entries and checks for matches.
>
> This not only cleans that function up and makes extensions much simpler,
> but also allows to reuse this information for the upcoming programming
> of those poly-phase setups.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/regulator/axp20x-regulator.c | 91 ++++++++++++++--------------
>  1 file changed, 45 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> index da891415efc0b..19c9a98d1835a 100644
> --- a/drivers/regulator/axp20x-regulator.c
> +++ b/drivers/regulator/axp20x-regulator.c
> @@ -1481,70 +1481,69 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
>         return regmap_update_bits(rdev->regmap, reg, mask, workmode);
>  }
>
> +struct dualphase_regulator {
> +       int axp_id;
> +       int reg1, reg2;

We don't care about the primary phase. The point is to make the secondary
phases disappear from the kernel's view.

reg1 should be part of the next patch, where you actually care.

> +       unsigned int polyphase_reg;
> +       unsigned int bitmask;
> +} dualphase_regulators[] = {
> +       { AXP323_ID, AXP313A_DCDC1, AXP313A_DCDC2,
> +               AXP323_DCDC_MODE_CTRL2, BIT(1), },
> +       { AXP803_ID, AXP803_DCDC2, AXP803_DCDC3, AXP803_POLYPHASE_CTRL,
> +               AXP803_DCDC23_POLYPHASE_DUAL, },
> +       { AXP803_ID, AXP803_DCDC5, AXP803_DCDC6, AXP803_POLYPHASE_CTRL,
> +               AXP803_DCDC56_POLYPHASE_DUAL, },
> +       /* AXP806's DCDC-A/B/C is a tri-phase regulator */

TODO: ?

ChenYu

> +       { AXP806_ID, AXP806_DCDCD, AXP806_DCDCE, AXP806_DCDC_MODE_CTRL2,
> +               AXP806_DCDCDE_POLYPHASE_DUAL, },
> +       { AXP813_ID, AXP803_DCDC2, AXP803_DCDC3, AXP803_POLYPHASE_CTRL,
> +               AXP803_DCDC23_POLYPHASE_DUAL, },
> +       { AXP813_ID, AXP803_DCDC5, AXP803_DCDC6, AXP803_POLYPHASE_CTRL,
> +               AXP803_DCDC56_POLYPHASE_DUAL, },
> +       { AXP15060_ID, AXP15060_DCDC2, AXP15060_DCDC3, AXP15060_DCDC_MODE_CTRL1,
> +               AXP15060_DCDC23_POLYPHASE_DUAL_MASK, },
> +       { AXP15060_ID, AXP15060_DCDC4, AXP15060_DCDC6, AXP15060_DCDC_MODE_CTRL1,
> +               AXP15060_DCDC46_POLYPHASE_DUAL_MASK, },
> +};
> +
>  /*
>   * This function checks whether a regulator is part of a poly-phase
>   * output setup based on the registers settings. Returns true if it is.
>   */
>  static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
>  {
> +       struct dualphase_regulator *dpreg;
>         u32 reg = 0;
> +       int i;
>
> -       /*
> -        * Currently in our supported AXP variants, only AXP803, AXP806,
> -        * AXP813 and AXP15060 have polyphase regulators.
> -        */
> -       switch (axp20x->variant) {
> -       case AXP803_ID:
> -       case AXP813_ID:
> -               regmap_read(axp20x->regmap, AXP803_POLYPHASE_CTRL, &reg);
> +       for (i = 0; i < ARRAY_SIZE(dualphase_regulators); i++) {
> +               dpreg = &dualphase_regulators[i];
>
> -               switch (id) {
> -               case AXP803_DCDC3:
> -                       return !!(reg & AXP803_DCDC23_POLYPHASE_DUAL);
> -               case AXP803_DCDC6:
> -                       return !!(reg & AXP803_DCDC56_POLYPHASE_DUAL);
> +               if (axp20x->variant != dpreg->axp_id)
> +                       continue;
> +               /* Is this the second regulator from a dual-phase pair? */
> +               if (id == dpreg->reg2) {
> +                       regmap_read(axp20x->regmap, dpreg->polyphase_reg, &reg);
> +
> +                       return !!(reg & dpreg->bitmask);
>                 }
> -               break;
> +       }
>
> -       case AXP806_ID:
> +       /*
> +        * DCDC-A/B/C can be configured either as a dual-phase (A+B) or
> +        * as a triple-phase regulator (A+B+C), but not in any other
> +        * combination. Treat this as a special case here.
> +        */
> +       if (axp20x->variant == AXP806_ID) {
>                 regmap_read(axp20x->regmap, AXP806_DCDC_MODE_CTRL2, &reg);
> -
> -               switch (id) {
> -               case AXP806_DCDCB:
> +               if (id == AXP806_DCDCB)
>                         return (((reg & AXP806_DCDCABC_POLYPHASE_MASK) ==
>                                 AXP806_DCDCAB_POLYPHASE_DUAL) ||
>                                 ((reg & AXP806_DCDCABC_POLYPHASE_MASK) ==
>                                 AXP806_DCDCABC_POLYPHASE_TRI));
> -               case AXP806_DCDCC:
> +               if (id == AXP806_DCDCC)
>                         return ((reg & AXP806_DCDCABC_POLYPHASE_MASK) ==
>                                 AXP806_DCDCABC_POLYPHASE_TRI);
> -               case AXP806_DCDCE:
> -                       return !!(reg & AXP806_DCDCDE_POLYPHASE_DUAL);
> -               }
> -               break;
> -
> -       case AXP15060_ID:
> -               regmap_read(axp20x->regmap, AXP15060_DCDC_MODE_CTRL1, &reg);
> -
> -               switch (id) {
> -               case AXP15060_DCDC3:
> -                       return !!(reg & AXP15060_DCDC23_POLYPHASE_DUAL_MASK);
> -               case AXP15060_DCDC6:
> -                       return !!(reg & AXP15060_DCDC46_POLYPHASE_DUAL_MASK);
> -               }
> -               break;
> -
> -       case AXP323_ID:
> -               regmap_read(axp20x->regmap, AXP323_DCDC_MODE_CTRL2, &reg);
> -
> -               switch (id) {
> -               case AXP313A_DCDC2:
> -                       return !!(reg & BIT(1));
> -               }
> -               break;
> -
> -       default:
> -               return false;
>         }
>
>         return false;
> --
> 2.46.4
>