[PATCH 1/2] regulator: anatop: Add power gating support to digital LDOs

[PATCH 1/2] regulator: anatop: Add power gating support to digital LDOs

[Philipp Zabel]

The ARM, PU, and SOC LDOs in the i.MX6 PMU can completely gate
their power output. Since power gating is configured by writing
zero to the voltage target bitfield,, store a copy of the
voltage selector to be restored when reenabling the regulator.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/regulator/anatop-regulator.c | 59 +++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index c734d09..93a6413 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -34,6 +34,8 @@
 #define LDO_RAMP_UP_UNIT_IN_CYCLES      64 /* 64 cycles per step */
 #define LDO_RAMP_UP_FREQ_IN_MHZ         24 /* cycle based on 24M OSC */
 
+#define LDO_POWER_GATE			0x00
+
 struct anatop_regulator {
 	const char *name;
 	u32 control_reg;
@@ -48,17 +50,28 @@ struct anatop_regulator {
 	int max_voltage;
 	struct regulator_desc rdesc;
 	struct regulator_init_data *initdata;
+	int sel;
 };
 
+static inline bool anatop_is_core_reg(struct anatop_regulator *anatop_reg)
+{
+	/* Only core regulators have the ramp up delay configuration. */
+	return anatop_reg->delay_bit_width;
+}
+
 static int anatop_regmap_set_voltage_sel(struct regulator_dev *reg,
 					unsigned selector)
 {
 	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
+	int ret;
 
 	if (!anatop_reg->control_reg)
 		return -ENOTSUPP;
 
-	return regulator_set_voltage_sel_regmap(reg, selector);
+	ret = regulator_set_voltage_sel_regmap(reg, selector);
+	if (!ret)
+		anatop_reg->sel = selector;
+	return ret;
 }
 
 static int anatop_regmap_set_voltage_time_sel(struct regulator_dev *reg,
@@ -97,7 +110,40 @@ static int anatop_regmap_get_voltage_sel(struct regulator_dev *reg)
 	return regulator_get_voltage_sel_regmap(reg);
 }
 
+static int anatop_regmap_enable(struct regulator_dev *reg)
+{
+	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
+
+	if (!anatop_is_core_reg(anatop_reg))
+		return 0;
+
+	return regulator_set_voltage_sel_regmap(reg, anatop_reg->sel);
+}
+
+static int anatop_regmap_disable(struct regulator_dev *reg)
+{
+	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
+
+	if (!anatop_is_core_reg(anatop_reg))
+		return -ENOTSUPP;
+
+	return regulator_set_voltage_sel_regmap(reg, LDO_POWER_GATE);
+}
+
+static int anatop_regmap_is_enabled(struct regulator_dev *reg)
+{
+	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
+
+	if (!anatop_is_core_reg(anatop_reg))
+		return 1;
+
+	return regulator_get_voltage_sel_regmap(reg) != LDO_POWER_GATE;
+}
+
 static struct regulator_ops anatop_rops = {
+	.enable = anatop_regmap_enable,
+	.disable = anatop_regmap_disable,
+	.is_enabled = anatop_regmap_is_enabled,
 	.set_voltage_sel = anatop_regmap_set_voltage_sel,
 	.set_voltage_time_sel = anatop_regmap_set_voltage_time_sel,
 	.get_voltage_sel = anatop_regmap_get_voltage_sel,
@@ -116,6 +162,7 @@ static int anatop_regulator_probe(struct platform_device *pdev)
 	struct regulator_init_data *initdata;
 	struct regulator_config config = { };
 	int ret = 0;
+	u32 val;
 
 	initdata = of_get_regulator_init_data(dev, np);
 	sreg = devm_kzalloc(dev, sizeof(*sreg), GFP_KERNEL);
@@ -199,6 +246,16 @@ static int anatop_regulator_probe(struct platform_device *pdev)
 	config.of_node = pdev->dev.of_node;
 	config.regmap = sreg->anatop;
 
+	if (sreg->control_reg && anatop_is_core_reg(sreg)) {
+		ret = regmap_read(config.regmap, rdesc->vsel_reg, &val);
+		if (ret) {
+			dev_err(dev, "failed to read initial state\n");
+			goto anatop_probe_end;
+		}
+
+		sreg->sel = (val & rdesc->vsel_mask) >> sreg->vol_bit_shift;
+	}
+
 	/* register regulator */
 	rdev = devm_regulator_register(dev, rdesc, &config);
 	if (IS_ERR(rdev)) {
-- 
1.8.5.3

[PATCH 2/2] regulator: anatop: Add bypass support to digital LDOs

[Philipp Zabel]

The ARM, PU, and SOC LDOs in the i.MX6 PMU can operate
in bypass mode. This allows to use external switching
regulators for cpu voltage scaling.

Since bypass and power gating modes are not configured
with their own bits, but via the voltage target bitfield,
store bypass state to be restored when reenabling the
regulator.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/regulator/anatop-regulator.c | 55 +++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 93a6413..7cfaa51 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -35,6 +35,7 @@
 #define LDO_RAMP_UP_FREQ_IN_MHZ         24 /* cycle based on 24M OSC */
 
 #define LDO_POWER_GATE			0x00
+#define LDO_FET_FULL_ON			0x1f
 
 struct anatop_regulator {
 	const char *name;
@@ -50,6 +51,7 @@ struct anatop_regulator {
 	int max_voltage;
 	struct regulator_desc rdesc;
 	struct regulator_init_data *initdata;
+	bool bypass;
 	int sel;
 };
 
@@ -68,6 +70,11 @@ static int anatop_regmap_set_voltage_sel(struct regulator_dev *reg,
 	if (!anatop_reg->control_reg)
 		return -ENOTSUPP;
 
+	if (anatop_reg->bypass) {
+		anatop_reg->sel = selector;
+		return 0;
+	}
+
 	ret = regulator_set_voltage_sel_regmap(reg, selector);
 	if (!ret)
 		anatop_reg->sel = selector;
@@ -107,17 +114,22 @@ static int anatop_regmap_get_voltage_sel(struct regulator_dev *reg)
 	if (!anatop_reg->control_reg)
 		return -ENOTSUPP;
 
+	if (anatop_reg->bypass)
+		return anatop_reg->sel;
+
 	return regulator_get_voltage_sel_regmap(reg);
 }
 
 static int anatop_regmap_enable(struct regulator_dev *reg)
 {
 	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
+	int sel;
 
 	if (!anatop_is_core_reg(anatop_reg))
 		return 0;
 
-	return regulator_set_voltage_sel_regmap(reg, anatop_reg->sel);
+	sel = anatop_reg->bypass ? LDO_FET_FULL_ON : anatop_reg->sel;
+	return regulator_set_voltage_sel_regmap(reg, sel);
 }
 
 static int anatop_regmap_disable(struct regulator_dev *reg)
@@ -140,6 +152,41 @@ static int anatop_regmap_is_enabled(struct regulator_dev *reg)
 	return regulator_get_voltage_sel_regmap(reg) != LDO_POWER_GATE;
 }
 
+static int anatop_regmap_get_bypass(struct regulator_dev *reg, bool *enable)
+{
+	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
+	int sel;
+
+	if (!anatop_is_core_reg(anatop_reg))
+		return -ENOTSUPP;
+
+	sel = regulator_get_voltage_sel_regmap(reg);
+	if (sel == LDO_FET_FULL_ON)
+		WARN_ON(!anatop_reg->bypass);
+	else if (sel != LDO_POWER_GATE)
+		WARN_ON(anatop_reg->bypass);
+
+	*enable = anatop_reg->bypass;
+	return 0;
+}
+
+static int anatop_regmap_set_bypass(struct regulator_dev *reg, bool enable)
+{
+	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
+	int sel;
+
+	if (!anatop_is_core_reg(anatop_reg))
+		return -ENOTSUPP;
+
+	if (enable == anatop_reg->bypass)
+		return 0;
+
+	sel = enable ? LDO_FET_FULL_ON : anatop_reg->sel;
+	anatop_reg->bypass = enable;
+
+	return regulator_set_voltage_sel_regmap(reg, sel);
+}
+
 static struct regulator_ops anatop_rops = {
 	.enable = anatop_regmap_enable,
 	.disable = anatop_regmap_disable,
@@ -149,6 +196,8 @@ static struct regulator_ops anatop_rops = {
 	.get_voltage_sel = anatop_regmap_get_voltage_sel,
 	.list_voltage = regulator_list_voltage_linear,
 	.map_voltage = regulator_map_voltage_linear,
+	.get_bypass = anatop_regmap_get_bypass,
+	.set_bypass = anatop_regmap_set_bypass,
 };
 
 static int anatop_regulator_probe(struct platform_device *pdev)
@@ -254,6 +303,10 @@ static int anatop_regulator_probe(struct platform_device *pdev)
 		}
 
 		sreg->sel = (val & rdesc->vsel_mask) >> sreg->vol_bit_shift;
+		if (sreg->sel == LDO_FET_FULL_ON) {
+			sreg->sel = 0;
+			sreg->bypass = true;
+		}
 	}
 
 	/* register regulator */
-- 
1.8.5.3

Re: [PATCH 1/2] regulator: anatop: Add power gating support to digital LDOs

[Shawn Guo]

On Thu, Feb 06, 2014 at 03:43:32PM +0100, Philipp Zabel wrote:
> The ARM, PU, and SOC LDOs in the i.MX6 PMU can completely gate
> their power output. Since power gating is configured by writing
> zero to the voltage target bitfield,, store a copy of the
> voltage selector to be restored when reenabling the regulator.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

Nice way to get disabling and bypass supported!  For both patches,

Acked-by: Shawn Guo <shawn.guo@linaro.org>

Re: [PATCH 1/2] regulator: anatop: Add power gating support to digital LDOs

[Mark Brown]

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

On Thu, Feb 06, 2014 at 03:43:32PM +0100, Philipp Zabel wrote:
> The ARM, PU, and SOC LDOs in the i.MX6 PMU can completely gate
> their power output. Since power gating is configured by writing
> zero to the voltage target bitfield,, store a copy of the
> voltage selector to be restored when reenabling the regulator.

This is mostly good but...

>  static int anatop_regmap_set_voltage_sel(struct regulator_dev *reg,
>  					unsigned selector)
>  {
>  	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
> +	int ret;
>  
>  	if (!anatop_reg->control_reg)
>  		return -ENOTSUPP;
>  
> -	return regulator_set_voltage_sel_regmap(reg, selector);
> +	ret = regulator_set_voltage_sel_regmap(reg, selector);
> +	if (!ret)
> +		anatop_reg->sel = selector;
> +	return ret;
>  }

...I don't understand this.  If the regulator is disabled won't this
cause it to be reenabled since we just write the stored selector in to
do that?  What I'd expect to see happening is the data being written to
the cache always and only written to the hardware if it's enabled.

> +static int anatop_regmap_disable(struct regulator_dev *reg)
> +{
> +	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
> +
> +	if (!anatop_is_core_reg(anatop_reg))
> +		return -ENOTSUPP;
> +
> +	return regulator_set_voltage_sel_regmap(reg, LDO_POWER_GATE);
> +}

It's starting to seem like it's worth having separate ops for the core
regulator rather than all these conditionals.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] regulator: anatop: Add power gating support to digital LDOs

[Philipp Zabel]

Hi Mark,

Am Montag, den 10.02.2014, 13:15 +0000 schrieb Mark Brown:
> On Thu, Feb 06, 2014 at 03:43:32PM +0100, Philipp Zabel wrote:
> > The ARM, PU, and SOC LDOs in the i.MX6 PMU can completely gate
> > their power output. Since power gating is configured by writing
> > zero to the voltage target bitfield,, store a copy of the
> > voltage selector to be restored when reenabling the regulator.
> 
> This is mostly good but...
>
> >  static int anatop_regmap_set_voltage_sel(struct regulator_dev *reg,
> >  					unsigned selector)
> >  {
> >  	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
> > +	int ret;
> >  
> >  	if (!anatop_reg->control_reg)
> >  		return -ENOTSUPP;
> >  
> > -	return regulator_set_voltage_sel_regmap(reg, selector);
> > +	ret = regulator_set_voltage_sel_regmap(reg, selector);
> > +	if (!ret)
> > +		anatop_reg->sel = selector;
> > +	return ret;
> >  }
> 
> ...I don't understand this.  If the regulator is disabled won't this
> cause it to be reenabled since we just write the stored selector in to
> do that?  What I'd expect to see happening is the data being written to
> the cache always and only written to the hardware if it's enabled.

yes, thanks. I'll fix this. What should happen if the
regulator_set_voltage_sel_regmap fails? Is it ok to still
set the cache in this case?

> > +static int anatop_regmap_disable(struct regulator_dev *reg)
> > +{
> > +	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
> > +
> > +	if (!anatop_is_core_reg(anatop_reg))
> > +		return -ENOTSUPP;
> > +
> > +	return regulator_set_voltage_sel_regmap(reg, LDO_POWER_GATE);
> > +}
> 
> It's starting to seem like it's worth having separate ops for the core
> regulator rather than all these conditionals.

Will do.

regards
Philipp

Re: [PATCH 1/2] regulator: anatop: Add power gating support to digital LDOs

[Mark Brown]

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

On Mon, Feb 10, 2014 at 02:27:55PM +0100, Philipp Zabel wrote:

> yes, thanks. I'll fix this. What should happen if the
> regulator_set_voltage_sel_regmap fails? Is it ok to still
> set the cache in this case?

Either way is fine but not updating the cache is probably better - if it
was a physical write we'd not actually be sure if the write made it to
hardware (it could also have written nonsense!) so we can't really
guarantee what the state is after a failed operation.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]