[PATCH V2 0/5] gpio: of: Add error handling and support for multiple gpio in gpio-hog

[PATCH V2 0/5] gpio: of: Add error handling and support for multiple gpio in gpio-hog

[Laxman Dewangan]

This series enhance the error print by adding error number in print message,
handle the error if gpio_hogd() fails and returns error to caller, and 
add the support of multiple GPIOs to be passed from property "gpios" under
gpio hogd node.

Changes from V1:
- revert the chnage of error message.
- Collected acks/reviewed by.
- Added label for name in gpio hog node.
- Reseeunce to have dt doc before driver change.

Laxman Dewangan (5):
  gpio: of: Scan available child node for gpio-hog
  gpio: gpiolib: Print error number if gpio hog failed
  gpio: of: Return error if gpio hog configuration failed
  gpio: DT: Rephrase "gpios" of hog node to support multiple gpios
  gpio: of: Add support to have multiple gpios in gpio-hog

 Documentation/devicetree/bindings/gpio/gpio.txt | 11 +++-
 drivers/gpio/gpiolib-of.c                       | 86 +++++++++++++++++++------
 drivers/gpio/gpiolib.c                          | 11 ++--
 3 files changed, 79 insertions(+), 29 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 1/5] gpio: of: Scan available child node for gpio-hog

[Laxman Dewangan]

Look for child node which are available when iterating for
gpio hog node for request/set GPIO initial configuration
during OF gpio chip registration.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Reviewed-by: Thierry Reding <treding@nvidia.com>

---
Changes from V1:
- Add Thierey's acks.
---
 drivers/gpio/gpiolib-of.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 42a4bb7..f2ba1a4 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -210,7 +210,7 @@ static void of_gpiochip_scan_gpios(struct gpio_chip *chip)
 	enum gpio_lookup_flags lflags;
 	enum gpiod_flags dflags;
 
-	for_each_child_of_node(chip->of_node, np) {
+	for_each_available_child_of_node(chip->of_node, np) {
 		if (!of_property_read_bool(np, "gpio-hog"))
 			continue;
 
-- 
2.1.4

Re: [PATCH V2 1/5] gpio: of: Scan available child node for gpio-hog

[Linus Walleij]

On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:

> Look for child node which are available when iterating for
> gpio hog node for request/set GPIO initial configuration
> during OF gpio chip registration.
>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> Reviewed-by: Thierry Reding <treding@nvidia.com>

So if I understand the patch correctly all it really does is make
it possible to set status = "disabled"; in the hog nodes, and
then they will not be applied?

That should be stated in the commit message.

Yours,
Linus Walleij

[PATCH V2 2/5] gpio: gpiolib: Print error number if gpio hog failed

[Laxman Dewangan]

Print the error number of GPIO hog failed during its configurations.
This helps in identifying the failure without instrumenting the code.

Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

---
Changes from V1:
- Keep originality of error message, just add the error number.
- It makes line to be >80 for string and understood that it is
  fine for string on some cases.
---
 drivers/gpio/gpiolib.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 1741ef4..b682d73 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2680,15 +2680,16 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
 
 	local_desc = gpiochip_request_own_desc(chip, hwnum, name);
 	if (IS_ERR(local_desc)) {
-		pr_err("requesting hog GPIO %s (chip %s, offset %d) failed\n",
-		       name, chip->label, hwnum);
-		return PTR_ERR(local_desc);
+		status = PTR_ERR(local_desc);
+		pr_err("requesting hog GPIO %s (chip %s, offset %d) failed, %d\n",
+		       name, chip->label, hwnum, status);
+		return status;
 	}
 
 	status = gpiod_configure_flags(desc, name, dflags);
 	if (status < 0) {
-		pr_err("setup of hog GPIO %s (chip %s, offset %d) failed\n",
-		       name, chip->label, hwnum);
+		pr_err("setup of hog GPIO %s (chip %s, offset %d) failed, %d\n",
+		       name, chip->label, hwnum, status);
 		gpiochip_free_own_desc(desc);
 		return status;
 	}
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 2/5] gpio: gpiolib: Print error number if gpio hog failed

[Linus Walleij]

On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:

> Print the error number of GPIO hog failed during its configurations.
> This helps in identifying the failure without instrumenting the code.
>
> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Looks good. (Will apply after the merge window.)

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

[PATCH V2 3/5] gpio: of: Return error if gpio hog configuration failed

[Laxman Dewangan]

If GPIO hog configuration failed while adding OF based
gpiochip() then return the error instead of ignoring it.

This helps of properly handling the gpio driver dependency.

When adding the gpio hog nodes for NVIDIA's Tegra210 platforms,
the gpio_hogd() fails with EPROBE_DEFER because pinctrl is not
ready at this time and gpio_request() for Tegra GPIO driver
returns error. The error was not causing the Tegra GPIO driver
to fail as the error was getting ignored.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Cc: Benoit Parrot <bparrot@ti.com>
Cc: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Thierry Reding <treding@nvidia.com>

---
Changes from V1:
- Add Thierry's ack.
---
 drivers/gpio/gpiolib-of.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index f2ba1a4..d81dbd8 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -201,14 +201,16 @@ static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
  *
  * This is only used by of_gpiochip_add to request/set GPIO initial
  * configuration.
+ * It retures error if it fails otherwise 0 on success.
  */
-static void of_gpiochip_scan_gpios(struct gpio_chip *chip)
+static int of_gpiochip_scan_gpios(struct gpio_chip *chip)
 {
 	struct gpio_desc *desc = NULL;
 	struct device_node *np;
 	const char *name;
 	enum gpio_lookup_flags lflags;
 	enum gpiod_flags dflags;
+	int ret;
 
 	for_each_available_child_of_node(chip->of_node, np) {
 		if (!of_property_read_bool(np, "gpio-hog"))
@@ -218,9 +220,12 @@ static void of_gpiochip_scan_gpios(struct gpio_chip *chip)
 		if (IS_ERR(desc))
 			continue;
 
-		if (gpiod_hog(desc, name, lflags, dflags))
-			continue;
+		ret = gpiod_hog(desc, name, lflags, dflags);
+		if (ret < 0)
+			return ret;
 	}
+
+	return 0;
 }
 
 /**
@@ -442,9 +447,7 @@ int of_gpiochip_add(struct gpio_chip *chip)
 
 	of_node_get(chip->of_node);
 
-	of_gpiochip_scan_gpios(chip);
-
-	return 0;
+	return of_gpiochip_scan_gpios(chip);
 }
 
 void of_gpiochip_remove(struct gpio_chip *chip)
-- 
2.1.4

Re: [PATCH V2 3/5] gpio: of: Return error if gpio hog configuration failed

[Linus Walleij]

On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:

> If GPIO hog configuration failed while adding OF based
> gpiochip() then return the error instead of ignoring it.
>
> This helps of properly handling the gpio driver dependency.
>
> When adding the gpio hog nodes for NVIDIA's Tegra210 platforms,
> the gpio_hogd() fails with EPROBE_DEFER because pinctrl is not
> ready at this time and gpio_request() for Tegra GPIO driver
> returns error. The error was not causing the Tegra GPIO driver
> to fail as the error was getting ignored.
>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> Cc: Benoit Parrot <bparrot@ti.com>
> Cc: Alexandre Courbot <acourbot@nvidia.com>
> Reviewed-by: Thierry Reding <treding@nvidia.com>

Oops is this something I should apply for fixes and tag
for stable?

Yours,
Linus Walleij

Re: [PATCH V2 3/5] gpio: of: Return error if gpio hog configuration failed

[Laxman Dewangan]

On Tuesday 15 March 2016 07:42 PM, Linus Walleij wrote:
> On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>
>> If GPIO hog configuration failed while adding OF based
>> gpiochip() then return the error instead of ignoring it.
>>
>> This helps of properly handling the gpio driver dependency.
>>
>> When adding the gpio hog nodes for NVIDIA's Tegra210 platforms,
>> the gpio_hogd() fails with EPROBE_DEFER because pinctrl is not
>> ready at this time and gpio_request() for Tegra GPIO driver
>> returns error. The error was not causing the Tegra GPIO driver
>> to fail as the error was getting ignored.
>>
>> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> Cc: Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org>
>> Cc: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> Reviewed-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Oops is this something I should apply for fixes and tag
> for stable?
>

I dont think that it is needed for stable as this is newly added feature.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 3/5] gpio: of: Return error if gpio hog configuration failed

[Linus Walleij]

On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:

> If GPIO hog configuration failed while adding OF based
> gpiochip() then return the error instead of ignoring it.
>
> This helps of properly handling the gpio driver dependency.
>
> When adding the gpio hog nodes for NVIDIA's Tegra210 platforms,
> the gpio_hogd() fails with EPROBE_DEFER because pinctrl is not
> ready at this time and gpio_request() for Tegra GPIO driver
> returns error. The error was not causing the Tegra GPIO driver
> to fail as the error was getting ignored.
>
> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Cc: Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org>
> Cc: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Rebased and applied this patch FWIW.
This is a fair and square bug fix so need to go in
no matter what happens with the hog patches.

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

Re: [PATCH V2 3/5] gpio: of: Return error if gpio hog configuration failed

[Laxman Dewangan]

On Wednesday 13 April 2016 06:13 PM, Linus Walleij wrote:
> On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>
>> If GPIO hog configuration failed while adding OF based
>> gpiochip() then return the error instead of ignoring it.
>>
>> This helps of properly handling the gpio driver dependency.
>>
>> When adding the gpio hog nodes for NVIDIA's Tegra210 platforms,
>> the gpio_hogd() fails with EPROBE_DEFER because pinctrl is not
>> ready at this time and gpio_request() for Tegra GPIO driver
>> returns error. The error was not causing the Tegra GPIO driver
>> to fail as the error was getting ignored.
>>
>> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> Cc: Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org>
>> Cc: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> Reviewed-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Rebased and applied this patch FWIW.
> This is a fair and square bug fix so need to go in
> no matter what happens with the hog patches.
>

Thank you very much for accepting the patch.
I think 1/5 and 2/5 is also fine as there is no more comment on this.


However, 4/5 and 5/5 needs further discussion.

Please let me know if I need to send 1/5 and 2/5 here.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 3/5] gpio: of: Return error if gpio hog configuration failed

[Linus Walleij]

On Wed, Apr 13, 2016 at 2:54 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> On Wednesday 13 April 2016 06:13 PM, Linus Walleij wrote:

> Thank you very much for accepting the patch.
> I think 1/5 and 2/5 is also fine as there is no more comment on this.

I have applied them now.

I had a comment on 1/5, I wanted a confirmation that
it was done to be able to set status = "disabled" in the
device node.

I assume this is true so now I added that to the commit
blurb.

Yours,
Linus Walleij

[PATCH V2 4/5] gpio: DT: Rephrase "gpios" of hog node to support multiple gpios

[Laxman Dewangan]

The property "gpios" of GPIO hog node support the multiple GPIO entries.
Rephrase the details of this property for this new support.

Add details of new property "label" for GPIO label name.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

---
Changes from V1:
- Add details for the new property "label".
- Resequence series to make this as 4/5 which was 5/5.
---
 Documentation/devicetree/bindings/gpio/gpio.txt | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
index 069cdf6..6270e2d 100644
--- a/Documentation/devicetree/bindings/gpio/gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio.txt
@@ -162,9 +162,9 @@ gpio-controller's driver probe function.
 Each GPIO hog 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).
+- gpios:      Store the GPIO information (id, flags, ...). Multiple GPIOs are
+	      possible to list here. Shall contain the number of cells
+	      specified in its parent node (GPIO controller node) per GPIOs.
 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
@@ -177,6 +177,11 @@ configuration.
 
 Optional properties:
 - line-name:  The GPIO label name. If not present the node name is used.
+- label:      The GPIO lable name. This can have multiple string for GPIO
+	      label names to match with the GPIOs in "gpios" properties.
+	      If line-name is prosent than name is taken from line-name. If
+	      it is not then the name will be taken from label. If both are
+	      not available then node name is used for GPIO label name.
 
 Example of two SOC GPIO banks defined as gpio-controller nodes:
 
-- 
2.1.4

Re: [PATCH V2 4/5] gpio: DT: Rephrase "gpios" of hog node to support multiple gpios

[Stephen Warren]

On 03/11/2016 06:43 AM, Laxman Dewangan wrote:
> The property "gpios" of GPIO hog node support the multiple GPIO entries.
> Rephrase the details of this property for this new support.
>
> Add details of new property "label" for GPIO label name.

> diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt

>   Optional properties:
>   - line-name:  The GPIO label name. If not present the node name is used.
> +- label:      The GPIO lable name. This can have multiple string for GPIO
> +	      label names to match with the GPIOs in "gpios" properties.
> +	      If line-name is prosent than name is taken from line-name. If
> +	      it is not then the name will be taken from label. If both are
> +	      not available then node name is used for GPIO label name.

Why are there two properties for the same thing? Why not just allow 
line-name to have multiple entries instead of introducing a new property?

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 4/5] gpio: DT: Rephrase "gpios" of hog node to support multiple gpios

[Laxman Dewangan]

On Monday 14 March 2016 10:01 PM, Stephen Warren wrote:
> On 03/11/2016 06:43 AM, Laxman Dewangan wrote:
>> The property "gpios" of GPIO hog node support the multiple GPIO entries.
>> Rephrase the details of this property for this new support.
>>
>> Add details of new property "label" for GPIO label name.
>
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt 
>> b/Documentation/devicetree/bindings/gpio/gpio.txt
>
>>   Optional properties:
>>   - line-name:  The GPIO label name. If not present the node name is 
>> used.
>> +- label:      The GPIO lable name. This can have multiple string for 
>> GPIO
>> +          label names to match with the GPIOs in "gpios" properties.
>> +          If line-name is prosent than name is taken from line-name. If
>> +          it is not then the name will be taken from label. If both are
>> +          not available then node name is used for GPIO label name.
>
> Why are there two properties for the same thing? Why not just allow 
> line-name to have multiple entries instead of introducing a new property?
>

We can use the lin-names also but per disucssion on the patch V1 of
     gpio: of: Add support to have multiple gpios in gpio-hog

Markus suggested the discussion about the discussion 
(https://lkml.org/lkml/2016/3/10/194):
     "Device tree binding documentation for gpio-switch"


and on that, label is used.  Also for names, "label" is going to very 
common.

So I added new property "label" to support multiple names.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 4/5] gpio: DT: Rephrase "gpios" of hog node to support multiple gpios

[Stephen Warren]

On 03/15/2016 12:37 AM, Laxman Dewangan wrote:
>
> On Monday 14 March 2016 10:01 PM, Stephen Warren wrote:
>> On 03/11/2016 06:43 AM, Laxman Dewangan wrote:
>>> The property "gpios" of GPIO hog node support the multiple GPIO entries.
>>> Rephrase the details of this property for this new support.
>>>
>>> Add details of new property "label" for GPIO label name.
>>
>>> diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt
>>> b/Documentation/devicetree/bindings/gpio/gpio.txt
>>
>>>   Optional properties:
>>>   - line-name:  The GPIO label name. If not present the node name is
>>> used.
>>> +- label:      The GPIO lable name. This can have multiple string for
>>> GPIO
>>> +          label names to match with the GPIOs in "gpios" properties.
>>> +          If line-name is prosent than name is taken from line-name. If
>>> +          it is not then the name will be taken from label. If both are
>>> +          not available then node name is used for GPIO label name.
>>
>> Why are there two properties for the same thing? Why not just allow
>> line-name to have multiple entries instead of introducing a new property?
>>
>
> We can use the lin-names also but per disucssion on the patch V1 of
>      gpio: of: Add support to have multiple gpios in gpio-hog
>
> Markus suggested the discussion about the discussion
> (https://lkml.org/lkml/2016/3/10/194):
>      "Device tree binding documentation for gpio-switch"
>
>
> and on that, label is used.  Also for names, "label" is going to very
> common.
>
> So I added new property "label" to support multiple names.

It makes sense to standardize on a common name for new bindings, but 
this binding has already picked a name. It'd be much simpler for anyone 
looking at the binding (and backwards-compatibility) to just stick with 
it. We have to support the old name forever no matter what. Supporting 
two different names will just be confusing.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 4/5] gpio: DT: Rephrase "gpios" of hog node to support multiple gpios

[Linus Walleij]

On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:

> The property "gpios" of GPIO hog node support the multiple GPIO entries.
> Rephrase the details of this property for this new support.
>
> Add details of new property "label" for GPIO label name.
>
> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Also Rob H has expressed that he prefers "label" for this kind of
stuff.

So instead of adding it as another optional property, add it above
label-name, declare line-name as deprecated (and also mention
that it does not support an array). Maybe move under a separate
heading "Deprecated optional properties" or something.

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

[PATCH V2 5/5] gpio: of: Add support to have multiple gpios in gpio-hog

[Laxman Dewangan]

The child node for gpio hogs under gpio controller's node
provide the mechanism to automatic GPIO request and
configuration as part of the gpio-controller's driver
probe function.

Currently, property "gpio" takes one gpios for such
configuration. Add support to have multiple GPIOs in
this property so that multiple GPIOs of gpio-controller
can be configured by this mechanism with one child node.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Cc: Benoit Parrot <bparrot@ti.com>
Cc: Alexandre Courbot <acourbot@nvidia.com>

---
Changes from V1:
- Add "labels" property for GPIO label names.
---
 drivers/gpio/gpiolib-of.c | 77 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 59 insertions(+), 18 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index d81dbd8..47a514d 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -118,6 +118,21 @@ int of_get_named_gpio_flags(struct device_node *np, const char *list_name,
 }
 EXPORT_SYMBOL(of_get_named_gpio_flags);
 
+static int of_gpio_get_gpio_cells_size(struct device_node *chip_np)
+{
+	u32 ncells;
+	int ret;
+
+	ret = of_property_read_u32(chip_np, "#gpio-cells", &ncells);
+	if (ret)
+		return ret;
+
+	if (ncells > MAX_PHANDLE_ARGS)
+		return -EINVAL;
+
+	return ncells;
+}
+
 /**
  * of_parse_own_gpio() - Get a GPIO hog descriptor, names and flags for GPIO API
  * @np:		device node to get GPIO from
@@ -131,6 +146,7 @@ EXPORT_SYMBOL(of_get_named_gpio_flags);
  */
 static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
 					   const char **name,
+					   int gpio_index,
 					   enum gpio_lookup_flags *lflags,
 					   enum gpiod_flags *dflags)
 {
@@ -139,8 +155,8 @@ static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
 	struct gg_data gg_data = {
 		.flags = &xlate_flags,
 	};
-	u32 tmp;
-	int i, ret;
+	int ncells;
+	int i, start_index, ret;
 
 	chip_np = np->parent;
 	if (!chip_np)
@@ -150,18 +166,17 @@ static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
 	*lflags = 0;
 	*dflags = 0;
 
-	ret = of_property_read_u32(chip_np, "#gpio-cells", &tmp);
-	if (ret)
-		return ERR_PTR(ret);
+	ncells = of_gpio_get_gpio_cells_size(chip_np);
+	if (ncells < 0)
+		return ERR_PTR(ncells);
 
-	if (tmp > MAX_PHANDLE_ARGS)
-		return ERR_PTR(-EINVAL);
+	start_index = ncells * gpio_index;
 
-	gg_data.gpiospec.args_count = tmp;
+	gg_data.gpiospec.args_count = ncells;
 	gg_data.gpiospec.np = chip_np;
-	for (i = 0; i < tmp; i++) {
-		ret = of_property_read_u32_index(np, "gpios", i,
-					   &gg_data.gpiospec.args[i]);
+	for (i = 0; i < ncells; i++) {
+		ret = of_property_read_u32_index(np, "gpios", start_index + i,
+						 &gg_data.gpiospec.args[i]);
 		if (ret)
 			return ERR_PTR(ret);
 	}
@@ -189,9 +204,16 @@ static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
 		return ERR_PTR(-EINVAL);
 	}
 
-	if (name && of_property_read_string(np, "line-name", name))
-		*name = np->name;
+	if (!name)
+		goto out;
 
+	ret = of_property_read_string(np, "line-name", name);
+	if (ret)
+		ret = of_property_read_string_index(np, "label", gpio_index,
+						    name);
+	if (ret)
+		*name = np->name;
+out:
 	return gg_data.out_gpio;
 }
 
@@ -211,18 +233,37 @@ static int of_gpiochip_scan_gpios(struct gpio_chip *chip)
 	enum gpio_lookup_flags lflags;
 	enum gpiod_flags dflags;
 	int ret;
+	int i, ncells, ngpios;
+
+	ncells = of_gpio_get_gpio_cells_size(chip->of_node);
+	if (ncells < 0)
+		return 0;
 
 	for_each_available_child_of_node(chip->of_node, np) {
 		if (!of_property_read_bool(np, "gpio-hog"))
 			continue;
 
-		desc = of_parse_own_gpio(np, &name, &lflags, &dflags);
-		if (IS_ERR(desc))
+		ngpios = of_property_count_u32_elems(np, "gpios");
+		if (ngpios < 0)
 			continue;
 
-		ret = gpiod_hog(desc, name, lflags, dflags);
-		if (ret < 0)
-			return ret;
+		if (ngpios % ncells) {
+			dev_warn(chip->parent, "Invalid GPIO entries at %s\n",
+				 np->name);
+			continue;
+		}
+
+		ngpios /= ncells;
+		for (i = 0; i < ngpios; i++) {
+			desc = of_parse_own_gpio(np, &name, i,
+						 &lflags, &dflags);
+			if (IS_ERR(desc))
+				continue;
+
+			ret = gpiod_hog(desc, name, lflags, dflags);
+			if (ret < 0)
+				return ret;
+		}
 	}
 
 	return 0;
-- 
2.1.4

Re: [PATCH V2 5/5] gpio: of: Add support to have multiple gpios in gpio-hog

[Linus Walleij]

On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:

> The child node for gpio hogs under gpio controller's node
> provide the mechanism to automatic GPIO request and
> configuration as part of the gpio-controller's driver
> probe function.
>
> Currently, property "gpio" takes one gpios for such
> configuration. Add support to have multiple GPIOs in
> this property so that multiple GPIOs of gpio-controller
> can be configured by this mechanism with one child node.
>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> Cc: Benoit Parrot <bparrot@ti.com>
> Cc: Alexandre Courbot <acourbot@nvidia.com>
>
> ---
> Changes from V1:
> - Add "labels" property for GPIO label names.

Actually it's just "label" as seen from the code and the binding.
Though it would make sense to have labels (pluralis) as it can be more
than one and accompanies "gpios" which is plural.

Rob: what is the pattern here?

(Grep the existing bindings to check how multiple labels are handled
in other subsystems...)

(...)
> -       if (name && of_property_read_string(np, "line-name", name))
> -               *name = np->name;
> +       if (!name)
> +               goto out;
>
> +       ret = of_property_read_string(np, "line-name", name);
> +       if (ret)
> +               ret = of_property_read_string_index(np, "label", gpio_index,
> +                                                   name);
> +       if (ret)
> +               *name = np->name;

This looks to me like if "line-name" is specified, all lines will get the
same name if gpios contain more than one item. Is this what we want?

Yours,
Linus Walleij

Re: [PATCH V2 5/5] gpio: of: Add support to have multiple gpios in gpio-hog

[Laxman Dewangan]

On Tuesday 15 March 2016 07:51 PM, Linus Walleij wrote:
> On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>
>> The child node for gpio hogs under gpio controller's node
>> provide the mechanism to automatic GPIO request and
>> configuration as part of the gpio-controller's driver
>> probe function.
>>
>> Currently, property "gpio" takes one gpios for such
>> configuration. Add support to have multiple GPIOs in
>> this property so that multiple GPIOs of gpio-controller
>> can be configured by this mechanism with one child node.
>>
>> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> Cc: Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org>
>> Cc: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>
>> ---
>> Changes from V1:
>> - Add "labels" property for GPIO label names.
> Actually it's just "label" as seen from the code and the binding.
> Though it would make sense to have labels (pluralis) as it can be more
> than one and accompanies "gpios" which is plural.
>
> Rob: what is the pattern here?
>
> (Grep the existing bindings to check how multiple labels are handled
> in other subsystems...)

No property found for "labels" in the bindings folder. However, "label" 
is used for single string.

If Rob is fine then we can go with "labels" to start something new..


> (...)
>> -       if (name && of_property_read_string(np, "line-name", name))
>> -               *name = np->name;
>> +       if (!name)
>> +               goto out;
>>
>> +       ret = of_property_read_string(np, "line-name", name);
>> +       if (ret)
>> +               ret = of_property_read_string_index(np, "label", gpio_index,
>> +                                                   name);
>> +       if (ret)
>> +               *name = np->name;
> This looks to me like if "line-name" is specified, all lines will get the
> same name if gpios contain more than one item. Is this what we want?
yaah, line-name is deprecated property and so not adding array string 
here. Array string will be supported with label only. I did not add 
print as deprecated property to avoid noise in log.

We will be hardly have "labels" and "line-names" together.



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html