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
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 <[email protected]>
Signed-off-by: Andre Przywara <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: John Watts <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: John Watts <[email protected]>
---
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
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 <[email protected]>
>
> Fixes: d2ac3df75c3a ("regulator: axp20x: add support for the AXP717")
> Signed-off-by: Andre Przywara <[email protected]>
> ---
> 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
>
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 <[email protected]>
>
> Fixes: b5bfc8ab2484 ("mfd: axp20x: Add support for AXP717 PMIC")
> Reported-by: Chris Morgan <[email protected]>
> Signed-off-by: Andre Przywara <[email protected]>
> ---
> 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
>
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?
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>
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 [李琼斯]
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 [李琼斯]
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 <[email protected]>
> Signed-off-by: Andre Przywara <[email protected]>
> ---
> drivers/mfd/axp20x.c | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Lee Jones <[email protected]>
> 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 [李琼斯]
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 <[email protected]>
> Reviewed-by: John Watts <[email protected]>
> ---
> drivers/mfd/axp20x.c | 2 ++
> include/linux/mfd/axp20x.h | 2 ++
> 2 files changed, 4 insertions(+)
Acked-by: Lee Jones <[email protected]>
> 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 [李琼斯]
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 <[email protected]>
> Acked-by: Krzysztof Kozlowski <[email protected]>
> ---
> Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Lee Jones <[email protected]>
> 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 [李琼斯]