[PATCH 1/2] regulator: support operating mode in the device tree structure

[PATCH 1/2] regulator: support operating mode in the device tree structure

[Kim, Milo]

For better power efficiency, regulator subsystem provides operating mode.
(fast, normal, idle and standby)
The operating mode is selective by a regulator consumer.

With this patch, regulator operating mode can be set in the DT structure.
If any operating mode is configured, then 'valid_ops_mask' should be set with
REGULATOR_CHANGE_MODE.

Signed-off-by: Milo(Woogyom) Kim <milo.kim@ti.com>
---
 .../devicetree/bindings/regulator/regulator.txt    |    4 ++++
 drivers/regulator/of_regulator.c                   |   13 +++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index ecfc6cc..2b89616 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -11,6 +11,10 @@ Optional properties:
 - regulator-boot-on: bootloader/firmware enabled regulator
 - <name>-supply: phandle to the parent supply/regulator node
 - regulator-ramp-delay: ramp delay for regulator(in uV/uS)
+- regulator-mode-fast: fast operating mode
+- regulator-mode-normal: normal operating mode
+- regulator-mode-idle: idle operating mode
+- regulator-mode-standby: standby operating mode
 
 Deprecated properties:
 - regulator-compatible: If a regulator chip contains multiple
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 66ca769..aebe9a4 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -64,6 +64,19 @@ static void of_get_regulation_constraints(struct device_node *np,
 	ramp_delay = of_get_property(np, "regulator-ramp-delay", NULL);
 	if (ramp_delay)
 		constraints->ramp_delay = be32_to_cpu(*ramp_delay);
+
+	/* Operation mode configuration */
+	if (of_find_property(np, "regulator-mode-fast", NULL))
+		constraints->valid_modes_mask |= REGULATOR_MODE_FAST;
+	if (of_find_property(np, "regulator-mode-normal", NULL))
+		constraints->valid_modes_mask |= REGULATOR_MODE_NORMAL;
+	if (of_find_property(np, "regulator-mode-idle", NULL))
+		constraints->valid_modes_mask |= REGULATOR_MODE_IDLE;
+	if (of_find_property(np, "regulator-mode-standby", NULL))
+		constraints->valid_modes_mask |= REGULATOR_MODE_STANDBY;
+
+	if (constraints->valid_modes_mask)
+		constraints->valid_ops_mask |= REGULATOR_CHANGE_MODE;
 }
 
 /**
-- 
1.7.9.5


Best Regards,
Milo

Re: [PATCH 1/2] regulator: support operating mode in the device tree structure

[Mark Brown]

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

On Tue, May 07, 2013 at 01:09:14PM +0000, Kim, Milo wrote:
> For better power efficiency, regulator subsystem provides operating mode.
> (fast, normal, idle and standby)
> The operating mode is selective by a regulator consumer.
> 
> With this patch, regulator operating mode can be set in the DT structure.
> If any operating mode is configured, then 'valid_ops_mask' should be set with
> REGULATOR_CHANGE_MODE.

It's not clear to me that these modes are a useful thing to be exposing
in device tree - they've always been rather arbatrary and are becoming
less and less relevant as regulator hardware improves in its ability to
adapt to load.  I think for fixed modes that don't vary dynamically at
runtime the best thing is just to make the modes custom so they map onto
the device datasheets clearly.  That seems to be most of the usage
anyway.

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

RE: [PATCH 1/2] regulator: support operating mode in the device tree structure

[Kim, Milo]

> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Wednesday, May 08, 2013 1:05 AM
> To: Kim, Milo
> Cc: Liam Girdwood; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/2] regulator: support operating mode in the
> device tree structure
> 
> On Tue, May 07, 2013 at 01:09:14PM +0000, Kim, Milo wrote:
> > For better power efficiency, regulator subsystem provides operating
> mode.
> > (fast, normal, idle and standby)
> > The operating mode is selective by a regulator consumer.
> >
> > With this patch, regulator operating mode can be set in the DT
> structure.
> > If any operating mode is configured, then 'valid_ops_mask' should be
> set with
> > REGULATOR_CHANGE_MODE.
> 
> It's not clear to me that these modes are a useful thing to be exposing
> in device tree - they've always been rather arbatrary and are becoming
> less and less relevant as regulator hardware improves in its ability to
> adapt to load.  I think for fixed modes that don't vary dynamically at
> runtime the best thing is just to make the modes custom so they map
> onto
> the device datasheets clearly.  That seems to be most of the usage
> anyway.

I agree the operating mode depends on the device.

It's my intention to guarantee same operations in the DT as the regulator
constraints in the platform side.
In case a regulator has specific 'valid_modes_mask', the operating mode is 
unable to be set with the device tree.
For some regulator drivers such as LP8720/5 and LP8788, this patch would be
useful because there is no difference between configuration in the platform side
 and the device tree.

Thanks,
Milo

Re: [PATCH 1/2] regulator: support operating mode in the device tree structure

[Mark Brown]

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

On Tue, May 07, 2013 at 11:46:12PM +0000, Kim, Milo wrote:

> It's my intention to guarantee same operations in the DT as the regulator
> constraints in the platform side.
> In case a regulator has specific 'valid_modes_mask', the operating mode is 
> unable to be set with the device tree.
> For some regulator drivers such as LP8720/5 and LP8788, this patch would be
> useful because there is no difference between configuration in the platform side
>  and the device tree.

I understand what the patch is doing but there's no reason why the
device tree binding needs to map directly onto our internal
configuration and indeed in cases where we didn't make ideal choices
that's not what we want to do.  Providing a binding that corresponds to
what the chip itself calls the configuration seems more useful.

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