diff mbox

[3/4] devicetree: add a binding for a group of regulator

Message ID 1416834123-23139-3-git-send-email-alban.bedel@avionic-design.de
State Needs Review / ACK, archived
Headers show

Checks

Context Check Description
robh/checkpatch warning total: 1 errors, 0 warnings, 0 lines checked
robh/patch-applied success

Commit Message

Alban Bedel Nov. 24, 2014, 1:02 p.m. UTC
Signed-off-by: Alban Bedel <alban.bedel@avionic-design.de>
---
 .../devicetree/bindings/regulator/group.txt        | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/group.txt

Comments

Mark Brown Nov. 24, 2014, 3:24 p.m. UTC | #1
On Mon, Nov 24, 2014 at 02:02:02PM +0100, Alban Bedel wrote:

> +This binding allow creating a group of regulators for use with simple
> +drivers that only expect a single power supply. Additionally it is
> +possible to enforce the enable ordering to create simple power up
> +sequences.

Absoutely not, this sort of scripting is not sensible - if the consumer
device has multiple supplies the consumer device should be working with
them independently and if the consumer has ordering constraints it needs
to enforce them itself.  Trying to solve this problem with a bodge in
the regulator API just isn't the right place, leaving aside the above
most power sequences involve things other than regulators like clocks
and reset signals so just doing things purely at the regulator API level
isn't ging to solve the problem.  

Please look for the generic power sequence stuff that was getting
discussed a while back and try to resurrect that if you feel there's a
compelling reason to have this functionality without doing it for
drivers.
Alban Bedel Nov. 24, 2014, 5:32 p.m. UTC | #2
On Mon, 24 Nov 2014 15:24:33 +0000
Mark Brown <broonie@kernel.org> wrote:

> On Mon, Nov 24, 2014 at 02:02:02PM +0100, Alban Bedel wrote:
> 
> > +This binding allow creating a group of regulators for use with simple
> > +drivers that only expect a single power supply. Additionally it is
> > +possible to enforce the enable ordering to create simple power up
> > +sequences.
> 
> Absoutely not, this sort of scripting is not sensible - if the consumer
> device has multiple supplies the consumer device should be working with
> them independently and if the consumer has ordering constraints it needs
> to enforce them itself.  Trying to solve this problem with a bodge in
> the regulator API just isn't the right place, leaving aside the above
> most power sequences involve things other than regulators like clocks
> and reset signals so just doing things purely at the regulator API level
> isn't ging to solve the problem.  
> 
> Please look for the generic power sequence stuff that was getting
> discussed a while back and try to resurrect that if you feel there's a
> compelling reason to have this functionality without doing it for
> drivers.

Honestly my primary aim wasn't the sequencing, but rather to increase
the usefulness of generic drivers. Generic driver generally only
manipulate a single supply, however many hardware might have more,
and won't need any specific power up ordering. Having to write a full
new driver just because of an extra supply doesn't seems to make much
sense to me.

As alternative solution to this problem I though about allowing a list
of regulator for the supplies:

 vin-supply = <&reg1>, <&reg2>;

The API could still return a single consumer but it would operate on
all the regulators in the list instead of just one. Would that be a
better solution?

Alban
Mark Brown Nov. 24, 2014, 5:55 p.m. UTC | #3
On Mon, Nov 24, 2014 at 06:32:04PM +0100, Alban Bedel wrote:

> Honestly my primary aim wasn't the sequencing, but rather to increase
> the usefulness of generic drivers. Generic driver generally only
> manipulate a single supply, however many hardware might have more,
> and won't need any specific power up ordering. Having to write a full
> new driver just because of an extra supply doesn't seems to make much
> sense to me.

I'm having a really hard time following the above - you say "generic
driver generally only manipulate a single supply" but that's absolutely
not the case.  A driver should control exactly as many supplies as the
device it is controlling does.  

The nearest I can get to something that I think I can understand is a
device variant that has some changes in supplies but varaints aren't
something that we need to write entirely new drivers for, just new
device IDs and a few lines of conditional code.

It's possible that I'm missing something but I'm really struggling to
see the problem that you're trying to solve here or why this is an
abstraction that makes sense.

> As alternative solution to this problem I though about allowing a list
> of regulator for the supplies:

>  vin-supply = <&reg1>, <&reg2>;

> The API could still return a single consumer but it would operate on
> all the regulators in the list instead of just one. Would that be a
> better solution?

No, that's even worse - this is just hacking around whatever problem
you're facing.  The device tree should accurately describe the hardware
not some random thing that vaguely looks like the hardware because it
happens to let us shoehorn things onto it.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/regulator/group.txt b/Documentation/devicetree/bindings/regulator/group.txt
new file mode 100644
index 0000000..5f811cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/group.txt
@@ -0,0 +1,26 @@ 
+Regulator Group
+
+This binding allow creating a group of regulators for use with simple
+drivers that only expect a single power supply. Additionally it is
+possible to enforce the enable ordering to create simple power up
+sequences.
+
+Required properties:
+- compatible : Must be "regulator-group".
+- regulator-supplies : List of the supplies names.
+- <name>-supply : One entry for each supply defined in regulator-supplies.
+
+Optional properties:
+- ordered-supplies : set if the supplies should be enabled in order,
+   otherwise they are all enable or disabled in parallel.
+- any property defined in regulator.txt
+
+Example:
+
+  regulator {
+	compatible = "regulator-group";
+	regulator-supplies = "vcc_a", "vcc_b";
+	vcc_a-supply = <&vcc_a>;
+	vcc_b-supply = <&vcc_b>;
+	ordered-supplies;
+  };