[PATCH v2 0/5] regulator: Fix AXP717 PMIC support

[PATCH v2 0/5] regulator: Fix AXP717 PMIC support

[Andre Przywara]

This is v2 of the fixes to the AXP717 PMIC support series. Lee put the
original patches in an immutable branch already, so these here go on top.
Patch 1 is new in v2, and adds the IRQ status and acknowledge registers
to the writable range. Thanks to Chris for pointing this out.
Patch 2 contains fixes to the regulator descriptions: the LDOs had the
wrong supply source, and two numbers were wrong. The datasheet describes
the voltage ranges and register values differently from what our macros
expect, in a way that literally begs for off-by-ones, so here you go.
Also there is an actual wrong number in the datasheet, add a comment to
document this.
I don't know if that's still feasible, but those two patches would be a
good candidate to squash into the patches that they fix.

The other three patches add the "boost" regulator, which is meant to
provide the 5V USB VBUS power when operating from the battery. It's the
usual trinity of binding/mfd/regulator patches.
Again this could be squashed into the respective patches from the
original series, if people agree.

Please have a look and test!

Based on mfd/ib-mfd-regulator-6.10, as detailed below.

Cheers,
Andre

Changelog v1 .. v2:
- add tags
- add patch to add missing IRQ ack register range
- add comment to document bug in datasheet

Andre Przywara (5):
  mfd: axp20x: AXP717: Fix missing IRQ status registers range
  regulator: axp20x: AXP717: fix LDO supply rails and off-by-ones
  dt-bindings: mfd: x-powers,axp152: add boost regulator
  mfd: axp20x: AXP717: Add support for boost regulator
  regulator: axp20x: AXP717: Add boost regulator

 .../bindings/mfd/x-powers,axp152.yaml         |  2 +-
 drivers/mfd/axp20x.c                          |  3 ++
 drivers/regulator/axp20x-regulator.c          | 37 ++++++++++++-------
 include/linux/mfd/axp20x.h                    |  3 ++
 4 files changed, 30 insertions(+), 15 deletions(-)


base-commit: 4cece764965020c22cff7665b18a012006359095
prerequisite-patch-id: 2b5fb10f68e0994071fc4c7dce73db7047c23220
prerequisite-patch-id: 5d0735de888d155b2c1cdb814e852a5852a17ec7
prerequisite-patch-id: 29c30894b4bf0b9e1e71de065cabbd842505e248
prerequisite-patch-id: 0ab87cbf7362b6dc2d577d2264eb9574be47b5f6
-- 
2.35.8

[PATCH v2 1/5] mfd: axp20x: AXP717: Fix missing IRQ status registers range

[Andre Przywara]

While we list the "IRQ status *and acknowledge*" registers as volatile,
they are missing from the writable range array, so acknowledging any
interrupts was met with an -EIO error.

Add the five registers that hold those bits to the writable array.

Fixes: b5bfc8ab2484 ("mfd: axp20x: Add support for AXP717 PMIC")
Reported-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/mfd/axp20x.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 48ce6ea693cea..d8ad4e120d379 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -210,6 +210,7 @@ static const struct regmap_access_table axp313a_volatile_table = {
 
 static const struct regmap_range axp717_writeable_ranges[] = {
 	regmap_reg_range(AXP717_IRQ0_EN, AXP717_IRQ4_EN),
+	regmap_reg_range(AXP717_IRQ0_STATE, AXP717_IRQ4_STATE),
 	regmap_reg_range(AXP717_DCDC_OUTPUT_CONTROL, AXP717_CPUSLDO_CONTROL),
 };
 
-- 
2.35.8

[PATCH v2 2/5] regulator: axp20x: AXP717: fix LDO supply rails and off-by-ones

[Andre Przywara]

The X-Powers AXP717 PMIC has separate input supply pins for each group
of LDOs, so they are not all using the same DCDC1 input, as described
currently.

Replace the "supply" member of each LDO description with the respective
group supply name, so that the supply dependencies can be correctly
described in the devicetree.

Also fix two off-by-ones in the regulator macros, after some double
checking the numbers against the datasheet. This uncovered a bug in the
datasheet: add a comment to document this.

Fixes: d2ac3df75c3a ("regulator: axp20x: add support for the AXP717")
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/regulator/axp20x-regulator.c | 33 ++++++++++++++++------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 34fcdd82b2eaa..f3c447ecdc3bf 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -140,7 +140,7 @@
 
 #define AXP717_DCDC1_NUM_VOLTAGES	88
 #define AXP717_DCDC2_NUM_VOLTAGES	107
-#define AXP717_DCDC3_NUM_VOLTAGES	104
+#define AXP717_DCDC3_NUM_VOLTAGES	103
 #define AXP717_DCDC_V_OUT_MASK		GENMASK(6, 0)
 #define AXP717_LDO_V_OUT_MASK		GENMASK(4, 0)
 
@@ -763,10 +763,15 @@ static const struct linear_range axp717_dcdc1_ranges[] = {
 	REGULATOR_LINEAR_RANGE(1220000, 71, 87, 20000),
 };
 
+/*
+ * The manual says that the last voltage is 3.4V, encoded as 0b1101011 (107),
+ * but every other method proves that this is wrong, so it's really 106 that
+ * programs the final 3.4V.
+ */
 static const struct linear_range axp717_dcdc2_ranges[] = {
 	REGULATOR_LINEAR_RANGE(500000,   0,  70,  10000),
 	REGULATOR_LINEAR_RANGE(1220000, 71,  87,  20000),
-	REGULATOR_LINEAR_RANGE(1600000, 88, 107, 100000),
+	REGULATOR_LINEAR_RANGE(1600000, 88, 106, 100000),
 };
 
 static const struct linear_range axp717_dcdc3_ranges[] = {
@@ -790,40 +795,40 @@ static const struct regulator_desc axp717_regulators[] = {
 	AXP_DESC(AXP717, DCDC4, "dcdc4", "vin4", 1000, 3700, 100,
 		 AXP717_DCDC4_CONTROL, AXP717_DCDC_V_OUT_MASK,
 		 AXP717_DCDC_OUTPUT_CONTROL, BIT(3)),
-	AXP_DESC(AXP717, ALDO1, "aldo1", "vin1", 500, 3500, 100,
+	AXP_DESC(AXP717, ALDO1, "aldo1", "aldoin", 500, 3500, 100,
 		 AXP717_ALDO1_CONTROL, AXP717_LDO_V_OUT_MASK,
 		 AXP717_LDO0_OUTPUT_CONTROL, BIT(0)),
-	AXP_DESC(AXP717, ALDO2, "aldo2", "vin1", 500, 3500, 100,
+	AXP_DESC(AXP717, ALDO2, "aldo2", "aldoin", 500, 3500, 100,
 		 AXP717_ALDO2_CONTROL, AXP717_LDO_V_OUT_MASK,
 		 AXP717_LDO0_OUTPUT_CONTROL, BIT(1)),
-	AXP_DESC(AXP717, ALDO3, "aldo3", "vin1", 500, 3500, 100,
+	AXP_DESC(AXP717, ALDO3, "aldo3", "aldoin", 500, 3500, 100,
 		 AXP717_ALDO3_CONTROL, AXP717_LDO_V_OUT_MASK,
 		 AXP717_LDO0_OUTPUT_CONTROL, BIT(2)),
-	AXP_DESC(AXP717, ALDO4, "aldo4", "vin1", 500, 3500, 100,
+	AXP_DESC(AXP717, ALDO4, "aldo4", "aldoin", 500, 3500, 100,
 		 AXP717_ALDO4_CONTROL, AXP717_LDO_V_OUT_MASK,
 		 AXP717_LDO0_OUTPUT_CONTROL, BIT(3)),
-	AXP_DESC(AXP717, BLDO1, "bldo1", "vin1", 500, 3500, 100,
+	AXP_DESC(AXP717, BLDO1, "bldo1", "bldoin", 500, 3500, 100,
 		 AXP717_BLDO1_CONTROL, AXP717_LDO_V_OUT_MASK,
 		 AXP717_LDO0_OUTPUT_CONTROL, BIT(4)),
-	AXP_DESC(AXP717, BLDO2, "bldo2", "vin1", 500, 3500, 100,
+	AXP_DESC(AXP717, BLDO2, "bldo2", "bldoin", 500, 3500, 100,
 		 AXP717_BLDO2_CONTROL, AXP717_LDO_V_OUT_MASK,
 		 AXP717_LDO0_OUTPUT_CONTROL, BIT(5)),
-	AXP_DESC(AXP717, BLDO3, "bldo3", "vin1", 500, 3500, 100,
+	AXP_DESC(AXP717, BLDO3, "bldo3", "bldoin", 500, 3500, 100,
 		 AXP717_BLDO3_CONTROL, AXP717_LDO_V_OUT_MASK,
 		 AXP717_LDO0_OUTPUT_CONTROL, BIT(6)),
-	AXP_DESC(AXP717, BLDO4, "bldo4", "vin1", 500, 3500, 100,
+	AXP_DESC(AXP717, BLDO4, "bldo4", "bldoin", 500, 3500, 100,
 		 AXP717_BLDO4_CONTROL, AXP717_LDO_V_OUT_MASK,
 		 AXP717_LDO0_OUTPUT_CONTROL, BIT(7)),
-	AXP_DESC(AXP717, CLDO1, "cldo1", "vin1", 500, 3500, 100,
+	AXP_DESC(AXP717, CLDO1, "cldo1", "cldoin", 500, 3500, 100,
 		 AXP717_CLDO1_CONTROL, AXP717_LDO_V_OUT_MASK,
 		 AXP717_LDO1_OUTPUT_CONTROL, BIT(0)),
-	AXP_DESC(AXP717, CLDO2, "cldo2", "vin1", 500, 3500, 100,
+	AXP_DESC(AXP717, CLDO2, "cldo2", "cldoin", 500, 3500, 100,
 		 AXP717_CLDO2_CONTROL, AXP717_LDO_V_OUT_MASK,
 		 AXP717_LDO1_OUTPUT_CONTROL, BIT(1)),
-	AXP_DESC(AXP717, CLDO3, "cldo3", "vin1", 500, 3500, 100,
+	AXP_DESC(AXP717, CLDO3, "cldo3", "cldoin", 500, 3500, 100,
 		 AXP717_CLDO3_CONTROL, AXP717_LDO_V_OUT_MASK,
 		 AXP717_LDO1_OUTPUT_CONTROL, BIT(2)),
-	AXP_DESC(AXP717, CLDO4, "cldo4", "vin1", 500, 3500, 100,
+	AXP_DESC(AXP717, CLDO4, "cldo4", "cldoin", 500, 3500, 100,
 		 AXP717_CLDO4_CONTROL, AXP717_LDO_V_OUT_MASK,
 		 AXP717_LDO1_OUTPUT_CONTROL, BIT(3)),
 	AXP_DESC(AXP717, CPUSLDO, "cpusldo", "vin1", 500, 1400, 50,
-- 
2.35.8

[PATCH v2 3/5] dt-bindings: mfd: x-powers,axp152: add boost regulator

[Andre Przywara]

The X-Powers AXP717 contains a boost regulator, that it meant to provide
the 5V USB VBUS voltage when the devices operates on battery.

Add the name "boost" to the regexp describing the allowed node names,
to allow the regulator to be described in the devicetree.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
index b8e8db0d58e9c..14ab367fc8871 100644
--- a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
+++ b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
@@ -274,7 +274,7 @@ properties:
           Defines the work frequency of DC-DC in kHz.
 
     patternProperties:
-      "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|(dc1)?sw|rtc(_|-)ldo|cpusldo|drivevbus|dc5ldo)$":
+      "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|(dc1)?sw|rtc(_|-)ldo|cpusldo|drivevbus|dc5ldo|boost)$":
         $ref: /schemas/regulator/regulator.yaml#
         type: object
         unevaluatedProperties: false
-- 
2.35.8

[PATCH v2 4/5] mfd: axp20x: AXP717: Add support for boost regulator

[Andre Przywara]

The AXP717 also contains a boost regulator, to provide the 5V USB VBUS
rail when running on battery.

Add the registers to the MFD description to be able to use them from the
regulator driver.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: John Watts <contact@jookia.org>
---
 drivers/mfd/axp20x.c       | 2 ++
 include/linux/mfd/axp20x.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index d8ad4e120d379..02513b1eff2e8 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -209,6 +209,8 @@ static const struct regmap_access_table axp313a_volatile_table = {
 };
 
 static const struct regmap_range axp717_writeable_ranges[] = {
+	regmap_reg_range(AXP717_MODULE_EN_CONTROL, AXP717_MODULE_EN_CONTROL),
+	regmap_reg_range(AXP717_BOOST_CONTROL, AXP717_BOOST_CONTROL),
 	regmap_reg_range(AXP717_IRQ0_EN, AXP717_IRQ4_EN),
 	regmap_reg_range(AXP717_IRQ0_STATE, AXP717_IRQ4_STATE),
 	regmap_reg_range(AXP717_DCDC_OUTPUT_CONTROL, AXP717_CPUSLDO_CONTROL),
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 8c0a33a2e9ce2..4dad54fdf67ee 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -115,6 +115,8 @@ enum axp20x_variants {
 #define AXP313A_IRQ_STATE		0x21
 
 #define AXP717_ON_INDICATE		0x00
+#define AXP717_MODULE_EN_CONTROL	0x19
+#define AXP717_BOOST_CONTROL		0x1e
 #define AXP717_IRQ0_EN			0x40
 #define AXP717_IRQ1_EN			0x41
 #define AXP717_IRQ2_EN			0x42
-- 
2.35.8

[PATCH v2 5/5] regulator: axp20x: AXP717: Add boost regulator

[Andre Przywara]

The AXP717 also contains an adjustable boost regulator, to provide the
5V USB VBUS rail when running on battery.

Add the regulator description that states the voltage range this
regulator can cover.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: John Watts <contact@jookia.org>
---
 drivers/regulator/axp20x-regulator.c | 4 ++++
 include/linux/mfd/axp20x.h           | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index f3c447ecdc3bf..20bef3971feca 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -143,6 +143,7 @@
 #define AXP717_DCDC3_NUM_VOLTAGES	103
 #define AXP717_DCDC_V_OUT_MASK		GENMASK(6, 0)
 #define AXP717_LDO_V_OUT_MASK		GENMASK(4, 0)
+#define AXP717_BOOST_V_OUT_MASK		GENMASK(7, 4)
 
 #define AXP803_PWR_OUT_DCDC1_MASK	BIT_MASK(0)
 #define AXP803_PWR_OUT_DCDC2_MASK	BIT_MASK(1)
@@ -834,6 +835,9 @@ static const struct regulator_desc axp717_regulators[] = {
 	AXP_DESC(AXP717, CPUSLDO, "cpusldo", "vin1", 500, 1400, 50,
 		 AXP717_CPUSLDO_CONTROL, AXP717_LDO_V_OUT_MASK,
 		 AXP717_LDO1_OUTPUT_CONTROL, BIT(4)),
+	AXP_DESC(AXP717, BOOST, "boost", "vin1", 4550, 5510, 64,
+		 AXP717_BOOST_CONTROL, AXP717_BOOST_V_OUT_MASK,
+		 AXP717_MODULE_EN_CONTROL, BIT(4)),
 };
 
 /* DCDC ranges shared with AXP813 */
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 4dad54fdf67ee..5e86b976c4caa 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -486,6 +486,7 @@ enum {
 	AXP717_CLDO3,
 	AXP717_CLDO4,
 	AXP717_CPUSLDO,
+	AXP717_BOOST,
 	AXP717_REG_ID_MAX,
 };
 
-- 
2.35.8

Re: [PATCH v2 2/5] regulator: axp20x: AXP717: fix LDO supply rails and off-by-ones

[John Watts]

On Thu, Apr 18, 2024 at 01:07:33AM +0100, Andre Przywara wrote:
> The X-Powers AXP717 PMIC has separate input supply pins for each group
> of LDOs, so they are not all using the same DCDC1 input, as described
> currently.
> 
> Replace the "supply" member of each LDO description with the respective
> group supply name, so that the supply dependencies can be correctly
> described in the devicetree.
> 
> Also fix two off-by-ones in the regulator macros, after some double
> checking the numbers against the datasheet. This uncovered a bug in the
> datasheet: add a comment to document this.

Hi,

This looks a lot better with the comment.

John.

Reviewed-by: John Watts <contact@jookia.org>

> 
> Fixes: d2ac3df75c3a ("regulator: axp20x: add support for the AXP717")
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/regulator/axp20x-regulator.c | 33 ++++++++++++++++------------
>  1 file changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> index 34fcdd82b2eaa..f3c447ecdc3bf 100644
> --- a/drivers/regulator/axp20x-regulator.c
> +++ b/drivers/regulator/axp20x-regulator.c
> @@ -140,7 +140,7 @@
>  
>  #define AXP717_DCDC1_NUM_VOLTAGES	88
>  #define AXP717_DCDC2_NUM_VOLTAGES	107
> -#define AXP717_DCDC3_NUM_VOLTAGES	104
> +#define AXP717_DCDC3_NUM_VOLTAGES	103
>  #define AXP717_DCDC_V_OUT_MASK		GENMASK(6, 0)
>  #define AXP717_LDO_V_OUT_MASK		GENMASK(4, 0)
>  
> @@ -763,10 +763,15 @@ static const struct linear_range axp717_dcdc1_ranges[] = {
>  	REGULATOR_LINEAR_RANGE(1220000, 71, 87, 20000),
>  };
>  
> +/*
> + * The manual says that the last voltage is 3.4V, encoded as 0b1101011 (107),
> + * but every other method proves that this is wrong, so it's really 106 that
> + * programs the final 3.4V.
> + */
>  static const struct linear_range axp717_dcdc2_ranges[] = {
>  	REGULATOR_LINEAR_RANGE(500000,   0,  70,  10000),
>  	REGULATOR_LINEAR_RANGE(1220000, 71,  87,  20000),
> -	REGULATOR_LINEAR_RANGE(1600000, 88, 107, 100000),
> +	REGULATOR_LINEAR_RANGE(1600000, 88, 106, 100000),
>  };
>  
>  static const struct linear_range axp717_dcdc3_ranges[] = {
> @@ -790,40 +795,40 @@ static const struct regulator_desc axp717_regulators[] = {
>  	AXP_DESC(AXP717, DCDC4, "dcdc4", "vin4", 1000, 3700, 100,
>  		 AXP717_DCDC4_CONTROL, AXP717_DCDC_V_OUT_MASK,
>  		 AXP717_DCDC_OUTPUT_CONTROL, BIT(3)),
> -	AXP_DESC(AXP717, ALDO1, "aldo1", "vin1", 500, 3500, 100,
> +	AXP_DESC(AXP717, ALDO1, "aldo1", "aldoin", 500, 3500, 100,
>  		 AXP717_ALDO1_CONTROL, AXP717_LDO_V_OUT_MASK,
>  		 AXP717_LDO0_OUTPUT_CONTROL, BIT(0)),
> -	AXP_DESC(AXP717, ALDO2, "aldo2", "vin1", 500, 3500, 100,
> +	AXP_DESC(AXP717, ALDO2, "aldo2", "aldoin", 500, 3500, 100,
>  		 AXP717_ALDO2_CONTROL, AXP717_LDO_V_OUT_MASK,
>  		 AXP717_LDO0_OUTPUT_CONTROL, BIT(1)),
> -	AXP_DESC(AXP717, ALDO3, "aldo3", "vin1", 500, 3500, 100,
> +	AXP_DESC(AXP717, ALDO3, "aldo3", "aldoin", 500, 3500, 100,
>  		 AXP717_ALDO3_CONTROL, AXP717_LDO_V_OUT_MASK,
>  		 AXP717_LDO0_OUTPUT_CONTROL, BIT(2)),
> -	AXP_DESC(AXP717, ALDO4, "aldo4", "vin1", 500, 3500, 100,
> +	AXP_DESC(AXP717, ALDO4, "aldo4", "aldoin", 500, 3500, 100,
>  		 AXP717_ALDO4_CONTROL, AXP717_LDO_V_OUT_MASK,
>  		 AXP717_LDO0_OUTPUT_CONTROL, BIT(3)),
> -	AXP_DESC(AXP717, BLDO1, "bldo1", "vin1", 500, 3500, 100,
> +	AXP_DESC(AXP717, BLDO1, "bldo1", "bldoin", 500, 3500, 100,
>  		 AXP717_BLDO1_CONTROL, AXP717_LDO_V_OUT_MASK,
>  		 AXP717_LDO0_OUTPUT_CONTROL, BIT(4)),
> -	AXP_DESC(AXP717, BLDO2, "bldo2", "vin1", 500, 3500, 100,
> +	AXP_DESC(AXP717, BLDO2, "bldo2", "bldoin", 500, 3500, 100,
>  		 AXP717_BLDO2_CONTROL, AXP717_LDO_V_OUT_MASK,
>  		 AXP717_LDO0_OUTPUT_CONTROL, BIT(5)),
> -	AXP_DESC(AXP717, BLDO3, "bldo3", "vin1", 500, 3500, 100,
> +	AXP_DESC(AXP717, BLDO3, "bldo3", "bldoin", 500, 3500, 100,
>  		 AXP717_BLDO3_CONTROL, AXP717_LDO_V_OUT_MASK,
>  		 AXP717_LDO0_OUTPUT_CONTROL, BIT(6)),
> -	AXP_DESC(AXP717, BLDO4, "bldo4", "vin1", 500, 3500, 100,
> +	AXP_DESC(AXP717, BLDO4, "bldo4", "bldoin", 500, 3500, 100,
>  		 AXP717_BLDO4_CONTROL, AXP717_LDO_V_OUT_MASK,
>  		 AXP717_LDO0_OUTPUT_CONTROL, BIT(7)),
> -	AXP_DESC(AXP717, CLDO1, "cldo1", "vin1", 500, 3500, 100,
> +	AXP_DESC(AXP717, CLDO1, "cldo1", "cldoin", 500, 3500, 100,
>  		 AXP717_CLDO1_CONTROL, AXP717_LDO_V_OUT_MASK,
>  		 AXP717_LDO1_OUTPUT_CONTROL, BIT(0)),
> -	AXP_DESC(AXP717, CLDO2, "cldo2", "vin1", 500, 3500, 100,
> +	AXP_DESC(AXP717, CLDO2, "cldo2", "cldoin", 500, 3500, 100,
>  		 AXP717_CLDO2_CONTROL, AXP717_LDO_V_OUT_MASK,
>  		 AXP717_LDO1_OUTPUT_CONTROL, BIT(1)),
> -	AXP_DESC(AXP717, CLDO3, "cldo3", "vin1", 500, 3500, 100,
> +	AXP_DESC(AXP717, CLDO3, "cldo3", "cldoin", 500, 3500, 100,
>  		 AXP717_CLDO3_CONTROL, AXP717_LDO_V_OUT_MASK,
>  		 AXP717_LDO1_OUTPUT_CONTROL, BIT(2)),
> -	AXP_DESC(AXP717, CLDO4, "cldo4", "vin1", 500, 3500, 100,
> +	AXP_DESC(AXP717, CLDO4, "cldo4", "cldoin", 500, 3500, 100,
>  		 AXP717_CLDO4_CONTROL, AXP717_LDO_V_OUT_MASK,
>  		 AXP717_LDO1_OUTPUT_CONTROL, BIT(3)),
>  	AXP_DESC(AXP717, CPUSLDO, "cpusldo", "vin1", 500, 1400, 50,
> -- 
> 2.35.8
> 

Re: [PATCH v2 1/5] mfd: axp20x: AXP717: Fix missing IRQ status registers range

[John Watts]

On Thu, Apr 18, 2024 at 01:07:32AM +0100, Andre Przywara wrote:
> While we list the "IRQ status *and acknowledge*" registers as volatile,
> they are missing from the writable range array, so acknowledging any
> interrupts was met with an -EIO error.
> 
> Add the five registers that hold those bits to the writable array.

Hi,

This change looks good to me. Checked against the datasheet.

John.

Reviewed-by: John Watts <contact@jookia.org>

> 
> Fixes: b5bfc8ab2484 ("mfd: axp20x: Add support for AXP717 PMIC")
> Reported-by: Chris Morgan <macromorgan@hotmail.com>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/mfd/axp20x.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 48ce6ea693cea..d8ad4e120d379 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -210,6 +210,7 @@ static const struct regmap_access_table axp313a_volatile_table = {
>  
>  static const struct regmap_range axp717_writeable_ranges[] = {
>  	regmap_reg_range(AXP717_IRQ0_EN, AXP717_IRQ4_EN),
> +	regmap_reg_range(AXP717_IRQ0_STATE, AXP717_IRQ4_STATE),
>  	regmap_reg_range(AXP717_DCDC_OUTPUT_CONTROL, AXP717_CPUSLDO_CONTROL),
>  };
>  
> -- 
> 2.35.8
> 

Re: [PATCH v2 0/5] regulator: Fix AXP717 PMIC support

[Mark Brown]

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

On Thu, Apr 18, 2024 at 01:07:31AM +0100, Andre Przywara wrote:
> This is v2 of the fixes to the AXP717 PMIC support series. Lee put the
> original patches in an immutable branch already, so these here go on top.
> Patch 1 is new in v2, and adds the IRQ status and acknowledge registers

Lee, this looks like more of a regulator series than a MFD one (it's got
a couple of small updates to the MFD but it's mainly focused on the
regualtor functionality) so it probably makes sense to merge via the
regulator tree?

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

Re: [PATCH v2 0/5] regulator: Fix AXP717 PMIC support

[Ryan Walklin]

On Thu, 18 Apr 2024, at 12:07 PM, Andre Przywara wrote:
> This is v2 of the fixes to the AXP717 PMIC support series. 
<snip>
> I don't know if that's still feasible, but those two patches would be a
> good candidate to squash into the patches that they fix.
>
> The other three patches add the "boost" regulator, which is meant to
> provide the 5V USB VBUS power when operating from the battery. It's the
> usual trinity of binding/mfd/regulator patches.
> Again this could be squashed into the respective patches from the
> original series, if people agree.
>
> Please have a look and test!
>
> Based on mfd/ib-mfd-regulator-6.10, as detailed below.

Looks good here, RSB communication, regulator and 5v boost support configured via DT working well on my H700 board, established by a combination of successful device bringup and kernel reporting. Concur with the request to be squashed into the mfd-next tree for 6.10 if possible, thanks!

Ryan

Tested-by: Ryan Walklin <ryan@testtoast,com>

Re: [PATCH v2 0/5] regulator: Fix AXP717 PMIC support

[Lee Jones]

On Sun, 28 Apr 2024, Ryan Walklin wrote:

> On Thu, 18 Apr 2024, at 12:07 PM, Andre Przywara wrote:
> > This is v2 of the fixes to the AXP717 PMIC support series. 
> <snip>
> > I don't know if that's still feasible, but those two patches would be a
> > good candidate to squash into the patches that they fix.
> >
> > The other three patches add the "boost" regulator, which is meant to
> > provide the 5V USB VBUS power when operating from the battery. It's the
> > usual trinity of binding/mfd/regulator patches.
> > Again this could be squashed into the respective patches from the
> > original series, if people agree.
> >
> > Please have a look and test!
> >
> > Based on mfd/ib-mfd-regulator-6.10, as detailed below.
> 
> Looks good here, RSB communication, regulator and 5v boost support configured via DT working well on my H700 board, established by a combination of successful device bringup and kernel reporting. Concur with the request to be squashed into the mfd-next tree for 6.10 if possible, thanks!
> 
> Ryan
> 
> Tested-by: Ryan Walklin <ryan@testtoast,com>

This doesn't look like a valid email address.

Did you manually type it out?  I suggest against doing that.

-- 
Lee Jones [李琼斯]

Re: [PATCH v2 0/5] regulator: Fix AXP717 PMIC support

[Lee Jones]

On Tue, 23 Apr 2024, Mark Brown wrote:

> On Thu, Apr 18, 2024 at 01:07:31AM +0100, Andre Przywara wrote:
> > This is v2 of the fixes to the AXP717 PMIC support series. Lee put the
> > original patches in an immutable branch already, so these here go on top.
> > Patch 1 is new in v2, and adds the IRQ status and acknowledge registers
> 
> Lee, this looks like more of a regulator series than a MFD one (it's got
> a couple of small updates to the MFD but it's mainly focused on the
> regualtor functionality) so it probably makes sense to merge via the
> regulator tree?

I'm fine with it.

Please ensure a succinct immutable branch is available in case of future conflicts.

-- 
Lee Jones [李琼斯]

Re: [PATCH v2 1/5] mfd: axp20x: AXP717: Fix missing IRQ status registers range

[Lee Jones]

On Thu, 18 Apr 2024, Andre Przywara wrote:

> While we list the "IRQ status *and acknowledge*" registers as volatile,
> they are missing from the writable range array, so acknowledging any
> interrupts was met with an -EIO error.
> 
> Add the five registers that hold those bits to the writable array.
> 
> Fixes: b5bfc8ab2484 ("mfd: axp20x: Add support for AXP717 PMIC")
> Reported-by: Chris Morgan <macromorgan@hotmail.com>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/mfd/axp20x.c | 1 +
>  1 file changed, 1 insertion(+)

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

> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 48ce6ea693cea..d8ad4e120d379 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -210,6 +210,7 @@ static const struct regmap_access_table axp313a_volatile_table = {
>  
>  static const struct regmap_range axp717_writeable_ranges[] = {
>  	regmap_reg_range(AXP717_IRQ0_EN, AXP717_IRQ4_EN),
> +	regmap_reg_range(AXP717_IRQ0_STATE, AXP717_IRQ4_STATE),
>  	regmap_reg_range(AXP717_DCDC_OUTPUT_CONTROL, AXP717_CPUSLDO_CONTROL),
>  };
>  
> -- 
> 2.35.8
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH v2 4/5] mfd: axp20x: AXP717: Add support for boost regulator

[Lee Jones]

On Thu, 18 Apr 2024, Andre Przywara wrote:

> The AXP717 also contains a boost regulator, to provide the 5V USB VBUS
> rail when running on battery.
> 
> Add the registers to the MFD description to be able to use them from the
> regulator driver.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Reviewed-by: John Watts <contact@jookia.org>
> ---
>  drivers/mfd/axp20x.c       | 2 ++
>  include/linux/mfd/axp20x.h | 2 ++
>  2 files changed, 4 insertions(+)

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

> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index d8ad4e120d379..02513b1eff2e8 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -209,6 +209,8 @@ static const struct regmap_access_table axp313a_volatile_table = {
>  };
>  
>  static const struct regmap_range axp717_writeable_ranges[] = {
> +	regmap_reg_range(AXP717_MODULE_EN_CONTROL, AXP717_MODULE_EN_CONTROL),
> +	regmap_reg_range(AXP717_BOOST_CONTROL, AXP717_BOOST_CONTROL),
>  	regmap_reg_range(AXP717_IRQ0_EN, AXP717_IRQ4_EN),
>  	regmap_reg_range(AXP717_IRQ0_STATE, AXP717_IRQ4_STATE),
>  	regmap_reg_range(AXP717_DCDC_OUTPUT_CONTROL, AXP717_CPUSLDO_CONTROL),
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 8c0a33a2e9ce2..4dad54fdf67ee 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -115,6 +115,8 @@ enum axp20x_variants {
>  #define AXP313A_IRQ_STATE		0x21
>  
>  #define AXP717_ON_INDICATE		0x00
> +#define AXP717_MODULE_EN_CONTROL	0x19
> +#define AXP717_BOOST_CONTROL		0x1e
>  #define AXP717_IRQ0_EN			0x40
>  #define AXP717_IRQ1_EN			0x41
>  #define AXP717_IRQ2_EN			0x42
> -- 
> 2.35.8
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH v2 3/5] dt-bindings: mfd: x-powers,axp152: add boost regulator

[Lee Jones]

On Thu, 18 Apr 2024, Andre Przywara wrote:

> The X-Powers AXP717 contains a boost regulator, that it meant to provide
> the 5V USB VBUS voltage when the devices operates on battery.
> 
> Add the name "boost" to the regexp describing the allowed node names,
> to allow the regulator to be described in the devicetree.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

> diff --git a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> index b8e8db0d58e9c..14ab367fc8871 100644
> --- a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> +++ b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> @@ -274,7 +274,7 @@ properties:
>            Defines the work frequency of DC-DC in kHz.
>  
>      patternProperties:
> -      "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|(dc1)?sw|rtc(_|-)ldo|cpusldo|drivevbus|dc5ldo)$":
> +      "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|(dc1)?sw|rtc(_|-)ldo|cpusldo|drivevbus|dc5ldo|boost)$":
>          $ref: /schemas/regulator/regulator.yaml#
>          type: object
>          unevaluatedProperties: false
> -- 
> 2.35.8
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH v2 0/5] regulator: Fix AXP717 PMIC support

[Mark Brown]

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

On Thu, Apr 18, 2024 at 01:07:31AM +0100, Andre Przywara wrote:

> Based on mfd/ib-mfd-regulator-6.10, as detailed below.

Please provide references like this in a form that's directly usable,
ideally the git URL plus tag format used for git pull requests.  The
only references lore has to an 'ib-mfd-regulator-6.10' are to this
thread...

> prerequisite-patch-id: 2b5fb10f68e0994071fc4c7dce73db7047c23220
> prerequisite-patch-id: 5d0735de888d155b2c1cdb814e852a5852a17ec7
> prerequisite-patch-id: 29c30894b4bf0b9e1e71de065cabbd842505e248
> prerequisite-patch-id: 0ab87cbf7362b6dc2d577d2264eb9574be47b5f6

Patch IDs aren't terribly useful for reverse engineering what you're
talking about here...

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

Re: [PATCH v2 0/5] regulator: Fix AXP717 PMIC support

[Mark Brown]

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

On Thu, May 02, 2024 at 10:38:36AM +0100, Lee Jones wrote:

> > Lee, this looks like more of a regulator series than a MFD one (it's got
> > a couple of small updates to the MFD but it's mainly focused on the
> > regualtor functionality) so it probably makes sense to merge via the
> > regulator tree?

> I'm fine with it.

> Please ensure a succinct immutable branch is available in case of future conflicts.

Actually now I look again it's fixes to stuff that's only in your tree
anyway so it probably makes more sense for it to go via MFD, sorry.

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

Re: [PATCH v2 0/5] regulator: Fix AXP717 PMIC support

[Mark Brown]

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

On Thu, Apr 18, 2024 at 01:07:31AM +0100, Andre Przywara wrote:
> This is v2 of the fixes to the AXP717 PMIC support series. Lee put the
> original patches in an immutable branch already, so these here go on top.

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

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