[PATCH v3 0/2] Re-Enable support to disable switch regulators

[PATCH v3 0/2] Re-Enable support to disable switch regulators

[Marco Felsch]

Hi,

Anson had added the support to disable the switched regulators, but
there were regressions [1] with old dtb's, so the commit was reverted [2].
At all, the support to disable the switch regulators seems to me to be a
good feature. But we have to add a special dt-property to avoid
regressions with older kernels. The property allows the user to decide if
the switch regulators should be disabled or not.

Since the revert patch is on Marks regulator repo, my patches are based
on his repo too. Each commit has a changelog, so I ommit it here. I
tested it on a custom board.

Used Kernel:
Repo:
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
Branch:
regulator/for-4.19

Regards,
Marco

[1] https://patchwork.kernel.org/patch/10490381/
[2] https://patchwork.kernel.org/patch/10500333/

Marco Felsch (2):
  dt-bindings: pfuze100: add optional disable switch-regulators binding
  regulator: pfuze100: add support to en-/disable switch regulators

 .../bindings/regulator/pfuze100.txt           |  9 +++++
 drivers/regulator/pfuze100-regulator.c        | 36 +++++++++++++++++++
 2 files changed, 45 insertions(+)

-- 
2.18.0

[PATCH v3 1/2] dt-bindings: pfuze100: add optional disable switch-regulators binding

[Marco Felsch]

This binding is used to keep the backward compatibility with the current
dtb's [1]. The binding informs the driver that the unused switch regulators
can be disabled.
If it is not specified, the driver doesn't disable the switch regulators.

[1] https://patchwork.kernel.org/patch/10490381/

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

---
Changes in v3:
  - rename dt binding, add -sw suffix
  - fix description

Changes in V2:
  - add more information about the binding
  - rename binding and add vendor prefix

 Documentation/devicetree/bindings/regulator/pfuze100.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/pfuze100.txt b/Documentation/devicetree/bindings/regulator/pfuze100.txt
index 672c939045ff..c7610718adff 100644
--- a/Documentation/devicetree/bindings/regulator/pfuze100.txt
+++ b/Documentation/devicetree/bindings/regulator/pfuze100.txt
@@ -4,6 +4,15 @@ Required properties:
 - compatible: "fsl,pfuze100", "fsl,pfuze200", "fsl,pfuze3000", "fsl,pfuze3001"
 - reg: I2C slave address
 
+Optional properties:
+- fsl,pfuze-support-disable-sw: Boolean, if present disable all unused switch
+  regulators to save power consumption. Attention, ensure that all important
+  regulators (e.g. DDR ref, DDR supply) has set the "regulator-always-on"
+  property. If not present, the switched regualtors are always on and can't be
+  disabled. This binding is a workaround to keep backward compatibility with
+  old dtb's which rely on the fact that the switched regulators are always on
+  and don't mark them explicit as "regulator-always-on".
+
 Required child node:
 - regulators: This is the list of child nodes that specify the regulator
   initialization data for defined regulators. Please refer to below doc
-- 
2.18.0

[PATCH v3 2/2] regulator: pfuze100: add support to en-/disable switch regulators

[Marco Felsch]

Add enable/disable support for switch regulators on pfuze100.

Based on commit 5fe156f1cab4 ("regulator: pfuze100: add enable/disable for
switch") which is reverted due to boot regressions by commit 464a5686e6c9
("regulator: Revert "regulator: pfuze100: add enable/disable for switch"").
Disabling the switch regulators will only be done if the user specifies
"fsl,pfuze-support-disable-sw" in its device tree to keep backward
compatibility with current dtb's [1].

[1] https://patchwork.kernel.org/patch/10490381/

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

---
Changes in V3:
  - Change dt binding, add -sw suffix

Changes in v2:
  - Don't trick the framework, use a seperate ops struct
    to register the correct callbacks.
  - Set the desc en/disable_val and enable_time only if it necessary
  - Change the dt property binding

Changes since https://patchwork.kernel.org/patch/10405723/
  - Use DT property to keep backward compatibility
  - Use the default register val 0x8 as enable_val

 drivers/regulator/pfuze100-regulator.c | 36 ++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/regulator/pfuze100-regulator.c b/drivers/regulator/pfuze100-regulator.c
index cde6eda1d283..31c3a236120a 100644
--- a/drivers/regulator/pfuze100-regulator.c
+++ b/drivers/regulator/pfuze100-regulator.c
@@ -17,6 +17,8 @@
 #include <linux/slab.h>
 #include <linux/regmap.h>
 
+#define PFUZE_FLAG_DISABLE_SW	BIT(1)
+
 #define PFUZE_NUMREGS		128
 #define PFUZE100_VOL_OFFSET	0
 #define PFUZE100_STANDBY_OFFSET	1
@@ -50,10 +52,12 @@ struct pfuze_regulator {
 	struct regulator_desc desc;
 	unsigned char stby_reg;
 	unsigned char stby_mask;
+	bool sw_reg;
 };
 
 struct pfuze_chip {
 	int	chip_id;
+	int     flags;
 	struct regmap *regmap;
 	struct device *dev;
 	struct pfuze_regulator regulator_descs[PFUZE100_MAX_REGULATOR];
@@ -170,6 +174,17 @@ static const struct regulator_ops pfuze100_sw_regulator_ops = {
 	.set_ramp_delay = pfuze100_set_ramp_delay,
 };
 
+static const struct regulator_ops pfuze100_sw_disable_regulator_ops = {
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_linear,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_time_sel = regulator_set_voltage_time_sel,
+	.set_ramp_delay = pfuze100_set_ramp_delay,
+};
+
 static const struct regulator_ops pfuze100_swb_regulator_ops = {
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -209,9 +224,12 @@ static const struct regulator_ops pfuze100_swb_regulator_ops = {
 			.uV_step = (step),	\
 			.vsel_reg = (base) + PFUZE100_VOL_OFFSET,	\
 			.vsel_mask = 0x3f,	\
+			.enable_reg = (base) + PFUZE100_MODE_OFFSET,	\
+			.enable_mask = 0xf,	\
 		},	\
 		.stby_reg = (base) + PFUZE100_STANDBY_OFFSET,	\
 		.stby_mask = 0x3f,	\
+		.sw_reg = true,		\
 	}
 
 #define PFUZE100_SWB_REG(_chip, _name, base, mask, voltages)	\
@@ -471,6 +489,9 @@ static int pfuze_parse_regulators_dt(struct pfuze_chip *chip)
 	if (!np)
 		return -EINVAL;
 
+	if (of_property_read_bool(np, "fsl,pfuze-support-disable-sw"))
+		chip->flags |= PFUZE_FLAG_DISABLE_SW;
+
 	parent = of_get_child_by_name(np, "regulators");
 	if (!parent) {
 		dev_err(dev, "regulators node not found\n");
@@ -703,6 +724,21 @@ static int pfuze100_regulator_probe(struct i2c_client *client,
 			}
 		}
 
+		/*
+		 * Allow SW regulators to turn off. Checking it trough a flag is
+		 * a workaround to keep the backward compatibility with existing
+		 * old dtb's which may relay on the fact that we didn't disable
+		 * the switched regulator till yet.
+		 */
+		if (pfuze_chip->flags & PFUZE_FLAG_DISABLE_SW) {
+			if (pfuze_chip->regulator_descs[i].sw_reg) {
+				desc->ops = &pfuze100_sw_disable_regulator_ops;
+				desc->enable_val = 0x8;
+				desc->disable_val = 0x0;
+				desc->enable_time = 500;
+			}
+		}
+
 		config.dev = &client->dev;
 		config.init_data = init_data;
 		config.driver_data = pfuze_chip;
-- 
2.18.0

Re: [PATCH v3 1/2] dt-bindings: pfuze100: add optional disable switch-regulators binding

[Fabio Estevam]

On Mon, Jul 23, 2018 at 4:47 AM, Marco Felsch <m.felsch@pengutronix.de> wrote:
> This binding is used to keep the backward compatibility with the current
> dtb's [1]. The binding informs the driver that the unused switch regulators
> can be disabled.
> If it is not specified, the driver doesn't disable the switch regulators.
>
> [1] https://patchwork.kernel.org/patch/10490381/
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

Re: [PATCH v3 1/2] dt-bindings: pfuze100: add optional disable switch-regulators binding

[Fabio Estevam]

On Mon, Jul 23, 2018 at 4:47 AM, Marco Felsch <m.felsch@pengutronix.de> wrote:
> This binding is used to keep the backward compatibility with the current
> dtb's [1]. The binding informs the driver that the unused switch regulators
> can be disabled.
> If it is not specified, the driver doesn't disable the switch regulators.
>
> [1] https://patchwork.kernel.org/patch/10490381/
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

Re: [PATCH v3 2/2] regulator: pfuze100: add support to en-/disable switch regulators

[Fabio Estevam]

On Mon, Jul 23, 2018 at 4:47 AM, Marco Felsch <m.felsch@pengutronix.de> wrote:
> Add enable/disable support for switch regulators on pfuze100.
>
> Based on commit 5fe156f1cab4 ("regulator: pfuze100: add enable/disable for
> switch") which is reverted due to boot regressions by commit 464a5686e6c9
> ("regulator: Revert "regulator: pfuze100: add enable/disable for switch"").
> Disabling the switch regulators will only be done if the user specifies
> "fsl,pfuze-support-disable-sw" in its device tree to keep backward
> compatibility with current dtb's [1].
>
> [1] https://patchwork.kernel.org/patch/10490381/
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

Re: [PATCH v3 2/2] regulator: pfuze100: add support to en-/disable switch regulators

[Fabio Estevam]

On Mon, Jul 23, 2018 at 4:47 AM, Marco Felsch <m.felsch@pengutronix.de> wrote:
> Add enable/disable support for switch regulators on pfuze100.
>
> Based on commit 5fe156f1cab4 ("regulator: pfuze100: add enable/disable for
> switch") which is reverted due to boot regressions by commit 464a5686e6c9
> ("regulator: Revert "regulator: pfuze100: add enable/disable for switch"").
> Disabling the switch regulators will only be done if the user specifies
> "fsl,pfuze-support-disable-sw" in its device tree to keep backward
> compatibility with current dtb's [1].
>
> [1] https://patchwork.kernel.org/patch/10490381/
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

Re: [PATCH v3 1/2] dt-bindings: pfuze100: add optional disable switch-regulators binding

[Rob Herring]

On Mon, Jul 23, 2018 at 09:47:46AM +0200, Marco Felsch wrote:
> This binding is used to keep the backward compatibility with the current
> dtb's [1]. The binding informs the driver that the unused switch regulators
> can be disabled.
> If it is not specified, the driver doesn't disable the switch regulators.
> 
> [1] https://patchwork.kernel.org/patch/10490381/
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> 
> ---
> Changes in v3:
>   - rename dt binding, add -sw suffix
>   - fix description
> 
> Changes in V2:
>   - add more information about the binding
>   - rename binding and add vendor prefix
> 
>  Documentation/devicetree/bindings/regulator/pfuze100.txt | 9 +++++++++
>  1 file changed, 9 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>