Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1355920 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH V2 5/5] gpio: of: Add support to have multiple gpios in gpio-hog Laxman Dewangan <ldewangan@nvidia.com> - 2016-03-11 15:00 +0100
    Re: [PATCH V2 5/5] gpio: of: Add support to have multiple gpios in gpio-hog Linus Walleij <linus.walleij@linaro.org> - 2016-03-15 15:30 +0100
      Re: [PATCH V2 5/5] gpio: of: Add support to have multiple gpios in  gpio-hog Laxman Dewangan <ldewangan@nvidia.com> - 2016-03-16 12:40 +0100

#1355920 — [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 <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

[toc] | [next] | [standalone]

#1358066

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

[toc] | [prev] | [next] | [standalone]

#1358906 — 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 <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...)

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web