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

Groups > linux.kernel > #1352946 > unrolled thread

[PATCH 4/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 4/5] gpio: of: Add support to have multiple gpios in gpio-hog Laxman Dewangan <ldewangan@nvidia.com> - 2016-03-08 13:20 +0100
    Re: [PATCH 4/5] gpio: of: Add support to have multiple gpios in  gpio-hog Markus Pargmann <mpa@pengutronix.de> - 2016-03-09 07:30 +0100
      Re: [PATCH 4/5] gpio: of: Add support to have multiple gpios in gpio-hog Laxman Dewangan <ldewangan@nvidia.com> - 2016-03-09 14:40 +0100
        Re: [PATCH 4/5] gpio: of: Add support to have multiple gpios in  gpio-hog Stephen Warren <swarren@wwwdotorg.org> - 2016-03-09 18:20 +0100
          Re: [PATCH 4/5] gpio: of: Add support to have multiple gpios in gpio-hog Laxman Dewangan <ldewangan@nvidia.com> - 2016-03-10 08:30 +0100
            Re: [PATCH 4/5] gpio: of: Add support to have multiple gpios in gpio-hog Markus Pargmann <mpa@pengutronix.de> - 2016-03-10 12:20 +0100
              Re: [PATCH 4/5] gpio: of: Add support to have multiple gpios in gpio-hog Laxman Dewangan <ldewangan@nvidia.com> - 2016-03-10 13:10 +0100
                Re: [PATCH 4/5] gpio: of: Add support to have multiple gpios in  gpio-hog Rob Herring <robh@kernel.org> - 2016-03-17 16:50 +0100

#1352946 — [PATCH 4/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>
---
 drivers/gpio/gpiolib-of.c | 64 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 49 insertions(+), 15 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index d81dbd8..0e4e8fd 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,17 +166,16 @@ 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,
+	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);
@@ -211,18 +226,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;
+
+		if (ngpios % ncells) {
+			dev_warn(chip->parent,
+				"GPIOs entries are not proper in gpios\n");
 			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;
+			ret = gpiod_hog(desc, name, lflags, dflags);
+			if (ret < 0)
+				return ret;
+		}
 	}
 
 	return 0;
-- 
2.1.4

[toc] | [next] | [standalone]

#1353859 — Re: [PATCH 4/5] gpio: of: Add support to have multiple gpios in gpio-hog

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Tue, Mar 08, 2016 at 05:32:07PM +0530, Laxman Dewangan 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.

So if I read this correctly you want to have multiple GPIOs with the
same line name? Why don't you use multiple child nodes with individual
line names?

Best Regards,

Markus

> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> Cc: Benoit Parrot <bparrot@ti.com>
> Cc: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpio/gpiolib-of.c | 64 ++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 49 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index d81dbd8..0e4e8fd 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,17 +166,16 @@ 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,
> +	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);
> @@ -211,18 +226,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;
> +
> +		if (ngpios % ncells) {
> +			dev_warn(chip->parent,
> +				"GPIOs entries are not proper in gpios\n");
>  			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;
> +			ret = gpiod_hog(desc, name, lflags, dflags);
> +			if (ret < 0)
> +				return ret;
> +		}
>  	}
>  
>  	return 0;
> -- 
> 2.1.4
> 
> --
> 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
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

#1354120

On Wednesday 09 March 2016 11:58 AM, Markus Pargmann wrote:
> * PGP Signed by an unknown key
>
> Hi,
>
> On Tue, Mar 08, 2016 at 05:32:07PM +0530, Laxman Dewangan 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.
> So if I read this correctly you want to have multiple GPIOs with the
> same line name? Why don't you use multiple child nodes with individual
> line names?
>
There is cases on which particular functional configuration needs sets 
of GPIO to set. On this case, making sub node for each GPIOs creates 
lots of sub-nodes and  add complexity on readability, usability and 
maintainability.
Example: for my board, I wanted to set GPIO H2 to input and H0 and H1 to 
be output high.
Instead of three nodes, I can have two here:
        gpio@0,6000d000 {
                wlan_input {
                        gpio-hog;
                        gpios = <TEGRA_GPIO(H, 2) 0>;
                        input;
                };

                wlan_output {
                        gpio-hog;
                        gpios = <TEGRA_GPIO(H, 0) 0 TEGRA_GPIO(H, 1) 0>;
                        output-high;
                };
        };

So here I am grouping the multiple output GPIO together.

This looks much similar if we have many GPIOs for one type of 
configurations.

Even it looks better if we have something:
        gpio@0,6000d000 {
                wlan_control {
                        gpio-hog;
                        gpios-input = <TEGRA_GPIO(H, 2) 0>;
                        gpios-output-high = <TEGRA_GPIO(H, 0) 0 
TEGRA_GPIO(H, 1) 0>;
                };
        };

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

#1354291 — Re: [PATCH 4/5] gpio: of: Add support to have multiple gpios in gpio-hog

On 03/09/2016 06:20 AM, Laxman Dewangan wrote:
>
> On Wednesday 09 March 2016 11:58 AM, Markus Pargmann wrote:
>> * PGP Signed by an unknown key
>>
>> Hi,
>>
>> On Tue, Mar 08, 2016 at 05:32:07PM +0530, Laxman Dewangan 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.
>> So if I read this correctly you want to have multiple GPIOs with the
>> same line name? Why don't you use multiple child nodes with individual
>> line names?
>>
> There is cases on which particular functional configuration needs sets
> of GPIO to set. On this case, making sub node for each GPIOs creates
> lots of sub-nodes and  add complexity on readability, usability and
> maintainability.
> Example: for my board, I wanted to set GPIO H2 to input and H0 and H1 to
> be output high.
> Instead of three nodes, I can have two here:
>         gpio@0,6000d000 {
>                 wlan_input {
>                         gpio-hog;
>                         gpios = <TEGRA_GPIO(H, 2) 0>;
>                         input;
>                 };
>
>                 wlan_output {
>                         gpio-hog;
>                         gpios = <TEGRA_GPIO(H, 0) 0 TEGRA_GPIO(H, 1) 0>;
>                         output-high;
>                 };
>         };
 >
> So here I am grouping the multiple output GPIO together.
>
> This looks much similar if we have many GPIOs for one type of
> configurations.
>
> Even it looks better if we have something:
>         gpio@0,6000d000 {
>                 wlan_control {
>                         gpio-hog;
>                         gpios-input = <TEGRA_GPIO(H, 2) 0>;
>                         gpios-output-high = <TEGRA_GPIO(H, 0) 0
> TEGRA_GPIO(H, 1) 0>;
>                 };
>         };

The problem with that is the description used when acquiring the GPIO is 
just "wlan_input", "wlan_output", or "wlan_control". There's nothing to 
indicate what those individual pins do (perhaps one is a reset signal, 
one is a regulator enable, etc.?) By requiring separate nodes for each 
GPIO, then the node name can provide a meaningful semantic 
name/description for each GPIO, which provides much more information.

If the approach in this patch is acceptable though, I think you want to 
update the description of "gpios" (in the GPIO hog definition section) 
in Documentation/devicetree/bindings/gpio/gpio.txt to mention that 
multiple GPIO entries are legal. Right now it says that property much 
contain exactly #gpio-cells, not a multiple of #gpio-cells.

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

#1354898

On Wednesday 09 March 2016 10:47 PM, Stephen Warren wrote:
> On 03/09/2016 06:20 AM, Laxman Dewangan wrote:
>>
>> On Wednesday 09 March 2016 11:58 AM, Markus Pargmann wrote:
>>> * PGP Signed by an unknown key
>>>
>>> Hi,
>>>
>>> On Tue, Mar 08, 2016 at 05:32:07PM +0530, Laxman Dewangan 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.
>>> So if I read this correctly you want to have multiple GPIOs with the
>>> same line name? Why don't you use multiple child nodes with individual
>>> line names?
>>>
>> There is cases on which particular functional configuration needs sets
>> of GPIO to set. On this case, making sub node for each GPIOs creates
>> lots of sub-nodes and  add complexity on readability, usability and
>> maintainability.
>> Example: for my board, I wanted to set GPIO H2 to input and H0 and H1 to
>> be output high.
>> Instead of three nodes, I can have two here:
>>         gpio@0,6000d000 {
>>                 wlan_input {
>>                         gpio-hog;
>>                         gpios = <TEGRA_GPIO(H, 2) 0>;
>>                         input;
>>                 };
>>
>>                 wlan_output {
>>                         gpio-hog;
>>                         gpios = <TEGRA_GPIO(H, 0) 0 TEGRA_GPIO(H, 1) 0>;
>>                         output-high;
>>                 };
>>         };
> >
>> So here I am grouping the multiple output GPIO together.
>>
>> This looks much similar if we have many GPIOs for one type of
>> configurations.
>>
>> Even it looks better if we have something:
>>         gpio@0,6000d000 {
>>                 wlan_control {
>>                         gpio-hog;
>>                         gpios-input = <TEGRA_GPIO(H, 2) 0>;
>>                         gpios-output-high = <TEGRA_GPIO(H, 0) 0
>> TEGRA_GPIO(H, 1) 0>;
>>                 };
>>         };
>
> The problem with that is the description used when acquiring the GPIO 
> is just "wlan_input", "wlan_output", or "wlan_control". There's 
> nothing to indicate what those individual pins do (perhaps one is a 
> reset signal, one is a regulator enable, etc.?) By requiring separate 
> nodes for each GPIO, then the node name can provide a meaningful 
> semantic name/description for each GPIO, which provides much more 
> information.
>

On this case, we have already property "line-name" and passed the name 
of the gpio via this property.
The property names is "line-name" which is good for one string. We can 
support other property "line-names" with multiple string per GPIO index.

line-names = "wlan-reset", "wlan-enable";


> If the approach in this patch is acceptable though, I think you want 
> to update the description of "gpios" (in the GPIO hog definition 
> section) in Documentation/devicetree/bindings/gpio/gpio.txt to mention 
> that multiple GPIO entries are legal. Right now it says that property 
> much contain exactly #gpio-cells, not a multiple of #gpio-cells.

I have 5th patch for this and will rearrange series as you suggested on 
5th patch.

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

#1355036

On Thursday 10 March 2016 12:37:32 Laxman Dewangan wrote:
> 
> On Wednesday 09 March 2016 10:47 PM, Stephen Warren wrote:
> > On 03/09/2016 06:20 AM, Laxman Dewangan wrote:
> >>
> >> On Wednesday 09 March 2016 11:58 AM, Markus Pargmann wrote:
> >>> * PGP Signed by an unknown key
> >>>
> >>> Hi,
> >>>
> >>> On Tue, Mar 08, 2016 at 05:32:07PM +0530, Laxman Dewangan 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.
> >>> So if I read this correctly you want to have multiple GPIOs with the
> >>> same line name? Why don't you use multiple child nodes with individual
> >>> line names?
> >>>
> >> There is cases on which particular functional configuration needs sets
> >> of GPIO to set. On this case, making sub node for each GPIOs creates
> >> lots of sub-nodes and  add complexity on readability, usability and
> >> maintainability.
> >> Example: for my board, I wanted to set GPIO H2 to input and H0 and H1 to
> >> be output high.
> >> Instead of three nodes, I can have two here:
> >>         gpio@0,6000d000 {
> >>                 wlan_input {
> >>                         gpio-hog;
> >>                         gpios = <TEGRA_GPIO(H, 2) 0>;
> >>                         input;
> >>                 };
> >>
> >>                 wlan_output {
> >>                         gpio-hog;
> >>                         gpios = <TEGRA_GPIO(H, 0) 0 TEGRA_GPIO(H, 1) 0>;
> >>                         output-high;
> >>                 };
> >>         };
> > >
> >> So here I am grouping the multiple output GPIO together.
> >>
> >> This looks much similar if we have many GPIOs for one type of
> >> configurations.
> >>
> >> Even it looks better if we have something:
> >>         gpio@0,6000d000 {
> >>                 wlan_control {
> >>                         gpio-hog;
> >>                         gpios-input = <TEGRA_GPIO(H, 2) 0>;
> >>                         gpios-output-high = <TEGRA_GPIO(H, 0) 0
> >> TEGRA_GPIO(H, 1) 0>;
> >>                 };
> >>         };
> >
> > The problem with that is the description used when acquiring the GPIO 
> > is just "wlan_input", "wlan_output", or "wlan_control". There's 
> > nothing to indicate what those individual pins do (perhaps one is a 
> > reset signal, one is a regulator enable, etc.?) By requiring separate 
> > nodes for each GPIO, then the node name can provide a meaningful 
> > semantic name/description for each GPIO, which provides much more 
> > information.
> >
> 
> On this case, we have already property "line-name" and passed the name 
> of the gpio via this property.
> The property names is "line-name" which is good for one string. We can 
> support other property "line-names" with multiple string per GPIO index.
> 
> line-names = "wlan-reset", "wlan-enable";

There is currently a discussion about the future bindings for subnodes in GPIO
controller nodes. Please have a look at these two mail threads:

	"Device tree binding documentation for gpio-switch"
	"gpio: of: Add support to have multiple gpios in gpio-hog"

Best Regards,

Markus

> 
> 
> > If the approach in this patch is acceptable though, I think you want 
> > to update the description of "gpios" (in the GPIO hog definition 
> > section) in Documentation/devicetree/bindings/gpio/gpio.txt to mention 
> > that multiple GPIO entries are legal. Right now it says that property 
> > much contain exactly #gpio-cells, not a multiple of #gpio-cells.
> 
> I have 5th patch for this and will rearrange series as you suggested on 
> 5th patch.
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

#1355058

On Thursday 10 March 2016 04:46 PM, Markus Pargmann wrote:
> On Thursday 10 March 2016 12:37:32 Laxman Dewangan wrote:
>> On Wednesday 09 March 2016 10:47 PM, Stephen Warren wrote:
>>> On 03/09/2016 06:20 AM, Laxman Dewangan wrote:
>>> The problem with that is the description used when acquiring the GPIO
>>> is just "wlan_input", "wlan_output", or "wlan_control". There's
>>> nothing to indicate what those individual pins do (perhaps one is a
>>> reset signal, one is a regulator enable, etc.?) By requiring separate
>>> nodes for each GPIO, then the node name can provide a meaningful
>>> semantic name/description for each GPIO, which provides much more
>>> information.
>>>
>> On this case, we have already property "line-name" and passed the name
>> of the gpio via this property.
>> The property names is "line-name" which is good for one string. We can
>> support other property "line-names" with multiple string per GPIO index.
>>
>> line-names = "wlan-reset", "wlan-enable";
> There is currently a discussion about the future bindings for subnodes in GPIO
> controller nodes. Please have a look at these two mail threads:
>
> 	"Device tree binding documentation for gpio-switch"
> 	"gpio: of: Add support to have multiple gpios in gpio-hog"

Second one is this patch only. Is it by intention?

The binding details about the gpio-switch and names are given by 
property "lable". I think property "label" is standard way of going 
forward i.e. I post similar patch for gpio-keys device name from DT 
after got review comment.

So here,  we can have the gpio names  under property "label" or "labels".


Or am I missing anything?

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

#1359972 — Re: [PATCH 4/5] gpio: of: Add support to have multiple gpios in gpio-hog

On Thu, Mar 10, 2016 at 05:23:55PM +0530, Laxman Dewangan wrote:
> 
> On Thursday 10 March 2016 04:46 PM, Markus Pargmann wrote:
> >On Thursday 10 March 2016 12:37:32 Laxman Dewangan wrote:
> >>On Wednesday 09 March 2016 10:47 PM, Stephen Warren wrote:
> >>>On 03/09/2016 06:20 AM, Laxman Dewangan wrote:
> >>>The problem with that is the description used when acquiring the GPIO
> >>>is just "wlan_input", "wlan_output", or "wlan_control". There's
> >>>nothing to indicate what those individual pins do (perhaps one is a
> >>>reset signal, one is a regulator enable, etc.?) By requiring separate
> >>>nodes for each GPIO, then the node name can provide a meaningful
> >>>semantic name/description for each GPIO, which provides much more
> >>>information.

I agree.

> >>On this case, we have already property "line-name" and passed the name
> >>of the gpio via this property.
> >>The property names is "line-name" which is good for one string. We can
> >>support other property "line-names" with multiple string per GPIO index.
> >>
> >>line-names = "wlan-reset", "wlan-enable";

Then what happens when someone wants to selectively disable gpio hogs?

status = "okay", "disabled", "okay";

While I often push things to fewer nodes and more compact descriptions, 
I don't think that is the right direction in this case.

> >There is currently a discussion about the future bindings for subnodes in GPIO
> >controller nodes. Please have a look at these two mail threads:
> >
> >	"Device tree binding documentation for gpio-switch"
> >	"gpio: of: Add support to have multiple gpios in gpio-hog"
> 
> Second one is this patch only. Is it by intention?
> 
> The binding details about the gpio-switch and names are given by property
> "lable". I think property "label" is standard way of going forward i.e. I
> post similar patch for gpio-keys device name from DT after got review
> comment.
> 
> So here,  we can have the gpio names  under property "label" or "labels".

label is standard. labels you just made up.

> Or am I missing anything?

The point is the more one off changes I see that are all inter-related, 
the less willing I am to accept any that don't consider all the cases. 
The inter-relationship here is how do we describe gpio lines that don't 
otherwise have a connection to another node and how to deal with them if 
that changes.

Rob

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web