2016-03-11 13:56:37

by Laxman Dewangan

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

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


2016-03-11 13:56:49

by Laxman Dewangan

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

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 <[email protected]>
Reviewed-by: Thierry Reding <[email protected]>

---
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

2016-03-11 13:57:00

by Laxman Dewangan

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

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 <[email protected]>

---
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

2016-03-11 13:57:05

by Laxman Dewangan

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

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 <[email protected]>
Cc: Benoit Parrot <[email protected]>
Cc: Alexandre Courbot <[email protected]>
Reviewed-by: Thierry Reding <[email protected]>

---
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

2016-03-11 13:57:31

by Laxman Dewangan

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

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 <[email protected]>
Cc: Benoit Parrot <[email protected]>
Cc: Alexandre Courbot <[email protected]>

---
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

2016-03-11 13:58:20

by Laxman Dewangan

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

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 <[email protected]>

---
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

2016-03-14 16:31:21

by Stephen Warren

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

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?

2016-03-15 06:50:53

by Laxman Dewangan

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


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.

2016-03-15 14:09:48

by Linus Walleij

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

On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <[email protected]> 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 <[email protected]>
> Reviewed-by: Thierry Reding <[email protected]>

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

2016-03-15 14:11:40

by Linus Walleij

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

On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <[email protected]> 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 <[email protected]>

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

Yours,
Linus Walleij

2016-03-15 14:13:04

by Linus Walleij

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

On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <[email protected]> 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 <[email protected]>
> Cc: Benoit Parrot <[email protected]>
> Cc: Alexandre Courbot <[email protected]>
> Reviewed-by: Thierry Reding <[email protected]>

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

Yours,
Linus Walleij

2016-03-15 14:16:45

by Linus Walleij

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

On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <[email protected]> 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 <[email protected]>

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

2016-03-15 14:21:46

by Linus Walleij

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

On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <[email protected]> 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 <[email protected]>
> Cc: Benoit Parrot <[email protected]>
> Cc: Alexandre Courbot <[email protected]>
>
> ---
> 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

2016-03-15 15:40:22

by Stephen Warren

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

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.

2016-03-16 11:23:54

by Laxman Dewangan

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


On Tuesday 15 March 2016 07:42 PM, Linus Walleij wrote:
> On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <[email protected]> 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 <[email protected]>
>> Cc: Benoit Parrot <[email protected]>
>> Cc: Alexandre Courbot <[email protected]>
>> Reviewed-by: Thierry Reding <[email protected]>
> 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.

2016-03-16 11:31:42

by Laxman Dewangan

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


On Tuesday 15 March 2016 07:51 PM, Linus Walleij wrote:
> On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <[email protected]> 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 <[email protected]>
>> Cc: Benoit Parrot <[email protected]>
>> Cc: Alexandre Courbot <[email protected]>
>>
>> ---
>> 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.



2016-04-13 12:43:11

by Linus Walleij

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

On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <[email protected]> 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 <[email protected]>
> Cc: Benoit Parrot <[email protected]>
> Cc: Alexandre Courbot <[email protected]>
> Reviewed-by: Thierry Reding <[email protected]>

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

2016-04-13 13:05:24

by Laxman Dewangan

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


On Wednesday 13 April 2016 06:13 PM, Linus Walleij wrote:
> On Fri, Mar 11, 2016 at 2:43 PM, Laxman Dewangan <[email protected]> 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 <[email protected]>
>> Cc: Benoit Parrot <[email protected]>
>> Cc: Alexandre Courbot <[email protected]>
>> Reviewed-by: Thierry Reding <[email protected]>
> 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.

2016-04-14 12:52:18

by Linus Walleij

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

On Wed, Apr 13, 2016 at 2:54 PM, Laxman Dewangan <[email protected]> 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