diff mbox

[v3,1/2] dt-bindings: GPIO: Add gpio-initval

Message ID 1447754878-21099-2-git-send-email-mpa@pengutronix.de
State Changes Requested, archived
Headers show

Commit Message

Markus Pargmann Nov. 17, 2015, 10:07 a.m. UTC
Add a binding for GPIO initialization.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 Documentation/devicetree/bindings/gpio/gpio.txt | 34 ++++++++++++++++---------
 1 file changed, 22 insertions(+), 12 deletions(-)

Comments

Rob Herring Nov. 17, 2015, 8:35 p.m. UTC | #1
On Tue, Nov 17, 2015 at 11:07:57AM +0100, Markus Pargmann wrote:
> Add a binding for GPIO initialization.

Some comments, but they are really more on the gpio hog. I must have 
missed them.
 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/gpio/gpio.txt | 34 ++++++++++++++++---------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
> index 069cdf6f9dac..d11abfa13add 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio.txt
> @@ -155,29 +155,39 @@ gpio-controller@00000000 {
>  	ngpios = <18>;
>  }
>  
> -The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism
> -providing automatic GPIO request and configuration as part of the
> -gpio-controller's driver probe function.
> +The GPIO chip may contain GPIO definitions. These define properties for single
> +GPIOs of this controller.
>  
> -Each GPIO hog definition is represented as a child node of the GPIO controller.
> +There are two types of GPIO definitions:
> +
> +- GPIO hogging is a mechanism providing automatic GPIO request and
> +  configuration as part of the gpio-controller driver's probe function. The
> +  GPIO is held until the gpio-controller is removed.

I can't say I like the term hogs, but that's just cosmetic.

> +- GPIO initialization provides an automatic initialization to known save
> +  values. The GPIO can be used normally afterwards.
> +
> +Each GPIO definition is represented as a child node of the GPIO controller.
>  Required properties:
> -- gpio-hog:   A property specifying that this child node represent a GPIO hog.
>  - gpios:      Store the GPIO information (id, flags, ...). Shall contain the
>  	      number of cells specified in its parent node (GPIO controller
>  	      node).
> -Only one of the following properties scanned in the order shown below.
> -This means that when multiple properties are present they will be searched
> -in the order presented below and the first match is taken as the intended
> -configuration.
> +
> +Optional properties:
> +- line-name:  The GPIO label name. If not present the node name is used.

We already have "label" for this purpose.

> +
> + The following two options are mutually exclusive. One of them should be
> + specified, but not both:
> +- gpio-hog:   A property specifying that this child node represent a GPIO hog.
> +- gpio-initval: This GPIO should be initialized to the specified configuration.
> +
> + The following three options are mutually exclusive. They change the setting of
> + the GPIO pin. One of them should be specified:
>  - input:      A property specifying to set the GPIO direction as input.
>  - output-low  A property specifying to set the GPIO direction as output with
>  	      the value low.
>  - output-high A property specifying to set the GPIO direction as output with
>  	      the value high.

If they are mutually exclusive, then it should just be one property with 
values. That is much more simple to validate.

But none of this is really even needed. We can do all this with existing 
bindings. Most gpio controllers use 2 cells and the 2nd cell was 
reserved for something like this. Simply adding a -gpios property in the 
gpio controller node would do the job:

init-gpios = <&self n (GPIO_ACTIVE_HIGH | GPIO_OUTPUT)>;
hog-gpios = <&self m GPIO_INPUT>;

Perhaps we don't want to use GPIO_ACTIVE_x here, but we have 31 more 
free bits to use.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Nov. 18, 2015, 10:20 a.m. UTC | #2
On Tue, Nov 17, 2015 at 9:35 PM, Rob Herring <robh@kernel.org> wrote:
> On Tue, Nov 17, 2015 at 11:07:57AM +0100, Markus Pargmann wrote:
>> Add a binding for GPIO initialization.
>
> Some comments, but they are really more on the gpio hog. I must have
> missed them.

They are already merged, and in use :(

>> +Optional properties:
>> +- line-name:  The GPIO label name. If not present the node name is used.
>
> We already have "label" for this purpose.

I would rather say we already have another instance of "line-name"
for this purpose. See
Documentation/devicetree/bindings/gpio/gpio.txt

"label" was not chosen because it's a Linuxism (that is what the internal
API is using) and not to the point.

I like the clear "line" specifier over "pin" etc since GPIOs can be
chip-internal and what not.

>> + The following two options are mutually exclusive. One of them should be
>> + specified, but not both:
>> +- gpio-hog:   A property specifying that this child node represent a GPIO hog.
>> +- gpio-initval: This GPIO should be initialized to the specified configuration.
>> +
>> + The following three options are mutually exclusive. They change the setting of
>> + the GPIO pin. One of them should be specified:
>>  - input:      A property specifying to set the GPIO direction as input.
>>  - output-low  A property specifying to set the GPIO direction as output with
>>             the value low.
>>  - output-high A property specifying to set the GPIO direction as output with
>>             the value high.
>
> If they are mutually exclusive, then it should just be one property with
> values. That is much more simple to validate.
>
> But none of this is really even needed. We can do all this with existing
> bindings. Most gpio controllers use 2 cells and the 2nd cell was
> reserved for something like this. Simply adding a -gpios property in the
> gpio controller node would do the job:
>
> init-gpios = <&self n (GPIO_ACTIVE_HIGH | GPIO_OUTPUT)>;
> hog-gpios = <&self m GPIO_INPUT>;
>
> Perhaps we don't want to use GPIO_ACTIVE_x here, but we have 31 more
> free bits to use.

I prefer if the flags to be used for characteristics of the line rather
than driving it in any way. Anyway the input/output-low/output-high is
already merged, and in use for hogs.

We looked at using -gpios in a self-referential manner, but that
made it impossible to name the lines, and that is a desired
property. Hogged lines need names so we can figure out who is
using them during debug etc. It also makes the DT more
human-readable.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
index 069cdf6f9dac..d11abfa13add 100644
--- a/Documentation/devicetree/bindings/gpio/gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio.txt
@@ -155,29 +155,39 @@  gpio-controller@00000000 {
 	ngpios = <18>;
 }
 
-The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism
-providing automatic GPIO request and configuration as part of the
-gpio-controller's driver probe function.
+The GPIO chip may contain GPIO definitions. These define properties for single
+GPIOs of this controller.
 
-Each GPIO hog definition is represented as a child node of the GPIO controller.
+There are two types of GPIO definitions:
+
+- GPIO hogging is a mechanism providing automatic GPIO request and
+  configuration as part of the gpio-controller driver's probe function. The
+  GPIO is held until the gpio-controller is removed.
+- GPIO initialization provides an automatic initialization to known save
+  values. The GPIO can be used normally afterwards.
+
+Each GPIO definition is represented as a child node of the GPIO controller.
 Required properties:
-- gpio-hog:   A property specifying that this child node represent a GPIO hog.
 - gpios:      Store the GPIO information (id, flags, ...). Shall contain the
 	      number of cells specified in its parent node (GPIO controller
 	      node).
-Only one of the following properties scanned in the order shown below.
-This means that when multiple properties are present they will be searched
-in the order presented below and the first match is taken as the intended
-configuration.
+
+Optional properties:
+- line-name:  The GPIO label name. If not present the node name is used.
+
+ The following two options are mutually exclusive. One of them should be
+ specified, but not both:
+- gpio-hog:   A property specifying that this child node represent a GPIO hog.
+- gpio-initval: This GPIO should be initialized to the specified configuration.
+
+ The following three options are mutually exclusive. They change the setting of
+ the GPIO pin. One of them should be specified:
 - input:      A property specifying to set the GPIO direction as input.
 - output-low  A property specifying to set the GPIO direction as output with
 	      the value low.
 - output-high A property specifying to set the GPIO direction as output with
 	      the value high.
 
-Optional properties:
-- line-name:  The GPIO label name. If not present the node name is used.
-
 Example of two SOC GPIO banks defined as gpio-controller nodes:
 
 	qe_pio_a: gpio-controller@1400 {