[PATCH 1/2] Documentation: gpio: board: add flags parameter to gpiod_get*() functions

[PATCH 1/2] Documentation: gpio: board: add flags parameter to gpiod_get*() functions

[Dirk Behme]

With commit 39b2bbe3d715 ("gpio: add flags argument to gpiod_get*()
functions") the gpiod_get*() functions got a 'flags' parameter. Reflect
this in the documentation, too.

Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
---
 Documentation/gpio/board.txt | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/Documentation/gpio/board.txt b/Documentation/gpio/board.txt
index b80606d..7605773 100644
--- a/Documentation/gpio/board.txt
+++ b/Documentation/gpio/board.txt
@@ -39,11 +39,11 @@ This property will make GPIOs 15, 16 and 17 available to the driver under the
 
 	struct gpio_desc *red, *green, *blue, *power;
 
-	red = gpiod_get_index(dev, "led", 0);
-	green = gpiod_get_index(dev, "led", 1);
-	blue = gpiod_get_index(dev, "led", 2);
+	red = gpiod_get_index(dev, "led", 0, GPIOD_OUT_HIGH);
+	green = gpiod_get_index(dev, "led", 1, GPIOD_OUT_HIGH);
+	blue = gpiod_get_index(dev, "led", 2, GPIOD_OUT_HIGH);
 
-	power = gpiod_get(dev, "power");
+	power = gpiod_get(dev, "power", GPIOD_OUT_HIGH);
 
 The led GPIOs will be active-high, while the power GPIO will be active-low (i.e.
 gpiod_is_active_low(power) will be true).
@@ -142,12 +142,11 @@ The driver controlling "foo.0" will then be able to obtain its GPIOs as follows:
 
 	struct gpio_desc *red, *green, *blue, *power;
 
-	red = gpiod_get_index(dev, "led", 0);
-	green = gpiod_get_index(dev, "led", 1);
-	blue = gpiod_get_index(dev, "led", 2);
+	red = gpiod_get_index(dev, "led", 0, GPIOD_OUT_HIGH);
+	green = gpiod_get_index(dev, "led", 1, GPIOD_OUT_HIGH);
+	blue = gpiod_get_index(dev, "led", 2, GPIOD_OUT_HIGH);
 
-	power = gpiod_get(dev, "power");
-	gpiod_direction_output(power, 1);
+	power = gpiod_get(dev, "power", GPIOD_OUT_HIGH);
 
 Since the "power" GPIO is mapped as active-low, its actual signal will be 0
 after this code. Contrary to the legacy integer GPIO interface, the active-low
-- 
2.4.6

[PATCH 2/2] Documentation: gpio: board: describe the con_id parameter

[Dirk Behme]

The con_id parameter has to match the GPIO description and is automatically
extended by the GPIO suffix if not NULL. I had to look into the code to
understand this and properly find the GPIO I've been looking for, so document
this.

Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
---
 Documentation/gpio/board.txt    | 36 ++++++++++++++++++++++++++++++++++++
 Documentation/gpio/consumer.txt |  3 +++
 2 files changed, 39 insertions(+)

diff --git a/Documentation/gpio/board.txt b/Documentation/gpio/board.txt
index 7605773..eac78fc 100644
--- a/Documentation/gpio/board.txt
+++ b/Documentation/gpio/board.txt
@@ -48,6 +48,42 @@ This property will make GPIOs 15, 16 and 17 available to the driver under the
 The led GPIOs will be active-high, while the power GPIO will be active-low (i.e.
 gpiod_is_active_low(power) will be true).
 
+If in the consumer device's node the property doesn't have a <function>- prefix,
+the second parameter (con_id) of the gpiod_get*() functions has to be NULL:
+
+E.g. if in the above device tree example the GPIO array is just called 'gpios'
+
+	foo_device {
+		compatible = "acme,foo";
+		...
+		gpios = <&gpio 15 GPIO_ACTIVE_HIGH>, /* red */
+			<&gpio 16 GPIO_ACTIVE_HIGH>, /* green */
+			<&gpio 17 GPIO_ACTIVE_HIGH>; /* blue */
+	};
+
+the gpiod_get*() calls will be:
+
+	struct gpio_desc *red, *green, *blue, *power;
+
+	red = gpiod_get_index(dev, NULL, 0, GPIOD_OUT_HIGH);
+	green = gpiod_get_index(dev, NULL, 1, GPIOD_OUT_HIGH);
+	blue = gpiod_get_index(dev, NULL, 2, GPIOD_OUT_HIGH);
+
+To summarize:
+
+The con_id string parameter has to be either NULL or the <function>-prefix
+of the GPIO suffixes ("gpios" or "gpio", automatically looked up by the
+gpiod functions internally):
+
+* If the GPIO description is just named with one of the GPIO suffixes
+  ("gpios" or "gpio"), use NULL.
+* If the GPIO description is prefixed with anything, e.g. "led-gpios", use the
+  prefix without the "-" as con_id parameter (in this example "led").
+
+In case con_id is not NULL, the GPIO subsystem prefixes the GPIO suffix
+("gpios" or "gpio") with the string passed in con_id to get the resulting string
+(snprintf(... "%s-%s", con_id, gpio_suffixes[]).
+
 ACPI
 ----
 ACPI also supports function names for GPIOs in a similar fashion to DT.
diff --git a/Documentation/gpio/consumer.txt b/Documentation/gpio/consumer.txt
index 75542b9..47ce4fd 100644
--- a/Documentation/gpio/consumer.txt
+++ b/Documentation/gpio/consumer.txt
@@ -39,6 +39,9 @@ device that displays digits), an additional index argument can be specified:
 					  const char *con_id, unsigned int idx,
 					  enum gpiod_flags flags)
 
+For a more detailed description of the con_id parameter in the DeviceTree case
+see Documentation/gpio/board.txt
+
 The flags parameter is used to optionally specify a direction and initial value
 for the GPIO. Values can be:
 
-- 
2.4.6

Re: [PATCH 2/2] Documentation: gpio: board: describe the con_id parameter

[Dirk Behme]

On 25.07.2015 09:11, Dirk Behme wrote:
> The con_id parameter has to match the GPIO description and is automatically
> extended by the GPIO suffix if not NULL. I had to look into the code to
> understand this and properly find the GPIO I've been looking for, so document
> this.
>
> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
> ---
>   Documentation/gpio/board.txt    | 36 ++++++++++++++++++++++++++++++++++++
>   Documentation/gpio/consumer.txt |  3 +++
>   2 files changed, 39 insertions(+)
>
> diff --git a/Documentation/gpio/board.txt b/Documentation/gpio/board.txt
> index 7605773..eac78fc 100644
> --- a/Documentation/gpio/board.txt
> +++ b/Documentation/gpio/board.txt
> @@ -48,6 +48,42 @@ This property will make GPIOs 15, 16 and 17 available to the driver under the
>   The led GPIOs will be active-high, while the power GPIO will be active-low (i.e.
>   gpiod_is_active_low(power) will be true).
>
> +If in the consumer device's node the property doesn't have a <function>- prefix,
> +the second parameter (con_id) of the gpiod_get*() functions has to be NULL:
> +
> +E.g. if in the above device tree example the GPIO array is just called 'gpios'
> +
> +	foo_device {
> +		compatible = "acme,foo";
> +		...
> +		gpios = <&gpio 15 GPIO_ACTIVE_HIGH>, /* red */
> +			<&gpio 16 GPIO_ACTIVE_HIGH>, /* green */
> +			<&gpio 17 GPIO_ACTIVE_HIGH>; /* blue */
> +	};
> +
> +the gpiod_get*() calls will be:
> +
> +	struct gpio_desc *red, *green, *blue, *power;
> +
> +	red = gpiod_get_index(dev, NULL, 0, GPIOD_OUT_HIGH);
> +	green = gpiod_get_index(dev, NULL, 1, GPIOD_OUT_HIGH);
> +	blue = gpiod_get_index(dev, NULL, 2, GPIOD_OUT_HIGH);
> +
> +To summarize:
> +
> +The con_id string parameter has to be either NULL or the <function>-prefix
> +of the GPIO suffixes ("gpios" or "gpio", automatically looked up by the
> +gpiod functions internally):
> +
> +* If the GPIO description is just named with one of the GPIO suffixes
> +  ("gpios" or "gpio"), use NULL.
> +* If the GPIO description is prefixed with anything, e.g. "led-gpios", use the
> +  prefix without the "-" as con_id parameter (in this example "led").
> +
> +In case con_id is not NULL, the GPIO subsystem prefixes the GPIO suffix
> +("gpios" or "gpio") with the string passed in con_id to get the resulting string
> +(snprintf(... "%s-%s", con_id, gpio_suffixes[]).
> +
>   ACPI
>   ----
>   ACPI also supports function names for GPIOs in a similar fashion to DT.
> diff --git a/Documentation/gpio/consumer.txt b/Documentation/gpio/consumer.txt
> index 75542b9..47ce4fd 100644
> --- a/Documentation/gpio/consumer.txt
> +++ b/Documentation/gpio/consumer.txt
> @@ -39,6 +39,9 @@ device that displays digits), an additional index argument can be specified:
>   					  const char *con_id, unsigned int idx,
>   					  enum gpiod_flags flags)
>
> +For a more detailed description of the con_id parameter in the DeviceTree case
> +see Documentation/gpio/board.txt
> +
>   The flags parameter is used to optionally specify a direction and initial value
>   for the GPIO. Values can be:


Any further comments on this? Or could this be applied?

Best regards

Dirk

Re: [PATCH 2/2] Documentation: gpio: board: describe the con_id parameter

[Alexandre Courbot]

On Fri, Aug 21, 2015 at 2:28 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
> On 25.07.2015 09:11, Dirk Behme wrote:
>>
>> The con_id parameter has to match the GPIO description and is
>> automatically
>> extended by the GPIO suffix if not NULL. I had to look into the code to
>> understand this and properly find the GPIO I've been looking for, so
>> document
>> this.
>>
>> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
>> ---
>>   Documentation/gpio/board.txt    | 36
>> ++++++++++++++++++++++++++++++++++++
>>   Documentation/gpio/consumer.txt |  3 +++
>>   2 files changed, 39 insertions(+)
>>
>> diff --git a/Documentation/gpio/board.txt b/Documentation/gpio/board.txt
>> index 7605773..eac78fc 100644
>> --- a/Documentation/gpio/board.txt
>> +++ b/Documentation/gpio/board.txt
>> @@ -48,6 +48,42 @@ This property will make GPIOs 15, 16 and 17 available
>> to the driver under the
>>   The led GPIOs will be active-high, while the power GPIO will be
>> active-low (i.e.
>>   gpiod_is_active_low(power) will be true).
>>
>> +If in the consumer device's node the property doesn't have a <function>-
>> prefix,
>> +the second parameter (con_id) of the gpiod_get*() functions has to be
>> NULL:
>> +
>> +E.g. if in the above device tree example the GPIO array is just called
>> 'gpios'
>> +
>> +       foo_device {
>> +               compatible = "acme,foo";
>> +               ...
>> +               gpios = <&gpio 15 GPIO_ACTIVE_HIGH>, /* red */
>> +                       <&gpio 16 GPIO_ACTIVE_HIGH>, /* green */
>> +                       <&gpio 17 GPIO_ACTIVE_HIGH>; /* blue */
>> +       };
>> +
>> +the gpiod_get*() calls will be:
>> +
>> +       struct gpio_desc *red, *green, *blue, *power;
>> +
>> +       red = gpiod_get_index(dev, NULL, 0, GPIOD_OUT_HIGH);
>> +       green = gpiod_get_index(dev, NULL, 1, GPIOD_OUT_HIGH);
>> +       blue = gpiod_get_index(dev, NULL, 2, GPIOD_OUT_HIGH);
>> +
>> +To summarize:
>> +
>> +The con_id string parameter has to be either NULL or the
>> <function>-prefix
>> +of the GPIO suffixes ("gpios" or "gpio", automatically looked up by the
>> +gpiod functions internally):
>> +
>> +* If the GPIO description is just named with one of the GPIO suffixes
>> +  ("gpios" or "gpio"), use NULL.
>> +* If the GPIO description is prefixed with anything, e.g. "led-gpios",
>> use the
>> +  prefix without the "-" as con_id parameter (in this example "led").
>> +
>> +In case con_id is not NULL, the GPIO subsystem prefixes the GPIO suffix
>> +("gpios" or "gpio") with the string passed in con_id to get the resulting
>> string
>> +(snprintf(... "%s-%s", con_id, gpio_suffixes[]).
>> +
>>   ACPI
>>   ----
>>   ACPI also supports function names for GPIOs in a similar fashion to DT.
>> diff --git a/Documentation/gpio/consumer.txt
>> b/Documentation/gpio/consumer.txt
>> index 75542b9..47ce4fd 100644
>> --- a/Documentation/gpio/consumer.txt
>> +++ b/Documentation/gpio/consumer.txt
>> @@ -39,6 +39,9 @@ device that displays digits), an additional index
>> argument can be specified:
>>                                           const char *con_id, unsigned int
>> idx,
>>                                           enum gpiod_flags flags)
>>
>> +For a more detailed description of the con_id parameter in the DeviceTree
>> case
>> +see Documentation/gpio/board.txt
>> +
>>   The flags parameter is used to optionally specify a direction and
>> initial value
>>   for the GPIO. Values can be:
>
>
>
> Any further comments on this? Or could this be applied?

Using the "gpios" property is considered deprecated, so I am not sure
that is a good idea to mention this too extensively in the boards file
(people might be tempted to use that option while we clearly don't
want them to).

A precise description of the lookup method is clearly missing though.
I don't know what the right place would be for it. consumer.txt?
board.txt in a more concise (and with a big "DEPRECATED") way?

Re: [PATCH 1/2] Documentation: gpio: board: add flags parameter to gpiod_get*() functions

[Dirk Behme]

On 25.07.2015 09:11, Dirk Behme wrote:
> With commit 39b2bbe3d715 ("gpio: add flags argument to gpiod_get*()
> functions") the gpiod_get*() functions got a 'flags' parameter. Reflect
> this in the documentation, too.
>
> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
> ---
>   Documentation/gpio/board.txt | 17 ++++++++---------
>   1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/gpio/board.txt b/Documentation/gpio/board.txt
> index b80606d..7605773 100644
> --- a/Documentation/gpio/board.txt
> +++ b/Documentation/gpio/board.txt
> @@ -39,11 +39,11 @@ This property will make GPIOs 15, 16 and 17 available to the driver under the
>
>   	struct gpio_desc *red, *green, *blue, *power;
>
> -	red = gpiod_get_index(dev, "led", 0);
> -	green = gpiod_get_index(dev, "led", 1);
> -	blue = gpiod_get_index(dev, "led", 2);
> +	red = gpiod_get_index(dev, "led", 0, GPIOD_OUT_HIGH);
> +	green = gpiod_get_index(dev, "led", 1, GPIOD_OUT_HIGH);
> +	blue = gpiod_get_index(dev, "led", 2, GPIOD_OUT_HIGH);
>
> -	power = gpiod_get(dev, "power");
> +	power = gpiod_get(dev, "power", GPIOD_OUT_HIGH);
>
>   The led GPIOs will be active-high, while the power GPIO will be active-low (i.e.
>   gpiod_is_active_low(power) will be true).
> @@ -142,12 +142,11 @@ The driver controlling "foo.0" will then be able to obtain its GPIOs as follows:
>
>   	struct gpio_desc *red, *green, *blue, *power;
>
> -	red = gpiod_get_index(dev, "led", 0);
> -	green = gpiod_get_index(dev, "led", 1);
> -	blue = gpiod_get_index(dev, "led", 2);
> +	red = gpiod_get_index(dev, "led", 0, GPIOD_OUT_HIGH);
> +	green = gpiod_get_index(dev, "led", 1, GPIOD_OUT_HIGH);
> +	blue = gpiod_get_index(dev, "led", 2, GPIOD_OUT_HIGH);
>
> -	power = gpiod_get(dev, "power");
> -	gpiod_direction_output(power, 1);
> +	power = gpiod_get(dev, "power", GPIOD_OUT_HIGH);
>
>   Since the "power" GPIO is mapped as active-low, its actual signal will be 0
>   after this code. Contrary to the legacy integer GPIO interface, the active-low


Any further comments on this? Or could this be applied?


Best regards

Dirk

Re: [PATCH 2/2] Documentation: gpio: board: describe the con_id parameter

[Dirk Behme]

On 21.08.2015 05:01, Alexandre Courbot wrote:
> On Fri, Aug 21, 2015 at 2:28 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
>> On 25.07.2015 09:11, Dirk Behme wrote:
>>>
>>> The con_id parameter has to match the GPIO description and is
>>> automatically
>>> extended by the GPIO suffix if not NULL. I had to look into the code to
>>> understand this and properly find the GPIO I've been looking for, so
>>> document
>>> this.
>>>
>>> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
>>> ---
>>>    Documentation/gpio/board.txt    | 36
>>> ++++++++++++++++++++++++++++++++++++
>>>    Documentation/gpio/consumer.txt |  3 +++
>>>    2 files changed, 39 insertions(+)
>>>
>>> diff --git a/Documentation/gpio/board.txt b/Documentation/gpio/board.txt
>>> index 7605773..eac78fc 100644
>>> --- a/Documentation/gpio/board.txt
>>> +++ b/Documentation/gpio/board.txt
>>> @@ -48,6 +48,42 @@ This property will make GPIOs 15, 16 and 17 available
>>> to the driver under the
>>>    The led GPIOs will be active-high, while the power GPIO will be
>>> active-low (i.e.
>>>    gpiod_is_active_low(power) will be true).
>>>
>>> +If in the consumer device's node the property doesn't have a <function>-
>>> prefix,
>>> +the second parameter (con_id) of the gpiod_get*() functions has to be
>>> NULL:
>>> +
>>> +E.g. if in the above device tree example the GPIO array is just called
>>> 'gpios'
>>> +
>>> +       foo_device {
>>> +               compatible = "acme,foo";
>>> +               ...
>>> +               gpios = <&gpio 15 GPIO_ACTIVE_HIGH>, /* red */
>>> +                       <&gpio 16 GPIO_ACTIVE_HIGH>, /* green */
>>> +                       <&gpio 17 GPIO_ACTIVE_HIGH>; /* blue */
>>> +       };
>>> +
>>> +the gpiod_get*() calls will be:
>>> +
>>> +       struct gpio_desc *red, *green, *blue, *power;
>>> +
>>> +       red = gpiod_get_index(dev, NULL, 0, GPIOD_OUT_HIGH);
>>> +       green = gpiod_get_index(dev, NULL, 1, GPIOD_OUT_HIGH);
>>> +       blue = gpiod_get_index(dev, NULL, 2, GPIOD_OUT_HIGH);
>>> +
>>> +To summarize:
>>> +
>>> +The con_id string parameter has to be either NULL or the
>>> <function>-prefix
>>> +of the GPIO suffixes ("gpios" or "gpio", automatically looked up by the
>>> +gpiod functions internally):
>>> +
>>> +* If the GPIO description is just named with one of the GPIO suffixes
>>> +  ("gpios" or "gpio"), use NULL.
>>> +* If the GPIO description is prefixed with anything, e.g. "led-gpios",
>>> use the
>>> +  prefix without the "-" as con_id parameter (in this example "led").
>>> +
>>> +In case con_id is not NULL, the GPIO subsystem prefixes the GPIO suffix
>>> +("gpios" or "gpio") with the string passed in con_id to get the resulting
>>> string
>>> +(snprintf(... "%s-%s", con_id, gpio_suffixes[]).
>>> +
>>>    ACPI
>>>    ----
>>>    ACPI also supports function names for GPIOs in a similar fashion to DT.
>>> diff --git a/Documentation/gpio/consumer.txt
>>> b/Documentation/gpio/consumer.txt
>>> index 75542b9..47ce4fd 100644
>>> --- a/Documentation/gpio/consumer.txt
>>> +++ b/Documentation/gpio/consumer.txt
>>> @@ -39,6 +39,9 @@ device that displays digits), an additional index
>>> argument can be specified:
>>>                                            const char *con_id, unsigned int
>>> idx,
>>>                                            enum gpiod_flags flags)
>>>
>>> +For a more detailed description of the con_id parameter in the DeviceTree
>>> case
>>> +see Documentation/gpio/board.txt
>>> +
>>>    The flags parameter is used to optionally specify a direction and
>>> initial value
>>>    for the GPIO. Values can be:
>>
>>
>>
>> Any further comments on this? Or could this be applied?
>
> Using the "gpios" property is considered deprecated, so I am not sure
> that is a good idea to mention this too extensively in the boards file
> (people might be tempted to use that option while we clearly don't
> want them to).
>
> A precise description of the lookup method is clearly missing though.
> I don't know what the right place would be for it. consumer.txt?
> board.txt in a more concise (and with a big "DEPRECATED") way?

Ok, thanks!

In a previous comment there was the proposal to put it into board.txt, 
as this applies only to the device tree case and board.txt has a 
device tree section.

I'll drop the description of the deprecated part and send an update.

Best regards

Dirk

Re: [PATCH 1/2] Documentation: gpio: board: add flags parameter to gpiod_get*() functions

[Alexandre Courbot]

On Sat, Jul 25, 2015 at 4:11 PM, Dirk Behme <dirk.behme@gmail.com> wrote:
> With commit 39b2bbe3d715 ("gpio: add flags argument to gpiod_get*()
> functions") the gpiod_get*() functions got a 'flags' parameter. Reflect
> this in the documentation, too.
>
> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
> ---
>  Documentation/gpio/board.txt | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/gpio/board.txt b/Documentation/gpio/board.txt
> index b80606d..7605773 100644
> --- a/Documentation/gpio/board.txt
> +++ b/Documentation/gpio/board.txt
> @@ -39,11 +39,11 @@ This property will make GPIOs 15, 16 and 17 available to the driver under the
>
>         struct gpio_desc *red, *green, *blue, *power;
>
> -       red = gpiod_get_index(dev, "led", 0);
> -       green = gpiod_get_index(dev, "led", 1);
> -       blue = gpiod_get_index(dev, "led", 2);
> +       red = gpiod_get_index(dev, "led", 0, GPIOD_OUT_HIGH);
> +       green = gpiod_get_index(dev, "led", 1, GPIOD_OUT_HIGH);
> +       blue = gpiod_get_index(dev, "led", 2, GPIOD_OUT_HIGH);
>
> -       power = gpiod_get(dev, "power");
> +       power = gpiod_get(dev, "power", GPIOD_OUT_HIGH);
>
>  The led GPIOs will be active-high, while the power GPIO will be active-low (i.e.
>  gpiod_is_active_low(power) will be true).
> @@ -142,12 +142,11 @@ The driver controlling "foo.0" will then be able to obtain its GPIOs as follows:
>
>         struct gpio_desc *red, *green, *blue, *power;
>
> -       red = gpiod_get_index(dev, "led", 0);
> -       green = gpiod_get_index(dev, "led", 1);
> -       blue = gpiod_get_index(dev, "led", 2);
> +       red = gpiod_get_index(dev, "led", 0, GPIOD_OUT_HIGH);
> +       green = gpiod_get_index(dev, "led", 1, GPIOD_OUT_HIGH);
> +       blue = gpiod_get_index(dev, "led", 2, GPIOD_OUT_HIGH);

You are right in that the documentation needs to be updated. But since
the new code sets an output value for the 3 leds GPIO, could you also
add a short note in this paragraph to mention the fact that it will
switch the LEDs on, similarly to the explanation for the "power" GPIO
below?

Oh, and apologies for taking so long to come back to this m(__)m