regulator: gpio-regulator: add DT bindings

PATCH In review Daniel Mack 3 participants 5 messages Sep 10, 2012 – Sep 12, 2012

Subsystems (2)

Device tree bindings Voltage and current regulator framework

lore

Share & tools

lore

Daniel Mack

author

2012-09-10 15:03 UTC #0 lore raw

PATCH v2 1/1 regulator: gpio-regulator: add DT bindings

Add DT bindings for the gpio-regulator driver and some documentation on how to use it.

Signed-off-by Daniel Mack <[email protected]>

---
v1 -> v2
Fixed wrong kzalloc() size.

 .../bindings/regulator/gpio-regulator.txt          |  58 ++++++++++
 drivers/regulator/gpio-regulator.c                 | 118 ++++++++++++++++++++-
 2 files changed, 175 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/gpio-regulator.txt

Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +58 -0

new file mode 100644
index 0000000..ea3eb09
@@ -0,0 +1,58 @@
+Device tree bindings for GPIO controlled voltages
+
+Voltage or current regulators on boards that are controlled by GPIOs can
+be used by the gpio-regulator driver.
+
+Required properties:
+
+ - "compatible":	must be set to "gpio-regulator"
+ - "regulator-name": 	must be set to a string describing the name of this
+			regulator.
+
+Optional properties:
+
+ - "startup-delay":		Start-up time in microseconds
+ - "enable-high":		Polarity of enable GPIO. Active high if
+				defined, otherwise active low
+ - "enabled-at-boot":		If set, the regulator has been enabled at boot
+				time
+ - "regulator-type-voltage":	The regulator controls a voltage
+ - "regulator-type-current":	The regulator controls a current
+ - "states":			An array of possible states, describing the
+				GPIO states to reach a given value
+	- "value":		The value of the state, in microvolts or
+				microamperes
+	- "gpios":		bitfield of gpio target-states for the value
+				The n-th bit in the bitfield describes the
+				state of the n-th GPIO from the gpios-array
+
+Also, all properties described in
+Documentation/devicetree/bindings/regulator/regulator.txt are supported as
+well.
+
+Example:
+
+	reg_gpio {
+		compatible = "gpio-regulator";
+		regulator-name = "voltage";
+		regulator-enable-high;
+		regulator-type-voltage;
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-always-on;
+
+		gpios = <&gpio 23 0>;
+
+		states {
+			state-on {
+				value = <3300000>;
+				gpios = <0x1>;
+			};
+
+			state-off {
+				value = <0>;
+				gpios = <0x0>;
+			};
+		};
+	};
+

drivers/regulator/gpio-regulator.c +117 -1

146 lines not on this page · raw message

Signature · 1 line

1.7.11.4

Stephen Warren

2012-09-10 19:40 UTC #1 lore raw

Quoted 5 lines · Daniel Mack

Add DT bindings for the gpio-regulator driver and some documentation on how to use it.
+++ b/Documentation/devicetree/bindings/regulator/gpio-regulator.txt

Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 10

Quoted 10 lines

Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +10 -0

@@ -0,0 +1,58 @@
+Device tree bindings for GPIO controlled voltages
+
+Voltage or current regulators on boards that are controlled by GPIOs can
+be used by the gpio-regulator driver.
+
+Required properties:
+
+ - "compatible":	must be set to "gpio-regulator"
+ - "regulator-name": 	must be set to a string describing the name of this
+			regulator.

I'd expect the "gpios" property to be listed here.

Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 16

Quoted 5 lines

Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +5 -0

+Optional properties:
+
+ - "startup-delay":		Start-up time in microseconds
+ - "enable-high":		Polarity of enable GPIO. Active high if
+				defined, otherwise active low

fixed-regulator.txt already has an "enable-active-high" property; it'd be nice to be consistent.

Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 18

Quoted 2 lines

Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +2 -0
+ - "enabled-at-boot":		If set, the regulator has been enabled at boot
+				time

Isn't that regulator-boot-on, as defined in regulator.txt that you mentioned?

Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 20

Quoted 2 lines

Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +2 -0
+ - "regulator-type-voltage":	The regulator controls a voltage
+ - "regulator-type-current":	The regulator controls a current

I wonder if it'd be better to differentiate this using different compatible values instead?

Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 24

Quoted 4 lines

Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +4 -0

+ - "states":			An array of possible states, describing the
+				GPIO states to reach a given value
+	- "value":		The value of the state, in microvolts or
+				microamperes

A name like "microvolts", "voltage", "microamps", or "current" might be more descriptive here.

Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 27

Quoted 3 lines

Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +3 -0
+	- "gpios":		bitfield of gpio target-states for the value
+				The n-th bit in the bitfield describes the
+				state of the n-th GPIO from the gpios-array

"gpios" sounds like the name of a property that defines which GPIOs are used, rather than the value of those GPIOs. Perhaps "gpio-values" instead?

Actually, I wonder if we really even need to represent that values explicitly; rather than have a big set of nodes, perhaps you can instead have a single property that lists the voltage (or current) that each combination of GPIO values gives; something like:

reg_gpio {
    ...
    gpios = <&ggpio 23 0 &gpio 24 0>;
    voltages = <0 1000000 2000000 3000000>;
};

(As inspiration for that, I looked at Documentation/devicetree/bindings/net/mdio-mux-gpio.txt which doesn't have a property defining the value of the GPIOs for each combination, but rather a node per valid combination).

Perhaps represent and invalid GPIO combination with 0xffffffff?

Quoted 19 lines

Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +19 -0

+Also, all properties described in
+Documentation/devicetree/bindings/regulator/regulator.txt are supported as
+well.
+
+Example:
+
+	reg_gpio {
+		compatible = "gpio-regulator";
+		regulator-name = "voltage";
+		regulator-enable-high;
+		regulator-type-voltage;
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-always-on;
+
+		gpios = <&gpio 23 0>;
+
+		states {
+			state-on {

Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 57

Quoted 10 lines

Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +10 -0
+				value = <3300000>;
+				gpios = <0x1>;
+			};
+
+			state-off {
+				value = <0>;
+				gpios = <0x0>;
+			};
+		};
+	};

That particular example only has on/off states, and so might be better covered using the existing fixed-regulator, with optional GPIO control. Perhaps an example using 2 GPIOs for 4 voltage states would be more useful?

Mark Brown

2012-09-10 23:41 UTC #2 lore raw

Quoted 7 lines · Stephen Warren

Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 18
Quoted 2 lines · Daniel Mack
+ - "enabled-at-boot": If set, the regulator has been enabled at boot
Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +1 -0
+				time
Isn't that regulator-boot-on, as defined in regulator.txt that you mentioned?

It's slightly different due to our unfortunate inability to support GPIO readback for output GPIOs - this means the regualtor is enabled at boot and the driver should use that to bootstrap state, the core one means the regulator should be turned on at boot. The latter would force the regulator to be be enabled during boot.

Daniel Mack

author

2012-09-12 08:57 UTC #3 lore raw

In reply to Stephen Warren

Hi Stephen,

Quoted 4 lines · Stephen Warren

Quoted 2 lines · Daniel Mack
Add DT bindings for the gpio-regulator driver and some documentation on how to use it.

Quoted 14 lines

Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 10
Quoted 12 lines
+++ b/Documentation/devicetree/bindings/regulator/gpio-regulator.txt
Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +10 -0
@@ -0,0 +1,58 @@
+Device tree bindings for GPIO controlled voltages
+
+Voltage or current regulators on boards that are controlled by GPIOs can
+be used by the gpio-regulator driver.
+
+Required properties:
+
+ - "compatible":	must be set to "gpio-regulator"
+ - "regulator-name": 	must be set to a string describing the name of this
+			regulator.
I'd expect the "gpios" property to be listed here.

Yes, I'll add that.

Quoted 9 lines

Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 16
Quoted 6 lines
Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +5 -0
+Optional properties:
+
+ - "startup-delay":		Start-up time in microseconds
+ - "enable-high":		Polarity of enable GPIO. Active high if
+				defined, otherwise active low
fixed-regulator.txt already has an "enable-active-high" property; it'd be nice to be consistent.

Ok.

Quoted 11 lines

Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 18
Quoted 2 lines
Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +2 -0
+ - "enabled-at-boot":		If set, the regulator has been enabled at boot
+				time
Isn't that regulator-boot-on, as defined in regulator.txt that you mentioned?
Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 20
Quoted 2 lines
Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +2 -0
+ - "regulator-type-voltage":	The regulator controls a voltage
+ - "regulator-type-current":	The regulator controls a current
I wonder if it'd be better to differentiate this using different compatible values instead?

I like this idea. Will make that "gpio-voltage-regulator" and "gpio-current-regulator".

Quoted 8 lines

Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 24
Quoted 5 lines
Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +4 -0
+ - "states":			An array of possible states, describing the
+				GPIO states to reach a given value
+	- "value":		The value of the state, in microvolts or
+				microamperes
A name like "microvolts", "voltage", "microamps", or "current" might be more descriptive here.

I'm for "microvolts" and "microamperes". Will change. I just thought it might be good to keep as close to the struct member names for making it easiest for users to convert over to DT, but you're right, a more descriptive property name is certainly better.

Quoted 18 lines

Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 27
Quoted 3 lines
Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +3 -0
+	- "gpios":		bitfield of gpio target-states for the value
+				The n-th bit in the bitfield describes the
+				state of the n-th GPIO from the gpios-array
"gpios" sounds like the name of a property that defines which GPIOs are used, rather than the value of those GPIOs. Perhaps "gpio-values" instead?
Actually, I wonder if we really even need to represent that values explicitly; rather than have a big set of nodes, perhaps you can instead have a single property that lists the voltage (or current) that each combination of GPIO values gives; something like:
reg_gpio {
    ...
    gpios = <&ggpio 23 0 &gpio 24 0>;
    voltages = <0 1000000 2000000 3000000>;
};

Quoted 6 lines

(As inspiration for that, I looked at Documentation/devicetree/bindings/net/mdio-mux-gpio.txt which doesn't have a property defining the value of the GPIOs for each combination, but rather a node per valid combination).
Perhaps represent and invalid GPIO combination with 0xffffffff?

Sorry, I don't follow you on this one. How would you specify the voltages that way? And The driver allows for arbitrary GPIO patterns to be set on the GPIOs in order to reach a specific voltage. Could you give me another example how that would look like in your approach?

Once that topic is agreed on, I'll send out a new version.

Quoted 25 lines

Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +25 -0

+Also, all properties described in
+Documentation/devicetree/bindings/regulator/regulator.txt are supported as
+well.
+
+Example:
+
+	reg_gpio {
+		compatible = "gpio-regulator";
+		regulator-name = "voltage";
+		regulator-enable-high;
+		regulator-type-voltage;
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-always-on;
+
+		gpios = <&gpio 23 0>;
+
+		states {
+			state-on {
+				value = <3300000>;
+				gpios = <0x1>;
+			};
+
+			state-off {
+				value = <0>;

Quoted 8 lines

Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 57
Quoted 4 lines
Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +4 -0
+				gpios = <0x0>;
+			};
+		};
+	};
That particular example only has on/off states, and so might be better covered using the existing fixed-regulator, with optional GPIO control. Perhaps an example using 2 GPIOs for 4 voltage states would be more useful?

Yes, that's true. Funny thing is - I forgot about the fixed-regulator's feature to control GPIOs and use the driver as in the example that I provided :) Anyway, I'll finish the conversion of this one now, as I'm on it.

Many thanks for your review,

Daniel

Stephen Warren

2012-09-12 18:47 UTC #4 lore raw

Quoted 4 lines · Daniel Mack

Quoted 3 lines · Stephen Warren
Quoted 2 lines · Daniel Mack
Add DT bindings for the gpio-regulator driver and some documentation on how to use it.

...

Quoted 1 line

Quoted 1 line
Quoted 1 line
+++ b/Documentation/devicetree/bindings/regulator/gpio-regulator.txt

...

Quoted 23 lines

Quoted 23 lines
Documentation/devicetree/bindings/regulator/gpio-regulator.txt line 27
Quoted 3 lines
Documentation/devicetree/bindings/regulator/gpio-regulator.txt new file +3 -0
+	- "gpios":		bitfield of gpio target-states for the value
+				The n-th bit in the bitfield describes the
+				state of the n-th GPIO from the gpios-array
"gpios" sounds like the name of a property that defines which GPIOs are used, rather than the value of those GPIOs. Perhaps "gpio-values" instead?
Actually, I wonder if we really even need to represent that values explicitly; rather than have a big set of nodes, perhaps you can instead have a single property that lists the voltage (or current) that each combination of GPIO values gives; something like:
reg_gpio {
    ...
    gpios = <&ggpio 23 0 &gpio 24 0>;
    voltages = <0 1000000 2000000 3000000>;
};
(As inspiration for that, I looked at Documentation/devicetree/bindings/net/mdio-mux-gpio.txt which doesn't have a property defining the value of the GPIOs for each combination, but rather a node per valid combination).

Quoted 8 lines

Quoted 1 line
Perhaps represent and invalid GPIO combination with 0xffffffff?
Sorry, I don't follow you on this one. How would you specify the voltages that way? And The driver allows for arbitrary GPIO patterns to be set on the GPIOs in order to reach a specific voltage. Could you give me another example how that would look like in your approach?
Once that topic is agreed on, I'll send out a new version.

The idea is that you treat the GPIOs as an array of bits. so, two entries in the gpios property and you've got two bits in your values. Use that value as the index into the "voltages" property. For each combination of GPIO values, you have an entry in "voltages" that indicates what voltage will be produced. This is much more compact than having a node per setting, but perhaps is more wordy if the subset of legal GPIO value is much more sparse than all possible 2^n GPIO values.

Put another way,

GPIO0=0, GPIO1=0 -> index 0 -> voltage 0 in the example above GPIO0=0, GPIO1=1 -> index 1 -> voltage 1000000 in the example above GPIO0=1, GPIO1=0 -> index 2 -> voltage 2000000 in the example above GPIO0=1, GPIO1=1 -> index 3 -> voltage 3000000 in the example above