[PATCH] regulator: da9063: disable unused voltage monitors

[PATCH] regulator: da9063: disable unused voltage monitors

[Benjamin Bara]

From: Benjamin Bara <benjamin.bara@skidata.com>

This enables the kernel to disable the voltage monitor of unused regs.

Some boards might use the PWR_OK functionality, which asserts GP_FB2
if each measured voltage is in-range.
However, if a regulator (which is enabled and monitored) is disabled,
also its voltage monitor must be disabled to keep a valid PWR_OK state.

Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
 drivers/regulator/da9063-regulator.c | 51 ++++++++++++++++++++++++++--
 include/linux/mfd/da9063/registers.h | 23 +++++++++++++
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/da9063-regulator.c b/drivers/regulator/da9063-regulator.c
index 82f52a2a031a..c0a8c88d9ef6 100644
--- a/drivers/regulator/da9063-regulator.c
+++ b/drivers/regulator/da9063-regulator.c
@@ -83,6 +83,9 @@ struct da9063_regulator_info {
 
 	/* DA9063 event detection bit */
 	struct reg_field oc_event;
+
+	/* DA9063 voltage monitor bit */
+	struct reg_field vmon;
 };
 
 /* Macros for LDO */
@@ -148,6 +151,7 @@ struct da9063_regulator {
 	struct regmap_field			*suspend;
 	struct regmap_field			*sleep;
 	struct regmap_field			*suspend_sleep;
+	struct regmap_field			*vmon;
 };
 
 /* Encapsulates all information for the regulators driver */
@@ -203,6 +207,24 @@ static const unsigned int da9063_bmem_bio_merged_limits[] = {
 	4600000, 4800000, 5000000, 5200000, 5400000, 5600000, 5800000, 6000000
 };
 
+static int da9063_vmon_disable(struct regulator_dev *rdev)
+{
+	struct da9063_regulator *regl = rdev_get_drvdata(rdev);
+
+	return regmap_field_write(regl->vmon, 0);
+}
+
+static int da9063_disable(struct regulator_dev *rdev)
+{
+	int ret;
+
+	ret = da9063_vmon_disable(rdev);
+	if (ret < 0)
+		return ret;
+
+	return regulator_disable_regmap(rdev);
+}
+
 static int da9063_buck_set_mode(struct regulator_dev *rdev, unsigned int mode)
 {
 	struct da9063_regulator *regl = rdev_get_drvdata(rdev);
@@ -542,7 +564,7 @@ static int da9063_buck_get_current_limit(struct regulator_dev *rdev)
 
 static const struct regulator_ops da9063_buck_ops = {
 	.enable			= regulator_enable_regmap,
-	.disable		= regulator_disable_regmap,
+	.disable		= da9063_disable,
 	.is_enabled		= regulator_is_enabled_regmap,
 	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
 	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
@@ -560,7 +582,7 @@ static const struct regulator_ops da9063_buck_ops = {
 
 static const struct regulator_ops da9063_ldo_ops = {
 	.enable			= regulator_enable_regmap,
-	.disable		= regulator_disable_regmap,
+	.disable		= da9063_disable,
 	.is_enabled		= regulator_is_enabled_regmap,
 	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
 	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
@@ -581,36 +603,42 @@ static const struct da9063_regulator_info da9063_regulator_info[] = {
 			    da9063_buck_a_limits,
 			    DA9063_REG_BUCK_ILIM_C, DA9063_BCORE1_ILIM_MASK),
 		DA9063_BUCK_COMMON_FIELDS(BCORE1),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_4, DA9063_BCORE1_MON_EN),
 	},
 	{
 		DA9063_BUCK(DA9063, BCORE2, 300, 10, 1570,
 			    da9063_buck_a_limits,
 			    DA9063_REG_BUCK_ILIM_C, DA9063_BCORE2_ILIM_MASK),
 		DA9063_BUCK_COMMON_FIELDS(BCORE2),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_4, DA9063_BCORE2_MON_EN),
 	},
 	{
 		DA9063_BUCK(DA9063, BPRO, 530, 10, 1800,
 			    da9063_buck_a_limits,
 			    DA9063_REG_BUCK_ILIM_B, DA9063_BPRO_ILIM_MASK),
 		DA9063_BUCK_COMMON_FIELDS(BPRO),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_4, DA9063_BPRO_MON_EN),
 	},
 	{
 		DA9063_BUCK(DA9063, BMEM, 800, 20, 3340,
 			    da9063_buck_b_limits,
 			    DA9063_REG_BUCK_ILIM_A, DA9063_BMEM_ILIM_MASK),
 		DA9063_BUCK_COMMON_FIELDS(BMEM),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_4, DA9063_BMEM_MON_EN),
 	},
 	{
 		DA9063_BUCK(DA9063, BIO, 800, 20, 3340,
 			    da9063_buck_b_limits,
 			    DA9063_REG_BUCK_ILIM_A, DA9063_BIO_ILIM_MASK),
 		DA9063_BUCK_COMMON_FIELDS(BIO),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_4, DA9063_BIO_MON_EN),
 	},
 	{
 		DA9063_BUCK(DA9063, BPERI, 800, 20, 3340,
 			    da9063_buck_b_limits,
 			    DA9063_REG_BUCK_ILIM_B, DA9063_BPERI_ILIM_MASK),
 		DA9063_BUCK_COMMON_FIELDS(BPERI),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_4, DA9063_BPERI_MON_EN),
 	},
 	{
 		DA9063_BUCK(DA9063, BCORES_MERGED, 300, 10, 1570,
@@ -618,6 +646,7 @@ static const struct da9063_regulator_info da9063_regulator_info[] = {
 			    DA9063_REG_BUCK_ILIM_C, DA9063_BCORE1_ILIM_MASK),
 		/* BCORES_MERGED uses the same register fields as BCORE1 */
 		DA9063_BUCK_COMMON_FIELDS(BCORE1),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_4, DA9063_BCORE1_MON_EN),
 	},
 	{
 		DA9063_BUCK(DA9063, BMEM_BIO_MERGED, 800, 20, 3340,
@@ -625,47 +654,59 @@ static const struct da9063_regulator_info da9063_regulator_info[] = {
 			    DA9063_REG_BUCK_ILIM_A, DA9063_BMEM_ILIM_MASK),
 		/* BMEM_BIO_MERGED uses the same register fields as BMEM */
 		DA9063_BUCK_COMMON_FIELDS(BMEM),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_4, DA9063_BMEM_MON_EN),
 	},
 	{
 		DA9063_LDO(DA9063, LDO3, 900, 20, 3440),
 		.oc_event = BFIELD(DA9063_REG_STATUS_D, DA9063_LDO3_LIM),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_2, DA9063_LDO3_MON_EN),
 	},
 	{
 		DA9063_LDO(DA9063, LDO7, 900, 50, 3600),
 		.oc_event = BFIELD(DA9063_REG_STATUS_D, DA9063_LDO7_LIM),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_2, DA9063_LDO7_MON_EN),
 	},
 	{
 		DA9063_LDO(DA9063, LDO8, 900, 50, 3600),
 		.oc_event = BFIELD(DA9063_REG_STATUS_D, DA9063_LDO8_LIM),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_2, DA9063_LDO8_MON_EN),
 	},
 	{
 		DA9063_LDO(DA9063, LDO9, 950, 50, 3600),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_3, DA9063_LDO9_MON_EN),
 	},
 	{
 		DA9063_LDO(DA9063, LDO11, 900, 50, 3600),
 		.oc_event = BFIELD(DA9063_REG_STATUS_D, DA9063_LDO11_LIM),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_3, DA9063_LDO11_MON_EN),
 	},
 
 	/* The following LDOs are present only on DA9063, not on DA9063L */
 	{
 		DA9063_LDO(DA9063, LDO1, 600, 20, 1860),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_2, DA9063_LDO1_MON_EN),
 	},
 	{
 		DA9063_LDO(DA9063, LDO2, 600, 20, 1860),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_2, DA9063_LDO2_MON_EN),
 	},
 	{
 		DA9063_LDO(DA9063, LDO4, 900, 20, 3440),
 		.oc_event = BFIELD(DA9063_REG_STATUS_D, DA9063_LDO4_LIM),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_2, DA9063_LDO4_MON_EN),
 	},
 	{
 		DA9063_LDO(DA9063, LDO5, 900, 50, 3600),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_2, DA9063_LDO5_MON_EN),
 	},
 	{
 		DA9063_LDO(DA9063, LDO6, 900, 50, 3600),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_2, DA9063_LDO6_MON_EN),
 	},
 
 	{
 		DA9063_LDO(DA9063, LDO10, 900, 50, 3600),
+		.vmon = BFIELD(DA9063_BB_REG_MON_REG_3, DA9063_LDO10_MON_EN),
 	},
 };
 
@@ -932,6 +973,12 @@ static int da9063_regulator_probe(struct platform_device *pdev)
 			if (IS_ERR(regl->suspend_sleep))
 				return PTR_ERR(regl->suspend_sleep);
 		}
+		if (regl->info->vmon.reg) {
+			regl->vmon = devm_regmap_field_alloc(&pdev->dev,
+				da9063->regmap, regl->info->vmon);
+			if (IS_ERR(regl->vmon))
+				return PTR_ERR(regl->vmon);
+		}
 
 		/* Register regulator */
 		memset(&config, 0, sizeof(config));
diff --git a/include/linux/mfd/da9063/registers.h b/include/linux/mfd/da9063/registers.h
index 6e0f66a2e727..7b8364bd08a0 100644
--- a/include/linux/mfd/da9063/registers.h
+++ b/include/linux/mfd/da9063/registers.h
@@ -1040,6 +1040,29 @@
 /* DA9063_REG_CONFIG_J (addr=0x10F) */
 #define DA9063_TWOWIRE_TO			0x40
 
+/* DA9063_REG_MON_REG_2 (addr=0x115) */
+#define DA9063_LDO1_MON_EN			0x01
+#define DA9063_LDO2_MON_EN			0x02
+#define DA9063_LDO3_MON_EN			0x04
+#define DA9063_LDO4_MON_EN			0x08
+#define DA9063_LDO5_MON_EN			0x10
+#define DA9063_LDO6_MON_EN			0x20
+#define DA9063_LDO7_MON_EN			0x40
+#define DA9063_LDO8_MON_EN			0x80
+
+/* DA9063_REG_MON_REG_3 (addr=0x116) */
+#define DA9063_LDO9_MON_EN			0x01
+#define DA9063_LDO10_MON_EN			0x02
+#define DA9063_LDO11_MON_EN			0x04
+
+/* DA9063_REG_MON_REG_4 (addr=0x117) */
+#define DA9063_BCORE1_MON_EN			0x04
+#define DA9063_BCORE2_MON_EN			0x08
+#define DA9063_BPRO_MON_EN			0x10
+#define DA9063_BIO_MON_EN			0x20
+#define DA9063_BMEM_MON_EN			0x40
+#define DA9063_BPERI_MON_EN			0x80
+
 /* DA9063_REG_MON_REG_5 (addr=0x116) */
 #define DA9063_MON_A8_IDX_MASK			0x07
 #define		DA9063_MON_A8_IDX_NONE		0x00
-- 
2.34.1

RE: [PATCH] regulator: da9063: disable unused voltage monitors

[DLG Adam Thomson]

On 13 July 2022 13:50, Benjamin Bara wrote:

> From: Benjamin Bara <benjamin.bara@skidata.com>
> 
> This enables the kernel to disable the voltage monitor of unused regs.
> 
> Some boards might use the PWR_OK functionality, which asserts GP_FB2
> if each measured voltage is in-range.
> However, if a regulator (which is enabled and monitored) is disabled,
> also its voltage monitor must be disabled to keep a valid PWR_OK state.

This proposal seemingly doesn't work. If you disable monitoring of a regulator,
and no other regulators are being monitored at that point, then GP_FB2 will no
longer be asserted and you have the same issue. What's more this patch impacts
all users and there's no means to re-enable monitoring. Certainly in the current
state this isn't a valid change.

What exactly is the problem you're attempting to resolve through this patch?

Re: [PATCH] regulator: da9063: disable unused voltage monitors

[Benjamin Bara]

On 7/14/22 11:36, DLG Adam Thomson wrote:

Thanks for the quick feedback.

> What exactly is the problem you're attempting to resolve through this patch?

I have some use cases where certain modules (e.g. audio) are not required.
Therefore, I have removed a couple of "always-on" from my DT to let the kernel
decide if the regulators are needed.
Since unfortunately some of the later disabled LDOs are monitored, the state
becomes invalid.


> If you disable monitoring of a regulator, and no other regulators are being
> monitored at that point, then GP_FB2 will no longer be asserted and you
> have the same issue.

Thanks for the clarification, I wasn't aware of that.
It's clear to me now that this would become a general problem.


> What's more this patch impacts all users and there's no means to re-enable
> monitoring.

I am aware that the patch is not complete to handle the whole voltage
monitoring of the da9063.
So if wanted, I can extend the patch to store the vmon state when disabling
it and restore it during the re-enable process (can also take a look for the
handling while sleep/suspend).

best regards & thanks again,
bb

P.S.
I checked if there is some existing kind of framework for voltage
monitoring of regulators, but I couldn't find something so far.
I can imagine it might make sense to have a DT property for
"regulator-monitor-voltage-on/-off" to override the OTP settings via DT,
but I am not sure if this is something that is needed/wanted/required.

RE: [PATCH] regulator: da9063: disable unused voltage monitors

[DLG Adam Thomson]

On 14 July 2022 20:15, Benjamin Bara wrote:

> I have some use cases where certain modules (e.g. audio) are not required.
> Therefore, I have removed a couple of "always-on" from my DT to let the kernel
> decide if the regulators are needed.
> Since unfortunately some of the later disabled LDOs are monitored, the state
> becomes invalid.

Ok, understood.

> I am aware that the patch is not complete to handle the whole voltage
> monitoring of the da9063.
> So if wanted, I can extend the patch to store the vmon state when disabling
> it and restore it during the re-enable process (can also take a look for the
> handling while sleep/suspend).

The problem is your fix isn't really a fix. If the LDO you're disabling is the
last monitored LDO, and you turn off monitoring for that LDO, your GP_FB2 line
will no longer be asserted so you still have a problem. I guess one solution
might be to enable monitoring for rails you know will always be present in your
system and disable monitoring for the ones you know will be dynamically
disabled/enabled. Only problem here is making sure that ordering is right so
you don't get to a point where all rail monitoring is disabled whilst
enabling/disabling monitoring of the specified rails. IIRC the ordering in DT
is preseved in terms of handling though so this might be trivial.

>
> best regards & thanks again,
> bb
>
> P.S.
> I checked if there is some existing kind of framework for voltage
> monitoring of regulators, but I couldn't find something so far.
> I can imagine it might make sense to have a DT property for
> "regulator-monitor-voltage-on/-off" to override the OTP settings via DT,
> but I am not sure if this is something that is needed/wanted/required.

Initially I was thinking that hwmon/IIO might be the place to support something
like this but looking further I'm not sure that would work. In the absence of
an obvious place, device specific DT bindings might be the best approach, to
specify for each rail if it's monitored or not. If you did take that approach
you would need to make sure it doesn't impact existing users so the binding
*couldn't* simply be a boolean flag.